POC implementation for deferred settings

Add a basic function queue to the main loop, and have settings-set
operations defer operation to it.   Rejigger the JSON lock as a binary
semaphrore, as it needs to pass ownership from one task to another.

First step towards a global fix for wled#4779.

Author: willmmiles

wled00/fcn_declare.h

@@ -508,7 +508,7 @@ size_t printSetFormIndex(Print& settingsScript, const char* key, int index);
 size_t printSetClassElementHTML(Print& settingsScript, const char* key, const int index, const char* val);
 void prepareHostname(char* hostname);
 [[gnu::pure]] bool isAsterisksOnly(const char* str, byte maxLen);
-bool requestJSONBufferLock(uint8_t moduleID=255);
+bool requestJSONBufferLock(uint8_t moduleID=255, bool wait = true);
 void releaseJSONBufferLock();
 uint8_t extractModeName(uint8_t mode, const char *src, char *dest, uint8_t maxLen);
 uint8_t extractModeSlider(uint8_t mode, uint8_t slider, char *dest, uint8_t maxLen, uint8_t *var = nullptr);

wled00/handler_queue.cpp

@@ -0,0 +1,49 @@
+#include "handler_queue.h"
+#include <deque>
+#include <Arduino.h>
+
+#if defined(ARDUINO_ARCH_ESP32)
+
+static StaticSemaphore_t handlerQueueMutexBuffer;
+static SemaphoreHandle_t handlerQueueMutex = xSemaphoreCreateMutexStatic(&handlerQueueMutexBuffer);
+struct guard_type {
+  SemaphoreHandle_t _mtx;
+  guard_type(SemaphoreHandle_t m) : _mtx(m) {
+    xSemaphoreTake(_mtx, portMAX_DELAY);  // todo: error check
+  }
+  ~guard_type() {    
+    xSemaphoreGive(_mtx);
+  }
+};
+#define guard() const guard_type guard(handlerQueueMutex)
+#else
+#define guard()
+#endif
+
+static std::deque<std::function<void()>> handler_queue;
+
+// Enqueue a function on the main task
+bool HandlerQueue::callOnMainTask(std::function<void()> func) {
+  guard();
+  handler_queue.push_back(std::move(func));
+  return true;  // TODO: queue limit
+}
+
+// Run the next task in the queue, if any
+void HandlerQueue::runNext() {
+  std::function<void()> f;
+  {
+    guard();
+    if (handler_queue.size()) {
+      f = std::move(handler_queue.front());
+      handler_queue.pop_front();
+    }
+  }
+  if (f) {
+    auto t1 = millis();    
+    f();
+    auto t2 = millis();
+    Serial.printf("Handler took: %d\n", t2-t1);
+  } 
+}
+

wled00/handler_queue.h

@@ -0,0 +1,13 @@
+// handler_queue.h
+// deferred execution handler queue.  Used for making access to WLED globals safe.
+
+#include <functional>
+
+namespace HandlerQueue {
+  
+  // Enqueue a function on the main task
+  bool callOnMainTask(std::function<void()> func);
+
+  // Run the next task in the queue, if any
+  void runNext();
+}

wled00/json.cpp

@@ -1123,7 +1123,7 @@ void serveJson(AsyncWebServerRequest* request)
     return;
   }
 
