Bug HAL_delay() off by 1ms

[Lama]

By coincidence I discovered a bug in HAL_delay().

__weak void HAL_Delay(uint32_t Delay)

{

 uint32_t tickstart = HAL_GetTick();

 uint32_t wait = Delay;

 /* Add a period to guaranty minimum wait */

 if (wait < HAL_MAX_DELAY)

 {

   wait++;

 }

 while((HAL_GetTick() - tickstart) < wait)

 {

 }

}

Since HAL_MAX_DELAY is 0xFFFFFFFFU you always get a 1 ms bonus. Please fix it ST this is pretty sloppy. I wonder how you test your software.

This is for MCU package STM32Cube_FW_L4_V1.13.0.

[waclawek.jan]

> you always get a 1 ms bonus

No, you don't. It depends on where within the granularity of HAL_GetTick() you call this function - it may well be that it's just about to tick when tickstart is read, and then the delay lasts almost exactly the length you requested. In other words, this ensures that the delay is *at least* of the requested length.

JW

[Tesla DeLorean]

Realistically if want better precision you start with a finer granularity time base, just saying...

[Lama]

Ok, understood, the semantic of the call is that you wait for at least the number of milliseconds specified. The granularity of the timer is 1ms.

So without the bias at an arbitrary moment the expected delay for n>0 would be: (n-0.5)±0.5ms with the bias (n+0.5)±0.5ms

However if two HAL_delay() calls are called directly after each other the second one will have a bias of 1ms.

[Tesla DeLorean]

Without the addition the minimum wait of the routine could be approaching zero.

One would really use a TIM clocking at 1 MHz and NOT interrupting, or DWT_CYCCNT to get some meaningful precision in timing..

HAL_Delay() is a blunt axe and used as such. Better than the initial attempts which didn't handle wrapping properly.

[Lama]

@Community member​  Indeed, I was using a HiRes timer, to test it I profiled HAL_Delay() and observed it's behavior that was well explained by @Community member​ 

[Zek_De]

what about if Delay is close overflow value, in this case code have to wait a lot to reach value expected

[Tesla DeLorean]

It is coded correctly to work across the 32-bit wrapping point, the code originally did not, and that's been beaten into a few people now.

start = current;

while((current - start) < delay);

[Zek_De]

I couldnt understand well.

for example let's assume about uint8_t and delay = 100 ms

uint8_t start = current(also assume this is uint8_t)

assume start = 250

while((250 - 250) < 100)

while((251 - 250) < 100) like that and number will get overflow and I have to wait more and more to catch < 100 ms

[Zek_De]

oh sorry , my fault .I handled it.