Skip to content

Wrong time data from NTPClient #105

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
afrixx opened this issue Apr 30, 2020 · 26 comments
Open

Wrong time data from NTPClient #105

afrixx opened this issue Apr 30, 2020 · 26 comments
Labels
type: imperfection Perceived defect in any part of project

Comments

@afrixx
Copy link

afrixx commented Apr 30, 2020

Describe the problem

Normally, this library provides correct time data, with a maximum 2s deviation.
But some days, let's say every 30 days, I get wrong time data from the library. Sometimes 5 minutes difference, sometimes 30 minutes. And although I have configured a refresh every 10 minutes, the time remains wrong for hours, sometimes only until a hard reset.

You could say it's a network problem, but I can access the webserver on the ESP all the time.

Additional context

I am using a Wemos D1 Mini board (ESP8266-based).

@WhymustIhaveaname
Copy link

WhymustIhaveaname commented May 1, 2020

It is an intricate problem. Can I have a look at your codes? You can include files in reply.

@afrixx
Copy link
Author

afrixx commented May 1, 2020

Thanks @WhymustIhaveaname for having a look at my code.
I included the 2 basic files:

wortuhr.ino

#include <ESP8266WiFi.h>  //ESP8266 Core WiFi Library
extern "C" {
#include "user_interface.h"  //to set hostname
}

#include <DNSServer.h>         //Local DNS Server used for redirecting all requests to the configuration portal
#include <ESP8266WebServer.h>  //Local WebServer used to serve the configuration portal
#include <WiFiManager.h>       //https://github.com/tzapu/WiFiManager WiFi Configuration Magic

#include <NTPClient.h>          //NTP Client to get current UTC time
#include <Adafruit_NeoPixel.h>  //Adafruit NeoPixel library to control WS2812B stripes

#include <WiFiUdp.h>

#include <EEPROM.h>  // to store brightness and color

#include <ESP8266mDNS.h>  // Include the mDNS library

#include "clockwords.h"
#include "colors.h"

#define RGBW 1  // 0=RGB stripe, 1=RGBW stripe

// Which pin on the Arduino is connected to the NeoPixels?
// On a Trinket or Gemma we suggest changing this to 1
#define PIN D1

// How many NeoPixels are attached to the Arduino?
#define NUMPIXELS 114

// When we setup the NeoPixel library, we tell it how many pixels, and which pin to use to send signals.
// Note that for older NeoPixel strips you might need to change the third parameter--see the strandtest
// example for more information on possible values.
#if RGBW == 0
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ800);
#else
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_GRBW + NEO_KHZ800);
#endif


char wiFiHostname[] = "wortuhr";
long currentTimeZone = 3600;  //offset in seconds

////////////////////////////////
/// DO NOT MODIFY AFTER HERE ///
////////////////////////////////

WiFiManager wifiManager;

WiFiUDP ntpUDP;


unsigned long timeZoneValidUntil = 0;
unsigned long currentUtcEpochTime = 0;
unsigned long lastUtcEpochTime = 0;  //added to detect bigger time jumps >3h into the past
unsigned long lastTimeUpdate = 0;    //time when last time NTP client synced from server

// You can specify the time server pool and the offset (in seconds, can be
// changed later with setTimeOffset() ). Additionaly you can specify the
// update interval (in milliseconds, can be changed using setUpdateInterval() ).
NTPClient timeClient(ntpUDP, "de.pool.ntp.org", currentTimeZone, 600000);

int uint8_tSize = sizeof(uint8_t);

uint8_t color_red = 0;
uint8_t color_green = 0;
uint8_t color_blue = 0;
uint8_t color_white = 0;

uint8_t colorNr = 0;
uint8_t brightness = 0;

uint8_t languageMode = 0;
uint8_t rgbMode = 0;

int currentMinute = -1;
int currentSecond = -1;
bool doTimeRepaint = false;

bool secondsModeOn = false;
uint8_t specialMode = 0;
unsigned long secondsOnUntil = 0;
ESP8266WebServer webServer;


