Suspend strip during operations

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[];

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

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<JsonObject>())
   {
     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);
 

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<JsonObject>();
@@ -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;
   }
 

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;

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();
     }

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();