Merge pull request #1642 from mcspr/ntp/async-state

Update NTP

code/espurna/libs/NtpClientWrap.h

@@ -0,0 +1,29 @@
+// -----------------------------------------------------------------------------
+// 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;
+        return true;
+    }
+
+};
+
+// 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,41 @@ void _ntpWebSocketOnSend(JsonObject& root) {
 
 #endif
 
-void _ntpStart() {
+time_t _ntpSyncProvider() {
+    _ntp_want_sync = true;
+    return 0;
+}
+
+void _ntpWantSync() {
+    _ntp_want_sync = true;
+}
+
+// Randomized in time to avoid clogging the server with simultaious requests from multiple devices
+// (for example, when multiple devices start up at the same time)
+int inline _ntpSyncInterval() {
+    return secureRandom(NTP_SYNC_INTERVAL, NTP_SYNC_INTERVAL * 2);
+}
 
-    _ntp_start = 0;
+int inline _ntpUpdateInterval() {
+    return secureRandom(NTP_UPDATE_INTERVAL, NTP_UPDATE_INTERVAL * 2);
+}
+
+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;
+
+    setSyncInterval(NTPw.getShortInterval());
+
 }
 
 void _ntpConfigure() {
@@ -58,30 +87,34 @@ 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);
+
+    // 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 +122,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 +179,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 +201,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 +214,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 +241,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 +263,9 @@ void ntpSetup() {
     espurnaRegisterLoop(_ntpLoop);
     espurnaRegisterReload([]() { _ntp_configure = true; });
 
+    // Sets up NTP instance, installs ours sync provider
+    _ntpStart();
+
 }
 
 #endif // NTP_SUPPORT