void setup() {
  Serial.begin(115200);
  Serial.println("");

  // Read EEPROM and load values
  InitEeprom();
  ReadEepromValues();
#if RGBW == 0
  Serial.println("*Wortuhr: Starting with RGB mode");
#else
  Serial.println("*Wortuhr: Starting with RGBW mode");
#endif

  pixels.begin();
  pixels.clear();
  Serial.println("*Wortuhr: Display FUNK");
  TurnByteArrayOn(&_word_funk[0], 255, 0, 0, 0);
  pixels.show();

  Serial.println("*Wortuhr: Connect to WIFI...");

  wifi_station_set_hostname(wiFiHostname);
  wifiManager.setAPCallback(configModeCallback);
  wifiManager.autoConnect("wortuhr");
  Serial.println("*Wortuhr: WIFI connected!");

  timeClient.begin();
  Serial.println("");

  // Start mDNS
  if (!MDNS.begin("wortuhr")) {  // Start the mDNS responder for esp8266.local
    Serial.println("*Wortuhr: Error setting up MDNS responder!");
  }
  Serial.println("*Wortuhr: mDNS responder started! You can connect by http://wortuhr.local\n");

  StartWebserver();
  StartApi();

  UdpInit();
}

void loop() {
  bool timeUpdated = timeClient.update();
  if (timeUpdated == true) {
    lastTimeUpdate = timeClient.getEpochTime() - currentTimeZone;
  }

  lastUtcEpochTime = currentUtcEpochTime;  //backup last time
  currentUtcEpochTime = timeClient.getEpochTime() - currentTimeZone;

  //check for time jumps backwards
  if (currentUtcEpochTime + (3 * 3600) < lastUtcEpochTime) {
    //time jumped back for >3h --> clear timeZoneValidUntil
    timeZoneValidUntil = 0;
    Serial.println("*Wortuhr: Time jumped back more then 3 hours, requesting new time zone information");
  }

  //check if time zone is not valid anymore
  if (currentUtcEpochTime > timeZoneValidUntil) {
    timeZoneValidUntil = currentUtcEpochTime + 60;  //security mechanism, to only ask the server every 60s at maximum (if not reachable etc.)
    SetTimeZone(currentUtcEpochTime);               //load and set the current time zone
  }

  //check if time is valid (> 1.1.1970)
  if (currentUtcEpochTime > (24 * 3600)) {
    int hour = timeClient.getHours();
    int minute = timeClient.getMinutes();  // Get the Minutes here and not the Seconds

    if (secondsModeOn == false && specialMode == 0) {
      if (minute != currentMinute || doTimeRepaint == true)  //time has changed!
      {
        doTimeRepaint = false;
        pixels.clear();  // Clear Stripe
        Serial.println("*Wortuhr: Display Time: " + timeClient.getFormattedTime() + ", utc epoch: " + String(currentUtcEpochTime));
        // Update Time
        UpdateTime(hour, minute);
        currentMinute = minute;
      }
    } else if (specialMode > 0) {
      if (specialMode == 1) { showXmasTree(); }
      if (specialMode == 2) { showEasterEggs(); }
      Serial.println("*Wortuhr: Display Time (SPECIAL MODE): " + timeClient.getFormattedTime());
      currentMinute = -1;

      if (secondsOnUntil > 0 && secondsOnUntil < currentUtcEpochTime) {
        secondsOnUntil = 0;
        specialMode = 0;
      }
    } else {
      int second = timeClient.getSeconds();
      if (second != currentSecond) {
        TurnSecondsOn(second);
        Serial.println("*Wortuhr: Display Time (SECONDS MODE): " + timeClient.getFormattedTime());
        currentMinute = -1;
        currentSecond = second;

        if (secondsOnUntil > 0 && secondsOnUntil < currentUtcEpochTime) {
          secondsOnUntil = 0;
          secondsModeOn = false;
        }
      }
    }
  } else {
    pixels.clear();  // Clear Stripe
    Serial.println("*Wortuhr: Could not get NTP time!");
    TurnByteArrayOn(&_word_ntp[0], 0, 0, 255, 0);  //display NTP
  }

  // Update Stripe
  pixels.show();

  //wait 1s and handle web client requests if necessary
  //wait 200ms when seconds are being displayed
  uint8_t count_max = (secondsModeOn == false && specialMode == 0) ? 10 : 2;

  for (uint8_t i = 0; i < count_max; i++)  //wait 1s
  {
    delay(100);
    webServer.handleClient();
  }

  UdpCheck();  //check for incoming UDP packages
}

