continue dest handler impl

docs/conf.py

@@ -17,12 +17,12 @@
 
 # -- Project information -----------------------------------------------------
 
-project = 'Flight Software Framework'
+project = "Flight Software Framework"
-copyright = '2021, Institute of Space Systems (IRS)'
+copyright = "2021, Institute of Space Systems (IRS)"
-author = 'Institute of Space Systems (IRS)'
+author = "Institute of Space Systems (IRS)"
 
 # The full version, including alpha/beta/rc tags
-release = '2.0.1'
+release = "2.0.1"
 
 
 # -- General configuration ---------------------------------------------------
@@ -30,17 +30,17 @@ release = '2.0.1'
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = [ "breathe" ]
+extensions = ["breathe"]
 
 breathe_default_project = "fsfw"
 
 # Add any paths that contain templates here, relative to this directory.
-templates_path = ['_templates']
+templates_path = ["_templates"]
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
 # This pattern also affects html_static_path and html_extra_path.
-exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
+exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"]
 
 
 # -- Options for HTML output -------------------------------------------------
@@ -48,9 +48,9 @@ exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
 #
-html_theme = 'alabaster'
+html_theme = "alabaster"
 
 # Add any paths that contain custom static files (such as style sheets) here,
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
-html_static_path = []
+html_static_path = []

scripts/helper.py

@@ -51,7 +51,7 @@ def main():
     parser.add_argument(
         "-g",
         "--generators",
-        default = "Ninja",
+        default="Ninja",
         action="store",
         help="CMake generators",
     )
@@ -165,10 +165,18 @@ def create_tests_build_cfg(args):
     os.mkdir(UNITTEST_FOLDER_NAME)
     os.chdir(UNITTEST_FOLDER_NAME)
     if args.windows:
-        cmake_cmd = 'cmake -G "' + args.generators + '" -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON \
+        cmake_cmd = (
+            'cmake -G "'
+            + args.generators
+            + '" -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON \
         -DGCOVR_PATH="py -m gcovr" ..'
+        )
     else:
-        cmake_cmd = 'cmake -G "' + args.generators + '" -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON ..'
+        cmake_cmd = (
+            'cmake -G "'
+            + args.generators
+            + '" -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON ..'
+        )
     cmd_runner(cmake_cmd)
     os.chdir("..")
 

src/fsfw/cfdp/FileSize.h

