Skip to content

wrong time occasionally #6

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

Open
gadjet opened this issue Feb 21, 2016 · 9 comments
Open

wrong time occasionally #6

gadjet opened this issue Feb 21, 2016 · 9 comments
Labels
type: imperfection Perceived defect in any part of project

Comments

@gadjet
Copy link

gadjet commented Feb 21, 2016

Hi,
I'm using the library (great job by the way) with an ESP-12 and I'm publishing some data to MQTT server, including time, then shutting down for 2 min's (Deep sleep) then starting again.

sometimes the time returned is 00:00:12 ~(the seconds vary) mostly the time returned is correct.

Is there a minimum time for the server to respond, is the 00:00:12 time returned because the server has not responded in time?

Any thoughts?

@gerardwr
Copy link

I'm using the (great!) library on an ESP8266 too.

I also experience the problem that in some cases the NTP time returned is 00:00:xx. I guess this happens when the connection to the NTP server fails.

I checked the source of the library to look for a fail/success flag but did not find any :-(

Perhaps the update() function could return a boolean that indicates the validity of the NTP time?

@sandeepmistry
Copy link
Contributor

Is there a minimum time for the server to respond, is the 00:00:12 time returned because the server has not responded in time?

The timeout is currently 1 second (10ms * 100 retry attempts). So, if the request or response gets "lost", getRawTime will return a value based on millis().

Perhaps the update() function could return a boolean that indicates the validity of the NTP time?

Another option is to have a hasUpdated API or something similar. @FWeinb what do you think?

@FWeinb
Copy link
Collaborator

FWeinb commented Apr 12, 2016

I don't see a reason not to return a bool from update. But if the timeout was hit we should ensure that update() will do a forceUpdate() the next time it is called. We might want to implement a backoff strategy too.

@gerardwr
Copy link

As a workaround i use the value of getRawTime().

The sketch looks something like:

    timeClient.update();
    if (timeClient.getRawTime() > 1000 ){
      // seems that it is a valid NTPtime
      out="NTP time updated : ";
      Serial.println(out);
      // set Arduino/ESP time to local time
      setTime(timeClient.getRawTime());
    }
    else Serial.println("Update NTP time failed");

For the current time the value is something like: 1460479647
NTP time updated : 16:47:27 1460479647
If the value is low (I use 1000 as limit) getting the NTP time apparently failed, and I leave the local Arduino time unchanged. If the value is higher I use the value to set the local Arduino time.

As a rule I get a valid NTP time within a couple of minutes after reboot (retry every minute).

@sandeepmistry
Copy link
Contributor

I don't see a reason not to return a bool from update

Ok, what do we want the behaviour to be for:

  • update() call before update interval has passed? true?
  • update() or forceUpdate() success: true
  • update()or forceUpdate() failure: false

@FWeinb
Copy link
Collaborator

FWeinb commented Apr 12, 2016

That is a good summary. Looks good. That should be useful to determine if the update succeed. We need to ensure that if forceUpdate() fails that update will call it again in the next run.

@sandeepmistry
Copy link
Contributor

We need to ensure that if forceUpdate() fails that update will call it again in the next run.

Hmm, can't we make it rely on _lastUpdate containing the old value? Then update() will trigger forceUpdate() again.

This does not cover a mixed use of forceUpdate() failing followed by an immediate call to update() if the update interval hasn't passed. However, I think this is ok.

@FWeinb
Copy link
Collaborator

FWeinb commented Apr 12, 2016

It looks like this isn't working correctly now, otherwise this issue should not be present in 2.0.0. But it would be great to find why this isn't currently working.

@blackketter
Copy link

Pull request #22 includes an updated() method for checking to see if the time has ever been updated.
Also, that patch makes updating non-blocking and may address the issue of wrong times being returned. Can you test?

@per1234 per1234 added the type: imperfection Perceived defect in any part of project label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

6 participants