[ZabaHD4K]

Related Issue(s)	#5881
Has Unit Tests (y/n)	y
Documentation Included (y/n)	y
Generative AI was used in this contribution (y/n)	y

---

Change Description

Implements EPP idle fill in Svc::Ccsds::AosFramer, following the direction @Willmac16 gave on #6053 ("why not write proper EPP Idle Serialization") rather than rejecting the configuration:

• New serialize_idle_epp_packet() builds an Encapsulation Idle Packet (CCSDS 133.1-B-3, Protocol ID idle) of exactly the remainder size, with the smallest header that can carry it: the one-octet packet (Length of Length Zero) for a 1-byte remainder, a two-octet header (One) up to 255 bytes, and a four-octet header (Two, with a zero UDF/Protocol ID Extension octet) above that. Packet Length fields hold the header-inclusive total, matching what AosDeframer::sizeEppPacket decodes. The eight-octet header is never needed since the M_PDU payload is bounded to U16 by the existing assert in fill_with_idle_packet.
• fill_with_idle_packet() now prefers EPP whenever EPP_MASK is permitted: since the minimum EPP is one octet, the fill always completes the current frame in place and never stripes onto the next frame the way the minimum-size (7-byte) SPP fill may have to. SPP-only configurations keep the existing behavior. This replaces the empty branch that consumed the condition and left sendNow frames unpadded and withheld (the bug in Svc/Ccsds/AosFramer: EPP idle-fill branch swallows the condition and leaves frames unpadded #5881).
• configure() now accepts any combination of SPP/EPP with at least one bit set and no invalid bits (PvnBitfield::VALID_MASK), instead of requiring SPP.
• SDD: idle-fill protocol selection documented.

Unit tests:

• testEppIdleFill (EPP-only, now a legal configuration): remainders that exercise all three header shapes, including the 1-byte remainder SPP cannot express without striping; checks header bytes, header-inclusive length fields, idle pattern fill, and that the sendNow frame is actually emitted.
• testEppPreferredOverSpp: the exact repro from Svc/Ccsds/AosFramer: EPP idle-fill branch swallows the condition and leaves frames unpadded #5881 (SPP_MASK | EPP_MASK), which previously emitted no frame.
• testIdlePvnsValidation: no-protocol and invalid-bit configurations assert; EPP-only configures cleanly.

Rationale

Fixes #5881 (CCB: Approved). In fill_with_idle_packet the EPP branch was an empty TODO that still consumed the if condition, so any configuration with EPP_MASK set skipped the SPP fill chain entirely: the partially-filled frame was never padded, check_and_send_vc never saw it full, and a sendNow frame was silently withheld until unrelated later data arrived.

Testing/Review Recommendations

fprime-util check in Svc/Ccsds/AosFramer passes on Linux x86_64 with the sanitizers enabled by the UT build.

Review focus: the header-shape selection boundaries (1 / ≤255 / >255) and the header-inclusive length convention — both were written against AosDeframer::sizeEppPacket, which treats a Protocol ID of idle as end-of-frame, so any of these fills terminates extraction as expected on the receive side.

Future Work

#5888 (duplicate SUCCESS on comStatusOut during tail-packing) is in the same component and tracked separately.

Contributor Checklist

• I have the rights to contribute this change set under the Apache-2.0 license.
• I agree to be bound by the contributor guidelines.

AI Usage (see policy)

An AI coding assistant was used, under my direction, to draft the implementation, the unit tests, and the SDD update. All changes were reviewed by me and verified by building and running the full Svc/Ccsds/AosFramer unit test suite locally.

• This contribution follows the F´ AI development skills (entered through AGENTS.md).

[lestarch-autobot]

Automated review summary (run 1)

Per-agent results

Findings open at the reviewed head, by tag; everything ever raised is under Since last run.

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	0	0	0	Go
F Prime C/C++ Design	0	1	2	0	3	Go
Documentation Currency	0	0	0	0	0	Go
Design	0	1	0	0	1	Go
Architecture	0	0	0	0	0	Go
Test Quality	0	1	1	0	2	Go
Correctness	0	0	0	0	0	Go
Operational	0	1	0	0	1	Go
Maintainability	1	0	2	0	3	No-Go
CI safety	—	—	—	—	—	Go
Totals (consolidated)	1	3	5	0	9	No-Go

The Operational row counts its concurrence on the Design thread at docs/sdd.md:22; Totals count that shared thread once.

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	clean
Review-system integrity	clean

Outstanding must-fix items (1)

Maintainability

• checkEppIdleFilledFrame is void but its declaration comment says it "returns the frame for further checks"; the helper hands the frame back to the component via dataReturnIn (AosFramerTester.hpp:63) — #6333 (comment)

Severity reconciliation (0 promoted)

Finding	Reviewer tag	Summary tag	Consequence	Link
EPP header-shape boundary (255/256-byte remainder) not exercised by testEppIdleFill	suggestion	suggestion (not promoted)	test-coverage gap; no §14 consequence stated	#6333 (comment)
EPP-only configure positive case asserts nothing observable	could fix	could fix (not promoted)	no-crash test; no §14 consequence stated	#6333 (comment)
SDD omits that the peer AosDeframer must enable EPP in pvnMask to accept EPP idle fill (Design, concurred by Operational)	suggestion	suggestion (not promoted)	rationale calls it a documentation gap, not a sentence the PR made false; unthrottled DisabledPvn warning is a configuration-pairing hazard, not a ground-driven assert	#6333 (comment)
Code comment cites AosDeframer::sizeEppPacket as decoding the idle packet's length field, which it never reads	could fix	could fix (not promoted)	misleading inline comment; §14's false-claim row covers SDD / user manual / Doxygen surfaces, not a .cpp body comment	#6333 (comment)
Idle-pattern fill loop duplicated between serialize_idle_spp_packet and serialize_idle_epp_packet	could fix	could fix (not promoted)	duplication / fragile-edit hazard; no §14 consequence stated	#6333 (comment)
CPP-19: status / headerLength declared without initializers in serialize_idle_epp_packet	suggestion	suggestion (not promoted)	rationale states every branch assigns before use — no uninitialized read today	#6333 (comment)
CPP-30: 0x7 EPP PVN literal where ComCfg::Pvn::ENCAPSULATION_PACKET_PROTOCOL exists	could fix	could fix (not promoted)	named-constant hygiene; no §14 consequence stated	#6333 (comment)
CPP-33: EPP idle encoder inlined in AosFramer instead of Svc/Ccsds/Utils/IdlePacket	could fix	could fix (not promoted)	missed-reuse / placement; no §14 consequence stated	#6333 (comment)

Merge readiness

Merge readiness: No-Go — Maintainability has 1 outstanding must-fix (misleading doc comment on checkEppIdleFilledFrame, AosFramerTester.hpp:63); all other lenses report Go and CI safety is Go.

---

One stale comment between this idle fill and a clean downlink — tidy it and the frames are go for launch.

[ZabaHD4K]

All run-1 findings addressed in 1e82343:

• [Maintainability, must fix] helper comment claiming a return value → reworded: the helper checks the frame and returns it to the component via dataReturnIn.
• [C++ Design, CPP-33] EPP encoder inlined in the component → moved to Svc/Ccsds/Utils/IdlePacket::serializeEpp(), beside the shared SPP idle serializer, in the same status-return style; AosFramer asserts on the returned status. This also removes the duplicated fill loop and the duplicate EPP_IDLE_DATA_PATTERN [Maintainability, could fix ×2].
• [C++ Design, CPP-30] 0x7 literal → ComCfg::Pvn::ENCAPSULATION_PACKET_PROTOCOL, in the encoder and the tester.
• [C++ Design, CPP-19] status/headerLength now carry explicit initializers in the moved encoder.
• [Maintainability, could fix] comment implying AosDeframer::sizeEppPacket decodes the idle Packet Length → corrected (an Idle Protocol ID marks the last packet in the frame); the tester comment now cites the standard only.
• [Design + Operational, sdd.md:22] receive-side pairing constraint documented: permitting EPP requires every receiver of the VC to accept PVN 7 (an SPP-only AosDeframer raises DisabledPvn WARNING_HI per idle-filled frame); guidance added to keep the SPP_MASK default otherwise.
• [Test Quality, suggestion] both sides of the 255/256 header-shape boundary now exercised (maxPayload - 255 / - 256).
• [Test Quality, could fix] the accepting configure path now asserts the stored idle_packet_types bitfield.

fprime-util check passes locally after the changes (10/10, sanitizers on).

[lestarch-autobot]

[Maintainability] could fix maint-unclear-naming: EPP_MIN_SIZE / EPP_MAX_SIZE sit beside the unprefixed MIN_SIZE / MAX_SIZE / serialize, which are now the SPP-only variants.

With the namespace documented as "SPP and EPP", the bare names read as the namespace-wide bounds, but MIN_SIZE is 7 while a valid EPP fill is 1 byte: a caller that guards serializeEpp with size >= MIN_SIZE compiles and silently rejects 1-6 byte remainders. Either prefix the SPP trio (SPP_MIN_SIZE, SPP_MAX_SIZE, serializeSpp; ripples to existing callers) or say "SPP" in the three existing doc comments so the two families are distinguishable.

Superseded by re-review run 2.

[lestarch-autobot]

Automated review summary (run 2)

Per-agent results

Findings open at the reviewed head, by tag; everything ever raised is under Since last run.

Agent	must fix	suggestion	could fix	future work	outstanding	Verdict
Security Vulnerabilities	0	0	0	0	0	Go
Supply Chain / Runner Safety	0	0	0	0	0	Go
F Prime C/C++ Design	0	0	0	0	0	Go
Documentation Currency	0	0	0	0	0	Go
Design	0	0	0	0	0	Go
Architecture	0	0	0	0	0	Go
Test Quality	0	0	0	0	0	Go
Correctness	0	0	0	0	0	Go
Operational	0	0	0	0	0	Go
Maintainability	0	0	1	0	1	Go
CI safety	—	—	—	—	—	Go
Totals	0	0	1	0	1	Go

Since last run

Agent	raised (all runs)	resolved	still open	newly added	incorrect-fix follow-ups	improperly resolved	disagreements escalated
Security Vulnerabilities	0	0	0	0	0	0	0
Supply Chain / Runner Safety	0	0	0	0	0	0	0
F Prime C/C++ Design	3	3	0	0	0	0	0
Documentation Currency	0	0	0	0	0	0	0
Design	1	1	0	0	0	0	0
Architecture	0	0	0	0	0	0	0
Test Quality	2	2	0	0	0	0	0
Correctness	0	0	0	0	0	0	0
Operational	1	1	0	0	0	0	0
Maintainability	4	3	0	1	0	0	0

Duplicates consolidated this run: 0 (threads closed by the §5h post-pass)

Supply-chain surfaces

Surface	Outstanding
Dependencies	clean
Vendored / submodule	clean
Build / test infrastructure	clean
Workflows / actions / scripts	clean
Generator output	clean
Prompt-injection	clean
Review-system integrity	clean

Severity reconciliation (0 promoted)

Finding	Reviewer tag	Summary tag	Consequence	Link
EPP_MIN_SIZE / EPP_MAX_SIZE sit beside the unprefixed SPP-only MIN_SIZE / MAX_SIZE / serialize in IdlePacket.hpp	could fix	could fix (not promoted)	naming-ambiguity hazard for a hypothetical future caller ("a caller that guards … compiles and silently rejects"); no existing false claim, assert, or failed-operation status is stated	#6333 (comment)

Merge readiness

Merge readiness: Go — all ten lenses completed at 1e82343 with Verdict Go; the single run-1 must-fix (AosFramerTester.hpp:63 comment) and the eight other run-1 threads are resolved, leaving one Maintainability could-fix open. CI safety is Go. Core maintainers @LeStarch and @thomas-bc have been requested for human review.

---

Every thread settled and the idle fill in place — this frame is cleared for the maintainers' pass.

[ZabaHD4K]

Converted to draft per the maintainer's request on #5881 to work one PR at a time — this one resumes once #6334 merges. All run-1 findings remain addressed (run 2: Go).