diff --git a/wled00/FX.h b/wled00/FX.h index 15dfca2303..707a8befa7 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -810,6 +810,11 @@ class Segment { friend class ParticleSystem1D; }; +// max wait times when waiting until !strip.isUpdating() - use with waitForLEDs(...) +constexpr unsigned STRIP_WAIT_VERYSHORT = 25; // 25 ms - when risk of flickering is low but delays should be avoided +constexpr unsigned STRIP_WAIT_SHORT = 50; // 50 ms - for cases where fluent animations are most important, and risk of flickering is low +constexpr unsigned STRIP_WAIT_MEDIUM = 150; // 150 ms - good balance to avoid flickering on -C3 (good up to 4000 ws2812b LEDs per pin) + // main "strip" class (108 bytes) class WS2812FX { typedef void (*mode_ptr)(); // pointer to mode function @@ -910,6 +915,11 @@ class WS2812FX { { if (_segments.size() < getMaxSegments()) _segments.emplace_back(sStart,sStop,sStartY,sStopY); } inline void suspend() { _suspend = true; } // will suspend (and canacel) strip.service() execution inline void resume() { _suspend = false; } // will resume strip.service() execution + inline bool isSuspended() const { return _suspend; } // true if strip.service() execution is suspended + // be nice, but not too nice - wait until LEDs are idle, or maxWaitMS have passed + // on 8266 this call will _not_ wait outside of the main loop context + // returns isUpdating() status after waiting + bool waitForLEDs(unsigned maxWaitMS, bool always = false) const; void restartRuntime(); void setTransitionMode(bool t); @@ -923,7 +933,6 @@ class WS2812FX { inline bool isServicing() const { return _isServicing; } // returns true if strip.service() is executing inline bool hasWhiteChannel() const { return _hasWhiteChannel; } // returns true if strip contains separate white chanel inline bool isOffRefreshRequired() const { return _isOffRefreshRequired; } // returns true if strip requires regular updates (i.e. TM1814 chipset) - inline bool isSuspended() const { return _suspend; } // returns true if strip.service() execution is suspended inline bool needsUpdate() const { return _triggered; } // returns true if strip received a trigger() request // uint8_t paletteBlend; // obsolete - use global paletteBlend instead of strip.paletteBlend diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index a30410daec..50d269b108 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -1837,6 +1837,31 @@ void WS2812FX::waitForIt() { #endif }; +/** + * Be nice, but not too nice - wait until LEDs are idle, or maxWaitMS milliseconds have passed. + * always=true enforces waiting even when using the RMTHI driver. + * On 8266 this call will _not_ wait outside the main loop context. + * Function returns isUpdating() status after waiting. + **/ +bool WS2812FX::waitForLEDs(unsigned maxWaitMS, bool always) const { + #if !defined(WLED_USE_SHARED_RMT) && !defined(__riscv) && !defined(ESP8266) + if (!always) return isUpdating(); // no waiting needed if we have the flicker-free RMTHI driver + #endif + #ifdef ARDUINO_ARCH_ESP32 + unsigned long waitStart = millis(); + while (isUpdating() && (millis() - waitStart < maxWaitMS)) delay(1); + #else + if (can_yield()) { + // If we are in a yieldable context (main loop), wait until the LEDs output finishes + yield(); + unsigned long waitStart = millis(); + while (isUpdating() && (millis() - waitStart < maxWaitMS)) delay(1); + yield(); + } + #endif + return isUpdating(); +} + void WS2812FX::setTargetFps(unsigned fps) { if (fps <= 250) _targetFps = fps; if (_targetFps > 0) _frametime = 1000 / _targetFps; diff --git a/wled00/file.cpp b/wled00/file.cpp index 5a169d6450..431ba24a72 100644 --- a/wled00/file.cpp +++ b/wled00/file.cpp @@ -34,13 +34,24 @@ static File f; // don't export to other cpp files //wrapper to find out how long closing takes void closeFile() { + if (!doCloseFile || !f) { doCloseFile = false; return; } // file not open, or no request to close -> nothing to do, nothing to wait #ifdef WLED_DEBUG_FS DEBUGFS_PRINT(F("Close -> ")); uint32_t s = millis(); #endif + doCloseFile = false; // consume flag early, to reduce the time window for concurrent closing attempts from several tasks. + + // f.close() may enter flash critical sections (interrupts/cache paused), so we wait for LED transmission to finish first to avoid WS281x glitches + // This is most relevant on ESP32-C3/C5/C6, where the RMT driver is very sensitive to interrupt timing. + bool haveSuspended = false; + #if defined(WLED_USE_SHARED_RMT) || defined(__riscv) || !defined(ARDUINO_ARCH_ESP32) + if (!strip.isSuspended()) { strip.suspend(); haveSuspended = true; } // prevent that a new strip.show() starts after waiting + strip.waitForLEDs(STRIP_WAIT_MEDIUM); // be nice, but not too nice. Waits up to 150ms + #endif + f.close(); // "if (f)" check is aleady done inside f.close(), and f cannot be nullptr -> no need for double checking before closing the file handle. + if (haveSuspended) strip.resume(); // end of critical section - new LEDs updates are allowed again DEBUGFS_PRINTF("took %lu ms\n", millis() - s); - doCloseFile = false; } //find() that reads and buffers data from file stream in 256-byte blocks. @@ -438,6 +449,7 @@ bool handleFileRead(AsyncWebServerRequest* request, String path){ } #endif if(WLED_FS.exists(path) || WLED_FS.exists(path + ".gz")) { + strip.waitForLEDs(STRIP_WAIT_SHORT); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering) request->send(request->beginResponse(WLED_FS, path, {}, request->hasArg(F("download")), {})); return true; } @@ -452,6 +464,7 @@ bool copyFile(const char* src_path, const char* dst_path) { return false; } + strip.waitForLEDs(STRIP_WAIT_MEDIUM, true); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering) bool success = true; // is set to false on error File src = WLED_FS.open(src_path, "r"); File dst = WLED_FS.open(dst_path, "w"); @@ -490,6 +503,7 @@ bool compareFiles(const char* path1, const char* path2) { return false; } + strip.waitForLEDs(STRIP_WAIT_SHORT); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering) bool identical = true; // set to false on mismatch File f1 = WLED_FS.open(path1, "r"); File f2 = WLED_FS.open(path2, "r"); diff --git a/wled00/image_loader.cpp b/wled00/image_loader.cpp index d74afc68d1..1b464d6a6d 100644 --- a/wled00/image_loader.cpp +++ b/wled00/image_loader.cpp @@ -28,10 +28,7 @@ int fileReadCallback(void) { } int fileReadBlockCallback(void * buffer, int numberOfBytes) { - #ifdef CONFIG_IDF_TARGET_ESP32C3 - unsigned t0 = millis(); - while (strip.isUpdating() && (millis() - t0 < 150)) yield(); // be nice, but not too nice. Waits up to 150ms to avoid glitches - #endif + strip.waitForLEDs(STRIP_WAIT_SHORT); return file.read((uint8_t*)buffer, numberOfBytes); } @@ -137,6 +134,7 @@ byte renderImageToSegment(Segment &seg) { DEBUG_PRINTF_P(PSTR("GIF decoder unsupported file: %s\n"), lastFilename); return IMAGE_ERROR_UNSUPPORTED_FORMAT; } + strip.waitForLEDs(STRIP_WAIT_MEDIUM); if (file) file.close(); if (!openGif(lastFilename)) { gifDecodeFailed = true; diff --git a/wled00/json.cpp b/wled00/json.cpp index b4388d27ef..e434669a44 100644 --- a/wled00/json.cpp +++ b/wled00/json.cpp @@ -849,6 +849,8 @@ void serializeInfo(JsonObject root) root[F("lwip")] = LWIP_VERSION_MAJOR; #endif + // calling ESP.getFreeHeap() during led update causes glitches on C3 and possibly on 8266, too + strip.waitForLEDs(STRIP_WAIT_SHORT); // be nice, but not too nice. Waits up to 50ms. No need to suspend effects - ESP.getFreeHeap() will not need much time root[F("freeheap")] = getFreeHeapSize(); #if defined(ARDUINO_ARCH_ESP32) && defined(BOARD_HAS_PSRAM) // Report PSRAM information diff --git a/wled00/ota_update.cpp b/wled00/ota_update.cpp index b078dd89d9..74c8863ed5 100644 --- a/wled00/ota_update.cpp +++ b/wled00/ota_update.cpp @@ -239,6 +239,7 @@ static bool beginOTA(AsyncWebServerRequest *request, UpdateContext* context) UsermodManager::onUpdateBegin(true); // notify usermods that update is about to begin (some may require task de-init) strip.suspend(); + strip.waitForLEDs(STRIP_WAIT_MEDIUM, true); // always wait for LED transmissions to finish backupConfig(); // backup current config in case the update ends badly strip.resetSegments(); // free as much memory as you can context->needsRestart = true; @@ -759,6 +760,7 @@ bool initBootloaderOTA(AsyncWebServerRequest *request) { #endif lastEditTime = millis(); // make sure PIN does not lock during update strip.suspend(); + strip.waitForLEDs(STRIP_WAIT_SHORT, true); // be sure that LED transmissions have finished strip.resetSegments(); // Check available heap before attempting allocation diff --git a/wled00/wled.cpp b/wled00/wled.cpp index 24b385cb33..3913ef9119 100644 --- a/wled00/wled.cpp +++ b/wled00/wled.cpp @@ -174,12 +174,9 @@ void WLED::loop() #ifdef ESP8266 uint32_t heap = getFreeHeapSize(); // ESP8266 needs ~8k of free heap for UI to work properly #else - #ifdef CONFIG_IDF_TARGET_ESP32C3 // calling getContiguousFreeHeap() during led update causes glitches on C3 // this can (probably) be removed once RMT driver for C3 is fixed - unsigned t0 = millis(); - while (strip.isUpdating() && (millis() - t0 < 150)) delay(1); // be nice, but not too nice. Waits up to 150ms - #endif + strip.waitForLEDs(STRIP_WAIT_MEDIUM); // be nice, but not too nice. Waits up to 150ms - we are in the main loop, so a new strip.show() cannot start while waiting uint32_t heap = getContiguousFreeHeap(); // ESP32 family needs ~10k of contiguous free heap for UI to work properly #endif if (heap < MIN_HEAP_SIZE - 1024) heapDanger+=5; // allow 1k of "wiggle room" for things that do not respect min heap limits @@ -494,6 +491,7 @@ void WLED::setup() DEBUG_PRINTLN(F("Initializing strip")); beginStrip(); + strip.waitForLEDs(STRIP_WAIT_MEDIUM); // prevent flickering - beginStrip() calls strip.show() DEBUG_PRINTF_P(PSTR("heap %u\n"), getFreeHeapSize()); DEBUG_PRINTLN(F("Usermods setup")); @@ -598,7 +596,7 @@ void WLED::setup() #endif #if defined(ARDUINO_ARCH_ESP32) && defined(WLED_DISABLE_BROWNOUT_DET) - WRITE_PERI_REG(RTC_CNTL_BROWN_OUT_REG, 1); //enable brownout detector + WRITE_PERI_REG(RTC_CNTL_BROWN_OUT_REG, 1); //disable brownout detector #endif markOTAvalid(); } diff --git a/wled00/wled_server.cpp b/wled00/wled_server.cpp index 0b4d0fb546..305ca7a5ef 100644 --- a/wled00/wled_server.cpp +++ b/wled00/wled_server.cpp @@ -212,7 +212,14 @@ static void handleUpload(AsyncWebServerRequest *request, const String& filename, request->_tempFile.write(data,len); } if (isFinal) { + bool haveSuspended = false; + #if defined(WLED_USE_SHARED_RMT) || defined(__riscv) || !defined(ARDUINO_ARCH_ESP32) + if (!strip.isSuspended()) { strip.suspend(); haveSuspended = true; } // prevent that a new strip.show() starts after waiting + strip.waitForLEDs(STRIP_WAIT_SHORT, true); // calling file.close() during LEDs sendout can cause glitches on C3 and on 8266 + #endif request->_tempFile.close(); + if (haveSuspended) strip.resume(); + if (filename.indexOf(F("cfg.json")) >= 0) { // check for filename with or without slash doReboot = true; request->send(200, FPSTR(CONTENT_TYPE_PLAIN), F("Config restore ok.\nRebooting...")); diff --git a/wled00/ws.cpp b/wled00/ws.cpp index 6e9038c101..736199231e 100644 --- a/wled00/ws.cpp +++ b/wled00/ws.cpp @@ -153,6 +153,7 @@ void sendDataWs(AsyncWebSocketClient * client) DEBUG_PRINTF_P(PSTR("JSON buffer size: %u for WS request (%u).\n"), pDoc->memoryUsage(), len); // the following may no longer be necessary as heap management has been fixed by @willmmiles in AWS + strip.waitForLEDs(STRIP_WAIT_VERYSHORT); // wait for LEDs ouptut to finish - prevents glitches on -C3 size_t heap1 = getFreeHeapSize(); DEBUG_PRINTF_P(PSTR("heap %u\n"), getFreeHeapSize()); AsyncWebSocketBuffer buffer(len);