void configModeCallback(WiFiManager *myWiFiManager) {
  //Wifi is now in access point config mode
  Serial.println("*Wortuhr: Entered access point config mode");
  TurnByteArrayOn(&_word_an1[0], 100, 100, 100, 100);
  TurnByteArrayOn(&_word_an2[0], 100, 100, 100, 100);
  pixels.show();
}

timezoneHelper.ino

#include <ArduinoJson.h>

WiFiClient wifiClient;

void SetTimeZone(unsigned long currentUtcEpochTime) {
  const char* timeZoneServer = "api.timezonedb.com";
  String timeZoneUrl = "/v2/get-time-zone?key=XXXXXXXXX&format=json&by=zone&zone=Europe/Berlin";

  const unsigned long HTTP_TIMEOUT = 10000;  // max respone time from server
  const size_t MAX_CONTENT_SIZE = 2048;      // max size of the HTTP response

  const unsigned long START_YEAR_2036 = 2082754800;

  //check if time is valid (> 1.1.1970) and before 1.1.2036 (NTP Bug of 7.2.2036)
  if ((currentUtcEpochTime > (24 * 3600)) && (currentUtcEpochTime < START_YEAR_2036)) {
    timeZoneUrl += "&time=";
    timeZoneUrl += String(currentUtcEpochTime);
  }

  if (!wifiClient.connect(timeZoneServer, 80)) {
    Serial.println("*Wortuhr: Could not connect to time zone server");
    wifiClient.stop();
    return;
  }

  wifiClient.print("GET ");
  wifiClient.print(timeZoneUrl);
  wifiClient.println(" HTTP/1.0");
  wifiClient.print("Host: ");
  wifiClient.println(timeZoneServer);
  wifiClient.println("Connection: close");
  wifiClient.println();

  // Skip HTTP headers so that we are at the beginning of the response's body
  // HTTP headers end with an empty line
  char endOfHeaders[] = "\r\n\r\n";

  wifiClient.setTimeout(HTTP_TIMEOUT);
  bool ok = wifiClient.find(endOfHeaders);

  if (!ok) {
    Serial.println("*Wortuhr: No response or invalid response from Timezone server!");
    wifiClient.stop();
    return;
  }

  //Parse JSON response
  const size_t bufferSize = JSON_OBJECT_SIZE(13) + 260;
  DynamicJsonBuffer jsonBuffer(bufferSize);

  //const char* json = "{\"status\":\"OK\",\"message\":\"\",\"countryCode\":\"DE\",\"countryName\":\"Germany\",\"zoneName\":\"Europe/Berlin\",\"abbreviation\":\"CEST\",\"gmtOffset\":7200,\"dst\":\"1\",\"dstStart\":1521939600,\"dstEnd\":1540688399,\"nextAbbreviation\":\"CET\",\"timestamp\":1526505696,\"formatted\":\"2018-05-16 21:21:36\"}";

  JsonObject& root = jsonBuffer.parseObject(wifiClient);

  const char* status = root["status"];  // "OK"
  //const char* message = root["message"]; // ""
  //const char* countryCode = root["countryCode"]; // "DE"
  //const char* countryName = root["countryName"]; // "Germany"
  //const char* zoneName = root["zoneName"]; // "Europe/Berlin"
  //const char* abbreviation = root["abbreviation"]; // "CEST"
  long gmtOffset = root["gmtOffset"];  // 7200
  //const char* dst = root["dst"]; // "1"
  //long dstStart = root["dstStart"]; // 1521939600
  long dstEnd = root["dstEnd"];  // 1540688399
  //const char* nextAbbreviation = root["nextAbbreviation"]; // "CET"
  //long timestamp = root["timestamp"]; // 1526505696
  //const char* formatted = root["formatted"]; // "2018-05-16 21:21:36"

  if (String(status) == "OK") {
    currentTimeZone = gmtOffset;
    if (dstEnd > 0) timeZoneValidUntil = dstEnd;
    timeClient.setTimeOffset(currentTimeZone);
    doTimeRepaint = true;  //do a repaint immediately
    Serial.println("*Wortuhr: Saved time zone: " + String(currentTimeZone) + "s (valid until epoch time: " + String(timeZoneValidUntil) + ")");
  } else {
    Serial.println("*Wortuhr: Timezone json invalid! (Status: '" + String(status) + "'");
  }
  wifiClient.stop();
}

