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

Re-works the update() method to avoid blocking #90

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshado
Copy link

@joshado joshado commented Jan 28, 2020

I was seeing weird issues following an unrelated failed OTA update where the NTP update() call would block the loop() method for 1024ms whilst packets timed out despite seeing replies going back to the ESP.

This PR tweaks the update() method to work asynchronously rather than blocking things to implement the timeout which sorted things out for me.

In addition, it also adds a backoff to timeout so we don't end up in a state constantly hammering the NTP server.

I'm using these changes in my project and, if you think they'll be generally useful, you're welcome to them under the same MIT license as the original work.

Cheers.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mlesniew
Copy link

@joshado, good job, the changes you submitted match what I planned to implement too, but since you've already done it I thought I could use your changes.

Looks like your PR would have been accepted already, the only thing that is missing is signing CLA as requested above. Would you mind doing that so others could use your excellent work?

@per1234 per1234 added the topic: code Related to content of the project itself label Jan 22, 2022
@per1234 per1234 linked an issue Jan 28, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

delay(10) to wait for NTP data messes interrupt routines
4 participants