fix(ram): carry TlmPacketizer I16 packetOffset patch (−27.5 KiB RAM Reduction) - #468
Draft
Mikefly123 wants to merge 1 commit into
Draft
fix(ram): carry TlmPacketizer I16 packetOffset patch (−27.5 KiB RAM Reduction)#468Mikefly123 wants to merge 1 commit into
Mikefly123 wants to merge 1 commit into
Conversation
Carry patches/fprime-tlmpacketizer-i16-offset.patch, applied to lib/fprime by `make submodules` with a reverse-apply idempotency check (same pattern as the existing fprime-yamcs patch step). Also exclude patches/ from the trailing-whitespace and end-of-file pre-commit hooks: patch bytes must byte-match the upstream tree or `git apply` breaks. TlmPacketizer::TlmEntry stores FwSignedSizeType packetOffset[MAX_PACKETIZER_PACKETS] per channel — F Prime's 64-bit size-type migration leaked into a persistent dense channels x packets offset matrix. The offsets index into ComBuffers <= 256 B, so I16 is ample (keeps the -1 sentinel, supports offsets to 32 KiB; the range FW_ASSERT in setPacketList is tightened to the I16 max). 3 functional lines. Measured on this deployment (MAX_PACKETIZER_PACKETS=22, MAX_PACKETIZER_CHANNELS=202): - CdhCore::tlmSend bss: 60,480 -> 33,008 B (-27,472 B; reproduced in this build, nm 0x80f0) - malloc arena (CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=-1): 173,272 -> 200,744 B - free heap at boot: 11,504 -> 38,976 B (3.4x; margin context in PR #467) Validation: fprime Svc/TlmPacketizer UT 12/12 pass (1 pre-existing skip); HWIL on v5e — stable boot, 49 channels decode sane, NO_OP and SEND_PKT pass, live sys_heap walk matches predicted numbers exactly. Upstream candidate for nasa/fprime; carried here until accepted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Mikefly123
changed the title
nateinaction
approved these changes
Jul 23, 2026
nateinaction
left a comment
nateinaction
left a comment
Collaborator
There was a problem hiding this comment.
Looks reasonable to me.
Contributor
|
omg tlmpacketizer :D |
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
3 participants
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.
What
Carries
patches/fprime-tlmpacketizer-i16-offset.patchagainstlib/fprime, applied bymake submoduleswith a reverse-apply idempotency check (same pattern as the existing fprime-yamcs patch step). The patch shrinksSvc::TlmPacketizer's per-channelpacketOffsetarray fromFwSignedSizeType(64-bit) toI16— 3 functional lines:TlmEntry::packetOffset[MAX_PACKETIZER_PACKETS]:FwSignedSizeType→I16(TlmPacketizer.hpp)FW_ASSERTinsetPacketListtightened to the I16 max + cast at the single assignment site (TlmPacketizer.cpp)Also excludes
patches/from the trailing-whitespace / end-of-file pre-commit hooks — patch bytes must byte-match the upstream tree orgit applysilently breaks.Why
F Prime's 64-bit size-type migration leaked into a persistent dense channels × packets offset matrix. With this deployment's
MAX_PACKETIZER_CHANNELS=202andMAX_PACKETIZER_PACKETS=22, the offsets alone are 202×22×8 B ≈ 35.5 KB of BSS. Offsets index into ComBuffers ≤ 256 B, so I16 (which preserves the −1 "not in this packet" sentinel and supports packets up to 32 KiB) is ample. Since our malloc arena is the SRAM left over after static allocation (CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=-1) and boot margin was measured at 11,504 B with a hard BufferManager-assert boot-loop past it (background: #467), this flows 1:1 into heap headroom.Measured (v5e reference deployment)
CdhCore::tlmSendbssThe −27,472 B was reproduced in this branch's build (
nm:tlmSend= 0x80f0).Validation
Svc/TlmPacketizerUT: 12/12 pass (1 pre-existing skip)make test-unit: 8/8 pass on this branchmake submodulesapply flow verified idempotent (reverse-apply check on second run)Status
Carried patch, pending upstream acceptance in nasa/fprime — upstreaming proposal tracked in #469.
🤖 Generated with Claude Code