Add tl-install to install tools not in pio registry. Using for install of newer OpenOCD#148
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes introduce updates across configuration, platform manifest, exception decoding, and platform logic. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Platform
participant idf_tools.py
participant ToolPackageManager
User->>Platform: Initiate build or debug/upload
Platform->>Platform: Check build type, debug/upload protocol, MCU
alt Tool installation required
Platform->>idf_tools.py: Run install_tool(TOOL)
idf_tools.py-->>Platform: Install/activate tool
Platform->>ToolPackageManager: Copy package metadata, clean up old dirs
end
Platform->>User: Proceed with build/upload/debug
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 3
🧹 Nitpick comments (4)
examples/arduino-blink/platformio.ini (1)
93-98:build_type = debugwill grow firmware considerablyGreat for development, but shipping this example with permanent debug builds may surprise newcomers (flash overflow & slower compile times).
Two easy fixes:
-[env:esp32-c6-devkitc-1] +; Enable debug only when the user explicitly selects the *debug* build profile +[env:esp32-c6-devkitc-1] +;build_type = release ; default +# Uncomment the next line to switch to debug +# build_type = debugor add a dedicated
[env:esp32-c6-devkitc-1-debug]environment.platform.py (3)
15-25: Import style – avoidfrom os.path import joinshadowing
join()is used only once and already referenced viaos.path.joinelsewhere. Mixing both styles is error-prone and confuses static analysers.-import shutil -from os.path import join +import shutiland replace the lone
join(call withos.path.join(.
90-93:tl-installde-activation logic can mis-fire
tl-installis marked optional only after checkingidf_tools.pyexistence inside the package.
On the first run the directory doesn’t exist yet → the package is forced to install twice (once via PlatformIO, once viapm.install).
Consider flipping the logic:
- Leave
tl-installoptional inplatform.json.- In
configure_default_packages, if the script is absent, setoptional=False.This prevents redundant downloads.
163-166: MCU list hard-codes every future variantThe tuple grows each release. A more maintainable approach:
RISC_V_MCUS = ("esp32c", "esp32h2", "esp32p4") if mcu.startswith(RISC_V_MCUS): ...Consider using regex or family prefix detection to avoid constant updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/arduino-blink/platformio.ini(1 hunks)monitor/filter_exception_decoder.py(1 hunks)platform.json(4 hunks)platform.py(4 hunks)
🔇 Additional comments (2)
platform.json (2)
86-97:⚠️ Potential issueUn-pinned GitHub ZIPs hurt reproducibility & security
Several new tools (
tool-esptoolpy,tl-install,tool-openocd-esp32, …) reference mutablemaster.zip/latest.zip-style URLs. A later retag/force-push will silently change the bits and break deterministic builds or introduce supply-chain risk.Consider:
-"version": "https://github.com/espressif/arduino-esp32/archive/master.zip" +"version": "https://github.com/espressif/arduino-esp32/archive/3ad36321e9c07cb9c0e.zip" # commit hashor attach SHA-256 in
package.jsonand use PlatformIO’schecksumfield.Likely an incorrect or invalid review comment.
152-169:Details
✅ Verification successful
Potential JSON duplication / trailing comma risk
The long
"packages"map now ends with"tool-scons"(line 165-168). Please double-check:
- No duplicate keys (
tool-sconswas already present earlier?).- Last item has no trailing comma; otherwise
json.loadswill fail during platform installation.Run:
to verify validity in CI.
🏁 Script executed:
python -m json.tool platform.jsonLength of output: 5460
JSON validity confirmed – no duplicate keys or trailing commas
Verified with
python -m json.tool platform.jsonthat:
- No duplicate entries for
"tool-scons"exist.- The final
"tool-scons"block ends without a trailing comma, keeping the JSON valid.No changes required.
1 participant
Description:
Currently it is only possible to install tools and toolchains from Platformio registry.
The PR adds the option to install tools from a github repo release section. To make this possible a tarball needs to be created which has a valid platformio
package.json(not platform specific!) and a manifest json with nametools.jsonin it. The syntax for the entrys intools.jsonis explained here The link to the tarball is added in theplatform.jsonunder the tool "version" entry. Make the tool option entry "optional" =TrueFor installing and activating the tool via the alternative installer it simply needs to be called like this
install_tool("tool-openocd-esp32")in platform.py. Now the tooltool-openocd-esp32will be installed (matching to the OS) from the espressif github OpenOCD repo and can be used like installed from Platformio registry.Checklist:
Summary by CodeRabbit