-
Notifications
You must be signed in to change notification settings - Fork 25
Build and integrate slave firmware #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new GitHub Actions workflow for parallel building of ESP32 Arduino libraries and framework packaging was added. The existing push workflow was updated to include slave firmware building and improved artifact handling. A new script automates slave firmware compilation for multiple ESP32 targets. The CI preparation script now uses Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Build Script
participant Artifact Store
participant Release Step
User->>GitHub Actions: Trigger workflow_dispatch (parallel_build.yaml)
GitHub Actions->>Build Script: build-libs job (parallel for each ESP32 target)
Build Script->>Artifact Store: Upload build artifacts
GitHub Actions->>Build Script: build-slave_firmware job
Build Script->>Artifact Store: Upload slave firmware artifact
GitHub Actions->>GitHub Actions: combine-artifacts job (after build-libs and build-slave_firmware)
GitHub Actions->>Artifact Store: Download all artifacts
GitHub Actions->>GitHub Actions: Extract, combine framework and slave firmware, repackage
GitHub Actions->>Artifact Store: Upload combined framework artifact and release info
GitHub Actions->>Release Step: release_framework job creates GitHub prerelease with artifacts
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
tools/prepare-ci.sh (1)
1-18
: Add strict bash flags & trap failures earlyThe script is still running in best-effort mode.
A single failed command (e.g. network hiccup while fetching uv) will silently continue and break subsequent steps in CI.#!/bin/bash +# fail fast & propagate the correct exit status +set -euo pipefailUsing
set -euo pipefail
at the top keeps the CI signal clean and prevents the rest of the workflow from running with a half-configured Python environment.
(You can drop-u
if unbound vars are intentional.)
🧹 Nitpick comments (1)
tools/compile_slave.sh (1)
29-36
: Quote variables & copy only when build succeededMinor robustness tweaks:
- cp ./build/network_adapter.bin ../wifi_copro_fw/network_adapter_"$target".bin + cp "./build/network_adapter.bin" "../wifi_copro_fw/network_adapter_${target}.bin"Quoting avoids surprises when paths contain spaces;
${target}
keeps a uniform style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/parallel_build.yaml
(1 hunks).github/workflows/push.yml
(1 hunks)tools/compile_slave.sh
(1 hunks)tools/prepare-ci.sh
(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/compile_slave.sh
[warning] 26-26: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 actionlint (1.7.7)
.github/workflows/parallel_build.yaml
2-2: unexpected key "description" for "workflow" section. expected one of "concurrency", "defaults", "env", "jobs", "name", "on", "permissions", "run-name"
(syntax-check)
⏰ 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). (2)
- GitHub Check: Build Arduino Libs
- GitHub Check: Build Slave Firmware
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tools/compile_slave.sh (1)
1-5
: Addset -euo pipefail
to fail fast and catch hidden errors
Re-raising an earlier suggestion: the script still continues even if any command in the build pipeline fails (e.g.idf.py build
). A single typo or out-of-disk condition will silently propagate bad artifacts downstream.#!/bin/bash +set -euo pipefail
🧹 Nitpick comments (3)
.github/workflows/push.yml (1)
71-76
: Replacels | head
withfind
and quote the variable
ls
parsing plus unquoted command substitution tripsshellcheck
(SC2012, SC2046) and can break on filenames containing spaces or newlines.- FRAMEWORK_ZIP=$(ls dist/framework*.zip | head -n 1) + FRAMEWORK_ZIP=$(find dist -maxdepth 1 -name 'framework*.zip' -print -quit) unzip -q "$FRAMEWORK_ZIP" -d dist/unpacked.github/workflows/parallel_build.yaml (2)
23-26
: Quote environment expansion to avoid word-splitting (SC2086)
$GITHUB_REF
can in theory contain spaces; quoting silences actionlint & shellcheck noise.- echo "GIT_BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}" >> $GITHUB_ENV + echo "GIT_BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}" >> "$GITHUB_ENV"
75-88
: Minor cleanup: drop trailing spaces & add safety flags to ad-hoc shell block
While not functionally harmful, trailing whitespace (YAML-lint) and missingset -euo pipefail
inside the multiline script reduce readability and robustness.- run: | - mkdir -p out dist/framework-arduinoespressif32 + run: | + set -euo pipefail + mkdir -p out dist/framework-arduinoespressif32 @@ - unzip -q "$zip_file" -d out + unzip -q "$zip_file" -d out
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/parallel_build.yaml
(1 hunks).github/workflows/push.yml
(1 hunks)tools/compile_slave.sh
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: .github/workflows/push.yml:71-75
Timestamp: 2025-07-11T15:27:03.955Z
Learning: In the esp32-arduino-lib-builder project, the slave firmware files should be copied to maintain the wifi_copro_fw/ subdirectory structure within tools/slave_firmware/. The correct final structure is tools/slave_firmware/wifi_copro_fw/network_adapter_*.bin, not directly in tools/slave_firmware/. This folder structure is intentional and correct according to maintainer Jason2866.
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: tools/prepare-ci.sh:15-17
Timestamp: 2025-07-11T15:23:40.785Z
Learning: In tools/prepare-ci.sh for the esp32-arduino-lib-builder project, virtual environment activation after `uv venv` is not needed according to the maintainer Jason2866's specific use case and workflow requirements.
.github/workflows/parallel_build.yaml (1)
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: .github/workflows/push.yml:71-75
Timestamp: 2025-07-11T15:27:03.955Z
Learning: In the esp32-arduino-lib-builder project, the slave firmware files should be copied to maintain the wifi_copro_fw/ subdirectory structure within tools/slave_firmware/. The correct final structure is tools/slave_firmware/wifi_copro_fw/network_adapter_*.bin, not directly in tools/slave_firmware/. This folder structure is intentional and correct according to maintainer Jason2866.
.github/workflows/push.yml (2)
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: .github/workflows/push.yml:71-75
Timestamp: 2025-07-11T15:27:03.955Z
Learning: In the esp32-arduino-lib-builder project, the slave firmware files should be copied to maintain the wifi_copro_fw/ subdirectory structure within tools/slave_firmware/. The correct final structure is tools/slave_firmware/wifi_copro_fw/network_adapter_*.bin, not directly in tools/slave_firmware/. This folder structure is intentional and correct according to maintainer Jason2866.
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: tools/prepare-ci.sh:15-17
Timestamp: 2025-07-11T15:23:40.785Z
Learning: In tools/prepare-ci.sh for the esp32-arduino-lib-builder project, virtual environment activation after `uv venv` is not needed according to the maintainer Jason2866's specific use case and workflow requirements.
tools/compile_slave.sh (2)
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: .github/workflows/push.yml:71-75
Timestamp: 2025-07-11T15:27:03.955Z
Learning: In the esp32-arduino-lib-builder project, the slave firmware files should be copied to maintain the wifi_copro_fw/ subdirectory structure within tools/slave_firmware/. The correct final structure is tools/slave_firmware/wifi_copro_fw/network_adapter_*.bin, not directly in tools/slave_firmware/. This folder structure is intentional and correct according to maintainer Jason2866.
Learnt from: Jason2866
PR: Jason2866/esp32-arduino-lib-builder#117
File: tools/prepare-ci.sh:15-17
Timestamp: 2025-07-11T15:23:40.785Z
Learning: In tools/prepare-ci.sh for the esp32-arduino-lib-builder project, virtual environment activation after `uv venv` is not needed according to the maintainer Jason2866's specific use case and workflow requirements.
🪛 actionlint (1.7.7)
.github/workflows/parallel_build.yaml
23-23: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/push.yml
71-71: shellcheck reported issue in this script: SC2012:info:1:17: Use find instead of ls to better handle non-alphanumeric filenames
(shellcheck)
71-71: shellcheck reported issue in this script: SC2046:warning:5:33: Quote this to prevent word splitting
(shellcheck)
🪛 YAMLlint (1.37.1)
.github/workflows/parallel_build.yaml
[error] 76-76: trailing spaces
(trailing-spaces)
[error] 81-81: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
tools/compile_slave.sh
[warning] 26-26: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
Summary by CodeRabbit
New Features
Chores