Release/9.1 into maintenance-10.x - #11793
Merged
Merged
Conversation
Stub only -- captures the buffer-decoupling and shrink-audit technique from the MSP tunnel reply buffer fix before it's lost, ahead of the tracked document-ram-flash-optimization-practices project writing the full guide and linking it from Development.md.
Surfaces RAM/flash regressions and creep on every PR instead of only via a hard CI failure (motivated by PR #11718's terrain-cache RAM overflow, which was only caught because CI happened to fail on that specific board). Extracts flash/RAM usage from each build's .elf right after compiling (no second build of the base branch), persists a baseline per branch as a release asset in the pr-test-builds companion repo, and diffs 4 representative targets (one per MCU family) against it on PR builds.
Several steps spliced \${{ }} expressions directly into run: script text
instead of passing them through env: — including one value (base_ref) that
round-trips through an artifact from a less-trusted PR build before being
used to construct a release tag. Route all of them through env: and
validate base_ref against safe git-ref characters before use.
…iene - Clarify that the 4 representative targets are size tiers, not one-per- silicon-family (two are STM32F7, none are AT32); flag the AT32 gap in code comments/README rather than silently changing the manager-approved target list. - Build size-report.json via jq instead of manual string concatenation, so target names are JSON-escaped rather than assumed safe. - Carry baseFlash/baseRam through diffSizeReports instead of reconstructing them by subtracting the delta back out in renderComment. - Drop a test that only re-asserted a literal copy of a source constant (no behavior exercised); derive the threshold-boundary test from NOISE_THRESHOLD_BYTES instead of hardcoding 32/31. - Note the accepted tradeoff that this (non-gating) script runs on the PR's own checkout, so its numbers are informational only.
Delete-then-recreate left a window where the release didn't exist at all, which a concurrent PR's baseline-fetch could hit and misreport as "no baseline available yet" for a baseline that actually existed. Create the release once, then only ever replace the asset in place on later pushes. Also add a short retry on the fetch side, since an individual asset replace still involves a brief delete-then-upload under the hood.
…f-comment-9.1 Add CI PR comment for RAM/flash usage delta vs. base branch
Contributor
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoAdd CI RAM/flash size-delta PR comments and seed optimization guide
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. Script execute-bit dependency
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+124
to
+125
| - name: Extract size report | ||
| run: .github/scripts/extract-size-report.sh build build/size-report.json |
There was a problem hiding this comment.
1. Script execute-bit dependency 🐞 Bug ☼ Reliability
ci.yml invokes the new .github/scripts/*.sh files directly; if they are not committed with +x, the workflow will fail with a permission error. This repo’s existing workflow pattern executes scripts via bash, so making these steps explicitly bash ... avoids relying on file mode.
Agent Prompt
## Issue description
`ci.yml` runs the new scripts as executables (e.g. `.github/scripts/extract-size-report.sh ...`). If the files are not committed with executable permissions, CI will fail at runtime with `Permission denied`.
## Issue Context
Elsewhere in this repo, workflows run scripts as `bash .github/scripts/<script>.sh`, which avoids depending on the file mode.
## Fix Focus Areas
- .github/workflows/ci.yml[124-126]
- .github/workflows/ci.yml[164-166]
- .github/workflows/ci.yml[260-262]
## Suggested fix
Change the `run:` commands to explicitly invoke bash, e.g.:
- `bash .github/scripts/extract-size-report.sh build build/size-report.json`
- `bash .github/scripts/merge-size-reports.sh ...`
(Alternatively, ensure the scripts are committed with mode `100755`, but using `bash` is the most robust and consistent with existing workflows.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
cmake/arm-none-eabi-checks.cmake downloads its own toolchain into tools/
and adds it to PATH, but only inside that cmake process via
set(ENV{PATH} ...) — that doesn't persist to a later CI step's shell.
extract-size-report.sh defaulted to bare `arm-none-eabi-size` on PATH,
which happened to work in local dev sandboxes with a system copy
installed but fails on actual GitHub Actions runners, which have only
the project's downloaded toolchain. Fall back to the same tools/
location CMake uses when nothing is found on PATH.
…f-comment-9.1 Fix arm-none-eabi-size not found on real CI runners
|
RAM / Flash usage vs. base branch — commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11793 244 targets built. Find your board's
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.