improve stability of DDP via tcp_task (websockets)

* protect pixel buffer changes with critical section
* protect segment list modification with critical section
* make _isServicing and _triggered atomic (move out of packed bitarray)
* allow custom timeout for waitUntilIdle - DDP needs a longer timeout
* protect strip.show() and effect drawing functions with mutex, to prevent crashes

wled00/FX.h

@@ -984,7 +984,7 @@ class WS2812FX {  // 96 bytes
       printSize(),
 #endif
       finalizeInit(),
-      waitUntilIdle(void),   // WLEDMM
+      waitUntilIdle(unsigned timeout = 0),   // WLEDMM
       service(void),
       setMode(uint8_t segid, uint8_t m),
       setColor(uint8_t slot, uint32_t c),
@@ -1037,8 +1037,8 @@ class WS2812FX {  // 96 bytes
       getActiveSegmentsNum(void)  const,
       __attribute__((pure)) getFirstSelectedSegId(void),
       getLastActiveSegmentId(void) const,
-      __attribute__((pure)) getActiveSegsLightCapabilities(bool selectedOnly = false),
-      setPixelSegment(uint8_t n);
+      __attribute__((pure)) getActiveSegsLightCapabilities(bool selectedOnly = false);
+      //setPixelSegment(uint8_t n);
 
     inline uint8_t getBrightness(void)  const { return _brightness; }
     inline uint8_t getSegmentsNum(void)  const { return _segments.size(); }  // returns currently present segments
@@ -1247,12 +1247,16 @@ class WS2812FX {  // 96 bytes
 #endif
 
     // will require only 1 byte
+    volatile bool _isServicing; // WLEDMM moved out of struct, so the flag can be updated in one atomic access
     struct {
-      bool _isServicing          : 1;
+      //bool _isServicing          : 1;
+      bool bullshit              : 1;
       bool _isOffRefreshRequired : 1; //periodic refresh is required for the strip to remain off.
       bool _hasWhiteChannel      : 1;
-      bool _triggered            : 1;
+      //bool _triggered            : 1;
+      bool bullshit2             : 1;
     };
+    volatile bool _triggered;   // WLEDMM moved out of struct, so the flag can be updated in one atomic access
 
     uint8_t                  _modeCount;
     std::vector<mode_ptr>    _mode;     // SRAM footprint: 4 bytes per element

wled00/FX_fcn.cpp

@@ -116,9 +116,18 @@ void Segment::allocLeds() {
   }
   if ((size > 0) && (!ledsrgb || size > ledsrgbSize)) {    //softhack dont allocate zero bytes
     DEBUG_PRINTF("allocLeds (%d,%d to %d,%d), %u from %u\n", start, startY, stop, stopY, size, ledsrgb?ledsrgbSize:0);
+
+    #if defined(ARDUINO_ARCH_ESP32) // WLEDMM protect against parallel access
+    portENTER_CRITICAL(&s_wled_strip_mux);
+    #endif
     if (ledsrgb) free(ledsrgb);   // we need a bigger buffer, so free the old one first
+    ledsrgb = nullptr;
     ledsrgb = (CRGB*)calloc(size, 1);
     ledsrgbSize = ledsrgb?size:0;
+    #if defined(ARDUINO_ARCH_ESP32) // WLEDMM protect against parallel access
+    portEXIT_CRITICAL(&s_wled_strip_mux);
+    #endif
+
     if (ledsrgb == nullptr) {
       USER_PRINTLN("allocLeds failed!!");
       errorFlag = ERR_LOW_BUF; // WLEDMM raise errorflag
@@ -1904,14 +1913,15 @@ void WS2812FX::finalizeInit(void)
 // on 8266 this function does nothing, because we can only do "busy waiting" on ESP32
 //#define MAX_IDLE_WAIT_MS 50  // seems to work in most cases
 #define MAX_IDLE_WAIT_MS 120   // better safe than sorry - similar to the timeout used by upstream WLED
-void WS2812FX::waitUntilIdle(void) {
+void WS2812FX::waitUntilIdle(unsigned timeout) {
 #if defined(ARDUINO_ARCH_ESP32) && defined(WLEDMM_PROTECT_SERVICE)
+  if (timeout < MAX_IDLE_WAIT_MS) timeout = MAX_IDLE_WAIT_MS;
   if (isServicing()) {
     unsigned long waitStarted = millis();
     do {
       delay(2);  // Suspending for 1 tick (or more) gives other tasks a chance to run.
       //yield(); // seems to be a no-op on esp32
-    } while (isServicing() && (millis() - waitStarted < MAX_IDLE_WAIT_MS));
+    } while (isServicing() && (millis() - waitStarted < timeout));
     DEBUG_PRINTF("strip.waitUntilIdle(): strip %sidle after %d ms. (task %s with prio=%d)\n", isServicing()?"not ":"", int(millis() - waitStarted), pcTaskGetTaskName(NULL), uxTaskPriorityGet(NULL));
     if (isServicing()) USER_PRINTF("strip.waitUntilIdle(): strip NOT idle after %d ms - overriding access. (task %s with prio=%d)\n", int(millis() - waitStarted), pcTaskGetTaskName(NULL), uxTaskPriorityGet(NULL));
   }
@@ -1924,6 +1934,10 @@ void WS2812FX::waitUntilIdle(void) {
 void WS2812FX::service() {
   unsigned long nowUp = millis(); // Be aware, millis() rolls over every 49 days // WLEDMM avoid losing precision
   if (OTAisRunning) return; // WLEDMM avoid flickering during OTA
+ 
+  //#ifdef ARDUINO_ARCH_ESP32
+  //if ((_isServicing == true) && (strncmp(pcTaskGetTaskName(NULL), "loopTask", 8) != 0)) return; // WLEDMM experimental: not in looptask context - avoid self-blocking (DDP over webSockets)
+  //#endif
 
   now = nowUp + timebase;
   unsigned long elapsed = nowUp - _lastServiceShow;
@@ -1980,6 +1994,9 @@ void WS2812FX::service() {
         // effect blending (execute previous effect)
         // actual code may be a bit more involved as effects have runtime data including allocated memory
         //if (seg.transitional && seg._modeP) (*_mode[seg._modeP])(progress());
+        #if defined(ARDUINO_ARCH_ESP32) // WLEDMM protect against parallel drawing
+        if (xSemaphoreTake(busDrawMux, 200) != pdTRUE) { delay(1); continue;}      // WLEDMM first acquire draw mutex
+        #endif
         frameDelay = (*_mode[seg.currentMode(seg.mode)])();
 
         if (frameDelay < speedLimit) frameDelay = FRAMETIME;                    // WLEDMM limit effects that want to go faster than target FPS
@@ -1990,6 +2007,9 @@ void WS2812FX::service() {
 
         seg.lastBri = seg.currentBri(seg.on ? seg.opacity:0);                   // WLEDMM remember for next time
         seg.handleTransition();
+        #if defined(ARDUINO_ARCH_ESP32)
+        xSemaphoreGive(busDrawMux); // WLEDMM unlock mutex
+        #endif
       }
 
       seg.next_time = nowUp + frameDelay;
@@ -2129,7 +2149,14 @@ void WS2812FX::show(void) {
   // some buses send asynchronously and this method will return before
   // all of the data has been sent.
   // See https://github.com/Makuna/NeoPixelBus/wiki/ESP32-NeoMethods#neoesp32rmt-methods
+
+  #if defined(ARDUINO_ARCH_ESP32) // WLEDMM protect against parallel access
+  if (xSemaphoreTake(busDrawMux, 200) != pdTRUE) { delay(1); return;}      // WLEDMM first acquire drawing permission (mutex), wait max 200ms
+  #endif
   busses.show();
+  #if defined(ARDUINO_ARCH_ESP32)
+  xSemaphoreGive(busDrawMux);                                              // WLEDMM return permissions
+  #endif
 
   unsigned long diff = showNow - _lastShow;
   uint16_t fpsCurr = 200;
@@ -2464,6 +2491,9 @@ void WS2812FX::makeAutoSegments(bool forceReset) {
 }
 
 void WS2812FX::fixInvalidSegments() {
+  #if defined(ARDUINO_ARCH_ESP32)          // WLEDMM protect against parallel access
+  portENTER_CRITICAL(&s_wled_strip_mux);   // this locks out all parallel tasks, including PSRAM and flash filesystem access
+  #endif
   //make sure no segment is longer than total (sanity check)
   for (size_t i = getSegmentsNum()-1; i > 0; i--) {
     if (isMatrix) {
@@ -2488,6 +2518,10 @@ void WS2812FX::fixInvalidSegments() {
   // this is always called as the last step after finalizeInit(), update covered bus types
   for (segment &seg : _segments)
     seg.refreshLightCapabilities();
+
+  #if defined(ARDUINO_ARCH_ESP32)
+  portEXIT_CRITICAL(&s_wled_strip_mux);
+  #endif
 }
 
 //true if all segments align with a bus, or if a segment covers the total length
@@ -2505,6 +2539,7 @@ bool WS2812FX::checkSegmentAlignment() {
   return true;
 }
 
+#if 0 // WLEDMM dead code
 //After this function is called, setPixelColor() will use that segment (offsets, grouping, ... will apply)
 //Note: If called in an interrupt (e.g. JSON API), original segment must be restored,
 //otherwise it can lead to a crash on ESP32 because _segment_index is modified while in use by the main thread
@@ -2516,6 +2551,7 @@ uint8_t WS2812FX::setPixelSegment(uint8_t n) {
   }
   return prevSegId;
 }
+#endif
 
 void WS2812FX::setRange(uint16_t i, uint16_t i2, uint32_t col) {
   if (i2 >= i)

wled00/e131.cpp

@@ -16,6 +16,7 @@ void handleDDPPacket(e131_packet_t* p) {
   int lastPushSeq = e131LastSequenceNumber[0];
 
   //reject late packets belonging to previous frame (assuming 4 packets max. before push)
+#if 0  // WLEDMM fixme - we definitely have more than 5-10 packets per frame !!!
   if (e131SkipOutOfSequence && lastPushSeq) {
     int sn = p->sequenceNum & 0xF;
     if (sn) {
@@ -26,9 +27,15 @@ void handleDDPPacket(e131_packet_t* p) {
       }
     }
   }
+#endif
 
   uint8_t ddpChannelsPerLed = ((p->dataType & 0b00111000)>>3 == 0b011) ? 4 : 3; // data type 0x1B (formerly 0x1A) is RGBW (type 3, 8 bit/channel)
 
+  // WLEDMM for debugging
+  static unsigned lastPush = millis();
+  static unsigned packets = 0;
+  static unsigned pixels = 0;
+
   uint32_t start =  htonl(p->channelOffset) / ddpChannelsPerLed;
   start += DMXAddress / ddpChannelsPerLed;
   uint16_t dataLen = htons(p->dataLen);
@@ -48,15 +55,29 @@ void handleDDPPacket(e131_packet_t* p) {
   realtimeLock(realtimeTimeoutMs, REALTIME_MODE_DDP);
 
   if (!realtimeOverride || (realtimeMode && useMainSegmentOnly)) {
+    #if defined(ARDUINO_ARCH_ESP32)
+    if (xSemaphoreTake(busDrawMux, 200) != pdTRUE) { delay(1);}  // WLEDMM first acquire drawing permission (wait max 200ms)
+    #endif
     for (uint16_t i = start; i < stop; i++) {
       setRealtimePixel(i, data[c], data[c+1], data[c+2], ddpChannelsPerLed >3 ? data[c+3] : 0);
       c += ddpChannelsPerLed;
+      pixels++;
     }
+    packets ++;
+    #if defined(ARDUINO_ARCH_ESP32)
+    xSemaphoreGive(busDrawMux);                                  // WLEDMM release drawing permissions
+    #endif
   }
 
   bool push = p->flags & DDP_PUSH_FLAG;
   ddpSeenPush |= push;
   if (!ddpSeenPush || push) { // if we've never seen a push, or this is one, render display
+    #ifdef WLED_DEBUG
+      if (push) { USER_PRINT("-P-");} else { USER_PRINT("--");}
+      USER_PRINTF("> %dms (%upck, %upix) ", int(millis() - lastPush), packets, pixels);
+      lastPush = millis();
+      pixels = packets = 0;
+    #endif
     e131NewData = true;
     byte sn = p->sequenceNum & 0xF;
     if (sn) e131LastSequenceNumber[0] = sn;

wled00/udp.cpp

@@ -162,9 +162,8 @@ void realtimeLock(uint32_t timeoutMs, byte md)
 
     if (strip.isServicing()) {
       USER_PRINTLN(F("realtimeLock() entering RTM: strip is still drawing effects."));
-      strip.waitUntilIdle();
+      strip.waitUntilIdle(350);
     }
-    strip.service(); // WLEDMM make sure that all segments are properly initialized
     busses.invalidateCache(true);
     // WLEDMM end
 

wled00/wled.cpp

@@ -476,6 +476,11 @@ void WLED::setup()
 
   init_math();  // WLEDMM: pre-calculate some lookup tables
 
+  #ifdef ARDUINO_ARCH_ESP32
+  busDrawMux = xSemaphoreCreateBinary();      // WLEDMM prevent concurrent running of strip.show and strip.service
+  xSemaphoreGive(busDrawMux);                 // init semaphores to initially allow drawing
+  #endif
+
   #ifdef ARDUINO_ARCH_ESP32
   #if defined(WLED_DEBUG) && (defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) || ARDUINO_USB_CDC_ON_BOOT)
   if (!Serial) delay(2500);  // WLEDMM allow CDC USB serial to initialise (WLED_DEBUG only)

wled00/wled.h

@@ -765,6 +765,11 @@ WLED_GLOBAL volatile bool loadLedmap _INIT(false);          // WLEDMM use as boo
 WLED_GLOBAL volatile uint8_t loadedLedmap _INIT(0);         // WLEDMM default 0
 WLED_GLOBAL volatile bool suspendStripService _INIT(false); // WLEDMM temporarily prevent running strip.service, when strip or segments are "under update" and inconsistent
 WLED_GLOBAL volatile bool OTAisRunning _INIT(false);        // WLEDMM temporarily stop led updates during OTA
+
+#ifdef ARDUINO_ARCH_ESP32
+WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr);    // WLEDMM prevent concurrent strip.show() and strip.service() -> for DDP over ws
+#endif
+
 #ifndef ESP8266
 WLED_GLOBAL char  *ledmapNames[WLED_MAX_LEDMAPS-1] _INIT_N(({nullptr}));
 #endif