PaulStoffregen / Time

Time library for Arduino
http://playground.arduino.cc/code/time
1.25k stars 666 forks source link

System crash upon startup? - info what might be the cause #102

Open ddTech opened 6 years ago

ddTech commented 6 years ago

After a changing my code to make use of a syncProvider function, my esp suddenly crashed directly in the setup(). It was firing NTP requests without delay until the WDT kicked in. It took me a moment and a closer look under the hood to understand what was going on, as my code (derived from the TimeNTP_ESP8266WiFi example) didn't (seem to) allow that.

As it turned out, the culprit was a logging function that writes out certain system events. This function uses now() to print out the current timestamp. now() however calls the syncProvider when the time is due for next synchronization. Therefore calling now() or a function that uses now() from within the syncProvider leads to an endless recursion.

This is not a bug. It's just a matter of understanding what's going on in the library.

On my side I circumvent the problem by temporarily setting a new nextSyncTime in function now() of Time.cpp, just before calling the syncProvider.

In my local copy I've changed

  ...
  if (nextSyncTime <= sysTime) {
    if (getTimePtr != 0) {
      time_t t = getTimePtr();
      if (t != 0) {
        setTime(t);
      } else {
        nextSyncTime = sysTime + syncInterval;
        Status = (Status == timeNotSet) ?  timeNotSet : timeNeedsSync;
      }
    } 
    ... 

to

  ...
  if (nextSyncTime <= sysTime) {
    if (getTimePtr != 0) {

      // DD 2018-07-21
      // avoid recursion when calling now() from within the syncProvider
      // "nextSyncTime" will be properly set in setTime() or here, when the
      // syncProvider fails. So we can safely manipulate it here.
      nextSyncTime = sysTime + 10;

      time_t t = getTimePtr();
      if (t != 0) {
        setTime(t);
      } else {
        nextSyncTime = sysTime + syncInterval;
        Status = (Status == timeNotSet) ?  timeNotSet : timeNeedsSync;
      }
    } 
    ... 

Just in case someone else falls over this or needs it.

Regards

Frank

DavidBerdik commented 1 year ago

Thank you very much for sharing this. I just encountered this as well.

@PaulStoffregen would you be open to implementing a fix for this so that us ES98266 users don't have to use this hacky solution?