@WhymustIhaveaname
Copy link

WhymustIhaveaname commented May 1, 2020

Entschuldigung, I didn't find the reason for your problem, although clues indicate this is the problem of this library. However, I found that you are not the only one, in issue #6 is there a bunch of people suffer from the same problem as you.

May you find the bug.

@WhymustIhaveaname
Copy link

You can try my fork. This fork, by default, prints many pieces of information about NTP via Serial, such as the time NTP package is sent and received, the times in the NTP response package. It also changes the return type of update from bool to byte, to record more information(1 on update and success, 0 on update but failed, 2 or 3 on not time to update). These may help you debug.

@FilipDem
Copy link

Hi, I have a similar (identical?) problem... I am using always the EPOCH date (and have the offset myself through an online API). However now I see that that the time jumps regularly with one hour... By coincidence the NTP Update interval is one hour... I already did some code reading of the NTPClient, but does not find anything what could explain this... I now added some syslog debug information in case the NTP Update failed (so in case of timeout).

The principle of my implementation is explained below. So I am very interested in the possible results of your debugging. I will also share mine once available.

In the Startup:

    // Start NTP Server connection and wait until time is received
    timeClient.begin();

    // Wait until NTP time received
    uint16_t NTPStart = millis();
    do {
      NTPUpdateDone |= timeClient.update();
      yield();
    } while ( !NTPUpdateDone && (millis()-NTPStart < NTP_TIMEOUT) );
    CurrentDateTime = timeClient.getEpochTime();

And in the Loop:

  boolean NTPUpdate = timeClient.update();
  CurrentDateTime = timeClient.getEpochTime();

Filip

@afrixx
Copy link
Author

afrixx commented May 14, 2020

I did a lot of more testing to find the deviation issue - but I was not successful.

For me I found another solution, which works fine and is much more easy:
The ESP8266 core now contains its own time library: You are able to define multiple NTP Servers, it works in background and there is also a timezone calculation included. So I removed both NTPClient library and Timezone client by adding this functions:
https://github.com/esp8266/Arduino/blob/9913e5210779d2f3c4197760d6813270dbba6232/cores/esp8266/time.c

There is a good example sketch to try out:
https://github.com/esp8266/Arduino/blob/61cd8d83859524db0066a647de3de3f6a0039bb2/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino

The setting of the timezone is very simple:

  // set timezone to Berlin
  setenv("TZ", "CET-1CEST-2,M3.5.0/02:00:00,M10.5.0/03:00:00", 3);
  tzset();

There you get most important time zone strings:
https://sites.google.com/a/usapiens.com/opnode/time-zones

So... if you're using the ESP, I can only recommend this internal ESP library.

@FilipDem
Copy link

FilipDem commented May 15, 2020 via email

@mitchsf
Copy link

mitchsf commented Jun 20, 2020

