From 4150f80740b22dc637b9d14f7b2d6a03bed6d69c Mon Sep 17 00:00:00 2001 From: Michael Pham <61564344+Mikefly123@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:23:25 -0700 Subject: [PATCH 1/4] Svc::ComAggregator: bound timeout-signal enqueue in FILL state timeout_handler forwards a periodic (10 Hz in our configuration) timeout signal into the component's internal state-machine queue. PR #4402 added an m_allow_timeout guard that suppresses this signal while the state machine is in WAIT_STATUS, preventing queue overflow when a com transmission is slow. That guard does not cover the FILL state (m_allow_timeout == true): if the component's own dispatch thread stalls (e.g. downstream backpressure on a serial/radio link, or thread starvation under system load) for longer than queue_depth / timeout_rate, ticks keep arriving and filling the depth-bounded queue while nothing drains it. Once full, the autocoded signal-send in the state machine (aggregationMachine_sendSignal timeout) hits its FW_ASSERT on QUEUE_FULL, which is fatal in flight configurations. fill and status are each flow-controlled to at most one in-flight message by the com protocol, so timeout is the only unbounded producer feeding this queue. Since timeout ticks are periodic and idempotent (a missed tick is retried on the next cycle), skip forwarding the signal whenever the queue does not have headroom for it plus the (at most one each) in-flight fill/status signals, rather than only checking m_allow_timeout. Observed on hardware (PROVES CubeSat, RP2350/Zephyr, F' v3.1.1) during HWIL soak testing: a stalled dispatch thread (CDC-ACM host-side USB stall) let 10 Hz ticks fill a depth-15 queue in ~1.5 s, triggering the queue-full assert and a hard reboot. Generative AI (Claude, Anthropic) was used to help root-cause this defect during HWIL debugging and to draft this change; disclosed per AI_POLICY.md in the upstream pull request description. --- Svc/ComAggregator/ComAggregator.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Svc/ComAggregator/ComAggregator.cpp b/Svc/ComAggregator/ComAggregator.cpp index c4edf6d8be7..2e313f99600 100644 --- a/Svc/ComAggregator/ComAggregator.cpp +++ b/Svc/ComAggregator/ComAggregator.cpp @@ -8,6 +8,12 @@ namespace Svc { +namespace { +//! Queue slots that must remain free for a timeout signal to be enqueued: the timeout itself plus one +//! in-flight 'fill' and one in-flight 'status' signal (each bounded to one message by the com protocol). +constexpr FwSizeType TIMEOUT_QUEUE_HEADROOM = 3; +} // namespace + // ---------------------------------------------------------------------- // Component construction and destruction // ---------------------------------------------------------------------- @@ -55,7 +61,16 @@ void ComAggregator ::timeout_handler(FwIndexType portNum, U32 context) { // // Behaviorally, this solution will work exactly like the naive implementation with an infinite queue depth, but // prevents queue overflow when using finite queues. - if (this->m_allow_timeout) { + // + // Even so, timeout remains the only signal source without flow control: 'fill' and 'status' are each bounded + // to one in-flight message by the com protocol, but the rate group keeps delivering ticks while this + // component's dispatch thread is stalled (downstream backpressure, thread starvation). In the FILL state + // (m_allow_timeout true) such a stall would still fill the queue with timeout signals and trip the queue-full + // assertion in the autocoded signal send. Ticks are periodic and idempotent, so additionally skip the signal + // unless the queue has headroom for it plus the in-flight flow-controlled signals; a skipped tick is simply + // retried on the next cycle. + if (this->m_allow_timeout && + (this->m_queue.getMessagesAvailable() + TIMEOUT_QUEUE_HEADROOM <= this->m_queue.getDepth())) { this->aggregationMachine_sendSignal_timeout(); } } From 97e1501ae65cf1c498ee1cc9188ce3eb63e42f22 Mon Sep 17 00:00:00 2001 From: Michael Pham <61564344+Mikefly123@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:23:40 -0700 Subject: [PATCH 2/4] Svc::ComAggregator: add UT for FILL-state timeout-flood headroom check The existing test_timeout_overflow_prevention only exercises the WAIT_STATUS-adjacent path: it drives a comStatusIn failure first, so m_allow_timeout is already false during the flood. It does not put the state machine in FILL and stall dispatch, so it does not cover the FILL-state gap closed by the previous commit. Add test_timeout_overflow_prevention_fill_state(), the complement of the existing test: run test_initial() to land in FILL (m_allow_timeout == true) with an empty aggregation buffer and empty queue, then invoke timeout 2 * TEST_INSTANCE_QUEUE_DEPTH times back-to-back with no intervening drain -- simulating a stalled dispatch thread receiving periodic sched ticks. After every invocation, assert the queue never reaches full depth. Then confirm the flood had no side effects (dataOut never fired), confirm recovery via dispatchCurrentMessages once the simulated stall clears, and confirm normal fill/timeout operation resumes cleanly afterward. Note: dispatchOne's underlying doDispatch() issues a blocking Os::Queue::receive with no "queue empty" return path of its own (MSG_DISPATCH_EMPTY is only produced by dispatchCurrentMessages, which snapshots the message count up front). Draining must use dispatchCurrentMessages rather than looping dispatchOne until an empty status, which would hang. Verified: with the bounded-timeout fix applied, all 11 ComAggregator UTs pass (including the new test). With the fix reverted, the new test fails deterministically at the 20th flood iteration (getMessagesAvailable() == TEST_INSTANCE_QUEUE_DEPTH == 20), confirming it exercises the regression this fix addresses. Generative AI (Claude, Anthropic) was used to help draft this test during HWIL follow-up work; disclosed per AI_POLICY.md in the upstream pull request description. --- .../test/ut/ComAggregatorTestMain.cpp | 6 +++ .../test/ut/ComAggregatorTester.cpp | 42 +++++++++++++++++++ .../test/ut/ComAggregatorTester.hpp | 4 ++ 3 files changed, 52 insertions(+) diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp b/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp index 5c118ae0087..44bd5914aca 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTestMain.cpp @@ -63,6 +63,12 @@ TEST(OffNominal, TimeoutOverflowPrevention) { tester.test_timeout(); } +TEST(OffNominal, TimeoutOverflowPreventionFillState) { + Svc::ComAggregatorTester tester; + tester.test_initial(); + tester.test_timeout_overflow_prevention_fill_state(); +} + TEST(Nominal, HoldWhileWaiting) { Svc::ComAggregatorTester tester; tester.test_initial(); diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp b/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp index 98c2055fab5..b41ee15dd82 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTester.cpp @@ -266,6 +266,48 @@ void ComAggregatorTester ::test_timeout_overflow_prevention() { this->clearHistory(); } +//! Tests that a stalled dispatch thread in the FILL state cannot flood the queue with timeout +//! signals past its depth, and that the component recovers cleanly once draining resumes +void ComAggregatorTester ::test_timeout_overflow_prevention_fill_state() { + // Precondition: initial has run. The state machine is now in the FILL state + // (m_allow_timeout == true) with an empty aggregation buffer and an empty queue. Unlike + // test_timeout_overflow_prevention (which drives m_allow_timeout to false via a comStatusIn + // failure before flooding, exercising the WAIT_STATUS-adjacent gate from the pre-existing + // fix), this test floods while m_allow_timeout stays true, exercising the FILL-state gap. + ASSERT_EQ(this->component.m_queue.getMessagesAvailable(), 0); + + // Simulate a stalled dispatch thread: the rate group keeps delivering 'timeout' ticks (this + // port is invoked directly here, exactly as a rate-group member call would) while nothing + // drains the component's message queue via dispatchOne. Loop well past the queue depth -- + // without the FILL-state headroom check, this would keep enqueueing right up to depth and + // trip the autocoded queue-full assertion inside aggregationMachine_sendSignal_timeout. + for (FwSizeType i = 0; i < static_cast(TEST_INSTANCE_QUEUE_DEPTH) * 2; i++) { + this->invoke_to_timeout(0, 0); + // The queue must never be allowed to reach its full depth: headroom is reserved for the + // (at most one each) in-flight 'fill'/'status' signals, so FILL-state flooding cannot trip + // the queue-full assert no matter how long the simulated stall runs. + ASSERT_LT(this->component.m_queue.getMessagesAvailable(), static_cast(TEST_INSTANCE_QUEUE_DEPTH)); + } + // With the aggregation buffer empty, isNotEmpty is false, so every dispatched timeout is a + // no-op transition (see test_timeout_zero); flooding has no side effects of its own. + ASSERT_from_dataOut_SIZE(0); + + // Confirm recovery once the (simulated) dispatch stall clears and draining resumes: every + // queued signal is still processable without error, and the queue returns to empty. + // (dispatchOne's underlying doDispatch() issues a BLOCKING queue receive, so drain exactly the + // snapshotted number of queued messages via dispatchCurrentMessages rather than looping until + // an "empty" status that a blocking dispatch will never produce.) + ASSERT_EQ(this->dispatchCurrentMessages(this->component), + Svc::ComAggregatorComponentBase::MsgDispatchStatus::MSG_DISPATCH_OK); + ASSERT_EQ(this->component.m_queue.getMessagesAvailable(), 0); + ASSERT_from_dataOut_SIZE(0); + this->clearHistory(); + + // Normal fill/timeout operation resumes cleanly after the flood and drain. + this->test_fill_multi(); + this->test_timeout(); +} + void ComAggregatorTester ::test_timeout_zero() { // Precondition: initialize has run this->invoke_to_timeout(0, 0); diff --git a/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp b/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp index 1ea0a1dcc23..856a323f455 100644 --- a/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp +++ b/Svc/ComAggregator/test/ut/ComAggregatorTester.hpp @@ -65,6 +65,10 @@ class ComAggregatorTester final : public ComAggregatorGTestBase { //! Tests timeout operation void test_timeout_overflow_prevention(); + //! Tests that a stalled dispatch thread in the FILL state cannot flood the queue with timeout + //! signals past its depth, and that the component recovers cleanly once draining resumes + void test_timeout_overflow_prevention_fill_state(); + //! Tests timeout operation sends no empty buffer void test_timeout_zero(); From 07ecfbbfc2eaa6803192d0217276c198d06d7231 Mon Sep 17 00:00:00 2001 From: Michael Pham <61564344+Mikefly123@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:23:59 -0700 Subject: [PATCH 3/4] Svc: drop periodic Sched/Ping ticks on queue-full instead of asserting Adds the drop queue-full annotation to two classes of periodic async input ports across Svc, where none currently exists: 1. The Svc.Sched async input on active components that receive a rate- group tick but did not have drop set: CmdSequencer.schedIn, CmdDispatcher.run, TlmChan.Run, TlmPacketizer.Run, FileDownlink.Run, BufferLogger.schedIn, DpManager.schedIn, DpWriter.schedIn. 2. PingIn/pingIn async ports across Svc, including ActiveRateGroup.PingIn and FpySequencer.pingIn (the latter previously priority 10 assert, with an open TODO questioning priority/behavior), plus two ports added upstream since this defect class was first scoped: BufferAccumulator.pingIn and FileDispatcher.pingIn. This is the same defect class fixed for ComAggregator (see companion PR): any active component fed periodic ticks by a rate group can, if its own dispatch thread stalls longer than queue_depth / tick_rate, accumulate enough queued ticks to trip the autocoded FW_ASSERT on queue-full -- turning a transient stall into an unrecoverable FATAL/reboot. Captured on hardware (PROVES CubeSat, RP2350/Zephyr) during HWIL soak testing, in two independently reproduced instances: - CmdSequencer::schedIn_handlerBase (10 Hz sched tick) hit the identical QUEUE_FULL assert as the ComAggregator case, while a sequencer's dispatch thread was stalled. - ActiveRateGroup::PingIn_handlerBase hit the same assert via Svc::Health's 1 Hz ping -- the health-check mechanism itself killed the board. Health's ping-timeout policy exists specifically to detect an unresponsive component and react gracefully; asserting on the ping enqueue short-circuits that design. Both classes of tick (rate-group sched, and ping) are periodic and idempotent by construction -- a dropped tick is simply retried on the next cycle -- so drop is behaviorally safe. Upstream already uses drop for exactly this reason on ComQueue.run and ActiveRateGroup.CycleIn; this extends the same reasoning to the remaining periodic producers that were missed. Related to issue #4195 ("Add a 'Drop But Warn' on Queue Full"), which is in the same design space but currently leans assert-by-default; this change is narrower in scope (periodic/idempotent producers only, per-port opt-in via the existing drop annotation, no new mechanism). This is an fpp-only annotation change consumed by the autocoder; no new C++ logic. A reviewer sweep of existing per-component UTs that assert on QUEUE_FULL behavior for these ports is recommended before merge. Generative AI (Claude, Anthropic) was used to help root-cause this defect class during HWIL debugging (two independently captured hardware instances of the same assert) and to enumerate affected ports; disclosed per AI_POLICY.md in the upstream pull request description. --- Svc/ActiveRateGroup/ActiveRateGroup.fpp | 2 +- Svc/BufferLogger/BufferLogger.fpp | 4 ++-- Svc/CmdDispatcher/CmdDispatcher.fpp | 4 ++-- Svc/CmdSequencer/CmdSequencer.fpp | 4 ++-- Svc/ComLogger/ComLogger.fpp | 2 +- Svc/DpCatalog/DpCatalog.fpp | 2 +- Svc/DpManager/DpManager.fpp | 2 +- Svc/DpWriter/DpWriter.fpp | 2 +- Svc/EventManager/EventManager.fpp | 2 +- Svc/FileDownlink/FileDownlink.fpp | 4 ++-- Svc/FileManager/FileManager.fpp | 2 +- Svc/FileUplink/FileUplink.fpp | 2 +- Svc/FpySequencer/FpySequencer.fpp | 2 +- Svc/PrmDb/PrmDb.fpp | 2 +- Svc/TlmChan/TlmChan.fpp | 4 ++-- Svc/TlmPacketizer/TlmPacketizer.fpp | 4 ++-- 16 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Svc/ActiveRateGroup/ActiveRateGroup.fpp b/Svc/ActiveRateGroup/ActiveRateGroup.fpp index 3ee1488e375..c60ad6d0d1e 100644 --- a/Svc/ActiveRateGroup/ActiveRateGroup.fpp +++ b/Svc/ActiveRateGroup/ActiveRateGroup.fpp @@ -15,7 +15,7 @@ module Svc { output port RateGroupMemberOut: [ActiveRateGroupOutputPorts] Sched @ Ping input port for health - async input port PingIn: Ping + async input port PingIn: Ping drop @ Ping output port for health output port PingOut: Ping diff --git a/Svc/BufferLogger/BufferLogger.fpp b/Svc/BufferLogger/BufferLogger.fpp index 6da2ffd6930..e045b185625 100644 --- a/Svc/BufferLogger/BufferLogger.fpp +++ b/Svc/BufferLogger/BufferLogger.fpp @@ -16,12 +16,12 @@ module Svc { async input port comIn: Fw.Com @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping - async input port schedIn: Svc.Sched + async input port schedIn: Svc.Sched drop # ---------------------------------------------------------------------- # Special ports diff --git a/Svc/CmdDispatcher/CmdDispatcher.fpp b/Svc/CmdDispatcher/CmdDispatcher.fpp index 66343b886e8..2f77328966e 100644 --- a/Svc/CmdDispatcher/CmdDispatcher.fpp +++ b/Svc/CmdDispatcher/CmdDispatcher.fpp @@ -24,10 +24,10 @@ module Svc { async input port seqCmdBuff: [CmdDispatcherSequencePorts] Fw.Com hook @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Run port used to emit telemetry - async input port run: Svc.Sched + async input port run: Svc.Sched drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/CmdSequencer/CmdSequencer.fpp b/Svc/CmdSequencer/CmdSequencer.fpp index f1c4012784f..ebc38165324 100644 --- a/Svc/CmdSequencer/CmdSequencer.fpp +++ b/Svc/CmdSequencer/CmdSequencer.fpp @@ -68,7 +68,7 @@ module Svc { async input port cmdResponseIn: Fw.CmdResponse @ Ping in port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping out port output port pingOut: Svc.Ping @@ -86,7 +86,7 @@ module Svc { output port comCmdOut: Fw.Com @ Schedule in port - async input port schedIn: Svc.Sched + async input port schedIn: Svc.Sched drop @ Notifies that a sequence has started running output port seqStartOut: Svc.CmdSeqIn diff --git a/Svc/ComLogger/ComLogger.fpp b/Svc/ComLogger/ComLogger.fpp index c4327b7ddb7..cf4bc223871 100644 --- a/Svc/ComLogger/ComLogger.fpp +++ b/Svc/ComLogger/ComLogger.fpp @@ -11,7 +11,7 @@ module Svc { async input port comIn: Fw.Com @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/DpCatalog/DpCatalog.fpp b/Svc/DpCatalog/DpCatalog.fpp index e1fd468dd2a..59ffc363f75 100644 --- a/Svc/DpCatalog/DpCatalog.fpp +++ b/Svc/DpCatalog/DpCatalog.fpp @@ -32,7 +32,7 @@ module Svc { # Component specific ports @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/DpManager/DpManager.fpp b/Svc/DpManager/DpManager.fpp index ebbf5a46fb4..74574f97f36 100644 --- a/Svc/DpManager/DpManager.fpp +++ b/Svc/DpManager/DpManager.fpp @@ -8,7 +8,7 @@ module Svc { # ---------------------------------------------------------------------- @ Schedule in port - async input port schedIn: Svc.Sched + async input port schedIn: Svc.Sched drop # ---------------------------------------------------------------------- # Ports for handling buffer requests diff --git a/Svc/DpWriter/DpWriter.fpp b/Svc/DpWriter/DpWriter.fpp index f24594f0925..152be91baf4 100644 --- a/Svc/DpWriter/DpWriter.fpp +++ b/Svc/DpWriter/DpWriter.fpp @@ -8,7 +8,7 @@ module Svc { # ---------------------------------------------------------------------- @ Schedule in port - async input port schedIn: Svc.Sched + async input port schedIn: Svc.Sched drop # ---------------------------------------------------------------------- # Ports for handling data products diff --git a/Svc/EventManager/EventManager.fpp b/Svc/EventManager/EventManager.fpp index b97a8d807e3..2ea88960ae9 100644 --- a/Svc/EventManager/EventManager.fpp +++ b/Svc/EventManager/EventManager.fpp @@ -53,7 +53,7 @@ module Svc { output port FatalAnnounce: Svc.FatalEvent @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/FileDownlink/FileDownlink.fpp b/Svc/FileDownlink/FileDownlink.fpp index eed9376aa41..5e710a3f41f 100644 --- a/Svc/FileDownlink/FileDownlink.fpp +++ b/Svc/FileDownlink/FileDownlink.fpp @@ -8,7 +8,7 @@ module Svc { # ---------------------------------------------------------------------- @ Run input port - async input port Run: Svc.Sched + async input port Run: Svc.Sched drop @ Mutexed Sendfile input port guarded input port SendFile: Svc.SendFileRequest @@ -23,7 +23,7 @@ module Svc { output port bufferSendOut: Fw.BufferSend @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/FileManager/FileManager.fpp b/Svc/FileManager/FileManager.fpp index bef777c7133..d85f42002f7 100644 --- a/Svc/FileManager/FileManager.fpp +++ b/Svc/FileManager/FileManager.fpp @@ -8,7 +8,7 @@ module Svc { # ---------------------------------------------------------------------- @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Scheduler input port for rate group operations sync input port schedIn: Sched diff --git a/Svc/FileUplink/FileUplink.fpp b/Svc/FileUplink/FileUplink.fpp index 90ef4e9a821..b2b67bd5c39 100644 --- a/Svc/FileUplink/FileUplink.fpp +++ b/Svc/FileUplink/FileUplink.fpp @@ -14,7 +14,7 @@ module Svc { output port bufferSendOut: Fw.BufferSend @ Ping in - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping out output port pingOut: Svc.Ping diff --git a/Svc/FpySequencer/FpySequencer.fpp b/Svc/FpySequencer/FpySequencer.fpp index 8af40d3d7ce..b9e80e1af03 100644 --- a/Svc/FpySequencer/FpySequencer.fpp +++ b/Svc/FpySequencer/FpySequencer.fpp @@ -38,7 +38,7 @@ module Svc { @ Ping in port # TODO should ping have highest prio? or lowest? - async input port pingIn: Svc.Ping priority 10 assert + async input port pingIn: Svc.Ping priority 10 drop @ port to trigger a wakeup or timeout check. increase frequency @ to increase temporal resolution of sequencer diff --git a/Svc/PrmDb/PrmDb.fpp b/Svc/PrmDb/PrmDb.fpp index bca4f172faa..b2a4b95b1e1 100644 --- a/Svc/PrmDb/PrmDb.fpp +++ b/Svc/PrmDb/PrmDb.fpp @@ -69,7 +69,7 @@ module Svc { async input port setPrm: Fw.PrmSet @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/TlmChan/TlmChan.fpp b/Svc/TlmChan/TlmChan.fpp index d15072ed08a..52b73b2d818 100644 --- a/Svc/TlmChan/TlmChan.fpp +++ b/Svc/TlmChan/TlmChan.fpp @@ -10,13 +10,13 @@ module Svc { guarded input port TlmGet: Fw.TlmGet @ Run port for starting packet send cycle - async input port Run: Svc.Sched + async input port Run: Svc.Sched drop @ Packet send port output port PktSend: Fw.Com @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping diff --git a/Svc/TlmPacketizer/TlmPacketizer.fpp b/Svc/TlmPacketizer/TlmPacketizer.fpp index 1655c3e2eb7..4125fdac9ca 100644 --- a/Svc/TlmPacketizer/TlmPacketizer.fpp +++ b/Svc/TlmPacketizer/TlmPacketizer.fpp @@ -28,13 +28,13 @@ module Svc { async input port controlIn: EnableSection @ Ping input port - async input port pingIn: Svc.Ping + async input port pingIn: Svc.Ping drop @ Ping output port output port pingOut: Svc.Ping @ Run port for starting packet send cycle - async input port Run: Svc.Sched + async input port Run: Svc.Sched drop @ Input configuration port async input port configureSectionGroupRate: ConfigureGroupRate From 6a26dce68d160b7feffa42d6aea3e8c39d2a155e Mon Sep 17 00:00:00 2001 From: Michael Pham <61564344+Mikefly123@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:43:14 -0700 Subject: [PATCH 4/4] Svc::TlmPacketizer: shrink per-channel packetOffset storage from FwSignedSizeType to I16 TlmEntry stores FwSignedSizeType packetOffset[MAX_PACKETIZER_PACKETS] per channel. The value is a byte offset into a ComBuffer-sized packet (bounded by FW_COM_BUFFER_MAX_SIZE, typically a few hundred bytes) or the -1 'not in this packet' sentinel -- a 64-bit signed type per slot is 4-8x oversized. Shrink to I16 (keeps the -1 sentinel; supports packet offsets to 32 KB) with a config-time FW_ASSERT range guard at the single assignment site in setPacketList. Validated on a real CubeSat deployment (MAX_PACKETIZER_CHANNELS=202, MAX_PACKETIZER_PACKETS=22, RP2350/Zephyr, v5e flight board): - -27,472 B BSS (60,480 -> 33,008 B, -45%) in CdhCore::tlmSend - Svc/TlmPacketizer UTs: 12/12 passed (1 pre-existing skip) - HWIL-verified: clean boot, NO_OP round-trip, packetized telemetry decodes bit-correct, SEND_PKT works; live heap walk matches nm prediction exactly (free-at-boot 11,504 -> 38,976 B) Tracked by Open-Source-Space-Foundation/proves-core-reference#469. --- Svc/TlmPacketizer/TlmPacketizer.cpp | 6 +++--- Svc/TlmPacketizer/TlmPacketizer.hpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Svc/TlmPacketizer/TlmPacketizer.cpp b/Svc/TlmPacketizer/TlmPacketizer.cpp index 89af22e1d01..ba1c685558c 100644 --- a/Svc/TlmPacketizer/TlmPacketizer.cpp +++ b/Svc/TlmPacketizer/TlmPacketizer.cpp @@ -89,10 +89,10 @@ void TlmPacketizer::setPacketList(const TlmPacketizerPacketList& packetList, entry.ignored = false; entry.channelSize = packetList.list[pktEntry]->list[tlmEntry].size; // the offset into the buffer will be the current packet length - // the offset must fit within FwSignedSizeType to allow for negative values - FW_ASSERT(packetLen <= static_cast(std::numeric_limits::max()), + // the offset must fit within I16 to allow for the -1 sentinel value + FW_ASSERT(packetLen <= static_cast(std::numeric_limits::max()), static_cast(packetLen)); - entry.packetOffset[pktEntry] = static_cast(packetLen); + entry.packetOffset[pktEntry] = static_cast(packetLen); packetLen += entry.channelSize; diff --git a/Svc/TlmPacketizer/TlmPacketizer.hpp b/Svc/TlmPacketizer/TlmPacketizer.hpp index 7f80d76d265..ffb58fc5d17 100644 --- a/Svc/TlmPacketizer/TlmPacketizer.hpp +++ b/Svc/TlmPacketizer/TlmPacketizer.hpp @@ -172,7 +172,7 @@ class TlmPacketizer final : public TlmPacketizerComponentBase, public Fw::ParamE FwChanIdType id; //!< telemetry id stored in slot // Offsets into packet buffers. // -1 means that channel is not in that packet - FwSignedSizeType packetOffset[MAX_PACKETIZER_PACKETS]; + I16 packetOffset[MAX_PACKETIZER_PACKETS]; FwSizeType channelSize; //!< max serialized size of the channel in bytes bool ignored; //!< ignored channel id bool hasValue; //!< if the entry has received a value at least once