implement async DNS lookup - no more stuck NTP requests

wled00/asyncDNS.h

@@ -0,0 +1,65 @@
+#pragma once
+/*
+  asyncDNS.h - wrapper class for asynchronous DNS lookups using lwIP
+  by @dedehai
+*/
+
+#include <Arduino.h>
+#include <atomic>
+#include <lwip/dns.h>
+#include <lwip/err.h>
+
+
+class AsyncDNS {
+public:
+  // note: passing the IP as a pointer to query() is not implemented because it is not thread-safe without mutexes
+  //       with the IDF V4 bug external error handling is required anyway or dns can just stay stuck
+  enum class result { Idle, Busy, Success, Error };
+
+  // non-blocking query function to start DNS lookup
+  result query(const char* hostname) {
+    if (_status == result::Busy) return result::Busy; // in progress, waiting for callback
+
+    _status = result::Busy;
+    err_t err = dns_gethostbyname(hostname, &_raw_addr, _dns_callback, this);
+    if (err == ERR_OK) {
+      _status = result::Success; // result already in cache
+    } else if (err != ERR_INPROGRESS) {
+      _status = result::Error;
+      _errorcount++;
+    }
+    return _status;
+  }
+
+  // get the IP once Success is returned
+  const IPAddress getIP() {
+    if (_status != result::Success) return IPAddress(0,0,0,0);
+    #ifdef ARDUINO_ARCH_ESP32
+      return IPAddress(_raw_addr.u_addr.ip4.addr);
+    #else
+      return IPAddress(_raw_addr.addr);
+    #endif
+  }
+
+  void renew() { _status = result::Idle; } // reset status to allow re-query
+  void reset() { _status = result::Idle; _errorcount = 0; } // reset status and error count
+  const result status() { return _status; }
+  const uint16_t getErrorCount() { return _errorcount; }
+
+  private:
+  ip_addr_t _raw_addr;
+  std::atomic<result> _status { result::Idle };
+  uint16_t _errorcount = 0;
+
+  // callback for dns_gethostbyname(), called when lookup is complete or timed out
+  static void _dns_callback(const char *name, const ip_addr_t *ipaddr, void *arg) {
+    AsyncDNS* instance = reinterpret_cast<AsyncDNS*>(arg);
+    if (ipaddr) {
+      instance->_raw_addr = *ipaddr;
+      instance->_status = result::Success;
+    } else {
+      instance->_status = result::Error; // note: if query timed out (~5s), DNS lookup is broken until WiFi connection is reset (IDF V4 bug)
+      instance->_errorcount++;
+    }
+  }
+};

wled00/bus_manager.cpp

@@ -712,13 +712,24 @@ size_t BusNetwork::getPins(uint8_t* pinArray) const {
 
 #ifdef ARDUINO_ARCH_ESP32
 void BusNetwork::resolveHostname() {
-  static unsigned long nextResolve = 0;
-  if (Network.isConnected() && millis() > nextResolve && _hostname.length() > 0) {
-    nextResolve = millis() + 600000; // resolve only every 10 minutes
+  static AsyncDNS DNSlookup; // TODO: make this dynamic? requires to handle the callback properly
+  if (Network.isConnected()) {
     IPAddress clnt;
-    if (strlen(cmDNS) > 0) clnt = MDNS.queryHost(_hostname);
-    else WiFi.hostByName(_hostname.c_str(), clnt);
-    if (clnt != IPAddress()) _client = clnt;
+    if (strlen(cmDNS) > 0) {
+      clnt = MDNS.queryHost(_hostname);
+      if (clnt != IPAddress()) _client = clnt; // update client IP if not null
+    }
+    else {
+      int timeout = 5000; // 5 seconds timeout
+      DNSlookup.reset();
+      DNSlookup.query(_hostname.c_str()); // start async DNS query
+      while (DNSlookup.status() == AsyncDNS::result::Busy && timeout-- > 0) {
+        delay(1);
+      }
+      if (DNSlookup.status() == AsyncDNS::result::Success) {
+        _client = DNSlookup.getIP(); // update client IP
+      }
+    }
   }
 }
 #endif
@@ -1300,12 +1311,17 @@ void BusManager::on() {
     }
   }
   #else
+  static uint32_t nextResolve = 0;  // initial resolve is done on bus creation
+  bool resolveNow = (millis() - nextResolve >= 600000); // wait at least 10 minutes between hostname resolutions (blocking call)
   for (auto &bus : busses) if (bus->isVirtual()) {
     // virtual/network bus should check for IP change if hostname is specified
     // otherwise there are no endpoints to force DNS resolution
     BusNetwork &b = static_cast<BusNetwork&>(*bus);
-    b.resolveHostname();
+    if (resolveNow)
+      b.resolveHostname();
   }
+  if (resolveNow)
+    nextResolve = millis();
   #endif
   #ifdef ESP32_DATA_IDLE_HIGH
   esp32RMTInvertIdle();

wled00/bus_manager.h

@@ -17,6 +17,9 @@
 #include "pin_manager.h"
 #include <vector>
 #include <memory>
+#ifdef ARDUINO_ARCH_ESP32
+#include "asyncDNS.h"
+#endif
 
 #if __cplusplus >= 201402L
 using std::make_unique;

wled00/cfg.cpp

@@ -131,7 +131,7 @@ bool deserializeConfig(JsonObject doc, bool fromFS) {
   getStringFromJson(apPass, ap["psk"] , 65); //normally not present due to security
   //int ap_pskl = ap[F("pskl")];
   CJSON(apChannel, ap[F("chan")]);
-  if (apChannel > 13 || apChannel < 1) apChannel = 1;
+  if (apChannel > 13 || apChannel < 1) apChannel = 6; // reset to default if invalid
   CJSON(apHide, ap[F("hide")]);
   if (apHide > 1) apHide = 1;
   CJSON(apBehavior, ap[F("behav")]);

wled00/ntp.cpp

@@ -1,6 +1,7 @@
 #include "src/dependencies/timezone/Timezone.h"
 #include "wled.h"
 #include "fcn_declare.h"
+#include "asyncDNS.h"
 
 // WARNING: may cause errors in sunset calculations on ESP8266, see #3400
 // building with `-D WLED_USE_REAL_MATH` will prevent those errors at the expense of flash and RAM
@@ -182,15 +183,50 @@ void handleTime() {
 
 void handleNetworkTime()
 {
+  static AsyncDNS* ntpDNSlookup = nullptr;
   if (ntpEnabled && ntpConnected && millis() - ntpLastSyncTime > (1000*NTP_SYNC_INTERVAL) && WLED_CONNECTED)
   {
     if (millis() - ntpPacketSentTime > 10000)
     {
       #ifdef ARDUINO_ARCH_ESP32   // I had problems using udp.flush() on 8266
       while (ntpUdp.parsePacket() > 0) ntpUdp.flush(); // flush any existing packets
       #endif
-      sendNTPPacket();
-      ntpPacketSentTime = millis();
+      if (!ntpServerIP.fromString(ntpServerName)) // check if server is IP or domain
+      {
+        if (ntpDNSlookup == nullptr) ntpDNSlookup = new AsyncDNS;
+        AsyncDNS::result res = ntpDNSlookup->status();
+        switch (res) {
+          case AsyncDNS::result::Idle:
+            //DEBUG_PRINTF_P(PSTR("Resolving NTP server name: %s\n"), ntpServerName);
+            ntpDNSlookup->query(ntpServerName); // start dnslookup asynchronously
+            return;
+
+          case AsyncDNS::result::Busy:
+            return; // still in progress
+
+          case AsyncDNS::result::Success:
+              ntpServerIP = ntpDNSlookup->getIP();
+              DEBUG_PRINTF_P(PSTR("NTP IP resolved: %s\n"), ntpServerIP.toString().c_str());
+              sendNTPPacket();
+              delete ntpDNSlookup;
+              ntpDNSlookup = nullptr;
+              break;
+
+          case AsyncDNS::result::Error:
+            DEBUG_PRINTLN(F("NTP DNS failed"));
+            ntpDNSlookup->renew(); // try a new lookup next time
+            if (ntpDNSlookup->getErrorCount() > 6) {
+              // after 6 failed attempts (30min), reset network connection as dns is probably stuck (TODO: IDF bug, should be fixed in V5)
+              if (offMode) forceReconnect = true; // do not disturb while LEDs are running
+              delete ntpDNSlookup;
+              ntpDNSlookup = nullptr;
+            }
+            ntpLastSyncTime = millis() - (1000*NTP_SYNC_INTERVAL - 300000); // pause for 5 minutes
+            break;
+        }
+      }
+      else
+        sendNTPPacket();
     }
     if (checkNTPResponse())
     {
@@ -201,14 +237,6 @@ void handleNetworkTime()
 
 void sendNTPPacket()
 {
-  if (!ntpServerIP.fromString(ntpServerName)) //see if server is IP or domain
-  {
-    #ifdef ESP8266
-    WiFi.hostByName(ntpServerName, ntpServerIP, 750);
-    #else
-    WiFi.hostByName(ntpServerName, ntpServerIP);
-    #endif
-  }
 
   DEBUG_PRINTLN(F("send NTP"));
   byte pbuf[NTP_PACKET_SIZE];
@@ -227,6 +255,7 @@ void sendNTPPacket()
   ntpUdp.beginPacket(ntpServerIP, 123); //NTP requests are to port 123
   ntpUdp.write(pbuf, NTP_PACKET_SIZE);
   ntpUdp.endPacket();
+  ntpPacketSentTime = millis();
 }
 
 static bool isValidNtpResponse(const byte* ntpPacket) {