From e8e58b821c88c9dac5bca41a40bb54b83a8b9923 Mon Sep 17 00:00:00 2001
From: Maxim Prokhorov <prokhorov.max@outlook.com>
Date: Thu, 21 Jan 2021 11:38:37 +0300
Subject: [PATCH] ntp: more checks when using DHCP servers

- make sure server is never empty
- disable dhcp option when server *is* empty
- update configure() to detect both tz and server changes
---
 code/espurna/config/general.h |  4 ++++
 code/espurna/ntp.cpp          | 41 +++++++++++++++++++++++++++++------
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/code/espurna/config/general.h b/code/espurna/config/general.h
index 93282656f3..4c53cbce47 100644
--- a/code/espurna/config/general.h
+++ b/code/espurna/config/general.h
@@ -1629,6 +1629,10 @@
 #define NTP_WAIT_FOR_SYNC           1               // Do not report any datetime until NTP sync'ed
 #endif
 
+#ifndef NTP_DHCP_SERVER
+#define NTP_DHCP_SERVER             1               // Automatically replace the NTP server value with the one received with the DHCP packet
+#endif
+
 // -----------------------------------------------------------------------------
 // ALEXA
 // -----------------------------------------------------------------------------
diff --git a/code/espurna/ntp.cpp b/code/espurna/ntp.cpp
index ca3b8fe61e..2fd0cd274b 100644
--- a/code/espurna/ntp.cpp
+++ b/code/espurna/ntp.cpp
@@ -166,13 +166,15 @@ void _ntpWebSocketOnConnected(JsonObject& root) {
 
 #endif
 
-// TODO: mention possibility of multiple servers
 String _ntpGetServer() {
     String server;
 
     server = sntp_getservername(0);
     if (!server.length()) {
-        server = IPAddress(sntp_getserver(0)).toString();
+        auto ip = IPAddress(sntp_getserver(0));
+        if (ip) {
+            server = ip.toString();
+        }
     }
 
     return server;
@@ -220,19 +222,32 @@ void _ntpReport() {
 }
 
 void _ntpConfigure() {
+    // Ignore or accept the DHCP SNTP option
+    // When enabled, it is possible that lwip will replace the NTP server pointer from under us
+    sntp_servermode_dhcp(getSetting("ntpDhcp", 1 == NTP_DHCP_SERVER));
+
     // Note: TZ_... provided by the Core are already wrapped with PSTR(...)
+    // but, String() already handles every char pointer as a flash-string
     const auto cfg_tz = getSetting("ntpTZ", NTP_TIMEZONE);
     const char* active_tz = getenv("TZ");
-    if (cfg_tz != active_tz) {
+
+    bool changed { cfg_tz != active_tz };
+    if (changed) {
         setenv("TZ", cfg_tz.c_str(), 1);
         tzset();
     }
 
     const auto cfg_server = getSetting("ntpServer", F(NTP_SERVER));
     const auto active_server = _ntpGetServer();
-    if (cfg_tz != active_tz) {
+    changed = (cfg_server != active_server) || changed;
+
+    // We skip configTime() API since we already set the TZ just above
+    // (and most of the time we expect NTP server to proxy to multiple servers instead of defining more than one here)
+    if (changed) {
+        sntp_stop();
         _ntp_server = cfg_server;
-        configTime(cfg_tz.c_str(), _ntp_server.c_str());
+        sntp_setservername(0, _ntp_server.c_str());
+        sntp_init();
         DEBUG_MSG_P(PSTR("[NTP] Server: %s, TZ: %s\n"), cfg_server.c_str(), cfg_tz.length() ? cfg_tz.c_str() : "UTC0");
     }
 }
@@ -428,8 +443,20 @@ void ntpSetup() {
     // make sure our logic does know about the actual server
     // in case dhcp sends out ntp settings
     static WiFiEventHandler on_sta = WiFi.onStationModeGotIP([](WiFiEventStationModeGotIP) {
-        const auto server = _ntpGetServer();
-        if (sntp_enabled() && (!_ntp_server.length() || (server != _ntp_server))) {
+        if (!sntp_enabled()) {
+            return;
+        }
+
+        auto server = _ntpGetServer();
+        if (!server.length()) {
+            DEBUG_MSG_P(PSTR("[NTP] Updating `ntpDhcp` to ignore the DHCP values\n"));
+            setSetting("ntpDhcp", "0");
+            sntp_servermode_dhcp(0);
+            schedule_function(_ntpConfigure);
+            return;
+        }
+
+        if (!_ntp_server.length() || (server != _ntp_server)) {
             DEBUG_MSG_P(PSTR("[NTP] Updating `ntpServer` setting from DHCP: %s\n"), server.c_str());
             _ntp_server = server;
             setSetting("ntpServer", server);