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

# CFDP acknowledged mode (class 2)

## Summary

Both CFDP handlers implement class 2. Before this, `BUSY_CLASS_2_ACKED` was a stub on the
destination side and the source handler ignored every incoming PDU.

Destination handler:

```diff
 RECEIVING_FILE_DATA_PDUS        track gaps per received segment
   EOF
+  SENDING_ACK_PDU               ACK(EOF) before the checksum pass
+  WAITING_FOR_MISSING_DATA      deferred NAK sequence, NAK timer, check timer
   TRANSFER_COMPLETION
   SENDING_FINISHED_PDU
+  WAITING_FOR_FINISHED_ACK      positive ACK timer, retransmit Finished
```

Source handler:

```diff
 SENDING_FILE_DATA
 SENDING_EOF
+WAIT_FOR_ACK                    positive ACK timer, answer NAKs with retransmissions
 WAIT_FOR_FINISH                 now actually parses the Finished PDU
+  send ACK(Finished)
 NOTICE_OF_COMPLETION            reports the received condition and delivery codes
```

Behaviour worth knowing:

- Lost metadata is recovered: the transaction starts from the first file data or EOF PDU and
  requests the metadata with a NAK of scope 0..0.
- An EOF for a transaction that is already finished is still acknowledged.
- The NAK and check limits count expirations without progress. Missing data arriving resets them
  (CFDP 4.6.4.7).
- A class 2 metadata only transaction (a proxy put request) waits for its EOF, so the Finished PDU
  never overtakes the EOF ACK.
- NAK segment requests that do not fit one PDU are carried into the next PDU of the sequence.

Fixes found along the way, all of them also affect class 1:

- Metadata, NAK and Finished readers take the PDU CRC out of the parsed range. Before, any PDU
  with a CRC and options, segment requests or filestore responses failed with
  `INVALID_TLV_TYPE`.
- A Cancel EOF is parsed and ends the transaction. It was dropped as unparseable.
- `PduSenderIF::sendPdu()` failures are no longer treated as sent. Both handlers retry, the
  Finished PDU with a bounded number of attempts.
- PDUs are filtered by transaction ID, so a late PDU from an earlier transaction cannot drive the
  current one.
- A metadata only transaction reports `DATA_COMPLETE` instead of the "Data Incomplete" defaults.
- `arrayprinter` prints in fixed chunks instead of a stack buffer sized from the input. Dumping a
  12 KB buffer overflowed an 8 KB task.
- Metadata options beyond the option array are rejected instead of written past it.

New configuration, all with defaults:

```text
RemoteEntityCfg      positiveAckTimerIntervalMs, positiveAckTimerExpirationLimit,
                     nakTimerIntervalMs, nakTimerExpirationLimit, immediateNakMode,
                     checkLimit, checkTimerIntervalMs
DestHandlerParams    maxSegmentRequestsPerNakPdu, maxFinishedPduSendAttempts
returnvalue          cfdp::TRANSMISSION_MODE_NOT_SUPPORTED
```

## Evidence

- **Before:** an acknowledged uplink failed after about 40 s. The sender retransmitted EOF to
  `POSITIVE_ACK_LIMIT_REACHED` because the destination never sent an ACK. An acknowledged put
  request left the source handler busy for good.
  **After:** `fsfw-tests "[cfdp]"` passes, 26 test cases. New sections include:

  ```text
  CFDP Dest Handler Acknowledged
    Nominal acknowledged transfer
    Dropped file data PDU is recovered via NAK
    Dropped metadata PDU is requested with a scope 0 to 0 NAK
    Finished PDU is retransmitted on positive ACK timeout
    NAK limit reached cancels the transaction
    Retransmitted data resets the NAK and check limits
    Metadata only transaction sends Finished after the EOF ACK
    EOF PDU for an inactive transaction is acknowledged
    Cancel EOF is parsed, acknowledged and ends the transaction
    A Finished ACK for a different transaction is ignored
  CFDP Source Handler Acknowledged
    Nominal acknowledged transfer
    EOF PDU is retransmitted on positive ACK timeout
    NAK PDU drives segment retransmission
    NAK PDU of scope 0 to 0 retransmits the metadata PDU
    A NAK keeps the inactivity guard from firing during recovery
    Finished PDU condition code is reported instead of a hardcoded success
  Finished / Metadata / NAK PDU with CRC
  ```

  Plus class 1 sections for send failures, Cancel EOF and metadata only transfers. The last two
  destination fixes were checked with their tests failing before the fix.

- **Hardware:** SOURCE flatsat, iOBC, over the maintenance port with a proxy dropping 15 % of the
  packets in each direction. A 200 kB uplink and the downlink of the same file both completed
  with `NO_ERROR`, and the downlinked file was byte identical to the original.

## Merge Danger

**Door:** two-way

Plain code changes, no stored data or formats change. A revert restores the old behaviour.

**Blast Radius:** CFDP users

Every mission using the CFDP handlers gets the class 1 fixes too: send failures are retried
instead of dropped, a Cancel EOF ends the transaction, and CRC-bearing PDUs with variable length
tails now parse. Existing `RemoteEntityCfg` setups compile unchanged, the new fields default to
values that leave class 1 alone. Not covered: the PDU creators still cannot size a CRC-bearing PDU,
so `crcOnTransmission` has to stay false as before.

---------

Co-authored-by: Tobias Baumgartl <tobias.baumgartl@ksat-stuttgart.de>
Reviewed-on: #73

CHANGELOG.md

@@ -23,18 +23,32 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/745
 - Small tweak for version getter
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/744
+- CFDP: Metadata, NAK and Finished PDU readers no longer parse the PDU CRC as a TLV or segment
+  request.
+- CFDP: A Cancel EOF PDU is parsed and ends the transaction.
+- CFDP: `PduSenderIF::sendPdu()` failures are retried instead of being treated as sent.
+- CFDP: PDUs are filtered by transaction ID.
+- CFDP: A metadata only transaction reports `DATA_COMPLETE`.
+- CFDP: Metadata options beyond the option array are rejected instead of written past it.
+- `arrayprinter` prints in fixed chunks instead of a stack buffer sized from the input.
 
 ## Added
 
+- `hk::Dataset::deSerializeWithValidityBlob`, the counterpart of serializing with validity blob
+- functions to configure pus routing
 - FreeRTOS monotonic clock which is not subjected to time jumps of the system clock
 - add CFDP subsystem ID
   https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/742
 - `PusTmZcWriter` now exposes API to set message counter field.
 - Relative timeshift in the PUS time service.
 - CFDP support for PDU checksums
+- CFDP acknowledged mode (class 2) in the source and destination handlers: positive ACK, NAK and
+  check timers, deferred NAK procedure and recovery of lost metadata. Configured through new
+  `RemoteEntityCfg` and `DestHandlerParams` fields, which all have defaults.
 
 ## Changed
 
+- send HK one-parameter-report back to sender instead of default hk queue
 - Complete overhaul of HK subsystem. Replaced local data pool manager by periodic HK
   helper. The shared pool and the periodic HK generation are now distinct concepts.
 - The local HK manager was replaced by a periodic HK helper which has reduced responsibilities.
@@ -68,7 +82,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   configurable.
 - Switched to vendored versions for both the Embedded Template Library (ETL) and the
   Catch2 unittesting library.
-
+- Increased maximum number of mode tables from 70 to 100
+- Exposed health table mutex via getter function
 ## Added
 
 - `EventManager`: Add function to print all listeners.

misc/archive/MgmRM3100Handler.h

@@ -18,13 +18,13 @@ class MgmRM3100Handler : public DeviceHandlerBase {
   static const uint8_t INTERFACE_ID = CLASS_ID::MGM_RM3100;
 
   //! [EXPORT] : [COMMENT] P1: TMRC value which was set, P2: 0
-  static constexpr Event tmrcSet = event::makeEvent(SUBSYSTEM_ID::MGM_RM3100, 0x00, severity::INFO);
+  static constexpr Event tmrcSet = event::makeEvent<SUBSYSTEM_ID::MGM_RM3100, 0x00, severity::INFO>();
 
   //! [EXPORT] : [COMMENT] Cycle counter set. P1: First two bytes new Cycle Count X
   //! P1: Second two bytes new Cycle Count Y
   //! P2: New cycle count Z
   static constexpr Event cycleCountersSet =
-      event::makeEvent(SUBSYSTEM_ID::MGM_RM3100, 0x01, severity::INFO);
+      event::makeEvent<SUBSYSTEM_ID::MGM_RM3100, 0x01, severity::INFO>();
 
   MgmRM3100Handler(object_id_t objectId, object_id_t deviceCommunication, CookieIF *comCookie,
                    uint32_t transitionDelay);

misc/defaultcfg/fsfwconfig/FSFWConfig.h

@@ -71,6 +71,7 @@ static constexpr size_t FSFW_EVENTMGMR_RANGEMATCHERS   = 120;
 static constexpr uint8_t FSFW_CSB_FIFO_DEPTH = 6;
 
 static constexpr size_t FSFW_PRINT_BUFFER_SIZE = 124;
+static constexpr size_t FSFW_PRINT_BUFFER_AMOUNT = 32;
 
 static constexpr size_t FSFW_MAX_TM_PACKET_SIZE = 2048;
 

scripts/auto-formatter.sh

@@ -18,12 +18,12 @@ else
   echo "No ${cmake_fmt} tool found, not formatting CMake files"
 fi
 
-cpp_format="clang-format"
+cpp_format="clang-format-19"
 file_selectors="-iname *.h -o -iname *.cpp -o -iname *.c -o -iname *.tpp"
 if command -v ${cpp_format} &> /dev/null; then
   for dir in ${folder_list[@]}; do
     echo "Auto-formatting ${dir} recursively"
-    find ${dir} ${file_selectors} | xargs clang-format --style=file -i
+    find ${dir} ${file_selectors} | xargs ${cpp_format} --style=file -i
   done
 else
   echo "No ${cpp_format} tool found, not formatting C++/C files"

src/fsfw/cfdp/handler/DestHandler.cpp

@@ -2,14 +2,20 @@
 
 #include <etl/crc32.h>
 
+#include <algorithm>
 #include <utility>
 
 #include "fsfw/FSFW.h"
+#include "fsfw/cfdp/pdu/AckPduCreator.h"
+#include "fsfw/cfdp/pdu/AckPduReader.h"
 #include "fsfw/cfdp/pdu/EofPduReader.h"
 #include "fsfw/cfdp/pdu/FileDataReader.h"
 #include "fsfw/cfdp/pdu/FinishedPduCreator.h"
 #include "fsfw/cfdp/pdu/HeaderReader.h"
+#include "fsfw/cfdp/pdu/KeepAlivePduCreator.h"
+#include "fsfw/cfdp/pdu/NakPduCreator.h"
 #include "fsfw/objectmanager.h"
+#include "fsfw/returnvalues/returnvalue.h"
 #include "fsfw/tmtcservices/TmTcMessage.h"
 
 using namespace returnvalue;
@@ -23,7 +29,13 @@ cfdp::DestHandler::DestHandler(PduSenderIF& pduSender, size_t pduBufSize, DestHa
       msgToUserVec(params.maxTlvsInOnePdu),
       transactionParams(params.maxFilenameLen),
       destParams(std::move(params)),
-      fsfwParams(fsfwParams) {
+      fsfwParams(fsfwParams),
+      // At least one request has to fit, otherwise sendNakSequence() indexes an empty buffer and
+      // the deferred lost segment procedure could not make progress anyway.
+      nakSegmentBuf(std::max<size_t>(destParams.maxSegmentRequestsPerNakPdu, 1)),
+      positiveAckTimer(0, false),
+      nakTimer(0, false),
+      checkTimer(0, false) {
   transactionParams.pduConf.direction = cfdp::Direction::TOWARDS_SENDER;
 }
 
@@ -40,8 +52,30 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine(
       return fsmRes;
     }
     PduPacketIF& pduPacket = *optPduPacket;
-    if (pduPacket.getPduType() == FILE_DATA or
-        (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA)) {
+    if (pduPacket.getPduType() == FILE_DIRECTIVE and
+        *pduPacket.getFileDirective() == EOF_DIRECTIVE) {
+      // An EOF PDU retransmitted after we already finished the transaction still has to be
+      // acknowledged. Without this the sender keeps retransmitting until its positive ACK limit
+      // and then declares a fault at the end of a transfer that actually succeeded.
+      result = ackInactiveEofPdu(pduPacket);
+      if (result != OK) {
+        fsmRes.result = result;
+        return fsmRes;
+      }
+      return updateFsmRes(errorIdx);
+    }
+    if (pduPacket.getPduType() == FILE_DATA) {
+      // In acknowledged mode a lost metadata PDU is recoverable: start the transaction from the
+      // PDU header and ask for the metadata with a NAK of scope 0..0. Only acknowledged mode
+      // transactions can do this, everything else stays an error.
+      result = startMetadatalessTransaction(pduPacket);
+      if (result == OK) {
+        return updateFsmRes(errorIdx);
+      }
+      fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU;
+      return fsmRes;
+    }
+    if (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA) {
       fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU;
       return fsmRes;
     }
@@ -50,6 +84,11 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine(
     return updateFsmRes(errorIdx);
   }
 
+  if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) {
+    fsmAcked(optPduPacket, errorIdx);
+    return updateFsmRes(errorIdx);
+  }
+
   if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
     if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) {
       if (!optPduPacket.has_value()) {
@@ -73,20 +112,88 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine(
     checkAndHandleError(result, errorIdx);
   }
   if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) {
-    result = sendFinishedPdu();
-    checkAndHandleError(result, errorIdx);
-    finish();
+    // Class 1 has no ACK for the Finished PDU, so the transaction is done the moment it goes
+    // out. It must actually go out first though: finishing on a failed send means the peer never
+    // hears that a transfer which did succeed completed, and it has no way to ask again.
+    if (trySendingFinishedPdu(errorIdx)) {
+      finish();
+    }
     return updateFsmRes(errorIdx);
   }
-  if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) {
-    // TODO: Will be implemented at a later stage
-#if FSFW_CPP_OSTREAM_ENABLED == 1
-    sif::warning << "CFDP state machine for acknowledged mode not implemented yet" << std::endl;
-#endif
-  }
   return updateFsmRes(errorIdx);
 }
 
+void cfdp::DestHandler::fsmAcked(
+    const std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket, uint8_t& errorIdx) {
+  ReturnValue_t result;
+  if (optPduPacket.has_value()) {
+    PduPacketIF& pduPacket = *optPduPacket;
+    if (pduPacket.getPduType() == FILE_DATA) {
+      // Retransmitted segments are only expected while the file is still being received.
+      // handleFileDataPdu writes at the PDU offset, so out of order writes are fine.
+      if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS or
+          fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) {
+        result = handleFileDataPdu(pduPacket);
+        checkAndHandleError(result, errorIdx);
+      }
+    } else if (pduPacket.getPduType() == FILE_DIRECTIVE) {
+      switch (*pduPacket.getFileDirective()) {
+        case (METADATA): {
+          // Either a duplicate, which startTransaction discards, or the retransmission we asked
+          // for after a lost metadata PDU.
+          result = handleMetadataPdu(pduPacket);
+          checkAndHandleError(result, errorIdx);
+          break;
+        }
+        case (EOF_DIRECTIVE): {
+          result = handleEofPdu(pduPacket);
+          checkAndHandleError(result, errorIdx);
+          break;
+        }
+        case (ACK): {
+          result = handleAckPdu(pduPacket);
+          checkAndHandleError(result, errorIdx);
+          break;
+        }
+        default:
+          break;
+      }
+    }
+  }
+  // The ACK for the EOF PDU is emitted here, before TRANSFER_COMPLETION runs the checksum
+  // pass over the whole received file. On an iOBC that pass takes seconds for a large file,
+  // which is long enough for the sender's positive ACK timer to fire.
+  if (fsmRes.step == TransactionStep::SENDING_ACK_PDU) {
+    result = handleSendingAckPdu();
+    checkAndHandleError(result, errorIdx);
+  }
+  if (fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) {
+    result = handleWaitingForMissingData();
+    checkAndHandleError(result, errorIdx);
+  }
+  if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) {
+    result = handleTransferCompletion();
+    checkAndHandleError(result, errorIdx);
+  }
+  if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) {
+    if (not trySendingFinishedPdu(errorIdx)) {
+      // Nothing went out, so there is no ACK to wait for: either the send is retried on the next
+      // call, or the attempt budget ran out and the transaction has already been released.
+      return;
+    }
+    // In acknowledged mode the Finished PDU is itself acknowledged, so the transaction is
+    // retained until the ACK arrives instead of being finished right here.
+    transactionParams.positiveAckCounter = 0;
+    positiveAckTimer.setTimeout(transactionParams.remoteCfg->positiveAckTimerIntervalMs);
+    fsmRes.step = TransactionStep::WAITING_FOR_FINISHED_ACK;
+    return;
+  }
+  if (fsmRes.step == TransactionStep::WAITING_FOR_FINISHED_ACK) {
+    result = handleWaitingForFinishedAck();
+    checkAndHandleError(result, errorIdx);
+  }
+}
+
 ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PduPacketIF& pduPacket) {
   // Process metadata PDU
   cfdp::StringLv sourceFileName;
@@ -116,6 +223,11 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) {
   if (result != OK) {
     return result;
   }
+  if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED and not transactionParams.metadataReceived) {
+    // The metadata PDU was lost, so there is no destination file name to write to yet. Discard
+    // the payload; it is re-requested by the NAK sequence once the metadata has arrived.
+    return OK;
+  }
   size_t fileSegmentLen = 0;
   const uint8_t* fileData = fdInfo.getFileData(&fileSegmentLen);
   if (destParams.cfg.indicCfg.fileSegmentRecvIndicRequired) {
@@ -148,8 +260,13 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) {
   }
   transactionParams.deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE;
   transactionParams.vfsErrorCount = 0;
-  if (fdInfo.getOffset().value() + fileSegmentLen > transactionParams.progress) {
-    transactionParams.progress = fdInfo.getOffset().value() + fileSegmentLen;
+  const uint64_t offset = fdInfo.getOffset().value();
+  const uint64_t endOfSegment = offset + fileSegmentLen;
+  if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) {
+    trackReceivedSegment(offset, endOfSegment);
+  }
+  if (endOfSegment > transactionParams.progress) {
+    transactionParams.progress = endOfSegment;
   }
   return result;
 }
@@ -158,7 +275,7 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) {
   size_t pduSize = 0;
   const auto rawPdu = info.getRawPduData(pduSize);
   // Process EOF PDU
-  EofInfo eofInfo(nullptr);
+  EofInfo eofInfo(&eofFaultLocation);
   EofPduReader reader(rawPdu, pduSize, eofInfo);
   ReturnValue_t result = reader.parseData();
   if (result != OK) {
@@ -177,12 +294,39 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) {
   if (destParams.cfg.indicCfg.eofRecvIndicRequired) {
     destParams.user.eofRecvIndication(getTransactionId());
   }
-  if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) {
-    if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
+  if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
+    if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) {
       fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
-    } else if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) {
-      fsmRes.step = TransactionStep::SENDING_ACK_PDU;
     }
+    return returnvalue::OK;
+  }
+  if (fsmRes.state != CfdpState::BUSY_CLASS_2_ACKED) {
+    return returnvalue::OK;
+  }
+  transactionParams.eofReceived = true;
+  transactionParams.eofConditionCode = eofInfo.getConditionCode();
+  if (eofInfo.getConditionCode() != ConditionCode::NO_ERROR) {
+    // A Cancel EOF ends the transaction. Adopt its condition code so that transfer completion
+    // reports the cancellation, instead of running a checksum pass over a file the sender has
+    // already abandoned and then reporting a checksum failure for it.
+    transactionParams.conditionCode = eofInfo.getConditionCode();
+  }
+  if (eofInfo.getConditionCode() == ConditionCode::NO_ERROR and
+      transactionParams.fileSize.value() > transactionParams.progress) {
+    // Everything between the highest offset seen so far and the file size announced by the EOF
+    // PDU is missing. This is also the only gap that exists for a transfer which lost its tail.
+    insertLostSegment(transactionParams.progress, transactionParams.fileSize.value());
+  }
+  if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS or
+      fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) {
+    // A duplicate EOF arriving during the lost segment procedure means our ACK did not make it
+    // back, so re-enter the ACK step, which also re-issues the NAK sequence.
+    fsmRes.step = TransactionStep::SENDING_ACK_PDU;
+  } else {
+    // Duplicate EOF for a transaction we are already completing. Re-acknowledge it without
+    // disturbing the step we are in.
+    return sendAckPdu(transactionParams.pduConf, FileDirective::EOF_DIRECTIVE,
+                      transactionParams.eofConditionCode, AckTransactionStatus::ACTIVE);
   }
   return returnvalue::OK;
 }
