Update NTP

- wrapper NTPClient class to avoid inadvertently calling NTP::getTime() while
setting things up
- only call NTP::getTime() when in loop(), install async TimeLib
sync provider and manually set TimeLib time
- randomize sync and delay times

code/espurna/libs/NtpClientWrap.h

@@ -0,0 +1,28 @@
+// -----------------------------------------------------------------------------
+// NtpClient overrides to avoid triggering network sync
+// -----------------------------------------------------------------------------
+
+#pragma once
+
+#include <WiFiUdp.h>
+#include <NtpClientLib.h>
+
+class NTPClientWrap : public NTPClient {
+
+public:
+
+    NTPClientWrap() : NTPClient() {
+        udp = new WiFiUDP();
+        _lastSyncd = 0;
+    }
+
+    bool setInterval(int shortInterval, int longInterval) {
+        _shortInterval = shortInterval;
+        _longInterval = longInterval;
+    }
+
+};
+
+// NOTE: original NTP should be discarded by the linker
+// TODO: allow NTP client object to be destroyed
+NTPClientWrap NTPw;

code/espurna/ntp.ino

@@ -9,13 +9,16 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #if NTP_SUPPORT
 
 #include <TimeLib.h>
-#include <NtpClientLib.h>
 #include <WiFiClient.h>
 #include <Ticker.h>
 
-unsigned long _ntp_start = 0;
-bool _ntp_update = false;
+#include <libs/NtpClientWrap.h>
+
+Ticker _ntp_defer;
+
+bool _ntp_report = false;
 bool _ntp_configure = false;
+bool _ntp_want_sync = false;
 
 // -----------------------------------------------------------------------------
 // NTP
