From 67439b428587c573dfe45b35ad1c7c9d8e04ed2b Mon Sep 17 00:00:00 2001
From: Robin Mueller <muellerr@irs.uni-stuttgart.de>
Date: Tue, 6 Sep 2022 16:03:26 +0200
Subject: [PATCH] refactor away some duplication

---
 src/fsfw/cfdp/handler/DestHandler.cpp      | 32 ++++++++--------------
 src/fsfw/cfdp/handler/DestHandler.h        |  1 +
 unittests/cfdp/handler/testDestHandler.cpp |  5 ++++
 3 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp
index ac773cfc..4bc7134c 100644
--- a/src/fsfw/cfdp/handler/DestHandler.cpp
+++ b/src/fsfw/cfdp/handler/DestHandler.cpp
@@ -32,10 +32,7 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::performStateMachine() {
       if (infoIter->pduType == PduType::FILE_DIRECTIVE and
           infoIter->directiveType == FileDirectives::METADATA) {
         result = handleMetadataPdu(*infoIter);
-        if (result != OK and errorIdx < 3) {
-          fsmRes.errorCodes[errorIdx] = result;
-          errorIdx++;
-        }
+        checkAndHandleError(result, errorIdx);
         // Store data was deleted in PDU handler because a store guard is used
         dp.packetListRef.erase(infoIter++);
       }
@@ -61,10 +58,7 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::performStateMachine() {
       for (auto infoIter = dp.packetListRef.begin(); infoIter != dp.packetListRef.end();) {
         if (infoIter->pduType == PduType::FILE_DATA) {
           result = handleFileDataPdu(*infoIter);
-          if (result != OK and errorIdx < 3) {
-            fsmRes.errorCodes[errorIdx] = result;
-            errorIdx++;
-          }
+          checkAndHandleError(result, errorIdx);
           // Store data was deleted in PDU handler because a store guard is used
           dp.packetListRef.erase(infoIter++);
         }
@@ -72,10 +66,7 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::performStateMachine() {
         if (infoIter->pduType == PduType::FILE_DIRECTIVE and
             infoIter->directiveType == FileDirectives::EOF_DIRECTIVE) {
           result = handleEofPdu(*infoIter);
-          if (result != OK and errorIdx < 3) {
-            fsmRes.errorCodes[errorIdx] = result;
-            errorIdx++;
-          }
+          checkAndHandleError(result, errorIdx);
           // Store data was deleted in PDU handler because a store guard is used
           dp.packetListRef.erase(infoIter++);
         }
@@ -84,17 +75,11 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::performStateMachine() {
     }
     if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) {
       result = handleTransferCompletion();
-      if (result != OK and errorIdx < 3) {
-        fsmRes.errorCodes[errorIdx] = result;
-        errorIdx++;
-      }
+      checkAndHandleError(result, errorIdx);
     }
     if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) {
       result = sendFinishedPdu();
-      if (result != OK and errorIdx < 3) {
-        fsmRes.errorCodes[errorIdx] = result;
-        errorIdx++;
-      }
+      checkAndHandleError(result, errorIdx);
       finish();
     }
     return updateFsmRes(errorIdx);
@@ -457,3 +442,10 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::updateFsmRes(uint8_t erro
 }
 
 const cfdp::TransactionId& cfdp::DestHandler::getTransactionId() const { return tp.transactionId; }
+
+void cfdp::DestHandler::checkAndHandleError(ReturnValue_t result, uint8_t& errorIdx) {
+  if (result != OK and errorIdx < 3) {
+    fsmRes.errorCodes[errorIdx] = result;
+    errorIdx++;
+  }
+}
diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h
index 71214fcb..8d2fc6e9 100644
--- a/src/fsfw/cfdp/handler/DestHandler.h
+++ b/src/fsfw/cfdp/handler/DestHandler.h
@@ -180,6 +180,7 @@ class DestHandler {
   ReturnValue_t noticeOfCompletion();
   ReturnValue_t checksumVerification();
   const FsmResult& updateFsmRes(uint8_t errors);
+  void checkAndHandleError(ReturnValue_t result, uint8_t& errorIdx);
   void finish();
 };
 
diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp
index 23d88352..85cbf23e 100644
--- a/unittests/cfdp/handler/testDestHandler.cpp
+++ b/unittests/cfdp/handler/testDestHandler.cpp
@@ -168,6 +168,11 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") {
     PacketInfo packetInfo(fdPduCreator.getPduType(), storeId, std::nullopt);
     packetInfoList.push_back(packetInfo);
     destHandler.performStateMachine();
+    REQUIRE(res.result == OK);
+    REQUIRE(res.state == CfdpStates::BUSY_CLASS_1_NACKED);
+    REQUIRE(res.step == DestHandler::TransactionStep::RECEIVING_FILE_DATA_PDUS);
+    REQUIRE(not tcStore.hasDataAtId(storeId));
+    REQUIRE(packetInfoList.empty());
     eofPreparation(cfdpFileSize, crc32);
     // After EOF, operation is done because no closure was requested
     destHandler.performStateMachine();