My update interval is set to 10 minutes, and the error manifests as 10 minutes behind. The error is highly dependent on the connection. Some ESP32s I have running in other locations, never experience this error. One always shows the error within an hour or two of operation.

I'm going to try a hopefully temporary workaround that will keep trying until the time received is greater than the previous update.

@morres83
Copy link

morres83 commented Jul 9, 2020

I can also confirm this issue; and the wrong time depends clearly on the update interval or multiples of it.
I think FilipDem is correct with his assumption about "last_update"

@FilipDem

This comment was marked as duplicate.

@mitchsf
Copy link

mitchsf commented Jul 9, 2020

I seem to have this under control now. There was a bug in NTPClient::update() that was causing it to return true when an update did not occur. That's why it was returning corrupted packets.

The bug has since been fixed: #56

I'm not going to research the cause, maybe it's something I changed inadvertently, or maybe a bug fix. It does make sense that the last good packet received was off by the update interval.

@morres83
Copy link

morres83 commented Jul 9, 2020

I think the problem lies in the forceUpdate function. The client receives "0", _lastUpdate will be set even though the reception was not correct. Once you read the time by getEpochTime(), you get the wrong offset.

I fixed it in another fork of a similar library, which I use. It combines a check for 0, return codes for update() and a check if the new time is OFFSET later than the current one. You might take a look if you want (ntpUpdate()-function). I think it fixes the issue (as far as I can tell today).

https://github.com/morres83/NTP/blob/master/NTP.cpp

@morres83
Copy link

Unfortunately also my Bugfix doesn't solve the problem completely, as I found 1 day later.

Clearly, there is a bug. But I wouldn't call this library user-friendly. But the Arduino core solution and the example sketch clearly is. That's why you cannot find any hint on Google about it.

@mitchsf
Copy link

mitchsf commented Jul 12, 2020

The latest release is working fine for me, at least so far. I added an additional method to get milliseconds. The library referred to by afrixx looks great, but I use the ESP32 and I don't see a version for it.

@afrixx
Copy link
Author

afrixx commented Jul 12, 2020

The latest release is working fine for me, at least so far. I added an additional method to get milliseconds. The library referred to by afrixx looks great, but I use the ESP32 and I don't see a version for it.

For ESP32 the integrated time library works fine, too:
https://randomnerdtutorials.com/esp32-date-time-ntp-client-server-arduino/

@mitchsf
Copy link

mitchsf commented Jul 13, 2020

Thanks. I think that Timezone is still required to automatically shift to DST, but that's easy enough to add.

@hedgemybets
Copy link

hedgemybets commented Feb 1, 2021

