Skip to content

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

Open
softhack007 wants to merge 8 commits into
mainfrom
backport_5435
Open

[16.0.x port] prevent glitches on -C3 by waiting for LEDs updates to finish#5691
softhack007 wants to merge 8 commits into
mainfrom
backport_5435

Conversation

@softhack007

@softhack007 softhack007 commented Jun 19, 2026

Copy link
Copy Markdown
Member

16.0.0 port of #5435

temporary solution for LEDs glitches on RISC-V boards, as discussed in #5683

previous discussion see #5683 and #5688

Summary by CodeRabbit

  • Bug Fixes

    • Improved LED output synchronization during uploads, file operations, GIF image loading, WebSocket info sending, and OTA update flows to reduce timing conflicts, flicker, and related glitches.
    • Ensured LED activity finishes before sensitive actions like retrieving system memory info and closing/resuming files, improving stability during transfers and updates.
  • Improvements

    • Added a standardized “wait for LED idle/settle” mechanism with selectable short/medium/very-short timing, improving consistency across common device workflows.

softhack007 and others added 4 commits June 19, 2026 20:26
…lly suspend interrupts (#5435)

* adding strip.waitForLEDs(waitMS) function
* wait for LEDs output to complete before file writing
* wait before ESP.getFreeHeap() - main loop
* wait before file close (upload) and before  getFreeHeap() (json info)
* avoid losing "trigger" events due to strip.suspend
---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* use constants instead of raw timeouts
* simplify usage - in many cases we don't need #ifdef any more
* missed one "wait" on image_loader
comment should match real timeout
@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Introduces WS2812FX::waitForLEDs(unsigned maxWaitMS, bool always = false) const along with STRIP_WAIT_SHORT and STRIP_WAIT_MEDIUM constants in FX.h, implements the helper with driver/target-conditional logic in FX_fcn.cpp, and replaces scattered platform-specific isUpdating() polling and yield() patterns with calls to this new API across file I/O, image loading, JSON serialization, OTA update, the main loop, setup initialization, upload handling, and WebSocket transmission.

Changes

waitForLEDs API and call-site integration

Layer / File(s) Summary
waitForLEDs API contract and constants
wled00/FX.h
Adds STRIP_WAIT_SHORT and STRIP_WAIT_MEDIUM constexpr timeout constants, declares waitForLEDs(unsigned maxWaitMS, bool always = false) const on WS2812FX, relocates isSuspended() inline adjacent to the new declaration, and removes the stale duplicate isSuspended() definition.
waitForLEDs implementation
wled00/FX_fcn.cpp
Implements WS2812FX::waitForLEDs() with conditional paths: skips waiting on RMTHI builds when always is false, loops on strip.isUpdating() with millisecond timeout on ESP32, and uses yield()-bracketed polling on other targets when can_yield() is true; returns the post-wait isUpdating() state.
File I/O call-site integration
wled00/file.cpp
closeFile() adds early-return guards, eagerly clears doCloseFile, and optionally suspends/resumes the strip around f.close() after waiting for LED transmission. handleFileRead(), copyFile(), and compareFiles() each prepend waitForLEDs() with appropriate timeout and always parameters.
Image loader call-site integration
wled00/image_loader.cpp
fileReadBlockCallback replaces the ESP32C3-specific yield() loop with strip.waitForLEDs(STRIP_WAIT_SHORT). renderImageToSegment() inserts strip.waitForLEDs(STRIP_WAIT_MEDIUM) before closing any open file and starting GIF decode.
JSON, OTA, initialization, loop, upload, and WebSocket integration
wled00/json.cpp, wled00/ota_update.cpp, wled00/wled.cpp, wled00/wled_server.cpp, wled00/ws.cpp
serializeInfo inserts waitForLEDs(STRIP_WAIT_SHORT) before the heap query. beginOTA and initBootloaderOTA each add a waitForLEDs call. The wled.cpp setup initializes with waitForLEDs(STRIP_WAIT_MEDIUM) before beginStrip() to prevent flickering, and the brownout detector comment is corrected. The main loop replaces C3-specific isUpdating() polling with waitForLEDs(STRIP_WAIT_MEDIUM). handleUpload finalization conditionally suspends, waits, closes the temp file, and resumes the strip. sendDataWs adds a short wait before heap and buffer allocation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • wled/WLED#5435: Directly introduces and wires WS2812FX::waitForLEDs(...) into the same call sites targeted by this PR (closeFile(), serializeInfo heap query, beginOTA/initBootloaderOTA, handleUpload, and WLED::loop()).
  • wled/WLED#5688: Addresses the same C3 long-strip LED timing/glitch behavior by modifying the wait/yield logic in wled.cpp and image_loader.cpp, the exact files this PR also targets.
  • wled/WLED#4793: Modifies wled00/file.cpp file-operation utilities such as copyFile() and compareFiles(), which this PR synchronizes with the new strip.waitForLEDs(...) calls.

Suggested reviewers

  • willmmiles
  • DedeHai
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective of the PR: preventing LED glitches on ESP32-C3 boards by implementing a mechanism to wait for LED updates to finish before proceeding with certain operations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

prevents flickering at startup
* space before comment
* use isUpdating() - not strip.isUpdating() - in waitForLEDs()
@softhack007 softhack007 added enhancement hardware (Likely) Setup-specific issue, e.g. flickering LEDs optimization re-working an existing feature to be faster, or use less memory labels Jun 19, 2026
coderabbitai[bot]

This comment was marked as resolved.

getFreeHeapSize() can cause glitches.
@softhack007

Copy link
Copy Markdown
Member Author

@bigfella5000 this is the -C3 flicker fix, brought to main (mentioned during discussion of your PR #5683). Can you check if this fixes the flickering you observed on your -C3 setup?

@softhack007 softhack007 self-assigned this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awaiting testing enhancement hardware (Likely) Setup-specific issue, e.g. flickering LEDs optimization re-working an existing feature to be faster, or use less memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant