TheThingsNetwork / arduino-device-lib

Arduino Library for TTN Devices
MIT License
208 stars 96 forks source link

Fix `readLine`'s `needsHardReset` condition #256

Closed frankleonrose closed 5 years ago

frankleonrose commented 5 years ago

As is, needsHardReset is never set by readLine.

If reads are failing, when attempt is finally zero and causes the while loop to exit, it is also post-decremented and becomes 255 (since it is unsigned). Hence the condition to set needsHardReset is never met.

In fact, the condition attempts<=0 is true only when the last read attempt actually succeeded. In this case, attempt was 1 and is decremented to 0 before the final call to readBytesUntil returns read>0. The while loop exits because !read is false and the value of attempt remains at 0, unchanged, because of short-circuited &&.

Use https://godbolt.org/z/000v_I as a playground to prove this to yourself.

The solution is to ignore attempts after the loop. It did its job already. The important question is whether read is zero or not. If so, we need to reset the module.

CLAassistant commented 5 years ago

CLA assistant check
All committers have signed the CLA.

frankleonrose commented 5 years ago

https://github.com/TheThingsNetwork/arduino-device-lib/issues/242

johanstokking commented 5 years ago

Thanks!