waitUntilIdle() improvements

* protect deserializeMap()
* better handling of recursion in deserializeSegment() and deserializeState
* don't use yield() on esp32

wled00/FX.h

@@ -31,6 +31,8 @@
 
 #include "const.h"
 
+void strip_wait_until_idle(String whoCalledMe); // WLEDMM implemented in FX_fcn.cpp
+
 #define FASTLED_INTERNAL //remove annoying pragma messages
 #define USE_GET_MILLISECOND_TIMER
 #include "FastLED.h"
@@ -494,6 +496,12 @@ typedef struct Segment {
       if (ledsrgb) Serial.printf(" [%u]", length()*sizeof(CRGB));
       Serial.println();
       //#endif
+
+      // WLEDMM only delete segments when they are not in use
+      #ifdef ARDUINO_ARCH_ESP32
+      strip_wait_until_idle("~Segment()");
+      #endif
+
       if (!Segment::_globalLeds && ledsrgb) free(ledsrgb);
       if (name) delete[] name;
       if (_t) delete _t;
@@ -721,6 +729,9 @@ class WS2812FX {  // 96 bytes
     }
 
     ~WS2812FX() {
+      //#ifdef WLED_DEBUG
+      if (Serial) Serial.println(F("~WS2812FX destroying strip.")); // WLEDMM can't use DEBUG_PRINTLN here
+      //#endif
       if (customMappingTable) delete[] customMappingTable;
       _mode.clear();
       _modeData.clear();

wled00/FX_fcn.cpp

@@ -72,6 +72,15 @@
   #error "Max segments must be at least max number of busses!"
 #endif
 
+// WLEDMM experimental . this is a "C style" wrapper for strip.waitUntilIdle()
+// This workaround is just needed for the segment class, that does't know about "strip"
+void strip_wait_until_idle(String whoCalledMe) {
+  if (strip.isServicing() && (strncmp(pcTaskGetTaskName(NULL), "loopTask", 8) != 0)) { // if we are in looptask (arduino loop), its safe to proceed without waiting
+  USER_PRINTLN(whoCalledMe + String(": strip is still drawing effects, waiting ..."));
+  strip.waitUntilIdle();
+  }
+}
+
 
 ///////////////////////////////////////////////////////////////////////////////
 // Segment class implementation
@@ -1515,8 +1524,8 @@ void WS2812FX::waitUntilIdle(void) {
   if (isServicing()) {
     unsigned long waitStarted = millis();
     do {
-      delay(1);
+      delay(1);  // Suspending for 1 tick or more gives other tasks a chance to run.
-      yield();
+      //yield(); // seems to be a no-op on esp32
     } while (isServicing() && (millis() - waitStarted < MAX_IDLE_WAIT_MS));
     USER_PRINTF("strip.waitUntilIdle(): strip %sidle after %d ms. (task %s)\n", isServicing()?"not ":"", int(millis() - waitStarted), pcTaskGetTaskName(NULL));
   }
@@ -2154,6 +2163,12 @@ bool WS2812FX::deserializeMap(uint8_t n) {
 
   if (!requestJSONBufferLock(7)) return false;
 
+  // WLEDMM: before changing maps, make sure our strip is _not_ servicing effects in parallel
+  if (strip.isServicing()) {
+    USER_PRINTLN(F("deserializeMap(): strip is still drawing effects, waiting ..."));
+    strip.waitUntilIdle();
+  }
+
   //WLEDMM: change upstream code: do not load complete ledmaps in json as this blows up memory, use file read instead
   //read the file
   File f;

wled00/json.cpp

@@ -46,8 +46,11 @@
  * JSON API (De)serialization
  */
 
+static bool inDeepCall = false; // WLEDMM needed so that recursive deserializeSegment() does not remove locks too early
+
 bool deserializeSegment(JsonObject elem, byte it, byte presetId)
 {
+  const bool iAmGroot = !inDeepCall;  // WLEDMM will only be true if this is the toplevel of the recursion.
   //WLEDMM add USER_PRINT
   if (elem.size()!=1 || elem["stop"] != 0) { // not for {"stop":0}
     String temp;
@@ -118,9 +121,11 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
       elem["start"] = start;
       elem["stop"]  = start + len;
       elem["rev"]   = !elem["rev"]; // alternate reverse on even/odd segments
-      deserializeSegment(elem, i, presetId); // recursive call with new id
+      inDeepCall = true;  // WLEDMM remember that we are going into recursion
+      deserializeSegment(elem, i, presetId); // recursive call with new id // WLEDMM expect problems like heap overflow
+      if (iAmGroot) inDeepCall = false;  // WLEDMM toplevel -> reset recursion flag
     }
-    suspendStripService = false; // WLEDMM release lock
+    if (iAmGroot) suspendStripService = false; // WLEDMM release lock
     return true;
   }
 
@@ -179,7 +184,7 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
   seg.set(start, stop, grp, spc, of, startY, stopY);
 
   if (seg.reset && seg.stop == 0) {
-    suspendStripService = false; // WLEDMM release lock
+    if (iAmGroot) suspendStripService = false; // WLEDMM release lock
     return true; // segment was deleted & is marked for reset, no need to change anything else
   }
 
@@ -345,7 +350,7 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
   // send UDP/WS if segment options changed (except selection; will also deselect current preset)
   if (seg.differs(prev) & 0x7F) stateChanged = true;
 
-  suspendStripService = false; // WLEDMM release lock
+  if (iAmGroot) suspendStripService = false; // WLEDMM release lock
   return true;
 }
 
@@ -353,6 +358,7 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
 // presetId is non-0 if called from handlePreset()
 bool deserializeState(JsonObject root, byte callMode, byte presetId)
 {
+  const bool iAmGroot = !inDeepCall;  // WLEDMM will only be true if this is the toplevel of the recursion.
   //WLEDMM add USER_PRINT
   String temp;
   serializeJson(root, temp);
@@ -471,22 +477,28 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId)
       for (size_t s = 0; s < strip.getSegmentsNum(); s++) {
         Segment &sg = strip.getSegment(s);
         if (sg.isSelected()) {
+          inDeepCall = true;  // WLEDMM remember that we are going into recursion
           deserializeSegment(segVar, s, presetId);
+          if (iAmGroot) inDeepCall = false;  // WLEDMM toplevel -> reset recursion flag
           //didSet = true;
         }
       }
       //TODO: not sure if it is good idea to change first active but unselected segment
       //if (!didSet) deserializeSegment(segVar, strip.getMainSegmentId(), presetId);
     } else {
+      inDeepCall = true;  // WLEDMM remember that we are going into recursion
       deserializeSegment(segVar, id, presetId); //apply only the segment with the specified ID
+      if (iAmGroot) inDeepCall = false;  // WLEDMM toplevel -> reset recursion flag
     }
   } else {
     size_t deleted = 0;
     JsonArray segs = segVar.as<JsonArray>();
+    inDeepCall = true;  // WLEDMM remember that we are going into recursion
     for (JsonObject elem : segs) {
       if (deserializeSegment(elem, it++, presetId) && !elem["stop"].isNull() && elem["stop"]==0) deleted++;
     }
     if (strip.getSegmentsNum() > 3 && deleted >= strip.getSegmentsNum()/2U) strip.purgeSegments(); // batch deleting more than half segments
+    if (iAmGroot) inDeepCall = false;  // WLEDMM toplevel -> reset recursion flag
   }
 
   usermods.readFromJsonState(root);
@@ -524,7 +536,7 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId)
       presetCycCurr = ps;
       unloadPlaylist();          // applying a preset unloads the playlist
       applyPreset(ps, callMode); // async load from file system (only preset ID was specified)
-      suspendStripService = false; // WLEDMM release lock
+      if (iAmGroot) suspendStripService = false; // WLEDMM release lock
       return stateResponse;
     }
   }
@@ -548,7 +560,7 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId)
   stateUpdated(callMode);
   if (presetToRestore) currentPreset = presetToRestore;
 
-  suspendStripService = false; // WLEDMM release lock
+  if (iAmGroot) suspendStripService = false; // WLEDMM release lock
   return stateResponse;
 }