@@ -226,8 +370,12 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result,
 }
 
 ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reader) {
-  if (fsmRes.state != CfdpState::IDLE) {
-    // According to standard, discard metadata PDU if we are busy
+  // A metadata PDU received while busy is normally a duplicate and is discarded per the standard.
+  // The one exception is an acknowledged transaction which was started from a file data or EOF
+  // PDU because the metadata was lost: this is the retransmission our NAK of scope 0..0 asked for.
+  const bool lateMetadata =
+      fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED and not transactionParams.metadataReceived;
+  if (fsmRes.state != CfdpState::IDLE and not lateMetadata) {
     return OK;
   }
   ReturnValue_t result = OK;
@@ -296,18 +444,23 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade
 #endif
     return FAILED;
   }
-  if (reader.getTransmissionMode() == TransmissionMode::UNACKNOWLEDGED) {
-    fsmRes.state = CfdpState::BUSY_CLASS_1_NACKED;
-  } else if (reader.getTransmissionMode() == TransmissionMode::ACKNOWLEDGED) {
-    fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED;
-  }
-  if (transactionParams.metadataOnly) {
-    fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
-  } else {
-    // Kind of ugly, make FSM working on packet per packet basis..
-    fsmRes.step = TransactionStep::TRANSACTION_START;
-    fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS;
+  if (not lateMetadata) {
+    if (reader.getTransmissionMode() == TransmissionMode::UNACKNOWLEDGED) {
+      fsmRes.state = CfdpState::BUSY_CLASS_1_NACKED;
+    } else if (reader.getTransmissionMode() == TransmissionMode::ACKNOWLEDGED) {
+      fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED;
+    }
+    if (transactionParams.metadataOnly and fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
+      fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+    } else {
+      // In class 2 a metadata only transaction still waits for its EOF PDU. The Finished PDU
+      // must follow the EOF ACK, the sender discards a Finished PDU while it waits for the ACK.
+      // Kind of ugly, make FSM working on packet per packet basis..
+      fsmRes.step = TransactionStep::TRANSACTION_START;
+      fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS;
+    }
   }
+  transactionParams.metadataReceived = true;
   auto& info = reader.getGenericInfo();
   transactionParams.checksumType = info.getChecksumType();
   transactionParams.closureRequested = info.isClosureRequested();
@@ -338,13 +491,25 @@ cfdp::CfdpState cfdp::DestHandler::getCfdpState() const { return fsmRes.state; }
 
 ReturnValue_t cfdp::DestHandler::handleTransferCompletion() {
   ReturnValue_t result;
-  if (transactionParams.checksumType != ChecksumType::NULL_CHECKSUM) {
+  if (transactionParams.conditionCode != ConditionCode::NO_ERROR) {
+    // The transaction was cancelled, for example because the NAK or check limit was reached.
+    // Report that condition code in the Finished PDU rather than running a checksum pass over a
+    // file which is known to be incomplete.
+    transactionParams.deliveryCode = FileDeliveryCode::DATA_INCOMPLETE;
+  } else if (transactionParams.checksumType != ChecksumType::NULL_CHECKSUM) {
     result = checksumVerification();
     if (result != OK) {
       // TODO: Warning / error handling?
     }
   } else {
     transactionParams.conditionCode = ConditionCode::NO_ERROR;
+    if (transactionParams.metadataOnly) {
+      // Nothing was expected, so nothing is missing: a metadata only transaction, for example a
+      // proxy put request, carries no file data and completes the moment its metadata arrives.
+      // The reset defaults would report it as "Data Incomplete" and "Discard deliberately".
+      transactionParams.deliveryCode = FileDeliveryCode::DATA_COMPLETE;
+      transactionParams.deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED;
+    }
   }
   result = noticeOfCompletion();
   if (result != OK) {
@@ -394,6 +559,7 @@ void cfdp::DestHandler::fileErrorHandler(Event event, ReturnValue_t result,
 }
 
 void cfdp::DestHandler::finish() {
+  destParams.lostSegmentsContainer.clear();
   transactionParams.reset();
   fsmRes.state = CfdpState::IDLE;
   fsmRes.step = TransactionStep::IDLE;
@@ -450,6 +616,19 @@ ReturnValue_t cfdp::DestHandler::noticeOfCompletion() {
   return OK;
 }
 
+ReturnValue_t cfdp::DestHandler::sendKeepAlivePdu() {
+  Fss progress(transactionParams.progress);
+  KeepAlivePduCreator keepAlivePdu(transactionParams.pduConf, progress);
+  size_t serLen = 0;
+  ReturnValue_t result =
+      keepAlivePdu.serialize(pduBuf.data(), serLen, keepAlivePdu.getSerializedSize());
+  if (result != OK) {
+    return result;
+  }
+  return pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::KEEP_ALIVE, pduBuf.data(),
+                           serLen);
+}
+
 ReturnValue_t cfdp::DestHandler::sendFinishedPdu() {
   FinishedInfo info(transactionParams.conditionCode, transactionParams.deliveryCode,
                     transactionParams.deliveryStatus);
@@ -467,7 +646,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() {
     fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0);
     return result;
   }
-  pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen);
+  result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen);
   if (result != OK) {
 #if FSFW_CPP_OSTREAM_ENABLED == 1
     sif::warning << "cfdp::DestHandler::sendFinishedPdu: Sending PDU failed" << std::endl;
@@ -479,6 +658,33 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() {
   return OK;
 }
 
+bool cfdp::DestHandler::trySendingFinishedPdu(uint8_t& errorIdx) {
+  transactionParams.finishedSendAttempts++;
+  const ReturnValue_t result = sendFinishedPdu();
+  checkAndHandleError(result, errorIdx);
+  if (result == OK) {
+    return true;
+  }
+  if (transactionParams.finishedSendAttempts < destParams.maxFinishedPduSendAttempts) {
+    // Leave the step where it is so the next state machine call retries the same PDU. A send
+    // failure here is usually a full TM store, which drains on its own.
+    return false;
+  }
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+  sif::warning << "cfdp::DestHandler: giving up on the Finished PDU after "
+               << transactionParams.finishedSendAttempts << " attempts" << std::endl;
+#else
+  sif::printWarning("cfdp::DestHandler: giving up on the Finished PDU after %u attempts\n",
+                    static_cast<unsigned>(transactionParams.finishedSendAttempts));
+#endif
+  // Only the peer's notification is lost: the file is complete on disk and the local user
+  // already got its indication from noticeOfCompletion(). So this is not a delivery fault, and
+  // releasing the handler matters more than the notification - while a transaction is held, every
+  // incoming metadata PDU is discarded and no new uplink can start.
+  finish();
+  return false;
+}
+
 cfdp::DestHandler::TransactionStep cfdp::DestHandler::getTransactionStep() const {
   return fsmRes.step;
 }
@@ -496,6 +702,8 @@ const cfdp::TransactionId& cfdp::DestHandler::getTransactionId() const {
   return transactionParams.transactionId;
 }
 
+uint64_t cfdp::DestHandler::getProgress() const { return transactionParams.progress; }
+
 void cfdp::DestHandler::checkAndHandleError(ReturnValue_t result, uint8_t& errorIdx) {
   if (result != OK and errorIdx < 3) {
     fsmRes.errorCodes[errorIdx] = result;
@@ -509,4 +717,368 @@ void cfdp::DestHandler::setEventReporter(EventReportingProxyIF& reporter) {
 
 const cfdp::DestHandlerParams& cfdp::DestHandler::getDestHandlerParams() const {
   return destParams;
-}
+}
+
+bool cfdp::DestHandler::pduBelongsToTransaction(const PduPacketIF& pduPacket) const {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  PduHeaderReader reader(rawPdu, pduSize);
+  if (reader.parseData() != OK) {
+    return false;
+  }
+  EntityId sourceId;
+  reader.getSourceId(sourceId);
+  TransactionSeqNum seqNum;
+  reader.getTransactionSeqNum(seqNum);
+  // Compared by value rather than with operator==, which also compares the encoded width: the
+  // peer is free to use a different width than we do for the same number.
+  return sourceId.getValue() == transactionParams.transactionId.entityId.getValue() and
+         seqNum.getValue() == transactionParams.transactionId.seqNum.getValue();
+}
+
+ReturnValue_t cfdp::DestHandler::handleAckPdu(const cfdp::PduPacketIF& info) {
+  size_t pduSize = 0;
+  const auto rawPdu = info.getRawPduData(pduSize);
+  AckInfo ackInfo;
+  AckPduReader reader(rawPdu, pduSize, ackInfo);
+  ReturnValue_t result = reader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  // ACKs for EOF PDUs belong to the source handler and are routed there, so the only ACK which
+  // can legitimately reach the destination handler is the one for our Finished PDU.
+  if (ackInfo.getAckedDirective() != FileDirective::FINISH) {
+    return OK;
+  }
+  if (not pduBelongsToTransaction(info)) {
+    // The CFDP handler routes ACK PDUs on the acknowledged directive alone, so a late ACK from
+    // an earlier transaction would otherwise finish whichever one is running now.
+    return OK;
+  }
+  if (fsmRes.step == TransactionStep::WAITING_FOR_FINISHED_ACK) {
+    finish();
+  }
+  return OK;
+}
+
+ReturnValue_t cfdp::DestHandler::handleSendingAckPdu() {
+  ReturnValue_t result =
+      sendAckPdu(transactionParams.pduConf, FileDirective::EOF_DIRECTIVE,
+                 transactionParams.eofConditionCode, AckTransactionStatus::ACTIVE);
+  if (result != OK) {
+    return result;
+  }
+  if (transactionParams.eofConditionCode != ConditionCode::NO_ERROR) {
+    // The sender cancelled the transaction. Nothing left to request, report what we have.
+    transactionParams.conditionCode = transactionParams.eofConditionCode;
+    fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+    return OK;
+  }
+  if (isFileComplete()) {
+    fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+    return OK;
+  }
+  // Deferred lost segment procedure. The NAK sequence for everything known to be missing is
+  // issued once, here, and then re-issued on NAK timer expiry.
+  fsmRes.step = TransactionStep::WAITING_FOR_MISSING_DATA;
+  transactionParams.nakCounter = 0;
+  transactionParams.checkCounter = 0;
+  result = sendNakSequence();
+  nakTimer.setTimeout(transactionParams.remoteCfg->nakTimerIntervalMs);
+  checkTimer.setTimeout(transactionParams.remoteCfg->checkTimerIntervalMs);
+  return result;
+}
+
+ReturnValue_t cfdp::DestHandler::handleWaitingForMissingData() {
+  if (isFileComplete()) {
+    fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+    return OK;
+  }
+  if (checkTimer.hasTimedOut()) {
+    transactionParams.checkCounter++;
+    checkTimer.resetTimer();
+    if (transactionParams.checkCounter > transactionParams.remoteCfg->checkLimit) {
+      // Without this an incomplete transfer pins the handler forever and no later uplink
+      // can start.
+      declareFault(ConditionCode::CHECK_LIMIT_REACHED);
+      fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+      return OK;
+    }
+  }
+  if (nakTimer.hasTimedOut()) {
+    transactionParams.nakCounter++;
+    nakTimer.resetTimer();
+    if (transactionParams.nakCounter > transactionParams.remoteCfg->nakTimerExpirationLimit) {
+      declareFault(ConditionCode::NAK_LIMIT_REACHED);
+      fsmRes.step = TransactionStep::TRANSFER_COMPLETION;
+      return OK;
+    }
+    return sendNakSequence();
+  }
+  return OK;
+}
+
+ReturnValue_t cfdp::DestHandler::handleWaitingForFinishedAck() {
+  if (not positiveAckTimer.hasTimedOut()) {
+    return OK;
+  }
+  transactionParams.positiveAckCounter++;
+  positiveAckTimer.resetTimer();
+  if (transactionParams.positiveAckCounter >
+      transactionParams.remoteCfg->positiveAckTimerExpirationLimit) {
+    // The sender is not acknowledging our Finished PDU. The file itself is already written, so
+    // report the fault and release the handler instead of holding the transaction forever.
+    declareFault(ConditionCode::POSITIVE_ACK_LIMIT_REACHED);
+    finish();
+    return OK;
+  }
+  return sendFinishedPdu();
+}
+
+ReturnValue_t cfdp::DestHandler::startMetadatalessTransaction(const PduPacketIF& pduPacket) {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  PduHeaderReader headerReader(rawPdu, pduSize);
+  ReturnValue_t result = headerReader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  if (headerReader.getTransmissionMode() != TransmissionMode::ACKNOWLEDGED) {
+    // Unacknowledged mode has no way of recovering the metadata PDU, so this stays an error.
+    return FAILED;
+  }
+  EntityId sourceId;
+  headerReader.getSourceId(sourceId);
+  if (not destParams.remoteCfgTable.getRemoteCfg(sourceId, &transactionParams.remoteCfg)) {
+    return FAILED;
+  }
+  headerReader.fillConfig(transactionParams.pduConf);
+  transactionParams.pduConf.crcFlag = transactionParams.remoteCfg->crcOnTransmission;
+  transactionParams.pduConf.direction = Direction::TOWARDS_SENDER;
+  transactionParams.transactionId.entityId = transactionParams.pduConf.sourceId;
+  transactionParams.transactionId.seqNum = transactionParams.pduConf.seqNum;
+  transactionParams.metadataReceived = false;
+  transactionParams.metadataOnly = false;
+  fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED;
+  fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS;
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+  sif::warning << "cfdp::DestHandler: file data PDU without metadata, requesting metadata"
+               << std::endl;
+#else
+  sif::printWarning("cfdp::DestHandler: file data PDU without metadata, requesting metadata\n");
+#endif
+  // Nothing at all can be done before the metadata arrives, so this one NAK is not deferred.
+  result = sendNakSequence();
+  transactionParams.nakCounter = 0;
+  transactionParams.checkCounter = 0;
+  nakTimer.setTimeout(transactionParams.remoteCfg->nakTimerIntervalMs);
+  checkTimer.setTimeout(transactionParams.remoteCfg->checkTimerIntervalMs);
+  return result;
+}
+
+ReturnValue_t cfdp::DestHandler::ackInactiveEofPdu(const PduPacketIF& pduPacket) {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  PduHeaderReader headerReader(rawPdu, pduSize);
+  ReturnValue_t result = headerReader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  if (headerReader.getTransmissionMode() != TransmissionMode::ACKNOWLEDGED) {
+    return DEST_NON_METADATA_PDU_AS_FIRST_PDU;
+  }
+  EofInfo eofInfo(&eofFaultLocation);
+  EofPduReader eofReader(rawPdu, pduSize, eofInfo);
+  result = eofReader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  PduConfig conf;
+  headerReader.fillConfig(conf);
+  conf.direction = Direction::TOWARDS_SENDER;
+  RemoteEntityCfg* remoteCfg = nullptr;
+  EntityId sourceId;
+  headerReader.getSourceId(sourceId);
+  if (destParams.remoteCfgTable.getRemoteCfg(sourceId, &remoteCfg) and remoteCfg != nullptr) {
+    conf.crcFlag = remoteCfg->crcOnTransmission;
+  } else {
+    conf.crcFlag = false;
+  }
+  return sendAckPdu(conf, FileDirective::EOF_DIRECTIVE, eofInfo.getConditionCode(),
+                    AckTransactionStatus::UNRECOGNIZED);
+}
+
+ReturnValue_t cfdp::DestHandler::sendAckPdu(PduConfig& conf, FileDirective ackedDirective,
+                                            ConditionCode conditionCode,
+                                            AckTransactionStatus status) {
+  // CFDP 5.2.4: the directive subtype code is 0b0001 for an acknowledged Finished PDU and
+  // 0b0000 for every other acknowledged directive.
+  AckInfo ackInfo(ackedDirective, conditionCode, status,
+                  ackedDirective == FileDirective::FINISH ? 1 : 0);
+  AckPduCreator ackPdu(ackInfo, conf);
+  size_t serLen = 0;
+  ReturnValue_t result = ackPdu.serialize(pduBuf.data(), serLen, ackPdu.getSerializedSize());
+  if (result != OK) {
+    fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0);
+    return result;
+  }
+  result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::ACK, pduBuf.data(), serLen);
+  if (result != OK) {
+    fsfwParams.eventReporter->forwardEvent(events::PDU_SEND_ERROR, result, 0);
+    return result;
+  }
+  fsmRes.packetsSent++;
+  return OK;
+}
+
+ReturnValue_t cfdp::DestHandler::sendNakSequence() {
+  const bool largeFile = transactionParams.pduConf.largeFile;
+  const uint64_t endOfScope = transactionParams.eofReceived ? transactionParams.fileSize.value()
+                                                            : transactionParams.progress;
+  ReturnValue_t worstResult = OK;
+  size_t idx = 0;
+  // The segment requests of one NAK PDU are bounded. What does not fit is carried into the
+  // next PDU of the sequence instead of being dropped or declared a fault.
+  auto flushNak = [&]() {
+    if (idx == 0) {
+      return;
+    }
+    NakInfo nakInfo(Fss(0, largeFile), Fss(endOfScope, largeFile));
+    size_t segLen = idx;
+    size_t maxSegLen = nakSegmentBuf.size();
+    nakInfo.setSegmentRequests(nakSegmentBuf.data(), &segLen, &maxSegLen);
+    NakPduCreator nakPdu(transactionParams.pduConf, nakInfo);
+    size_t serLen = 0;
+    ReturnValue_t result = nakPdu.serialize(pduBuf.data(), serLen, nakPdu.getSerializedSize());
+    if (result != OK) {
+      fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0);
+      worstResult = result;
+      idx = 0;
+      return;
+    }
+    result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::NAK, pduBuf.data(), serLen);
+    if (result != OK) {
+      fsfwParams.eventReporter->forwardEvent(events::PDU_SEND_ERROR, result, 0);
+      worstResult = result;
+    } else {
+      fsmRes.packetsSent++;
+    }
+    idx = 0;
+  };
+
+  if (not transactionParams.metadataReceived) {
+    // CFDP 5.2.6: a segment request of 0 to 0 requests the metadata PDU.
+    nakSegmentBuf[idx++] = {Fss(0, largeFile), Fss(0, largeFile)};
+    if (idx == nakSegmentBuf.size()) {
+      flushNak();
+    }
+  }
+  for (const auto& lostSegment : destParams.lostSegmentsContainer) {
+    nakSegmentBuf[idx++] = {Fss(lostSegment.first, largeFile), Fss(lostSegment.second, largeFile)};
+    if (idx == nakSegmentBuf.size()) {
+      flushNak();
+    }
+  }
+  flushNak();
+  return worstResult;
+}
+
+bool cfdp::DestHandler::isFileComplete() const {
+  // Class 2 completion is "no gaps left and EOF received", not the class 1 "progress reached the
+  // file size": a transfer can pass the announced file size with a retransmission while a gap in
+  // the middle is still outstanding.
+  return transactionParams.metadataReceived and transactionParams.eofReceived and
+         destParams.lostSegmentsContainer.empty() and
+         transactionParams.progress >= transactionParams.fileSize.value();
+}
+
+void cfdp::DestHandler::trackReceivedSegment(uint64_t offset, uint64_t endOfSegment) {
+  if (offset > transactionParams.progress) {
+    // Everything between the high water mark and this segment was skipped over.
+    insertLostSegment(transactionParams.progress, offset);
+  }
+  if (removeReceivedRange(offset, endOfSegment) and
+      fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) {
+    // CFDP 4.6.4.7: receiving missing data resets the NAK activity parameters. The limits bound
+    // time without progress, not the total time a lossy link needs. The check timer is reset as
+    // well, otherwise it cancels a recovery that is still converging.
+    transactionParams.nakCounter = 0;
+    transactionParams.checkCounter = 0;
+    nakTimer.resetTimer();
+    checkTimer.resetTimer();
+  }
+}
+
+void cfdp::DestHandler::insertLostSegment(uint64_t start, uint64_t end) {
+  if (end <= start) {
+    return;
+  }
+  auto& container = destParams.lostSegmentsContainer;
+  // Merge with every entry this range touches so the list cannot accumulate duplicate or
+  // overlapping gaps when an EOF PDU is retransmitted.
+  for (auto it = container.begin(); it != container.end();) {
+    if (it->second < start or it->first > end) {
+      ++it;
+      continue;
+    }
+    start = std::min(start, it->first);
+    end = std::max(end, it->second);
+    it = container.erase(it);
+  }
+  if (container.full()) {
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    sif::warning << "cfdp::DestHandler: lost segment list full, gap dropped" << std::endl;
+#else
+    sif::printWarning("cfdp::DestHandler: lost segment list full, gap dropped\n");
+#endif
+    return;
+  }
+  container.insert({start, end});
+}
+
+bool cfdp::DestHandler::removeReceivedRange(uint64_t start, uint64_t end) {
+  if (end <= start) {
+    return false;
+  }
+  auto& container = destParams.lostSegmentsContainer;
+  // A received range can span several gaps, but only the first and the last of them can be left
+  // with a remainder, so two pending re-insertions are always enough.
+  std::array<etl::pair<uint64_t, uint64_t>, 2> pending{};
+  size_t pendingLen = 0;
+  bool removed = false;
+  for (auto it = container.begin(); it != container.end();) {
+    if (it->second <= start or it->first >= end) {
+      ++it;
+      continue;
+    }
+    removed = true;
+    const uint64_t gapStart = it->first;
+    const uint64_t gapEnd = it->second;
+    it = container.erase(it);
+    if (gapStart < start and pendingLen < pending.size()) {
+      pending[pendingLen++] = {gapStart, start};
+    }
+    if (gapEnd > end and pendingLen < pending.size()) {
+      pending[pendingLen++] = {end, gapEnd};
+    }
+  }
+  for (size_t pendingIdx = 0; pendingIdx < pendingLen; pendingIdx++) {
+    if (container.full()) {
+      break;
+    }
+    container.insert(pending[pendingIdx]);
+  }
+  return removed;
+}
+
+void cfdp::DestHandler::declareFault(ConditionCode code) {
+  transactionParams.conditionCode = code;
+  transactionParams.deliveryCode = FileDeliveryCode::DATA_INCOMPLETE;
+  destParams.cfg.fhBase.reportFault(transactionParams.transactionId, code);
+}
+
+size_t cfdp::DestHandler::getNumLostSegments() const {
+  return destParams.lostSegmentsContainer.size();
+}
+
+uint32_t cfdp::DestHandler::getNakCounter() const { return transactionParams.nakCounter; }

src/fsfw/cfdp/handler/DestHandler.h

@@ -14,10 +14,14 @@
 #include "fsfw/cfdp/handler/PduPacketIF.h"
 #include "fsfw/cfdp/handler/PduSenderIF.h"
 #include "fsfw/cfdp/handler/mib.h"
+#include "fsfw/cfdp/pdu/HeaderReader.h"
 #include "fsfw/cfdp/pdu/MetadataPduReader.h"
+#include "fsfw/cfdp/pdu/NakInfo.h"
 #include "fsfw/cfdp/pdu/PduConfig.h"
+#include "fsfw/cfdp/tlv/EntityIdTlv.h"
 #include "fsfw/cfdp/tlv/MessageToUserTlv.h"
 #include "fsfw/storagemanager/StorageManagerIF.h"
+#include "fsfw/timemanager/Countdown.h"
 #include "fsfw/tmtcservices/AcceptsTelemetryIF.h"
 
 namespace cfdp {
@@ -47,6 +51,17 @@ struct DestHandlerParams {
   LostSegmentsListBase& lostSegmentsContainer;
   uint8_t maxTlvsInOnePdu = 20;
   size_t maxFilenameLen = 255;
+  //! Upper bound on the number of segment requests packed into a single NAK PDU. The remaining
+  //! lost segments are carried over into the next NAK of the sequence, so this bounds the PDU
+  //! size without bounding what can be requested. Each segment request is 8 bytes for a small
+  //! file and 16 bytes for a large one.
+  size_t maxSegmentRequestsPerNakPdu = 20;
+  //! Attempts at sending the Finished PDU before the transaction is released without it. The
+  //! send can fail on a transient downstream condition (a full TM store), and treating that as
+  //! sent loses the peer's only notification that the transfer completed. The retry has to be
+  //! bounded though: a busy destination handler discards incoming metadata PDUs, so holding a
+  //! transaction forever means no later uplink can start.
+  uint32_t maxFinishedPduSendAttempts = 5;
 };
 
 class DestHandler {
@@ -57,7 +72,13 @@ class DestHandler {
     RECEIVING_FILE_DATA_PDUS = 2,
     SENDING_ACK_PDU = 3,
     TRANSFER_COMPLETION = 4,
-    SENDING_FINISHED_PDU = 5
+    SENDING_FINISHED_PDU = 5,
+    //! Class 2 only: the deferred lost segment procedure is running. NAK PDUs have been issued
+    //! for the known gaps and the handler is waiting for the retransmissions.
+    WAITING_FOR_MISSING_DATA = 6,
+    //! Class 2 only: the Finished PDU was sent and its ACK is outstanding. The transaction is
+    //! retained until the ACK arrives or the positive ACK limit is reached.
+    WAITING_FOR_FINISHED_ACK = 7
   };
 
   struct FsmResult {
@@ -101,8 +122,14 @@ class DestHandler {
 
   [[nodiscard]] CfdpState getCfdpState() const;
   [[nodiscard]] TransactionStep getTransactionStep() const;
+  [[nodiscard]] uint64_t getProgress() const;
+  ReturnValue_t sendKeepAlivePdu();
   [[nodiscard]] const TransactionId& getTransactionId() const;
   [[nodiscard]] const DestHandlerParams& getDestHandlerParams() const;
+  //! Number of gaps currently tracked by the deferred lost segment procedure. Always 0 in class 1.
+  [[nodiscard]] size_t getNumLostSegments() const;
+  //! Number of NAK sequences issued for the current transaction.
+  [[nodiscard]] uint32_t getNakCounter() const;
 
  private:
   struct TransactionParams {
@@ -126,8 +153,29 @@ class DestHandler {
       closureRequested = false;
       vfsErrorCount = 0;
       checksumType = ChecksumType::NULL_CHECKSUM;
+      metadataReceived = false;
+      eofReceived = false;
+      eofConditionCode = ConditionCode::NO_ERROR;
+      positiveAckCounter = 0;
+      nakCounter = 0;
+      checkCounter = 0;
+      finishedSendAttempts = 0;
     }
 
+    //! Attempts made at sending the Finished PDU, see
+    //! DestHandlerParams::maxFinishedPduSendAttempts. Used by both transmission modes.
+    uint32_t finishedSendAttempts = 0;
+
+    //! Class 2 only: false while the transaction was started from a file data or EOF PDU because
+    //! the metadata PDU was lost. The metadata is then requested with a NAK of scope 0..0.
+    bool metadataReceived = false;
+    bool eofReceived = false;
+    //! Condition code of the received EOF PDU, needed to build the matching ACK PDU.
+    ConditionCode eofConditionCode = ConditionCode::NO_ERROR;
+    uint32_t positiveAckCounter = 0;
+    uint32_t nakCounter = 0;
+    uint32_t checkCounter = 0;
+
     bool metadataOnly = false;
     ChecksumType checksumType = ChecksumType::NULL_CHECKSUM;
     bool closureRequested = false;
@@ -153,6 +201,20 @@ class DestHandler {
   DestHandlerParams destParams;
   cfdp::FsfwParams fsfwParams;
   FsmResult fsmRes;
+  //! Scratch space for the segment requests of one NAK PDU. Sized from
+  //! DestHandlerParams::maxSegmentRequestsPerNakPdu.
+  std::vector<NakInfo::SegmentRequest> nakSegmentBuf;
+  //! Guards the Finished PDU, see RemoteEntityCfg::positiveAckTimerIntervalMs.
+  Countdown positiveAckTimer;
+  //! Drives the deferred lost segment procedure, see RemoteEntityCfg::nakTimerIntervalMs.
+  Countdown nakTimer;
+  //! Guards an incomplete transaction after EOF reception, see RemoteEntityCfg::checkLimit.
+  Countdown checkTimer;
+  //! Receives the fault location of an incoming EOF PDU. EofPduReader refuses to parse any EOF
+  //! whose condition code is not NO_ERROR unless it has somewhere to put that TLV, so without
+  //! these every Cancel EOF the sender emits would be dropped as unparseable.
+  cfdp::EntityId eofFaultLocationId;
+  EntityIdTlv eofFaultLocation{eofFaultLocationId};
 
   ReturnValue_t startTransaction(const MetadataPduReader& reader);
   ReturnValue_t handleMetadataPdu(const PduPacketIF& pduPacket);
@@ -160,9 +222,38 @@ class DestHandler {
   ReturnValue_t handleEofPdu(const PduPacketIF& info);
   ReturnValue_t handleMetadataParseError(ReturnValue_t result, const uint8_t* rawData,
                                          size_t maxSize);
+  ReturnValue_t handleAckPdu(const PduPacketIF& info);
+  //! True if the PDU's source entity ID and sequence number match the running transaction. The
+  //! CFDP handler routes PDUs by direction only, so this is the only transaction level filter.
+  [[nodiscard]] bool pduBelongsToTransaction(const PduPacketIF& pduPacket) const;
   ReturnValue_t handleTransferCompletion();
+  //! Class 2 substates, driven from stateMachine().
+  void fsmAcked(std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket, uint8_t& errorIdx);
+  ReturnValue_t handleSendingAckPdu();
+  ReturnValue_t handleWaitingForMissingData();
+  ReturnValue_t handleWaitingForFinishedAck();
+  ReturnValue_t startMetadatalessTransaction(const PduPacketIF& pduPacket);
+  ReturnValue_t ackInactiveEofPdu(const PduPacketIF& pduPacket);
+  ReturnValue_t sendAckPdu(PduConfig& conf, FileDirective ackedDirective,
+                           ConditionCode conditionCode, AckTransactionStatus status);
+  ReturnValue_t sendNakSequence();
+  [[nodiscard]] bool isFileComplete() const;
+  void trackReceivedSegment(uint64_t offset, uint64_t endOfSegment);
+  void insertLostSegment(uint64_t start, uint64_t end);
+  //! Returns true if the range filled any part of a tracked gap.
+  bool removeReceivedRange(uint64_t start, uint64_t end);
+  void declareFault(ConditionCode code);
   ReturnValue_t tryBuildingAbsoluteDestName(size_t destNameSize);
   ReturnValue_t sendFinishedPdu();
+  /**
+   * Sends the Finished PDU, retrying a bounded number of times on a failed send.
+   *
+   * @return True if the PDU went out and the caller should advance. False means the caller must
+   *         leave the step alone and do nothing else this iteration: either the send is being
+   *         retried on the next call, or the attempt budget ran out and the transaction was
+   *         already released with finish().
+   */
+  bool trySendingFinishedPdu(uint8_t& errorIdx);
   ReturnValue_t noticeOfCompletion();
   ReturnValue_t checksumVerification();
   void fileErrorHandler(Event event, ReturnValue_t result, const char* info) const;

src/fsfw/cfdp/handler/PutRequest.cpp

@@ -14,7 +14,10 @@ cfdp::PutRequest::PutRequest(cfdp::EntityId destId, const uint8_t *msgsToUser,
 
 cfdp::PutRequest::PutRequest(cfdp::EntityId destId, cfdp::StringLv &sourceName,
                              cfdp::StringLv &destName)
-    : destId(std::move(destId)), sourceName(std::move(sourceName)), destName(std::move(destName)) {}
+    : destId(std::move(destId)),
+      metadataOnly(false),
+      sourceName(std::move(sourceName)),
+      destName(std::move(destName)) {}
 
 [[nodiscard]] bool cfdp::PutRequest::isMetadataOnly() const { return metadataOnly; }
 

src/fsfw/cfdp/handler/SourceHandler.cpp

@@ -4,9 +4,15 @@
 
 #include <array>
 
+#include "fsfw/cfdp/pdu/AckPduCreator.h"
+#include "fsfw/cfdp/pdu/AckPduReader.h"
 #include "fsfw/cfdp/pdu/EofPduCreator.h"
 #include "fsfw/cfdp/pdu/FileDataCreator.h"
+#include "fsfw/cfdp/pdu/FileDirectiveReader.h"
+#include "fsfw/cfdp/pdu/FinishedPduReader.h"
+#include "fsfw/cfdp/pdu/HeaderReader.h"
 #include "fsfw/cfdp/pdu/MetadataPduCreator.h"
+#include "fsfw/cfdp/pdu/NakPduReader.h"
 #include "fsfw/filesystem/HasFileSystemIF.h"
 #include "fsfw/globalfunctions/arrayprinter.h"
 #include "fsfw/objectmanager.h"
@@ -16,12 +22,28 @@
 
 using namespace returnvalue;
 
+namespace {
+
+//! True if the PDU is long enough to hold the Finished PDU fields which are mandatory, i.e. the
+//! directive byte plus the byte carrying the condition code, delivery code and file status.
+//! Anything shorter cannot be trusted even partially.
+bool mandatoryFinishedFieldsPresent(const uint8_t* rawPdu, size_t pduSize) {
+  FileDirectiveReader directiveReader(rawPdu, pduSize);
+  if (directiveReader.parseData() != returnvalue::OK) {
+    return false;
+  }
+  return directiveReader.getWholePduSize() > directiveReader.getHeaderSize();
+}
+
+}  // namespace
+
 cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize,
                                    SourceHandlerParams params, FsfwParams fsfwParams)
     : pduSender(pduSender),
       pduBuf(pduBufferSize),
       sourceParams(std::move(params)),
-      fsfwParams(fsfwParams) {
+      fsfwParams(fsfwParams),
+      positiveAckTimer(0, false) {
   // The entity ID portion of the transaction ID will always remain fixed.
   transactionParams.id.entityId = sourceParams.cfg.localId;
   transactionParams.pduConf.sourceId = sourceParams.cfg.localId;
@@ -47,8 +69,19 @@ cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize,
   transactionParams.pduConf.seqNum.setValue(0);
 }
 
-cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() {
+cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked(
+    const std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket) {
   ReturnValue_t result;
+  if (optPduPacket.has_value() and step == TransactionStep::WAIT_FOR_FINISH) {
+    PduPacketIF& pduPacket = *optPduPacket;
+    if (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() == FINISH and
+        pduBelongsToTransaction(pduPacket)) {
+      result = handleFinishedPdu(pduPacket);
+      if (result != OK) {
+        addError(result);
+      }
+    }
+  }
   if (step == TransactionStep::IDLE) {
     step = TransactionStep::TRANSACTION_START;
   }
@@ -86,6 +119,10 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() {
     }
     if (transactionParams.closureRequested) {
       step = TransactionStep::WAIT_FOR_FINISH;
+      // Bound the wait for the Finished PDU with the positive ACK timer configuration so a lost
+      // Finished PDU cannot pin the handler.
+      transactionParams.positiveAckCounter = 0;
+      positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs);
       // fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY;
     } else {
       step = TransactionStep::NOTICE_OF_COMPLETION;
@@ -94,7 +131,20 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() {
     return fsmResult;
   }
   if (step == TransactionStep::WAIT_FOR_FINISH) {
-    // TODO: In case this is a request with closure, wait for finish.
+    if (not transactionParams.finishedReceived) {
+      if (not positiveAckTimer.hasTimedOut()) {
+        return fsmResult;
+      }
+      transactionParams.positiveAckCounter++;
+      positiveAckTimer.resetTimer();
+      if (transactionParams.positiveAckCounter <=
+          transactionParams.remoteCfg.positiveAckTimerExpirationLimit) {
+        return fsmResult;
+      }
+      // Unacknowledged mode has no retransmission, so the only thing left is to report that the
+      // peer never confirmed the transfer.
+      declareFault(ConditionCode::INACTIVITY_DETECTED);
+    }
     // Done, issue notice of completion
     step = TransactionStep::NOTICE_OF_COMPLETION;
   }
@@ -117,7 +167,10 @@ const cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::stateMachine(
     return fsmResult;
   }
   if (state == cfdp::CfdpState::BUSY_CLASS_1_NACKED) {
-    return fsmNacked();
+    return fsmNacked(optPduPacket);
+  }
+  if (state == cfdp::CfdpState::BUSY_CLASS_2_ACKED) {
+    return fsmAcked(optPduPacket);
   }
   return fsmResult;
 }
@@ -210,7 +263,10 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo
     state = cfdp::CfdpState::BUSY_CLASS_2_ACKED;
   } else if (transactionParams.pduConf.mode == TransmissionMode::UNACKNOWLEDGED) {
     state = cfdp::CfdpState::BUSY_CLASS_1_NACKED;
+  } else {
+    return TRANSMISSION_MODE_NOT_SUPPORTED;
   }
+  retransmitState.reset();
   step = TransactionStep::IDLE;
   uint64_t fileSize = 0;
   sourceParams.user.vfs.getFileSize(transactionParams.sourceName.data(), fileSize);
@@ -230,15 +286,7 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo
 }
 
 ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() {
-  cfdp::StringLv sourceName(transactionParams.sourceName.data(), transactionParams.sourceNameSize);
-  cfdp::StringLv destName(transactionParams.destName.data(), transactionParams.destNameSize);
-  auto metadataInfo =
-      MetadataGenericInfo(transactionParams.closureRequested, transactionParams.checksumType,
-                          transactionParams.fileSize);
-  auto metadataPdu =
-      MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0);
-  ReturnValue_t result =
-      sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu);
+  ReturnValue_t result = sendMetadataPdu();
   if (result != OK) {
     return result;
   }
@@ -247,8 +295,18 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() {
   return OK;
 }
 
+ReturnValue_t cfdp::SourceHandler::sendMetadataPdu() {
+  cfdp::StringLv sourceName(transactionParams.sourceName.data(), transactionParams.sourceNameSize);
+  cfdp::StringLv destName(transactionParams.destName.data(), transactionParams.destNameSize);
+  auto metadataInfo =
+      MetadataGenericInfo(transactionParams.closureRequested, transactionParams.checksumType,
+                          transactionParams.fileSize);
+  auto metadataPdu =
+      MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0);
+  return sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu);
+}
+
 ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDataPdu) {
-  cfdp::Fss offset(transactionParams.progress);
   uint64_t lenToRead;
   uint64_t fileSize = transactionParams.fileSize.value();
   noFileDataPdu = false;
@@ -267,19 +325,7 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDat
       lenToRead = transactionParams.remoteCfg.maxFileSegmentLen;
     }
   }
-  FileOpParams fileParams(transactionParams.sourceName.data(), lenToRead);
-  fileParams.offset = transactionParams.progress;
-  size_t readLen = 0;
-  ReturnValue_t result = sourceParams.user.vfs.readFromFile(
-      transactionParams.sourceName.data(), transactionParams.progress, lenToRead, fileBuf.data(),
-      readLen, fileBuf.size());
-  if (result != returnvalue::OK) {
-    addError(result);
-    return result;
-  }
-  auto fileDataInfo = FileDataInfo(offset, fileBuf.data(), lenToRead);
-  auto fileDataPdu = FileDataCreator(transactionParams.pduConf, fileDataInfo);
-  result = sendGenericPdu(PduType::FILE_DATA, std::nullopt, fileDataPdu);
+  ReturnValue_t result = sendFileDataPdu(transactionParams.progress, lenToRead);
   if (result != OK) {
     return result;
   }
@@ -319,20 +365,21 @@ ReturnValue_t cfdp::SourceHandler::sendGenericPdu(PduType pduType,
     addError(result);
     return result;
   }
-  pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen);
+  result = pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen);
+  if (result != OK) {
+    addError(result);
+    return result;
+  }
   fsmResult.packetsSent += 1;
-  return result;
+  return OK;
 }
 
 ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() {
   if (sourceParams.cfg.indicCfg.transactionFinishedIndicRequired) {
-    // TODO: This could still be improved by caching the Finished PDU parameters.
-    FileDeliveryStatus deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED;
-    if (transactionParams.closureRequested) {
-      deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE;
-    }
-    cfdp::TransactionFinishedParams params(transactionParams.id, ConditionCode::NO_ERROR,
-                                           FileDeliveryCode::DATA_COMPLETE, deliveryStatus);
+    // The values reported by the peer's Finished PDU, if one was received.
+    cfdp::TransactionFinishedParams params(
+        transactionParams.id, transactionParams.finishedConditionCode,
+        transactionParams.finishedDeliveryCode, transactionParams.finishedDeliveryStatus);
     sourceParams.user.transactionFinishedIndication(params);
   }
   return OK;
@@ -341,6 +388,7 @@ ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() {
 ReturnValue_t cfdp::SourceHandler::reset() {
   step = TransactionStep::IDLE;
   state = cfdp::CfdpState::IDLE;
+  retransmitState.reset();
   // fsmResult.callStatus = CallStatus::DONE;
   transactionParams.reset();
   return OK;
@@ -356,3 +404,338 @@ void cfdp::SourceHandler::addError(ReturnValue_t error) {
     fsmResult.result = error;
   }
 }
+
+cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmAcked(
+    const std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket) {
+  ReturnValue_t result;
+  if (optPduPacket.has_value()) {
+    handleAckedPdu(*optPduPacket);
+  }
+  if (step == TransactionStep::IDLE) {
+    step = TransactionStep::TRANSACTION_START;
+  }
+  if (step == TransactionStep::TRANSACTION_START) {
+    sourceParams.user.transactionIndication(transactionParams.id);
+    result = checksumGeneration();
+    if (result != OK) {
+      addError(result);
+    }
+    step = TransactionStep::SENDING_METADATA;
+  }
+  if (step == TransactionStep::SENDING_METADATA) {
+    result = prepareAndSendMetadataPdu();
+    if (result != OK) {
+      addError(result);
+    }
+    return fsmResult;
+  }
+  if (step == TransactionStep::SENDING_FILE_DATA) {
+    bool noFdPdu = false;
+    result = prepareAndSendNextFileDataPdu(noFdPdu);
+    if (result == OK and !noFdPdu) {
+      return fsmResult;
+    }
+  }
+  if (step == TransactionStep::SENDING_EOF) {
+    result = prepareAndSendEofPdu();
+    if (result != OK) {
+      addError(result);
+    }
+    if (sourceParams.cfg.indicCfg.eofSentIndicRequired) {
+      sourceParams.user.eofSentIndication(transactionParams.id);
+    }
+    // The EOF PDU is acknowledged in class 2. Closure is meaningless here: the Finished PDU is
+    // mandatory, so the transaction always runs to WAIT_FOR_FINISH.
+    transactionParams.positiveAckCounter = 0;
+    positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs);
+    step = TransactionStep::WAIT_FOR_ACK;
+    return fsmResult;
+  }
+  // Retransmissions requested by the peer take priority over any timer work: a source which
+  // cannot answer a NAK deadlocks the transfer. One PDU per call keeps the burst bounded in
+  // exactly the same way the regular file data phase is.
+  if (servicePendingRetransmissions(result)) {
+    if (result != OK) {
+      addError(result);
+    }
+    return fsmResult;
+  }
+  if (step == TransactionStep::WAIT_FOR_ACK) {
+    if (not positiveAckTimer.hasTimedOut()) {
+      return fsmResult;
+    }
+    transactionParams.positiveAckCounter++;
+    positiveAckTimer.resetTimer();
+    if (transactionParams.positiveAckCounter >
+        transactionParams.remoteCfg.positiveAckTimerExpirationLimit) {
+      declareFault(ConditionCode::POSITIVE_ACK_LIMIT_REACHED);
+      noticeOfCompletion();
+      reset();
+      return fsmResult;
+    }
+    result = prepareAndSendEofPdu();
+    if (result != OK) {
+      addError(result);
+    }
+    return fsmResult;
+  }
+  if (step == TransactionStep::WAIT_FOR_FINISH) {
+    if (not transactionParams.finishedReceived) {
+      if (not positiveAckTimer.hasTimedOut()) {
+        return fsmResult;
+      }
+      transactionParams.positiveAckCounter++;
+      positiveAckTimer.resetTimer();
+      if (transactionParams.positiveAckCounter <=
+          transactionParams.remoteCfg.positiveAckTimerExpirationLimit) {
+        return fsmResult;
+      }
+      // The receiver retransmits its Finished PDU on its own positive ACK timer, and any NAK it
+      // sends re-arms this guard, so reaching this point means nothing has been heard from it at
+      // all - the downlink is gone, rather than a single PDU being lost or a NAK recovery still
+      // running.
+      declareFault(ConditionCode::INACTIVITY_DETECTED);
+    }
+    step = TransactionStep::NOTICE_OF_COMPLETION;
+  }
+  if (step == TransactionStep::NOTICE_OF_COMPLETION) {
+    noticeOfCompletion();
+    reset();
+  }
+  return fsmResult;
+}
+
+bool cfdp::SourceHandler::pduBelongsToTransaction(const PduPacketIF& pduPacket) const {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  PduHeaderReader reader(rawPdu, pduSize);
+  if (reader.parseData() != OK) {
+    return false;
+  }
+  EntityId sourceId;
+  reader.getSourceId(sourceId);
+  TransactionSeqNum seqNum;
+  reader.getTransactionSeqNum(seqNum);
+  // Compared by value rather than with operator==, which also compares the encoded width: the
+  // peer is free to use a different width than we do for the same number.
+  return sourceId.getValue() == transactionParams.id.entityId.getValue() and
+         seqNum.getValue() == transactionParams.id.seqNum.getValue();
+}
+
+void cfdp::SourceHandler::handleAckedPdu(PduPacketIF& pduPacket) {
+  if (pduPacket.getPduType() != FILE_DIRECTIVE) {
+    return;
+  }
+  if (not pduBelongsToTransaction(pduPacket)) {
+    // Nothing upstream filters by transaction: the CFDP handler routes on the PDU direction
+    // alone. A late ACK or Finished PDU from an earlier transaction would otherwise drive
+    // whichever transaction is running now.
+    return;
+  }
+  ReturnValue_t result = OK;
+  switch (*pduPacket.getFileDirective()) {
+    case (FileDirective::ACK): {
+      result = handleAckPdu(pduPacket);
+      break;
+    }
+    case (FileDirective::NAK): {
+      result = handleNakPdu(pduPacket);
+      break;
+    }
+    case (FileDirective::FINISH): {
+      result = handleFinishedPdu(pduPacket);
+      break;
+    }
+    default:
+      // Keep Alive PDUs carry progress information only, there is nothing to drive from them.
+      break;
+  }
+  if (result != OK) {
+    addError(result);
+  }
+}
+
+ReturnValue_t cfdp::SourceHandler::handleAckPdu(const PduPacketIF& pduPacket) {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  AckInfo ackInfo;
+  AckPduReader reader(rawPdu, pduSize, ackInfo);
+  ReturnValue_t result = reader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  // ACKs for Finished PDUs are routed to the destination handler, so only the EOF ACK can
+  // legitimately arrive here.
+  if (ackInfo.getAckedDirective() != FileDirective::EOF_DIRECTIVE) {
+    return OK;
+  }
+  if (step == TransactionStep::WAIT_FOR_ACK) {
+    step = TransactionStep::WAIT_FOR_FINISH;
+    // Re-arm the same timer as an inactivity guard for the Finished PDU.
+    transactionParams.positiveAckCounter = 0;
+    positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs);
+  }
+  return OK;
+}
+
+ReturnValue_t cfdp::SourceHandler::handleFinishedPdu(const PduPacketIF& pduPacket) {
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  FinishedInfo finishedInfo;
+  FinishPduReader reader(rawPdu, pduSize, finishedInfo);
+  // The condition code, delivery code and file status are parsed before any TLV, so they are
+  // usable even if this handler cannot hold the optional filestore responses.
+  ReturnValue_t result = reader.parseData();
+  if (result != OK and not mandatoryFinishedFieldsPresent(rawPdu, pduSize)) {
+    // The PDU is truncated before those fields, so it says nothing at all about how the transfer
+    // went. Completing the transaction on it would report the default constructed DATA_COMPLETE,
+    // i.e. a fabricated success, and would discard the state the peer's retransmission needs.
+    return result;
+  }
+  transactionParams.finishedReceived = true;
+  transactionParams.finishedConditionCode = finishedInfo.getConditionCode();
+  transactionParams.finishedDeliveryCode = finishedInfo.getDeliveryCode();
+  transactionParams.finishedDeliveryStatus = finishedInfo.getFileStatus();
+  if (state == CfdpState::BUSY_CLASS_2_ACKED) {
+    // The Finished PDU is acknowledged in class 2. This has to happen even for a duplicate,
+    // because a duplicate means our previous ACK was lost.
+    ReturnValue_t ackResult =
+        sendAckPdu(FileDirective::FINISH, transactionParams.finishedConditionCode);
+    if (ackResult != OK) {
+      return ackResult;
+    }
+  }
+  if (result != OK) {
+    // Only the optional TLVs failed to parse, the delivery result above is still valid.
+    return OK;
+  }
+  return OK;
+}
+
+ReturnValue_t cfdp::SourceHandler::handleNakPdu(const PduPacketIF& pduPacket) {
+  // A NAK is proof the receiver is alive and still working on this transaction, so it clears the
+  // inactivity guard armed for the Finished PDU. Without this the guard fires during a healthy
+  // deferred NAK recovery: the receiver cannot send a Finished PDU until it has every segment, so
+  // between NAK rounds it is deliberately silent, and that silence is not evidence of a dead
+  // downlink.
+  transactionParams.positiveAckCounter = 0;
+  positiveAckTimer.resetTimer();
+
+  size_t pduSize = 0;
+  const auto rawPdu = pduPacket.getRawPduData(pduSize);
+  NakInfo nakInfo(Fss(0), Fss(0));
+  size_t maxSegments = retransmitState.segments.size();
+  size_t segmentLen = 0;
+  nakInfo.setSegmentRequests(retransmitState.segments.data(), &segmentLen, &maxSegments);
+  // The reader writes straight into the segment array, so the previous NAK's bookkeeping is
+  // invalid the moment parsing starts, whether or not it succeeds. Drop it up front rather than
+  // leaving indices pointing into a half overwritten array on an error return.
+  retransmitState.reset();
+  NakPduReader reader(rawPdu, pduSize, nakInfo);
+  ReturnValue_t result = reader.parseData();
+  // The segment requests may have been truncated, but whatever was parsed completely into the
+  // array is still worth retransmitting: the receiver re-NAKs what it does not get.
+  retransmitState.numSegments = nakInfo.getSegmentRequestsLen();
+  retransmitState.currentIdx = 0;
+  retransmitState.cursor = 0;
+  retransmitState.metadataPending = false;
+  // CFDP 5.2.6: a segment request of 0 to 0 asks for the metadata PDU rather than file data.
+  // Compact it out of the list so the file data path does not have to special case it.
+  size_t writeIdx = 0;
+  for (size_t readIdx = 0; readIdx < retransmitState.numSegments; readIdx++) {
+    const auto& segment = retransmitState.segments[readIdx];
+    if (segment.first.value() == 0 and segment.second.value() == 0) {
+      retransmitState.metadataPending = true;
+      continue;
+    }
+    retransmitState.segments[writeIdx++] = segment;
+  }
+  retransmitState.numSegments = writeIdx;
+  // Reported so a truncated NAK is visible as an error even though the requests which did parse
+  // are serviced normally.
+  return result;
+}
+
+bool cfdp::SourceHandler::servicePendingRetransmissions(ReturnValue_t& result) {
+  result = OK;
+  if (retransmitState.metadataPending) {
+    result = sendMetadataPdu();
+    if (result == OK) {
+      // Only clear the request once it actually went out. The peer asked for the metadata
+      // because it cannot write the file at all without it, and it will not ask again until its
+      // NAK timer expires.
+      retransmitState.metadataPending = false;
+    }
+    return true;
+  }
+  const uint64_t fileSize = transactionParams.fileSize.value();
+  while (retransmitState.currentIdx < retransmitState.numSegments) {
+    const auto& segment = retransmitState.segments[retransmitState.currentIdx];
+    uint64_t start = segment.first.value();
+    uint64_t end = segment.second.value();
+    if (retransmitState.cursor > start) {
+      start = retransmitState.cursor;
+    }
+    if (end > fileSize) {
+      end = fileSize;
+    }
+    if (start >= end) {
+      retransmitState.currentIdx++;
+      retransmitState.cursor = 0;
+      continue;
+    }
+    uint64_t lenToSend = end - start;
+    if (lenToSend > transactionParams.remoteCfg.maxFileSegmentLen) {
+      lenToSend = transactionParams.remoteCfg.maxFileSegmentLen;
+    }
+    result = sendFileDataPdu(start, lenToSend);
+    if (result != OK) {
+      // The cursor must not move past data which was never enqueued for downlink, exactly as in
+      // the forward-only path: the next call retries this same offset. Advancing here would drop
+      // the segment until the peer's NAK timer re-requests it, and that costs a NAK limit credit.
+      return true;
+    }
+    retransmitState.cursor = start + lenToSend;
+    if (retransmitState.cursor >= end) {
+      retransmitState.currentIdx++;
+      retransmitState.cursor = 0;
+    }
+    return true;
+  }
+  return false;
+}
+
+ReturnValue_t cfdp::SourceHandler::sendFileDataPdu(uint64_t offset, size_t lenToRead) {
+  if (lenToRead > fileBuf.size()) {
+    addError(FILE_SEGMENT_LEN_INVALID);
+    return FILE_SEGMENT_LEN_INVALID;
+  }
+  size_t readLen = 0;
+  ReturnValue_t result =
+      sourceParams.user.vfs.readFromFile(transactionParams.sourceName.data(), offset, lenToRead,
+                                         fileBuf.data(), readLen, fileBuf.size());
+  if (result != returnvalue::OK) {
+    addError(result);
+    return result;
+  }
+  cfdp::Fss offsetFss(offset, transactionParams.pduConf.largeFile);
+  auto fileDataInfo = FileDataInfo(offsetFss, fileBuf.data(), lenToRead);
+  auto fileDataPdu = FileDataCreator(transactionParams.pduConf, fileDataInfo);
+  return sendGenericPdu(PduType::FILE_DATA, std::nullopt, fileDataPdu);
+}
+
+ReturnValue_t cfdp::SourceHandler::sendAckPdu(FileDirective ackedDirective,
+                                              ConditionCode conditionCode) {
+  // CFDP 5.2.4: the directive subtype code is 0b0001 for an acknowledged Finished PDU and
+  // 0b0000 for every other acknowledged directive.
+  AckInfo ackInfo(ackedDirective, conditionCode, AckTransactionStatus::ACTIVE,
+                  ackedDirective == FileDirective::FINISH ? 1 : 0);
+  AckPduCreator ackPdu(ackInfo, transactionParams.pduConf);
+  return sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::ACK, ackPdu);
+}
+
+void cfdp::SourceHandler::declareFault(ConditionCode code) {
+  transactionParams.finishedConditionCode = code;
+  transactionParams.finishedDeliveryCode = FileDeliveryCode::DATA_INCOMPLETE;
+  sourceParams.cfg.fhBase.reportFault(transactionParams.id, code);
+}

