From 30fcd6efadaf538229d834e05029ada83eb4dbb9 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sun, 21 Dec 2025 23:56:09 +0100 Subject: [PATCH] segment & bus.show mutex redesign * separate mutexes for improved performance: - busses.show and effect drawing - critical changes to segment vector - block out critical segment changes while strip.service() loops over segments * UDP realtime acquires mutex before drawing - prevents corrupted outputs, caused by parallel tasks updating pixels while busses.show() runs --- wled00/FX_fcn.cpp | 52 +++++++++++++---------- wled00/json.cpp | 6 ++- wled00/udp.cpp | 105 +++++++++++++++++++++++----------------------- wled00/wled.cpp | 2 + wled00/wled.h | 1 + 5 files changed, 90 insertions(+), 76 deletions(-) diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 72b3b6ce..51318168 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -561,21 +561,26 @@ void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t markForReset(); return; } - if (i1 < Segment::maxWidth || (i1 >= Segment::maxWidth*Segment::maxHeight && i1 < strip.getLengthTotal())) start = i1; // Segment::maxWidth equals strip.getLengthTotal() for 1D - stop = i2 > Segment::maxWidth*Segment::maxHeight ? min(i2,strip.getLengthTotal()) : (i2 > Segment::maxWidth ? Segment::maxWidth : max((uint16_t)1,i2)); // WLEDMM: use native min/max - startY = 0; - stopY = 1; - #ifndef WLED_DISABLE_2D - if (Segment::maxHeight>1) { // 2D - if (i1Y < Segment::maxHeight) startY = i1Y; - stopY = i2Y > Segment::maxHeight ? Segment::maxHeight : max((uint16_t)1,i2Y); // WLEDMM: use native min/max + + if (esp32SemTake(segmentMux, portMAX_DELAY) == pdTRUE) { + // WLEDMM acquire lock before doing critical changes to segment + if (i1 < Segment::maxWidth || (i1 >= Segment::maxWidth*Segment::maxHeight && i1 < strip.getLengthTotal())) start = i1; // Segment::maxWidth equals strip.getLengthTotal() for 1D + stop = i2 > Segment::maxWidth*Segment::maxHeight ? min(i2,strip.getLengthTotal()) : (i2 > Segment::maxWidth ? Segment::maxWidth : max((uint16_t)1,i2)); // WLEDMM: use native min/max + startY = 0; + stopY = 1; + #ifndef WLED_DISABLE_2D + if (Segment::maxHeight>1) { // 2D + if (i1Y < Segment::maxHeight) startY = i1Y; + stopY = i2Y > Segment::maxHeight ? Segment::maxHeight : max((uint16_t)1,i2Y); // WLEDMM: use native min/max + } + #endif + if (grp) { + grouping = grp; + spacing = spc; + } + if (ofs < UINT16_MAX) offset = ofs; + esp32SemGive(segmentMux); } - #endif - if (grp) { - grouping = grp; - spacing = spc; - } - if (ofs < UINT16_MAX) offset = ofs; if (!boundsUnchanged) { markForReset(); @@ -1956,6 +1961,7 @@ void WS2812FX::service() { _isServicing = true; _segment_index = 0; + if (esp32SemTake(segmentMux, 250) == pdTRUE) { // WLEDMM prevent changes to segments while servicing for (segment &seg : _segments) { #ifdef WLEDMM_FASTPATH _currentSeg = &seg; @@ -2012,6 +2018,8 @@ void WS2812FX::service() { } _segment_index++; } + esp32SemGive(segmentMux); + } // end of critical section if (_triggered) doShow = true; // WLEDMM "triggered" always means "show" #ifdef WLEDMM_FASTPATH @@ -2352,7 +2360,7 @@ void WS2812FX::purgeSegments(bool force) { int deleted = 0; if (_segments.size() <= 1) return; // WLEDMM protect against parallel access while drawing - if (esp32SemTake(busDrawMux, 300) != pdTRUE) return; + if (esp32SemTake(segmentMux, 300) != pdTRUE) return; for (size_t i = _segments.size()-1; i > 0; i--) { if (_segments[i].stop == 0 || force) { @@ -2363,7 +2371,7 @@ void WS2812FX::purgeSegments(bool force) { _segments.shrink_to_fit(); /*if (_mainSegment >= _segments.size())*/ setMainSegmentId(0); } - esp32SemGive(busDrawMux); + esp32SemGive(segmentMux); } Segment& WS2812FX::getSegment(uint8_t id) { @@ -2392,7 +2400,7 @@ void WS2812FX::resetSegments(bool boundsOnly) { //WLEDMM add boundsonly DEBUG_PRINTF("resetSegments %d %dx%d\n", boundsOnly, Segment::maxWidth, Segment::maxHeight); if (!boundsOnly) { // WLEDMM protect against parallel access while drawing - if (esp32SemTake(busDrawMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout + if (esp32SemTake(segmentMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout _segments.clear(); // destructs all Segment as part of clearing #ifndef WLED_DISABLE_2D @@ -2402,7 +2410,7 @@ void WS2812FX::resetSegments(bool boundsOnly) { //WLEDMM add boundsonly #endif _segments.push_back(seg); _mainSegment = 0; - esp32SemGive(busDrawMux); + esp32SemGive(segmentMux); } else { //WLEDMM boundsonly for (segment &seg : _segments) { #ifndef WLED_DISABLE_2D @@ -2422,7 +2430,7 @@ void WS2812FX::resetSegments(bool boundsOnly) { //WLEDMM add boundsonly void WS2812FX::makeAutoSegments(bool forceReset) { if (autoSegments) { //make one segment per bus // WLEDMM protect against parallel access while drawing - if (esp32SemTake(busDrawMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout + if (esp32SemTake(segmentMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout uint16_t segStarts[MAX_NUM_SEGMENTS] = {0}; uint16_t segStops [MAX_NUM_SEGMENTS] = {0}; @@ -2472,7 +2480,7 @@ void WS2812FX::makeAutoSegments(bool forceReset) { for (size_t i = 1; i < s; i++) { _segments.push_back(Segment(segStarts[i], segStops[i])); } - esp32SemGive(busDrawMux); + esp32SemGive(segmentMux); } else { if (forceReset || getSegmentsNum() == 0) resetSegments(); @@ -2499,7 +2507,7 @@ void WS2812FX::makeAutoSegments(bool forceReset) { void WS2812FX::fixInvalidSegments() { // WLEDMM protect against parallel access while drawing - if (esp32SemTake(busDrawMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout + if (esp32SemTake(segmentMux, portMAX_DELAY) != pdTRUE) return; // warning: this waits until the mutex becomes availeable, no timeout //make sure no segment is longer than total (sanity check) for (size_t i = getSegmentsNum()-1; i > 0; i--) { @@ -2520,7 +2528,7 @@ void WS2812FX::fixInvalidSegments() { if (_segments[i].stop > _length) _segments[i].stop = _length; } } - esp32SemGive(busDrawMux); // give back the lock now, so purgeSegments can acquire it again + esp32SemGive(segmentMux); // give back the lock now, so purgeSegments can acquire it again // if any segments were deleted free memory purgeSegments(); diff --git a/wled00/json.cpp b/wled00/json.cpp index 5c989b36..fc88cab3 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -94,7 +94,11 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) // if using vectors use this code to append segment if (id >= strip.getSegmentsNum()) { if (stop <= 0) return false; // ignore empty/inactive segments - strip.appendSegment(Segment(0, strip.getLengthTotal())); + if (esp32SemTake(segmentMux, portMAX_DELAY) == pdTRUE) { + // WLEDMM make sure we have exclusive access to the segment list + strip.appendSegment(Segment(0, strip.getLengthTotal())); + esp32SemGive(segmentMux); + } id = strip.getSegmentsNum()-1; // segments are added at the end of list newSeg = true; } diff --git a/wled00/udp.cpp b/wled00/udp.cpp index 378347f7..ae414c28 100644 --- a/wled00/udp.cpp +++ b/wled00/udp.cpp @@ -184,7 +184,10 @@ void realtimeLock(uint32_t timeoutMs, byte md) stop = strip.getLengthTotal(); } // clear strip/segment - for (size_t i = start; i < stop; i++) strip.setPixelColor(i,BLACK); + if (esp32SemTake(busDrawMux, 200) == pdTRUE) { // WLEDMM acquire drawing permission (wait max 200ms) before setting pixels + for (size_t i = start; i < stop; i++) strip.setPixelColor(i,BLACK); + esp32SemGive(busDrawMux); + } // if WLED was off and using main segment only, freeze non-main segments so they stay off if (useMainSegmentOnly && bri == 0) { for (size_t s=0; s < strip.getSegmentsNum(); s++) { @@ -326,10 +329,13 @@ void handleNotifications() #endif uint16_t id = 0; uint16_t totalLen = strip.getLengthTotal(); - for (int i = 0; i < packetSize -2; i += 3) - { - setRealtimePixel(id, lbuf[i], lbuf[i+1], lbuf[i+2], 0); - id++; if (id >= totalLen) break; + if (esp32SemTake(busDrawMux, 200) == pdTRUE) { // WLEDMM acquire drawing permission (wait max 200ms) before setting pixels + for (int i = 0; i < packetSize -2; i += 3) + { + setRealtimePixel(id, lbuf[i], lbuf[i+1], lbuf[i+2], 0); + id++; if (id >= totalLen) break; + } + esp32SemGive(busDrawMux); } if (!(realtimeMode && useMainSegmentOnly)) strip.show(); return; @@ -573,14 +579,16 @@ void handleNotifications() uint16_t id = (tpmPayloadFrameSize/3)*(packetNum-1); //start LED uint16_t totalLen = strip.getLengthTotal(); - for (size_t i = 6; i < tpmPayloadFrameSize + 4U; i += 3) - { - if (id < totalLen) + if (esp32SemTake(busDrawMux, 200) == pdTRUE) { // WLEDMM acquire drawing permission (wait max 200ms) before setting pixels + for (size_t i = 6; i < tpmPayloadFrameSize + 4U; i += 3) { - setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); - id++; + if (id < totalLen) { + setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); + id++; + } + else break; } - else break; + esp32SemGive(busDrawMux); } if (tpmPacketCount == numPackets) //reset packet count and show if all packets were received { @@ -606,49 +614,40 @@ void handleNotifications() } if (realtimeOverride && !(realtimeMode && useMainSegmentOnly)) return; - uint16_t totalLen = strip.getLengthTotal(); - if (udpIn[0] == 1 && packetSize > 5) //warls - { - for (int i = 2; i < packetSize -3; i += 4) - { - setRealtimePixel(udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3], 0); - } - } else if (udpIn[0] == 2 && packetSize > 4) //drgb - { - uint16_t id = 0; - for (int i = 2; i < packetSize -2; i += 3) - { - setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); - - id++; if (id >= totalLen) break; - } - } else if (udpIn[0] == 3 && packetSize > 6) //drgbw - { - uint16_t id = 0; - for (int i = 2; i < packetSize -3; i += 4) - { - setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]); - - id++; if (id >= totalLen) break; - } - } else if (udpIn[0] == 4 && packetSize > 7) //dnrgb - { - uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00); - for (int i = 4; i < packetSize -2; i += 3) - { - if (id >= totalLen) break; - setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); - id++; - } - } else if (udpIn[0] == 5 && packetSize > 8) //dnrgbw - { - uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00); - for (int i = 4; i < packetSize -2; i += 4) - { - if (id >= totalLen) break; - setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]); - id++; + if (esp32SemTake(busDrawMux, 250) == pdTRUE) { // WLEDMM acquire drawing permission (wait max 200ms) before setting pixels + uint16_t totalLen = strip.getLengthTotal(); + if (udpIn[0] == 1 && packetSize > 5) { //warls + for (int i = 2; i < packetSize -3; i += 4) { + setRealtimePixel(udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3], 0); + } + } else if (udpIn[0] == 2 && packetSize > 4) { //drgb + uint16_t id = 0; + for (int i = 2; i < packetSize -2; i += 3) { + setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); + id++; if (id >= totalLen) break; + } + } else if (udpIn[0] == 3 && packetSize > 6) { //drgbw + uint16_t id = 0; + for (int i = 2; i < packetSize -3; i += 4) { + setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]); + id++; if (id >= totalLen) break; + } + } else if (udpIn[0] == 4 && packetSize > 7) { //dnrgb + uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00); + for (int i = 4; i < packetSize -2; i += 3) { + if (id >= totalLen) break; + setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); + id++; + } + } else if (udpIn[0] == 5 && packetSize > 8) { //dnrgbw + uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00); + for (int i = 4; i < packetSize -2; i += 4) { + if (id >= totalLen) break; + setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]); + id++; + } } + esp32SemGive(busDrawMux); // end of critical section } strip.show(); return; diff --git a/wled00/wled.cpp b/wled00/wled.cpp index 6d148bb6..588b8737 100644 --- a/wled00/wled.cpp +++ b/wled00/wled.cpp @@ -478,7 +478,9 @@ void WLED::setup() #ifdef ARDUINO_ARCH_ESP32 busDrawMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent running of strip.show and strip.service + segmentMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent segment changes while effects are running xSemaphoreGiveRecursive(busDrawMux); // init semaphores to initially allow drawing + xSemaphoreGiveRecursive(segmentMux); #endif #ifdef ARDUINO_ARCH_ESP32 diff --git a/wled00/wled.h b/wled00/wled.h index 247b55d5..bd4c8dfd 100644 --- a/wled00/wled.h +++ b/wled00/wled.h @@ -769,6 +769,7 @@ WLED_GLOBAL volatile bool OTAisRunning _INIT(false); // WLEDMM temporaril // WLEDMM prevent concurrent strip.show() and strip.service() -> for DDP over ws, and other background tasks #ifdef ARDUINO_ARCH_ESP32 WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr); +WLED_GLOBAL SemaphoreHandle_t segmentMux _INIT(nullptr); #define esp32SemTake(mux,timeout) xSemaphoreTakeRecursive(mux, timeout) // convenience macro that expands to xSemaphoreTakeRecursive #define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive #else