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
This commit is contained in:
Frank
2025-12-21 23:56:09 +01:00
parent c520d75729
commit 30fcd6efad
5 changed files with 90 additions and 76 deletions

View File

@@ -561,21 +561,26 @@ void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t
markForReset(); markForReset();
return; 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 if (esp32SemTake(segmentMux, portMAX_DELAY) == pdTRUE) {
startY = 0; // WLEDMM acquire lock before doing critical changes to segment
stopY = 1; if (i1 < Segment::maxWidth || (i1 >= Segment::maxWidth*Segment::maxHeight && i1 < strip.getLengthTotal())) start = i1; // Segment::maxWidth equals strip.getLengthTotal() for 1D
#ifndef WLED_DISABLE_2D 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
if (Segment::maxHeight>1) { // 2D startY = 0;
if (i1Y < Segment::maxHeight) startY = i1Y; stopY = 1;
stopY = i2Y > Segment::maxHeight ? Segment::maxHeight : max((uint16_t)1,i2Y); // WLEDMM: use native min/max #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) { if (!boundsUnchanged) {
markForReset(); markForReset();
@@ -1956,6 +1961,7 @@ void WS2812FX::service() {
_isServicing = true; _isServicing = true;
_segment_index = 0; _segment_index = 0;
if (esp32SemTake(segmentMux, 250) == pdTRUE) { // WLEDMM prevent changes to segments while servicing
for (segment &seg : _segments) { for (segment &seg : _segments) {
#ifdef WLEDMM_FASTPATH #ifdef WLEDMM_FASTPATH
_currentSeg = &seg; _currentSeg = &seg;
@@ -2012,6 +2018,8 @@ void WS2812FX::service() {
} }
_segment_index++; _segment_index++;
} }
esp32SemGive(segmentMux);
} // end of critical section
if (_triggered) doShow = true; // WLEDMM "triggered" always means "show" if (_triggered) doShow = true; // WLEDMM "triggered" always means "show"
#ifdef WLEDMM_FASTPATH #ifdef WLEDMM_FASTPATH
@@ -2352,7 +2360,7 @@ void WS2812FX::purgeSegments(bool force) {
int deleted = 0; int deleted = 0;
if (_segments.size() <= 1) return; if (_segments.size() <= 1) return;
// WLEDMM protect against parallel access while drawing // 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--) { for (size_t i = _segments.size()-1; i > 0; i--) {
if (_segments[i].stop == 0 || force) { if (_segments[i].stop == 0 || force) {
@@ -2363,7 +2371,7 @@ void WS2812FX::purgeSegments(bool force) {
_segments.shrink_to_fit(); _segments.shrink_to_fit();
/*if (_mainSegment >= _segments.size())*/ setMainSegmentId(0); /*if (_mainSegment >= _segments.size())*/ setMainSegmentId(0);
} }
esp32SemGive(busDrawMux); esp32SemGive(segmentMux);
} }
Segment& WS2812FX::getSegment(uint8_t id) { 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); DEBUG_PRINTF("resetSegments %d %dx%d\n", boundsOnly, Segment::maxWidth, Segment::maxHeight);
if (!boundsOnly) { if (!boundsOnly) {
// WLEDMM protect against parallel access while drawing // 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 _segments.clear(); // destructs all Segment as part of clearing
#ifndef WLED_DISABLE_2D #ifndef WLED_DISABLE_2D
@@ -2402,7 +2410,7 @@ void WS2812FX::resetSegments(bool boundsOnly) { //WLEDMM add boundsonly
#endif #endif
_segments.push_back(seg); _segments.push_back(seg);
_mainSegment = 0; _mainSegment = 0;
esp32SemGive(busDrawMux); esp32SemGive(segmentMux);
} else { //WLEDMM boundsonly } else { //WLEDMM boundsonly
for (segment &seg : _segments) { for (segment &seg : _segments) {
#ifndef WLED_DISABLE_2D #ifndef WLED_DISABLE_2D
@@ -2422,7 +2430,7 @@ void WS2812FX::resetSegments(bool boundsOnly) { //WLEDMM add boundsonly
void WS2812FX::makeAutoSegments(bool forceReset) { void WS2812FX::makeAutoSegments(bool forceReset) {
if (autoSegments) { //make one segment per bus if (autoSegments) { //make one segment per bus
// WLEDMM protect against parallel access while drawing // 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 segStarts[MAX_NUM_SEGMENTS] = {0};
uint16_t segStops [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++) { for (size_t i = 1; i < s; i++) {
_segments.push_back(Segment(segStarts[i], segStops[i])); _segments.push_back(Segment(segStarts[i], segStops[i]));
} }
esp32SemGive(busDrawMux); esp32SemGive(segmentMux);
} else { } else {
if (forceReset || getSegmentsNum() == 0) resetSegments(); if (forceReset || getSegmentsNum() == 0) resetSegments();
@@ -2499,7 +2507,7 @@ void WS2812FX::makeAutoSegments(bool forceReset) {
void WS2812FX::fixInvalidSegments() { void WS2812FX::fixInvalidSegments() {
// WLEDMM protect against parallel access while drawing // 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) //make sure no segment is longer than total (sanity check)
for (size_t i = getSegmentsNum()-1; i > 0; i--) { 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; 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 // if any segments were deleted free memory
purgeSegments(); purgeSegments();

View File

@@ -94,7 +94,11 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
// if using vectors use this code to append segment // if using vectors use this code to append segment
if (id >= strip.getSegmentsNum()) { if (id >= strip.getSegmentsNum()) {
if (stop <= 0) return false; // ignore empty/inactive segments 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 id = strip.getSegmentsNum()-1; // segments are added at the end of list
newSeg = true; newSeg = true;
} }

View File

@@ -184,7 +184,10 @@ void realtimeLock(uint32_t timeoutMs, byte md)
stop = strip.getLengthTotal(); stop = strip.getLengthTotal();
} }
// clear strip/segment // 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 WLED was off and using main segment only, freeze non-main segments so they stay off
if (useMainSegmentOnly && bri == 0) { if (useMainSegmentOnly && bri == 0) {
for (size_t s=0; s < strip.getSegmentsNum(); s++) { for (size_t s=0; s < strip.getSegmentsNum(); s++) {
@@ -326,10 +329,13 @@ void handleNotifications()
#endif #endif
uint16_t id = 0; uint16_t id = 0;
uint16_t totalLen = strip.getLengthTotal(); uint16_t totalLen = strip.getLengthTotal();
for (int i = 0; i < packetSize -2; i += 3) 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; setRealtimePixel(id, lbuf[i], lbuf[i+1], lbuf[i+2], 0);
id++; if (id >= totalLen) break;
}
esp32SemGive(busDrawMux);
} }
if (!(realtimeMode && useMainSegmentOnly)) strip.show(); if (!(realtimeMode && useMainSegmentOnly)) strip.show();
return; return;
@@ -573,14 +579,16 @@ void handleNotifications()
uint16_t id = (tpmPayloadFrameSize/3)*(packetNum-1); //start LED uint16_t id = (tpmPayloadFrameSize/3)*(packetNum-1); //start LED
uint16_t totalLen = strip.getLengthTotal(); uint16_t totalLen = strip.getLengthTotal();
for (size_t i = 6; i < tpmPayloadFrameSize + 4U; i += 3) 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)
if (id < totalLen)
{ {
setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); if (id < totalLen) {
id++; 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 if (tpmPacketCount == numPackets) //reset packet count and show if all packets were received
{ {
@@ -606,49 +614,40 @@ void handleNotifications()
} }
if (realtimeOverride && !(realtimeMode && useMainSegmentOnly)) return; if (realtimeOverride && !(realtimeMode && useMainSegmentOnly)) return;
uint16_t totalLen = strip.getLengthTotal(); if (esp32SemTake(busDrawMux, 250) == pdTRUE) { // WLEDMM acquire drawing permission (wait max 200ms) before setting pixels
if (udpIn[0] == 1 && packetSize > 5) //warls uint16_t totalLen = strip.getLengthTotal();
{ if (udpIn[0] == 1 && packetSize > 5) { //warls
for (int i = 2; i < packetSize -3; i += 4) for (int i = 2; i < packetSize -3; i += 4) {
{ setRealtimePixel(udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3], 0);
setRealtimePixel(udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3], 0); }
} } else if (udpIn[0] == 2 && packetSize > 4) { //drgb
} else if (udpIn[0] == 2 && packetSize > 4) //drgb uint16_t id = 0;
{ for (int i = 2; i < packetSize -2; i += 3) {
uint16_t id = 0; setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0);
for (int i = 2; i < packetSize -2; i += 3) id++; if (id >= totalLen) break;
{ }
setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0); } else if (udpIn[0] == 3 && packetSize > 6) { //drgbw
uint16_t id = 0;
id++; if (id >= totalLen) break; for (int i = 2; i < packetSize -3; i += 4) {
} setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]);
} else if (udpIn[0] == 3 && packetSize > 6) //drgbw id++; if (id >= totalLen) break;
{ }
uint16_t id = 0; } else if (udpIn[0] == 4 && packetSize > 7) { //dnrgb
for (int i = 2; i < packetSize -3; i += 4) uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00);
{ for (int i = 4; i < packetSize -2; i += 3) {
setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]); if (id >= totalLen) break;
setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], 0);
id++; if (id >= totalLen) break; id++;
} }
} else if (udpIn[0] == 4 && packetSize > 7) //dnrgb } else if (udpIn[0] == 5 && packetSize > 8) { //dnrgbw
{ uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00);
uint16_t id = ((udpIn[3] << 0) & 0xFF) + ((udpIn[2] << 8) & 0xFF00); for (int i = 4; i < packetSize -2; i += 4) {
for (int i = 4; i < packetSize -2; i += 3) if (id >= totalLen) break;
{ setRealtimePixel(id, udpIn[i], udpIn[i+1], udpIn[i+2], udpIn[i+3]);
if (id >= totalLen) break; id++;
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(); strip.show();
return; return;

View File

@@ -478,7 +478,9 @@ void WLED::setup()
#ifdef ARDUINO_ARCH_ESP32 #ifdef ARDUINO_ARCH_ESP32
busDrawMux = xSemaphoreCreateRecursiveMutex(); // WLEDMM prevent concurrent running of strip.show and strip.service 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(busDrawMux); // init semaphores to initially allow drawing
xSemaphoreGiveRecursive(segmentMux);
#endif #endif
#ifdef ARDUINO_ARCH_ESP32 #ifdef ARDUINO_ARCH_ESP32

View File

@@ -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 // WLEDMM prevent concurrent strip.show() and strip.service() -> for DDP over ws, and other background tasks
#ifdef ARDUINO_ARCH_ESP32 #ifdef ARDUINO_ARCH_ESP32
WLED_GLOBAL SemaphoreHandle_t busDrawMux _INIT(nullptr); 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 esp32SemTake(mux,timeout) xSemaphoreTakeRecursive(mux, timeout) // convenience macro that expands to xSemaphoreTakeRecursive
#define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive #define esp32SemGive(mux) xSemaphoreGiveRecursive(mux) // convenience macro that expands to xSemaphoreGiveRecursive
#else #else