-  if (!requestJSONBufferLock(17)) {
+  if (!requestJSONBufferLock(17, false)) {
     request->deferResponse();    
     return;
   }

wled00/set.cpp

@@ -652,7 +652,7 @@ void handleSettingsSet(AsyncWebServerRequest *request, byte subPage)
   //USERMODS
   if (subPage == SUBPAGE_UM)
   {
-    if (!requestJSONBufferLock(5)) {
+    if (!requestJSONBufferLock(5, false)) {
       request->deferResponse();
       return;
     }

wled00/util.cpp

@@ -151,18 +151,19 @@ bool isAsterisksOnly(const char* str, byte maxLen)
 
 
 //threading/network callback details: https://github.com/wled-dev/WLED/pull/2336#discussion_r762276994
-bool requestJSONBufferLock(uint8_t moduleID)
+bool requestJSONBufferLock(uint8_t moduleID, bool wait)
 {
   if (pDoc == nullptr) {
     DEBUG_PRINTLN(F("ERROR: JSON buffer not allocated!"));
     return false;
   }
 
 #if defined(ARDUINO_ARCH_ESP32)
-  // Use a recursive mutex type in case our task is the one holding the JSON buffer.
-  // This can happen during large JSON web transactions.  In this case, we continue immediately
-  // and then will return out below if the lock is still held.
-  if (xSemaphoreTakeRecursive(jsonBufferLockMutex, 250) == pdFALSE) return false;  // timed out waiting
+  if (xSemaphoreTake(jsonBufferLockMutex, wait ? 250 : 0) == pdFALSE) {
+    // TODO, coaelesce with below
+    DEBUG_PRINTF_P(PSTR("ERROR: Locking JSON buffer mutex (%d) failed! (locked by %d??)\n"), moduleID, jsonBufferLock);
+    return false;  // timed out waiting
+  }
 #elif defined(ARDUINO_ARCH_ESP8266)
   // If we're in system context, delay() won't return control to the user context, so there's
   // no point in waiting.
@@ -176,9 +177,6 @@ bool requestJSONBufferLock(uint8_t moduleID)
   // If the lock is still held - by us, or by another task
   if (jsonBufferLock) {
     DEBUG_PRINTF_P(PSTR("ERROR: Locking JSON buffer (%d) failed! (still locked by %d)\n"), moduleID, jsonBufferLock);
-#ifdef ARDUINO_ARCH_ESP32
-    xSemaphoreGiveRecursive(jsonBufferLockMutex);
-#endif
     return false;
   }
 
@@ -194,7 +192,7 @@ void releaseJSONBufferLock()
   DEBUG_PRINTF_P(PSTR("JSON buffer released. (%d)\n"), jsonBufferLock);
   jsonBufferLock = 0;
 #ifdef ARDUINO_ARCH_ESP32
-  xSemaphoreGiveRecursive(jsonBufferLockMutex);
+  xSemaphoreGive(jsonBufferLockMutex);
 #endif  
 }
 

wled00/wled.cpp

@@ -1,6 +1,7 @@
 #define WLED_DEFINE_GLOBAL_VARS //only in one source file, wled.cpp!
 #include "wled.h"
 #include "wled_ethernet.h"
+#include "handler_queue.h"
 #ifdef WLED_ENABLE_AOTA
   #define NO_OTA_PORT
   #include <ArduinoOTA.h>
@@ -184,6 +185,9 @@ void WLED::loop()
     heapTime = millis();
   }
 
+  // Run next deferred request
+  HandlerQueue::runNext();
+
   //LED settings have been saved, re-init busses
   //This code block causes severe FPS drop on ESP32 with the original "if (busConfigs[0] != nullptr)" conditional. Investigate!
   if (doInitBusses) {
@@ -320,6 +324,7 @@ void WLED::setup()
 
   #ifdef ARDUINO_ARCH_ESP32
   pinMode(hardwareRX, INPUT_PULLDOWN); delay(1);        // suppress noise in case RX pin is floating (at low noise energy) - see issue #3128
+  xSemaphoreGive(jsonBufferLockMutex);                  // Init JSON buffer lock
   #endif
   #ifdef WLED_BOOTUPDELAY
   delay(WLED_BOOTUPDELAY); // delay to let voltage stabilize, helps with boot issues on some setups

wled00/wled.h

@@ -970,7 +970,7 @@ WLED_GLOBAL int8_t spi_sclk  _INIT(SPISCLKPIN);
 // global ArduinoJson buffer
 #if defined(ARDUINO_ARCH_ESP32)
 WLED_GLOBAL JsonDocument *pDoc _INIT(nullptr);
-WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(xSemaphoreCreateRecursiveMutex());
+WLED_GLOBAL SemaphoreHandle_t jsonBufferLockMutex _INIT(xSemaphoreCreateBinary());
 #else
 WLED_GLOBAL StaticJsonDocument<JSON_BUFFER_SIZE> gDoc;
 WLED_GLOBAL JsonDocument *pDoc _INIT(&gDoc);

wled00/wled_server.cpp

@@ -1,4 +1,5 @@
 #include "wled.h"
+#include "handler_queue.h"
 
 #ifndef WLED_DISABLE_OTA
   #ifdef ESP8266
@@ -299,7 +300,7 @@ void initServer()
   });
 
   server.on(F("/settings"), HTTP_POST, [](AsyncWebServerRequest *request){
-    serveSettings(request, true);
+    HandlerQueue::callOnMainTask([=]{ serveSettings(request, true); });    
   });
 
   const static char _json[] PROGMEM = "/json";
@@ -308,10 +309,7 @@ void initServer()
   });
 
   AsyncCallbackJsonWebHandler* handler = new AsyncCallbackJsonWebHandler(FPSTR(_json), [](AsyncWebServerRequest *request) {
-    bool verboseResponse = false;
-    bool isConfig = false;
-
-    if (!requestJSONBufferLock(14)) {
+    if (!requestJSONBufferLock(14, false)) {
       request->deferResponse();
       return;
     }
@@ -325,37 +323,42 @@ void initServer()
     }
     if (root.containsKey("pin")) checkSettingsPIN(root["pin"].as<const char*>());
 
-    const String& url = request->url();
-    isConfig = url.indexOf(F("cfg")) > -1;
-    if (!isConfig) {
-      /*
-      #ifdef WLED_DEBUG
-        DEBUG_PRINTLN(F("Serialized HTTP"));
-        serializeJson(root,Serial);
-        DEBUG_PRINTLN();
-      #endif
-      */
-      verboseResponse = deserializeState(root);
-    } else {
-      if (!correctPIN && strlen(settingsPIN)>0) {
-        releaseJSONBufferLock();
-        serveJsonError(request, 401, ERR_DENIED);
-        return;
-      }
-      verboseResponse = deserializeConfig(root); //use verboseResponse to determine whether cfg change should be saved immediately
-    }
-    releaseJSONBufferLock();
+    HandlerQueue::callOnMainTask([=](){
+      bool verboseResponse = false;
+      bool isConfig = false;
 
-    if (verboseResponse) {
+      const String& url = request->url();
+      isConfig = url.indexOf(F("cfg")) > -1;
       if (!isConfig) {
-        lastInterfaceUpdate = millis(); // prevent WS update until cooldown
-        interfaceUpdateCallMode = CALL_MODE_WS_SEND; // schedule WS update
-        serveJson(request); return; //if JSON contains "v"
+        /*
+        #ifdef WLED_DEBUG
+          DEBUG_PRINTLN(F("Serialized HTTP"));
+          serializeJson(root,Serial);
+          DEBUG_PRINTLN();
+        #endif
+        */
+        verboseResponse = deserializeState(root);
       } else {
-        configNeedsWrite = true; //Save new settings to FS
+        if (!correctPIN && strlen(settingsPIN)>0) {
+          releaseJSONBufferLock();
+          serveJsonError(request, 401, ERR_DENIED);
+          return;
+        }
+        verboseResponse = deserializeConfig(root); //use verboseResponse to determine whether cfg change should be saved immediately
       }
-    }
-    request->send(200, CONTENT_TYPE_JSON, F("{\"success\":true}"));
+      releaseJSONBufferLock();
+
+      if (verboseResponse) {
+        if (!isConfig) {
+          lastInterfaceUpdate = millis(); // prevent WS update until cooldown
+          interfaceUpdateCallMode = CALL_MODE_WS_SEND; // schedule WS update
+          serveJson(request); return; //if JSON contains "v"
+        } else {
+          configNeedsWrite = true; //Save new settings to FS
+        }
+      }
+      request->send(200, CONTENT_TYPE_JSON, F("{\"success\":true}"));
+    });
   }, JSON_BUFFER_SIZE);
   server.addHandler(handler);
 
@@ -509,7 +512,10 @@ void initServer()
       return;
     }
 
-    if(handleSet(request, request->url())) return;
+    if (request->url().indexOf("win") >= 0) {
+      HandlerQueue::callOnMainTask([=]() { handleSet(request, request->url()); });
+      return;
+    }
     #ifndef WLED_DISABLE_ALEXA
     if(espalexa.handleAlexaApiCall(request)) return;
     #endif

wled00/ws.cpp

@@ -1,4 +1,5 @@
 #include "wled.h"
+#include "handler_queue.h"
 
 /*
  * WebSockets server for bidirectional communication
@@ -34,8 +35,7 @@ void wsEvent(AsyncWebSocket * server, AsyncWebSocketClient * client, AwsEventTyp
           client->text(F("pong"));
           return;
         }
-
-        bool verboseResponse = false;
+        
         if (!requestJSONBufferLock(11)) {
           client->text(F("{\"error\":3}")); // ERR_NOBUF
           return;
@@ -47,26 +47,31 @@ void wsEvent(AsyncWebSocket * server, AsyncWebSocketClient * client, AwsEventTyp
           releaseJSONBufferLock();
           return;
         }
-        if (root["v"] && root.size() == 1) {
-          //if the received value is just "{"v":true}", send only to this client
-          verboseResponse = true;
-        } else if (root.containsKey("lv")) {
-          wsLiveClientId = root["lv"] ? client->id() : 0;
-        } else {
-          verboseResponse = deserializeState(root);
-        }
-        releaseJSONBufferLock();
 
-        if (!interfaceUpdateCallMode) { // individual client response only needed if no WS broadcast soon
-          if (verboseResponse) {
-            sendDataWs(client);
+        // Handle the rest on the main task
+        HandlerQueue::callOnMainTask([=](){
+          bool verboseResponse = false;
+          if (root["v"] && root.size() == 1) {
+            //if the received value is just "{"v":true}", send only to this client
+            verboseResponse = true;
+          } else if (root.containsKey("lv")) {
+            wsLiveClientId = root["lv"] ? client->id() : 0;
           } else {
-            // we have to send something back otherwise WS connection closes
-            client->text(F("{\"success\":true}"));
+            verboseResponse = deserializeState(root);
           }
-          // force broadcast in 500ms after updating client
-          //lastInterfaceUpdate = millis() - (INTERFACE_UPDATE_COOLDOWN -500); // ESP8266 does not like this
-        }
+          releaseJSONBufferLock();
+
+          if (!interfaceUpdateCallMode) { // individual client response only needed if no WS broadcast soon
+            if (verboseResponse) {
+              sendDataWs(client);
+            } else {
+              // we have to send something back otherwise WS connection closes
+              client->text(F("{\"success\":true}"));
+            }
+            // force broadcast in 500ms after updating client
+            //lastInterfaceUpdate = millis() - (INTERFACE_UPDATE_COOLDOWN -500); // ESP8266 does not like this
+          }
+        });
       }
     } else {
       //message is comprised of multiple frames or the frame is split into multiple packets