KSat/fsfw

forked from fsfw/fsfw

WIP: cfdp: implement acknowledged mode (class 2) #73

Draft

tbaumgartl wants to merge 8 commits from baumgartl/cfdp-class-2 into main

Author	SHA1	Message	Date
Tobias BaumgartlandClaude Opus 5	6faea2b0f0	cfdp: report a metadata only transaction as a complete delivery A metadata only transaction - a proxy put request, for instance - carries no file data and completes the moment its metadata arrives. handleTransferCompletion took the branch for a null checksum, which sets the condition code and touches neither delivery field, so both were reported at their reset defaults: a transaction that succeeded announced itself as Finish Condition: No Error (0) File delivery code: Data Incomplete (1) File delivery status: Discard deliberately (0) which contradicts itself, and goes out in the Finished PDU to the sender, not only into the OBSW log. It was noticed on the flatsat, where every CFDP downlink begins with exactly this kind of transaction carrying the proxy put request, and each one reported a failed delivery on the console. Nothing was expected of it and nothing is missing, so the delivery code is DATA_COMPLETE; there is no file whose status could be reported, so the status is FILE_STATUS_UNREPORTED. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4eBFnanCACYWdKCzhHMcC	2026-09-16 16:40:12 +02:00
Tobias BaumgartlandClaude Opus 5	844faf850f	Stop arrayprinter from sizing its stack buffer from the input printHex and printDec built a variable length array of (size + 1) * 7 + 1 and size * 4 + 1 + lines bytes respectively, on the stack, from the caller's buffer size. Nothing bounded that against the stack it ran on. This reset an iOBC on the flatsat. A CFDP uplink driven without inter packet spacing filled the USLP receive buffer with about 12 KB in one 300 ms cycle, a frame parse error asked for the serial stream to be dumped, and printHex tried to place an 84 KB array on an 8 KB task stack. FreeRTOS caught it as STACK OVERFLOW DETECTED in USLP_RX and restarted the OBC. The trigger needs a parse error, so it hid for as long as the link stayed clean: USLP_RX sat at 800 bytes of its 8 KB, and the first corrupted frame with a full receive buffer behind it was fatal. Emit the output in fixed 128 byte chunks instead, flushing as it is built. The rendered text is unchanged - verified byte for byte against the previous implementation for sizes 0 to 4096 and several line widths, including the line break boundaries. printBin was already safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4eBFnanCACYWdKCzhHMcC	2026-09-16 12:02:57 +02:00
Tobias BaumgartlandClaude Opus 5	e256fa4b92	cfdp: parse and act on a Cancel EOF at the destination handleEofPdu built its EofInfo with a null fault location TLV pointer. EofPduReader refuses to parse any EOF whose condition code is not NO_ERROR unless it has somewhere to put that TLV, so every Cancel EOF a sender emits was rejected with "Ca not deserialize fault location" and dropped before the handler saw it. The consequences were invisible from the ground until now: the sender's cancellation was never acknowledged, so it retransmitted the EOF to its positive ACK limit and declared a fault, while this handler kept the transaction open until its own check limit expired. A flatsat uplink hit exactly that, twice ten seconds apart, which is the sender's retransmission interval. Give both EOF parse sites a real EntityIdTlv, held by the handler so no allocation happens per PDU, and adopt a non-NO_ERROR condition code into the transaction. Without the second part the cancellation would parse but then fall through to transfer completion, which would run a checksum pass over a file the sender has already abandoned and report a checksum failure instead of the cancellation. The new test fails without the fix in the same way the flatsat did: no ACK is emitted at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4eBFnanCACYWdKCzhHMcC	2026-09-16 11:54:34 +02:00
Tobias BaumgartlandClaude Opus 5	9d609b6d2d	cfdp: exclude the PDU CRC from variable length PDU tails FileDataReader takes the two CRC bytes out of the parsed range before it reads its payload. The three readers with a variable length tail did not, and each of them walks that tail until the range is exhausted: MetadataPduReader over its option TLVs, NakPduReader over its segment requests, FinishPduReader over its filestore and fault location TLVs. With crcOnTransmission set at the sender, the CRC is therefore parsed as one more TLV or segment request and the PDU is rejected, metadata and Finished with INVALID_TLV_TYPE. Reception of CRC bearing PDUs only ever worked for the PDUs which have no tail at all, which is why it went unnoticed: a plain file uplink's metadata carries no options. A proxy put request always carries one, and in acknowledged mode so do the NAK and Finished PDUs a ground source sends, so this broke the OBSW as a destination for any request carrying a message to user, and as a source for every acknowledged downlink. MetadataPduReader had a partial guard for this - an early return when the CRC was the only thing left - which covered the no-options case and hid the defect for the case that has them. It is replaced by the same up front subtraction the other two now do. The tests build CRC bearing PDUs by hand through the new PduCrcHelper, because the creators cannot produce one: they append the CRC without counting it in the directive data field length, which is a separate defect left alone here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4eBFnanCACYWdKCzhHMcC	2026-09-16 10:38:08 +02:00
Tobias BaumgartlandClaude Opus 5	892fdff164	cfdp: apply clang-format to the class 2 handler changes origin/main is clang-format clean, this branch was not: the class 2 work left six violations across four files. Purely mechanical reflowing, no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZpKfWUzvTkBMXEv9NEoTM	2026-09-15 01:08:30 +02:00
Tobias BaumgartlandClaude Opus 5	30965ed058	cfdp: fix acknowledged mode error handling and PDU transaction filtering Review of the class 2 implementation turned up seven defects, all in the paths that only run when something has already gone wrong on the link. Send failures were still being treated as successful sends in two places the earlier fix missed. SourceHandler::servicePendingRetransmissions() advanced its cursor past a segment whose sendFileDataPdu() failed and cleared metadataPending before the metadata PDU had gone out, so a NAK answered while the TM store was full dropped exactly the data the peer asked for. Both now only advance on success, matching the forward-only path. A NAK carrying more segment requests than the reader's array can hold was a complete no-op rather than a partial one: NakPduReader::parseData() returns NAK_CANT_PARSE_OPTIONS without ever calling setSegmentRequestLen(), so the length stayed at the 0 set before the loop and handleNakPdu() acted on nothing. Every early return in that loop now reports the number of complete requests parsed. handleNakPdu() also resets its retransmit state before parsing, because the reader writes straight into the segment array and a hard parse error used to leave the previous NAK's indices pointing into a half overwritten one. handleFinishedPdu() marked the transaction finished and copied the delivery result before checking whether the PDU had parsed at all, so a Finished PDU truncated before its condition code byte completed the transfer and reported the default constructed DATA_COMPLETE - a fabricated success. Only a failure inside the optional TLVs is tolerated now. Neither handler checked which transaction an incoming ACK or Finished PDU belonged to, and CfdpHandler routes on direction and directive alone. A late ACK from a previous transaction therefore drove whichever one was running now, up to and including finishing it. Both handlers now compare the PDU's source entity ID and sequence number against the running transaction, by value rather than with operator==, which also compares the encoded width. The Finished PDU send is now retried on failure instead of being assumed sent, in both transmission modes, bounded by maxFinishedPduSendAttempts. Class 1 used to finish() regardless, so the peer never heard that a transfer which actually succeeded had completed and had no way to ask again. Bounding it is the point: a busy destination handler discards incoming metadata PDUs, so retrying forever would mean no later uplink could start. Exhausting the budget releases the transaction without declaring a fault - the file is complete on disk and the local user already got its indication, only the notification is lost. Finally, DestHandler's NAK segment scratch buffer is sized to hold at least one request, so a maxSegmentRequestsPerNakPdu of 0 is a clamped configuration rather than an out of bounds write. Eleven new test sections cover all of it. PduSenderMock gains failSendAtIdx and failSendsFromIdx alongside failNextSend, for the cases where the call under test emits several PDUs or where the downstream stays broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZpKfWUzvTkBMXEv9NEoTM	2026-09-15 00:46:37 +02:00
Tobias BaumgartlandClaude Sonnet 5	fa7ecca728	cfdp: stop swallowing PduSenderIF::sendPdu() failures SourceHandler::sendGenericPdu() and DestHandler::sendFinishedPdu() discarded sendPdu()'s return value, so a downstream send failure (e.g. a full TM store) was invisible to the state machine: transactionParams.progress advanced past file data that was never actually enqueued for downlink, and a retransmit hit the same swallowed-error path. Both now propagate the result so a failed send retries the same segment instead of being silently treated as sent. Adds a failure-injection knob to PduSenderMock and a SourceHandler test covering the retry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZpKfWUzvTkBMXEv9NEoTM	2026-09-14 23:27:09 +02:00
Tobias Baumgartl	0224523cc5	cfdp: implement acknowledged mode (class 2) in both handlers Class 1 has no retransmission at all: on a lossy uplink a single lost PDU corrupts a transfer and a lost metadata PDU strands it completely. The PDU layer for class 2 was already complete and unit tested, but neither handler implemented the procedures on top of it - the destination handler had a warning stub for BUSY_CLASS_2_ACKED and a SENDING_ACK_PDU step no branch serviced, and the source handler discarded every incoming PDU and parked permanently in BUSY_CLASS_2_ACKED. RemoteEntityCfg gains the positive ACK, NAK and check timer parameters. The names mirror cfdppy.mib.RemoteEntityConfig field for field so both ends of a link can be configured from the same numbers. The defaults are inert for class 1. Destination handler: - tracks received segments in the lostSegmentsContainer that was already plumbed through but never read, so completion is "no gaps and EOF seen" rather than the class 1 "progress reached the file size" - emits the ACK for the EOF PDU before the transfer completion step, because the checksum pass reads the whole file back from the SD card and would otherwise run inside the sender's positive ACK timer - runs the deferred lost segment procedure: one NAK sequence when the EOF arrives, re-issued on NAK timer expiry, NAK_LIMIT_REACHED on the limit. Segment requests are batched per PDU and the remainder carried over - retains the transaction until its Finished PDU is acknowledged, retransmits it on positive ACK timeout, POSITIVE_ACK_LIMIT_REACHED on the limit - can start a transaction from a file data PDU when the metadata was lost and request the metadata with a NAK of scope 0 to 0 - acknowledges an EOF PDU for an inactive transaction, otherwise the sender declares a fault at the end of an otherwise successful transfer - runs the check timer after EOF so an incomplete file is cancelled instead of pinning the handler forever Source handler: - consumes incoming PDUs instead of dropping them on the floor - waits for the ACK of its EOF PDU and retransmits on timeout - answers NAK PDUs by retransmitting the requested segments, one PDU per state machine call, and the metadata PDU for a scope 0 to 0 request. The read and send path is split from the forward-only progress cursor for this - implements WAIT_FOR_FINISH properly: parses the Finished PDU, acknowledges it and reports the received condition and delivery codes instead of a hardcoded NO_ERROR / DATA_COMPLETE. This also fixes class 1 with closure, which reported success for a transfer the receiver had rejected. The wait is bounded so a lost Finished PDU cannot pin the handler AckPduCreator and NakPduCreator get the `using FileDirectiveCreator::serialize` that FinishedPduCreator already had, so the convenience overloads are usable. crcOnTransmission stays unusable and unused: the CRC sizing bug in the PDU creators is a separate, self-contained fix.	2026-09-11 16:06:53 +02:00