-
Notifications
You must be signed in to change notification settings - Fork 80
TimeService: check connection status before network/ntp time request #317
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #317 +/- ##
=======================================
Coverage 94.87% 94.87%
=======================================
Files 27 27
Lines 1113 1113
=======================================
Hits 1056 1056
Misses 57 57 Continue to review full report at Codecov.
|
Memory usage change @ 398bef8
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
TESTS: |
Background: a user reported a crash using ArduinoIoTCloud on ESP32 .
After a bit of research we discovered the root cause was an early NTP request due to a property initialization into the setup function. In the arduino-esp32 core the network stack is initialized when the
.begin()
function is called and in case of this library this happens at the first call ofArduinoCloud.update()
This PR slightly chage the behaviour of TimeSerivice adding a connection check before starting a remote time request to the connection handler or via NTP.
Doing in this way we avoid the crash reported on esp32 core and for the other platforms to submit time request that would return invalid values.