docs: expand RAM/flash optimization guide, rewrite as rule-first reference - #11827
Open
sensei-hacker wants to merge 12 commits into
Open
docs: expand RAM/flash optimization guide, rewrite as rule-first reference#11827sensei-hacker wants to merge 12 commits into
sensei-hacker wants to merge 12 commits into
Conversation
…nique 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.
Adds four techniques from shrink-ledstrip-dma-buffer: chunking a one-shot DMA buffer into a small circular buffer with a refill interrupt, exploiting minimum-vs-maximum protocol timing constraints to avoid dedicating buffer space to them, bounding work/transfers to the runtime-configured count instead of the theoretical maximum, and verifying DMA element width against the destination register on real hardware rather than by inspection alone.
…ss boards The AOCODARCH7DUAL white-LED regression is a second-order instance of this doc's own register-width lesson: a uint16_t element size validated on one board's CCR width still routed DMA configuration through a different, less-tested code path on H7, exposing an unrelated typo. Capture the narrower point separately since it's about generalizing a verified fix, not about the original verification step.
…ansition) review Pure-inline-logic-header pattern, table-vs-if/else-chain confirmation, bumpless-handoff full-state-duplication explanation, and a review methodology note on CI's RAM delta spanning multiple linker regions (RAM+CCM on F4/F7/H7).
…ion; condense doc for concision Adds three new patterns found while tracing programmingFrameworkUpdateTask's +5,168B growth: LTO merges unrelated functions into one symbol so a size-diff can misattribute growth to the wrong nearby change; forcing NOINLINE at both compare points isolates one function's real standalone cost inside such a blob (NOINLINE is a no-op on F4 - USE_ITCM_RAM only); and bisecting a long-lived branch should diff each commit against its own parent, not a fixed base, since resync history isn't one consistent timeline. Also adds a pattern distinguishing avoidable code duplication (PID controller functions not yet parameterized) from necessary state duplication (the existing bumpless-handoff entry). Condensed every existing entry to be substantially shorter, matching the terse header-plus-example style of the buffer-concatenation/inlining entries, so the doc stays a fast reference rather than a narrative.
Internal-linkage definitions in headers are duplicated per translation unit; the MAVLink helper CRC table was the concrete 8,088 B case on BLUEBERRYF405. Prefer extern + single .c definition, and check for a vendor's documented switch (MAVLINK_SEPARATE_HELPERS) before patching generated files.
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 |
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+41
to
+43
| Example: a first pass on the tunnel buffer checked only MSP2 handlers | ||
| (432 B largest reply); MSP1 legacy handlers reachable through the same | ||
| buffer needed 512 B (`MSP_LED_STRIP_CONFIG`, unbounded). |
There was a problem hiding this comment.
2. Fixed reply called unbounded 🐞 Bug ≡ Correctness
MSP_LED_STRIP_CONFIG is not unbounded: it always writes one 32-bit value for each of the compile-time-bounded 128 LED slots, yielding the cited 512-byte reply. Calling it unbounded obscures the actual sizing rule and can lead readers to conclude that no finite tunnel buffer is safe.
Agent Prompt
## Issue description
The guide incorrectly labels `MSP_LED_STRIP_CONFIG` as unbounded even though its output is fixed at 512 bytes.
## Issue Context
The handler loops over the compile-time `LED_MAX_STRIP_LENGTH` of 128 and emits one four-byte configuration value per slot.
## Fix Focus Areas
- docs/development/ram-and-flash-optimization.md[41-43]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
PR Summary by QodoIntegrate multi-protocol I/O, output assignment, and platform updates
AI Description
Diagram
High-Level Assessment
Files changed (144)
|
…m-flash-optimization-seed # Conflicts: # docs/development/ram-and-flash-optimization.md
sensei-hacker
changed the title
The WS2812 DMA buffer element is timerDMASafeType_t (uint32_t) on every F4/F7/H7/AT32 family since 2019 (4427b87), so the full 3,115-element buffer is 12,460 B and the 2x4-LED circular design is 768 B, not the 2-byte-element figures previously stated (6,230 B / 384 B). The >16x reduction ratio is unchanged.
…review Three new generalizable rules from the terrain_nav layer review: - budget speculative cache reads against the shared cache's size (lookahead capped at (CACHE_SIZE - 3) * 540 m plus a time horizon) - consume an existing cache in place, never a private copy - keep a feature's whole static state in one caller-owned struct (the entire hold state machine is ~100 B of static RAM)
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.
Summary
Expand
docs/development/ram-and-flash-optimization.mdfrom the seed stubinto a rule-first guide to minimizing flash and RAM usage on
resource-constrained targets.
Changes
example. All techniques and concrete numbers are preserved (buffer-sizing
audits, circular-buffer chunking, minimum-vs-maximum timing constraints,
enum dispatch tables, state-vs-code duplication, LTO measurement, DMA
width pitfalls, static-linkage duplication).
linkage into one decision rule: be wary of
staticin a header(private copy per including TU, unmergeable by the linker — prefer
extern+ a single.cdefinition), with the exception that pure,stateless
static inlinefunctions are safe while the compiler inlinesor discards them. The exception also states explicitly that inline
multiplies copies with multiple callers (per call site, and per TU when
not inlined).
functions/linkage/headers, state machines, duplication, measuring size,
hardware width pitfalls.
Conflict resolution
This branch diverged from
maintenance-10.xbefore the seed stub(
27e6322c2c) was added there, so both sides had independently added thefile. Merged
upstream/maintenance-10.xin and kept this branch's fullguide — it supersedes the seed (same techniques, same numbers). The only
conflicted file was the doc itself; the branch touches nothing else.
Testing
Related
release/9.1as PR docs: expand RAM/flash optimization guide, rewrite as rule-first reference #11828.document-ram-flash-optimization-practiceswill linkthis guide from
Development.mdand extend review checklists.