search

pololu / vl53l0x-arduino

Pololu Arduino library for VL53L0X time-of-flight distance sensor
https://www.pololu.com/product/2490
Other
345 stars 163 forks source link

Fix rare, intermittent, endless loop in timeouts #32

Open Tarim8 opened 5 years ago

Tarim8 commented 5 years ago

Don't reduce millis() and timeout_start_ms to 16 bit as this causes wrapping every 65 seconds which can lead to endless loops from checkTimeoutExpired() if the sensor times out when the 16 bit millis() is within io_timeout of wrapping.

kevin-pololu commented 5 years ago

As long as we aren't checking for any timeouts longer than 65 seconds, it isn't necessary to use 32-bit arithmetic since, if we cast the result of the subtraction to uint16_t, it will underflow and wrap around to give the expected result. (For example, if the lower 16 bits of millis() is 200 and timeout_start_ms is 65400, 200 - 65400 produces 336 when the result is truncated to 16 bits.)

That being said, there is a bug with the code: it is casting one of the operands to 16 bits, not the result of the subtraction. This should still work fine on 8-bit processors, but it will probably give unexpected results on a 32-bit processor (where both operands will probably be promoted to 32 bits and the result will be 32 bits as well). In other words, it should be

(uint16_t)(millis() - timeout_start_ms)

instead of 

((uint16_t)millis() - timeout_start_ms)

I think this affects a lot of our other libraries as well.

swilson86 commented 5 years ago

A rare bug? I get this all the time. Easily reproduced if you put your hand in front of the sensor then point it at something like the ceiling, then your hand again. It will fail with the 65535 TIMEOUT stuck in that loop until you reboot. I hope you merge this fix soon. Thanks for the good work on the library so far!!

ghost commented 4 years ago

A rare bug? I get this all the time. Easily reproduced if you put your hand in front of the sensor then point it at something like the ceiling, then your hand again. It will fail with the 65535 TIMEOUT stuck in that loop until you reboot. I hope you merge this fix soon. Thanks for the good work on the library so far!!

I experience the same behavior consistently