Update and clean up HK and Local Pool Modules

CHANGELOG.md

@@ -26,8 +26,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Added
 
-- 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.
@@ -36,21 +34,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## 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
+- Complete refactoring 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.
-  It takes care of tracking the HK generation using a set specification provided by the user.n
-  However, it leaves serialization of the HK data completely to the developer. This removes a major
-  constraint on the format of the HK data, which was previously constrained to implementors of a
-  certain base class.
-- The former set classes and pool objects are still available for HK set specification and
-  generation. The API has changed, but the general usage and their architecture has not.
-- A new set of set classes and helper objects to specify HK sets and data which does not need to be
-  shared was added as well. The majority of datasets do not need to be shared anyway.
-- The non-shared API retain the capability of appending of a validity blob for each piece of set
-  data at the end of the HK data. For both non-shared and shared data, this capability can be
-  specified in the constructor, and defaults to true.
 - Improved File System Abstraction to be more in line with normal filesystems.
 - CFDP implementation was improved, has now even less dependencies on other FSFW components
   and allows one inserted packet per state machine call.
@@ -70,8 +55,7 @@ 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.

CMakeLists.txt

@@ -142,7 +142,7 @@ if(FSFW_BUILD_TESTS)
   configure_file(unittests/testcfg/TestsConfig.h.in tests/TestsConfig.h)
 
   project(${FSFW_TEST_TGT} CXX C)
-  add_executable(${FSFW_TEST_TGT})
+  add_executable(${FSFW_TEST_TGT} unittests/datapool/testDataset.cpp)
   if(IPO_SUPPORTED AND FSFW_ENABLE_IPO)
     set_property(TARGET ${FSFW_TEST_TGT} PROPERTY INTERPROCEDURAL_OPTIMIZATION
                                                   TRUE)

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,7 +71,6 @@ 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-19"
+cpp_format="clang-format"
 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 ${cpp_format} --style=file -i
+    find ${dir} ${file_selectors} | xargs clang-format --style=file -i
   done
 else
   echo "No ${cpp_format} tool found, not formatting C++/C files"

src/fsfw/cfdp/handler/DestHandler.cpp

@@ -2,20 +2,14 @@
 
 #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;
@@ -29,13 +23,7 @@ cfdp::DestHandler::DestHandler(PduSenderIF& pduSender, size_t pduBufSize, DestHa
       msgToUserVec(params.maxTlvsInOnePdu),
       transactionParams(params.maxFilenameLen),
       destParams(std::move(params)),
-      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) {
+      fsfwParams(fsfwParams) {
   transactionParams.pduConf.direction = cfdp::Direction::TOWARDS_SENDER;
 }
 
@@ -52,31 +40,8 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine(
       return fsmRes;
     }
     PduPacketIF& pduPacket = *optPduPacket;
-    if (pduPacket.getPduType() == FILE_DIRECTIVE and
-        *pduPacket.getFileDirective() == EOF_DIRECTIVE) {
-      // D7 of the class 2 plan: 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) {
+    if (pduPacket.getPduType() == FILE_DATA or
+        (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA)) {
       fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU;
       return fsmRes;
     }
@@ -85,11 +50,6 @@ 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()) {
@@ -113,88 +73,20 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine(
     checkAndHandleError(result, errorIdx);
   }
   if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) {
-    // 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();
-    }
+    result = sendFinishedPdu();
+    checkAndHandleError(result, 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;
-      }
-    }
-  }
-  // D2: 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;
@@ -224,11 +116,6 @@ 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) {
@@ -261,13 +148,8 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) {
   }
   transactionParams.deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE;
   transactionParams.vfsErrorCount = 0;
-  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;
+  if (fdInfo.getOffset().value() + fileSegmentLen > transactionParams.progress) {
+    transactionParams.progress = fdInfo.getOffset().value() + fileSegmentLen;
   }
   return result;
 }
@@ -276,7 +158,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(&eofFaultLocation);
+  EofInfo eofInfo(nullptr);
   EofPduReader reader(rawPdu, pduSize, eofInfo);
   ReturnValue_t result = reader.parseData();
   if (result != OK) {
@@ -295,39 +177,12 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) {
   if (destParams.cfg.indicCfg.eofRecvIndicRequired) {
     destParams.user.eofRecvIndication(getTransactionId());
   }
-  if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
-    if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) {
+  if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) {
+    if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) {
       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;
 }
@@ -371,12 +226,8 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result,
 }
 
 ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reader) {
-  // 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) {
+  if (fsmRes.state != CfdpState::IDLE) {
+    // According to standard, discard metadata PDU if we are busy
     return OK;
   }
   ReturnValue_t result = OK;
@@ -445,21 +296,18 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade
 #endif
     return FAILED;
   }
-  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) {
-      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 (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;
   }
-  transactionParams.metadataReceived = true;
   auto& info = reader.getGenericInfo();
   transactionParams.checksumType = info.getChecksumType();
   transactionParams.closureRequested = info.isClosureRequested();
@@ -490,27 +338,13 @@ cfdp::CfdpState cfdp::DestHandler::getCfdpState() const { return fsmRes.state; }
 
 ReturnValue_t cfdp::DestHandler::handleTransferCompletion() {
   ReturnValue_t result;
-  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) {
+  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 - a proxy put
-      // request, say - carries no file data and completes the moment its metadata arrives. Both
-      // fields are otherwise left at the reset defaults, which report a successful transaction as
-      // "Data Incomplete" and "Discard deliberately". That contradicts the NO_ERROR condition code
-      // beside it, and it goes out in the Finished PDU, not just into the log.
-      transactionParams.deliveryCode = FileDeliveryCode::DATA_COMPLETE;
-      transactionParams.deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED;
-    }
   }
   result = noticeOfCompletion();
   if (result != OK) {
@@ -560,7 +394,6 @@ 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;
@@ -617,19 +450,6 @@ 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);
@@ -647,7 +467,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() {
     fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0);
     return result;
   }
-  result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen);
+  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;
@@ -659,33 +479,6 @@ 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;
 }
@@ -703,8 +496,6 @@ 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;
@@ -718,356 +509,4 @@ 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;
-  }
-  // D3: 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) {
-      // A7: 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;
-  // D5: 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);
-  }
-  removeReceivedRange(offset, endOfSegment);
-}
-
-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});
-}
-
-void cfdp::DestHandler::removeReceivedRange(uint64_t start, uint64_t end) {
-  if (end <= start) {
-    return;
-  }
-  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;
-  for (auto it = container.begin(); it != container.end();) {
-    if (it->second <= start or it->first >= end) {
-      ++it;
-      continue;
-    }
-    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]);
-  }
-}
-
-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,14 +14,10 @@
 #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 {
