diff --git a/wled00/FX_2Dfcn.cpp b/wled00/FX_2Dfcn.cpp index 751bc5ac14..063d3a6bb3 100644 --- a/wled00/FX_2Dfcn.cpp +++ b/wled00/FX_2Dfcn.cpp @@ -17,6 +17,7 @@ // note: matrix may be comprised of multiple panels each with different orientation // but ledmap takes care of that. ledmap is constructed upon initialization // so matrix should disable regular ledmap processing +// WARNING: effect drawing has to be suspended (strip.suspend()) or must be called from loop() context void WS2812FX::setUpMatrix() { #ifndef WLED_DISABLE_2D // isMatrix is set in cfg.cpp or set.cpp @@ -45,12 +46,12 @@ void WS2812FX::setUpMatrix() { return; } - suspend(); - waitForIt(); - customMappingSize = 0; // prevent use of mapping if anything goes wrong d_free(customMappingTable); + // Segment::maxWidth and Segment::maxHeight are set according to panel layout + // and the product will include at least all leds in matrix + // if actual LEDs are more, getLengthTotal() will return correct number of LEDs customMappingTable = static_cast(d_malloc(sizeof(uint16_t)*getLengthTotal())); // prefer to not use SPI RAM if (customMappingTable) { @@ -113,7 +114,6 @@ void WS2812FX::setUpMatrix() { // delete gap array as we no longer need it p_free(gapTable); - resume(); #ifdef WLED_DEBUG DEBUG_PRINT(F("Matrix ledmap:")); diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 2d98dc0447..39f87434c1 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -1681,9 +1681,13 @@ void WS2812FX::setTransitionMode(bool t) { resume(); } -// wait until frame is over (service() has finished or time for 1 frame has passed; yield() crashes on 8266) +// wait until frame is over (service() has finished or time for 2 frames have passed; yield() crashes on 8266) +// the latter may, in rare circumstances, lead to incorrectly assuming strip is done servicing but will not block +// other processing "indefinitely" +// rare circumstances are: setting FPS to high number (i.e. 120) and have very slow effect that will need more +// time than 2 * _frametime (1000/FPS) to draw content void WS2812FX::waitForIt() { - unsigned long maxWait = millis() + getFrameTime() + 100; // TODO: this needs a proper fix for timeout! + unsigned long maxWait = millis() + 2*getFrameTime() + 100; // TODO: this needs a proper fix for timeout! see #4779 while (isServicing() && maxWait > millis()) delay(1); #ifdef WLED_DEBUG if (millis() >= maxWait) DEBUG_PRINTLN(F("Waited for strip to finish servicing.")); @@ -1810,7 +1814,11 @@ Segment& WS2812FX::getSegment(unsigned id) { return _segments[id >= _segments.size() ? getMainSegmentId() : id]; // vectors } +// WARNING: resetSegments(), makeAutoSegments() and fixInvalidSegments() must not be called while +// strip is being serviced (strip.service()), you must call suspend prior if changing segments outside +// loop() context void WS2812FX::resetSegments() { + if (isServicing()) return; _segments.clear(); // destructs all Segment as part of clearing _segments.emplace_back(0, isMatrix ? Segment::maxWidth : _length, 0, isMatrix ? Segment::maxHeight : 1); _segments.shrink_to_fit(); // just in case ... @@ -1818,6 +1826,7 @@ void WS2812FX::resetSegments() { } void WS2812FX::makeAutoSegments(bool forceReset) { + if (isServicing()) return; if (autoSegments) { //make one segment per bus unsigned segStarts[MAX_NUM_SEGMENTS] = {0}; unsigned segStops [MAX_NUM_SEGMENTS] = {0}; @@ -1889,6 +1898,7 @@ void WS2812FX::makeAutoSegments(bool forceReset) { } void WS2812FX::fixInvalidSegments() { + if (isServicing()) return; //make sure no segment is longer than total (sanity check) for (size_t i = getSegmentsNum()-1; i > 0; i--) { if (isMatrix) { @@ -1951,6 +1961,7 @@ void WS2812FX::printSize() { // load custom mapping table from JSON file (called from finalizeInit() or deserializeState()) // if this is a matrix set-up and default ledmap.json file does not exist, create mapping table using setUpMatrix() from panel information +// WARNING: effect drawing has to be suspended (strip.suspend()) or must be called from loop() context bool WS2812FX::deserializeMap(unsigned n) { char fileName[32]; strcpy_P(fileName, PSTR("/ledmap")); @@ -1980,9 +1991,6 @@ bool WS2812FX::deserializeMap(unsigned n) { } else DEBUG_PRINTF_P(PSTR("Reading LED map from %s\n"), fileName); - suspend(); - waitForIt(); - JsonObject root = pDoc->as(); // if we are loading default ledmap (at boot) set matrix width and height from the ledmap (compatible with WLED MM ledmaps) if (n == 0 && (!root[F("width")].isNull() || !root[F("height")].isNull())) { @@ -2040,8 +2048,6 @@ bool WS2812FX::deserializeMap(unsigned n) { DEBUG_PRINTLN(F("ERROR LED map allocation error.")); } - resume(); - releaseJSONBufferLock(); return (customMappingSize > 0); } diff --git a/wled00/set.cpp b/wled00/set.cpp index 893081b986..baa2f305a7 100644 --- a/wled00/set.cpp +++ b/wled00/set.cpp @@ -815,8 +815,13 @@ void handleSettingsSet(AsyncWebServerRequest *request, byte subPage) } } strip.panel.shrink_to_fit(); // release unused memory + // we are changing matrix/ledmap geometry which *will* affect existing segments + // since we are not in loop() context we must make sure that effects are not running. credit @blazonchek for properly fixing #4911 + strip.suspend(); + strip.waitForIt(); strip.deserializeMap(); // (re)load default ledmap (will also setUpMatrix() if ledmap does not exist) strip.makeAutoSegments(true); // force re-creation of segments + strip.resume(); } #endif