[16.0.x port] prevent glitches on -C3 by waiting for LEDs updates to finish

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

wled00/FX_fcn.cpp

@@ -1837,6 +1837,33 @@ 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 {
+  #ifdef ARDUINO_ARCH_ESP32 
+    #if !defined(WLED_USE_SHARED_RMT) && !defined(__riscv)
+    // shortcut: don't wait if we have the RMTHI driver, unless requested with "always = true"
+    if (!always) return isUpdating();
+    #endif
+
+    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;

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.
@@ -437,6 +448,7 @@ bool handleFileRead(AsyncWebServerRequest* request, String path){
     }
   }
   #endif
+  strip.waitForLEDs(STRIP_WAIT_SHORT); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering)
   if(WLED_FS.exists(path) || WLED_FS.exists(path + ".gz")) {
     request->send(request->beginResponse(WLED_FS, path, {}, request->hasArg(F("download")), {}));
     return true;
@@ -447,6 +459,7 @@ bool handleFileRead(AsyncWebServerRequest* request, String path){
 // copy a file, delete destination file if incomplete to prevent corrupted files
 bool copyFile(const char* src_path, const char* dst_path) {
   DEBUG_PRINTF("copyFile from %s to %s\n", src_path, dst_path);
+  strip.waitForLEDs(STRIP_WAIT_MEDIUM, true); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering)
   if(!WLED_FS.exists(src_path)) {
    DEBUG_PRINTLN(F("file not found"));
    return false;
@@ -485,6 +498,7 @@ bool copyFile(const char* src_path, const char* dst_path) {
 // compare two files, return true if identical
 bool compareFiles(const char* path1, const char* path2) {
   DEBUG_PRINTF("compareFile %s and %s\n", path1, path2);
+  strip.waitForLEDs(STRIP_WAIT_SHORT); // wait for LEDs before file access (not using strip.suspend(), to avoid effect stuttering)
   if (!WLED_FS.exists(path1) || !WLED_FS.exists(path2)) {
     DEBUG_PRINTLN(F("file not found"));
     return false;
@@ -543,6 +557,7 @@ bool restoreFile(const char* filename) {
   char backupname[32];
   snprintf_P(backupname, sizeof(backupname), s_backup_fmt, filename + 1); // skip leading '/' in filename
 
+  strip.waitForLEDs(STRIP_WAIT_MEDIUM); // wait for LEDs before file existence checking
   if (!WLED_FS.exists(backupname)) {
     DEBUG_PRINTLN(F("no backup found"));
     return false;

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

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

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

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); //re-enable brownout detector
   #endif
   markOTAvalid();
 }

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..."));

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