@@ -51,17 +47,6 @@ 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 {
@@ -72,13 +57,7 @@ class DestHandler {
     RECEIVING_FILE_DATA_PDUS = 2,
     SENDING_ACK_PDU = 3,
     TRANSFER_COMPLETION = 4,
-    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
+    SENDING_FINISHED_PDU = 5
   };
 
   struct FsmResult {
@@ -122,14 +101,8 @@ 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 {
@@ -153,29 +126,8 @@ 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;
@@ -201,20 +153,6 @@ 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);
@@ -222,37 +160,9 @@ 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);
-  void 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,10 +14,7 @@ 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)),
-      metadataOnly(false),
-      sourceName(std::move(sourceName)),
-      destName(std::move(destName)) {}
+    : destId(std::move(destId)), sourceName(std::move(sourceName)), destName(std::move(destName)) {}
 
 [[nodiscard]] bool cfdp::PutRequest::isMetadataOnly() const { return metadataOnly; }
 

src/fsfw/cfdp/handler/SourceHandler.cpp

@@ -4,15 +4,9 @@
 
 #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"
@@ -22,28 +16,12 @@
 
 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),
-      positiveAckTimer(0, false) {
+      fsfwParams(fsfwParams) {
   // The entity ID portion of the transaction ID will always remain fixed.
   transactionParams.id.entityId = sourceParams.cfg.localId;
   transactionParams.pduConf.sourceId = sourceParams.cfg.localId;
@@ -69,19 +47,8 @@ cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize,
   transactionParams.pduConf.seqNum.setValue(0);
 }
 
-cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked(
-    const std::optional<std::reference_wrapper<PduPacketIF>> optPduPacket) {
+cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() {
   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;
   }
@@ -119,11 +86,6 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked(
     }
     if (transactionParams.closureRequested) {
       step = TransactionStep::WAIT_FOR_FINISH;
-      // The Finished PDU is now actually waited for. Bound the wait with the positive ACK timer
-      // configuration so a lost Finished PDU cannot pin the handler: without closure the
-      // transaction completed immediately, so a hang here would be a regression.
-      transactionParams.positiveAckCounter = 0;
-      positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs);
       // fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY;
     } else {
       step = TransactionStep::NOTICE_OF_COMPLETION;
@@ -132,20 +94,7 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked(
     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;
-      }
-      // Unacknowledged mode has no retransmission, so the only thing left is to report that the
-      // peer never confirmed the transfer.
-      declareFault(ConditionCode::INACTIVITY_DETECTED);
-    }
+    // TODO: In case this is a request with closure, wait for finish.
     // Done, issue notice of completion
     step = TransactionStep::NOTICE_OF_COMPLETION;
   }
@@ -168,10 +117,7 @@ const cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::stateMachine(
     return fsmResult;
   }
   if (state == cfdp::CfdpState::BUSY_CLASS_1_NACKED) {
-    return fsmNacked(optPduPacket);
-  }
-  if (state == cfdp::CfdpState::BUSY_CLASS_2_ACKED) {
-    return fsmAcked(optPduPacket);
+    return fsmNacked();
   }
   return fsmResult;
 }
@@ -264,10 +210,7 @@ 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);
@@ -287,7 +230,15 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo
 }
 
 ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() {
-  ReturnValue_t result = 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);
+  ReturnValue_t result =
+      sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu);
   if (result != OK) {
     return result;
   }
@@ -296,18 +247,8 @@ 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;
@@ -326,7 +267,19 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDat
       lenToRead = transactionParams.remoteCfg.maxFileSegmentLen;
     }
   }
-  ReturnValue_t result = sendFileDataPdu(transactionParams.progress, lenToRead);
+  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);
   if (result != OK) {
     return result;
   }
@@ -366,23 +319,20 @@ ReturnValue_t cfdp::SourceHandler::sendGenericPdu(PduType pduType,
     addError(result);
     return result;
   }
-  result = pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen);
-  if (result != OK) {
-    addError(result);
-    return result;
-  }
+  pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen);
   fsmResult.packetsSent += 1;
-  return OK;
+  return result;
 }
 
 ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() {
   if (sourceParams.cfg.indicCfg.transactionFinishedIndicRequired) {
-    // The values reported by the peer's Finished PDU, if one was received. Reporting a hardcoded
-    // NO_ERROR / DATA_COMPLETE here meant a transfer the receiver rejected still looked
-    // successful on this side.
-    cfdp::TransactionFinishedParams params(
-        transactionParams.id, transactionParams.finishedConditionCode,
-        transactionParams.finishedDeliveryCode, transactionParams.finishedDeliveryStatus);
+    // 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);
     sourceParams.user.transactionFinishedIndication(params);
   }
   return OK;
@@ -391,7 +341,6 @@ 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;
@@ -407,328 +356,3 @@ 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 (D8 of the class 2
-    // plan): 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, so reaching
-      // this point means the downlink is gone rather than a single PDU being lost.
-      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) {
-  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,7 +1,6 @@
 #ifndef FSFW_CFDP_CFDPSOURCEHANDLER_H
 #define FSFW_CFDP_CFDPSOURCEHANDLER_H
 
-#include <array>
 #include <cstdint>
 #include <vector>
 
@@ -12,10 +11,8 @@
 #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"
 
@@ -84,52 +81,15 @@ 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, which is what the handler did unconditionally before
-    //! the Finished PDU was actually parsed.
-    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;
@@ -138,29 +98,13 @@ 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(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);
+  FsmResult& fsmNacked();
   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,6 +38,4 @@ 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 (yet).
-static constexpr ReturnValue_t TRANSMISSION_MODE_NOT_SUPPORTED = returnvalue::makeCode(CID, 10);
 }  // namespace cfdp

src/fsfw/cfdp/handler/mib.h

@@ -36,31 +36,6 @@ 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,9 +23,6 @@ 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,15 +11,6 @@ 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,7 +14,6 @@ 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,15 +17,6 @@ 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;
   }
@@ -47,6 +38,10 @@ 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;

src/fsfw/cfdp/pdu/NakPduCreator.h

@@ -25,9 +25,6 @@ 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,14 +11,6 @@ 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;
   }
@@ -42,24 +34,17 @@ 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/controller/ExtendedControllerBase.h

@@ -34,12 +34,11 @@ class ExtendedControllerBase : public ControllerBase,
   ActionHelper actionHelper;
 
   // Periodic HK methods, default method assumes that no shared pool is required.
-  datapool::SharedPool* getOptionalSharedPool() override = 0;
+  virtual datapool::SharedPool* getOptionalSharedPool() override;
 
   // Periodic HK abstract methods.
-  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf,
-                                   size_t maxSize) override = 0;
-  ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) override = 0;
+  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf, size_t maxSize) = 0;
+  ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) = 0;
 
   /**
    * Implemented by child class. Handle all command messages which are

src/fsfw/datapool/LocalPoolObjectBase.cpp

@@ -48,14 +48,6 @@ PoolObjectBase::PoolObjectBase(object_id_t poolOwner, id_t poolId, DataSetIF* da
     return;
   }
   sharedPool = hkOwner->getOptionalSharedPool();
-  if (sharedPool == nullptr) {
-#if FSFW_CPP_OSTREAM_ENABLED == 1
-    sif::error << "PoolObjectBase: HK owner 0x" << std::hex << poolOwner << std::dec
-               << "does not have a shared pool " << std::endl;
-#else
-    sif::printError("PoolObjectBase: HK owner 0x%08x does not have a shared pool\n", poolOwner);
-#endif
-  }
 
   if (dataSet != nullptr) {
     dataSet->registerVariable(this);

src/fsfw/datapool/PoolDataSetBase.cpp

@@ -9,9 +9,9 @@
 
 PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray,
                                  const size_t maxFillCount, bool serializeWithValidityBlob)
-    : serializeWithValidityBlob(serializeWithValidityBlob),
-      registeredVariables(registeredVariablesArray),
-      maxFillCount(maxFillCount) {}
+    : registeredVariables(registeredVariablesArray),
+      maxFillCount(maxFillCount),
+      serializeWithValidityBlob(serializeWithValidityBlob) {}
 
 PoolDataSetBase::~PoolDataSetBase() = default;
 
@@ -251,9 +251,6 @@ size_t PoolDataSetBase::getSerializedSize() const {
   for (uint16_t count = 0; count < fillCount; count++) {
     size += registeredVariables[count]->getSerializedSize();
   }
-  if (serializeWithValidityBlob) {
-    size += std::ceil(static_cast<float>(fillCount) / 8.0);
-  }
   return size;
 }
 

src/fsfw/datapool/PoolDataSetBase.h

@@ -139,8 +139,6 @@ class PoolDataSetBase : public PoolDataSetIF, public SerializeIF {
    */
   void setChildrenValidity(bool valid);
 
-  bool serializeWithValidityBlob = false;
-
  protected:
   /**
    * @brief	The fill_count attribute ensures that the variables
@@ -168,6 +166,7 @@ class PoolDataSetBase : public PoolDataSetIF, public SerializeIF {
    */
   PoolVariableIF** registeredVariables = nullptr;
   const size_t maxFillCount = 0;
+  bool serializeWithValidityBlob = false;
 
   void setContainer(PoolVariableIF** variablesContainer);
   PoolVariableIF** getContainer() const;

src/fsfw/datapool/PoolReadGuard.h

@@ -20,9 +20,9 @@ class PoolReadGuard {
       if (readResult != returnvalue::OK) {
 #if FSFW_VERBOSE_LEVEL == 1
 #if FSFW_CPP_OSTREAM_ENABLED == 1
-        sif::error << "PoolReadGuard: Read failed!" << std::endl;
+        sif::error << "PoolReadHelper: Read failed!" << std::endl;
 #else
-        sif::printError("PoolReadGuard: Read failed!\n");
+        sif::printError("PoolReadHelper: Read failed!\n");
 #endif /* FSFW_PRINT_VERBOSITY_LEVEL == 1 */
 #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */
       }

src/fsfw/datapool/PoolVariable.h

@@ -210,7 +210,6 @@ inline ReturnValue_t PoolVariable<T>::readWithoutLock() {
   }
 
   this->value = *(poolEntry->getDataPtr());
-  this->valid = poolEntry->getValid();
   return returnvalue::OK;
 }
 
@@ -242,7 +241,6 @@ ReturnValue_t PoolVariable<T>::commitWithoutLock() {
   }
 
   *(poolEntry->getDataPtr()) = this->value;
-  poolEntry->setValid(this->valid);
   return returnvalue::OK;
 }
 

src/fsfw/datapool/SharedSet.cpp

