Remove old V3 IDF #4836
Remove old V3 IDF #4836
Conversation
WalkthroughCentralizes ESP32 PlatformIO configuration around a new ESP-IDF v4 block, adds global build cache and extra config, refactors ESP32 environments to inherit the V4 settings, removes one legacy env, and adds a dedicated esp32dev_V4 environment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
platformio.ini (1)
295-295: Typo/stray macro: “-DCO” sneaked into S2/C3/S3 flags“-DCO” is likely a truncated “-D CORE_DEBUG_LEVEL=0” and currently defines an empty CO macro, which can cause subtle compile-time issues. Remove it (CORE_DEBUG_LEVEL is already set globally in [common.build_flags]).
[esp32s2] - -DCO [esp32c3] - -DCO [esp32s3] - -DCOIf you did intend a debug-level define here, replace with an explicit, correct macro name/value.
Also applies to: 313-313, 333-333
🧹 Nitpick comments (3)
platformio.ini (3)
17-19: Global build cache path may be brittle across OS/CI — prefer sysenv or move to overrideUsing a literal tilde in INI may not expand consistently on Windows or some CI runners. Either move this to platformio_override.ini (developer-local) or use ${sysenv.HOME} for portability.
Apply one of the following diffs:
Option A (portable path):
-build_cache_dir = ~/.buildcache +build_cache_dir = ${sysenv.HOME}/.buildcacheOption B (move to local override; keep repo defaults clean):
-build_cache_dir = ~/.buildcache -extra_configs = - platformio_override.ini +; build_cache_dir is best set in platformio_override.ini to avoid cross-OS issues +extra_configs = + platformio_override.iniIf you choose option B, add
build_cache_dir = ${sysenv.HOME}/.buildcachein your local platformio_override.ini.
268-268: Outdated comment: says arduino-esp32 v2.0.5 but platform pins 2.0.9Revise the note to avoid confusion.
-;; build environment for ESP32 using ESP-IDF 4.4.x / arduino-esp32 v2.0.5 +;; build environment for ESP32 using ESP-IDF 4.4.x / arduino-esp32 v2.0.9
470-479: esp32_eth rebased to V4 — switch flags source for consistencyYou already set platform = ${esp32_idf_V4.platform}. For consistency and to decouple from the intermediate [esp32] section, reference ${esp32_idf_V4.build_flags} directly.
-build_flags = ${common.build_flags} ${esp32.build_flags} -D WLED_RELEASE_NAME="ESP32_Ethernet" -D RLYPIN=-1 -D WLED_USE_ETHERNET -D BTNPIN=-1 +build_flags = ${common.build_flags} ${esp32_idf_V4.build_flags} -D WLED_RELEASE_NAME="ESP32_Ethernet" -D RLYPIN=-1 -D WLED_USE_ETHERNET -D BTNPIN=-1Functionally equivalent today, but more robust if [esp32] ever diverges again.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
platformio.ini(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-19T12:43:34.199Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Applied to files:
platformio.ini
📚 Learning: 2025-07-06T16:15:24.051Z
Learnt from: Jason2866
PR: wled/WLED#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
🔇 Additional comments (2)
platformio.ini (2)
13-13: default_envs trimmed to V4-only ESP32 builds — aligns with PR goalDropping esp32dev_V4 from defaults and keeping only V4-based ESP32 envs matches “Make all ESP32 builds use the V4 ESP-IDF.” No concerns.
482-493: WROVER now inherits from esp32_idf_V4 — good reuseExtending the V4 base reduces duplication and keeps the PSRAM-specific flags localized. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
platformio.ini (1)
266-273: Comment/version mismatch: header states v2.0.5, platform pins v2.0.9The header says “arduino-esp32 v2.0.5,” but the pinned platform zip is 2.0.9 (IDF 4.4.4). Update the comment to avoid confusion for maintainers.
Apply:
-[esp32_idf_V4] -;; build environment for ESP32 using ESP-IDF 4.4.x / arduino-esp32 v2.0.5 +[esp32_idf_V4] +;; build environment for ESP32 using ESP-IDF 4.4.x / arduino-esp32 v2.0.9 @@ -;; select arduino-esp32 v2.0.9 (arduino-esp32 2.0.10 thru 2.0.14 are buggy so avoid them) +;; select arduino-esp32 v2.0.9 (arduino-esp32 2.0.10 thru 2.0.14 are buggy so avoid them) platform = https://github.com/tasmota/platform-espressif32/releases/download/2023.06.02/platform-espressif32.zip ;; Tasmota Arduino Core 2.0.9 with IPv6 support, based on IDF 4.4.4
♻️ Duplicate comments (1)
platformio.ini (1)
246-251: Quick repo scan for lingering V3-era referencesSince the goal is “all ESP32 builds use V4,” scan for any stray IDF v3 / Arduino-core v1.x references, pins, or package overrides.
#!/bin/bash set -euo pipefail echo "Searching for V3/legacy references across repo..." rg -n -A1 -B1 $'idf[^0-9]*3\\.|ESP-IDF *3|arduino-esp32 v1\\.|framework-arduinoespressif32 @ 1\\.|esp32_idf_V3|V3 build' || true echo "Check for explicit espressif32@ platform pins:" rg -n -A1 -B1 $'^platform\\s*=.*espressif32@' || true echo "Check for platform_packages that pull in framework-arduinoespressif32 explicitly:" rg -n -A1 -B1 $'platform_packages\\s*=.*framework-arduinoespressif32' || true
🧹 Nitpick comments (1)
platformio.ini (1)
246-251: ESP32 base consolidated to IDF V4 — good; clarify empty platform_packagesNice centralization to
${esp32_idf_V4.platform}and shared flags/deps. The empty platform_packages is easy to misread as accidental; add a clarifying comment to make the intent explicit.Apply:
platform = ${esp32_idf_V4.platform} -platform_packages = +platform_packages = ; intentionally blank to avoid legacy/overridden framework packages build_unflags = ${common.build_unflags} build_flags = ${esp32_idf_V4.build_flags} lib_deps = ${esp32_idf_V4.lib_deps}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
platformio.ini(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-06T16:15:24.051Z
Learnt from: Jason2866
PR: wled/WLED#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
📚 Learning: 2025-02-19T12:43:34.199Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
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). (1)
- GitHub Check: wled_build / Build Environments (usermods)
🔇 Additional comments (3)
platformio.ini (3)
470-470: Move esp32_eth to IDF V4 — good alignmentPlatform now references the shared V4 block, consistent with the PR objective. No functional concerns spotted with this change itself.
258-258: No action neededLine-only change/whitespace. Nothing to do here.
478-478: Confirm flash_mode for esp32-poe (env:esp32_eth) — currently DIOScan of platformio.ini shows env:esp32_eth (board = esp32-poe) sets
board_build.flash_mode = dio(platformio.ini:478). Several other ESP32 envs useqio(and one usesdout); there's also a commentedqionearby. Please confirm this matches esp32-poe hardware guidance and prior WLED images to avoid regressions.Attention locations:
- platformio.ini:468–479 — [env:esp32_eth] (board = esp32-poe) — board_build.flash_mode = dio (line 478)
- platformio.ini:450–453 — commented
board_build.flash_mode = qio(line 452) — possible prior/default- Other qio occurrences (examples): lines 318, 484, 526, 545, 588, 595
- Other dio occurrences: lines 438, 466, 624
Snippet: board_build.flash_mode = dio (platformio.ini:478)
Please confirm whether to keep DIO (and document why) or change to QIO to match other ESP32 targets / previous WLED images.
| # (use `platformio_override.ini` when building for your own board; see `platformio_override.ini.sample` for an example) | ||
| # ------------------------------------------------------------------------------ | ||
|
|
||
| # CI/release binaries |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm removal of V3-specific envs and default artifacts impact
"default_envs" no longer includes any V3/legacy ESP32 envs (e.g., esp32dev_V4). Given that esp32dev now targets V4, ensure CI/release pipelines and artifact naming don’t rely on the old V4-suffixed env. Adjust downstream packaging/docs if needed.
Run this quick check to spot any references to deprecated V3/V4-suffixed env names in the repo:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for old env names that might still be mentioned in docs/scripts
rg -n -A1 -B1 $'\[env:esp32dev_V4\]|esp32dev_V3|esp32_idf_V3' || trueLength of output: 89
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Finding platformio.ini files..."
files=$(fd -HI --hidden --exclude node_modules platformio.ini 2>/dev/null || true)
if [ -z "$files" ]; then
files=$(rg --files --hidden --glob '!node_modules' | rg "platformio\.ini$" || true)
fi
if [ -z "$files" ]; then
echo "No platformio.ini files found in the repo."
exit 0
fi
echo "platformio.ini files found:"
printf '%s\n' "$files"
echo
for f in $files; do
echo "== $f =="
nl -ba "$f" | sed -n '1,240p'
echo
echo "-- default_envs lines in $f --"
rg -n '^\s*default_envs\s*=' "$f" || echo " (none)"
echo
echo "-- [env:...] entries in $f --"
rg -n '^\[env:' "$f" || echo " (none)"
echo
done
echo "Searching entire repo for env sections containing V3/V4 (case-insensitive)..."
rg -n --hidden --glob '!node_modules' -i '\[env:[^\]]*(?:v3|v4)[^\]]*\]' || true
echo "Searching for esp32dev / esp32_idf mentions with V3/V4 suffixes (case-insensitive)..."
rg -n --hidden --glob '!node_modules' -i '(?:esp32dev|esp32_idf)[-_]?v[34]\b' || true
echo "Searching for any esp32dev mentions (for broader context)..."
rg -n --hidden --glob '!node_modules' -i 'esp32dev' || trueLength of output: 550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locating platformio.ini..."
files=$(fd -HI --hidden --exclude node_modules platformio.ini 2>/dev/null || true)
if [ -z "$files" ]; then
files=$(rg --files --hidden --glob '!node_modules' | rg "platformio\.ini$" || true)
fi
if [ -z "$files" ]; then
echo "No platformio.ini found in the repo."
exit 0
fi
echo "platformio.ini files found:"
printf '%s\n' "$files"
echo
for f in $files; do
echo "== $f =="
echo "---- file (first 240 lines) ----"
awk 'NR<=240{printf("%5d %s\n",NR,$0)}' "$f" || true
echo
echo "-- default_envs lines in $f --"
rg -n '^\s*default_envs\s*=' "$f" || echo " (none)"
echo
echo "-- [env:...] entries in $f --"
rg -n '^\[env:' "$f" || echo " (none)"
echo
done
echo "Searching repo for V3/V4-suffixed env names and esp32dev mentions (case-insensitive)..."
rg -n --hidden --glob '!node_modules' -i '\[env:esp32dev_V4\]|\[env:esp32dev_V3\]|\besp32dev[_-]v[34]\b|\besp32dev_v[34]\b|\besp32_idf_v[34]\b' || true
echo
echo "Searching for any default_envs occurrences across repo..."
rg -n --hidden --glob '!node_modules' -i '^\s*default_envs\s*=' || true
echo
echo "Searching CI/docs/scripts for explicit env references (platformio/pio calls, release artifacts)..."
rg -n --hidden --glob '!node_modules' -i 'platformio .* -e|pio run -e|esp32dev[_-]v[34]|esp32_idf[_-]v[34]|esp32dev_v[34]' || trueLength of output: 22717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
show() {
file="$1"
start="$2"
end="$3"
if [ -f "$file" ]; then
echo "== $file ($start-$end) =="
sed -n "${start},${end}p" "$file" || true
echo
else
echo "== $file not found =="
echo
fi
}
# platformio.ini relevant ranges
show platformio.ini 230 340
show platformio.ini 420 640
# sample override and usermods files that were matched earlier
show platformio_override.sample.ini 1 30
show platformio_override.sample.ini 240 272
show usermods/platformio_override.usermods.ini 1 80
show usermods/usermod_v2_rotary_encoder_ui_ALT/platformio_override.sample.ini 1 80
show usermods/usermod_v2_four_line_display_ALT/platformio_override.sample.ini 1 80
# CI workflows
show .github/workflows/build.yml 1 240
show .github/workflows/usermods.yml 1 240Length of output: 25629
Action: Update sample overrides / usermods that still reference esp32dev_V4
Verified: platformio.ini default_envs no longer contains V4-suffixed ESP32 envs (default_envs uses "esp32dev"). CI builds read default_envs via pio project config and name artifacts as firmware-${{ matrix.environment }} — so artifact names will follow the new env names. However several sample/override and usermod files still reference esp32dev_V4 (or esp32dev_V4_dio80) and should be updated or have compatibility envs added.
Files/locations to fix:
- platformio.ini — line 13: default_envs = … esp32dev (confirmed removal of _V4 from defaults)
- platformio_override.sample.ini — default_envs includes esp32dev_V4_dio80 and contains [env:esp32dev_V4_dio80] (sample override that will confuse users if copied)
- usermods/platformio_override.usermods.ini — extends = env:esp32dev_V4 (used by Usermods CI when copying this file)
- usermods/usermod_v2_rotary_encoder_ui_ALT/platformio_override.sample.ini — extends = env:esp32dev_V4
- usermods/usermod_v2_four_line_display_ALT/platformio_override.sample.ini — extends = env:esp32dev_V4
- .github/workflows/build.yml & .github/workflows/usermods.yml — CI builds derive the matrix from pio config and upload artifacts named firmware-${{ matrix.environment }}; update downstream packaging/docs if they expect old V4-suffixed artifact names.
Recommendation (choose one):
- Replace esp32dev_V4* references in samples/usermods with the new env names (e.g., esp32dev / the appropriate env), OR
- Reintroduce compatibility env aliases (esp32dev_V4 -> esp32dev) if you must preserve old artifact names/overrides.
| AR_lib_deps = ;; for pre-usermod-library platformio_override compatibility | ||
|
|
||
|
|
||
| [esp32_idf_V4] |
There was a problem hiding this comment.
If all are V4, do we need to state so? I.e. remove this?
There was a problem hiding this comment.
Yeah I wasn't sure if it made more sense to update the regular esp32 block or move things over.
When considering users with their own forks or at least platformio_override I thought it might possibly be best to use explicit V4 params in our example so it was clearer what was being used, but then for backwards compatibility set the default esp32 to reference V4
I'm happy to hear other suggestions
One other thing to bare in mind is what happens if we start adding some V5 builds into the mix too
There was a problem hiding this comment.
thats a good point. in that case keep the V4. Does it need to be added to S2/S3/C3 envs?
There was a problem hiding this comment.
yes, c3, s2 and s3 all use platform = ${esp32_idf_V4.platform}
| platform = [email protected] | ||
| platform_packages = framework-arduinoespressif32 @ https://github.com/Aircoookie/arduino-esp32.git#1.0.6.4 | ||
| platform = ${esp32_idf_V4.platform} | ||
| platform_packages = |
There was a problem hiding this comment.
I saw a PR about adding these as they are missing for s3 etc. So are the packages required? Should they be added to the base definitions?
There was a problem hiding this comment.
My (limited) understanding is that you only need to define if you are overriding the default tools for the platform
|
Generally looks good to me |
2 participants
Make all ESP32 builds use the V4 ESP-IDF
Summary by CodeRabbit
New Features
Refactor
Chores