arduino-libraries / NTPClient

Connect to a NTP server
533 stars 366 forks source link

Avoid blocking in NTPClient::update() #163

Open mlesniew opened 2 years ago

mlesniew commented 2 years ago

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 https://github.com/arduino-libraries/NTPClient/pull/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.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

github-actions[bot] commented 2 years ago

Memory usage change @ 3f9957dcf14cc3d6a8250ba86f5f686b87492a0b

Board flash % RAM for global variables %
esp8266:esp8266:huzzah :green_heart: -64 - -64 -0.01 - -0.01 :small_red_triangle: +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
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 ```
AmirHmZz commented 9 months ago

Any updates on this?

mlesniew commented 9 months ago

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