@@ -3,7 +3,7 @@
 using namespace dp;
 
 SharedSet::SharedSet(dp::SharedPool& sharedPool, uint32_t setId, const size_t maxNumberOfVariables,
-                     bool serializeWithValidityBlob)
+                     bool serializeWithValídityBlob)
     : SharedSetBase(sharedPool, setId, nullptr, maxNumberOfVariables, serializeWithValidityBlob),
       poolVarList(maxNumberOfVariables) {
   this->setContainer(poolVarList.data());

src/fsfw/datapool/SharedSetBase.cpp

@@ -105,6 +105,9 @@ ReturnValue_t SharedSetBase::deSerialize(const uint8_t **buffer, size_t *size,
 
 ReturnValue_t SharedSetBase::serialize(uint8_t **buffer, size_t *size, size_t maxSize,
                                        SerializeIF::Endianness streamEndianness) const {
+  if (serializeWithValidityBlob) {
+    return base.doSerializeWithValidityBlob(buffer, size, maxSize, streamEndianness);
+  }
   return base.serialize(buffer, size, maxSize, streamEndianness);
 }
 
@@ -140,17 +143,10 @@ void SharedSetBase::setAllVariablesReadOnly() {
 void SharedSetBase::printSet() { return; }
 
 ReturnValue_t SharedSetBase::read(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) {
-  lockDataPool(timeoutType, timeoutMs);
-  ReturnValue_t result = base.read(timeoutType, timeoutMs);
-  unlockDataPool();
-  return result;
+  return base.read(timeoutType, timeoutMs);
 }
-
 ReturnValue_t SharedSetBase::commit(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) {
-  lockDataPool(timeoutType, timeoutMs);
-  ReturnValue_t result = base.commit(timeoutType, timeoutMs);
-  unlockDataPool();
-  return result;
+  return base.commit(timeoutType, timeoutMs);
 }
 uint16_t SharedSetBase::getFillCount() const { return base.getFillCount(); }
 
@@ -162,8 +158,3 @@ void SharedSetBase::setContainer(PoolVariableIF **variablesContainer) {
   return base.setContainer(variablesContainer);
 }
 PoolVariableIF **SharedSetBase::getContainer() const { return base.getContainer(); }
-
-void SharedSetBase::updateValidityBlobSerialization(bool enable) {
-  base.serializeWithValidityBlob = enable;
-}
-bool SharedSetBase::getValidityBlobSerialization() const { return base.serializeWithValidityBlob; }

src/fsfw/datapool/SharedSetBase.h

@@ -159,8 +159,11 @@ class SharedSetBase : public SerializeIF, public PoolDataSetIF {
    */
   void setChildrenValidity(bool valid);
 
-  void updateValidityBlobSerialization(bool enable);
-  bool getValidityBlobSerialization() const;
+  /**
+   * If the valid state of a dataset is always relevant to the whole
+   * data set we can use this flag.
+   */
+  bool serializeWithValidityBlob = false;
 
  protected:
   PoolDataSetBase base;

src/fsfw/devicehandlers/AssemblyBase.cpp

@@ -71,8 +71,7 @@ bool AssemblyBase::handleChildrenChangedHealth() {
   if (iter == childrenMap.end()) {
     return false;
   }
-  HealthState healthState =
-      healthHelper.healthTable->getHealth(convertToDeviceObjectId(iter->first));
+  HealthState healthState = healthHelper.healthTable->getHealth(iter->first);
   if (healthState == HasHealthIF::NEEDS_RECOVERY) {
     triggerEvent(TRYING_RECOVERY, iter->first, 0);
     recoveryState = RECOVERY_STARTED;
@@ -92,14 +91,10 @@ 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;
-        ReturnValue_t result = commandChildren(targetMode, targetSubmode);
-        if (result == NEED_SECOND_STEP) {
-          internalState = STATE_NEED_SECOND_STEP;
-        }
+        commandChildren(targetMode, targetSubmode);
         return;
-      }
       case STATE_OVERWRITE_HEALTH: {
         internalState = STATE_SINGLE_STEP;
         ReturnValue_t result = commandChildren(mode, submode);
@@ -175,7 +170,7 @@ ReturnValue_t AssemblyBase::checkChildrenStateOff() {
 
 ReturnValue_t AssemblyBase::checkChildOff(uint32_t objectId) {
   ChildInfo childInfo = childrenMap.find(objectId)->second;
-  if (healthHelper.healthTable->isCommandable(convertToDeviceObjectId(objectId))) {
+  if (healthHelper.healthTable->isCommandable(objectId)) {
     if (childInfo.submode != SUBMODE_NONE) {
       return returnvalue::FAILED;
     } else {
@@ -232,7 +227,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.
-      // The timeout can be defined by #setRecoveryWaitTimer
+      // TODO: make time period configurable
       recoveryState = RECOVERY_WAIT;
       recoveryOffTimer.resetTimer();
       return true;
@@ -240,14 +235,14 @@ bool AssemblyBase::checkAndHandleRecovery() {
       if (recoveryOffTimer.isBusy()) {
         return true;
       }
-      triggerEvent(RECOVERY_WAITING, recoveringDevice->first);
+      triggerEvent(RECOVERY_STEP, 0);
       sendHealthCommand(recoveringDevice->second.commandQueue, HEALTHY);
       internalState = STATE_NONE;
       recoveryState = RECOVERY_ONGOING;
       // Don't check state!
       return true;
     case RECOVERY_ONGOING:
-      triggerEvent(RECOVERY_RESTARTING, recoveringDevice->first);
+      triggerEvent(RECOVERY_STEP, 1);
       recoveryState = RECOVERY_ONGOING_2;
       recoveringDevice->second.healthChanged = false;
       // Device should be healthy again, so restart a transition.
@@ -255,7 +250,7 @@ bool AssemblyBase::checkAndHandleRecovery() {
       doStartTransition(targetMode, targetSubmode);
       return true;
     case RECOVERY_ONGOING_2:
-      triggerEvent(RECOVERY_DONE, recoveringDevice->first);
+      triggerEvent(RECOVERY_DONE);
       // Now we're through, but not sure if it was successful.
       recoveryState = RECOVERY_IDLE;
       return false;
@@ -269,14 +264,7 @@ void AssemblyBase::overwriteDeviceHealth(object_id_t objectId, HasHealthIF::Heal
   triggerEvent(OVERWRITING_HEALTH, objectId, oldHealth);
   internalState = STATE_OVERWRITE_HEALTH;
   modeHelper.setForced(true);
-  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
-  }
+  sendHealthCommand(childrenMap[objectId].commandQueue, EXTERNAL_CONTROL);
 }
 
 void AssemblyBase::triggerModeHelperEvents(Mode_t mode, Submode_t submode) {
@@ -286,7 +274,3 @@ 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,8 +206,6 @@ 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<uint32_t> bufferWrapper(rawData, rawDataLen);
+  SerialBufferAdapter bufferWrapper(rawData, rawDataLen);
   handleDeviceTm(bufferWrapper, replyId, forceDirectTm);
 }
 

src/fsfw/devicehandlers/FreshDeviceHandlerBase.h

@@ -148,24 +148,20 @@ class FreshDeviceHandlerBase : public SystemObject,
   // System Object overrides.
   ReturnValue_t initialize() override;
 
+  // Default implementation assumes that no optional shared pool is required.
+  datapool::SharedPool* getOptionalSharedPool() override;
+
   /**
    * This function is implemented to serialize a housekeeping packet when a HK message to
-   * generate the packet is received, or periodic generation is necessary. The user should serialize
-   * the HK set into the provided buffer, which will have the size specified in the set
-   * specification.
-   */
-  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf,
-                                   size_t maxSize) override = 0;
-  /**
-   * This function is implemented by the user to specify the available housekeeping sets.
+   * generate the packet is received, or periodic generation is necessary. The user should serialize the HK
+   * set into the provided buffer, which will have the size specified in the set specification.
    */
+  ReturnValue_t serializeHkDataset(dp::sid_t structureId, uint8_t* buf, size_t maxSize) override = 0;
+    /**
+     * This function is implemented by the user to specify the available housekeeping sets.
+     */
   ReturnValue_t specifyHkDatasets(std::vector<hk::SetSpecification>& setList) override = 0;
 
-  /*
-   *  If this device handler has an optional pool, return it. Otherwise, nullptr can be returned.
-   */
-  datapool::SharedPool* getOptionalSharedPool() override = 0;
-
   /**
    * Implemented by child class. Handle all command messages which are
    * not health, mode, action or housekeeping messages.
@@ -206,6 +202,7 @@ class FreshDeviceHandlerBase : public SystemObject,
    * @return
    */
   virtual ReturnValue_t performOperation(uint8_t opCode) override;
+  ReturnValue_t initializeAfterTaskCreation() override;
 
   /**
    * This calls the FDIR instance event trigger function.

src/fsfw/events/Event.h

@@ -14,6 +14,8 @@ 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 {
@@ -22,14 +24,11 @@ constexpr EventId_t getEventId(Event event) { return (event & 0xFFFF); }
 
 constexpr EventSeverity_t getSeverity(Event event) { return ((event >> 16) & 0xFF); }
 
-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!");
+constexpr Event makeEvent(uint8_t subsystemId, UniqueEventId_t uniqueEventId,
+                          EventSeverity_t eventSeverity) {
   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,7 +2,6 @@ target_sources(
   ${LIB_FSFW_NAME}
   PRIVATE arrayprinter.cpp
           AsciiConverter.cpp
-          CobsEncoder.cpp
           CRC.cpp
           DleEncoder.cpp
           DleParser.cpp

src/fsfw/globalfunctions/CobsEncoder.cpp

@@ -1,78 +0,0 @@
-#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

@@ -1,113 +0,0 @@
-#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,33 +56,26 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, size_t maxCharPerL
   std::cout << std::dec << std::setfill(' ');
   std::cout << "]" << std::endl;
 #else
-#if FSFW_DISABLE_PRINTOUT == 0
-  // Emitted in fixed size chunks. This used to size one buffer from the input - a variable length
-  // array of (size + 1) * 7 + 1 bytes on the stack - which is unusable for the sizes this is
-  // actually called with: dumping a 12 KB receive buffer asks for 84 KB of stack, and overflowed
-  // an 8 KB task on the iOBC as soon as a frame parse error made it dump one. The output is
-  // unchanged, it is just flushed as it is built.
-  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] = {};
+  // 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] = {};
   size_t currentPos = 0;
-  printf("hex [");
   for (size_t i = 0; i < size; i++) {
-    if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
-      printf("%s", printBuffer);
-      printBuffer[0] = '\0';
-      currentPos = 0;
+    // To avoid buffer overflows.
+    if (sizeof(printBuffer) - currentPos <= 7) {
+      break;
     }
-    currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%02x", data[i]);
+
+    currentPos += snprintf(printBuffer + currentPos, 6, "%02x", data[i]);
     if (i < size - 1) {
-      currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
+      currentPos += sprintf(printBuffer + currentPos, ",");
       if ((i + 1) % maxCharPerLine == 0) {
-        currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
+        currentPos += sprintf(printBuffer + currentPos, "\n");
       }
     }
   }
-  printf("%s]\n", printBuffer);
+#if FSFW_DISABLE_PRINTOUT == 0
+  printf("hex [%s]\n", printBuffer);
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 #endif
 }
@@ -105,30 +98,27 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, size_t maxCharPerL
   }
   std::cout << "]" << std::endl;
 #else
-#if FSFW_DISABLE_PRINTOUT == 0
-  // Chunked for the same reason as printHex above: the buffer used to be a variable length array
-  // sized from the input.
-  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] = {};
+  // 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] = {};
   size_t currentPos = 0;
-  printf("dec [");
   for (size_t i = 0; i < size; i++) {
-    if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) {
-      printf("%s", printBuffer);
-      printBuffer[0] = '\0';
-      currentPos = 0;
+    // To avoid buffer overflows.
+    if (sizeof(printBuffer) - currentPos <= 4) {
+      break;
     }
-    currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%d", data[i]);
+
+    currentPos += snprintf(printBuffer + currentPos, 4, "%d", data[i]);
     if (i < size - 1) {
-      currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ",");
+      currentPos += sprintf(printBuffer + currentPos, ",");
       if ((i + 1) % maxCharPerLine == 0) {
-        currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n");
+        currentPos += sprintf(printBuffer + currentPos, "\n");
       }
     }
   }
-  printf("%s]\n", printBuffer);
+#if FSFW_DISABLE_PRINTOUT == 0
+  printf("dec [%s]\n", printBuffer);
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 #endif
 }

src/fsfw/health/HasHealthIF.h

@@ -27,19 +27,13 @@ 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).
-  //! P1: Object Id of the recovering device.
+  //! Someone starts a recovery of a component (typically power-cycle). No parameters.
   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 initialState) {
+                                          HasHealthIF::HealthState initilialState) {
   if (healthMap.count(object) != 0) {
     return returnvalue::FAILED;
   }
-  healthMap.emplace(object, initialState);
+  healthMap.emplace(object, initilialState);
   return returnvalue::OK;
 }
 
@@ -112,5 +112,3 @@ 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 initialState = HasHealthIF::HEALTHY) override;
+      object_id_t object, HasHealthIF::HealthState initilialState = 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,8 +28,6 @@ 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 initialState = HasHealthIF::HEALTHY) = 0;
+      object_id_t object, HasHealthIF::HealthState initilialState = HasHealthIF::HEALTHY) = 0;
 
   virtual ReturnValue_t removeObject(object_id_t objectId) = 0;
 

src/fsfw/housekeeping.h

@@ -1,4 +1,9 @@
-#pragma once
+#ifndef FSFW_INC_FSFW_HOUSEKEEPING_H_
+#define FSFW_INC_FSFW_HOUSEKEEPING_H_
 
-#include "fsfw/housekeeping/Dataset.h"
-#include "fsfw/housekeeping/DatasetElement.h"
+#include "src/core/housekeeping/HousekeepingMessage.h"
+#include "src/core/housekeeping/HousekeepingPacketDownlink.h"
+#include "src/core/housekeeping/HousekeepingSetPacket.h"
+#include "src/core/housekeeping/HousekeepingSnapshot.h"
+
+#endif /* FSFW_INC_FSFW_HOUSEKEEPING_H_ */

src/fsfw/housekeeping/Dataset.h

@@ -17,6 +17,12 @@ class Dataset : public SerializeIF {
 
   [[nodiscard]] dp::structure_id_t getStructureId() const { return sid; }
 
+  void setAllChildrenValidity(bool valid) {
+    for (auto &serializable : serializables) {
+      serializable.get().setValid(valid);
+    }
+  }
+
   void addSerializable(const std::reference_wrapper<SerializableWithValidityIF> serializable) {
     serializables.push_back(serializable);
   }
@@ -89,20 +95,11 @@ class Dataset : public SerializeIF {
     return SerializeIF::serialize(buffer, serSize, maxSize, streamEndianness);
   }
 
-  void setChildrenValidity(bool valid) {
-    for (auto &serializable : serializables) {
-      serializable.get().setValid(valid);
-    }
-  }
-
   [[nodiscard]] size_t getSerializedSize() const override {
     size_t size = 0;
     for (auto &serializable : serializables) {
       size += serializable.get().getSerializedSize();
     }
-    if (serializeWithValidityBlob) {
-      size += std::ceil(static_cast<float>(serializables.size()) / 8.0);
-    }
     return size;
   }
 

src/fsfw/housekeeping/PeriodicHkHelper.cpp

@@ -9,6 +9,7 @@
 #include "fsfw/housekeeping/HousekeepingSnapshot.h"
 #include "fsfw/ipc/QueueFactory.h"
 #include "fsfw/objectmanager/ObjectManager.h"
+#include "fsfw/timemanager/CCSDSTime.h"
 
 using namespace hk;
 
@@ -16,7 +17,7 @@ PeriodicHelper::PeriodicHelper(GeneratesPeriodicHkIF* owner, MessageQueueIF* que
                                MessageQueueId_t hkDestQueue)
     : hkDestinationId(hkDestQueue) {
   if (owner == nullptr) {
-    printWarningOrError(sif::OutputTypes::OUT_WARNING, "PeriodicHkHelper", returnvalue::FAILED,
+    printWarningOrError(sif::OutputTypes::OUT_WARNING, "LocalDataPoolManager", returnvalue::FAILED,
                         "Invalid supplied owner");
     return;
   }
@@ -83,7 +84,6 @@ 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), sender);
+      return generateHousekeepingPacket(HousekeepingMessage::getStructureId(message));
     }
 
     default:
@@ -200,10 +200,11 @@ void PeriodicHelper::performPeriodicHkGeneration(SetSpecification& setSpec, time
     if (result != returnvalue::OK) {
       // Configuration error
 #if FSFW_CPP_OSTREAM_ENABLED == 1
-      sif::warning << "hk::PeriodicHelper::performPeriodicHkOperation: HK generation failed."
+      sif::warning << "LocalDataPoolManager::performPeriodicHkOperation: HK generation failed."
                    << std::endl;
 #else
-      sif::printWarning("hk::PeriodicHelper::performPeriodicHkOperation: HK generation failed.\n");
+      sif::printWarning(
+          "LocalDataPoolManager::performPeriodicHkOperation: HK generation failed.\n");
 #endif
       return;
     }

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(), ignoreFault);
+  return sendMessageFrom(sendTo, message, this->getId(), false);
 }
 
 ReturnValue_t MessageQueueBase::sendToDefaultFrom(MessageQueueMessageIF* message,

src/fsfw/ipc/MutexGuard.h

@@ -47,10 +47,7 @@ class MutexGuard {
   ReturnValue_t getLockResult() const { return result; }
 
   ~MutexGuard() {
-    // 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) {
+    if (internalMutex != nullptr) {
       internalMutex->unlockMutex();
     }
   }

src/fsfw/objectmanager/frameworkObjects.h

@@ -36,8 +36,6 @@ 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

@@ -6,7 +6,6 @@
 #include "FreeRTOS.h"
 #include "fsfw/globalfunctions/timevalOperations.h"
 #include "fsfw/osal/freertos/Timekeeper.h"
-#include "fsfw/serviceinterface/ServiceInterfacePrinter.h"
 #include "task.h"
 
 // TODO sanitize input?
@@ -48,9 +47,8 @@ ReturnValue_t Clock::getClock(timeval* time) {
 }
 
 ReturnValue_t Clock::getClockMonotonic(timeval* time) {
-  // Retrieves the FreeRTOS tick count, which is hopefully monotonic
-  *time = getUptime();
-  return returnvalue::OK;
+  // TODO: I don't actually know if the timekeeper is monotonic..
+  return getClock_timeval(time);
 }
 
 ReturnValue_t Clock::getUptime(timeval* uptime) {
@@ -60,7 +58,7 @@ ReturnValue_t Clock::getUptime(timeval* uptime) {
 }
 
 timeval Clock::getUptime() {
-  TickType_t ticksSinceStart = Timekeeper::instance()->getTicks();
+  TickType_t ticksSinceStart = xTaskGetTickCount();
   return Timekeeper::ticksToTimeval(ticksSinceStart);
 }
 

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.c_str());
+  sif::printWarning("TaskFactory::printMissedDeadline: %s\n", name);
 #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 = event::makeEvent<SUBSYSTEM_ID, 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,8 +1,6 @@
 #ifndef FSFW_PUS_SERVICE8FUNCTIONMANAGEMENT_H_
 #define FSFW_PUS_SERVICE8FUNCTIONMANAGEMENT_H_
 
-#include <cstdint>
-
 #include "fsfw/action/ActionMessage.h"
 #include "fsfw/tmtcservices/CommandingServiceBase.h"
 
@@ -37,13 +35,6 @@ 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;
@@ -57,6 +48,13 @@ 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,14 +1,11 @@
 #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"
 
 /**
@@ -25,41 +22,14 @@ 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,7 +85,6 @@ 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 ServiceInterfacePrinterTask.cpp)
+          ServiceInterfacePrinter.cpp)

src/fsfw/serviceinterface/ServiceInterfacePrinter.cpp

@@ -1,12 +1,9 @@
 #include "fsfw/serviceinterface/ServiceInterfacePrinter.h"
 
-#include <algorithm>
 #include <cstdarg>
-#include <cstring>
+#include <cstdint>
 
 #include "fsfw/FSFW.h"
-#include "fsfw/ipc/MutexFactory.h"
-#include "fsfw/ipc/MutexGuard.h"
 #include "fsfw/serviceinterface/serviceInterfaceDefintions.h"
 #include "fsfw/timemanager/Clock.h"
 
@@ -17,32 +14,11 @@ static bool consoleInitialized = false;
 
 #if FSFW_DISABLE_PRINTOUT == 0
 
-namespace {
+static bool addCrAtEnd = false;
 
-// 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;
+uint8_t printBuffer[fsfwconfig::FSFW_PRINT_BUFFER_SIZE];
 
-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) {
+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);
@@ -54,156 +30,100 @@ 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;
   }
 
-  static const char* const labels[] = {"", "ERROR  ", "WARNING", "INFO   ", "DEBUG  "};
+  /* Log message to terminal */
+
 #if FSFW_COLORED_OUTPUT == 1
-  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 = "";
+  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);
 #endif
 
   Clock::TimeOfDay_t now;
   Clock::getDateAndTime(&now);
+  /*
+   * Log current time to terminal if desired.
+   */
+  len += sprintf(bufferPosition + len, " | %lu:%02lu:%02lu.%03lu | ", (unsigned long)now.hour,
+                 (unsigned long)now.minute, (unsigned long)now.second,
+                 (unsigned long)now.usecond / 1000);
 
-  char buf[fsfwconfig::FSFW_PRINT_BUFFER_SIZE + 2];  // slack for the CR/newline
+  len += vsnprintf(bufferPosition + len, sizeof(printBuffer) - len, fmt, arg);
 
-  // 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';
+  if (addCrAtEnd) {
+    len += sprintf(bufferPosition + len, "\r");
   }
 
-  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;
+  printf("%s", printBuffer);
 }
 
-}  // namespace
-
-void sif::setToAddCrAtEnd(const bool addCrAtEnd_) { addCrAtEnd = addCrAtEnd_; }
-
-void sif::setReplaceLastCharWithNewline(const bool replace) {
-  replaceLastCharWithNewline = replace;
-}
-
-void sif::printInfo(const char* fmt, ...) {
+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::printError(const char* fmt, ...) {
+void sif::setToAddCrAtEnd(bool addCrAtEnd_) { addCrAtEnd = addCrAtEnd_; }
+
+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::printCallback() {}
-void sif::init() {}
-uint32_t sif::getDroppedMessagesCount() { return 0; }
+void sif::printInfo(const char *fmt, ...) {}
+void sif::printWarning(const char *fmt, ...) {}
+void sif::printDebug(const char *fmt, ...) {}
+void sif::printError(const char *fmt, ...) {}
 
 #endif /* FSFW_DISABLE_PRINTOUT == 0 */
 

src/fsfw/serviceinterface/ServiceInterfacePrinter.h

@@ -39,11 +39,6 @@ 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.
@@ -56,21 +51,6 @@ 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

@@ -1,14 +0,0 @@
-#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

@@ -1,10 +0,0 @@
-#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 = 100;
+  static const uint8_t MAX_NUMBER_OF_TABLES_OR_SEQUENCES = 70;
 
   static const uint8_t MAX_LENGTH_OF_TABLE_OR_SEQUENCE = 20;
 

src/fsfw/subsystem/SubsystemBase.cpp

@@ -78,36 +78,23 @@ void SubsystemBase::executeTable(HybridIterator<ModeListEntry> tableIter, Submod
       submodeToCommand = targetSubmode;
     }
 
-    if (healthHelper.healthTable->hasHealth(convertToDeviceObjectId(object))) {
-      switch (healthHelper.healthTable->getHealth(convertToDeviceObjectId(object))) {
-        case NEEDS_RECOVERY:
-        case FAULTY:
-        case PERMANENT_FAULTY:
-          ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND, HasModesIF::MODE_OFF,
-                                      SUBMODE_NONE);
-          break;
-        case HEALTHY:
-          if (modeHelper.isForced()) {
-            ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND_FORCED,
-                                        tableIter.value->getMode(), submodeToCommand);
-          } else {
+    if (healthHelper.healthTable->hasHealth(object)) {
+      if (healthHelper.healthTable->isFaulty(object)) {
+        ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND, HasModesIF::MODE_OFF,
+                                    SUBMODE_NONE);
+      } else {
+        if (modeHelper.isForced()) {
+          ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND_FORCED,
+                                      tableIter.value->getMode(), submodeToCommand);
+        } else {
+          if (healthHelper.healthTable->isCommandable(object)) {
             ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND,
                                         tableIter.value->getMode(), submodeToCommand);
-          }
-          break;
-        case EXTERNAL_CONTROL:
-          if (modeHelper.isForced()) {
-            ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND_FORCED,
-                                        tableIter.value->getMode(), submodeToCommand);
           } else {
             continue;
           }
-          break;
-        default:
-          // This never happens
-          break;
+        }
       }
-
     } else {
       ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND,
                                   tableIter.value->getMode(), submodeToCommand);
@@ -352,5 +339,3 @@ 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
    */
-  virtual void executeTable(HybridIterator<ModeListEntry> tableIter,
-                            Submode_t targetSubmode = SUBMODE_NONE);
+  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,12 +153,6 @@ 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,10 +18,6 @@ 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();
@@ -135,7 +131,8 @@ ReturnValue_t PusDistributor::initialize() {
     return ObjectManagerIF::CHILD_INIT_FAILED;
   }
   if (verifyChannel == nullptr) {
-    verifyChannel = ObjectManager::instance()->get<VerificationReporterIF>(verificationReporter);
+    verifyChannel =
+        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
     if (verifyChannel == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }

src/fsfw/tcdistribution/PusDistributor.h

@@ -43,10 +43,6 @@ 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 {
@@ -79,8 +75,6 @@ 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 = event::makeEvent<SUBSYSTEM_ID, 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/thermal/TemperatureSensor.h

@@ -142,7 +142,7 @@ class TemperatureSensor : public AbstractTemperatureSensor {
 
       deltaTime = (uptime.tv_sec + uptime.tv_usec / 1000000.) -
                   (uptimeOfOldTemperature.tv_sec + uptimeOfOldTemperature.tv_usec / 1000000.);
-      deltaTemp = oldTemperature - outputTemperature.value;
+      deltaTemp = oldTemperature - outputTemperature;
       if (deltaTemp < 0) {
         deltaTemp = -deltaTemp;
       }
@@ -160,13 +160,13 @@ class TemperatureSensor : public AbstractTemperatureSensor {
       outputTemperature.setValid(PoolVariableIF::INVALID);
       outputTemperature = thermal::INVALID_TEMPERATURE;
     } else {
-      oldTemperature = outputTemperature.value;
+      oldTemperature = outputTemperature;
       uptimeOfOldTemperature = uptime;
     }
   }
 
  public:
-  float getTemperature() { return outputTemperature.value; }
+  float getTemperature() { return outputTemperature; }
 
   bool isValid() { return outputTemperature.isValid(); }
 

src/fsfw/tmstorage/TmStoreMessage.cpp

@@ -114,11 +114,6 @@ 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,7 +21,6 @@ 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);
@@ -55,7 +54,6 @@ 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>(verificationReporterId);
+        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
     if (verificationReporter == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }
@@ -136,10 +136,6 @@ 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,8 +126,6 @@ class CommandingServiceBase : public SystemObject,
 
   ReturnValue_t initialize() override;
 
-  void setVerificationReporter(object_id_t verificationReporterId_);
-
   /**
    * Implementation of ExecutableObjectIF function
    *
@@ -264,8 +262,6 @@ 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>(pusDistributor);
+    psbParams.pusDistributor = ObjectManager::instance()->get<PusDistributorIF>(PUS_DISTRIBUTOR);
     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>(verificationReporter);
+        ObjectManager::instance()->get<VerificationReporterIF>(objects::VERIFICATION_REPORTER);
     if (psbParams.verifReporter == nullptr) {
       return ObjectManagerIF::CHILD_INIT_FAILED;
     }
@@ -134,14 +134,6 @@ 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,9 +201,6 @@ 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
@@ -231,9 +228,6 @@ 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} PRIVATE HostFilesystem.cpp)
+target_sources(${LIB_FSFW_NAME} PUBLIC 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,12 +15,6 @@ 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;
@@ -38,18 +32,5 @@ 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,20 +1,15 @@
 #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"
@@ -51,13 +46,12 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") {
   PduConfig conf;
   auto destHandler = DestHandler(senderMock, 4096, dp, fp);
 
-  auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType,
-                                 bool closureRequested = false) {
+  auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType) {
     const std::string srcNameString = "hello.txt";
     const std::string destNameString = "hello-cpy.txt";
     StringLv srcName(srcNameString);
     StringLv destName(destNameString);
-    MetadataGenericInfo info(closureRequested, checksumType, std::move(cfdpFileSize));
+    MetadataGenericInfo info(false, checksumType, std::move(cfdpFileSize));
     const TransactionSeqNum seqNum(UnsignedByteField<uint16_t>(1));
     conf.sourceId = remoteId;
     conf.destId = localId;
@@ -132,36 +126,6 @@ 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. The delivery fields used to be left at their reset defaults on this path, reporting
-    // a successful transaction as "Data Incomplete" and "Discard deliberately" next to a NO_ERROR
-    // condition code, in the Finished PDU as well as in the log.
-    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);
@@ -255,412 +219,4 @@ 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, so a failed send used to be indistinguishable
-    // from a successful one: the transaction was finished either way and the sender never heard
-    // that a transfer which actually succeeded had 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);
-    // D2: 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("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, so this used to
-    // fail with "Ca not deserialize fault location" and the whole PDU was dropped: the sender
-    // never got its ACK and retransmitted to its positive ACK limit, while this handler held the
-    // transaction open until its own check limit expired.
-    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("EOF PDU for an inactive transaction is acknowledged") {
-    // D7: 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,25 +1,18 @@
 #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"
@@ -203,39 +196,6 @@ 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
@@ -311,289 +271,4 @@ 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("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

@@ -1,38 +0,0 @@
-#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,7 +1,6 @@
 #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"
@@ -188,53 +187,3 @@ 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 used to run
-    // straight into it and reject the PDU as an invalid TLV type.
-    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,7 +2,6 @@
 #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"
@@ -223,58 +222,3 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") {
     }
   }
 }
-
-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. Parsing it used to fail with INVALID_TLV_TYPE, because the option loop
-    // ran into the CRC and deserialized it 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, which the reader handled correctly even
-    // before the option loop was taught about it.
-    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,7 +1,6 @@
 #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"
@@ -151,54 +150,3 @@ 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

@@ -62,7 +62,6 @@ TEST_CASE("Pool Dataset Test", "[datapool]") {
     size_t serLen = 0;
     uint8_t* dataPtr = buf;
     dataset.serializeWithValidityBlob = true;
-    CHECK(dataset.getSerializedSize() == 6);
     CHECK(dataset.serialize(&dataPtr, &serLen, sizeof(buf), SerializeIF::Endianness::NETWORK) ==
           returnvalue::OK);
     CHECK(buf[5] == 0b11000000);
@@ -71,7 +70,7 @@ TEST_CASE("Pool Dataset Test", "[datapool]") {
   SECTION("Larger Pool Dataset Serialization With Validity") {
     uint8_t buf[64]{};
     TestDatasetLarger datasetLarge;
-    datasetLarge.setChildrenValidity(true);
+    datasetLarge.setAllChildrenValidity(true);
     size_t serLen = 0;
     uint8_t* dataPtr = buf;
     datasetLarge.serializeWithValidityBlob = true;

unittests/datapool/testSharedSet.cpp

@@ -23,7 +23,6 @@ TEST_CASE("DataSetTest", "[datapool]") {
   SECTION("BasicTest") {
     /* Test some basic functions */
     CHECK(localSet.getReportingEnabled() == false);
-    CHECK(localSet.getSerializedSize() == 11);
     CHECK(localSet.getLocalPoolIdsSerializedSize() == 3 * sizeof(dp::id_t));
     CHECK(localSet.getStructureId() == lpool::testSid1);
     CHECK(localSet.getCreatorObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE);
@@ -157,7 +156,7 @@ TEST_CASE("DataSetTest", "[datapool]") {
   }
 
   SECTION("Serialize with Validity Blob") {
-    localSet.updateValidityBlobSerialization(true);
+    localSet.serializeWithValidityBlob = true;
     CHECK(!localSet.localPoolVarUint8.isValid());
     CHECK(!localSet.localPoolUint16Vec.isValid());
     CHECK(!localSet.localPoolVarFloat.isValid());
@@ -169,7 +168,6 @@ TEST_CASE("DataSetTest", "[datapool]") {
     // Already reserve additional space for validity buffer, will be needed later
     uint8_t buffer[128]{};
     uint8_t* buffPtr = buffer;
-    CHECK(localSet.getSerializedSize() == 12);
     CHECK(localSet.serialize(&buffPtr, &serSize, sizeof(buffer),
                              SerializeIF::Endianness::MACHINE) == returnvalue::OK);
     CHECK(serSize == 12);

unittests/globalfunctions/CMakeLists.txt

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

unittests/globalfunctions/testCobsEncoder.cpp

@@ -1,336 +0,0 @@
-#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());