@@ -8,10 +8,12 @@ namespace cfdp {
 
 struct FileSize : public SerializeIF {
  public:
-  FileSize() : largeFile(false){};
+  FileSize() = default;
 
   explicit FileSize(uint64_t fileSize, bool isLarge = false) { setFileSize(fileSize, isLarge); };
 
+  [[nodiscard]] uint64_t value() const { return fileSize; }
+
   ReturnValue_t serialize(bool isLarge, uint8_t **buffer, size_t *size, size_t maxSize,
                           Endianness streamEndianness) {
     this->largeFile = isLarge;
@@ -50,20 +52,20 @@ struct FileSize : public SerializeIF {
     }
   }
 
-  ReturnValue_t setFileSize(uint64_t fileSize, bool largeFile) {
+  ReturnValue_t setFileSize(uint64_t fileSize_, bool largeFile_) {
     if (not largeFile and fileSize > UINT32_MAX) {
       // TODO: emit warning here
       return returnvalue::FAILED;
     }
-    this->fileSize = fileSize;
+    this->fileSize = fileSize_;
-    this->largeFile = largeFile;
+    this->largeFile = largeFile_;
     return returnvalue::OK;
   }
 
   [[nodiscard]] bool isLargeFile() const { return largeFile; }
-  uint64_t getSize(bool *largeFile = nullptr) const {
+  uint64_t getSize(bool *largeFile_ = nullptr) const {
-    if (largeFile != nullptr) {
+    if (largeFile_ != nullptr) {
-      *largeFile = this->largeFile;
+      *largeFile_ = this->largeFile;
     }
     return fileSize;
   }

src/fsfw/cfdp/handler/DestHandler.cpp

@@ -2,10 +2,11 @@
 
 #include <utility>
 
+#include "fsfw/FSFW.h"
+#include "fsfw/cfdp/pdu/EofPduReader.h"
 #include "fsfw/cfdp/pdu/FileDataReader.h"
 #include "fsfw/cfdp/pdu/HeaderReader.h"
 #include "fsfw/objectmanager.h"
-#include "fsfw/serviceinterface.h"
 
 using namespace returnvalue;
 
@@ -29,7 +30,7 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() {
         if (result != OK) {
           status = result;
         }
-        // metadata packet was deleted in metadata handler because a store guard is used
+        // Store data was deleted in PDU handler because a store guard is used
         dp.packetListRef.erase(infoIter++);
       }
     }
@@ -54,9 +55,15 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() {
         if (result != OK) {
           status = result;
         }
+        // Store data was deleted in PDU handler because a store guard is used
+        dp.packetListRef.erase(infoIter++);
+      }
+      if (infoIter->pduType == PduType::FILE_DIRECTIVE and
+          infoIter->directiveType == FileDirectives::EOF_DIRECTIVE) {
+        result = handleEofPdu(*infoIter);
       }
     }
-    return returnvalue::OK;
+    return OK;
   }
   if (cfdpState == CfdpStates::BUSY_CLASS_2_ACKED) {
     // TODO: Will be implemented at a later stage
@@ -64,32 +71,32 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() {
     sif::warning << "CFDP state machine for acknowledged mode not implemented yet" << std::endl;
 #endif
   }
-  return returnvalue::OK;
+  return OK;
 }
 
 ReturnValue_t cfdp::DestHandler::passPacket(PacketInfo packet) {
   if (dp.packetListRef.full()) {
-    return returnvalue::FAILED;
+    return FAILED;
   }
   dp.packetListRef.push_back(packet);
-  return returnvalue::OK;
+  return OK;
 }
 
 ReturnValue_t cfdp::DestHandler::initialize() {
   if (fp.tmStore == nullptr) {
     fp.tmStore = ObjectManager::instance()->get<StorageManagerIF>(objects::TM_STORE);
     if (fp.tmStore == nullptr) {
-      return returnvalue::FAILED;
+      return FAILED;
     }
   }
 
   if (fp.tcStore == nullptr) {
     fp.tcStore = ObjectManager::instance()->get<StorageManagerIF>(objects::TC_STORE);
     if (fp.tcStore == nullptr) {
-      return returnvalue::FAILED;
+      return FAILED;
     }
   }
-  return returnvalue::OK;
+  return OK;
 }
 
 ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PacketInfo& info) {
@@ -122,11 +129,55 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const cfdp::PacketInfo& info)
   auto constAccessorPair = fp.tcStore->getData(info.storeId);
   if (constAccessorPair.first != OK) {
     // TODO: This is not a CFDP error. Event and/or warning?
-    cfdp::FileSize offset;
-    FileDataInfo fdInfo(offset);
-    FileDataReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), fdInfo);
     return constAccessorPair.first;
   }
+  cfdp::FileSize offset;
+  FileDataInfo fdInfo(offset);
+  FileDataReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), fdInfo);
+  ReturnValue_t result = reader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  size_t fileSegmentLen = 0;
+  const uint8_t* fileData = fdInfo.getFileData(&fileSegmentLen);
+  FileOpParams fileOpParams(tp.sourceName.data(), fileSegmentLen);
+  result = dp.user.vfs.writeToFile(fileOpParams, fileData);
+  if (dp.cfg.indicCfg.fileSegmentRecvIndicRequired) {
+    FileSegmentRecvdParams segParams;
+    segParams.offset = offset.value();
+    segParams.id = tp.transactionId;
+    segParams.length = fileSegmentLen;
+    segParams.recContState = fdInfo.getRecordContinuationState();
+    size_t segmentMetadatLen = 0;
+    auto* segMetadata = fdInfo.getSegmentMetadata(&segmentMetadatLen);
+    segParams.segmentMetadata = {segMetadata, segmentMetadatLen};
+    dp.user.fileSegmentRecvdIndication(segParams);
+  }
+  if (result != returnvalue::OK) {
+    // TODO: Proper Error handling
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+    sif::error << "File write error" << std::endl;
+#endif
+  }
+  return result;
+}
+
+ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PacketInfo& info) {
+  // Process EOF PDU
+  auto constAccessorPair = fp.tcStore->getData(info.storeId);
+  if (constAccessorPair.first != OK) {
+    // TODO: This is not a CFDP error. Event and/or warning?
+    return constAccessorPair.first;
+  }
+  EofInfo eofInfo(nullptr);
+  EofPduReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), eofInfo);
+  ReturnValue_t result = reader.parseData();
+  if (result != OK) {
+    return result;
+  }
+  if (eofInfo.getConditionCode() == ConditionCode::NO_ERROR) {
+    tp.crc = eofInfo.getChecksum();
+  }
   return returnvalue::OK;
 }
 

