From cf3f6ede7249e3bfe0e01066544e3202499cb417 Mon Sep 17 00:00:00 2001 From: Blaz Kristan Date: Fri, 22 Dec 2023 15:39:07 +0100 Subject: [PATCH] Suspend strip during operations --- wled00/FX.h | 13 ++++++++++--- wled00/FX_fcn.cpp | 26 +++++++++++++++----------- wled00/json.cpp | 7 ++++--- wled00/presets.cpp | 6 +++++- wled00/udp.cpp | 16 +++++++++------- wled00/wled.cpp | 2 +- wled00/wled_server.cpp | 4 +++- 7 files changed, 47 insertions(+), 27 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index 1b5b00dae..1df13292c 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -531,7 +531,7 @@ typedef struct Segment { #endif static void handleRandomPalette(); - void setUp(uint16_t i1, uint16_t i2, uint8_t grp=1, uint8_t spc=0, uint16_t ofs=UINT16_MAX, uint16_t i1Y=0, uint16_t i2Y=1, uint8_t segId = 255); + void setUp(uint16_t i1, uint16_t i2, uint8_t grp=1, uint8_t spc=0, uint16_t ofs=UINT16_MAX, uint16_t i1Y=0, uint16_t i2Y=1); bool setColor(uint8_t slot, uint32_t c); //returns true if changed void setCCT(uint16_t k); void setOpacity(uint8_t o); @@ -697,6 +697,7 @@ class WS2812FX { // 96 bytes _colors_t{0,0,0}, _virtualSegmentLength(0), // true private variables + _suspend(false), _length(DEFAULT_LED_COUNT), _brightness(DEFAULT_BRIGHTNESS), _transitionDur(750), @@ -754,7 +755,7 @@ class WS2812FX { // 96 bytes setCCT(uint16_t k), setBrightness(uint8_t b, bool direct = false), setRange(uint16_t i, uint16_t i2, uint32_t col), - purgeSegments(bool force = false), + purgeSegments(void), setSegment(uint8_t n, uint16_t start, uint16_t stop, uint8_t grouping = 1, uint8_t spacing = 0, uint16_t offset = UINT16_MAX, uint16_t startY=0, uint16_t stopY=1), setMainSegmentId(uint8_t n), resetSegments(), @@ -777,6 +778,8 @@ class WS2812FX { // 96 bytes inline void setShowCallback(show_callback cb) { _callback = cb; } inline void setTransition(uint16_t t) { _transitionDur = t; } inline void appendSegment(const Segment &seg = Segment()) { if (_segments.size() < getMaxSegments()) _segments.push_back(seg); } + inline void suspend(void) { _suspend = true; } + inline void resume(void) { _suspend = false; } bool paletteFade, @@ -790,6 +793,7 @@ class WS2812FX { // 96 bytes inline bool isServicing(void) { return _isServicing; } inline bool hasWhiteChannel(void) {return _hasWhiteChannel;} inline bool isOffRefreshRequired(void) {return _isOffRefreshRequired;} + inline bool isSuspended(void) { return _suspend; } uint8_t paletteBlend, @@ -899,6 +903,8 @@ class WS2812FX { // 96 bytes friend class Segment; private: + volatile bool _suspend; + uint16_t _length; uint8_t _brightness; uint16_t _transitionDur; @@ -932,9 +938,10 @@ class WS2812FX { // 96 bytes uint16_t _qStart, _qStop, _qStartY, _qStopY; uint8_t _qGrouping, _qSpacing; uint16_t _qOffset; - +/* void setUpSegmentFromQueuedChanges(void); +*/ }; extern const char JSON_mode_names[]; diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 42984a051..961f1ba73 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -468,7 +468,7 @@ void Segment::handleRandomPalette() { } // segId is given when called from network callback, changes are queued if that segment is currently in its effect function -void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t ofs, uint16_t i1Y, uint16_t i2Y, uint8_t segId) { +void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t ofs, uint16_t i1Y, uint16_t i2Y) { // return if neither bounds nor grouping have changed bool boundsUnchanged = (start == i1 && stop == i2); #ifndef WLED_DISABLE_2D @@ -1137,13 +1137,15 @@ void WS2812FX::finalizeInit(void) { void WS2812FX::service() { unsigned long nowUp = millis(); // Be aware, millis() rolls over every 49 days now = nowUp + timebase; - if (nowUp - _lastShow < MIN_SHOW_DELAY) return; + if (nowUp - _lastShow < MIN_SHOW_DELAY || _suspend) return; bool doShow = false; _isServicing = true; _segment_index = 0; Segment::handleRandomPalette(); // move it into for loop when each segment has individual random palette for (segment &seg : _segments) { + if (_suspend) break; // immediately stop processing segments if suspend requested during service() + // process transition (mode changes in the middle of transition) seg.handleTransition(); // reset the segment runtime data if needed @@ -1190,7 +1192,7 @@ void WS2812FX::service() { seg.next_time = nowUp + delay; } - if (_segment_index == _queuedChangesSegId) setUpSegmentFromQueuedChanges(); +// if (_segment_index == _queuedChangesSegId) setUpSegmentFromQueuedChanges(); _segment_index++; } _virtualSegmentLength = 0; @@ -1394,18 +1396,18 @@ bool WS2812FX::hasCCTBus(void) { return false; } -void WS2812FX::purgeSegments(bool force) { +void WS2812FX::purgeSegments() { // remove all inactive segments (from the back) int deleted = 0; if (_segments.size() <= 1) return; for (size_t i = _segments.size()-1; i > 0; i--) - if (_segments[i].stop == 0 || force) { + if (_segments[i].stop == 0) { deleted++; _segments.erase(_segments.begin() + i); } if (deleted) { _segments.shrink_to_fit(); - /*if (_mainSegment >= _segments.size())*/ setMainSegmentId(0); + setMainSegmentId(0); } } @@ -1420,7 +1422,7 @@ void WS2812FX::setSegment(uint8_t segId, uint16_t i1, uint16_t i2, uint8_t group appendSegment(Segment(0, strip.getLengthTotal())); segId = getSegmentsNum()-1; // segments are added at the end of list } - +/* if (_queuedChangesSegId == segId) _queuedChangesSegId = 255; // cancel queued change if already queued for this segment if (segId < getMaxSegments() && segId == getCurrSegmentId() && isServicing()) { // queue change to prevent concurrent access @@ -1432,17 +1434,19 @@ void WS2812FX::setSegment(uint8_t segId, uint16_t i1, uint16_t i2, uint8_t group DEBUG_PRINT(F("Segment queued: ")); DEBUG_PRINTLN(segId); return; // queued changes are applied immediately after effect function returns } - +*/ + suspend(); _segments[segId].setUp(i1, i2, grouping, spacing, offset, startY, stopY); + resume(); if (segId > 0 && segId == getSegmentsNum()-1 && i2 <= i1) _segments.pop_back(); // if last segment was deleted remove it from vector } - +/* void WS2812FX::setUpSegmentFromQueuedChanges() { if (_queuedChangesSegId >= getSegmentsNum()) return; - getSegment(_queuedChangesSegId).setUp(_qStart, _qStop, _qGrouping, _qSpacing, _qOffset, _qStartY, _qStopY); + _segments[_queuedChangesSegId].setUp(_qStart, _qStop, _qGrouping, _qSpacing, _qOffset, _qStartY, _qStopY); _queuedChangesSegId = 255; } - +*/ void WS2812FX::resetSegments() { _segments.clear(); // destructs all Segment as part of clearing #ifndef WLED_DISABLE_2D diff --git a/wled00/json.cpp b/wled00/json.cpp index 5b20baac1..6bbc5c04b 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -117,9 +117,8 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) if (stop > start && of > len -1) of = len -1; // update segment (delete if necessary) - // do not call seg.setUp() here, as it may cause a crash due to concurrent access if the segment is currently drawing effects - // WS2812FX handles queueing of the change - strip.setSegment(id, start, stop, grp, spc, of, startY, stopY); + seg.setUp(start, stop, grp, spc, of, startY, stopY); // strip needs to be suspended for this to work without issues + if (newSeg) seg.refreshLightCapabilities(); // fix for #3403 if (seg.reset && seg.stop == 0) { @@ -381,6 +380,7 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId) int it = 0; JsonVariant segVar = root["seg"]; + if (!segVar.isNull()) strip.suspend(); if (segVar.is()) { int id = segVar["id"] | -1; @@ -408,6 +408,7 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId) } if (strip.getSegmentsNum() > 3 && deleted >= strip.getSegmentsNum()/2U) strip.purgeSegments(); // batch deleting more than half segments } + strip.resume(); usermods.readFromJsonState(root); diff --git a/wled00/presets.cpp b/wled00/presets.cpp index 975b68974..60b3cf692 100644 --- a/wled00/presets.cpp +++ b/wled00/presets.cpp @@ -24,7 +24,9 @@ static void doSaveState() { bool persist = (presetToSave < 251); const char *filename = getFileName(persist); - if (!requestJSONBufferLock(10)) return; // will set fileDoc + unsigned long start = millis(); + while (strip.isUpdating() && millis()-start > (2*FRAMETIME_FIXED)+1) yield(); // wait 2 frames + if (strip.isUpdating() || !requestJSONBufferLock(10)) return; // will set fileDoc initPresetsFile(); // just in case if someone deleted presets.json using /edit JsonObject sObj = doc.to(); @@ -132,7 +134,9 @@ void applyPresetWithFallback(uint8_t index, uint8_t callMode, uint8_t effectID, void handlePresets() { if (presetToSave) { + strip.suspend(); doSaveState(); + strip.resume(); return; } diff --git a/wled00/udp.cpp b/wled00/udp.cpp index e00fe0766..b7a98c768 100644 --- a/wled00/udp.cpp +++ b/wled00/udp.cpp @@ -286,10 +286,10 @@ void parseNotifyPacket(uint8_t *udpIn) { uint16_t stopY = version > 11 ? (udpIn[34+ofs] << 8 | udpIn[35+ofs]) : 1; uint16_t offset = (udpIn[7+ofs] << 8 | udpIn[8+ofs]); if (!receiveSegmentOptions) { - //selseg.setUp(start, stop, selseg.grouping, selseg.spacing, offset, startY, stopY); - // we have to use strip.setSegment() instead of selseg.setUp() to prevent crash if segment would change during drawing DEBUG_PRINTF("Set segment w/o options: %d [%d,%d;%d,%d]\n", id, (int)start, (int)stop, (int)startY, (int)stopY); - strip.setSegment(id, start, stop, selseg.grouping, selseg.spacing, offset, startY, stopY); + strip.suspend(); //should not be needed as UDP handling is not done in ISR callbacks but still added "just in case" + selseg.setUp(start, stop, selseg.grouping, selseg.spacing, offset, startY, stopY); + strip.resume(); continue; // we do receive bounds, but not options } selseg.options = (selseg.options & 0x0071U) | (udpIn[9 +ofs] & 0x0E); // ignore selected, freeze, reset & transitional @@ -326,12 +326,14 @@ void parseNotifyPacket(uint8_t *udpIn) { } if (receiveSegmentBounds) { DEBUG_PRINTF("Set segment w/ options: %d [%d,%d;%d,%d]\n", id, (int)start, (int)stop, (int)startY, (int)stopY); - // we have to use strip.setSegment() instead of selseg.setUp() to prevent crash if segment would change during drawing - strip.setSegment(id, start, stop, udpIn[5+ofs], udpIn[6+ofs], offset, startY, stopY); + strip.suspend(); //should not be needed as UDP handling is not done in ISR callbacks but still added "just in case" + selseg.setUp(start, stop, udpIn[5+ofs], udpIn[6+ofs], offset, startY, stopY); + strip.resume(); } else { - // we have to use strip.setSegment() instead of selseg.setUp() to prevent crash if segment would change during drawing DEBUG_PRINTF("Set segment grouping: %d [%d,%d]\n", id, (int)udpIn[5+ofs], (int)udpIn[6+ofs]); - strip.setSegment(id, selseg.start, selseg.stop, udpIn[5+ofs], udpIn[6+ofs], selseg.offset, selseg.startY, selseg.stopY); + strip.suspend(); //should not be needed as UDP handling is not done in ISR callbacks but still added "just in case" + selseg.setUp(selseg.start, selseg.stop, udpIn[5+ofs], udpIn[6+ofs], selseg.offset, selseg.startY, selseg.stopY); + strip.resume(); } } stateChanged = true; diff --git a/wled00/wled.cpp b/wled00/wled.cpp index 57a79d4d4..e50bc8310 100644 --- a/wled00/wled.cpp +++ b/wled00/wled.cpp @@ -846,7 +846,7 @@ void WLED::handleConnection() DEBUG_PRINT(F("Heap too low! ")); DEBUG_PRINTLN(heap); forceReconnect = true; - strip.purgeSegments(true); // remove all but one segments from memory + strip.resetSegments(); } else if (heap < MIN_HEAP_SIZE) { strip.purgeSegments(); } diff --git a/wled00/wled_server.cpp b/wled00/wled_server.cpp index 7d9d1a1ec..11020abff 100644 --- a/wled00/wled_server.cpp +++ b/wled00/wled_server.cpp @@ -289,8 +289,9 @@ void initServer() #endif usermods.onUpdateBegin(true); // notify usermods that update is about to begin (some may require task de-init) lastEditTime = millis(); // make sure PIN does not lock during update + strip.suspend(); #ifdef ESP8266 - strip.purgeSegments(true); // free as much memory as you can + strip.resetSegments(); // free as much memory as you can Update.runAsync(true); #endif Update.begin((ESP.getFreeSketchSpace() - 0x1000) & 0xFFFFF000); @@ -301,6 +302,7 @@ void initServer() DEBUG_PRINTLN(F("Update Success")); } else { DEBUG_PRINTLN(F("Update Failed")); + strip.resume(); usermods.onUpdateBegin(false); // notify usermods that update has failed (some may require task init) #if WLED_WATCHDOG_TIMEOUT > 0 WLED::instance().enableWatchdog();