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

Avoid blocking in NTPClient::update() #163

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mlesniew
Copy link

When a NTP request is sent, it may take several milliseconds to retrieve the response. This commit changes the NTPClient::update() behaviour to asynchronous allowing a NTP request to be sent with one update() call and handle the response when it's available in another call, eliminating active waiting.

This commit also changes the NTPClient::forceUpdate() implementation to rely on the logic in NTPClient::update(). However, the behaviour of this function does not change from the API user's perspective. It is still synchronous, it only returns when all processing is complete.

This fixes issue #112.

Note that this is equivalent to what was suggested in PR #90. I decided to reimplement it, because the other PR seems to be forgotten by the author and he still hasn't signed the CLA.

When a NTP request is sent, it may take several milliseconds to retrieve
the response.  This commit changes the NTPClient::update() behaviour to
asynchronous allowing a NTP request to be sent with one update() call
and handle the response when it's available, in another call eliminating
active waiting.

This commit also changes the NTPClient::forceUpdate() implementation to
rely on the logic in NTPClient::update().  However, the behaviour of
this function does not change from the API user's perspective.  It is
still synchronous, it only returns when all processing is complete.
@CLAassistant
Copy link

CLAassistant commented Jan 28, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

Memory usage change @ 3f9957d

Board flash % RAM for global variables %
esp8266:esp8266:huzzah 💚 -64 - -64 -0.01 - -0.01 🔺 +8 - +8 +0.01 - +0.01
Click for full report table
Board examples/Advanced
flash
% examples/Advanced
RAM for global variables
% examples/Basic
flash
% examples/Basic
RAM for global variables
% examples/IsTimeSet
flash
% examples/IsTimeSet
RAM for global variables
%
esp8266:esp8266:huzzah -64 -0.01 8 0.01 -64 -0.01 8 0.01 -64 -0.01 8 0.01
Click for full report CSV
Board,examples/Advanced<br>flash,%,examples/Advanced<br>RAM for global variables,%,examples/Basic<br>flash,%,examples/Basic<br>RAM for global variables,%,examples/IsTimeSet<br>flash,%,examples/IsTimeSet<br>RAM for global variables,%
esp8266:esp8266:huzzah,-64,-0.01,8,0.01,-64,-0.01,8,0.01,-64,-0.01,8,0.01

@per1234 per1234 linked an issue Jan 28, 2022 that may be closed by this pull request
@per1234 per1234 added the topic: code Related to content of the project itself label Jan 28, 2022
@AmirHmZz
Copy link

Any updates on this?

@mlesniew
Copy link
Author

I believe this PR is ready to be merged, we're just waiting for one of the maintainers to accept it.

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