Check the ESP32's documented sizes, and correct them - #70
Merged
Conversation
The ESP32 was the one target whose figures no check covered, and it showed: the README's verification block read 10.8 % RAM and 17.5 % flash, from a build before the two pages and the network update existed, while the app slot table further down the same file put the image at three times that share. Two halves of one file disagreeing by a factor of two, for long enough that neither looked suspicious. Measured now: 16 % of 320 KB and 33 % of the 3.19 MB app slot. The derived table was mildly stale too and --fix corrected it - 31/83/55 became 33/85/57. Flash is the size of firmware.bin rather than a sum over sections, and that is the whole reason this check took a moment to get right. The S3's ELF carries .ext_ram.dummy, .flash_rodata_dummy and .dram0.dummy - padding and aliases the loader never writes - so adding up what sits at a flash address counts over a megabyte that is not in the image. The image file is not a reconstruction of the artefact, it is the artefact. Same lesson as the AVR figure, where asking avr-size replaced adding .text to .data and being wrong by six bytes. RAM has no such trap once the dummy is skipped: .dram0.data plus .dram0.bss land within two bytes of what PlatformIO prints. The two mentions that cannot be checked are gone rather than left to rot: the parts table now points at the slot table instead of repeating a percentage, and "the image is about 1.05 MB" is replaced by the table that says the same thing three ways and is measured. The firmware CI job runs it, so this cannot drift again without failing the pull request that caused it. Verified: the check passes on a current build of all three targets, fails with exit 1 when a number is edited to a wrong one, and refuses --esp32-bin without --esp32-elf rather than silently skipping the RAM figure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Follow-up to the find in #69: the ESP32 was the one target whose sizes
documented-sizes.pydid not cover, and it had rotted far enough that the same file contradicted itself.What was wrong
platform/esp32/README.mdsaid, in its verification block:and, three paragraphs down, that the image was "about 1.05 MB" against a 3.19 MB app slot — i.e. about a third, not a sixth. Two halves of one file disagreeing by a factor of two, from a build predating the two web pages and the network update. Nothing caught it because the AVR and the RP2350 have CI checks and the ESP32 did not.
Measured on a current build: 16 % of 320 KB and 33 % of the 3.19 MB app slot. The derived slot table was mildly stale too —
--fixturned 31/83/55 into 33/85/57.Why flash is measured off
firmware.bin, not off sectionsThis is the part worth reviewing. The obvious implementation — sum the sections at flash addresses, as
rp2350_sizesdoes — is wrong here. The S3's ELF carries.ext_ram.dummy(1 114 080 bytes),.flash_rodata_dummy(851 968) and.dram0.dummy: padding and aliases the loader never writes. Summing them counts over a megabyte that is not in the image.So flash is
os.path.getsize(firmware.bin)— not a reconstruction of the artefact but the artefact, the bytes esptool wrote and the app partition receives. That is the same lesson the AVR figure already carries in this script's docstring, where askingavr-sizereplaced adding.textto.dataand being wrong by six bytes.RAM has no such trap once the dummy is skipped:
.dram0.data+.dram0.bssland within two bytes of what PlatformIO prints (52 158 against 52 160), so a percentage with a point of slack is safe.Hence the two flags —
--esp32-binfor flash,--esp32-elffor RAM. Passing one without the other is refused rather than silently skipping the RAM figure.Unmeasurable claims removed rather than left to rot
CI
The existing
firmwarejob runs the check against the image it just built, next to where therp2350andavr-dxjobs already do. The file's header comment said the size check belonged to the AVR job; it now says all three run it.Verified
avr-dx 37/10,esp32 16/33and33 85 57,rp2350 15/16,web 69 11 57 6.7.documented 16 17, measured 16 33and exit 1.--esp32-binalone exits with "…go together".ESP32_SIZEset — it globs PlatformIO'stoolchain-xtensa-esp-elf, which the firmware job installs as part of building.CLAUDE.md's "Sizes in prose" section gains the two new flags and a sentence on why the ESP32 needs two paths, so the next person does not "tidy away" the asymmetry.🤖 Generated with Claude Code