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

Enable fractional second timestamps (re-integration of #102) #146

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

Conversation

tickelton
Copy link

Since PR #102 seems inactive this rebases the changes from that PR to master and fixes the remaining CI issues.

Description of the original PR:

increase time accuracy to not above 100ms
    complete decimal part of NTP
    use high-freq polling
    add _current_epoc_dec and get_millis() so that ms can be known
rewrite update()
    solved overflow problem
    change update()'s return type to byte in order to carry more info
    add a not-request-too-fast feature by adding _last_fail in update()

In addition this PR fixes some spelling and white space issues of the original PR and refactors the new example to look more idiomatic as compared to the existing examples.

CI passes and I compiled all examples for NodeMCU and tested on actual hardware.

WhymustIhaveaname and others added 4 commits May 26, 2021 20:54
* Rebases WhymustIhaveaname:master to arduino-libraries:master.
* Fixes issues with Spell Check action.
* Fixes HiRes example compilation.
* Refactors HiRes example to be more idiomatic as compared to the
  existing examples.
* Fixes several line ending and white space inconsistencies.
@CLAassistant
Copy link

CLAassistant commented May 26, 2021

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

Memory usage change @ e4d04dd

Board flash % RAM for global variables %
esp8266:esp8266:huzzah 🔺 +1744 - +1744 +0.17 - +0.17 🔺 +192 - +192 +0.23 - +0.23
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/HiRes
flash
% examples/HiRes
RAM for global variables
% examples/IsTimeSet
flash
% examples/IsTimeSet
RAM for global variables
%
esp8266:esp8266:huzzah 1744 0.17 192 0.23 1744 0.17 192 0.23 N/A N/A N/A N/A 1744 0.17 192 0.23
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/HiRes<br>flash,%,examples/HiRes<br>RAM for global variables,%,examples/IsTimeSet<br>flash,%,examples/IsTimeSet<br>RAM for global variables,%
esp8266:esp8266:huzzah,1744,0.17,192,0.23,1744,0.17,192,0.23,N/A,N/A,N/A,N/A,1744,0.17,192,0.23

@chudsaviet
Copy link

Hi, any progress on merging this?

@tickelton
Copy link
Author

@chudsaviet I haven't received any feedback on this PR, and frankly by now I no longer have any use for it because I implemented an NTP client for the project I was working on from scratch to address this and a couple of other requirements I had that the NTPClient project seemed reluctant to address.

@chudsaviet
Copy link

@tickelton , would you mind if I take this commit over and make it production-ready?

@tickelton
Copy link
Author

@chudsaviet you are welcome to do so.

return this->forceUpdate();
int8_t NTPClient::update() {
uint32_t now=millis();
if(now>=this->_lastUpdate){ //if not overflow
Copy link

@jamesmyatt jamesmyatt Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure you need this logic. millis() - this->_lastUpdate should always give a positive integer by the power of integer arithmetic.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

millis() rollover (depending on mcu) every ~49 days

@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Jun 23, 2024
@per1234 per1234 linked an issue Jun 23, 2024 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 type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An 0.5s System Error
7 participants