Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix readLine's needsHardReset condition #256

Merged

Conversation

frankleonrose
Copy link
Contributor

@frankleonrose frankleonrose commented Jun 17, 2019

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
Copy link

CLAassistant commented Jun 17, 2019

CLA assistant check
All committers have signed the CLA.

@frankleonrose frankleonrose changed the title Fix failed attempts condition Fix readLine's needsHardReset condition Jun 17, 2019
@frankleonrose
Copy link
Contributor Author

#242

@johanstokking johanstokking merged commit e8db3d9 into TheThingsNetwork:master Jun 18, 2019
@johanstokking
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants