Fix OTA update for C3 from 0.15 #5072
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…support OTA from 0.15
Contributor
WalkthroughAdds an ESP32-C3 QIO build environment and sets esp32c3dev flash mode to DIO; adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
platformio.ini (2)
341-341: Clarify why the [esp32c3] template defaults to QIO (line 341).The [esp32c3] base template sets
board_build.flash_mode = qio(line 341) without explanation, then [env:esp32c3dev] explicitly overrides it to DIO with a clear comment (line 547). Adding a brief comment at line 341 explaining the template default would help future maintainers understand the inheritance model. This is optional, but recommended for maintainability.Also applies to: 547-547, 564-564
541-541: Minor: LOLIN_WIFI_FIX comment differs between DIO and QIO environments.Line 541 (esp32c3dev):
; seems to work much better with this
Line 558 (esp32c3dev_qio):; lower TX power, seems to work much better with thisThe QIO variant adds "lower TX power" context. If this is intentional documentation of a difference between the two, it's fine; otherwise, consider aligning the comments for consistency.
Also applies to: 558-558
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platformio.ini(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
platformio.ini
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use platformio.ini as the single source of truth for hardware build targets and settings
Files:
platformio.ini
🧠 Learnings (5)
📓 Common learnings
Learnt from: willmmiles
Repo: wled/WLED PR: 4998
File: wled00/ota_update.cpp:120-124
Timestamp: 2025-10-11T17:56:24.923Z
Learning: In the WLED OTA update implementation (wled00/ota_update.cpp), the parameter name for bypassing OTA validation is "skipValidation", not "ignoreRelease". The intent evolved from the original spec in issue #4929 to disable all validation checks, not just release name checks, which is why the broader parameter name "skipValidation" is used.
Learnt from: blazoncek
Repo: wled/WLED PR: 4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Build at least one hardware environment locally (e.g., `pio run -e esp32dev`) before marking work complete
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings
Applied to files:
platformio.ini
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
platformio.ini
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
platformio.ini
📚 Learning: 2025-07-06T16:15:24.051Z
Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:15:24.051Z
Learning: In espressif Arduino framework, `-fno-lto` is set as a standard default in the `platformio-build.py` script. Therefore, using `-fno-lto` in `build_unflags` is not only valid but necessary to remove this default setting before enabling LTO with `-flto=auto` or `-flto=4` in `build_flags`.
Applied to files:
platformio.ini
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
🔇 Additional comments (1)
platformio.ini (1)
28-28: Correctly implements the PR objectives—DIO safe default for OTA compatibility, new QIO variant for faster new installs.The changes properly set up two build environments for ESP32-C3:
- [env:esp32c3dev] with DIO flash mode (line 547) for OTA compatibility from 0.15 with a clear explanatory comment
- [env:esp32c3dev_qio] with QIO flash mode (lines 549-564) for new installs offering faster performance
Both environments are added to default_envs (line 28), ensuring CI/release builds include both variants. The configuration inheritance is correct—both environments properly extend the [esp32c3] template and override board_build.flash_mode as needed. Build flags, release names (ESP32-C3 vs ESP32-C3-QIO), partitions, and lib_deps are consistent and appropriate across both.
As per coding guidelines, platformio.ini is used as the single source of truth for hardware build targets and settings.
Also applies to: 547-548, 549-564
willmmiles
approved these changes
Nov 11, 2025
netmindz
reviewed
Nov 11, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platformio.ini(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
platformio.ini
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use platformio.ini as the single source of truth for hardware build targets and settings
Files:
platformio.ini
🧠 Learnings (6)
📓 Common learnings
Learnt from: willmmiles
Repo: wled/WLED PR: 4998
File: wled00/ota_update.cpp:120-124
Timestamp: 2025-10-11T17:56:24.923Z
Learning: In the WLED OTA update implementation (wled00/ota_update.cpp), the parameter name for bypassing OTA validation is "skipValidation", not "ignoreRelease". The intent evolved from the original spec in issue #4929 to disable all validation checks, not just release name checks, which is why the broader parameter name "skipValidation" is used.
Learnt from: blazoncek
Repo: wled/WLED PR: 4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Build at least one hardware environment locally (e.g., `pio run -e esp32dev`) before marking work complete
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings
Applied to files:
platformio.ini
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Build at least one hardware environment locally (e.g., `pio run -e esp32dev`) before marking work complete
Applied to files:
platformio.ini
📚 Learning: 2025-09-02T02:27:39.220Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Applied to files:
platformio.ini
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
platformio.ini
📚 Learning: 2025-07-06T16:15:24.051Z
Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:15:24.051Z
Learning: In espressif Arduino framework, `-fno-lto` is set as a standard default in the `platformio-build.py` script. Therefore, using `-fno-lto` in `build_unflags` is not only valid but necessary to remove this default setting before enabling LTO with `-flto=auto` or `-flto=4` in `build_flags`.
Applied to files:
platformio.ini
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
🔇 Additional comments (1)
platformio.ini (1)
28-28: Configuration verified—both environments are dynamically included in CI builds.The CI workflow dynamically discovers environments via
pio project config, which extracts the default_envs list from platformio.ini. Your platformio.ini correctly includes both:
- Line 28:
esp32c3dev_qioin the default_envs list- Line 532–547:
[env:esp32c3dev]withboard_build.flash_mode = dio(OTA-safe default)- Line 549–553:
[env:esp32c3dev_qio]withboard_build.flash_mode = qio(faster variant)Both environments will be built automatically by the CI pipeline on each push. The approach correctly implements the DIO/QIO split as described in your review comment, with appropriate flash mode settings and build flags for each target.
Collaborator
Author
|
@netmindz I fixed the qio env, good to merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #5069
@lost-hope as suggested in the discussion about this in #5069 the new default for 0.16 for the web-installer should be the QIO env for C3 as it is faster, the DIO is only needed for OTA updates from 0.15 and for rare cases where a board may only support DIO as they provide two extra pins.
Summary by CodeRabbit
New Features
Bug Fixes