src/fsfw/cfdp/handler/SourceHandler.h

@@ -1,6 +1,7 @@
 #ifndef FSFW_CFDP_CFDPSOURCEHANDLER_H
 #define FSFW_CFDP_CFDPSOURCEHANDLER_H
 
+#include <array>
 #include <cstdint>
 #include <vector>
 
@@ -11,8 +12,10 @@
 #include "fsfw/cfdp/Fss.h"
 #include "fsfw/cfdp/handler/PutRequest.h"
 #include "fsfw/cfdp/handler/mib.h"
+#include "fsfw/cfdp/pdu/NakInfo.h"
 #include "fsfw/events/EventReportingProxyIF.h"
 #include "fsfw/storagemanager/StorageManagerIF.h"
+#include "fsfw/timemanager/Countdown.h"
 #include "fsfw/tmtcservices/AcceptsTelemetryIF.h"
 #include "fsfw/util/ProvidesSeqCountIF.h"
 
@@ -81,15 +84,51 @@ class SourceHandler {
     PduConfig pduConf;
     cfdp::TransactionId id{};
 
+    //! Number of positive ACK timer expirations for the EOF PDU, or of inactivity timer
+    //! expirations while waiting for the Finished PDU.
+    uint32_t positiveAckCounter = 0;
+    bool finishedReceived = false;
+    //! Delivery result reported by the peer in its Finished PDU. The defaults are what a
+    //! transfer without closure reports.
+    ConditionCode finishedConditionCode = ConditionCode::NO_ERROR;
+    FileDeliveryCode finishedDeliveryCode = FileDeliveryCode::DATA_COMPLETE;
+    FileDeliveryStatus finishedDeliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED;
+
     void reset() {
       sourceNameSize = 0;
       destNameSize = 0;
       fileSize.setFileSize(0, false);
       progress = 0;
       closureRequested = false;
+      positiveAckCounter = 0;
+      finishedReceived = false;
+      finishedConditionCode = ConditionCode::NO_ERROR;
+      finishedDeliveryCode = FileDeliveryCode::DATA_COMPLETE;
+      finishedDeliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED;
     }
   } transactionParams;
 
+  //! Pending retransmissions requested by the last received NAK PDU. Only the most recent NAK is
+  //! kept: it is the peer's authoritative statement about what is still missing, and bounding the
+  //! state this way keeps a NAK storm from growing the handler's memory footprint.
+  struct RetransmitState {
+    static constexpr size_t MAX_SEGMENTS = 32;
+    std::array<NakInfo::SegmentRequest, MAX_SEGMENTS> segments{};
+    size_t numSegments = 0;
+    size_t currentIdx = 0;
+    //! Absolute file offset reached inside the segment at currentIdx, 0 if it was not started.
+    uint64_t cursor = 0;
+    bool metadataPending = false;
+
+    void reset() {
+      numSegments = 0;
+      currentIdx = 0;
+      cursor = 0;
+      metadataPending = false;
+    }
+    [[nodiscard]] bool empty() const { return not metadataPending and currentIdx >= numSegments; }
+  } retransmitState;
+
   PduSenderIF& pduSender;
   std::vector<uint8_t> pduBuf;
   cfdp::CfdpState state = cfdp::CfdpState::IDLE;
@@ -98,13 +137,29 @@ class SourceHandler {
   SourceHandlerParams sourceParams;
   cfdp::FsfwParams fsfwParams;
   FsmResult fsmResult;
+  //! Guards the EOF PDU in acknowledged mode and the wait for the Finished PDU in both modes.
+  Countdown positiveAckTimer;
 
-  FsmResult& fsmNacked();
+  FsmResult& fsmNacked(std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket);
+  FsmResult& fsmAcked(std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket);
+  //! True if the PDU's source entity ID and sequence number match the running transaction. The
+  //! CFDP handler routes PDUs by direction only, so this is the only transaction level filter.
+  [[nodiscard]] bool pduBelongsToTransaction(const PduPacketIF& pduPacket) const;
+  void handleAckedPdu(PduPacketIF& pduPacket);
+  ReturnValue_t handleFinishedPdu(const PduPacketIF& pduPacket);
+  ReturnValue_t handleAckPdu(const PduPacketIF& pduPacket);
+  ReturnValue_t handleNakPdu(const PduPacketIF& pduPacket);
+  //! Sends one PDU of the outstanding retransmissions, if there are any.
+  bool servicePendingRetransmissions(ReturnValue_t& result);
+  ReturnValue_t sendAckPdu(FileDirective ackedDirective, ConditionCode conditionCode);
   ReturnValue_t checksumGeneration();
+  ReturnValue_t sendMetadataPdu();
   ReturnValue_t prepareAndSendMetadataPdu();
+  ReturnValue_t sendFileDataPdu(uint64_t offset, size_t lenToRead);
   ReturnValue_t prepareAndSendNextFileDataPdu(bool& noFileDataPdu);
   ReturnValue_t prepareAndSendEofPdu();
   ReturnValue_t noticeOfCompletion();
+  void declareFault(ConditionCode code);
   ReturnValue_t reset();
 
   [[nodiscard]] ReturnValue_t sendGenericPdu(PduType pduType,

src/fsfw/cfdp/handler/defs.h

@@ -19,13 +19,13 @@ struct FsfwParams {
 };
 
 namespace events {
-static constexpr Event PDU_SEND_ERROR = event::makeEvent(SSID, 1, severity::LOW);
-static constexpr Event SERIALIZATION_ERROR = event::makeEvent(SSID, 2, severity::LOW);
-static constexpr Event FILESTORE_ERROR = event::makeEvent(SSID, 3, severity::LOW);
+static constexpr Event PDU_SEND_ERROR = event::makeEvent<SSID, 1, severity::LOW>();
+static constexpr Event SERIALIZATION_ERROR = event::makeEvent<SSID, 2, severity::LOW>();
+static constexpr Event FILESTORE_ERROR = event::makeEvent<SSID, 3, severity::LOW>();
 //! [EXPORT] : [COMMENT] P1: Transaction step ID, P2: 0 for source file name, 1 for dest file name
-static constexpr Event FILENAME_TOO_LARGE_ERROR = event::makeEvent(SSID, 4, severity::LOW);
+static constexpr Event FILENAME_TOO_LARGE_ERROR = event::makeEvent<SSID, 4, severity::LOW>();
 //! [EXPORT] : [COMMENT] CFDP request handling failed. P2: Returncode.
-static constexpr Event HANDLING_CFDP_REQUEST_FAILED = event::makeEvent(SSID, 5, severity::LOW);
+static constexpr Event HANDLING_CFDP_REQUEST_FAILED = event::makeEvent<SSID, 5, severity::LOW>();
 }  // namespace events
 
 static constexpr ReturnValue_t SOURCE_TRANSACTION_PENDING = returnvalue::makeCode(CID, 0);
@@ -38,4 +38,6 @@ static constexpr ReturnValue_t TARGET_MSG_QUEUE_FULL = returnvalue::makeCode(CID
 static constexpr ReturnValue_t TM_STORE_FULL = returnvalue::makeCode(CID, 7);
 static constexpr ReturnValue_t DEST_NON_METADATA_PDU_AS_FIRST_PDU = returnvalue::makeCode(CID, 8);
 static constexpr ReturnValue_t PDU_BUFFER_TOO_SMALL = returnvalue::makeCode(CID, 9);
+//! The resolved transmission mode of a request is not supported by this handler.
+static constexpr ReturnValue_t TRANSMISSION_MODE_NOT_SUPPORTED = returnvalue::makeCode(CID, 10);
 }  // namespace cfdp

src/fsfw/cfdp/handler/mib.h

@@ -36,6 +36,31 @@ struct RemoteEntityCfg {
   TransmissionMode defaultTransmissionMode = TransmissionMode::UNACKNOWLEDGED;
   ChecksumType defaultChecksum = ChecksumType::NULL_CHECKSUM;
   uint8_t version = CFDP_VERSION_2;
+
+  // Acknowledged mode (class 2) parameters. The names mirror cfdppy.mib.RemoteEntityConfig field
+  // for field so both ends of a link can be configured from the same set of numbers. The defaults
+  // are inert for class 1, which uses none of them.
+
+  //! Interval of the positive acknowledgment timer, which guards EOF (source side) and Finished
+  //! (destination side) PDUs. Must be larger than the peer's worst case time to answer with the
+  //! matching ACK PDU, or both sides retransmit over each other.
+  uint32_t positiveAckTimerIntervalMs = 10000;
+  //! Number of positive ACK timer expirations after which POSITIVE_ACK_LIMIT_REACHED is declared.
+  uint32_t positiveAckTimerExpirationLimit = 2;
+  //! Interval of the NAK timer used by the deferred lost segment procedure.
+  uint32_t nakTimerIntervalMs = 10000;
+  //! Number of NAK timer expirations after which NAK_LIMIT_REACHED is declared.
+  uint32_t nakTimerExpirationLimit = 2;
+  //! If true, a NAK is issued as soon as a gap is detected. If false, the deferred procedure is
+  //! used and the NAK sequence is only issued once the EOF PDU has arrived. Deferred is the
+  //! default: on a lossy link the immediate procedure produces a burst of NAK PDUs in the
+  //! opposite direction exactly when the link is already struggling.
+  bool immediateNakMode = false;
+  //! Number of check timer expirations after EOF reception after which CHECK_LIMIT_REACHED is
+  //! declared and an incomplete transaction is cancelled instead of pinning the handler.
+  uint32_t checkLimit = 2;
+  //! Interval of the check timer, see checkLimit.
+  uint32_t checkTimerIntervalMs = 10000;
 };
 
 }  // namespace cfdp

src/fsfw/cfdp/pdu/AckPduCreator.h

@@ -23,6 +23,9 @@ class AckPduCreator : public FileDirectiveCreator {
   ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
                           Endianness streamEndianness) const override;
 
+  //! Un-hide the convenience overloads of the base class, same as FinishedPduCreator does.
+  using FileDirectiveCreator::serialize;
+
  private:
   AckInfo& ackInfo;
 };

src/fsfw/cfdp/pdu/FinishedPduReader.cpp

