CI: track RAM (.data+.bss) in size reports and gate on per-env budgets - #10899
Merged
Conversation
The 2.8.0 nRF52840 heap regression (99% heap in field reports) shipped
invisibly because CI only tracked flash. On nRF52840 the heap arena is the
linker gap after .bss, so every byte of static .data+.bss growth shrinks the
usable heap 1:1 - RAM needs the same guardrails flash already has.
What's added:
- bin/platformio-custom.py emits ram_bytes (.data + .bss from the ELF, via
the toolchain size tool) into each .mt.json manifest. Heap/stack
placeholder sections are deliberately excluded.
- bin/collect_sizes.py records {flash_bytes, ram_bytes} per env;
bin/size_report.py grows RAM and RAM-delta columns in the PR size report.
Older artifacts without ram_bytes (and legacy int-schema baselines)
degrade to "n/a" instead of crashing.
- bin/ram_budgets.json: per-env RAM/flash budgets, enforced only for envs
listed there. Seeded with rak4631: ram 113,000 (current 110,948 + ~2 KB
slack), flash 786,000 (current 765,192 + ~20 KB; the app region is
0x27000..0xEA000 = 798,720 and the image must stay clear of the
warm-store ring guard in extra_scripts/nrf52_warm_region.py).
- New size-budget-gate CI job runs size_report.py --enforce-budgets and
fails the build on violation; the informational firmware-size-report job
now also renders budget usage into the PR comment.
- src/main.cpp: opt-in boot heap watermark (-DMESHTASTIC_HEAP_WATERMARK_CHECK)
logs LOG_ERROR when less than 20% of the heap is free at the end of
setup(). Off by default; skipped on platforms without heap accounting.
How budgets are raised: deliberately, never automatically. If a change needs
more headroom, bump the env's limit in bin/ram_budgets.json in the same PR
and justify the increase in the PR description.
Verified: python3 bin/test_size_scripts.py (23/23 pass, including ram_bytes
parsing, n/a fallback, and over/under/missing-env budget-gate cases).
Contributor
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (26)
Build artifacts expire on 2026-08-05. Updated for |
github-actions
Bot
added
needs-review
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds static RAM measurement to firmware manifests, propagates RAM and flash sizes through size collection and reporting, adds budget data and CI enforcement, expands tests for the new size schema, and adds a boot-time heap watermark check in ChangesSize Budget Tracking and CI Enforcement
Boot-time Heap Watermark Check
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/size_report.py (1)
300-343: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
--enforce-budgetscan silently pass when no sizes were collected at all.
if not new_sizes: return(pre-existing, Line 307-308) executes before budgets are ever loaded/checked. Ifcollect_sizes.py's output is empty for this run — e.g. an artifact-glob/path misconfiguration in the newsize-budget-gateCI job —main()returns with exit code 0 even though--enforce-budgetswas requested. This is exactly the kind of silent-pass failure mode the budget gate is meant to prevent, and it's untested (no test inbin/test_size_scripts.pycovers empty sizes +--enforce-budgets).🛡️ Suggested fix
new_sizes = load_sizes(args.new_sizes) # Silence output when no targets were built - repo maintainer choice if not new_sizes: + if args.enforce_budgets: + print( + "Error: --enforce-budgets requested but no sizes were found " + f"in {args.new_sizes}; refusing to silently pass the budget gate", + file=sys.stderr, + ) + sys.exit(1) return🤖 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 `@bin/size_report.py` around lines 300 - 343, `main()` in `bin/size_report.py` returns early on empty `new_sizes`, which lets `--enforce-budgets` exit 0 without ever checking budgets. Move the empty-input handling to happen after the `args.enforce_budgets`/budget logic, or make it fail fast when `args.enforce_budgets` is set and `load_sizes(args.new_sizes)` returns nothing. Add a test in `bin/test_size_scripts.py` covering empty collected sizes with `--enforce-budgets` to ensure it fails instead of silently passing.
🤖 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 @.github/workflows/main_matrix.yml:
- Around line 390-404: The size-budget-gate workflow can silently pass with
missing manifests because the download step in the manifest collection flow uses
continue-on-error and the later fallback creates an empty manifests directory.
Remove the fail-open behavior in the Download current manifests / Collect
current firmware sizes sequence so the job fails closed when manifests cannot be
fetched, and make size_report.py enforcement depend on a complete manifest set
for the gated envs rather than proceeding with missing data.
In `@bin/platformio-custom.py`:
- Around line 48-86: Section-name matching in compute_ram_bytes is too narrow
and target-agnostic. Update the section scan in compute_ram_bytes to explicitly
include RISC-V small-data sections like .sdata/.sbss for ESP32-C3/C6, while
excluding non-main-SRAM sections such as ESP-IDF RTC regions like
.rtc.data/.rtc.bss. Use target-aware logic based on the build environment or
platform so the RAM total reflects only heap-competing SRAM.
In `@bin/size_report.py`:
- Around line 246-269: In budget_markdown, guard the percentage calculation
against zero or negative budgets so a bad entry does not trigger a
ZeroDivisionError. Update the flow around check_budgets/load_budgets to reject
non-positive budget values with a clear validation error, and keep
budget_markdown resilient by handling unexpected invalid budgets when rendering
rows.
In `@src/main.cpp`:
- Around line 1161-1164: Trim the explanatory comment near the heap watermark
guardrail in main.cpp to no more than two lines while preserving the essential
intent. Keep only a concise summary around the heap watermark check and the
low-free-space warning, and remove the extra platform-specific explanation from
the surrounding comment block. Use the existing guardrail comment above the
setup() check as the target for the rewrite.
---
Outside diff comments:
In `@bin/size_report.py`:
- Around line 300-343: `main()` in `bin/size_report.py` returns early on empty
`new_sizes`, which lets `--enforce-budgets` exit 0 without ever checking
budgets. Move the empty-input handling to happen after the
`args.enforce_budgets`/budget logic, or make it fail fast when
`args.enforce_budgets` is set and `load_sizes(args.new_sizes)` returns nothing.
Add a test in `bin/test_size_scripts.py` covering empty collected sizes with
`--enforce-budgets` to ensure it fails instead of silently passing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec2ccc36-5899-42bf-9b31-9a1f8832c962
📒 Files selected for processing (7)
.github/workflows/main_matrix.ymlbin/collect_sizes.pybin/platformio-custom.pybin/ram_budgets.jsonbin/size_report.pybin/test_size_scripts.pysrc/main.cpp
This was referenced Jul 6, 2026
Contributor
Firmware Size Report22 targets | vs
Show 17 more target(s)
Updated for 97d607b |
- size-budget-gate workflow: drop continue-on-error on the manifest download and the empty-dir fallback, so the job fails when the data it gates on cannot be fetched. - size_report.py --enforce-budgets now fails closed on every missing-data path instead of trivially passing: empty collected sizes, a budgeted env that was not built, or a manifest without the budgeted metric. Report-only mode keeps rendering those as n/a. - load_budgets() rejects zero/negative/non-integer budgets with a clear error (a typo'd budget could previously crash budget_markdown with ZeroDivisionError or silently skip the check); the percentage render keeps a defensive guard for direct callers. - compute_ram_bytes(): count RISC-V small-data sections (.sdata/.sbss) and exclude ESP-IDF .rtc.* sections, which live outside the heap-competing SRAM. - Trim the heap-watermark comment in main.cpp to two lines. bin/test_size_scripts.py: 27/27 - the two fail-open assertions are flipped to fail-closed, with new report-only counterparts plus cases for empty sizes under enforcement and malformed budgets.
Contributor
Author
|
All five review findings addressed in 08ae317:
|
Itzdavid01
pushed a commit
to Itzdavid01/firmware
that referenced
this pull request
Sep 5, 2026
meshtastic#10899) * CI: track RAM (.data+.bss) in size reports and gate on per-env budgets The 2.8.0 nRF52840 heap regression (99% heap in field reports) shipped invisibly because CI only tracked flash. On nRF52840 the heap arena is the linker gap after .bss, so every byte of static .data+.bss growth shrinks the usable heap 1:1 - RAM needs the same guardrails flash already has. What's added: - bin/platformio-custom.py emits ram_bytes (.data + .bss from the ELF, via the toolchain size tool) into each .mt.json manifest. Heap/stack placeholder sections are deliberately excluded. - bin/collect_sizes.py records {flash_bytes, ram_bytes} per env; bin/size_report.py grows RAM and RAM-delta columns in the PR size report. Older artifacts without ram_bytes (and legacy int-schema baselines) degrade to "n/a" instead of crashing. - bin/ram_budgets.json: per-env RAM/flash budgets, enforced only for envs listed there. Seeded with rak4631: ram 113,000 (current 110,948 + ~2 KB slack), flash 786,000 (current 765,192 + ~20 KB; the app region is 0x27000..0xEA000 = 798,720 and the image must stay clear of the warm-store ring guard in extra_scripts/nrf52_warm_region.py). - New size-budget-gate CI job runs size_report.py --enforce-budgets and fails the build on violation; the informational firmware-size-report job now also renders budget usage into the PR comment. - src/main.cpp: opt-in boot heap watermark (-DMESHTASTIC_HEAP_WATERMARK_CHECK) logs LOG_ERROR when less than 20% of the heap is free at the end of setup(). Off by default; skipped on platforms without heap accounting. How budgets are raised: deliberately, never automatically. If a change needs more headroom, bump the env's limit in bin/ram_budgets.json in the same PR and justify the increase in the PR description. Verified: python3 bin/test_size_scripts.py (23/23 pass, including ram_bytes parsing, n/a fallback, and over/under/missing-env budget-gate cases). * Address review: fail the budget gate closed, fix RAM section matching - size-budget-gate workflow: drop continue-on-error on the manifest download and the empty-dir fallback, so the job fails when the data it gates on cannot be fetched. - size_report.py --enforce-budgets now fails closed on every missing-data path instead of trivially passing: empty collected sizes, a budgeted env that was not built, or a manifest without the budgeted metric. Report-only mode keeps rendering those as n/a. - load_budgets() rejects zero/negative/non-integer budgets with a clear error (a typo'd budget could previously crash budget_markdown with ZeroDivisionError or silently skip the check); the percentage render keeps a defensive guard for direct callers. - compute_ram_bytes(): count RISC-V small-data sections (.sdata/.sbss) and exclude ESP-IDF .rtc.* sections, which live outside the heap-competing SRAM. - Trim the heap-watermark comment in main.cpp to two lines. bin/test_size_scripts.py: 27/27 - the two fail-open assertions are flipped to fail-closed, with new report-only counterparts plus cases for empty sizes under enforcement and malformed budgets.
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.
Motivation
The 2.8.0 nRF52840 heap regression (field reports of 99% heap usage) shipped invisibly: CI tracks flash sizes, but nothing tracks RAM. On nRF52840 the heap arena is the linker gap after
.bss, so every byte of static.data+.bssgrowth shrinks the usable heap 1:1. rak4631 is also at 95.8% of its app flash region (0x27000..0xEA000= 798,720 B). This PR gives RAM the same CI guardrails flash already has, plus a hard budget gate for the tightest target.What's added
1. RAM in the size manifests and PR size report
bin/platformio-custom.pynow emitsram_bytes(.data+.bsssummed fromsize -Aon the ELF, via the toolchain's$SIZETOOL) into each.mt.jsonmanifest. Heap/stack placeholder sections (e.g. the nRF52.heapsection) are deliberately excluded - the heap arena is the linker gap, not a static allocation.bin/collect_sizes.pyrecords{"flash_bytes": ..., "ram_bytes": ...}per env;bin/size_report.pygrows RAM and RAM-delta columns. Older artifacts withoutram_bytes(including legacy int-schema baseline artifacts from previous develop/master runs) degrade gracefully ton/a- never crash.Sample output (synthetic manifests; heltec-v3's manifest lacks
ram_bytes, baseline is mixed legacy/new schema):2 targets | vs
develop: 2 increased, net +2,528 (+2.5 KB)developdevelopheltec-v3rak4631Size budgets
rak4631rak46312. Budget gate (
bin/ram_budgets.json+size-budget-gatejob){"<env>": {"ram_bytes": <budget>, "flash_bytes": <budget>}}; enforced only for envs listed in the file (and only when that env was built in the run)."_comment"keys document the semantics.extra_scripts/nrf52_warm_region.py).size-budget-gateCI job runssize_report.py --budgets bin/ram_budgets.json --enforce-budgetsand fails the build on violation - it is deliberately separate fromfirmware-size-report, which is informational andcontinue-on-error. That job now additionally renders the budget-usage table into the PR comment.Budgets are raised deliberately, never automatically: bump the limit in
bin/ram_budgets.jsonin the same PR that needs the headroom and justify it in the PR description.3. Boot heap watermark flag (firmware, opt-in)
At the very end of
setup(), under#ifdef MESHTASTIC_HEAP_WATERMARK_CHECK, logLOG_ERRORwhen less than 20% of the heap is free at boot. Off by default; CI/test builds enable with-DMESHTASTIC_HEAP_WATERMARK_CHECK. Platforms without heap accounting (getHeapSize()returningUINT32_MAX/0, seesrc/memGet.cpp) are skipped.Verification
python3 bin/test_size_scripts.py: 23/23 pass, including new cases forram_bytescollection, malformed/missingram_bytes, legacy-schema baselines, then/afallback, and the budget gate (over-RAM, over-flash, under, env-not-built, missing-metric, render-only,--enforce-budgetswithout--budgets).size -Ashapes: nRF52 output yields exactly 110,948 (.data2,504 +.bss108,444,.heap/.stack_dummyexcluded); ESP32.dram0.data/.dram0.bssare picked up,.flash.rodata/debug sections excluded.collect_sizes.py→size_report.pyagainst a synthetic manifest pair (with/withoutram_bytes) and a mixed-schema baseline - the tables above are that run's output; enforce mode verified exit 1 over budget, exit 0 under.trunk fmt/trunk checkclean on touched files (no new issues).Summary by CodeRabbit