diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index e1578c9b..ac773cfc 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -174,7 +174,7 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const cfdp::PacketInfo& info) } size_t fileSegmentLen = 0; const uint8_t* fileData = fdInfo.getFileData(&fileSegmentLen); - FileOpParams fileOpParams(tp.sourceName.data(), fileSegmentLen); + FileOpParams fileOpParams(tp.destName.data(), fileSegmentLen); if (dp.cfg.indicCfg.fileSegmentRecvIndicRequired) { FileSegmentRecvdParams segParams; segParams.offset = offset.value(); @@ -372,9 +372,9 @@ ReturnValue_t cfdp::DestHandler::checksumVerification() { // TODO: Checksum verification and notice of completion etl::crc32 crcCalc; uint64_t currentOffset = 0; - FileOpParams params(tp.sourceName.data(), buf.size()); + FileOpParams params(tp.destName.data(), tp.fileSize.value()); while (currentOffset < tp.fileSize.value()) { - uint64_t readLen = 0; + uint64_t readLen; if (currentOffset + buf.size() > tp.fileSize.value()) { readLen = tp.fileSize.value() - currentOffset; } else { diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index b1c56721..71214fcb 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -4,6 +4,7 @@ #include #include +#include #include #include "RemoteConfigTableIF.h" @@ -20,11 +21,12 @@ namespace cfdp { struct PacketInfo { - PacketInfo(PduType type, FileDirectives directive, store_address_t storeId) + PacketInfo(PduType type, store_address_t storeId, + std::optional directive = std::nullopt) : pduType(type), directiveType(directive), storeId(storeId) {} PduType pduType = PduType::FILE_DATA; - FileDirectives directiveType = FileDirectives::INVALID_DIRECTIVE; + std::optional directiveType = FileDirectives::INVALID_DIRECTIVE; store_address_t storeId = store_address_t::invalid(); PacketInfo() = default; }; @@ -121,7 +123,7 @@ class DestHandler { [[nodiscard]] CfdpStates getCfdpState() const; [[nodiscard]] TransactionStep getTransactionStep() const; - const TransactionId& getTransactionId() const; + [[nodiscard]] const TransactionId& getTransactionId() const; private: struct TransactionParams { diff --git a/src/fsfw/cfdp/pdu/FileDataCreator.cpp b/src/fsfw/cfdp/pdu/FileDataCreator.cpp index 0ef8f862..956752fb 100644 --- a/src/fsfw/cfdp/pdu/FileDataCreator.cpp +++ b/src/fsfw/cfdp/pdu/FileDataCreator.cpp @@ -15,13 +15,17 @@ void FileDataCreator::update() { ReturnValue_t FileDataCreator::serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const { + if (buffer == nullptr or size == nullptr) { + return returnvalue::FAILED; + } + if (*size + getSerializedSize() > maxSize) { + return SerializeIF::BUFFER_TOO_SHORT; + } ReturnValue_t result = HeaderCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; } - if (*size + this->getSerializedSize() > maxSize) { - return SerializeIF::BUFFER_TOO_SHORT; - } + const uint8_t* readOnlyPtr = nullptr; if (this->hasSegmentMetadataFlag()) { size_t segmentMetadataLen = info.getSegmentMetadataLen(); diff --git a/src/fsfw/cfdp/pdu/FileDataCreator.h b/src/fsfw/cfdp/pdu/FileDataCreator.h index 774baf00..2ce8989d 100644 --- a/src/fsfw/cfdp/pdu/FileDataCreator.h +++ b/src/fsfw/cfdp/pdu/FileDataCreator.h @@ -11,6 +11,10 @@ class FileDataCreator : public HeaderCreator { void update(); + ReturnValue_t serialize(uint8_t* buf, size_t& serLen, size_t maxSize) const { + return SerializeIF::serialize(buf, serLen, maxSize, SerializeIF::Endianness::NETWORK); + } + ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const override; diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp index c6be9c14..23d88352 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -1,9 +1,11 @@ #include #include +#include #include "fsfw/cfdp.h" #include "fsfw/cfdp/pdu/EofPduCreator.h" +#include "fsfw/cfdp/pdu/FileDataCreator.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" #include "mocks/AcceptsTmMock.h" #include "mocks/EventReportingProxyMock.h" @@ -43,15 +45,17 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { fp.tmStore = &tmStore; uint8_t* buf = nullptr; size_t serLen = 0; + store_address_t storeId; + PduConfig conf; auto destHandler = DestHandler(dp, fp); CHECK(destHandler.initialize() == OK); - auto metadataPreparation = [&](PduConfig& conf, FileSize cfdpFileSize, store_address_t& storeId) { + auto metadataPreparation = [&](FileSize cfdpFileSize, ChecksumTypes checksumType) { std::string srcNameString = "hello.txt"; std::string destNameString = "hello-cpy.txt"; StringLv srcName(srcNameString); StringLv destName(destNameString); - MetadataInfo info(false, cfdp::ChecksumTypes::NULL_CHECKSUM, cfdpFileSize, srcName, destName); + MetadataInfo info(false, checksumType, cfdpFileSize, srcName, destName); TransactionSeqNum seqNum(UnsignedByteField(1)); conf.sourceId = remoteId; conf.destId = localId; @@ -60,12 +64,13 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { MetadataPduCreator metadataCreator(conf, info); REQUIRE(tcStore.getFreeElement(&storeId, metadataCreator.getSerializedSize(), &buf) == OK); REQUIRE(metadataCreator.serialize(buf, serLen, metadataCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(metadataCreator.getPduType(), metadataCreator.getDirectiveCode(), - storeId); + PacketInfo packetInfo(metadataCreator.getPduType(), storeId, + metadataCreator.getDirectiveCode()); packetInfoList.push_back(packetInfo); }; - auto metadataCheck = [&](const cfdp::DestHandler::FsmResult& res, store_address_t storeId, const char* sourceName, const char* destName) { + auto metadataCheck = [&](const cfdp::DestHandler::FsmResult& res, const char* sourceName, + const char* destName, size_t fileLen) { REQUIRE(res.result == OK); REQUIRE(res.callStatus == CallStatus::CALL_AGAIN); // Assert that the packet was deleted after handling @@ -75,7 +80,7 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { auto& idMetadataPair = userMock.metadataRecvd.back(); REQUIRE(idMetadataPair.first == destHandler.getTransactionId()); REQUIRE(idMetadataPair.second.sourceId.getValue() == 3); - REQUIRE(idMetadataPair.second.fileSize == 0); + REQUIRE(idMetadataPair.second.fileSize == fileLen); REQUIRE(strcmp(idMetadataPair.second.destFileName, destName) == 0); REQUIRE(strcmp(idMetadataPair.second.sourceFileName, sourceName) == 0); userMock.metadataRecvd.pop(); @@ -85,6 +90,31 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { REQUIRE(res.step == DestHandler::TransactionStep::RECEIVING_FILE_DATA_PDUS); }; + auto eofPreparation = [&](FileSize cfdpFileSize, uint32_t crc) { + EofInfo eofInfo(cfdp::ConditionCode::NO_ERROR, crc, std::move(cfdpFileSize)); + EofPduCreator eofCreator(conf, eofInfo); + REQUIRE(tcStore.getFreeElement(&storeId, eofCreator.getSerializedSize(), &buf) == OK); + REQUIRE(eofCreator.serialize(buf, serLen, eofCreator.getSerializedSize()) == OK); + PacketInfo packetInfo(eofCreator.getPduType(), storeId, eofCreator.getDirectiveCode()); + packetInfoList.push_back(packetInfo); + }; + + auto eofCheck = [&](const cfdp::DestHandler::FsmResult& res, const TransactionId& id) { + REQUIRE(res.result == OK); + REQUIRE(res.state == CfdpStates::IDLE); + REQUIRE(res.step == DestHandler::TransactionStep::IDLE); + // Assert that the packet was deleted after handling + REQUIRE(not tcStore.hasDataAtId(storeId)); + REQUIRE(packetInfoList.empty()); + REQUIRE(userMock.eofsRevd.size() == 1); + auto& eofId = userMock.eofsRevd.back(); + CHECK(eofId == id); + REQUIRE(userMock.finishedRecvd.size() == 1); + auto& idParamPair = userMock.finishedRecvd.back(); + CHECK(idParamPair.first == id); + CHECK(idParamPair.second.condCode == ConditionCode::NO_ERROR); + }; + SECTION("State") { CHECK(destHandler.getCfdpState() == CfdpStates::IDLE); CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); @@ -103,35 +133,16 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { const DestHandler::FsmResult& res = destHandler.performStateMachine(); CHECK(res.result == OK); FileSize cfdpFileSize(0); - store_address_t storeId; - PduConfig conf; - metadataPreparation(conf, cfdpFileSize, storeId); + metadataPreparation(cfdpFileSize, ChecksumTypes::NULL_CHECKSUM); destHandler.performStateMachine(); - metadataCheck(res, storeId, "hello.txt", "hello-cpy.txt"); + metadataCheck(res, "hello.txt", "hello-cpy.txt", 0); destHandler.performStateMachine(); REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); - EofInfo eofInfo(cfdp::ConditionCode::NO_ERROR, 0, cfdpFileSize); - EofPduCreator eofCreator(conf, eofInfo); - REQUIRE(tcStore.getFreeElement(&storeId, eofCreator.getSerializedSize(), &buf) == OK); - REQUIRE(eofCreator.serialize(buf, serLen, eofCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(eofCreator.getPduType(), eofCreator.getDirectiveCode(), storeId); - packetInfoList.push_back(packetInfo); auto transactionId = destHandler.getTransactionId(); + eofPreparation(cfdpFileSize, 0); // After EOF, operation is done because no closure was requested destHandler.performStateMachine(); - REQUIRE(res.result == OK); - REQUIRE(res.state == CfdpStates::IDLE); - REQUIRE(res.step == DestHandler::TransactionStep::IDLE); - // Assert that the packet was deleted after handling - REQUIRE(not tcStore.hasDataAtId(storeId)); - REQUIRE(packetInfoList.empty()); - REQUIRE(userMock.eofsRevd.size() == 1); - auto& eofId = userMock.eofsRevd.back(); - CHECK(eofId == transactionId); - REQUIRE(userMock.finishedRecvd.size() == 1); - auto& idParamPair = userMock.finishedRecvd.back(); - CHECK(idParamPair.first == transactionId); - CHECK(idParamPair.second.condCode == ConditionCode::NO_ERROR); + eofCheck(res, transactionId); } SECTION("Small File Transfer") { @@ -141,14 +152,26 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { etl::crc32 crcCalc; crcCalc.add(fileData.begin(), fileData.end()); uint32_t crc32 = crcCalc.value(); - FileSize cfdpFileSize(0); - store_address_t storeId; - PduConfig conf; - metadataPreparation(conf, cfdpFileSize, storeId); + FileSize cfdpFileSize(fileData.size()); + metadataPreparation(cfdpFileSize, ChecksumTypes::CRC_32); destHandler.performStateMachine(); - metadataCheck(res, storeId, "hello.txt", "hello-cpy.txt"); + metadataCheck(res, "hello.txt", "hello-cpy.txt", fileData.size()); destHandler.performStateMachine(); REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); + auto transactionId = destHandler.getTransactionId(); + FileSize offset(0); + FileDataInfo fdPduInfo(offset, reinterpret_cast(fileData.data()), + fileData.size()); + FileDataCreator fdPduCreator(conf, fdPduInfo); + REQUIRE(tcStore.getFreeElement(&storeId, fdPduCreator.getSerializedSize(), &buf) == OK); + REQUIRE(fdPduCreator.serialize(buf, serLen, fdPduCreator.getSerializedSize()) == OK); + PacketInfo packetInfo(fdPduCreator.getPduType(), storeId, std::nullopt); + packetInfoList.push_back(packetInfo); + destHandler.performStateMachine(); + eofPreparation(cfdpFileSize, crc32); + // After EOF, operation is done because no closure was requested + destHandler.performStateMachine(); + eofCheck(res, transactionId); } SECTION("Segmented File Transfer") {}