src/fsfw/cfdp/handler/DestHandler.h

@@ -76,10 +76,6 @@ class DestHandler {
 
   ReturnValue_t initialize();
 
-  ReturnValue_t handleMetadataPdu(const PacketInfo& info);
-  ReturnValue_t handleFileDataPdu(const PacketInfo& info);
-  ReturnValue_t handleMetadataParseError(const uint8_t* rawData, size_t maxSize);
-
   [[nodiscard]] CfdpStates getCfdpState() const;
 
  private:
@@ -96,12 +92,13 @@ class DestHandler {
         : sourceName(maxFileNameLen), destName(maxFileNameLen) {}
 
     ChecksumType checksumType = ChecksumType::NULL_CHECKSUM;
-    bool closureRequested{};
+    bool closureRequested = false;
     std::vector<char> sourceName;
     std::vector<char> destName;
     cfdp::FileSize fileSize;
     TransactionId transactionId;
     PduConfig pduConf;
+    uint32_t crc = 0;
     RemoteEntityCfg* remoteCfg = nullptr;
   };
 
@@ -114,6 +111,10 @@ class DestHandler {
   TransactionParams tp;
 
   ReturnValue_t startTransaction(MetadataPduReader& reader, MetadataInfo& info);
+  ReturnValue_t handleMetadataPdu(const PacketInfo& info);
+  ReturnValue_t handleFileDataPdu(const PacketInfo& info);
+  ReturnValue_t handleEofPdu(const PacketInfo& info);
+  ReturnValue_t handleMetadataParseError(const uint8_t* rawData, size_t maxSize);
 };
 
 }  // namespace cfdp

src/fsfw/cfdp/handler/UserBase.h

@@ -64,6 +64,8 @@ struct FileSegmentRecvdParams {
  * @param vfs Virtual Filestore Object. Will be used for all file operations
  */
 class UserBase {
+  friend class DestHandler;
+
  public:
   explicit UserBase(HasFileSystemIF& vfs);
 

src/fsfw/cfdp/pdu/CMakeLists.txt

@@ -9,8 +9,8 @@ target_sources(
           AckPduSerializer.cpp
           AckPduDeserializer.cpp
           EofInfo.cpp
-          EofPduSerializer.cpp
+          EofPduCreator.cpp
-          EofPduDeserializer.cpp
+          EofPduReader.cpp
           NakInfo.cpp
           NakPduSerializer.cpp
           NakPduDeserializer.cpp

src/fsfw/cfdp/pdu/EofInfo.h

@@ -7,16 +7,16 @@
 
 struct EofInfo {
  public:
-  EofInfo(EntityIdTlv* faultLoc = nullptr);
+  explicit EofInfo(EntityIdTlv* faultLoc = nullptr);
   EofInfo(cfdp::ConditionCode conditionCode, uint32_t checksum, cfdp::FileSize fileSize,
           EntityIdTlv* faultLoc = nullptr);
 
   size_t getSerializedSize(bool fssLarge = false);
 
-  uint32_t getChecksum() const;
+  [[nodiscard]] uint32_t getChecksum() const;
-  cfdp::ConditionCode getConditionCode() const;
+  [[nodiscard]] cfdp::ConditionCode getConditionCode() const;
 
-  EntityIdTlv* getFaultLoc() const;
+  [[nodiscard]] EntityIdTlv* getFaultLoc() const;
   cfdp::FileSize& getFileSize();
   void setChecksum(uint32_t checksum);
   void setConditionCode(cfdp::ConditionCode conditionCode);

src/fsfw/cfdp/pdu/EofPduCreator.cpp

@@ -1,19 +1,16 @@
-#include "EofPduSerializer.h"
+#include "EofPduCreator.h"
 
 #include "fsfw/FSFW.h"
-#include "fsfw/serviceinterface.h"
 
-EofPduSerializer::EofPduSerializer(PduConfig &conf, EofInfo &info)
+EofPduCreator::EofPduCreator(PduConfig &conf, EofInfo &info)
     : FileDirectiveCreator(conf, cfdp::FileDirectives::EOF_DIRECTIVE, 9), info(info) {
-  setDirectiveDataFieldLen(info.getSerializedSize(getLargeFileFlag()));
+  setDirectiveDataFieldLen(info.getSerializedSize(HeaderCreator::getLargeFileFlag()));
 }
 
-size_t EofPduSerializer::getSerializedSize() const {
+size_t EofPduCreator::getSerializedSize() const { return FileDirectiveCreator::getWholePduSize(); }
-  return FileDirectiveCreator::getWholePduSize();
-}
 
-ReturnValue_t EofPduSerializer::serialize(uint8_t **buffer, size_t *size, size_t maxSize,
+ReturnValue_t EofPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize,
-                                          Endianness streamEndianness) const {
+                                       Endianness streamEndianness) const {
   ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness);
   if (result != returnvalue::OK) {
     return result;

src/fsfw/cfdp/pdu/EofPduCreator.h

@@ -5,11 +5,11 @@
 #include "fsfw/cfdp/pdu/FileDirectiveCreator.h"
 #include "fsfw/cfdp/tlv/EntityIdTlv.h"
 
-class EofPduSerializer : public FileDirectiveCreator {
+class EofPduCreator : public FileDirectiveCreator {
  public:
-  EofPduSerializer(PduConfig& conf, EofInfo& info);
+  EofPduCreator(PduConfig& conf, EofInfo& info);
 
-  size_t getSerializedSize() const override;
+  [[nodiscard]] size_t getSerializedSize() const override;
 
   ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize,
                           Endianness streamEndianness) const override;

src/fsfw/cfdp/pdu/EofPduReader.cpp

@@ -1,12 +1,12 @@
-#include "EofPduDeserializer.h"
+#include "EofPduReader.h"
 
 #include "fsfw/FSFW.h"
 #include "fsfw/serviceinterface.h"
 
-EofPduDeserializer::EofPduDeserializer(const uint8_t* pduBuf, size_t maxSize, EofInfo& eofInfo)
+EofPduReader::EofPduReader(const uint8_t* pduBuf, size_t maxSize, EofInfo& eofInfo)
     : FileDirectiveReader(pduBuf, maxSize), info(eofInfo) {}
 
-ReturnValue_t EofPduDeserializer::parseData() {
+ReturnValue_t EofPduReader::parseData() {
   ReturnValue_t result = FileDirectiveReader::parseData();
   if (result != returnvalue::OK) {
     return result;

src/fsfw/cfdp/pdu/EofPduReader.h

@@ -4,11 +4,11 @@
 #include "EofInfo.h"
 #include "fsfw/cfdp/pdu/FileDirectiveReader.h"
 
-class EofPduDeserializer : public FileDirectiveReader {
+class EofPduReader : public FileDirectiveReader {
  public:
-  EofPduDeserializer(const uint8_t* pduBuf, size_t maxSize, EofInfo& eofInfo);
+  EofPduReader(const uint8_t* pduBuf, size_t maxSize, EofInfo& eofInfo);
 
-  virtual ReturnValue_t parseData() override;
+  ReturnValue_t parseData() override;
 
  private:
   EofInfo& info;

src/fsfw/cfdp/pdu/FileDataInfo.cpp

@@ -57,16 +57,16 @@ ReturnValue_t FileDataInfo::addSegmentMetadataInfo(cfdp::RecordContinuationState
   return returnvalue::OK;
 }
 
-const uint8_t *FileDataInfo::getFileData(size_t *fileSize) const {
+const uint8_t *FileDataInfo::getFileData(size_t *fileSize_) const {
-  if (fileSize != nullptr) {
+  if (fileSize_ != nullptr) {
-    *fileSize = this->fileSize;
+    *fileSize_ = this->fileSize;
   }
   return fileData;
 }
 
-const uint8_t *FileDataInfo::getSegmentMetadata(size_t *segmentMetadataLen) {
+const uint8_t *FileDataInfo::getSegmentMetadata(size_t *segmentMetadataLen_) {
-  if (segmentMetadataLen != nullptr) {
+  if (segmentMetadataLen_ != nullptr) {
-    *segmentMetadataLen = this->segmentMetadataLen;
+    *segmentMetadataLen_ = this->segmentMetadataLen;
   }
   return segmentMetadata;
 }

unittests/cfdp/pdu/testEofPdu.cpp

@@ -1,8 +1,8 @@
 #include <array>
 #include <catch2/catch_test_macros.hpp>
 
-#include "fsfw/cfdp/pdu/EofPduDeserializer.h"
+#include "fsfw/cfdp/pdu/EofPduCreator.h"
-#include "fsfw/cfdp/pdu/EofPduSerializer.h"
+#include "fsfw/cfdp/pdu/EofPduReader.h"
 #include "fsfw/globalfunctions/arrayprinter.h"
 
 TEST_CASE("EOF PDU", "[cfdp][pdu]") {
@@ -22,7 +22,7 @@ TEST_CASE("EOF PDU", "[cfdp][pdu]") {
 
   PduConfig pduConf(sourceId, destId, TransmissionModes::ACKNOWLEDGED, seqNum);
 
-  auto eofSerializer = EofPduSerializer(pduConf, eofInfo);
+  auto eofSerializer = EofPduCreator(pduConf, eofInfo);
   SECTION("Serialize") {
     result = eofSerializer.serialize(&bufPtr, &sz, buf.size(), SerializeIF::Endianness::NETWORK);
     REQUIRE(result == returnvalue::OK);
@@ -45,7 +45,7 @@ TEST_CASE("EOF PDU", "[cfdp][pdu]") {
     eofInfo.setFileSize(0x10ffffff10, true);
     pduConf.largeFile = true;
     // Should serialize with fault location now
-    auto serializeWithFaultLocation = EofPduSerializer(pduConf, eofInfo);
+    auto serializeWithFaultLocation = EofPduCreator(pduConf, eofInfo);
     bufPtr = buf.data();
     sz = 0;
     result = serializeWithFaultLocation.serialize(&bufPtr, &sz, buf.size(),
@@ -83,7 +83,7 @@ TEST_CASE("EOF PDU", "[cfdp][pdu]") {
     REQUIRE(result == returnvalue::OK);
     EntityIdTlv tlv(destId);
     EofInfo emptyInfo(&tlv);
-    auto deserializer = EofPduDeserializer(buf.data(), buf.size(), emptyInfo);
+    auto deserializer = EofPduReader(buf.data(), buf.size(), emptyInfo);
     result = deserializer.parseData();
     REQUIRE(result == returnvalue::OK);
     REQUIRE(emptyInfo.getConditionCode() == cfdp::ConditionCode::NO_ERROR);
@@ -94,12 +94,12 @@ TEST_CASE("EOF PDU", "[cfdp][pdu]") {
     eofInfo.setFileSize(0x10ffffff10, true);
     pduConf.largeFile = true;
     // Should serialize with fault location now
-    auto serializeWithFaultLocation = EofPduSerializer(pduConf, eofInfo);
+    auto serializeWithFaultLocation = EofPduCreator(pduConf, eofInfo);
     bufPtr = buf.data();
     sz = 0;
     result = serializeWithFaultLocation.serialize(&bufPtr, &sz, buf.size(),
                                                   SerializeIF::Endianness::NETWORK);
-    auto deserializer2 = EofPduDeserializer(buf.data(), buf.size(), emptyInfo);
+    auto deserializer2 = EofPduReader(buf.data(), buf.size(), emptyInfo);
     result = deserializer2.parseData();
     REQUIRE(result == returnvalue::OK);
     REQUIRE(emptyInfo.getConditionCode() == cfdp::ConditionCode::FILESTORE_REJECTION);
@@ -110,7 +110,7 @@ TEST_CASE("EOF PDU", "[cfdp][pdu]") {
     uint16_t destId = emptyInfo.getFaultLoc()->getEntityId().getValue();
     REQUIRE(destId == 2);
     for (size_t maxSz = 0; maxSz < deserializer2.getWholePduSize() - 1; maxSz++) {
-      auto invalidDeser = EofPduDeserializer(buf.data(), maxSz, emptyInfo);
+      auto invalidDeser = EofPduReader(buf.data(), maxSz, emptyInfo);
       result = invalidDeser.parseData();
       REQUIRE(result != returnvalue::OK);
     }