arduino-libraries / NTPClient

Connect to a NTP server
542 stars 372 forks source link

sometime time server does not provide valid time , this need to be managed inside NTPClient::forceUpdate() #133

Open jef-fr opened 3 years ago

jef-fr commented 3 years ago

RFC4330 says: The server reply should be discarded if any of the LI, Stratum, or Transmit Timestamp fields is 0 or the Mode field is not 4 (unicast) or 5 (broadcast).

I have detected that sometime the time server reply with Transmit Timestamp fields equal to 0, this error case is not managed inside forceUpdate and lead to totaly wrong time. In the error case I have seen LI,Stratum,Mode field where correct.

I suggest to check this inside forceUpdate Proposed changes (obviously many other method, could also just return false if Transmit Timestamp fields equal to 0.

bool NTPClient::forceUpdate() {
  #ifdef DEBUG_NTPClient
    Serial.println("Update from NTP Server");
  #endif

unsigned long secsSince1900=0L;
int nbtry=0;

  do {
      this->sendNTPPacket();

      // Wait till data is there or timeout...
      byte timeout = 0;
      int cb = 0;
      do {
        delay ( 10 );
        cb = this->_udp->parsePacket();
        if (timeout > 100) 
        {
            return false; // timeout after 1000 ms
        }
        timeout++;
     } while (cb == 0);
      this->_udp->read(this->_packetBuffer, NTP_PACKET_SIZE);

      unsigned long highWord = word(this->_packetBuffer[40], this->_packetBuffer[41]);
      unsigned long lowWord = word(this->_packetBuffer[42], this->_packetBuffer[43]);
      // combine the four bytes (two words) into a long integer
      // this is NTP time (seconds since Jan 1 1900):
       secsSince1900 = highWord << 16 | lowWord;
      if (secsSince1900 == 0L) 
      {
          if (++nbtry >10)
          {
            return false;
          }
      }
      else
      {
        this->_lastUpdate = millis() - (10 * (timeout + 1)); // Account for delay in reading the time
        this->_currentEpoc = secsSince1900 - SEVENZYYEARS;
      }
   } while ( secsSince1900 == 0L);
  return true;
}