Add factory binary generation for release builds#5712
Conversation
Post-build script uses esptool merge_bin to produce a combined bootloader + partition table + firmware image (.factory.bin) alongside the existing OTA bin, enabling single-file flashing of blank devices. Only runs when platformio_release.ini is active (release builds). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| return 0x10000 | ||
|
|
||
| def create_factory_bin(source, target, env): | ||
| if env["PIOPLATFORM"] == "espressif8266": |
There was a problem hiding this comment.
Might be better to run this gate the other way around (ie. if env["PIOPLATFORM"] != "espressif32": return).
If we add support for any other platforms in the future, they'll almost certainly need a different approach.
| release_name_def = _get_cpp_define_value(env, "WLED_RELEASE_NAME") | ||
| if release_name_def: | ||
| release_name = release_name_def.replace("\\\"", "") | ||
| with open("package.json") as f: | ||
| version = json.load(f)["version"] | ||
| out_file = os.path.join(OUTPUT_DIR, "release", f"WLED_{version}_{release_name}.factory.bin") | ||
| else: | ||
| out_file = os.path.join(OUTPUT_DIR, "firmware", f"{env['PIOENV']}.factory.bin") | ||
|
|
There was a problem hiding this comment.
This code is duplicated from output_bins.py. Normally I'd suggest making a common function to compute the filename, but I've found it kind of a pain to do with platformio scripts - it is possible to make a shared import from the pio-scripts folder, but it's a bit cumbersome; you have to play some games with the import paths.
The technique I ended up using with usermods was to store the relevant value in the environment. (WLED_MODULES exists to pass the resolved usermod list from load_usermods.py to validate_usermods.py).
Probably the thing to do here is to have output_bins.py calculate the path at script execution time, and save it for usage in both action functions.
|
is this similar to |
|
@coderabbitai please compare this PR to #5274. Are there things we can learn & improve when comparing both implementations? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== PR 5712 (current) ==="
gh pr view 5712 --repo wled/WLED --json title,body,files -q '.title, .body' || true
echo "--- files ---"
gh pr view 5712 --repo wled/WLED --json files -q '.files[].path' || true
echo "--- diff ---"
gh pr diff 5712 --repo wled/WLED || true
echo "=== PR 5274 ==="
gh pr view 5274 --repo wled/WLED --json title,body,files -q '.title, .body' || true
echo "--- files ---"
gh pr view 5274 --repo wled/WLED --json files -q '.files[].path' || true
echo "--- diff ---"
gh pr diff 5274 --repo wled/WLED || trueLength of output: 23215
Scope & integration
Partition offset parsing
Firmware-size safety check
Safeboot awareness
Filesystem bundling & flash-size auto-detection
Output handling
Bottom line: the two PRs solve different problems — yours is a minimal, safe, CI-oriented factory-bin generator for release artifacts; |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pio-scripts/factory_bin.py`:
- Around line 33-34: The factory_bin script is relying on PlatformIO internal
env hooks for flash settings, which ties it to unstable implementation details.
Update the logic in the factory-bin generation path to use the documented
BoardConfig accessors instead of env["__get_board_flash_mode"] and
env["__get_board_f_flash"], and keep the existing flash_mode/flash_freq handling
through the public API if those values are available there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dea91443-73e7-4276-beda-ee78af415b99
📒 Files selected for processing (2)
.github/platformio_release.ini.templatepio-scripts/factory_bin.py
| flash_mode = env["__get_board_flash_mode"](env) | ||
| flash_freq = env["__get_board_f_flash"](env) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Are "__get_board_flash_mode" and "__get_board_f_flash" documented/stable PlatformIO SCons environment functions?
💡 Result:
The functions get_board_flash_mode and get_board_f_flash are not documented, stable, or public PlatformIO API functions [1][2]. They are internal helper functions defined within specific PlatformIO development platforms, most notably the espressif32 platform [3][4][5][6]. These functions are injected into the SCons construction environment (env) by platform-specific builder scripts to facilitate the dynamic resolution of flash configuration parameters (like flash mode and flash frequency) from board configuration files during the build process [4][5][6]. Because they are implementation details of the platform's build logic, they are subject to change without notice and are not intended for use in user-level advanced scripts [1][2]. Attempting to use these functions in your own scripts can lead to runtime errors, particularly if the expected board configuration keys are missing or if the platform architecture changes [7]. If you need to access board configuration data in your scripts, you should use the official, documented methods, such as accessing the board configuration object via env.BoardConfig [1][8].
Citations:
- 1: https://dokk.org/documentation/platformio/v5.0.1/projectconf/advanced_scripting/
- 2: https://docs.platformio.org/en/stable/scripting/middlewares.html
- 3: https://github.com/platformio/platform-espressif32/blob/develop/builder/main.py
- 4: https://github.com/platformio/platform-espressif32/blob/f6ec3926f9f660ee9abada8540ffe1e205da4bbf/builder/main.py
- 5: https://github.com/pioarduino/platform-espressif32/blob/main/builder/main.py
- 6: https://github.com/espressif/arduino-esp32/blob/2.0.5/tools/platformio-build.py
- 7: https://community.platformio.org/t/error-when-running-project-with-custom-board/46549
- 8: https://github.com/platformio/platformio-core/blob/3022cb6955e4f099a15c6b5ece53a48e9af436f4/platformio/builder/tools/platformio.py
🏁 Script executed:
rg -n "__get_board_flash_mode|__get_board_f_flash|output_bins\.py|BoardConfig" .Repository: wled/WLED
Length of output: 623
🏁 Script executed:
sed -n '1,220p' pio-scripts/output_bins.pyRepository: wled/WLED
Length of output: 2522
🏁 Script executed:
rg -n "upload\.flash_mode|upload\.flash_freq|build\.flash_mode|build\.flash_freq|flash_mode|flash_freq" pio-scripts platformio.ini .Repository: wled/WLED
Length of output: 3254
🏁 Script executed:
sed -n '1,160p' pio-scripts/factory_bin.pyRepository: wled/WLED
Length of output: 2467
🏁 Script executed:
rg -n 'BoardConfig\(\)\.get\("build\.flash_mode"|BoardConfig\(\)\.get\("build\.f_flash"|BoardConfig\(\)\.get\("upload\.flash_mode"|BoardConfig\(\)\.get\("upload\.flash_freq"' .Repository: wled/WLED
Length of output: 147
Avoid PlatformIO internal env hooks here. env["__get_board_flash_mode"] and env["__get_board_f_flash"] are PlatformIO internals, not public scripting API, so this script is coupled to implementation details that can change on core/platform upgrades. Prefer the documented BoardConfig path if it exposes the needed flash settings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pio-scripts/factory_bin.py` around lines 33 - 34, The factory_bin script is
relying on PlatformIO internal env hooks for flash settings, which ties it to
unstable implementation details. Update the logic in the factory-bin generation
path to use the documented BoardConfig accessors instead of
env["__get_board_flash_mode"] and env["__get_board_f_flash"], and keep the
existing flash_mode/flash_freq handling through the public API if those values
are available there.
There was a problem hiding this comment.
Are these hooks available in pioarduino and newer tasmota framework releases, as we use them in our V5 branch?
There was a problem hiding this comment.
🧩 Analysis chain
🤖 web_search executed:
Does pioarduino platform-espressif32 builder/main.py define get_board_flash_mode and get_board_f_flash functions in recent releases (2024/2025)?
Length of output: 13446
@softhack007 Good news — I checked the pioarduino fork directly, and yes, both hooks are present there too.
In pioarduino/platform-espressif32's builder/main.py, the platform defines internal helpers _get_board_flash_mode(env) and _get_board_f_flash(env), and then injects them into the SCons environment via:
env.Replace(
__get_board_boot_mode=_get_board_boot_mode,
__get_board_f_flash=_get_board_f_flash,
__get_board_f_image=_get_board_f_image,
__get_board_f_boot=_get_board_f_boot,
__get_board_flash_mode=_get_board_flash_mode,
__get_board_memory_type=_get_board_memory_type,
...
)This is consistent across recent pioarduino commits I looked at (main, 879780c5, a9fac073), and the same pattern exists in the upstream platformio/platform-espressif32 develop branch as well. So env["__get_board_flash_mode"] and env["__get_board_f_flash"] should work fine on the pioarduino/tasmota-framework builds used in your V5 branch.
That said, my original caution still stands as a general principle: these are prefixed with double-underscore and are implementation details of the builder script (not part of PlatformIO's documented public scripting API per env.BoardConfig() docs). They happen to be consistently maintained across both the official and pioarduino forks today, but a future refactor of builder/main.py in either fork could rename/remove them without a deprecation notice, since they're not a versioned/stable contract. Given they're currently reliable in practice and already used elsewhere in your codebase (output_bins.py), I don't think this needs to block the PR — just something to keep in mind if a platform upgrade ever breaks the factory-bin script.
3 participants
Post-build script uses esptool merge_bin to produce a combined bootloader + partition table + firmware image (.factory.bin) alongside the existing OTA bin, enabling single-file flashing of blank devices. Only runs when platformio_release.ini is active (release builds).
Summary by CodeRabbit