implement async DNS lookup - no more stuck NTP requests

wled00/asyncDNS.h

@@ -0,0 +1,93 @@
+#pragma once
+/*
+  asyncDNS.h - wrapper class for asynchronous DNS lookups using lwIP
+  by @dedehai
+*/
+
+#include <Arduino.h>
+#include <atomic>
+#include <memory>
+#include <lwip/dns.h>
+#include <lwip/err.h>
+
+
+class AsyncDNS {
+
+  // C++14 shim
+#if __cplusplus < 201402L
+  // Really simple C++11 shim for non-array case; implementation from cppreference.com
+  template<class T, class... Args>
+  static std::unique_ptr<T>
+  make_unique(Args&&... args)
+  {
+      return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+  }
+#endif
+
+  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
+  static std::shared_ptr<AsyncDNS> query(const char* hostname, std::shared_ptr<AsyncDNS> current = {}) {    
+    if (!current || (current->_status == result::Busy)) {
+      current.reset(new AsyncDNS());
+    }
+
+#if __cplusplus >= 201402L
+    using std::make_unique;
+#endif
+
+    std::unique_ptr<std::shared_ptr<AsyncDNS>> callback_state = make_unique<std::shared_ptr<AsyncDNS> >(current);
+    if (!callback_state) return {};
+
+    current->_status = result::Busy;
+    err_t err = dns_gethostbyname(hostname, &current->_raw_addr, _dns_callback, callback_state.get());
+    if (err == ERR_OK) {
+      current->_status = result::Success; // result already in cache
+    } else if (err == ERR_INPROGRESS) {
+      callback_state.release(); // belongs to the callback now
+    } else {
+      Serial.printf("DNS fail: %d\n", err);
+      current->_status = result::Error;
+      current->_errorcount++;
+    }
+    return current;
+  }
+
+  // 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 reset() { _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;
+
+  AsyncDNS(){}; // may not be explicitly instantiated - use query()
+
+  // 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) {
+    std::shared_ptr<AsyncDNS>* instance_ptr = reinterpret_cast<std::shared_ptr<AsyncDNS>*>(arg);
+    AsyncDNS& instance = **instance_ptr;
+    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++;
+    }
+    delete instance_ptr;
+  }
+};

wled00/bus_manager.cpp

@@ -712,13 +712,23 @@ 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 std::shared_ptr<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 = AsyncDNS::query(_hostname.c_str(), DNSlookup); // 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 +1310,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,8 @@
 #include "src/dependencies/timezone/Timezone.h"
 #include "wled.h"
 #include "fcn_declare.h"
+#include "asyncDNS.h"
+#include <memory>
 
 // 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
@@ -180,17 +182,53 @@ void handleTime() {
   }
 }
 
+
 void handleNetworkTime()
 {
+  static std::shared_ptr<AsyncDNS> ntpDNSlookup;
+
   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
+      {
+        AsyncDNS::result res = ntpDNSlookup ? ntpDNSlookup->status() : AsyncDNS::result::Idle;
+        switch (res) {
+          case AsyncDNS::result::Idle:
+            //DEBUG_PRINTF_P(PSTR("Resolving NTP server name: %s\n"), ntpServerName);
+            ntpDNSlookup = AsyncDNS::query(ntpServerName, ntpDNSlookup); // 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();
+            ntpDNSlookup.reset();
+            break;
+
+          case AsyncDNS::result::Error:
+            DEBUG_PRINTLN(F("NTP DNS failed"));
+            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
+              ntpDNSlookup.reset();
+            } else {
+              // Retry
+              ntpDNSlookup = AsyncDNS::query(ntpServerName, ntpDNSlookup);
+            }
+            ntpLastSyncTime = millis() - (1000*NTP_SYNC_INTERVAL - 300000); // pause for 5 minutes
+            break;
+        }
+      }
+      else
+        sendNTPPacket();
     }
     if (checkNTPResponse())
     {
@@ -201,14 +239,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 +257,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) {