@@ -11,6 +11,15 @@ ReturnValue_t FinishPduReader::parseData() {
   size_t currentIdx = FileDirectiveReader::getHeaderSize();
   const uint8_t* buf = pointers.rawPtr + currentIdx;
   size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx;
+  // Drop the PDU CRC from the parsed range before parseTlvs below, which runs until the range is
+  // exhausted. Without this a Finished PDU carrying a CRC is rejected as an invalid TLV type -
+  // including the common NO_ERROR case, where the CRC is the only thing left after the first byte.
+  if (getCrcFlag()) {
+    if (remSize < 2) {
+      return SerializeIF::STREAM_TOO_SHORT;
+    }
+    remSize -= 2;
+  }
   if (remSize < 1) {
     return SerializeIF::STREAM_TOO_SHORT;
   }

src/fsfw/cfdp/pdu/KeepAlivePduCreator.h

@@ -14,6 +14,7 @@ class KeepAlivePduCreator : public FileDirectiveCreator {
 
   ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
                           Endianness streamEndianness) const override;
+  using FileDirectiveCreator::serialize;
 
  private:
   cfdp::Fss& progress;

src/fsfw/cfdp/pdu/MetadataPduReader.cpp

@@ -17,6 +17,15 @@ ReturnValue_t MetadataPduReader::parseData() {
   size_t currentIdx = FileDirectiveReader::getHeaderSize();
   const uint8_t* buf = pointers.rawPtr + currentIdx;
   size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx;
+  // The PDU CRC occupies the last two bytes of the PDU. Take it out of the parsed range up front,
+  // like FileDataReader does: the option loop below consumes bytes until the range is exhausted,
+  // so a CRC left in place would be deserialized as another TLV and rejected as an invalid type.
+  if (getCrcFlag()) {
+    if (remSize < 2) {
+      return SerializeIF::STREAM_TOO_SHORT;
+    }
+    remSize -= 2;
+  }
   if (remSize < 1) {
     return SerializeIF::STREAM_TOO_SHORT;
   }
@@ -38,17 +47,13 @@ ReturnValue_t MetadataPduReader::parseData() {
     return result;
   }
 
-  if (getCrcFlag() && remSize == 2) {
-    return returnvalue::OK;
-  }
-
   if (remSize > 0) {
     if (optionArrayMaxSize == 0 or optionArray == nullptr) {
       return cfdp::METADATA_CANT_PARSE_OPTIONS;
     }
     size_t optsIdx = 0;
     while (remSize > 0) {
-      if (optsIdx > optionArrayMaxSize) {
+      if (optsIdx >= optionArrayMaxSize) {
         return cfdp::METADATA_CANT_PARSE_OPTIONS;
       }
       result = optionArray[optsIdx].deSerialize(&buf, &remSize, endianness);

src/fsfw/cfdp/pdu/NakPduCreator.h

@@ -25,6 +25,9 @@ class NakPduCreator : public FileDirectiveCreator {
   ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
                           Endianness streamEndianness) const override;
 
+  //! Un-hide the convenience overloads of the base class, same as FinishedPduCreator does.
+  using FileDirectiveCreator::serialize;
+
   /**
    * If you change the info struct, you might need to update the directive field length
    * manually

src/fsfw/cfdp/pdu/NakPduReader.cpp

@@ -11,6 +11,14 @@ ReturnValue_t NakPduReader::parseData() {
   size_t currentIdx = FileDirectiveReader::getHeaderSize();
   const uint8_t* buffer = pointers.rawPtr + currentIdx;
   size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx;
+  // Drop the PDU CRC from the parsed range before the segment request loop below, which runs
+  // until the range is exhausted and would otherwise read the CRC as a truncated segment request.
+  if (getCrcFlag()) {
+    if (remSize < 2) {
+      return SerializeIF::STREAM_TOO_SHORT;
+    }
+    remSize -= 2;
+  }
   if (remSize < 1) {
     return SerializeIF::STREAM_TOO_SHORT;
   }
@@ -34,17 +42,24 @@ ReturnValue_t NakPduReader::parseData() {
     if (segReqs != nullptr) {
       size_t idx = 0;
       while (remSize > 0) {
+        // Every early return below reports the number of *complete* segment requests written so
+        // far. Leaving the length at 0 would make a partially parsed NAK indistinguishable from
+        // an empty one, and a caller which tolerates the error code would then act on nothing.
         if (idx == maxSegReqs) {
+          nakInfo.setSegmentRequestLen(idx);
           return cfdp::NAK_CANT_PARSE_OPTIONS;
         }
         result =
             segReqs[idx].first.deSerialize(&buffer, &remSize, SerializeIF::Endianness::NETWORK);
         if (result != returnvalue::OK) {
+          nakInfo.setSegmentRequestLen(idx);
           return result;
         }
         result =
             segReqs[idx].second.deSerialize(&buffer, &remSize, SerializeIF::Endianness::NETWORK);
         if (result != returnvalue::OK) {
+          // The entry at idx is half written, so it is not counted.
+          nakInfo.setSegmentRequestLen(idx);
           return result;
         }
         idx++;

src/fsfw/devicehandlers/AssemblyBase.cpp

@@ -71,7 +71,8 @@ bool AssemblyBase::handleChildrenChangedHealth() {
   if (iter == childrenMap.end()) {
     return false;
   }
-  HealthState healthState = healthHelper.healthTable->getHealth(iter->first);
+  HealthState healthState =
+      healthHelper.healthTable->getHealth(convertToDeviceObjectId(iter->first));
   if (healthState == HasHealthIF::NEEDS_RECOVERY) {
     triggerEvent(TRYING_RECOVERY, iter->first, 0);
     recoveryState = RECOVERY_STARTED;
@@ -91,10 +92,14 @@ bool AssemblyBase::handleChildrenChangedHealth() {
 void AssemblyBase::handleChildrenTransition() {
   if (commandsOutstanding <= 0) {
     switch (internalState) {
-      case STATE_NEED_SECOND_STEP:
+      case STATE_NEED_SECOND_STEP: {
         internalState = STATE_SECOND_STEP;
-        commandChildren(targetMode, targetSubmode);
+        ReturnValue_t result = commandChildren(targetMode, targetSubmode);
+        if (result == NEED_SECOND_STEP) {
+          internalState = STATE_NEED_SECOND_STEP;
+        }
         return;
+      }
       case STATE_OVERWRITE_HEALTH: {
         internalState = STATE_SINGLE_STEP;
         ReturnValue_t result = commandChildren(mode, submode);
@@ -170,7 +175,7 @@ ReturnValue_t AssemblyBase::checkChildrenStateOff() {
 
 ReturnValue_t AssemblyBase::checkChildOff(uint32_t objectId) {
   ChildInfo childInfo = childrenMap.find(objectId)->second;
-  if (healthHelper.healthTable->isCommandable(objectId)) {
+  if (healthHelper.healthTable->isCommandable(convertToDeviceObjectId(objectId))) {
     if (childInfo.submode != SUBMODE_NONE) {
       return returnvalue::FAILED;
     } else {
@@ -227,7 +232,7 @@ bool AssemblyBase::checkAndHandleRecovery() {
     case RECOVERY_STARTED:
       // The recovery was already start in #handleChildrenChangedHealth and we just need
       // to wait for an off time period.
-      // TODO: make time period configurable
+      // The timeout can be defined by #setRecoveryWaitTimer
       recoveryState = RECOVERY_WAIT;
       recoveryOffTimer.resetTimer();
       return true;
@@ -235,14 +240,14 @@ bool AssemblyBase::checkAndHandleRecovery() {
       if (recoveryOffTimer.isBusy()) {
         return true;
       }
-      triggerEvent(RECOVERY_STEP, 0);
+      triggerEvent(RECOVERY_WAITING, recoveringDevice->first);
       sendHealthCommand(recoveringDevice->second.commandQueue, HEALTHY);
       internalState = STATE_NONE;
       recoveryState = RECOVERY_ONGOING;
       // Don't check state!
       return true;
     case RECOVERY_ONGOING:
-      triggerEvent(RECOVERY_STEP, 1);
+      triggerEvent(RECOVERY_RESTARTING, recoveringDevice->first);
       recoveryState = RECOVERY_ONGOING_2;
       recoveringDevice->second.healthChanged = false;
       // Device should be healthy again, so restart a transition.
@@ -250,7 +255,7 @@ bool AssemblyBase::checkAndHandleRecovery() {
       doStartTransition(targetMode, targetSubmode);
       return true;
     case RECOVERY_ONGOING_2:
-      triggerEvent(RECOVERY_DONE);
+      triggerEvent(RECOVERY_DONE, recoveringDevice->first);
       // Now we're through, but not sure if it was successful.
       recoveryState = RECOVERY_IDLE;
       return false;
@@ -264,7 +269,14 @@ void AssemblyBase::overwriteDeviceHealth(object_id_t objectId, HasHealthIF::Heal
   triggerEvent(OVERWRITING_HEALTH, objectId, oldHealth);
   internalState = STATE_OVERWRITE_HEALTH;
   modeHelper.setForced(true);
-  sendHealthCommand(childrenMap[objectId].commandQueue, EXTERNAL_CONTROL);
+  if (childrenMap.find(objectId) != childrenMap.end()) {
+    sendHealthCommand(childrenMap.at(objectId).commandQueue, EXTERNAL_CONTROL);
+  } else {
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    sif::debug << std::hex << SystemObject::getObjectId() << ": invalid mode table entry"
+               << std::endl;
+#endif
+  }
 }
 
 void AssemblyBase::triggerModeHelperEvents(Mode_t mode, Submode_t submode) {
@@ -274,3 +286,7 @@ void AssemblyBase::triggerModeHelperEvents(Mode_t mode, Submode_t submode) {
     triggerEvent(CHANGING_MODE, mode, submode);
   }
 }
+
+void AssemblyBase::setRecoveryWaitTimer(uint32_t timeoutMS) {
+  recoveryOffTimer.setTimeout(timeoutMS);
+}

src/fsfw/devicehandlers/AssemblyBase.h

@@ -206,6 +206,8 @@ class AssemblyBase : public SubsystemBase {
   void overwriteDeviceHealth(object_id_t objectId, HasHealthIF::HealthState oldHealth);
 
   void triggerModeHelperEvents(Mode_t mode, Submode_t submode);
+
+  void setRecoveryWaitTimer(uint32_t timeoutMS);
 };
 
 #endif /* FSFW_DEVICEHANDLERS_ASSEMBLYBASE_H_ */

src/fsfw/devicehandlers/DeviceHandlerBase.cpp

@@ -1268,7 +1268,7 @@ ReturnValue_t DeviceHandlerBase::letChildHandleMessage(CommandMessage* message)
 
 void DeviceHandlerBase::handleDeviceTm(const uint8_t* rawData, size_t rawDataLen,
                                        DeviceCommandId_t replyId, bool forceDirectTm) {
-  SerialBufferAdapter bufferWrapper(rawData, rawDataLen);
+  SerialBufferAdapter<uint32_t> bufferWrapper(rawData, rawDataLen);
   handleDeviceTm(bufferWrapper, replyId, forceDirectTm);
 }
 

src/fsfw/events/Event.h

@@ -14,8 +14,6 @@ enum Severity : EventSeverity_t { INFO = 1, LOW = 2, MEDIUM = 3, HIGH = 4 };
 
 }  // namespace severity
 
-#define MAKE_EVENT(id, severity) (((severity) << 16) + (SUBSYSTEM_ID * 100) + (id))
-
 typedef uint32_t Event;
 
 namespace event {
@@ -24,11 +22,14 @@ constexpr EventId_t getEventId(Event event) { return (event & 0xFFFF); }
 
 constexpr EventSeverity_t getSeverity(Event event) { return ((event >> 16) & 0xFF); }
 
-constexpr Event makeEvent(uint8_t subsystemId, UniqueEventId_t uniqueEventId,
-                          EventSeverity_t eventSeverity) {
+template <uint8_t subsystemId, UniqueEventId_t uniqueEventId, EventSeverity_t eventSeverity>
+constexpr Event makeEvent() {
+  static_assert(uniqueEventId < 100, "The unique event ID must be smaller than 100!");
   return (eventSeverity << 16) + (subsystemId * 100) + uniqueEventId;
 }
 
 }  // namespace event
 
+#define MAKE_EVENT(id, severity) event::makeEvent<SUBSYSTEM_ID, id, severity>();
+
 #endif /* EVENTOBJECT_EVENT_H_ */

src/fsfw/globalfunctions/CMakeLists.txt

@@ -2,6 +2,7 @@ target_sources(
   ${LIB_FSFW_NAME}
   PRIVATE arrayprinter.cpp
           AsciiConverter.cpp
+          CobsEncoder.cpp
           CRC.cpp
           DleEncoder.cpp
           DleParser.cpp

src/fsfw/globalfunctions/CobsEncoder.cpp

@@ -0,0 +1,78 @@
+#include "fsfw/globalfunctions/CobsEncoder.h"
+
+#include <cstddef>
+#include <cstring>
+
+ReturnValue_t CobsEncoder::encode(const uint8_t* sourceStream, size_t sourceLen,
+                                  uint8_t* destStream, size_t maxDestLen, size_t* encodedLen) {
+  *encodedLen = 0;
+
+  if (maxDestLen < worstCaseEncodedLen(sourceLen)) return INSUFFICIENT_SPACE;
+
+  size_t codeIdx = 0;
+  size_t outIdx = 1;
+  uint8_t code = 1;
+
+  for (size_t i = 0; i < sourceLen; ++i) {
+    const auto isZero = sourceStream[i] == 0x00;
+
+    if (not isZero) {
+      destStream[outIdx++] = sourceStream[i];
+      code += 1;
+    }
+
+    if (isZero or code == CobsEncoder::MAX_BLOCK_LEN + 1) {  // + delimiter
+      destStream[codeIdx] = code;
+      codeIdx = outIdx++;
+      code = 1;
+    }
+  }
+
+  destStream[codeIdx] = code;
+  destStream[outIdx++] = 0x00;
+
+  *encodedLen = outIdx;
+  return returnvalue::OK;
+}
+
+ReturnValue_t CobsEncoder::decode(const uint8_t* sourceStream, size_t sourceLen, size_t* readLen,
+                                  uint8_t* destStream, size_t maxDestLen, size_t* decodedLen) {
+  *readLen = 0;
+  *decodedLen = 0;
+
+  if (sourceLen == 0) return NO_DATA_AVAILABLE;
+
+  // COBS, so 0 byte never occures. Used as delimiters.
+  const auto* delimiter = static_cast<const uint8_t*>(std::memchr(sourceStream, 0x00, sourceLen));
+  if (delimiter == nullptr) return STREAM_TOO_SHORT;
+
+  const size_t frameLen = delimiter - sourceStream;
+  const size_t consumedLen = frameLen + 1;  // + delimiter
+
+  size_t inIdx = 0;
+  size_t outIdx = 0;
+
+  while (inIdx < frameLen) {
+    const size_t blockLen = sourceStream[inIdx++] - 1;  // COBS, so for all bytes > 0
+    const auto isFullBlock = blockLen == MAX_BLOCK_LEN;
+
+    // COBS data block is bogus => bad data (set readLen != 0)
+    if (inIdx + blockLen > frameLen) return *readLen = consumedLen, DECODING_ERROR;
+    // Data block won't fit into provided buffer
+    if (maxDestLen < outIdx + blockLen) return INSUFFICIENT_SPACE;
+
+    std::memcpy(destStream + outIdx, sourceStream + inIdx, blockLen);
+    inIdx += blockLen;
+    outIdx += blockLen;
+
+    // COBS: != 0xFF, then meant to be zero
+    if (not isFullBlock and inIdx < frameLen) {
+      if (maxDestLen < outIdx + 1) return INSUFFICIENT_SPACE;
+      destStream[outIdx++] = 0x00;
+    }
+  }
+
+  *readLen = consumedLen;
+  *decodedLen = outIdx;
+  return returnvalue::OK;
+}

src/fsfw/globalfunctions/CobsEncoder.h

@@ -0,0 +1,113 @@
+#ifndef FSFW_GLOBALFUNCTIONS_COBSENCODER_H_
+#define FSFW_GLOBALFUNCTIONS_COBSENCODER_H_
+
+#include <cstddef>
+#include <cstdint>
+
+#include "fsfw/returnvalues/returnvalue.h"
+
+/**
+ * @brief   This COBS Encoder (Consistent Overhead Byte Stuffing) can be used to encode and
+ *          decode arbitrary data.
+ *
+ * @details
+ * Protocol information: https://en.wikipedia.org/wiki/Consistent_Overhead_Byte_Stuffing
+ *
+ * A COBS frame contains no zero bytes but is terminated by one, so frames can be picked out of a
+ * byte stream by looking for that delimiter.
+ */
+class CobsEncoder {
+ public:
+  CobsEncoder() = delete;
+  virtual ~CobsEncoder() = delete;
+
+  static constexpr uint8_t INTERFACE_ID = CLASS_ID::COBS_ENCODER;
+
+  /** The source stream holds no frame delimiter yet, so the frame may still be arriving.
+   * Nothing was consumed and the caller should retry once more data has been received. */
+  static constexpr ReturnValue_t STREAM_TOO_SHORT = MAKE_RETURN_CODE(1);
+  /** The frame is delimited but its block structure is malformed, so it cannot be recovered.
+   * `readLen` skips past the whole frame so decoding can resynchronise on the next one. */
+  static constexpr ReturnValue_t DECODING_ERROR = MAKE_RETURN_CODE(2);
+  /** The input stream is empty. */
+  static constexpr ReturnValue_t NO_DATA_AVAILABLE = MAKE_RETURN_CODE(3);
+  /** The output buffer is not large enough to fit the input data. */
+  static constexpr ReturnValue_t INSUFFICIENT_SPACE = MAKE_RETURN_CODE(4);
+
+  /** Longest run of data bytes a single code byte can describe. */
+  static constexpr size_t MAX_BLOCK_LEN = 254;
+
+  /**
+   * Upper bound on the encoded size of `sourceLen` bytes, including the frame delimiter.
+   * Exact when the input holds no zero bytes and at most one byte per full block too large
+   * otherwise.
+   * Use this during compiletime to create a correctly sized output buffer.
+   * @param sourceLen Max length of buffer to encode
+   * @return Use as follows: `uint8_t destBuffer[worstCaseEncodedLen(srcBufferSize)];`.
+   */
+  static constexpr size_t worstCaseEncodedLen(size_t sourceLen) {
+    // Derivation:
+    // We know: Frame length = (data bytes) + (code bytes) + (delimiter)
+    // Let n be the source length, z the number of zero bytes in it and s the number of times a data
+    // block is over 254 and splits.
+    // We have (data bytes) = n - z, since only non-zero bytes get copied over.
+    // (code bytes) = 1 + z + s, since starting code, blocks end at zero and 254 splits.
+    // So, Frame length = (data bytes) + (code bytes) + (delimiter)
+    //                  = (n - z) + (1 + z + s) + 1 = n + s + 2
+    // s depends on the data, so we derive the upper bound, since we know these splits happen every
+    // 254 bytes (or less since maybe there are enough zeros spread out to not need a 254 split):
+    // s <= (n - z) / 254 <= n / 254. (z >= 0)
+    // Thus, n + s + 2 <= n + (n / 254) + 2
+    // Note: We do integer division (floor), since a 254 code only appears every FULL 254.
+    return sourceLen + sourceLen / MAX_BLOCK_LEN + 2;
+  }
+
+  /**
+   * Encodes the given data stream into COBS format
+   * @param sourceStream Start of the source buffer
+   * @param sourceLen Length of the source buffer
+   * @param destStream Destination buffer
+   * @param maxDestLen Maximum length of the destination buffer
+   * @param encodedLen Out pointer which is written with the actual amount of data written to the
+   * destination buffer.
+   * @return
+   *  - returnvalue::OK for successful encoding operation
+   *  - INSUFFICIENT_SPACE if `maxDestLen` is below `worstCaseEncodedLen(sourceLen)`.
+   *    Note: Technically smaller sizes would fit but `worstCaseEncodedLen` is easy to compute.
+   */
+  static ReturnValue_t encode(const uint8_t* sourceStream, size_t sourceLen, uint8_t* destStream,
+                              size_t maxDestLen, size_t* encodedLen);
+
+  /**
+   * Converts an encoded stream back from COBS format.
+   *
+   * This function only ever decodes a single COBS frame.
+   * To drain a stream, advance it by `readLen` and call again until `STREAM_TOO_SHORT` reports that
+   * no complete frame is left.
+   * Because `readLen` is also set for a corrupt frame, a damaged frame never blocks the intact ones
+   * queued behind it.
+   *
+   * An empty frame, meaning a delimiter with no data in front of it, decodes successfully with a
+   * `decodedLen` of zero.
+   * Callers that treat runs of delimiters as idle filler should ignore those.
+   * @param sourceStream Start of the source buffer
+   * @param sourceStreamLen Length of the source buffer
+   * @param readLen Out pointer which is written with the amount of data that was actually read from
+   *        the source buffer. Set on success and on DECODING_ERROR, both times covering the whole
+   *        frame including its delimiter. Left at zero otherwise.
+   * @param destStream Destination buffer
+   * @param maxDestStreamlen Maximum length of the destination buffer
+   * @param decodedLen Out pointer which is written with the actual amount of data written to the
+   *        destination buffer.
+   * @return
+   *  - returnvalue::OK for successful decode operation
+   *  - STREAM_TOO_SHORT if the source stream holds no complete frame yet
+   *  - DECODING_ERROR if the frame is delimited but malformed. Skip `readLen` bytes to resync
+   *  - INSUFFICIENT_SPACE if the destination buffer cannot hold the decoded frame
+   *  - NO_DATA_AVAILABLE if the source stream is empty
+   */
+  static ReturnValue_t decode(const uint8_t* sourceStream, size_t sourceStreamLen, size_t* readLen,
+                              uint8_t* destStream, size_t maxDestStreamlen, size_t* decodedLen);
+};
+
+#endif /* FSFW_GLOBALFUNCTIONS_COBSENCODER_H_ */

src/fsfw/globalfunctions/arrayprinter.cpp

@@ -56,26 +56,30 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, size_t maxCharPerL
   std::cout << std::dec << std::setfill(' ');
   std::cout << "]" << std::endl;
 #else
-  // General format: 0x01, 0x02, 0x03 so it is number of chars times 6
-  // plus line break plus small safety margin.
-  char printBuffer[(size + 1) * 7 + 1] = {};
+#if FSFW_DISABLE_PRINTOUT == 0
+  // Emitted in fixed size chunks instead of one stack buffer sized from the input. Dumping a
+  // 12 KB receive buffer would otherwise need 84 KB of stack.
+  constexpr size_t CHUNK_LEN = 128;
+  // An entry appends at most two hex digits, a separator and a line break.
+  constexpr size_t MAX_ENTRY_LEN = 4;
+  char printBuffer[CHUNK_LEN] = {};
   size_t currentPos = 0;
+  printf("hex [");
   for (size_t i = 0; i < size; i++) {
-    // To avoid buffer overflows.
-    if (sizeof(printBuffer) - currentPos <= 7) {
-      break;
+    if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
+      printf("%s", printBuffer);
+      printBuffer[0] = '\0';
+      currentPos = 0;
     }
-
-    currentPos += snprintf(printBuffer + currentPos, 6, "%02x", data[i]);
+    currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%02x", data[i]);
     if (i < size - 1) {
-      currentPos += sprintf(printBuffer + currentPos, ",");
+      currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
       if ((i + 1) % maxCharPerLine == 0) {
-        currentPos += sprintf(printBuffer + currentPos, "\n");
+        currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
       }
     }
   }
-#if FSFW_DISABLE_PRINTOUT == 0
-  printf("hex [%s]\n", printBuffer);
+  printf("%s]\n", printBuffer);
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 #endif
 }
@@ -98,27 +102,29 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, size_t maxCharPerL
   }
   std::cout << "]" << std::endl;
 #else
-  // General format: 32,243,-12 so it is number of chars times 4
-  // plus line break plus small safety margin.
-  uint16_t expectedLines = ceil((double)size / maxCharPerLine);
-  char printBuffer[size * 4 + 1 + expectedLines] = {};
+#if FSFW_DISABLE_PRINTOUT == 0
+  // Chunked for the same reason as printHex above.
+  constexpr size_t CHUNK_LEN = 128;
+  // An entry appends at most three digits, a separator and a line break.
+  constexpr size_t MAX_ENTRY_LEN = 5;
+  char printBuffer[CHUNK_LEN] = {};
   size_t currentPos = 0;
+  printf("dec [");
   for (size_t i = 0; i < size; i++) {
-    // To avoid buffer overflows.
-    if (sizeof(printBuffer) - currentPos <= 4) {
-      break;
+    if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
+      printf("%s", printBuffer);
+      printBuffer[0] = '\0';
+      currentPos = 0;
     }
-
-    currentPos += snprintf(printBuffer + currentPos, 4, "%d", data[i]);
+    currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%d", data[i]);
     if (i < size - 1) {
-      currentPos += sprintf(printBuffer + currentPos, ",");
+      currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
       if ((i + 1) % maxCharPerLine == 0) {
-        currentPos += sprintf(printBuffer + currentPos, "\n");
+        currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
       }
     }
   }
-#if FSFW_DISABLE_PRINTOUT == 0
-  printf("dec [%s]\n", printBuffer);
+  printf("%s]\n", printBuffer);
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 #endif
 }

src/fsfw/health/HasHealthIF.h

@@ -27,13 +27,19 @@ class HasHealthIF {
   static const Event CHILD_PROBLEMS = MAKE_EVENT(8, severity::LOW);
   //! Assembly overwrites health information of children to keep satellite alive.
   static const Event OVERWRITING_HEALTH = MAKE_EVENT(9, severity::LOW);
-  //! Someone starts a recovery of a component (typically power-cycle). No parameters.
+  //! Someone starts a recovery of a component (typically power-cycle).
+  //! P1: Object Id of the recovering device.
   static const Event TRYING_RECOVERY = MAKE_EVENT(10, severity::MEDIUM);
-  //! Recovery is ongoing. Comes twice during recovery.
-  //! P1: 0 for the first, 1 for the second event. P2: 0
-  static const Event RECOVERY_STEP = MAKE_EVENT(11, severity::MEDIUM);
   //! Recovery was completed. Not necessarily successful. No parameters.
+  //! P1: Object Id of the recovering device.
   static const Event RECOVERY_DONE = MAKE_EVENT(12, severity::MEDIUM);
+  //! Recovery is ongoing. The recovering device is currently OFF, waiting for restart.
+  //! P1: Object Id of the recovering device.
+  static const Event RECOVERY_WAITING = MAKE_EVENT(13, severity::MEDIUM);
+  //! Recovery is ongoing. Restarting the recovering device.
+  //! P1: Object Id of the recovering device.
+  static const Event RECOVERY_RESTARTING = MAKE_EVENT(14, severity::MEDIUM);
+
   virtual ~HasHealthIF() {}
 
   virtual MessageQueueId_t getCommandQueue() const = 0;

src/fsfw/health/HealthTable.cpp

@@ -17,11 +17,11 @@ void HealthTable::setMutexTimeout(MutexIF::TimeoutType timeoutType, uint32_t tim
 HealthTable::~HealthTable() { MutexFactory::instance()->deleteMutex(mutex); }
 
 ReturnValue_t HealthTable::registerObject(object_id_t object,
-                                          HasHealthIF::HealthState initilialState) {
+                                          HasHealthIF::HealthState initialState) {
   if (healthMap.count(object) != 0) {
     return returnvalue::FAILED;
   }
-  healthMap.emplace(object, initilialState);
+  healthMap.emplace(object, initialState);
   return returnvalue::OK;
 }
 
@@ -112,3 +112,5 @@ ReturnValue_t HealthTable::iterate(HealthEntry* value, bool reset) {
   mapIterator++;
   return result;
 }
+
+MutexIF* HealthTable::getMutex() { return mutex; }

src/fsfw/health/HealthTable.h

@@ -18,7 +18,7 @@ class HealthTable : public HealthTableIF, public SystemObject {
 
   /** HealthTableIF overrides */
   virtual ReturnValue_t registerObject(
-      object_id_t object, HasHealthIF::HealthState initilialState = HasHealthIF::HEALTHY) override;
+      object_id_t object, HasHealthIF::HealthState initialState = HasHealthIF::HEALTHY) override;
   ReturnValue_t removeObject(object_id_t object) override;
   virtual size_t getPrintSize() override;
   virtual void printAll(uint8_t* pointer, size_t maxSize) override;
@@ -28,6 +28,8 @@ class HealthTable : public HealthTableIF, public SystemObject {
   virtual void setHealth(object_id_t object, HasHealthIF::HealthState newState) override;
   virtual HasHealthIF::HealthState getHealth(object_id_t) override;
 
+  MutexIF* getMutex();
+
  protected:
   using HealthMap = std::map<object_id_t, HasHealthIF::HealthState>;
   using HealthEntry = std::pair<object_id_t, HasHealthIF::HealthState>;

src/fsfw/health/HealthTableIF.h

@@ -12,7 +12,7 @@ class HealthTableIF : public ManagesHealthIF {
   virtual ~HealthTableIF() {}
 
   virtual ReturnValue_t registerObject(
-      object_id_t object, HasHealthIF::HealthState initilialState = HasHealthIF::HEALTHY) = 0;
+      object_id_t object, HasHealthIF::HealthState initialState = HasHealthIF::HEALTHY) = 0;
 
   virtual ReturnValue_t removeObject(object_id_t objectId) = 0;
 

src/fsfw/housekeeping/Dataset.h

@@ -91,7 +91,7 @@ class Dataset : public SerializeIF {
 
   void setChildrenValidity(bool valid) {
     for (auto &serializable : serializables) {
-      serializable.get().setValid(true);
+      serializable.get().setValid(valid);
     }
   }
 
@@ -118,6 +118,35 @@ class Dataset : public SerializeIF {
     return result;
   }
 
+  /**
+   * Counterpart of doSerializeWithValidityBlob: reads the values, then sets the validity of each
+   * serializable from the validity blob. deSerialize ignores the blob.
+   */
+  ReturnValue_t deSerializeWithValidityBlob(const uint8_t **buffer, size_t *size,
+                                            const Endianness streamEndianness) {
+    const size_t validityMaskSize = (serializables.size() + 7) / 8;
+    size_t valuesSize = 0;
+    for (auto &serializable : serializables) {
+      valuesSize += serializable.get().getSerializedSize();
+    }
+    // Checked up front so a short stream leaves the dataset untouched
+    if (*size < valuesSize + validityMaskSize) {
+      return SerializeIF::STREAM_TOO_SHORT;
+    }
+    ReturnValue_t result = Dataset::deSerialize(buffer, size, streamEndianness);
+    if (result != returnvalue::OK) {
+      return result;
+    }
+    for (size_t idx = 0; idx < serializables.size(); idx++) {
+      bool valid = false;
+      bitutil::get(*buffer + idx / 8, idx % 8, valid);
+      serializables[idx].get().setValid(valid);
+    }
+    *buffer += validityMaskSize;
+    *size -= validityMaskSize;
+    return returnvalue::OK;
+  }
+
   bool serializeWithValidityBlob = false;
 
  private:

src/fsfw/housekeeping/PeriodicHkHelper.cpp

@@ -9,7 +9,6 @@
 #include "fsfw/housekeeping/HousekeepingSnapshot.h"
 #include "fsfw/ipc/QueueFactory.h"
 #include "fsfw/objectmanager/ObjectManager.h"
-#include "fsfw/timemanager/CCSDSTime.h"
 
 using namespace hk;
 
@@ -84,6 +83,7 @@ ReturnValue_t PeriodicHelper::performHkOperation() {
 
 ReturnValue_t PeriodicHelper::handleHousekeepingMessage(CommandMessage* message) {
   Command_t command = message->getCommand();
+  MessageQueueId_t sender = message->getSender();
   dp::sid_t sid = HousekeepingMessage::getStructureId(message);
   ReturnValue_t result = returnvalue::OK;
   switch (command) {
@@ -113,7 +113,7 @@ ReturnValue_t PeriodicHelper::handleHousekeepingMessage(CommandMessage* message)
     }
 
     case (HousekeepingMessage::GENERATE_ONE_PARAMETER_REPORT): {
-      return generateHousekeepingPacket(HousekeepingMessage::getStructureId(message));
+      return generateHousekeepingPacket(HousekeepingMessage::getStructureId(message), sender);
     }
 
     default:

src/fsfw/ipc/MessageQueueBase.cpp

@@ -45,7 +45,7 @@ bool MessageQueueBase::isDefaultDestinationSet() const { return (defaultDest !=
 
 ReturnValue_t MessageQueueBase::sendMessage(MessageQueueId_t sendTo, MessageQueueMessageIF* message,
                                             bool ignoreFault) {
-  return sendMessageFrom(sendTo, message, this->getId(), false);
+  return sendMessageFrom(sendTo, message, this->getId(), ignoreFault);
 }
 
 ReturnValue_t MessageQueueBase::sendToDefaultFrom(MessageQueueMessageIF* message,

src/fsfw/ipc/MutexGuard.h

@@ -47,7 +47,10 @@ class MutexGuard {
   ReturnValue_t getLockResult() const { return result; }
 
   ~MutexGuard() {
-    if (internalMutex != nullptr) {
+    // Only unlock what was actually locked. Unlocking after a failed take gives away a mutex held
+    // by another task, which trips configASSERT(pxTCB == pxCurrentTCB) in FreeRTOS'
+    // xTaskPriorityDisinherit and halts the system.
+    if (internalMutex != nullptr and result == returnvalue::OK) {
       internalMutex->unlockMutex();
     }
   }

src/fsfw/objectmanager/frameworkObjects.h

@@ -36,6 +36,8 @@ enum framework_objects : object_id_t {
   TIME_STAMPER = 0x53500010,
   VERIFICATION_REPORTER = 0x53500020,
 
+  SIF_PRINT_TASK = 0x53600000,
+
   FSFW_OBJECTS_END = 0x53ffffff,
   NO_OBJECT = 0xFFFFFFFF
 };

src/fsfw/osal/common/UdpTcPollingTask.cpp

@@ -41,7 +41,7 @@ UdpTcPollingTask::UdpTcPollingTask(object_id_t objectId, object_id_t tmtcUdpBrid
 
 [[noreturn]] ReturnValue_t UdpTcPollingTask::performOperation(uint8_t opCode) {
   /* Sender Address is cached here. */
-  struct sockaddr senderAddress {};
+  struct sockaddr senderAddress{};
   socklen_t senderAddressSize = sizeof(senderAddress);
 
   /* Poll for new UDP datagrams in permanent loop. */

src/fsfw/osal/freertos/Clock.cpp

@@ -5,8 +5,8 @@
 
 #include "FreeRTOS.h"
 #include "fsfw/globalfunctions/timevalOperations.h"
-#include "fsfw/serviceinterface/ServiceInterfacePrinter.h"
 #include "fsfw/osal/freertos/Timekeeper.h"
+#include "fsfw/serviceinterface/ServiceInterfacePrinter.h"
 #include "task.h"
 
 // TODO sanitize input?
@@ -48,7 +48,8 @@ ReturnValue_t Clock::getClock(timeval* time) {
 }
 
 ReturnValue_t Clock::getClockMonotonic(timeval* time) {
-	*time = Timekeeper::instance()->getMonotonicClockOffset() + getUptime();
+  // Retrieves the FreeRTOS tick count, which is hopefully monotonic
+  *time = getUptime();
   return returnvalue::OK;
 }
 

src/fsfw/osal/freertos/Timekeeper.cpp

@@ -17,13 +17,7 @@ Timekeeper* Timekeeper::instance() {
   return myinstance;
 }
 
-void Timekeeper::setOffset(const timeval& offset) {
-	if (not monotonicClockInitialized) {
-		this->monotonicClockOffset = offset;
-		monotonicClockInitialized = true;
-	}
-	this->offset = offset;
-}
+void Timekeeper::setOffset(const timeval& offset) { this->offset = offset; }
 
 timeval Timekeeper::ticksToTimeval(TickType_t ticks) {
   timeval uptime;
@@ -39,7 +33,3 @@ timeval Timekeeper::ticksToTimeval(TickType_t ticks) {
 }
 
 TickType_t Timekeeper::getTicks() { return xTaskGetTickCount(); }
-
-const timeval Timekeeper::getMonotonicClockOffset() const {
-	return monotonicClockOffset;
-}

src/fsfw/osal/freertos/Timekeeper.h

@@ -18,14 +18,9 @@ class Timekeeper {
   Timekeeper();
 
   timeval offset;
-  // Set when offset is initialized the first time
-  timeval monotonicClockOffset;
-  bool monotonicClockInitialized = false;
 
   static Timekeeper* myinstance;
 
-  void setMonotonicClockOffset(const timeval& monotonicClockOffset);
-
  public:
   static Timekeeper* instance();
   virtual ~Timekeeper();
@@ -39,7 +34,6 @@ class Timekeeper {
 
   const timeval& getOffset() const;
   void setOffset(const timeval& offset);
-  const timeval getMonotonicClockOffset() const;
 };
 
 #endif /* FRAMEWORK_OSAL_FREERTOS_TIMEKEEPER_H_ */

src/fsfw/osal/host/Clock.cpp

@@ -173,7 +173,7 @@ ReturnValue_t Clock::getDateAndTime(TimeOfDay_t* time) {
 }
 
 ReturnValue_t Clock::convertTimeOfDayToTimeval(const TimeOfDay_t* from, timeval* to) {
-  struct tm time_tm {};
+  struct tm time_tm{};
 
   time_tm.tm_year = from->year - 1900;
   time_tm.tm_mon = from->month - 1;

src/fsfw/osal/host/TaskFactory.cpp

@@ -52,6 +52,6 @@ void TaskFactory::printMissedDeadline() {
 #if FSFW_CPP_OSTREAM_ENABLED == 1
   sif::warning << "TaskFactory::printMissedDeadline: " << name << std::endl;
 #else
-  sif::printWarning("TaskFactory::printMissedDeadline: %s\n", name);
+  sif::printWarning("TaskFactory::printMissedDeadline: %s\n", name.c_str());
 #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */
 }

src/fsfw/pus/Service11TelecommandScheduling.h

@@ -49,7 +49,7 @@ class Service11TelecommandScheduling final : public PusServiceBase {
 
   //! [EXPORT] : [COMMENT] Deletion of a TC from the map failed.
   //! P1: First 32 bit of request ID, P2. Last 32 bit of Request ID
-  static constexpr Event TC_DELETION_FAILED = MAKE_EVENT(0, severity::MEDIUM);
+  static constexpr Event TC_DELETION_FAILED = event::makeEvent<SUBSYSTEM_ID, 0, severity::MEDIUM>();
 
   // The types of PUS-11 subservices
   enum Subservice : uint8_t {

src/fsfw/pus/Service8FunctionManagement.h

@@ -1,6 +1,8 @@
 #ifndef FSFW_PUS_SERVICE8FUNCTIONMANAGEMENT_H_
 #define FSFW_PUS_SERVICE8FUNCTIONMANAGEMENT_H_
 
+#include <cstdint>
+
 #include "fsfw/action/ActionMessage.h"
 #include "fsfw/tmtcservices/CommandingServiceBase.h"
 
@@ -35,6 +37,13 @@ class Service8FunctionManagement : public CommandingServiceBase {
                              uint16_t commandTimeoutSeconds = 60);
   ~Service8FunctionManagement() override;
 
+  enum class Subservice : uint8_t {
+    //!< [EXPORT] : [COMMAND] Functional commanding
+    COMMAND_DIRECT_COMMANDING = 128,
+    //!< [EXPORT] : [REPLY] Data reply
+    REPLY_DIRECT_COMMANDING_DATA = 130,
+  };
+
  protected:
   /* CSB abstract functions implementation . See CSB documentation. */
   ReturnValue_t isValidSubservice(uint8_t subservice) override;
@@ -48,13 +57,6 @@ class Service8FunctionManagement : public CommandingServiceBase {
                             bool* isStep) override;
 
  private:
-  enum class Subservice {
-    //!< [EXPORT] : [COMMAND] Functional commanding
-    COMMAND_DIRECT_COMMANDING = 128,
-    //!< [EXPORT] : [REPLY] Data reply
-    REPLY_DIRECT_COMMANDING_DATA = 130,
-  };
-
   ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t* messageQueueToSet,
                                                      object_id_t* objectId);
   ReturnValue_t prepareDirectCommand(CommandMessage* message, const uint8_t* tcData,

src/fsfw/pus/servicepackets/Service8Packets.h

@@ -1,11 +1,14 @@
 #ifndef FSFW_PUS_SERVICEPACKETS_SERVICE8PACKETS_H_
 #define FSFW_PUS_SERVICEPACKETS_SERVICE8PACKETS_H_
 
+#include <cstdint>
+
 #include "../../action/ActionMessage.h"
 #include "../../objectmanager/SystemObjectIF.h"
+#include "../../returnvalues/returnvalue.h"
 #include "../../serialize/SerialBufferAdapter.h"
-#include "../../serialize/SerialFixedArrayListAdapter.h"
 #include "../../serialize/SerialLinkedListAdapter.h"
+#include "../../serialize/SerializeAdapter.h"
 #include "../../serialize/SerializeElement.h"
 
 /**
@@ -22,14 +25,41 @@ class DirectCommand
     parametersSize = size;
   }
 
+  DirectCommand() : parametersSize(0), parameterBuffer(nullptr) {}
+
   ActionId_t getActionId() const { return actionId; }
+  void setActionId(ActionId_t actionId) { this->actionId = actionId; }
 
   object_id_t getObjectId() const { return objectId; }
+  void setObjectId(object_id_t objectId) { this->objectId = objectId; }
 
   const uint8_t* getParameters() { return parameterBuffer; }
+  // The given pointer is not deallocated and must outlive the DirectCommand!
+  void setParameters(const uint8_t* parameters, uint32_t parametersSize) {
+    this->parameterBuffer = parameters;
+    this->parametersSize = parametersSize;
+  }
 
   uint32_t getParametersSize() const { return parametersSize; }
 
+  // ^SerializeIF
+  virtual ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
+                                  Endianness streamEndianness) const override {
+    auto const oldSize = *size;
+    auto result = SerializeAdapter::serialize(&objectId, buffer, size, maxSize, streamEndianness);
+    if (result != returnvalue::OK) return result;
+    result = SerializeAdapter::serialize(&actionId, buffer, size, maxSize - ((*size) - oldSize),
+                                         streamEndianness);
+    if (result != returnvalue::OK) return result;
+
+    auto remainingSize = maxSize - ((*size) - oldSize);
+    if (remainingSize < parametersSize) return returnvalue::FAILED;
+    memcpy(*buffer, parameterBuffer, parametersSize);
+    *size += parametersSize;
+
+    return returnvalue::OK;
+  }
+
  private:
   DirectCommand(const DirectCommand& command);
   object_id_t objectId = 0;

src/fsfw/returnvalues/FwClassIds.h

@@ -85,6 +85,7 @@ enum : uint8_t {
   MGM_LIS3MDL,                    // MGMLIS3
   MGM_RM3100,                     // MGMRM3100
   SPACE_PACKET_PARSER,            // SPPA
+  COBS_ENCODER,                   // COBS
   FW_CLASS_ID_COUNT               // [EXPORT] : [END]
 
 };

src/fsfw/serviceinterface/CMakeLists.txt

@@ -1,4 +1,4 @@
 target_sources(
   ${LIB_FSFW_NAME}
   PRIVATE ServiceInterfaceStream.cpp ServiceInterfaceBuffer.cpp
-          ServiceInterfacePrinter.cpp)
+          ServiceInterfacePrinter.cpp ServiceInterfacePrinterTask.cpp)

src/fsfw/serviceinterface/ServiceInterfacePrinter.cpp

@@ -1,9 +1,12 @@
 #include "fsfw/serviceinterface/ServiceInterfacePrinter.h"
 
+#include <algorithm>
 #include <cstdarg>
-#include <cstdint>
+#include <cstring>
 
 #include "fsfw/FSFW.h"
+#include "fsfw/ipc/MutexFactory.h"
+#include "fsfw/ipc/MutexGuard.h"
 #include "fsfw/serviceinterface/serviceInterfaceDefintions.h"
 #include "fsfw/timemanager/Clock.h"
 
@@ -14,11 +17,32 @@ static bool consoleInitialized = false;
 
 #if FSFW_DISABLE_PRINTOUT == 0
 
-static bool addCrAtEnd = false;
+namespace {
 
-uint8_t printBuffer[fsfwconfig::FSFW_PRINT_BUFFER_SIZE];
+// Formatted messages are staged in a ring buffer and drained by the print task
+// in bounded chunks, so producers never block on the slow debug UART.
+constexpr size_t RING_SIZE =
+    fsfwconfig::FSFW_PRINT_BUFFER_SIZE * fsfwconfig::FSFW_PRINT_BUFFER_AMOUNT;
 
-void fsfwPrint(sif::PrintLevel printType, const char *fmt, va_list arg) {
+constexpr size_t MAX_BYTES_PER_CYCLE = 512;
+
+// The print ring buffer
+char ring[RING_SIZE];
+MutexIF* ringMutex = nullptr;
+
+size_t readIdx = 0;    // Start of data to print out
+size_t bytesUsed = 0;  // Pending data amount to print out
+uint32_t droppedMessages = 0;
+uint32_t droppedMessagesTotal = 0;
+
+bool addCrAtEnd = false;
+bool replaceLastCharWithNewline = false;
+
+// False until the print task runs.
+// Prints go directly to stdout before that.
+bool taskRunning = false;
+
+void fsfwPrint(sif::PrintLevel printType, const char* fmt, va_list arg) {
 #if defined(WIN32) && FSFW_COLORED_OUTPUT == 1
   if (not consoleInitialized) {
     HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
@@ -30,100 +54,156 @@ void fsfwPrint(sif::PrintLevel printType, const char *fmt, va_list arg) {
   consoleInitialized = true;
 #endif
 
-  size_t len = 0;
-  char *bufferPosition = reinterpret_cast<char *>(printBuffer);
-
-  /* Check logger level */
   if (printType == sif::PrintLevel::NONE or printType > printLevel) {
     return;
   }
 
-  /* Log message to terminal */
-
+  static const char* const labels[] = {"", "ERROR  ", "WARNING", "INFO   ", "DEBUG  "};
 #if FSFW_COLORED_OUTPUT == 1
-  if (printType == sif::PrintLevel::INFO_LEVEL) {
-    len += sprintf(bufferPosition, sif::ANSI_COLOR_GREEN);
-  } else if (printType == sif::PrintLevel::DEBUG_LEVEL) {
-    len += sprintf(bufferPosition, sif::ANSI_COLOR_CYAN);
-  } else if (printType == sif::PrintLevel::WARNING_LEVEL) {
-    len += sprintf(bufferPosition, sif::ANSI_COLOR_YELLOW);
-  } else if (printType == sif::PrintLevel::ERROR_LEVEL) {
-    len += sprintf(bufferPosition, sif::ANSI_COLOR_RED);
-  }
-#endif
-
-  if (printType == sif::PrintLevel::INFO_LEVEL) {
-    len += sprintf(bufferPosition + len, "INFO   ");
-  }
-  if (printType == sif::PrintLevel::DEBUG_LEVEL) {
-    len += sprintf(bufferPosition + len, "DEBUG  ");
-  }
-  if (printType == sif::PrintLevel::WARNING_LEVEL) {
-    len += sprintf(bufferPosition + len, "WARNING");
-  }
-
-  if (printType == sif::PrintLevel::ERROR_LEVEL) {
-    len += sprintf(bufferPosition + len, "ERROR  ");
-  }
-
-#if FSFW_COLORED_OUTPUT == 1
-  len += sprintf(bufferPosition + len, sif::ANSI_COLOR_RESET);
+  static const char* const colors[] = {"", sif::ANSI_COLOR_RED, sif::ANSI_COLOR_YELLOW,
+                                       sif::ANSI_COLOR_GREEN, sif::ANSI_COLOR_CYAN};
+  const char* color = colors[printType];
+  const char* reset = sif::ANSI_COLOR_RESET;
+#else
+  const char* color = "";
+  const char* reset = "";
 #endif
 
   Clock::TimeOfDay_t now;
   Clock::getDateAndTime(&now);
-  /*
-   * Log current time to terminal if desired.
-   */
-  len += sprintf(bufferPosition + len, " | %02lu:%02lu:%02lu.%03lu | ", (unsigned long)now.hour,
-                 (unsigned long)now.minute, (unsigned long)now.second,
-                 (unsigned long)now.usecond / 1000);
 
-  len += vsnprintf(bufferPosition + len, sizeof(printBuffer) - len, fmt, arg);
+  char buf[fsfwconfig::FSFW_PRINT_BUFFER_SIZE + 2];  // slack for the CR/newline
 
-  if (addCrAtEnd) {
-    len += sprintf(bufferPosition + len, "\r");
+  // Need to clamp, since snprintf returns WOULD be written length (not actual; could be higher than
+  // really)
+  const auto prefixLen =
+      std::min(static_cast<size_t>(snprintf(
+                   buf, fsfwconfig::FSFW_PRINT_BUFFER_SIZE, "%s%s%s | %02u:%02u:%02u.%03u | ",
+                   color, labels[printType], reset, static_cast<unsigned>(now.hour),
+                   static_cast<unsigned>(now.minute), static_cast<unsigned>(now.second),
+                   static_cast<unsigned>(now.usecond / 1000))),
+               fsfwconfig::FSFW_PRINT_BUFFER_SIZE - 1);
+
+  // Same here
+  const auto msgLen =
+      std::min(static_cast<size_t>(vsnprintf(
+                   buf + prefixLen, fsfwconfig::FSFW_PRINT_BUFFER_SIZE - prefixLen, fmt, arg)),
+               fsfwconfig::FSFW_PRINT_BUFFER_SIZE - 1 - prefixLen);
+
+  size_t textLen = prefixLen + msgLen;
+  if (addCrAtEnd and buf[textLen - 1] == '\n') {
+    buf[textLen++] = '\r';
+  }
+  if (replaceLastCharWithNewline and buf[textLen - 1] != '\n' and buf[textLen - 1] != '\r') {
+    buf[textLen++] = '\n';
   }
 
-  printf("%s", printBuffer);
+  if (not taskRunning) {
+    fwrite(buf, 1, textLen, stdout);
+    return;
+  }
+
+  MutexGuard guard(ringMutex, MutexIF::TimeoutType::BLOCKING);
+
+  if (textLen > RING_SIZE - bytesUsed) {
+    ++droppedMessages;
+    return;
+  }
+
+  const size_t writeIdx = (readIdx + bytesUsed) % RING_SIZE;
+  const size_t firstPart = std::min(textLen, RING_SIZE - writeIdx);
+
+  std::memcpy(ring + writeIdx, buf, firstPart);
+  std::memcpy(ring, buf + firstPart, textLen - firstPart);
+
+  bytesUsed += textLen;
 }
 
-void sif::printInfo(const char *fmt, ...) {
+}  // namespace
+
+void sif::setToAddCrAtEnd(const bool addCrAtEnd_) { addCrAtEnd = addCrAtEnd_; }
+
+void sif::setReplaceLastCharWithNewline(const bool replace) {
+  replaceLastCharWithNewline = replace;
+}
+
+void sif::printInfo(const char* fmt, ...) {
   va_list args;
   va_start(args, fmt);
   fsfwPrint(sif::PrintLevel::INFO_LEVEL, fmt, args);
   va_end(args);
 }
 
-void sif::printWarning(const char *fmt, ...) {
+void sif::printWarning(const char* fmt, ...) {
   va_list args;
   va_start(args, fmt);
   fsfwPrint(sif::PrintLevel::WARNING_LEVEL, fmt, args);
   va_end(args);
 }
 
-void sif::printDebug(const char *fmt, ...) {
+void sif::printDebug(const char* fmt, ...) {
   va_list args;
   va_start(args, fmt);
   fsfwPrint(sif::PrintLevel::DEBUG_LEVEL, fmt, args);
   va_end(args);
 }
 
-void sif::setToAddCrAtEnd(bool addCrAtEnd_) { addCrAtEnd = addCrAtEnd_; }
-
-void sif::printError(const char *fmt, ...) {
+void sif::printError(const char* fmt, ...) {
   va_list args;
   va_start(args, fmt);
   fsfwPrint(sif::PrintLevel::ERROR_LEVEL, fmt, args);
   va_end(args);
 }
 
+void sif::printCallback() {
+  taskRunning = true;
+
+  size_t chunk;
+  {
+    MutexGuard guard(ringMutex, MutexIF::TimeoutType::BLOCKING);
+    chunk = std::min(bytesUsed, MAX_BYTES_PER_CYCLE);
+  }
+
+  if (chunk > 0) {
+    // Safe without the mutex: producers only touch the ring beyond
+    // readIdx + bytesUsed and only the print task advances readIdx.
+    const size_t firstPart = std::min(chunk, RING_SIZE - readIdx);
+    fwrite(ring + readIdx, 1, firstPart, stdout);
+    fwrite(ring, 1, chunk - firstPart, stdout);
+    fflush(stdout);
+  }
+
+  uint32_t dropped;
+  {
+    MutexGuard guard(ringMutex, MutexIF::TimeoutType::BLOCKING);
+    readIdx = (readIdx + chunk) % RING_SIZE;
+    bytesUsed -= chunk;
+    dropped = droppedMessages;
+    droppedMessages = 0;
+  }
+
+  droppedMessagesTotal += dropped;
+
+  if (dropped != 0) {
+    sif::printError("ServiceInterfacePrinter: Dropped %lu messages\n",
+                    static_cast<unsigned long>(dropped));
+  }
+}
+
+void sif::init() { ringMutex = MutexFactory::instance()->createMutex(); }
+
+uint32_t sif::getDroppedMessagesCount() { return droppedMessagesTotal; }
+
 #else
 
-void sif::printInfo(const char *fmt, ...) {}
-void sif::printWarning(const char *fmt, ...) {}
-void sif::printDebug(const char *fmt, ...) {}
-void sif::printError(const char *fmt, ...) {}
+void sif::printInfo(const char* fmt, ...) {}
+void sif::printWarning(const char* fmt, ...) {}
+void sif::printDebug(const char* fmt, ...) {}
+void sif::printError(const char* fmt, ...) {}
+
+void sif::printCallback() {}
+void sif::init() {}
+uint32_t sif::getDroppedMessagesCount() { return 0; }
 
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 

src/fsfw/serviceinterface/ServiceInterfacePrinter.h

@@ -39,6 +39,11 @@ PrintLevel getPrintLevel();
 
 void setToAddCrAtEnd(bool addCrAtEnd_);
 
+/**
+ * Replaces the last char of a print buffer with a newline
+ */
+void setReplaceLastCharWithNewline(bool replace);
+
 /**
  * These functions can be used like the C stdio printf and forward the
  * supplied formatted string arguments to a printf function.
@@ -51,6 +56,21 @@ void printWarning(const char* fmt, ...);
 void printDebug(const char* fmt, ...);
 void printError(const char* fmt, ...);
 
+/**
+ * This function is to be called periodically by a dedicated print task.
+ */
+void printCallback();
+
+/**
+ * Initializes the global state for the print task.
+ */
+void init();
+
+/**
+ * Gets the total estimated number of dropped messages
+ */
+uint32_t getDroppedMessagesCount();
+
 }  // namespace sif
 
 #endif /* FSFW_SERVICEINTERFACE_SERVICEINTERFACEPRINTER */

src/fsfw/serviceinterface/ServiceInterfacePrinterTask.cpp

@@ -0,0 +1,14 @@
+#include "ServiceInterfacePrinterTask.h"
+
+#include "ServiceInterfacePrinter.h"
+#include "fsfw/objectmanager/SystemObject.h"
+
+ServiceInterfacePrinterTask::ServiceInterfacePrinterTask(object_id_t objectId)
+    : SystemObject(objectId) {
+  sif::init();
+}
+
+ReturnValue_t ServiceInterfacePrinterTask::performOperation(uint8_t operationCode) {
+  sif::printCallback();
+  return returnvalue::OK;
+}

src/fsfw/serviceinterface/ServiceInterfacePrinterTask.h

@@ -0,0 +1,10 @@
+#pragma once
+
+#include "fsfw/objectmanager/SystemObject.h"
+#include "fsfw/tasks/ExecutableObjectIF.h"
+
+class ServiceInterfacePrinterTask : public ExecutableObjectIF, public SystemObject {
+ public:
+  explicit ServiceInterfacePrinterTask(object_id_t objectId);
+  ReturnValue_t performOperation(uint8_t operationCode) override;
+};

src/fsfw/subsystem/Subsystem.h

@@ -99,7 +99,7 @@ class Subsystem : public SubsystemBase, public HasModeSequenceIF {
     EntryPointer entries;
   };
 
-  static const uint8_t MAX_NUMBER_OF_TABLES_OR_SEQUENCES = 70;
+  static const uint8_t MAX_NUMBER_OF_TABLES_OR_SEQUENCES = 100;
 
   static const uint8_t MAX_LENGTH_OF_TABLE_OR_SEQUENCE = 20;
 

src/fsfw/subsystem/SubsystemBase.cpp

@@ -78,9 +78,8 @@ void SubsystemBase::executeTable(HybridIterator<ModeListEntry> tableIter, Submod
       submodeToCommand = targetSubmode;
     }
 
-    if (healthHelper.healthTable->hasHealth(object)) {
-
-      switch (healthHelper.healthTable->getHealth(object)) {
+    if (healthHelper.healthTable->hasHealth(convertToDeviceObjectId(object))) {
+      switch (healthHelper.healthTable->getHealth(convertToDeviceObjectId(object))) {
         case NEEDS_RECOVERY:
         case FAULTY:
         case PERMANENT_FAULTY:
@@ -353,3 +352,5 @@ ReturnValue_t SubsystemBase::registerChild(object_id_t childObjectId, MessageQue
   }
   return returnvalue::OK;
 }
+
+object_id_t SubsystemBase::convertToDeviceObjectId(object_id_t id) { return id; }

src/fsfw/subsystem/SubsystemBase.h

@@ -113,8 +113,8 @@ class SubsystemBase : public SystemObject,
    * We need to know the target Submode, as children are able to inherit the submode
    * Still, we have a default for all child implementations which do not use submode inheritance
    */
-  void executeTable(HybridIterator<ModeListEntry> tableIter,
-                    Submode_t targetSubmode = SUBMODE_NONE);
+  virtual void executeTable(HybridIterator<ModeListEntry> tableIter,
+                            Submode_t targetSubmode = SUBMODE_NONE);
 
   ReturnValue_t updateChildMode(MessageQueueId_t queue, Mode_t mode, Submode_t submode);
   ReturnValue_t updateChildModeByObjId(object_id_t objectId, Mode_t mode, Submode_t submode);
@@ -153,6 +153,12 @@ class SubsystemBase : public SystemObject,
   virtual void announceMode(bool recursive) override;
 
   virtual void modeChanged();
+
+  /**
+   * @brief Provides an adaptation point for the user to change an objectId into
+   * a different objectId.
+   */
+  virtual object_id_t convertToDeviceObjectId(object_id_t id);
 };
 
 #endif /* FSFW_SUBSYSTEM_SUBSYSTEMBASE_H_ */

src/fsfw/tcdistribution/PusDistributor.cpp

@@ -18,6 +18,10 @@ PusDistributor::PusDistributor(uint16_t setApid, object_id_t setObjectId, Storag
 
 PusDistributor::~PusDistributor() = default;
 
+void PusDistributor::setVerificationReporter(object_id_t verificationReporter_) {
+  verificationReporter = verificationReporter_;
+}
+
 ReturnValue_t PusDistributor::selectDestination(MessageQueueId_t& destId) {
 #if FSFW_CPP_OSTREAM_ENABLED == 1 && PUS_DISTRIBUTOR_DEBUGGING == 1
   store_address_t storeId = currentMessage.getStorageId();
@@ -131,8 +135,7 @@ ReturnValue_t PusDistributor::initialize() {
     return ObjectManagerIF::CHILD_INIT_FAILED;
   }
   if (verifyChannel == nullptr) {
-    verifyChannel =
-        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
+    verifyChannel = ObjectManager::instance()->get<VerificationReporterIF>(verificationReporter);
     if (verifyChannel == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }

src/fsfw/tcdistribution/PusDistributor.h

@@ -43,6 +43,10 @@ class PusDistributor : public TcDistributorBase,
   [[nodiscard]] MessageQueueId_t getRequestQueue() const override;
   ReturnValue_t initialize() override;
   [[nodiscard]] uint32_t getIdentifier() const override;
+  /**
+   * @brief Can be used to set the verification reporter if another than the default should be used
+   */
+  void setVerificationReporter(object_id_t verificationReporter_);
 
  protected:
   struct ServiceInfo {
@@ -75,6 +79,8 @@ class PusDistributor : public TcDistributorBase,
    */
   ReturnValue_t tcStatus;
 
+  object_id_t verificationReporter = objects::VERIFICATION_REPORTER;
+
   /**
    * This method reads the packet service, checks if such a service is
    * registered and forwards the packet to the destination.

src/fsfw/tcdistribution/definitions.h

@@ -25,7 +25,7 @@ static constexpr ReturnValue_t INCORRECT_SECONDARY_HEADER = MAKE_RETURN_CODE(11)
 
 static constexpr uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::TMTC_DISTRIBUTION;
 //! P1: Returnvalue, P2: 0 for TM issues, 1 for TC issues
-static constexpr Event HANDLE_PACKET_FAILED = MAKE_EVENT(0, severity::LOW);
+static constexpr Event HANDLE_PACKET_FAILED = event::makeEvent<SUBSYSTEM_ID, 0, severity::LOW>();
 
 };  // namespace tmtcdistrib
 #endif  // FSFW_TMTCPACKET_DEFINITIONS_H

src/fsfw/timemanager/Clock.h

@@ -192,8 +192,6 @@ class Clock {
   static MutexIF *timeMutex;
   static uint16_t leapSeconds;
   static bool leapSecondsSet;
-  static bool monotonicClockInitialized;
-  static timeval monotonicClockOffset;
 };
 
 #endif /* FSFW_TIMEMANAGER_CLOCK_H_ */

src/fsfw/tmstorage/TmStoreMessage.cpp

@@ -114,6 +114,11 @@ void TmStoreMessage::setDownlinkContentTimeMessage(CommandMessage* cmd, store_ad
   cmd->setParameter2(storeId.raw);
 }
 
+void TmStoreMessage::setStopDownlinkContentMessage(CommandMessage* cmd, store_address_t storeId) {
+  cmd->setCommand(STOP_DOWNLINK_STORE_CONTENT);
+  cmd->setParameter2(storeId.raw);
+}
+
 uint32_t TmStoreMessage::getAddressLow(CommandMessage* cmd) { return cmd->getParameter(); }
 uint32_t TmStoreMessage::getAddressHigh(CommandMessage* cmd) { return cmd->getParameter2(); }
 

src/fsfw/tmstorage/TmStoreMessage.h

@@ -21,6 +21,7 @@ class TmStoreMessage {
   static void setStoreCatalogueReportMessage(CommandMessage* cmd, object_id_t objectId,
                                              store_address_t storeId);
   static void setDownlinkContentTimeMessage(CommandMessage* cmd, store_address_t storeId);
+  static void setStopDownlinkContentMessage(CommandMessage* cmd, store_address_t storeId);
   static void setIndexReportMessage(CommandMessage* cmd, store_address_t storeId);
   static ReturnValue_t setDeleteBlocksMessage(CommandMessage* cmd, uint32_t addressLow,
                                               uint32_t addressHigh);
@@ -54,6 +55,7 @@ class TmStoreMessage {
   static const Command_t DOWNLINK_STORE_CONTENT_BLOCKS = MAKE_COMMAND_ID(12);
   static const Command_t REPORT_INDEX_REQUEST = MAKE_COMMAND_ID(13);
   static const Command_t INDEX_REPORT = MAKE_COMMAND_ID(14);
+  static const Command_t STOP_DOWNLINK_STORE_CONTENT = MAKE_COMMAND_ID(15);
 
  private:
   TmStoreMessage();

src/fsfw/tmtcservices/CommandingServiceBase.cpp

@@ -128,7 +128,7 @@ ReturnValue_t CommandingServiceBase::initialize() {
 
   if (verificationReporter == nullptr) {
     verificationReporter =
-        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
+        ObjectManager::instance()->get<VerificationReporterIF>(verificationReporterId);
     if (verificationReporter == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }
@@ -136,6 +136,10 @@ ReturnValue_t CommandingServiceBase::initialize() {
   return returnvalue::OK;
 }
 
+void CommandingServiceBase::setVerificationReporter(object_id_t verificationReporterId_) {
+  verificationReporterId = verificationReporterId_;
+}
+
 void CommandingServiceBase::handleCommandQueue() {
   CommandMessage reply;
   ReturnValue_t result;

src/fsfw/tmtcservices/CommandingServiceBase.h

@@ -126,6 +126,8 @@ class CommandingServiceBase : public SystemObject,
 
   ReturnValue_t initialize() override;
 
+  void setVerificationReporter(object_id_t verificationReporterId_);
+
   /**
    * Implementation of ExecutableObjectIF function
    *
@@ -262,6 +264,8 @@ class CommandingServiceBase : public SystemObject,
 
   const uint16_t timeoutSeconds;
 
+  object_id_t verificationReporterId = objects::VERIFICATION_REPORTER;
+
   PusTcReader tcReader;
   TmStoreHelper tmStoreHelper;
   TmSendHelper tmSendHelper;

src/fsfw/tmtcservices/PusServiceBase.cpp

@@ -111,7 +111,7 @@ ReturnValue_t PusServiceBase::initialize() {
   }
 
   if (psbParams.pusDistributor == nullptr) {
-    psbParams.pusDistributor = ObjectManager::instance()->get<PusDistributorIF>(PUS_DISTRIBUTOR);
+    psbParams.pusDistributor = ObjectManager::instance()->get<PusDistributorIF>(pusDistributor);
     if (psbParams.pusDistributor != nullptr) {
       registerService(*psbParams.pusDistributor);
     }
@@ -126,7 +126,7 @@ ReturnValue_t PusServiceBase::initialize() {
 
   if (psbParams.verifReporter == nullptr) {
     psbParams.verifReporter =
-        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
+        ObjectManager::instance()->get<VerificationReporterIF>(verificationReporter);
     if (psbParams.verifReporter == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }
@@ -134,6 +134,14 @@ ReturnValue_t PusServiceBase::initialize() {
   return returnvalue::OK;
 }
 
+void PusServiceBase::setPusDistributor(object_id_t pusDistributor_) {
+  pusDistributor = pusDistributor_;
+}
+
+void PusServiceBase::setVerificationReporter(object_id_t verificationReporter_) {
+  verificationReporter = verificationReporter_;
+}
+
 void PusServiceBase::setTcPool(StorageManagerIF& tcPool) { psbParams.tcPool = &tcPool; }
 
 void PusServiceBase::setErrorReporter(InternalErrorReporterIF& errReporter_) {

src/fsfw/tmtcservices/PusServiceBase.h

@@ -201,6 +201,9 @@ class PusServiceBase : public ExecutableObjectIF,
   void setTaskIF(PeriodicTaskIF* taskHandle) override;
   [[nodiscard]] const char* getName() const override;
 
+  void setPusDistributor(object_id_t pusDistributor_);
+  void setVerificationReporter(object_id_t verificationReporter_);
+
  protected:
   /**
    * @brief   Handle to the underlying task
@@ -228,6 +231,9 @@ class PusServiceBase : public ExecutableObjectIF,
   static object_id_t PACKET_DESTINATION;
   static object_id_t PUS_DISTRIBUTOR;
 
+  object_id_t pusDistributor = PUS_DISTRIBUTOR;
+  object_id_t verificationReporter = objects::VERIFICATION_REPORTER;
+
  private:
   void handleRequestQueue();
 };

src/fsfw_hal/CMakeLists.txt

@@ -1,7 +1,7 @@
 add_subdirectory(common)
-add_subdirectory(host)
 
 if(UNIX)
+  add_subdirectory(host)
   add_subdirectory(linux)
 endif()
 

src/fsfw_hal/host/CMakeLists.txt

@@ -1 +1 @@
-target_sources(${LIB_FSFW_NAME} PUBLIC HostFilesystem.cpp)
+target_sources(${LIB_FSFW_NAME} PRIVATE HostFilesystem.cpp)

src/fsfw_hal/linux/CommandExecutor.h

@@ -116,7 +116,7 @@ class CommandExecutor {
   int currentFd = 0;
   bool printOutput = true;
   std::vector<char> readVec;
-  struct pollfd waiter {};
+  struct pollfd waiter{};
   SimpleRingBuffer* ringBuffer = nullptr;
   DynamicFIFO<uint16_t>* sizesFifo = nullptr;
 

unittests/cfdp/PduSenderMock.h

@@ -15,6 +15,12 @@ class PduSenderMock : public cfdp::PduSenderIF {
   ReturnValue_t sendPdu(cfdp::PduType pduType, std::optional<cfdp::FileDirective> fileDirective,
 
                         const uint8_t* pdu, size_t pduSize) override {
+    const size_t callIdx = totalSendCalls++;
+    if (failNextSend or (failSendAtIdx.has_value() and *failSendAtIdx == callIdx) or
+        (failSendsFromIdx.has_value() and callIdx >= *failSendsFromIdx)) {
+      failNextSend = false;
+      return FAILED_SEND_RESULT;
+    }
     SentPdu sentPdu;
     sentPdu.pduType = pduType;
     sentPdu.fileDirective = fileDirective;
@@ -32,5 +38,18 @@ class PduSenderMock : public cfdp::PduSenderIF {
     return nextPdu;
   }
 
+  // Simulates a downstream send failure (e.g. a full TM store), consumed by the next sendPdu()
+  // call. Used to verify a caller does not treat a failed send as if the PDU went out.
+  static constexpr ReturnValue_t FAILED_SEND_RESULT = returnvalue::FAILED;
+  bool failNextSend = false;
+
+  // Fails the sendPdu() call at this 0-based index instead of the next one. Needed when the
+  // state machine call under test emits several PDUs and only a later one must fail.
+  std::optional<size_t> failSendAtIdx = std::nullopt;
+  // Fails every sendPdu() call from this 0-based index on, i.e. a downstream which stays broken
+  // (a TM store which never drains because the downlink itself is stalled).
+  std::optional<size_t> failSendsFromIdx = std::nullopt;
+  size_t totalSendCalls = 0;
+
   std::deque<SentPdu> sentPdus;
 };

unittests/cfdp/handler/testDestHandler.cpp

@@ -1,15 +1,20 @@
 #include <etl/crc32.h>
 
 #include <catch2/catch_test_macros.hpp>
+#include <chrono>
 #include <random>
+#include <thread>
 #include <utility>
 
 #include "OwnedPduPacket.h"
 #include "cfdp/PduSenderMock.h"
 #include "fsfw/cfdp.h"
+#include "fsfw/cfdp/pdu/AckPduCreator.h"
+#include "fsfw/cfdp/pdu/AckPduReader.h"
 #include "fsfw/cfdp/pdu/EofPduCreator.h"
 #include "fsfw/cfdp/pdu/FileDataCreator.h"
 #include "fsfw/cfdp/pdu/MetadataPduCreator.h"
+#include "fsfw/cfdp/pdu/NakPduReader.h"
 #include "mock/AcceptsTmMock.h"
 #include "mock/EventReportingProxyMock.h"
 #include "mock/FilesystemMock.h"
@@ -46,12 +51,13 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") {
   PduConfig conf;
   auto destHandler = DestHandler(senderMock, 4096, dp, fp);
 
-  auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType) {
+  auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType,
+                                 bool closureRequested = false) {
     const std::string srcNameString = "hello.txt";
     const std::string destNameString = "hello-cpy.txt";
     StringLv srcName(srcNameString);
     StringLv destName(destNameString);
-    MetadataGenericInfo info(false, checksumType, std::move(cfdpFileSize));
+    MetadataGenericInfo info(closureRequested, checksumType, std::move(cfdpFileSize));
     const TransactionSeqNum seqNum(UnsignedByteField<uint16_t>(1));
     conf.sourceId = remoteId;
     conf.destId = localId;
@@ -126,6 +132,34 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") {
     CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE);
   }
 
+  SECTION("Metadata only transfer reports a complete delivery") {
+    // A proxy put request is metadata only: empty file names, the request itself in a message to
+    // user, and no file data to wait for - so the transaction completes the moment the metadata
+    // lands. It must report a complete delivery, not the "Data Incomplete" reset defaults.
+    StringLv emptySrcName;
+    StringLv emptyDestName;
+    MetadataGenericInfo info(false, ChecksumType::NULL_CHECKSUM, Fss(0));
+    const TransactionSeqNum seqNum(UnsignedByteField<uint16_t>(1));
+    conf.sourceId = remoteId;
+    conf.destId = localId;
+    conf.mode = TransmissionMode::UNACKNOWLEDGED;
+    conf.seqNum = seqNum;
+    const MetadataPduCreator creator(conf, info, emptySrcName, emptyDestName, nullptr, 0);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    auto packet =
+        OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+
+    destHandler.stateMachine(packet);
+    destHandler.stateMachineNoPacket();
+
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    const auto& finished = userMock.finishedRecvd.back().second;
+    CHECK(finished.condCode == ConditionCode::NO_ERROR);
+    CHECK(finished.deliveryCode == FileDeliveryCode::DATA_COMPLETE);
+    CHECK(finished.status == FileDeliveryStatus::FILE_STATUS_UNREPORTED);
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+  }
+
   SECTION("Empty File Transfer") {
     const DestHandler::FsmResult& res = destHandler.stateMachineNoPacket();
     CHECK(res.result == OK);
@@ -219,4 +253,469 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") {
     destHandler.stateMachine(eofPacket);
     eofCheck(res, transactionId);
   }
-}
+
+  SECTION("A failed Finished PDU send is retried, then the transaction is released") {
+    // Class 1 has no ACK for the Finished PDU. Finishing on a failed send means the sender never
+    // hears that the transfer completed.
+    std::string fileData = "hello test data";
+    etl::crc32 crcCalc;
+    crcCalc.add(fileData.begin(), fileData.end());
+    Fss cfdpFileSize(fileData.size());
+    auto metadataPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32, true);
+    const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPacket);
+    Fss offset(0);
+    FileDataInfo fdPduInfo(offset, reinterpret_cast<const uint8_t*>(fileData.data()),
+                           fileData.size());
+    FileDataCreator fdPduCreator(conf, fdPduInfo);
+    REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == OK);
+    OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen);
+    destHandler.stateMachine(fdPdu);
+
+    // The Finished PDU send fails on the first attempt only.
+    senderMock.failNextSend = true;
+    auto eofPacket = eofPreparation(cfdpFileSize, crcCalc.value());
+    destHandler.stateMachine(eofPacket);
+    CHECK(not senderMock.getNextSentPacket().has_value());
+    REQUIRE(res.step == DestHandler::TransactionStep::SENDING_FINISHED_PDU);
+
+    // The retry succeeds and only then is the transaction released.
+    destHandler.stateMachineNoPacket();
+    auto optPacket = senderMock.getNextSentPacket();
+    REQUIRE(optPacket.has_value());
+    REQUIRE(optPacket->fileDirective.has_value());
+    CHECK(*optPacket->fileDirective == FileDirective::FINISH);
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+  }
+
+  SECTION("A Finished PDU which can never be sent still releases the handler") {
+    // The retry has to be bounded. While a transaction is held the handler refuses every new
+    // metadata PDU, so retrying forever on a downstream which stays broken would mean no uplink
+    // can ever start again.
+    std::string fileData = "hello test data";
+    etl::crc32 crcCalc;
+    crcCalc.add(fileData.begin(), fileData.end());
+    Fss cfdpFileSize(fileData.size());
+    auto metadataPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32, true);
+    destHandler.stateMachine(metadataPacket);
+    Fss offset(0);
+    FileDataInfo fdPduInfo(offset, reinterpret_cast<const uint8_t*>(fileData.data()),
+                           fileData.size());
+    FileDataCreator fdPduCreator(conf, fdPduInfo);
+    REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == OK);
+    OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen);
+    destHandler.stateMachine(fdPdu);
+
+    senderMock.failSendsFromIdx = 0;
+    auto eofPacket = eofPreparation(cfdpFileSize, crcCalc.value());
+    destHandler.stateMachine(eofPacket);
+    for (int idx = 0; idx < 100; idx++) {
+      destHandler.stateMachineNoPacket();
+    }
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+    CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE);
+  }
+}
+TEST_CASE("CFDP Dest Handler Acknowledged", "[cfdp]") {
+  using namespace cfdp;
+  using namespace returnvalue;
+  auto localId = EntityId(UnsignedByteField<uint16_t>(2));
+  auto remoteId = EntityId(UnsignedByteField<uint16_t>(3));
+  FaultHandlerMock fhMock;
+  LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock);
+  FilesystemMock fsMock;
+  UserMock userMock(fsMock);
+  RemoteConfigTableMock remoteCfgTableMock;
+  LostSegmentsList<128> lostSegmentsList;
+  DestHandlerParams dp(localEntityCfg, userMock, remoteCfgTableMock, lostSegmentsList);
+  EventReportingProxyMock eventReporterMock;
+  PduSenderMock senderMock;
+  FsfwParams fp(&eventReporterMock);
+  RemoteEntityCfg cfg(remoteId);
+  // Keep the timers short so the timeout paths are testable without stalling the suite.
+  cfg.positiveAckTimerIntervalMs = 1;
+  cfg.positiveAckTimerExpirationLimit = 2;
+  cfg.nakTimerIntervalMs = 1;
+  cfg.nakTimerExpirationLimit = 2;
+  cfg.checkTimerIntervalMs = 100000;
+  cfg.checkLimit = 2;
+  remoteCfgTableMock.addRemoteConfig(cfg);
+  std::array<uint8_t, 4096> pduBuf{};
+  size_t serLen = 0;
+  PduConfig conf;
+  auto destHandler = DestHandler(senderMock, 4096, dp, fp);
+
+  const std::string srcNameString = "hello.txt";
+  const std::string destNameString = "hello-cpy.txt";
+  const TransactionSeqNum seqNum(UnsignedByteField<uint16_t>(1));
+  conf.sourceId = remoteId;
+  conf.destId = localId;
+  conf.mode = TransmissionMode::ACKNOWLEDGED;
+  conf.seqNum = seqNum;
+
+  std::array<uint8_t, 1024> fileData{};
+  for (size_t idx = 0; idx < fileData.size(); idx++) {
+    fileData[idx] = static_cast<uint8_t>(idx);
+  }
+  etl::crc32 crcCalc;
+  crcCalc.add(fileData.begin(), fileData.end());
+  const uint32_t crc32 = crcCalc.value();
+
+  auto makeMetadataPdu = [&]() {
+    StringLv srcName(srcNameString);
+    StringLv destName(destNameString);
+    MetadataGenericInfo info(false, ChecksumType::CRC_32, Fss(fileData.size()));
+    const MetadataPduCreator creator(conf, info, srcName, destName, nullptr, 0);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  auto makeFileDataPdu = [&](uint64_t offset, size_t len) {
+    Fss offsetFss(offset);
+    FileDataInfo info(offsetFss, fileData.data() + offset, len);
+    FileDataCreator creator(conf, info);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), std::nullopt, pduBuf.data(), serLen);
+  };
+  auto makeEofPdu = [&]() {
+    EofInfo info(ConditionCode::NO_ERROR, crc32, Fss(fileData.size()));
+    EofPduCreator creator(conf, info);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  auto makeFinishedAckPdu = [&]() {
+    AckInfo info(FileDirective::FINISH, ConditionCode::NO_ERROR, AckTransactionStatus::ACTIVE, 1);
+    PduConfig ackConf = conf;
+    AckPduCreator creator(info, ackConf);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  // Pops the next sent PDU and checks it is the expected directive.
+  auto expectDirective = [&](FileDirective expected) {
+    auto optPacket = senderMock.getNextSentPacket();
+    REQUIRE(optPacket.has_value());
+    REQUIRE(optPacket->pduType == PduType::FILE_DIRECTIVE);
+    REQUIRE(optPacket->fileDirective.has_value());
+    REQUIRE(*optPacket->fileDirective == expected);
+    return *optPacket;
+  };
+  auto parseNak = [&](const SentPdu& pdu, std::vector<std::pair<uint64_t, uint64_t>>& segments) {
+    NakInfo info(Fss(0), Fss(0));
+    std::array<NakInfo::SegmentRequest, 16> segBuf{};
+    size_t segLen = 0;
+    size_t maxSegLen = segBuf.size();
+    info.setSegmentRequests(segBuf.data(), &segLen, &maxSegLen);
+    NakPduReader reader(pdu.rawPdu.data(), pdu.rawPdu.size(), info);
+    REQUIRE(reader.parseData() == OK);
+    segments.clear();
+    for (size_t idx = 0; idx < info.getSegmentRequestsLen(); idx++) {
+      segments.emplace_back(segBuf[idx].first.value(), segBuf[idx].second.value());
+    }
+  };
+
+  SECTION("Nominal acknowledged transfer") {
+    auto metadataPdu = makeMetadataPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu);
+    REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED);
+    REQUIRE(res.step == DestHandler::TransactionStep::RECEIVING_FILE_DATA_PDUS);
+    auto fdPdu = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(fdPdu);
+    REQUIRE(destHandler.getNumLostSegments() == 0);
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    // The ACK for the EOF PDU is emitted before the Finished PDU, and before the checksum
+    // verification which produced it.
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::FINISH);
+    // The transaction is retained until the Finished PDU is acknowledged.
+    REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED);
+    REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR);
+    auto ackPdu = makeFinishedAckPdu();
+    destHandler.stateMachine(ackPdu);
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+    CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE);
+  }
+
+  SECTION("Metadata only transaction sends Finished after the EOF ACK") {
+    // The shape of a proxy put request in class 2. The sender ignores a Finished PDU while it is
+    // still waiting for the EOF ACK, so the order matters.
+    StringLv emptySrcName;
+    StringLv emptyDestName;
+    MetadataGenericInfo info(false, ChecksumType::NULL_CHECKSUM, Fss(0));
+    const MetadataPduCreator mdCreator(conf, info, emptySrcName, emptyDestName, nullptr, 0);
+    REQUIRE(mdCreator.serialize(pduBuf.data(), serLen, mdCreator.getSerializedSize()) == OK);
+    auto metadataPdu =
+        OwnedPduPacket(mdCreator.getPduType(), mdCreator.getDirectiveCode(), pduBuf.data(), serLen);
+    const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu);
+    CHECK(res.state == CfdpState::BUSY_CLASS_2_ACKED);
+    // The CFDP task keeps calling the state machine while the EOF PDU is still on its way.
+    destHandler.stateMachineNoPacket();
+    CHECK(not senderMock.getNextSentPacket().has_value());
+
+    EofInfo eofInfo(ConditionCode::NO_ERROR, 0, Fss(0));
+    EofPduCreator eofCreator(conf, eofInfo);
+    REQUIRE(eofCreator.serialize(pduBuf.data(), serLen, eofCreator.getSerializedSize()) == OK);
+    auto eofPdu = OwnedPduPacket(eofCreator.getPduType(), eofCreator.getDirectiveCode(),
+                                 pduBuf.data(), serLen);
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_COMPLETE);
+    auto ackPdu = makeFinishedAckPdu();
+    destHandler.stateMachine(ackPdu);
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+  }
+
+  SECTION("Cancel EOF is parsed, acknowledged and ends the transaction") {
+    // A Cancel EOF carries a condition code other than NO_ERROR and a fault location TLV. The
+    // reader refuses to parse one unless it is given somewhere to put that TLV.
+    auto metadataPdu = makeMetadataPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu);
+    REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED);
+    // Only part of the file arrives before the sender gives up on it.
+    auto fdPdu = makeFileDataPdu(0, 10);
+    destHandler.stateMachine(fdPdu);
+
+    EntityId faultLocId(UnsignedByteField<uint16_t>(2));
+    EntityIdTlv faultLoc(faultLocId);
+    EofInfo cancelInfo(ConditionCode::CANCEL_REQUEST_RECEIVED, 0, Fss(fileData.size()), &faultLoc);
+    EofPduCreator cancelCreator(conf, cancelInfo);
+    REQUIRE(cancelCreator.serialize(pduBuf.data(), serLen, cancelCreator.getSerializedSize()) ==
+            OK);
+    auto cancelEof = OwnedPduPacket(cancelCreator.getPduType(), cancelCreator.getDirectiveCode(),
+                                    pduBuf.data(), serLen);
+    destHandler.stateMachine(cancelEof);
+
+    // The cancellation is acknowledged rather than ignored, and it is reported with the condition
+    // code the sender gave instead of a checksum failure over the partial file.
+    auto ackPdu = expectDirective(FileDirective::ACK);
+    AckInfo ackInfo;
+    AckPduReader ackReader(ackPdu.rawPdu.data(), ackPdu.rawPdu.size(), ackInfo);
+    REQUIRE(ackReader.parseData() == OK);
+    CHECK(ackInfo.getAckedDirective() == FileDirective::EOF_DIRECTIVE);
+    CHECK(ackInfo.getAckedConditionCode() == ConditionCode::CANCEL_REQUEST_RECEIVED);
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::CANCEL_REQUEST_RECEIVED);
+    CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE);
+  }
+
+  SECTION("Dropped file data PDU is recovered via NAK") {
+    auto metadataPdu = makeMetadataPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu);
+    // The segment from 256 to 512 is dropped on the way.
+    auto firstPdu = makeFileDataPdu(0, 256);
+    destHandler.stateMachine(firstPdu);
+    auto thirdPdu = makeFileDataPdu(512, 512);
+    destHandler.stateMachine(thirdPdu);
+    REQUIRE(destHandler.getNumLostSegments() == 1);
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    auto nakPdu = expectDirective(FileDirective::NAK);
+    std::vector<std::pair<uint64_t, uint64_t>> segments;
+    parseNak(nakPdu, segments);
+    REQUIRE(segments.size() == 1);
+    CHECK(segments[0].first == 256);
+    CHECK(segments[0].second == 512);
+    REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_MISSING_DATA);
+    // The retransmission closes the gap and the transfer completes.
+    auto retransmit = makeFileDataPdu(256, 256);
+    destHandler.stateMachine(retransmit);
+    CHECK(destHandler.getNumLostSegments() == 0);
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR);
+    CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_COMPLETE);
+  }
+
+  SECTION("Dropped metadata PDU is requested with a scope 0 to 0 NAK") {
+    // The first PDU the destination sees is a file data PDU, which in class 2 starts the
+    // transaction instead of being rejected.
+    auto fdPdu = makeFileDataPdu(0, 256);
+    const DestHandler::FsmResult& res = destHandler.stateMachine(fdPdu);
+    REQUIRE(res.result == OK);
+    REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED);
+    auto nakPdu = expectDirective(FileDirective::NAK);
+    std::vector<std::pair<uint64_t, uint64_t>> segments;
+    parseNak(nakPdu, segments);
+    REQUIRE(segments.size() == 1);
+    CHECK(segments[0].first == 0);
+    CHECK(segments[0].second == 0);
+    // Nothing was written, there is no destination file name yet.
+    CHECK(fsMock.fileMap.find(destNameString) == fsMock.fileMap.end());
+    // The metadata retransmission completes the transaction setup.
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    CHECK(fsMock.fileMap.find(destNameString) != fsMock.fileMap.end());
+    auto allData = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(allData);
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::FINISH);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR);
+  }
+
+  SECTION("Finished PDU is retransmitted on positive ACK timeout") {
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto fdPdu = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(fdPdu);
+    auto eofPdu = makeEofPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK);
+    // Two expirations are tolerated, each retransmits the Finished PDU.
+    for (uint32_t idx = 0; idx < cfg.positiveAckTimerExpirationLimit; idx++) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(5));
+      destHandler.stateMachineNoPacket();
+      expectDirective(FileDirective::FINISH);
+      CHECK(destHandler.getCfdpState() == CfdpState::BUSY_CLASS_2_ACKED);
+    }
+    // The next one reaches the limit and releases the handler.
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+    destHandler.stateMachineNoPacket();
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+    auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR);
+    REQUIRE(fhInfo.callCount == 1);
+    CHECK(fhInfo.condCodes.front() == ConditionCode::POSITIVE_ACK_LIMIT_REACHED);
+  }
+
+  SECTION("NAK limit reached cancels the transaction") {
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto firstPdu = makeFileDataPdu(0, 256);
+    destHandler.stateMachine(firstPdu);
+    auto eofPdu = makeEofPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::NAK);
+    REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_MISSING_DATA);
+    for (uint32_t idx = 0; idx < cfg.nakTimerExpirationLimit; idx++) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(5));
+      destHandler.stateMachineNoPacket();
+      expectDirective(FileDirective::NAK);
+    }
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+    destHandler.stateMachineNoPacket();
+    // The transaction is cancelled and the failure is reported in the Finished PDU rather than
+    // pinning the handler.
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NAK_LIMIT_REACHED);
+    CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE);
+    auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR);
+    REQUIRE(fhInfo.callCount == 1);
+    CHECK(fhInfo.condCodes.front() == ConditionCode::NAK_LIMIT_REACHED);
+  }
+
+  SECTION("Retransmitted data resets the NAK and check limits") {
+    // Expires on every state machine call, so without the reset the check limit is reached in
+    // the second round.
+    remoteCfgTableMock.remoteCfgTable.at(remoteId).checkTimerIntervalMs = 1;
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto firstPdu = makeFileDataPdu(0, 256);
+    destHandler.stateMachine(firstPdu);
+    auto eofPdu = makeEofPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::NAK);
+    // More NAK rounds than the limit allows, but each one recovers part of the gap.
+    for (uint64_t offset = 256; offset < 768; offset += 128) {
+      for (uint32_t idx = 0; idx < cfg.nakTimerExpirationLimit; idx++) {
+        std::this_thread::sleep_for(std::chrono::milliseconds(5));
+        destHandler.stateMachineNoPacket();
+        expectDirective(FileDirective::NAK);
+      }
+      auto retransmission = makeFileDataPdu(offset, 128);
+      destHandler.stateMachine(retransmission);
+      REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_MISSING_DATA);
+      CHECK(destHandler.getNakCounter() == 0);
+    }
+    auto rest = makeFileDataPdu(768, fileData.size() - 768);
+    destHandler.stateMachine(rest);
+    expectDirective(FileDirective::FINISH);
+    CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR);
+  }
+
+  SECTION("EOF PDU for an inactive transaction is acknowledged") {
+    // The sender retransmitted an EOF after we already finished. Without an ACK it would
+    // retransmit to its own limit and declare a fault at the end of a successful transfer.
+    auto eofPdu = makeEofPdu();
+    const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu);
+    CHECK(res.result == OK);
+    auto ackPdu = expectDirective(FileDirective::ACK);
+    AckInfo ackInfo;
+    AckPduReader reader(ackPdu.rawPdu.data(), ackPdu.rawPdu.size(), ackInfo);
+    REQUIRE(reader.parseData() == OK);
+    CHECK(ackInfo.getAckedDirective() == FileDirective::EOF_DIRECTIVE);
+    CHECK(ackInfo.getTransactionStatus() == AckTransactionStatus::UNRECOGNIZED);
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+  }
+
+  SECTION("A Finished PDU which failed to send is not treated as awaiting its ACK") {
+    // The EOF PDU produces two sends in one state machine call: the ACK first, then the Finished
+    // PDU. Fail only the second one, so the peer never sees a Finished PDU at all.
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto fdPdu = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(fdPdu);
+    senderMock.failSendAtIdx = 1;
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    CHECK(not senderMock.getNextSentPacket().has_value());
+
+    // Waiting for the ACK of a PDU that was never sent costs a full positive ACK interval before
+    // the first retransmission, so the step has to stay on the send instead.
+    CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::SENDING_FINISHED_PDU);
+  }
+
+  SECTION("An unsendable Finished PDU does not pin the handler in acknowledged mode") {
+    // Keeping the step on SENDING_FINISHED_PDU retries the send, but nothing arms a timer or
+    // counts the attempts there, so a downstream which stays broken has to be bounded the same
+    // way class 1 bounds it. Otherwise the transaction is never released and, because a busy
+    // handler discards incoming metadata PDUs, no later uplink can start.
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto fdPdu = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(fdPdu);
+    // The EOF ACK goes out, every Finished PDU send after it fails.
+    senderMock.failSendsFromIdx = 1;
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    for (int idx = 0; idx < 100; idx++) {
+      destHandler.stateMachineNoPacket();
+    }
+    CHECK(destHandler.getCfdpState() == CfdpState::IDLE);
+    CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE);
+  }
+
+  SECTION("A Finished ACK for a different transaction is ignored") {
+    // CfdpHandler routes ACK PDUs on the acked directive alone, so a late ACK from an earlier
+    // transaction reaches whichever transaction is running now.
+    auto metadataPdu = makeMetadataPdu();
+    destHandler.stateMachine(metadataPdu);
+    auto fdPdu = makeFileDataPdu(0, fileData.size());
+    destHandler.stateMachine(fdPdu);
+    auto eofPdu = makeEofPdu();
+    destHandler.stateMachine(eofPdu);
+    expectDirective(FileDirective::ACK);
+    expectDirective(FileDirective::FINISH);
+    REQUIRE(destHandler.getTransactionStep() ==
+            DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK);
+
+    conf.seqNum = TransactionSeqNum(UnsignedByteField<uint16_t>(42));
+    auto staleAck = makeFinishedAckPdu();
+    destHandler.stateMachine(staleAck);
+    CHECK(destHandler.getCfdpState() == CfdpState::BUSY_CLASS_2_ACKED);
+    CHECK(destHandler.getTransactionStep() ==
+          DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK);
+  }
+}

unittests/cfdp/handler/testSourceHandler.cpp

@@ -1,18 +1,25 @@
 #include <etl/crc32.h>
 
 #include <catch2/catch_test_macros.hpp>
+#include <chrono>
 #include <filesystem>
 #include <random>
+#include <thread>
 
+#include "OwnedPduPacket.h"
 #include "cfdp/PduSenderMock.h"
 #include "fsfw/cfdp.h"
 #include "fsfw/cfdp/handler/PutRequest.h"
 #include "fsfw/cfdp/handler/SourceHandler.h"
+#include "fsfw/cfdp/pdu/AckPduCreator.h"
+#include "fsfw/cfdp/pdu/AckPduReader.h"
 #include "fsfw/cfdp/pdu/EofPduCreator.h"
 #include "fsfw/cfdp/pdu/EofPduReader.h"
 #include "fsfw/cfdp/pdu/FileDataReader.h"
+#include "fsfw/cfdp/pdu/FinishedPduCreator.h"
 #include "fsfw/cfdp/pdu/MetadataPduCreator.h"
 #include "fsfw/cfdp/pdu/MetadataPduReader.h"
+#include "fsfw/cfdp/pdu/NakPduCreator.h"
 #include "fsfw/tmtcservices/TmTcMessage.h"
 #include "fsfw/util/SeqCountProvider.h"
 #include "mock/AcceptsTmMock.h"
@@ -196,6 +203,39 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") {
     genericNoticeOfCompletionCheck(fsmResult, expectedSeqNum);
   }
 
+  SECTION("File data PDU send failure is retried, not dropped") {
+    // A downstream send failure (e.g. a full TM store) must not be treated as if the PDU went
+    // out: the FSM has to retry the same segment, not silently advance past lost data.
+    uint16_t expectedSeqNum = 0;
+    fsMock.createFile(srcFileName.c_str());
+    std::string fileContent = "hello world\n";
+    size_t expectedFileSize = fileContent.size();
+    fsMock.writeToFile(srcFileName.c_str(), 0, reinterpret_cast<const uint8_t*>(fileContent.data()),
+                       expectedFileSize);
+    CHECK(sourceHandler.transactionStart(putRequest, cfg) == OK);
+    const SourceHandler::FsmResult& fsmResult = sourceHandler.stateMachineNoPacket();
+    genericMetadataCheck(fsmResult, expectedFileSize, expectedSeqNum);
+
+    // The file data PDU send fails. No packet must be recorded and no progress made.
+    pduSender.failNextSend = true;
+    sourceHandler.stateMachineNoPacket();
+    CHECK(fsmResult.packetsSent == 0);
+    CHECK(fsmResult.errors == 1);
+    CHECK(not pduSender.getNextSentPacket().has_value());
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::SENDING_FILE_DATA);
+
+    // Retrying must send the same segment from offset 0, not skip ahead.
+    sourceHandler.stateMachineNoPacket();
+    onePduSentCheck(fsmResult);
+    auto optNextPacket = pduSender.getNextSentPacket();
+    CHECK(optNextPacket.has_value());
+    const auto& [pduType, fileDirective, rawPdu] = *optNextPacket;
+    FileDataInfo fdInfo;
+    FileDataReader fdReader(rawPdu.data(), rawPdu.size(), fdInfo);
+    CHECK(fdReader.parseData() == OK);
+    CHECK(fdInfo.getOffset().value() == 0);
+  }
+
   SECTION("Transfer two segment file") {
     uint16_t expectedSeqNum = 0;
     // Create 400 bytes of random data. This should result in two file segments, with one
@@ -271,4 +311,328 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") {
     sourceHandler.stateMachineNoPacket();
     genericNoticeOfCompletionCheck(fsmResult, expectedSeqNum);
   }
-}
+}
+TEST_CASE("CFDP Source Handler Acknowledged", "[cfdp]") {
+  using namespace cfdp;
+  using namespace returnvalue;
+  constexpr size_t MAX_FILE_SEGMENT_SIZE = 256;
+
+  auto localId = EntityId(UnsignedByteField<uint16_t>(2));
+  auto remoteId = EntityId(UnsignedByteField<uint16_t>(5));
+  FaultHandlerMock fhMock;
+  LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock);
+  FilesystemMock fsMock;
+  UserMock userMock(fsMock);
+  SeqCountProviderU16 seqCountProvider;
+  SourceHandlerParams dp(localEntityCfg, userMock, seqCountProvider);
+  PduSenderMock pduSender;
+  EventReportingProxyMock eventReporterMock;
+  FsfwParams fp(&eventReporterMock);
+  auto sourceHandler = SourceHandler(pduSender, 4096, dp, fp);
+
+  RemoteEntityCfg cfg;
+  cfg.maxFileSegmentLen = MAX_FILE_SEGMENT_SIZE;
+  cfg.remoteId = remoteId;
+  cfg.defaultTransmissionMode = TransmissionMode::ACKNOWLEDGED;
+  // Keep the timers short so the timeout paths are testable without stalling the suite.
+  cfg.positiveAckTimerIntervalMs = 1;
+  cfg.positiveAckTimerExpirationLimit = 2;
+
+  std::string srcFileName = "/tmp/cfdp-acked-test.txt";
+  std::string destFileName = "/tmp/cfdp-acked-test2.txt";
+  std::array<uint8_t, 1024> fileData{};
+  for (size_t idx = 0; idx < fileData.size(); idx++) {
+    fileData[idx] = static_cast<uint8_t>(idx);
+  }
+  fsMock.createFile(srcFileName.c_str());
+  fsMock.writeToFile(srcFileName.c_str(), 0, fileData.data(), fileData.size());
+  cfdp::StringLv srcNameLv(srcFileName.c_str(), srcFileName.length());
+  cfdp::StringLv destNameLv(destFileName.c_str(), destFileName.length());
+  PutRequest putRequest(remoteId, srcNameLv, destNameLv);
+  CHECK(sourceHandler.initialize() == OK);
+
+  // The PDU configuration the peer would use to address this handler.
+  PduConfig peerConf;
+  peerConf.sourceId = localId;
+  peerConf.destId = remoteId;
+  peerConf.mode = TransmissionMode::ACKNOWLEDGED;
+  peerConf.seqNum = TransactionSeqNum(UnsignedByteField<uint16_t>(0));
+  peerConf.direction = Direction::TOWARDS_SENDER;
+  std::array<uint8_t, 1024> pduBuf{};
+  size_t serLen = 0;
+
+  auto makeEofAckPdu = [&]() {
+    AckInfo info(FileDirective::EOF_DIRECTIVE, ConditionCode::NO_ERROR,
+                 AckTransactionStatus::ACTIVE, 0);
+    AckPduCreator creator(info, peerConf);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  auto makeFinishedPdu = [&](ConditionCode condCode, FileDeliveryCode deliveryCode,
+                             FileDeliveryStatus status) {
+    FinishedInfo info(condCode, deliveryCode, status);
+    FinishPduCreator creator(peerConf, info);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  auto makeNakPdu = [&](const std::vector<std::pair<uint64_t, uint64_t>>& segments) {
+    std::vector<NakInfo::SegmentRequest> segBuf;
+    segBuf.reserve(segments.size());
+    for (const auto& segment : segments) {
+      segBuf.emplace_back(Fss(segment.first), Fss(segment.second));
+    }
+    NakInfo info(Fss(0), Fss(fileData.size()));
+    size_t segLen = segBuf.size();
+    size_t maxSegLen = segBuf.size();
+    info.setSegmentRequests(segBuf.data(), &segLen, &maxSegLen);
+    NakPduCreator creator(peerConf, info);
+    REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK);
+    return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen);
+  };
+  auto expectDirective = [&](FileDirective expected) {
+    auto optPacket = pduSender.getNextSentPacket();
+    REQUIRE(optPacket.has_value());
+    REQUIRE(optPacket->pduType == PduType::FILE_DIRECTIVE);
+    REQUIRE(optPacket->fileDirective.has_value());
+    CHECK(*optPacket->fileDirective == expected);
+    return *optPacket;
+  };
+  auto expectFileData = [&](uint64_t expectedOffset, size_t expectedLen) {
+    auto optPacket = pduSender.getNextSentPacket();
+    REQUIRE(optPacket.has_value());
+    REQUIRE(optPacket->pduType == PduType::FILE_DATA);
+    FileDataInfo fdInfo;
+    FileDataReader reader(optPacket->rawPdu.data(), optPacket->rawPdu.size(), fdInfo);
+    REQUIRE(reader.parseData() == OK);
+    CHECK(fdInfo.getOffset().value() == expectedOffset);
+    size_t len = 0;
+    const uint8_t* data = fdInfo.getFileData(&len);
+    CHECK(len == expectedLen);
+    for (size_t idx = 0; idx < len; idx++) {
+      CHECK(data[idx] == fileData[expectedOffset + idx]);
+    }
+  };
+  // Drives the handler until the EOF PDU has been sent, draining every PDU on the way.
+  auto runUntilEofSent = [&]() {
+    REQUIRE(sourceHandler.transactionStart(putRequest, cfg) == OK);
+    REQUIRE(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+    sourceHandler.stateMachineNoPacket();
+    expectDirective(FileDirective::METADATA);
+    for (size_t offset = 0; offset < fileData.size(); offset += MAX_FILE_SEGMENT_SIZE) {
+      sourceHandler.stateMachineNoPacket();
+      expectFileData(offset, MAX_FILE_SEGMENT_SIZE);
+    }
+    sourceHandler.stateMachineNoPacket();
+    expectDirective(FileDirective::EOF_DIRECTIVE);
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK);
+  };
+
+  SECTION("Nominal acknowledged transfer") {
+    runUntilEofSent();
+    auto ackPdu = makeEofAckPdu();
+    sourceHandler.stateMachine(ackPdu);
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH);
+    auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE,
+                                       FileDeliveryStatus::RETAINED_IN_FILESTORE);
+    sourceHandler.stateMachine(finishedPdu);
+    // The Finished PDU is acknowledged and the transaction completes.
+    expectDirective(FileDirective::ACK);
+    CHECK(sourceHandler.getState() == CfdpState::IDLE);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    const auto& params = userMock.finishedRecvd.back().second;
+    CHECK(params.condCode == ConditionCode::NO_ERROR);
+    CHECK(params.deliveryCode == FileDeliveryCode::DATA_COMPLETE);
+    CHECK(params.status == FileDeliveryStatus::RETAINED_IN_FILESTORE);
+  }
+
+  SECTION("Finished PDU condition code is reported instead of a hardcoded success") {
+    runUntilEofSent();
+    auto ackPdu = makeEofAckPdu();
+    sourceHandler.stateMachine(ackPdu);
+    auto finishedPdu =
+        makeFinishedPdu(ConditionCode::FILE_CHECKSUM_FAILURE, FileDeliveryCode::DATA_INCOMPLETE,
+                        FileDeliveryStatus::DISCARDED_DELIBERATELY);
+    sourceHandler.stateMachine(finishedPdu);
+    expectDirective(FileDirective::ACK);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    const auto& params = userMock.finishedRecvd.back().second;
+    CHECK(params.condCode == ConditionCode::FILE_CHECKSUM_FAILURE);
+    CHECK(params.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE);
+  }
+
+  SECTION("EOF PDU is retransmitted on positive ACK timeout") {
+    runUntilEofSent();
+    for (uint32_t idx = 0; idx < cfg.positiveAckTimerExpirationLimit; idx++) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(5));
+      sourceHandler.stateMachineNoPacket();
+      expectDirective(FileDirective::EOF_DIRECTIVE);
+      CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+    }
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+    sourceHandler.stateMachineNoPacket();
+    CHECK(sourceHandler.getState() == CfdpState::IDLE);
+    auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR);
+    REQUIRE(fhInfo.callCount == 1);
+    CHECK(fhInfo.condCodes.front() == ConditionCode::POSITIVE_ACK_LIMIT_REACHED);
+    REQUIRE(userMock.finishedRecvd.size() == 1);
+    CHECK(userMock.finishedRecvd.back().second.condCode ==
+          ConditionCode::POSITIVE_ACK_LIMIT_REACHED);
+  }
+
+  SECTION("NAK PDU drives segment retransmission") {
+    runUntilEofSent();
+    // Two gaps, the first of which spans more than one file segment.
+    auto nakPdu = makeNakPdu({{256, 768}, {896, 1024}});
+    sourceHandler.stateMachine(nakPdu);
+    // One PDU per state machine call, exactly like the regular file data phase.
+    expectFileData(256, MAX_FILE_SEGMENT_SIZE);
+    sourceHandler.stateMachineNoPacket();
+    expectFileData(512, MAX_FILE_SEGMENT_SIZE);
+    sourceHandler.stateMachineNoPacket();
+    expectFileData(896, 128);
+    // Nothing is outstanding any more, so the handler is back to guarding the EOF PDU.
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK);
+    auto ackPdu = makeEofAckPdu();
+    sourceHandler.stateMachine(ackPdu);
+    auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE,
+                                       FileDeliveryStatus::RETAINED_IN_FILESTORE);
+    sourceHandler.stateMachine(finishedPdu);
+    expectDirective(FileDirective::ACK);
+    CHECK(sourceHandler.getState() == CfdpState::IDLE);
+  }
+
+  SECTION("A NAK keeps the inactivity guard from firing during recovery") {
+    // After the EOF is acknowledged the positive ACK timer is re-armed as an inactivity guard for
+    // the Finished PDU. In deferred NAK mode the receiver cannot send one until it has every
+    // segment, so between NAK rounds it is deliberately silent. Every serviced NAK has to reset
+    // the guard, otherwise it fires while recovery is still working.
+    runUntilEofSent();
+    auto ackPdu = makeEofAckPdu();
+    sourceHandler.stateMachine(ackPdu);
+    REQUIRE(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH);
+
+    // Several rounds of: the guard expires while the receiver is quiet, then a NAK arrives and is
+    // answered. Enough rounds that the expiries alone would exceed the limit.
+    for (uint32_t round = 0; round < cfg.positiveAckTimerExpirationLimit + 2; round++) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(5));
+      // No retransmission outstanding, so this call reaches the guard rather than returning early.
+      sourceHandler.stateMachineNoPacket();
+      REQUIRE(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+
+      auto nakPdu = makeNakPdu({{256, 512}});
+      sourceHandler.stateMachine(nakPdu);
+      expectFileData(256, MAX_FILE_SEGMENT_SIZE);
+      // Answering a NAK puts the handler back to guarding the EOF, so acknowledge it again to
+      // return to the state the guard belongs to.
+      auto reAck = makeEofAckPdu();
+      sourceHandler.stateMachine(reAck);
+    }
+    CHECK(fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR).callCount == 0);
+    CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+
+    // The guard still fires when the receiver really has gone silent.
+    for (uint32_t idx = 0; idx < cfg.positiveAckTimerExpirationLimit + 2; idx++) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(5));
+      sourceHandler.stateMachineNoPacket();
+    }
+    CHECK(sourceHandler.getState() == CfdpState::IDLE);
+    CHECK(fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR).condCodes.front() ==
+          ConditionCode::INACTIVITY_DETECTED);
+  }
+
+  SECTION("NAK PDU of scope 0 to 0 retransmits the metadata PDU") {
+    runUntilEofSent();
+    auto nakPdu = makeNakPdu({{0, 0}});
+    sourceHandler.stateMachine(nakPdu);
+    expectDirective(FileDirective::METADATA);
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK);
+  }
+
+  SECTION("A failed retransmission send is retried, not skipped") {
+    // Same contract as "File data PDU send failure is retried, not dropped" for the forward
+    // path: a downstream send failure must leave the requested segment outstanding. The peer
+    // only re-requests it on its NAK timer, so dropping it here burns a NAK limit credit and,
+    // once the limit is reached, loses the transfer.
+    runUntilEofSent();
+    auto nakPdu = makeNakPdu({{256, 512}});
+    pduSender.failNextSend = true;
+    sourceHandler.stateMachine(nakPdu);
+    CHECK(not pduSender.getNextSentPacket().has_value());
+
+    // The retransmission is still outstanding, so the next call has to send that same segment.
+    sourceHandler.stateMachineNoPacket();
+    expectFileData(256, MAX_FILE_SEGMENT_SIZE);
+  }
+
+  SECTION("A failed metadata retransmission is retried, not dropped") {
+    // The scope 0 to 0 request is the receiver saying it never got the metadata PDU and cannot
+    // write the file at all. Losing the answer to a full TM store strands the whole transfer.
+    runUntilEofSent();
+    auto nakPdu = makeNakPdu({{0, 0}});
+    pduSender.failNextSend = true;
+    sourceHandler.stateMachine(nakPdu);
+    CHECK(not pduSender.getNextSentPacket().has_value());
+
+    sourceHandler.stateMachineNoPacket();
+    expectDirective(FileDirective::METADATA);
+  }
+
+  SECTION("A NAK with more segment requests than fit still answers the ones which do") {
+    // NakPduReader fills the caller's array incrementally but reports a segment request length
+    // of 0 when it runs out of room, so the handler must not rely on that length alone. A peer
+    // which packs more requests into one NAK than RetransmitState can hold otherwise gets no
+    // retransmission at all.
+    runUntilEofSent();
+    std::vector<std::pair<uint64_t, uint64_t>> segments;
+    for (uint64_t idx = 0; idx < 33; idx++) {
+      segments.emplace_back(idx * 2, idx * 2 + 1);
+    }
+    auto nakPdu = makeNakPdu(segments);
+    sourceHandler.stateMachine(nakPdu);
+    expectFileData(0, 1);
+  }
+
+  SECTION("An unparseable Finished PDU does not complete the transaction") {
+    // A Finished PDU truncated before its condition code byte cannot say anything about how the
+    // transfer went. Completing on it reports the default-constructed DATA_COMPLETE, i.e. a
+    // fabricated success, and discards the state the peer's retransmission would have landed in.
+    runUntilEofSent();
+    auto eofAck = makeEofAckPdu();
+    sourceHandler.stateMachine(eofAck);
+    REQUIRE(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH);
+
+    auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE,
+                                       FileDeliveryStatus::RETAINED_IN_FILESTORE);
+    // Cut the PDU short so the header declares more bytes than are actually present, which is
+    // what a truncated uplink looks like.
+    finishedPdu.rawPdu.resize(finishedPdu.rawPdu.size() - 1);
+    sourceHandler.stateMachine(finishedPdu);
+
+    CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+    CHECK(userMock.finishedRecvd.empty());
+  }
+
+  SECTION("An ACK PDU for a different transaction is ignored") {
+    // Nothing upstream filters by transaction: CfdpHandler routes on direction alone. A late ACK
+    // from a previous transaction therefore lands in whichever one is running now.
+    runUntilEofSent();
+    peerConf.seqNum = TransactionSeqNum(UnsignedByteField<uint16_t>(42));
+    auto staleAck = makeEofAckPdu();
+    sourceHandler.stateMachine(staleAck);
+    CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK);
+  }
+
+  SECTION("A Finished PDU for a different transaction is ignored") {
+    runUntilEofSent();
+    auto eofAck = makeEofAckPdu();
+    sourceHandler.stateMachine(eofAck);
+    REQUIRE(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH);
+
+    peerConf.seqNum = TransactionSeqNum(UnsignedByteField<uint16_t>(42));
+    auto staleFinished = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE,
+                                         FileDeliveryStatus::RETAINED_IN_FILESTORE);
+    sourceHandler.stateMachine(staleFinished);
+    CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED);
+    CHECK(userMock.finishedRecvd.empty());
+  }
+}

unittests/cfdp/pdu/PduCrcHelper.h

@@ -0,0 +1,38 @@
+#ifndef FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_
+#define FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_
+
+#include <cstddef>
+#include <cstdint>
+
+#include "fsfw/globalfunctions/CRC.h"
+
+namespace cfdp::test {
+
+/**
+ * Turn an already serialized PDU without a CRC into one carrying a PDU CRC, in place.
+ *
+ * The PDU creators cannot produce this themselves: they append the two CRC bytes without counting
+ * them in the directive data field length, so the PDU they emit is inconsistent with its own
+ * header. Ground implementations do set the flag - cfdppy does by default - so the readers have to
+ * cope with it, and building the reference bytes here keeps these tests independent of that
+ * separate creator defect.
+ *
+ * Sets the CRC flag in the first header byte, grows the PDU data field length by two and appends
+ * the CRC-16/CCITT over the whole PDU, so that a CRC run across the result yields zero - which is
+ * what PduHeaderReader::performCrcCheckIfApplicable checks.
+ */
+inline void addPduCrc(uint8_t* buf, size_t& size) {
+  buf[0] |= 0x02;
+  auto dataFieldLen = static_cast<uint16_t>((buf[1] << 8) | buf[2]);
+  dataFieldLen += 2;
+  buf[1] = (dataFieldLen >> 8) & 0xff;
+  buf[2] = dataFieldLen & 0xff;
+  uint16_t crc = CRC::crc16ccitt(buf, size);
+  buf[size] = (crc >> 8) & 0xff;
+  buf[size + 1] = crc & 0xff;
+  size += 2;
+}
+
+}  // namespace cfdp::test
+
+#endif /* FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_ */

unittests/cfdp/pdu/testFinishedPdu.cpp

@@ -1,6 +1,7 @@
 #include <array>
 #include <catch2/catch_test_macros.hpp>
 
+#include "PduCrcHelper.h"
 #include "fsfw/cfdp/pdu/FinishedPduCreator.h"
 #include "fsfw/cfdp/pdu/FinishedPduReader.h"
 #include "fsfw/globalfunctions/arrayprinter.h"
@@ -187,3 +188,53 @@ TEST_CASE("Finished PDU", "[cfdp][pdu]") {
     }
   }
 }
+
+TEST_CASE("Finished PDU with CRC", "[cfdp][pdu]") {
+  using namespace cfdp;
+  std::array<uint8_t, 256> fnBuffer = {};
+  uint8_t* buffer = fnBuffer.data();
+  size_t sz = 0;
+  EntityId destId(WidthInBytes::TWO_BYTES, 2);
+  TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15);
+  EntityId sourceId(WidthInBytes::TWO_BYTES, 1);
+  PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum);
+
+  SECTION("Nominal completion") {
+    // The case every acknowledged transfer ends on: no filestore responses and no fault location,
+    // so the CRC is the only thing following the condition code byte. The TLV loop must not
+    // parse it as a TLV.
+    FinishedInfo info(cfdp::ConditionCode::NO_ERROR, cfdp::FileDeliveryCode::DATA_COMPLETE,
+                      cfdp::FileDeliveryStatus::RETAINED_IN_FILESTORE);
+    FinishPduCreator creator(pduConf, info);
+    REQUIRE(creator.serialize(&buffer, &sz, fnBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(fnBuffer.data(), sz);
+
+    FinishedInfo infoDeser;
+    FinishPduReader reader(fnBuffer.data(), sz, infoDeser);
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(reader.getCrcFlag());
+    REQUIRE(infoDeser.getConditionCode() == cfdp::ConditionCode::NO_ERROR);
+    REQUIRE(infoDeser.getDeliveryCode() == cfdp::FileDeliveryCode::DATA_COMPLETE);
+    REQUIRE(infoDeser.getFileStatus() == cfdp::FileDeliveryStatus::RETAINED_IN_FILESTORE);
+  }
+
+  SECTION("With fault location") {
+    EntityIdTlv faultLoc(destId);
+    FinishedInfo info(cfdp::ConditionCode::FILESTORE_REJECTION,
+                      cfdp::FileDeliveryCode::DATA_INCOMPLETE,
+                      cfdp::FileDeliveryStatus::DISCARDED_DELIBERATELY);
+    info.setFaultLocation(&faultLoc);
+    FinishPduCreator creator(pduConf, info);
+    REQUIRE(creator.serialize(&buffer, &sz, fnBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(fnBuffer.data(), sz);
+
+    EntityIdTlv faultLocDeser(destId);
+    FinishedInfo infoDeser;
+    infoDeser.setFaultLocation(&faultLocDeser);
+    FinishPduReader reader(fnBuffer.data(), sz, infoDeser);
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(infoDeser.getConditionCode() == cfdp::ConditionCode::FILESTORE_REJECTION);
+  }
+}

unittests/cfdp/pdu/testMetadataPdu.cpp

@@ -2,6 +2,7 @@
 #include <catch2/catch_test_macros.hpp>
 #include <iostream>
 
+#include "PduCrcHelper.h"
 #include "fsfw/cfdp/pdu/MetadataPduCreator.h"
 #include "fsfw/cfdp/pdu/MetadataPduReader.h"
 #include "fsfw/cfdp/tlv/FilestoreResponseTlv.h"
@@ -221,4 +222,68 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") {
       }
     }
   }
+
+  SECTION("More options than the option array holds") {
+    MetadataPduCreator serializer(pduConf, info, sourceFileName, destFileName, options.data(),
+                                  options.size());
+    serializer.updateDirectiveFieldLen();
+    result = serializer.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK);
+    REQUIRE(result == returnvalue::OK);
+
+    MetadataPduReader reader(mdBuffer.data(), mdBuffer.size(), info, tlvDeser.data(), 1);
+    REQUIRE(reader.parseData() == cfdp::METADATA_CANT_PARSE_OPTIONS);
+  }
+}
+
+TEST_CASE("Metadata PDU with CRC", "[cfdp][pdu]") {
+  using namespace cfdp;
+  std::array<uint8_t, 256> mdBuffer = {};
+  uint8_t* buffer = mdBuffer.data();
+  size_t sz = 0;
+  EntityId destId(WidthInBytes::TWO_BYTES, 2);
+  TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15);
+  EntityId sourceId(WidthInBytes::TWO_BYTES, 1);
+  PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum);
+  Fss fileSize(0);
+  MetadataGenericInfo info(false, ChecksumType::NULL_CHECKSUM, fileSize);
+  std::array<Tlv, 5> tlvDeser{};
+
+  SECTION("Metadata only with message to user") {
+    // This is the shape of a proxy put request: no file names, the request itself travels in a
+    // message to user TLV. The option loop must not deserialize the CRC as a further TLV.
+    cfdp::StringLv emptySourceName;
+    cfdp::StringLv emptyDestName;
+    std::array<uint8_t, 3> msg = {0x41, 0x42, 0x43};
+    MessageToUserTlv msgToUser(msg.data(), msg.size());
+    std::array<Tlv*, 1> options{&msgToUser};
+    MetadataPduCreator creator(pduConf, info, emptySourceName, emptyDestName, options.data(),
+                               options.size());
+    creator.updateDirectiveFieldLen();
+    REQUIRE(creator.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(mdBuffer.data(), sz);
+
+    MetadataPduReader reader(mdBuffer.data(), sz, info, tlvDeser.data(), tlvDeser.max_size());
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(reader.getCrcFlag());
+    REQUIRE(reader.getNumberOfParsedOptions() == 1);
+    REQUIRE(tlvDeser[0].getType() == cfdp::TlvType::MSG_TO_USER);
+    REQUIRE(tlvDeser[0].getLengthField() == msg.size());
+  }
+
+  SECTION("No options") {
+    // The CRC is all that follows the file names here.
+    std::string name = "hello.txt";
+    cfdp::StringLv sourceFileName(name);
+    cfdp::StringLv destFileName(name);
+    MetadataPduCreator creator(pduConf, info, sourceFileName, destFileName, nullptr, 0);
+    creator.updateDirectiveFieldLen();
+    REQUIRE(creator.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(mdBuffer.data(), sz);
+
+    MetadataPduReader reader(mdBuffer.data(), sz, info, tlvDeser.data(), tlvDeser.max_size());
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(reader.getNumberOfParsedOptions() == 0);
+  }
 }

unittests/cfdp/pdu/testNakPdu.cpp

@@ -1,6 +1,7 @@
 #include <array>
 #include <catch2/catch_test_macros.hpp>
 
+#include "PduCrcHelper.h"
 #include "fsfw/cfdp/pdu/NakPduCreator.h"
 #include "fsfw/cfdp/pdu/NakPduReader.h"
 #include "fsfw/cfdp/pdu/PduConfig.h"
@@ -150,3 +151,54 @@ TEST_CASE("NAK PDU", "[cfdp][pdu]") {
     REQUIRE(info.getSegmentRequestsMaxLen() == 5);
   }
 }
+
+TEST_CASE("NAK PDU with CRC", "[cfdp][pdu]") {
+  using namespace cfdp;
+  std::array<uint8_t, 256> nakBuffer = {};
+  uint8_t* buffer = nakBuffer.data();
+  size_t sz = 0;
+  EntityId destId(WidthInBytes::TWO_BYTES, 2);
+  TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15);
+  EntityId sourceId(WidthInBytes::TWO_BYTES, 1);
+  PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum);
+  Fss startOfScope(50);
+  Fss endOfScope(1050);
+  NakInfo info(startOfScope, endOfScope);
+
+  SECTION("With segment requests") {
+    std::array<NakInfo::SegmentRequest, 2> segReqs{
+        NakInfo::SegmentRequest(cfdp::Fss(2020), cfdp::Fss(2520)),
+        NakInfo::SegmentRequest(cfdp::Fss(2932), cfdp::Fss(3021))};
+    size_t segReqLen = segReqs.size();
+    info.setSegmentRequests(segReqs.data(), &segReqLen, nullptr);
+    NakPduCreator creator(pduConf, info);
+    REQUIRE(creator.serialize(&buffer, &sz, nakBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(nakBuffer.data(), sz);
+
+    std::array<NakInfo::SegmentRequest, 4> segReqsDeser{};
+    NakInfo infoDeser(Fss(0), Fss(0));
+    size_t maxSegReqs = segReqsDeser.size();
+    infoDeser.setSegmentRequests(segReqsDeser.data(), nullptr, &maxSegReqs);
+    NakPduReader reader(nakBuffer.data(), sz, infoDeser);
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(reader.getCrcFlag());
+    REQUIRE(infoDeser.getSegmentRequestsLen() == 2);
+    REQUIRE(segReqsDeser[0].first.value() == 2020);
+    REQUIRE(segReqsDeser[1].second.value() == 3021);
+  }
+
+  SECTION("Without segment requests") {
+    NakPduCreator creator(pduConf, info);
+    REQUIRE(creator.serialize(&buffer, &sz, nakBuffer.size(), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+    cfdp::test::addPduCrc(nakBuffer.data(), sz);
+
+    NakInfo infoDeser(Fss(0), Fss(0));
+    NakPduReader reader(nakBuffer.data(), sz, infoDeser);
+    REQUIRE(reader.parseData() == returnvalue::OK);
+    REQUIRE(infoDeser.getStartOfScope().value() == 50);
+    REQUIRE(infoDeser.getEndOfScope().value() == 1050);
+    REQUIRE(infoDeser.getSegmentRequestsLen() == 0);
+  }
+}

unittests/datapool/testDataset.cpp

@@ -82,4 +82,52 @@ TEST_CASE("Pool Dataset Test", "[datapool]") {
     CHECK(buf[31] == 0b10000000);
     CHECK(buf[32] == 0);
   }
+
+  SECTION("Pool Dataset Deserialization With Validity") {
+    uint8_t buf[64]{};
+    TestDatasetLarger source;
+    source.setChildrenValidity(true);
+    source.test1 = 502392;
+    source.test1.setValid(false);
+    source.test7 = -5;
+    source.test8.entry[1] = 1234;
+    source.test8.setValid(false);
+    source.serializeWithValidityBlob = true;
+    size_t serLen = 0;
+    uint8_t* dataPtr = buf;
+    REQUIRE(source.serialize(&dataPtr, &serLen, sizeof(buf), SerializeIF::Endianness::NETWORK) ==
+            returnvalue::OK);
+
+    TestDatasetLarger target;
+    target.setChildrenValidity(true);
+    const uint8_t* readPtr = buf;
+    size_t remaining = serLen;
+    CHECK(target.deSerializeWithValidityBlob(&readPtr, &remaining,
+                                             SerializeIF::Endianness::NETWORK) == returnvalue::OK);
+    CHECK(remaining == 0);
+    CHECK(readPtr == buf + serLen);
+    CHECK(target.test0.isValid());
+    CHECK(!target.test1.isValid());
+    CHECK(target.test1.get() == 502392);
+    CHECK(target.test7.isValid());
+    CHECK(target.test7.get() == -5);
+    CHECK(!target.test8.isValid());
+    CHECK(target.test8.entry[1] == 1234);
+  }
+
+  SECTION("Pool Dataset Deserialization With Validity Short Stream") {
+    uint8_t buf[64]{};
+    TestDatasetSmall target;
+    target.test0 = 7;
+    target.test0.setValid(true);
+    const uint8_t* readPtr = buf;
+    // One byte short of the 5 value bytes and the 1 byte validity blob
+    size_t remaining = 5;
+    CHECK(target.deSerializeWithValidityBlob(&readPtr, &remaining,
+                                             SerializeIF::Endianness::NETWORK) ==
+          SerializeIF::STREAM_TOO_SHORT);
+    CHECK(remaining == 5);
+    CHECK(target.test0.get() == 7);
+    CHECK(target.test0.isValid());
+  }
 }

unittests/globalfunctions/CMakeLists.txt

@@ -1,3 +1,4 @@
 target_sources(
-  ${FSFW_TEST_TGT} PRIVATE testDleEncoder.cpp testOpDivider.cpp testBitutil.cpp
-                           testCRC.cpp testTimevalOperations.cpp)
+  ${FSFW_TEST_TGT}
+  PRIVATE testCobsEncoder.cpp testDleEncoder.cpp testOpDivider.cpp
+          testBitutil.cpp testCRC.cpp testTimevalOperations.cpp)

unittests/globalfunctions/testCobsEncoder.cpp

@@ -0,0 +1,336 @@
+#include <algorithm>
+#include <array>
+#include <cstdint>
+#include <vector>
+
+#include "catch2/catch_test_macros.hpp"
+#include "fsfw/globalfunctions/CobsEncoder.h"
+#include "fsfw/returnvalues/returnvalue.h"
+
+// no zero bytes
+constexpr std::array<uint8_t, 3> ti0 = {1, 2, 43};
+constexpr std::array<uint8_t, 5> to0 = {0x04, 0x01, 0x02, 0x2b, 0x00};
+
+// single zero byte
+constexpr std::array<uint8_t, 4> ti1 = {1, 2, 0, 3};
+constexpr std::array<uint8_t, 6> to1 = {3, 1, 2, 2, 3, 0};
+
+// multiple zero bytes in sequence
+constexpr std::array<uint8_t, 6> ti2 = {1, 2, 0, 0, 0, 3};
+constexpr std::array<uint8_t, 8> to2 = {3, 1, 2, 1, 1, 2, 3, 0};
+
+// stuffing bytes required because of the length without a single zero byte
+// clang-format off
+constexpr std::array<uint8_t, 257> ti3 = {
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  // #257
+  0xaa,
+};
+constexpr std::array<uint8_t, 260> to3 = {
+  0xff,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
+  1, 2, 3, 4,5 , 6, 7, 8, 9, 10, 11, 12, 13, 14,
+  // #255 -> stuffing
+  0x04,
+  15, 16,
+  0xaa, 0
+};
+// clang-format on
+
+// real packet data
+
+// PLOC startup packets (usually in one or two messages, but 3 separate COBS encoded packets)
+// clang-format off
+constexpr std::array<uint8_t, 36> ploc_startup_1_i = {
+  0x08,0x42,0xc0,0x00,0x00,0x1d,0x20,0x05,0x02,0x00,0x00,0x00,0x00,0x40,0x61,0x0d,0x02,0x8e,
+  0xb4,0xef,0x2e,0xe1,0x44,0x00,0x1f,0x03,0x00,0x00,0x52,0x18,0x00,0x00,0x00,0x00,0x1b,0xe2,
+};
+constexpr std::array<uint8_t, 36> ploc_startup_2_i = {
+  0x08,0x42,0xc0,0x01,0x00,0x1d,0x20,0x05,0x02,0x00,0x01,0x00,0x00,0x40,0x61,0x0d,0x02,0x8e,
+  0xb4,0xef,0x30,0x74,0x44,0x00,0x1f,0x07,0x00,0x00,0x50,0x03,0x00,0x00,0x00,0x00,0x98,0xe3,
+};
+constexpr std::array<uint8_t, 36> ploc_startup_3_i = {
+  0x08,0x42,0xc0,0x02,0x00,0x1d,0x20,0x05,0x01,0x00,0x02,0x00,0x00,0x40,0x61,0x0d,0x02,0x8e,
+  0xb5,0xb8,0x30,0x73,0x44,0x00,0x1f,0x07,0x00,0x00,0x3b,0x04,0x00,0x00,0x00,0x00,0x22,0x68,
+};
+constexpr std::array<uint8_t, 38> ploc_startup_1_o = {
+  0x04,0x08,0x42,0xc0,0x01,0x05,0x1d,0x20,0x05,0x02,
+  0x01,0x01,0x01,0x0b,0x40,0x61,0x0d,0x02,0x8e,0xb4,
+  0xef,0x2e,0xe1,0x44,0x03,0x1f,0x03,0x01,0x03,0x52,
+  0x18,0x01,0x01,0x01,0x03,0x1b,0xe2, 0x00,
+};
+constexpr std::array<uint8_t, 38> ploc_startup_2_o = {
+  0x05, 0x08,
+  0x42,0xc0,0x01,0x05,0x1d,0x20,0x05,0x02,0x02,0x01,
+  0x01,0x0b,0x40,0x61,0x0d,0x02,0x8e,0xb4,0xef,0x30,
+  0x74,0x44,0x03,0x1f,0x07,0x01,0x03,0x50,0x03,0x01,
+  0x01,0x01,0x03,0x98,0xe3,0x00
+};
+constexpr std::array<uint8_t, 38> ploc_startup_3_o = {
+  0x05,0x08,0x42,0xc0,0x02,0x05,0x1d,0x20,0x05,0x01,
+  0x02,0x02,0x01,0x0b,0x40,0x61,0x0d,0x02,0x8e,0xb5,
+  0xb8,0x30,0x73,0x44,0x03,0x1f,0x07,0x01,0x03,0x3b,
+  0x04,0x01,0x01,0x01,0x03,0x22,0x68,0x00,
+};
+// clang-format on
+
+// PLOC Ping (sent from OBC)
+// clang-format off
+constexpr std::array<uint8_t, 13> ploc_ping_i = {
+  // as received by PLOC
+  0x18,0x42,0xc0,0x00,0x00,0x06,0x2f,0x11,0x01,0x00,
+  0x00,0xd5,0xc8,
+};
+constexpr std::array<uint8_t, 15> ploc_ping_o = {
+  // as sent from OBC
+  0x04,0x18,0x42,0xc0,0x01,0x05,0x06,0x2f,0x11,0x01,0x01,0x03,0xd5,0xc8,0x00,
+};
+// clang-format on
+// Corresponding PLOC Pong (sent from PLOC)
+// clang-format off
+constexpr std::array<uint8_t, 26> ploc_pong_i = {
+  // as received by OBC
+  0x08,0x42,0xc0,0x05,0x00,0x13,0x20,0x01,0x07,0x00,
+  0x01,0x00,0x00,0x40,0x61,0x0d,0x02,0x98,0xbd,0xab,
+  0x18,0x42,0xc0,0x00,0x13,0xd5,
+};
+constexpr std::array<uint8_t, 28> ploc_pong_o = {
+  // as sent by PLOC
+  0x05,0x08,0x42,0xc0,0x05,0x05,0x13,0x20,0x01,0x07,
+  0x02,0x01,0x01,0x0b,0x40,0x61,0x0d,0x02,0x98,0xbd,
+  0xab,0x18,0x42,0xc0,0x03,0x13,0xd5,0x00,
+};
+// clang-format on
+
+#define FOR_EACH_TEST_ARRAY(macro)           \
+  macro(ti0, to0);                           \
+  macro(ti1, to1);                           \
+  macro(ti2, to2);                           \
+  macro(ti3, to3);                           \
+  macro(ploc_startup_1_i, ploc_startup_1_o); \
+  macro(ploc_startup_2_i, ploc_startup_2_o); \
+  macro(ploc_startup_3_i, ploc_startup_3_o); \
+  macro(ploc_ping_i, ploc_ping_o);           \
+  macro(ploc_pong_i, ploc_pong_o);
+
+template <typename FI, typename FE, typename SI>
+void check_iter_equal(FI first_begin, FE first_end, SI second_begin) {
+  auto first_iter = first_begin;
+  auto second_iter = second_begin;
+  auto i = 0;
+  while (first_iter < first_end) {
+    CHECK(*first_iter == *second_iter);
+    ++first_iter;
+    ++second_iter;
+    ++i;
+  }
+
+  // safety check
+  CHECK(std::equal(first_begin, first_end, second_begin));
+}
+
+#define ENCODE_TEST(input, output)                                                  \
+  TEST_CASE("COBS encode " #input, "[cobs]") {                                      \
+    std::vector<uint8_t> outstream;                                                 \
+    outstream.resize(output.size());                                                \
+    size_t encodedLen = 0;                                                          \
+    auto result = CobsEncoder::encode(input.data(), input.size(), outstream.data(), \
+                                      outstream.size(), &encodedLen);               \
+    if (result == returnvalue::OK) {                                                \
+      CHECK(encodedLen == output.size());                                           \
+      check_iter_equal(outstream.begin(), outstream.end(), output.begin());         \
+    } else {                                                                        \
+      CHECK(result == returnvalue::OK);                                             \
+    }                                                                               \
+  }
+
+FOR_EACH_TEST_ARRAY(ENCODE_TEST)
+#undef ENCODE_TEST
+
+#define DECODE_TEST(input, output)                                                             \
+  TEST_CASE("COBS decode " #output, "[cobs]") {                                                \
+    std::vector<uint8_t> instream;                                                             \
+    instream.resize(input.size());                                                             \
+    size_t decodedLen = 0;                                                                     \
+    size_t readLen = 0;                                                                        \
+    auto result = CobsEncoder::decode(output.data(), output.size(), &readLen, instream.data(), \
+                                      instream.size(), &decodedLen);                           \
+    if (result == returnvalue::OK) {                                                           \
+      CHECK(readLen == output.size());                                                         \
+      CHECK(decodedLen == input.size());                                                       \
+      check_iter_equal(instream.begin(), instream.end(), input.begin());                       \
+    } else {                                                                                   \
+      CHECK(result == returnvalue::OK);                                                        \
+    }                                                                                          \
+  }
+
+FOR_EACH_TEST_ARRAY(DECODE_TEST)
+#undef DECODE_TEST
+
+// payloads far past any block count round-trip and the worst case bound is tight enough to hold
+// them
+TEST_CASE("Large payload", "[cobs]") {
+  // no zero bytes at all, so this needs the maximum number of blocks the input length allows
+  std::vector<uint8_t> input(64 * 1024, 1);
+  std::vector<uint8_t> encoded(CobsEncoder::worstCaseEncodedLen(input.size()));
+  size_t encodedLen = 0;
+
+  auto result =
+      CobsEncoder::encode(input.data(), input.size(), encoded.data(), encoded.size(), &encodedLen);
+  REQUIRE(result == returnvalue::OK);
+  CHECK(encodedLen == encoded.size());
+
+  std::vector<uint8_t> decoded(input.size());
+  size_t decodedLen = 0;
+  size_t readLen = 0;
+  result = CobsEncoder::decode(encoded.data(), encodedLen, &readLen, decoded.data(), decoded.size(),
+                               &decodedLen);
+  REQUIRE(result == returnvalue::OK);
+  CHECK(readLen == encodedLen);
+  REQUIRE(decodedLen == input.size());
+  CHECK(std::equal(decoded.begin(), decoded.end(), input.begin()));
+}
+
+// check that too small target buffers are not blindly used
+TEST_CASE("Insufficient space", "[cobs]") {
+  std::vector<uint8_t> output{0, 2};
+  size_t encodedLen = 0;
+
+  auto result =
+      CobsEncoder::encode(ti1.data(), ti1.size(), output.data(), output.size(), &encodedLen);
+  CHECK(result == CobsEncoder::INSUFFICIENT_SPACE);
+
+  // even equal size should not work
+  output.resize(ti1.size());
+  result = CobsEncoder::encode(ti1.data(), ti1.size(), output.data(), output.size(), &encodedLen);
+  CHECK(result == CobsEncoder::INSUFFICIENT_SPACE);
+
+  // check reverse direction for decoding
+  output.resize(2);
+  size_t readLen;
+  size_t decodedLen;
+  result = CobsEncoder::decode(to1.data(), to1.size(), &readLen, output.data(), output.size(),
+                               &decodedLen);
+  CHECK(result == CobsEncoder::INSUFFICIENT_SPACE);
+}
+
+TEST_CASE("Malformed COBS data", "[cobs]") {
+  // decode a cut-off packet
+  std::vector<uint8_t> input{std::begin(to2), std::end(to2)};
+  input.resize(to2.size() / 2);
+
+  std::vector<uint8_t> output;
+  output.resize(ti2.size() * 2);
+  size_t encodedLen;
+  size_t readLen;
+
+  auto result = CobsEncoder::decode(input.data(), input.size(), &readLen, output.data(),
+                                    output.size(), &encodedLen);
+  CHECK(result == CobsEncoder::STREAM_TOO_SHORT);
+
+  // inject a zero byte somewhere in the middle of a list of nonzero bytes
+  input = {10, 1, 2, 3, 4, 0, 6, 7, 8, 9, 0};
+  result = CobsEncoder::decode(input.data(), input.size(), &readLen, output.data(), output.size(),
+                               &encodedLen);
+  CHECK(result == CobsEncoder::DECODING_ERROR);
+  // the whole broken frame is skipped, up to and including its delimiter
+  CHECK(readLen == 6);
+}
+
+// a corrupt frame must not hold back the intact frames queued behind it
+TEST_CASE("Resynchronisation after a corrupt frame", "[cobs]") {
+  // a block claiming far more bytes than the frame holds, terminated like a real frame
+  std::vector<uint8_t> stream{0x40, 0x11, 0x22, 0x00};
+  const size_t corruptLen = stream.size();
+  stream.insert(stream.end(), to1.begin(), to1.end());
+
+  std::vector<uint8_t> output(ti1.size());
+  size_t readLen = 0;
+  size_t decodedLen = 0;
+
+  auto result = CobsEncoder::decode(stream.data(), stream.size(), &readLen, output.data(),
+                                    output.size(), &decodedLen);
+  REQUIRE(result == CobsEncoder::DECODING_ERROR);
+  REQUIRE(readLen == corruptLen);
+
+  // the next frame decodes from where the corrupt one ended
+  result = CobsEncoder::decode(stream.data() + readLen, stream.size() - readLen, &readLen,
+                               output.data(), output.size(), &decodedLen);
+  REQUIRE(result == returnvalue::OK);
+  CHECK(readLen == to1.size());
+  REQUIRE(decodedLen == ti1.size());
+  CHECK(std::equal(output.begin(), output.end(), ti1.begin()));
+}
+
+// without a delimiter the frame may simply still be arriving, so nothing may be consumed
+TEST_CASE("Incomplete frame is not consumed", "[cobs]") {
+  std::vector<uint8_t> output(16);
+  size_t readLen = 0xdead;
+  size_t decodedLen = 0xdead;
+
+  // a self-consistent frame that is merely missing its delimiter
+  std::vector<uint8_t> input{3, 1, 2, 2, 3};
+  auto result = CobsEncoder::decode(input.data(), input.size(), &readLen, output.data(),
+                                    output.size(), &decodedLen);
+  CHECK(result == CobsEncoder::STREAM_TOO_SHORT);
+  CHECK(readLen == 0);
+  CHECK(decodedLen == 0);
+
+  // once the delimiter arrives, the very same bytes decode
+  input.push_back(0);
+  result = CobsEncoder::decode(input.data(), input.size(), &readLen, output.data(), output.size(),
+                               &decodedLen);
+  REQUIRE(result == returnvalue::OK);
+  CHECK(readLen == input.size());
+  CHECK(decodedLen == ti1.size());
+}
+
+// an empty payload survives a round trip and is reported as a zero length frame
+TEST_CASE("Empty frame", "[cobs]") {
+  std::vector<uint8_t> encoded(CobsEncoder::worstCaseEncodedLen(0));
+  size_t encodedLen = 0;
+
+  auto result = CobsEncoder::encode(nullptr, 0, encoded.data(), encoded.size(), &encodedLen);
+  REQUIRE(result == returnvalue::OK);
+  REQUIRE(encodedLen == 2);
+  CHECK(encoded[0] == 0x01);
+  CHECK(encoded[1] == 0x00);
+
+  std::vector<uint8_t> output(4);
+  size_t readLen = 0;
+  size_t decodedLen = 0;
+  result = CobsEncoder::decode(encoded.data(), encodedLen, &readLen, output.data(), output.size(),
+                               &decodedLen);
+  CHECK(result == returnvalue::OK);
+  CHECK(readLen == 2);
+  CHECK(decodedLen == 0);
+}

unittests/tmtcpacket/testPusTmReader.cpp

@@ -38,7 +38,7 @@ TEST_CASE("PUS TM Reader", "[pus-tm-reader]") {
       readerPtr->setTimeReader(&timeStamperAndReader);
       deleteReader = true;
     }
-    REQUIRE(not *readerPtr);
+    REQUIRE(not*readerPtr);
     REQUIRE(readerPtr->isNull());
     REQUIRE(readerPtr->parseDataWithCrcCheck() == returnvalue::OK);
     REQUIRE(not readerPtr->isNull());