@@ -38,15 +41,46 @@ void _ntpWebSocketOnSend(JsonObject& root) {
 
 #endif
 
-void _ntpStart() {
+time_t _ntpSyncProvider() {
+    _ntp_want_sync = true;
+    return 0;
+}
+
+void _ntpWantSync() {
+    _ntp_want_sync = true;
+}
+
+int _ntpSyncInterval() {
+    return secureRandom(NTP_SYNC_INTERVAL, NTP_SYNC_INTERVAL * 2);
+}
+
+int _ntpUpdateInterval() {
+    return secureRandom(NTP_UPDATE_INTERVAL, NTP_UPDATE_INTERVAL * 2);
+}
 
-    _ntp_start = 0;
+void inline _ntpSetSyncInterval(bool shortInterval = false) {
+    if (shortInterval) {
+        setSyncInterval(_ntpSyncInterval());
+    } else {
+        setSyncInterval(_ntpUpdateInterval());
+    }
+}
+
+void _ntpStart() {
 
-    NTP.begin(getSetting("ntpServer", NTP_SERVER));
-    NTP.setInterval(NTP_SYNC_INTERVAL, NTP_UPDATE_INTERVAL);
-    NTP.setNTPTimeout(NTP_TIMEOUT);
     _ntpConfigure();
 
+    // short (initial) and long (after sync) intervals
+    NTPw.setInterval(_ntpSyncInterval(), _ntpUpdateInterval());
+    DEBUG_MSG_P(PSTR("[NTP] Update intervals: %us / %us\n"),
+        NTPw.getShortInterval(), NTPw.getLongInterval());
+
+    // setSyncProvider will immediatly call given function by setting next sync time to the current time.
+    // Avoid triggering sync immediatly by canceling sync provider flag and resetting sync interval again
+    setSyncProvider(_ntpSyncProvider);
+    _ntp_want_sync = false;
+    _ntpSetSyncInterval(true);
+
 }
 
 void _ntpConfigure() {
@@ -58,30 +92,37 @@ void _ntpConfigure() {
     offset = abs(offset);
     int tz_hours = sign * (offset / 60);
     int tz_minutes = sign * (offset % 60);
-    if (NTP.getTimeZone() != tz_hours || NTP.getTimeZoneMinutes() != tz_minutes) {
-        NTP.setTimeZone(tz_hours, tz_minutes);
-        _ntp_update = true;
+    if (NTPw.getTimeZone() != tz_hours || NTPw.getTimeZoneMinutes() != tz_minutes) {
+        NTPw.setTimeZone(tz_hours, tz_minutes);
+        _ntp_report = true;
     }
 
     bool daylight = getSetting("ntpDST", NTP_DAY_LIGHT).toInt() == 1;
-    if (NTP.getDayLight() != daylight) {
-        NTP.setDayLight(daylight);
-        _ntp_update = true;
+    if (NTPw.getDayLight() != daylight) {
+        NTPw.setDayLight(daylight);
+        _ntp_report = true;
     }
 
     String server = getSetting("ntpServer", NTP_SERVER);
-    if (!NTP.getNtpServerName().equals(server)) {
-        NTP.setNtpServerName(server);
+    if (!NTPw.getNtpServerName().equals(server)) {
+        NTPw.setNtpServerName(server);
     }
 
     uint8_t dst_region = getSetting("ntpRegion", NTP_DST_REGION).toInt();
-    NTP.setDSTZone(dst_region);
+    NTPw.setDSTZone(dst_region);
+
+    // Set short interval to sync again shortly, randomized in time to avoid clogging the server with simultaious requests from multiple devices (i.e. on power surge, when devices start up at the same time)
+    setSyncInterval(true);
+
+    // Some remote servers can be slow to respond, increase accordingly
+    // TODO does this need upper constrain?
+    NTPw.setNTPTimeout(getSetting("ntpTimeout", NTP_TIMEOUT).toInt());
 
 }
 
-void _ntpUpdate() {
+void _ntpReport() {
 
-    _ntp_update = false;
+    _ntp_report = false;
 
     #if WEB_SUPPORT
         wsSend(_ntpWebSocketOnSend);
@@ -89,30 +130,48 @@ void _ntpUpdate() {
 
     if (ntpSynced()) {
         time_t t = now();
-        DEBUG_MSG_P(PSTR("[NTP] UTC Time  : %s\n"), (char *) ntpDateTime(ntpLocal2UTC(t)).c_str());
-        DEBUG_MSG_P(PSTR("[NTP] Local Time: %s\n"), (char *) ntpDateTime(t).c_str());
+        DEBUG_MSG_P(PSTR("[NTP] UTC Time  : %s\n"), ntpDateTime(ntpLocal2UTC(t)).c_str());
+        DEBUG_MSG_P(PSTR("[NTP] Local Time: %s\n"), ntpDateTime(t).c_str());
     }
 
 }
 
+#if BROKER_SUPPORT
+
+void inline _ntpBroker() {
+    static unsigned char last_minute = 60;
+    if (ntpSynced() && (minute() != last_minute)) {
+        last_minute = minute();
+        brokerPublish(BROKER_MSG_TYPE_DATETIME, MQTT_TOPIC_DATETIME, ntpDateTime().c_str());
+    }
+}
+
+#endif
+
 void _ntpLoop() {
 
-    if (0 < _ntp_start && _ntp_start < millis()) _ntpStart();
+    // Disable ntp sync when softAP is active. This will not crash, but instead spam debug-log with pointless sync failures.
+    if (!wifiConnected()) return;
+
     if (_ntp_configure) _ntpConfigure();
-    if (_ntp_update) _ntpUpdate();
 
-    now();
+    // NTPClientLib will trigger callback with sync status
+    // see: NTPw.onNTPSyncEvent([](NTPSyncEvent_t error){ ... }) below
+    if (_ntp_want_sync) {
+        _ntp_want_sync = false;
+        NTPw.getTime();
+    }
+
+    // Print current time whenever configuration changes or after successful sync
+    if (_ntp_report) _ntpReport();
 
     #if BROKER_SUPPORT
-        static unsigned char last_minute = 60;
-        if (ntpSynced() && (minute() != last_minute)) {
-            last_minute = minute();
-            brokerPublish(BROKER_MSG_TYPE_DATETIME, MQTT_TOPIC_DATETIME, ntpDateTime().c_str());
-        }
+        _ntpBroker();
     #endif
 
 }
 
+// TODO: remove me!
 void _ntpBackwards() {
     moveSetting("ntpServer1", "ntpServer");
     delSetting("ntpServer2");
@@ -128,8 +187,10 @@ void _ntpBackwards() {
 
 bool ntpSynced() {
     #if NTP_WAIT_FOR_SYNC
-        return (NTP.getLastNTPSync() > 0);
+        // Has synced at least once
+        return (NTPw.getFirstSync() > 0);
     #else
+        // TODO: runtime setting?
         return true;
     #endif
 }
@@ -148,9 +209,10 @@ String ntpDateTime() {
     return String();
 }
 
+// XXX: returns garbage during DST switch
 time_t ntpLocal2UTC(time_t local) {
     int offset = getSetting("ntpOffset", NTP_TIME_OFFSET).toInt();
-    if (NTP.isSummerTime()) offset += 60;
+    if (NTPw.isSummerTime()) offset += 60;
     return local - offset * 60;
 }
 
@@ -160,7 +222,23 @@ void ntpSetup() {
 
     _ntpBackwards();
 
-    NTP.onNTPSyncEvent([](NTPSyncEvent_t error) {
+    #if TERMINAL_SUPPORT
+        terminalRegisterCommand(F("NTP"), [](Embedis* e) {
+            if (ntpSynced()) {
+                _ntpReport();
+                terminalOK();
+            } else {
+                DEBUG_MSG_P(PSTR("[NTP] Not synced\n"));
+            }
+        });
+
+        terminalRegisterCommand(F("NTP.SYNC"), [](Embedis* e) {
+            _ntpWantSync();
+            terminalOK();
+        });
+    #endif
+
+    NTPw.onNTPSyncEvent([](NTPSyncEvent_t error) {
         if (error) {
             #if WEB_SUPPORT
                 wsSend_P(PSTR("{\"ntpStatus\": false}"));
@@ -171,12 +249,17 @@ void ntpSetup() {
                 DEBUG_MSG_P(PSTR("[NTP] Error: Invalid NTP server address\n"));
             }
         } else {
-            _ntp_update = true;
+            _ntp_report = true;
+            setTime(NTPw.getLastNTPSync());
         }
     });
 
     wifiRegister([](justwifi_messages_t code, char * parameter) {
-        if (code == MESSAGE_CONNECTED) _ntp_start = millis() + NTP_START_DELAY;
+        if (code == MESSAGE_CONNECTED) {
+            if (!ntpSynced()) {
+                _ntp_defer.once_ms(secureRandom(NTP_START_DELAY, NTP_START_DELAY * 15), _ntpWantSync);
+            }
+        }
     });
 
     #if WEB_SUPPORT
@@ -188,6 +271,9 @@ void ntpSetup() {
     espurnaRegisterLoop(_ntpLoop);
     espurnaRegisterReload([]() { _ntp_configure = true; });
 
+    // Sets up NTP instance, installs ours sync provider
+    _ntpStart();
+
 }
 
 #endif // NTP_SUPPORT