From 5e20abd7f18f745f6f4ef1369513f9ace411b4f8 Mon Sep 17 00:00:00 2001 From: cschwinne Date: Thu, 13 Jul 2023 13:08:36 +0200 Subject: [PATCH] Move segment bounds queuing to WS2812FX --- wled00/FX.h | 22 +++++++++++++----- wled00/FX_fcn.cpp | 57 ++++++++++++++++++++++++----------------------- wled00/json.cpp | 5 +++-- wled00/set.cpp | 2 +- 4 files changed, 50 insertions(+), 36 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index 75a5a4c9..d9426624 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -395,9 +395,6 @@ typedef struct Segment { uint16_t _dataLen; static uint16_t _usedSegmentData; - static uint16_t _qStart, _qStop, _qStartY, _qStopY; - static uint8_t _queuedChangesSegId; - // transition data, valid only if transitional==true, holds values during transition (72 bytes) struct Transition { uint32_t _colorT[NUM_COLORS]; @@ -689,7 +686,15 @@ class WS2812FX { // 96 bytes customMappingSize(0), _lastShow(0), _segment_index(0), - _mainSegment(0) + _mainSegment(0), + _queuedChangesSegId(255), + _qStart(0), + _qStop(0), + _qStartY(0), + _qStopY(0), + _qGrouping(0), + _qSpacing(0), + _qOffset(0) { WS2812FX::instance = this; _mode.reserve(_modeCount); // allocate memory to prevent initial fragmentation (does not increase size()) @@ -745,7 +750,7 @@ class WS2812FX { // 96 bytes inline void trigger(void) { _triggered = true; } // Forces the next frame to be computed on all active segments. inline void setShowCallback(show_callback cb) { _callback = cb; } inline void setTransition(uint16_t t) { _transitionDur = t; } - inline void appendSegment(const Segment &seg = Segment()) { _segments.push_back(seg); } + inline void appendSegment(const Segment &seg = Segment()) { if (_segments.size() < getMaxSegments()) _segments.push_back(seg); } bool checkSegmentAlignment(void), @@ -899,9 +904,16 @@ class WS2812FX { // 96 bytes uint8_t _segment_index; uint8_t _mainSegment; + uint8_t _queuedChangesSegId; + uint16_t _qStart, _qStop, _qStartY, _qStopY; + uint8_t _qGrouping, _qSpacing; + uint16_t _qOffset; uint8_t estimateCurrentAndLimitBri(void); + + void + setUpSegmentFromQueuedChanges(void); }; extern const char JSON_mode_names[]; diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index c0949ae3..3518e223 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -76,9 +76,6 @@ uint16_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[] uint16_t Segment::maxWidth = DEFAULT_LED_COUNT; uint16_t Segment::maxHeight = 1; -uint8_t Segment::_queuedChangesSegId = 255U; -uint16_t Segment::_qStart = 0, Segment::_qStop = 0; -uint16_t Segment::_qStartY = 0, Segment::_qStopY = 0; // copy constructor Segment::Segment(const Segment &orig) { @@ -181,10 +178,6 @@ void Segment::resetIfRequired() { deallocateData(); next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0; - if (_queuedChangesSegId == strip.getCurrSegmentId()) { // apply queued changes - setUp(_qStart, _qStop, grouping, spacing, offset, _qStartY, _qStopY); - _queuedChangesSegId = 255; - } reset = false; } @@ -362,8 +355,6 @@ void Segment::handleTransition() { // 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) { - if (_queuedChangesSegId == segId) _queuedChangesSegId = 255; // cancel queued change if already queued for this segment - // return if neither bounds nor grouping have changed bool boundsUnchanged = (start == i1 && stop == i2); #ifndef WLED_DISABLE_2D @@ -384,17 +375,7 @@ void Segment::setUp(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, uint16_t if (ofs < UINT16_MAX) offset = ofs; markForReset(); - if (boundsUnchanged) return; // TODO test if it is save to change grp/spc/ofs without queueing - // queuing a change for a second segment will lead to the loss of the first change if not yet applied - // however this is not a problem as the queued change is applied immediately after the effect function in that segment returns - if (segId < MAX_NUM_SEGMENTS && segId == strip.getCurrSegmentId() && strip.isServicing()) { // queue change to prevent concurrent access - _qStart = i1; - _qStop = i2; - _qStartY = i1Y; - _qStopY = i2Y; - _queuedChangesSegId = segId; - return; // queued changes are applied immediately after effect function returns - } + if (boundsUnchanged) return; // apply change immediately if (i2 <= i1) { //disable segment @@ -1098,10 +1079,8 @@ void WS2812FX::service() { // reset the segment runtime data if needed seg.resetIfRequired(); - if (!seg.isActive()) { _segment_index++; continue; } - // last condition ensures all solid segments are updated at the same time - if(nowUp > seg.next_time || _triggered || (doShow && seg.mode == FX_MODE_STATIC)) + if (seg.isActive() && (nowUp > seg.next_time || _triggered || (doShow && seg.mode == FX_MODE_STATIC))) { doShow = true; uint16_t delay = FRAMETIME; @@ -1126,7 +1105,7 @@ void WS2812FX::service() { seg.next_time = nowUp + delay; } - seg.resetIfRequired(); // another reset chance, mainly to apply new segment bounds if queued + if (_segment_index == _queuedChangesSegId) setUpSegmentFromQueuedChanges(); _segment_index++; } _virtualSegmentLength = 0; @@ -1453,10 +1432,32 @@ Segment& WS2812FX::getSegment(uint8_t id) { return _segments[id >= _segments.size() ? getMainSegmentId() : id]; // vectors } -// compatibility method (deprecated) -void WS2812FX::setSegment(uint8_t n, uint16_t i1, uint16_t i2, uint8_t grouping, uint8_t spacing, uint16_t offset, uint16_t startY, uint16_t stopY) { - if (n >= _segments.size()) return; - _segments[n].setUp(i1, i2, grouping, spacing, offset, startY, stopY); +// sets new segment bounds, queues if that segment is currently running +void WS2812FX::setSegment(uint8_t segId, uint16_t i1, uint16_t i2, uint8_t grouping, uint8_t spacing, uint16_t offset, uint16_t startY, uint16_t stopY) { + if (segId >= getSegmentsNum()) { + if (i2 <= i1) return; // do not append empty/inactive segments + 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 + // queuing a change for a second segment will lead to the loss of the first change if not yet applied + // however this is not a problem as the queued change is applied immediately after the effect function in that segment returns + _qStart = i1; _qStop = i2; _qStartY = startY; _qStopY = stopY; + _qGrouping = grouping; _qSpacing = spacing; _qOffset = offset; + _queuedChangesSegId = segId; + return; // queued changes are applied immediately after effect function returns + } + + _segments[segId].setUp(i1, i2, grouping, spacing, offset, startY, stopY); +} + +void WS2812FX::setUpSegmentFromQueuedChanges() { + if (_queuedChangesSegId >= getSegmentsNum()) return; + getSegment(_queuedChangesSegId).setUp(_qStart, _qStop, _qGrouping, _qSpacing, _qOffset, _qStartY, _qStopY); + _queuedChangesSegId = 255; } void WS2812FX::restartRuntime() { diff --git a/wled00/json.cpp b/wled00/json.cpp index cbf1407c..c0c72989 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -112,8 +112,9 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId) if (stop > start && of > len -1) of = len -1; // update segment (delete if necessary) - // we must not change segment dimensions during drawing of effects in that segment as concurrent access may cause a crash - seg.setUp(start, stop, grp, spc, of, startY, stopY, id); + // 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); if (seg.reset && seg.stop == 0) return true; // segment was deleted & is marked for reset, no need to change anything else diff --git a/wled00/set.cpp b/wled00/set.cpp index fb3e40c2..71db6c06 100644 --- a/wled00/set.cpp +++ b/wled00/set.cpp @@ -797,7 +797,7 @@ bool handleSet(AsyncWebServerRequest *request, const String& req, bool apply) if (pos > 0) { spcI = getNumVal(&req, pos); } - selseg.setUp(startI, stopI, grpI, spcI, UINT16_MAX, startY, stopY); + strip.setSegment(selectedSeg, startI, stopI, grpI, spcI, UINT16_MAX, startY, stopY); pos = req.indexOf(F("RV=")); //Segment reverse if (pos > 0) selseg.reverse = req.charAt(pos+3) != '0';