From dffce43e6b662f098e964e66f2026111c6bd2477 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 3 Aug 2023 15:30:38 +0200 Subject: [PATCH] metadata PDU seems to be correct --- src/fsfw/cfdp/handler/SourceHandler.cpp | 18 +++++++---- src/fsfw/cfdp/handler/SourceHandler.h | 1 - src/fsfw/cfdp/pdu/MetadataPduReader.h | 4 +-- src/fsfw/cfdp/tlv/StringLv.cpp | 7 +++- src/fsfw/cfdp/tlv/StringLv.h | 3 +- .../cfdp/handler/testReservedMsgParser.cpp | 12 +++---- unittests/cfdp/handler/testSourceHandler.cpp | 32 +++++++++++-------- 7 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/fsfw/cfdp/handler/SourceHandler.cpp b/src/fsfw/cfdp/handler/SourceHandler.cpp index 0b92fb74..e31903ae 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.cpp +++ b/src/fsfw/cfdp/handler/SourceHandler.cpp @@ -156,8 +156,8 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo if (putRequest.getDestName().getValueLen() == 0) { return DEST_NAME_EMPTY; } - const char* srcNamePtr = putRequest.getSourceName().getString(transactionParams.sourceNameSize); - const char* destNamePtr = putRequest.getDestName().getString(transactionParams.destNameSize); + const char* srcNamePtr = putRequest.getSourceName().getCString(transactionParams.sourceNameSize); + const char* destNamePtr = putRequest.getDestName().getCString(transactionParams.destNameSize); std::strncpy(transactionParams.sourceName.data(), srcNamePtr, transactionParams.sourceNameSize); std::strncpy(transactionParams.destName.data(), destNamePtr, transactionParams.destNameSize); FilesystemParams params(transactionParams.sourceName.data()); @@ -176,10 +176,15 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo transactionParams.closureRequested = cfg.closureRequested; } const EntityId& destId = putRequest.getDestId(); - transactionParams.pduConf.destId = destId; - // Adapt source ID width to necessary width. The width of the source and destination ID must be - // the same. - transactionParams.pduConf.sourceId.setWidth(destId.getWidth()); + // The width of the source and destination ID must be the same. Use the larger ID value to + // ensure the width is large enough for both IDs + if (destId.getWidth() > transactionParams.pduConf.sourceId.getWidth()) { + transactionParams.pduConf.destId = destId; + transactionParams.pduConf.sourceId.setWidth(destId.getWidth()); + } else { + transactionParams.pduConf.destId.setValueAndWidth(transactionParams.pduConf.sourceId.getWidth(), + destId.getValue()); + } // Only used for PDU forwarding, file is sent to file receiver regularly here. transactionParams.pduConf.direction = Direction::TOWARDS_RECEIVER; transactionParams.id.seqNum.setValue(sourceParams.seqCountProvider.getAndIncrement()); @@ -301,7 +306,6 @@ ReturnValue_t cfdp::SourceHandler::sendGenericPdu(const SerializeIF& pdu) const if (result != OK) { return result; } - arrayprinter::print(dataPtr, serializedLen); TmTcMessage tmMsg(storeId); return fsfwParams.msgQueue->sendMessage(fsfwParams.packetDest.getReportReceptionQueue(), &tmMsg); } diff --git a/src/fsfw/cfdp/handler/SourceHandler.h b/src/fsfw/cfdp/handler/SourceHandler.h index ea7d95a4..c31ebc26 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.h +++ b/src/fsfw/cfdp/handler/SourceHandler.h @@ -76,7 +76,6 @@ class SourceHandler { RemoteEntityCfg remoteCfg; PduConfig pduConf; cfdp::TransactionId id{}; - // cfdp::WidthInBytes seqCountWidth{}; void reset() { sourceNameSize = 0; diff --git a/src/fsfw/cfdp/pdu/MetadataPduReader.h b/src/fsfw/cfdp/pdu/MetadataPduReader.h index 89174f2f..e619d4f5 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduReader.h +++ b/src/fsfw/cfdp/pdu/MetadataPduReader.h @@ -11,8 +11,8 @@ class MetadataPduReader : public FileDirectiveReader { ReturnValue_t parseData() override; - const cfdp::StringLv& getSourceFileName() const; - const cfdp::StringLv& getDestFileName() const; + [[nodiscard]] const cfdp::StringLv& getSourceFileName() const; + [[nodiscard]] const cfdp::StringLv& getDestFileName() const; [[nodiscard]] size_t getNumberOfParsedOptions() const; diff --git a/src/fsfw/cfdp/tlv/StringLv.cpp b/src/fsfw/cfdp/tlv/StringLv.cpp index 551a23a9..ce290460 100644 --- a/src/fsfw/cfdp/tlv/StringLv.cpp +++ b/src/fsfw/cfdp/tlv/StringLv.cpp @@ -8,6 +8,11 @@ cfdp::StringLv::StringLv(const char* filename, size_t len) cfdp::StringLv::StringLv() : Lv() {} -const char* cfdp::StringLv::getString(size_t& fileSize) const { +const char* cfdp::StringLv::getCString(size_t& fileSize) const { return reinterpret_cast(getValue(&fileSize)); } + +std::string cfdp::StringLv::getString() const { + size_t fileSize; + return {getCString(fileSize), fileSize}; +} diff --git a/src/fsfw/cfdp/tlv/StringLv.h b/src/fsfw/cfdp/tlv/StringLv.h index c837c976..3a1dba36 100644 --- a/src/fsfw/cfdp/tlv/StringLv.h +++ b/src/fsfw/cfdp/tlv/StringLv.h @@ -13,7 +13,8 @@ class StringLv : public Lv { explicit StringLv(const std::string& fileName); explicit StringLv(const char* filename, size_t len); - const char* getString(size_t& fileSize) const; + const char* getCString(size_t& fileSize) const; + std::string getString() const; // Delete the move constructor to avoid passing in a temporary StringLv(const std::string&&) = delete; }; diff --git a/unittests/cfdp/handler/testReservedMsgParser.cpp b/unittests/cfdp/handler/testReservedMsgParser.cpp index eb6c1aa9..3379f563 100644 --- a/unittests/cfdp/handler/testReservedMsgParser.cpp +++ b/unittests/cfdp/handler/testReservedMsgParser.cpp @@ -58,14 +58,10 @@ TEST_CASE("Reserved Message Parser", "[cfdp]") { REQUIRE(putRequest.deSerialize(&data, &dummy, SerializeIF::Endianness::MACHINE) == OK); CHECK(putRequest.getDestId().getValue() == entityId.getValue()); CHECK(putRequest.getDestId().getWidth() == entityId.getWidth()); - size_t sourceNameSize = 0; auto& sourceNameLv = putRequest.getSourceName(); - const char* sourceString = sourceNameLv.getString(sourceNameSize); - CHECK(sourceNameSize == srcFileName.size()); - CHECK(std::strncmp(sourceString, srcFileName.c_str(), sourceNameSize) == 0); - size_t destNameSize = 0; + std::string srcNameRead = sourceNameLv.getString(); + CHECK(srcNameRead == srcFileName); auto& destNameLv = putRequest.getDestName(); - const char* destString = destNameLv.getString(destNameSize); - CHECK(destNameSize == destFileName.size()); - CHECK(std::strncmp(destString, destFileName.c_str(), destNameSize) == 0); + std::string destNameRead = destNameLv.getString(); + CHECK(destNameRead == destFileName); } diff --git a/unittests/cfdp/handler/testSourceHandler.cpp b/unittests/cfdp/handler/testSourceHandler.cpp index 1ab5cadc..ec93f2bf 100644 --- a/unittests/cfdp/handler/testSourceHandler.cpp +++ b/unittests/cfdp/handler/testSourceHandler.cpp @@ -54,18 +54,21 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { } SECTION("Transfer empty file") { - // using StorageManagerIF::getData; RemoteEntityCfg cfg; - EntityId id(cfdp::WidthInBytes::ONE_BYTE, 5); + EntityId id(cfdp::WidthInBytes::TWO_BYTES, 5); cfg.remoteId = id; - FilesystemParams srcFileName("/tmp/cfdp-test.txt"); - fsMock.createFile(srcFileName); - cfdp::StringLv srcNameLv(srcFileName.path, std::strlen(srcFileName.path)); - FilesystemParams destFileName("/tmp/cfdp-test2.txt"); - cfdp::StringLv destNameLv(destFileName.path, std::strlen(destFileName.path)); + std::string srcFileName = "/tmp/cfdp-test.txt"; + std::string destFileName = "/tmp/cfdp-test2.txt"; + FilesystemParams srcFileNameFs(srcFileName.c_str()); + fsMock.createFile(srcFileNameFs); + cfdp::StringLv srcNameLv(srcFileNameFs.path, std::strlen(srcFileNameFs.path)); + FilesystemParams destFileNameFs(destFileName.c_str()); + cfdp::StringLv destNameLv(destFileNameFs.path, std::strlen(destFileNameFs.path)); PutRequest putRequest(id, srcNameLv, destNameLv); CHECK(sourceHandler.transactionStart(putRequest, cfg) == OK); SourceHandler::FsmResult& fsmResult = sourceHandler.stateMachine(); + + // Verify metadata PDU was sent. CHECK(fsmResult.packetsSent == 1); CHECK(mqMock.numberOfSentMessages() == 1); TmTcMessage tmtcMessage; @@ -76,12 +79,15 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { CHECK(accessor.second.size() == 55); MetadataGenericInfo metadataInfo; MetadataPduReader metadataReader(pduPtr, accessor.second.size(), metadataInfo, nullptr, 0); - size_t srcFileSize = 0; - arrayprinter::print(pduPtr, accessor.second.size()); REQUIRE(metadataReader.parseData() == OK); - const char* srcNameRead = metadataReader.getSourceFileName().getString(srcFileSize); - REQUIRE(srcNameRead != nullptr); - std::string srcNameReadStr(srcNameRead, srcFileSize); - CHECK(std::string(srcFileName.path) == srcNameReadStr); + std::string srcNameRead = metadataReader.getSourceFileName().getString(); + CHECK(srcNameRead == srcFileName); + std::string destNameRead = metadataReader.getDestFileName().getString(); + CHECK(destNameRead == destFileName); + CHECK(metadataInfo.getChecksumType() == ChecksumType::NULL_CHECKSUM); + CHECK(metadataInfo.getFileSize().value() == 0); + CHECK(!metadataInfo.isClosureRequested()); + + // Verify EOF PDU was sent. No file data PDU is sent for an empty file. } } \ No newline at end of file