I just noticed this issue recently and found that the version I had did not have the following code near the beginning of forceUpdate(). It does appear in version 3.2:

  // flush any existing packets
  while(this->_udp->parsePacket() != 0) {
    this->_udp->flush();

What I believe was happening in my case is that after an unsuccessful forceUpdate(), the NTP packet actually came in after the timeout period. When called the next time, the udp->parsePacket() immediately returns the packet from the previous iteration. With a debug statement, I found that the timeout count is then always 1 after a delayed NTP response. Ten milliseconds is not long enough for a distant server to respond. The flush ensures that the previous packet is cleared so you get the latest update.

@mitchsf
Copy link

mitchsf commented Feb 1, 2021

I just noticed this issue recently and found that the version I had did not have the following code near the beginning of forceUpdate(). It does appear in version 3.2:

// flush any existing packets
while(this->_udp->parsePacket() != 0) {
this->_udp->flush();

I think this is exactly what fixed the problem.

@DrTron
Copy link

DrTron commented Mar 9, 2021

Just jumping on this train, I also noticed that my clock showed the wrong time after a while, with the offset of the ntp-update interval. Same library version (3.2.0, I suppose, as it has not been updated for a while).
So I also modified my sketch to use the integrated time library, which made things even simpler. I only added a secondary loop to run the ntp-update every 10 minutes, based on the "blink without delay" example.

@robertknutzen
Copy link

Just piling on to say that I have the same issue. My "fix" is pretty simple. I seem to be getting small numbers for my epoch time

the last few times it happened I got "6" but its hard to know if thats coincidence or not. I'll edit my sketch to use the integrated library mentioned above later, but i'll leave my serial monitor open and see if I always get 6 which might mean something?

I'm using ntp just to keep an RTC Module in sync, so I decided the easiest course of action is to ignore bad results my RTC will keep good enough time for my project

void updateTime(){
  Serial.println("UpdatingTime");
    timeClient.update();
    long realTime=timeClient.getEpochTime();
    //sometimes ntp was telling the the time was a very small number if that happens we just ignore it and keep the rtc the same
    if (realTime > 1000000){
        rtc.adjust(DateTime(realTime));

    }
  Serial.println(realTime);

}

@0815Creeper
Copy link

same problem here. Off by one hour (updateperiod was set to 5min but updates where done only by forceupdate by an external trigger about once every hour) Fixed itself after about a day. Happend twice this month with continously running setup. server was europe.pool.ntp.org.

@alirezaimi
Copy link

same problem here. ‍‍‍‍‍.update(); run in every loop and there is some minutes behind ! and time goes wrong after a while /

@per1234 per1234 added the type: imperfection Perceived defect in any part of project label Mar 28, 2022
@salasidis
Copy link

salasidis commented Aug 12, 2022

An old thread, and not sure if this is related to the same problem. Using NTPClient on Arduino Portenta. My code, and the example code, had an offset in the NTP time by the amount equal to the update interval.

In the example case, the update interval was 10sec, and the time being returned was 10 sec behind.

In my code, I use a 60 second interval (Portenta 32kHz clock issues), and the time returned was always 60 sec behind.

What I did was issue a

timeClient.update();

a second time in the loop

for(;;) {

     timeClient.update();

    if (timeClient.updated()) {

      SerDebugln("********UPDATED********");

    } else {

      SerDebugln("******NOT UPDATED******");

      rtos::ThisThread::sleep_for(mbed::chrono::milliseconds_u32(1000L));

      continue;

    }

    timeClient.update();    // reread in case buffer issues

It now seems to return the correct time, and I am able to set the arduino clock to within a couple of seconds of the clock on my PC.

@salasidis
Copy link

Never mind on my last posting. It worked for a short time, and then started to be behind by one minute again.

I got rid of the code, and I am using the sendNTPpacket software that is also generally available. That one works, however, because of the forced delay for 1 second after sending the packet to parse the packet, the synced clock is 1 second behind. I compensate by just adding 1 second to the clock time that is retrieved.

@pisunio
Copy link

pisunio commented Oct 13, 2022

"4. End-User agrees that he or she will not:
(a) Change default settings to make more frequent request of the Services, if using an ntp daemon (“NTP Protocol”);
(b) Request time from the Services more than once every thirty (30) minutes (ideally at even longer intervals), if using SNTP.
(c) Set the time more often than is necessary for the purposes of the device; if the device can keep reasonably accurate time for several days, End-User will not set the time every hour. Additionally, if the device only uses whole seconds, End-User will not optimize the device for millisecond accuracy.
(d) Set up a regular time sync interval such as “top of the hour”, “top of the minute”, or “at minute or second X”."
https://www.ntppool.org/tos.html

Calling timeClient.update() in every hour solved my issue.

@dDenVil
Copy link

dDenVil commented Jan 4, 2023

I think, I have fixed this.
So I have made the telegram bot on ESP8265 that shows whether I have light at home or not. If there are no light it goes into light sleep. After external interrupt (light goes on) in wakes up and getting ntp time (almost every time wrong). I've tried a lot of cases and now seems that this one is working. Pasted it after waking up and connecting to wi-fi:

    while ( timeClient.update() == false){
      Serial.print("_ ");
      timeClient.update();
      delay(500); 
    }

It takes from 10 to 50 seconds to get real time. At least it works correctly now

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