From c25f0822316b3e7723a060f689c68dacf6fe6429 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Fri, 19 Dec 2025 18:13:20 +0100 Subject: [PATCH] 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 | 14 +++++++++----- wled00/FX_fcn.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- wled00/e131.cpp | 21 +++++++++++++++++++++ wled00/udp.cpp | 3 +-- wled00/wled.cpp | 5 +++++ wled00/wled.h | 5 +++++ 6 files changed, 79 insertions(+), 9 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index d0f3d403..1dcf5b41 100644 --- a/wled00/FX.h +++ b/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; // SRAM footprint: 4 bytes per element diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index d9417063..5380a933 100644 --- a/wled00/FX_fcn.cpp +++ b/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) diff --git a/wled00/e131.cpp b/wled00/e131.cpp index 27ce1e3c..2f34bb4c 100644 --- a/wled00/e131.cpp +++ b/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; diff --git a/wled00/udp.cpp b/wled00/udp.cpp index 3fc630e9..6b01702f 100644 --- a/wled00/udp.cpp +++ b/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 diff --git a/wled00/wled.cpp b/wled00/wled.cpp index 7274f2aa..0f467d93 100644 --- a/wled00/wled.cpp +++ b/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) diff --git a/wled00/wled.h b/wled00/wled.h index 26dca9cd..03825ab0 100644 --- a/wled00/wled.h +++ b/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