From 0f208ec75a5d0a0151a06614ed0b6be9e5250f3f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 27 Jul 2023 14:55:46 +0200 Subject: [PATCH] seems to work well --- src/fsfw/cfdp/handler/DestHandler.cpp | 18 +++++---- src/fsfw/cfdp/handler/DestHandler.h | 4 +- src/fsfw/cfdp/handler/PutRequest.cpp | 7 ++-- .../cfdp/handler/ReservedMessageParser.cpp | 4 +- src/fsfw/cfdp/handler/SourceHandler.cpp | 5 ++- src/fsfw/cfdp/handler/UserBase.h | 7 ++-- .../{MetadataInfo.h => MetadataGenericInfo.h} | 18 +++------ src/fsfw/cfdp/pdu/MetadataInfo.cpp | 39 +++++------------- src/fsfw/cfdp/pdu/MetadataPduCreator.cpp | 14 +++++-- src/fsfw/cfdp/pdu/MetadataPduCreator.h | 13 ++++-- src/fsfw/cfdp/pdu/MetadataPduReader.cpp | 13 ++++-- src/fsfw/cfdp/pdu/MetadataPduReader.h | 12 ++++-- src/fsfw/cfdp/tlv/Lv.cpp | 40 +++++++++++-------- src/fsfw/cfdp/tlv/Lv.h | 8 +++- unittests/cfdp/handler/testDestHandler.cpp | 6 +-- unittests/cfdp/handler/testDistributor.cpp | 5 +-- unittests/cfdp/handler/testPutRequest.cpp | 3 +- .../cfdp/handler/testReservedMsgParser.cpp | 7 ++-- unittests/cfdp/pdu/testMetadataPdu.cpp | 27 +++++++------ unittests/cfdp/testLvs.cpp | 16 +++++--- 20 files changed, 142 insertions(+), 124 deletions(-) rename src/fsfw/cfdp/pdu/{MetadataInfo.h => MetadataGenericInfo.h} (53%) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 6f1e9f40..83801df7 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -17,9 +17,9 @@ using namespace returnvalue; cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) : tlvVec(params.maxTlvsInOnePdu), msgToUserVec(params.maxTlvsInOnePdu), + transactionParams(params.maxFilenameLen), destParams(std::move(params)), - fsfwParams(fsfwParams), - transactionParams(params.maxFilenameLen) { + fsfwParams(fsfwParams) { transactionParams.pduConf.direction = cfdp::Direction::TOWARDS_SENDER; } @@ -135,7 +135,7 @@ ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PacketInfo& info) { } cfdp::StringLv sourceFileName; cfdp::StringLv destFileName; - MetadataInfo metadataInfo(transactionParams.fileSize, sourceFileName, destFileName); + MetadataGenericInfo metadataInfo(transactionParams.fileSize); MetadataPduReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), metadataInfo, tlvVec.data(), tlvVec.size()); ReturnValue_t result = reader.parseData(); @@ -275,7 +275,8 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, return returnvalue::FAILED; } -ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, MetadataInfo& info) { +ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, + MetadataGenericInfo& info) { if (fsmRes.state != CfdpState::IDLE) { // According to standard, discard metadata PDU if we are busy return OK; @@ -283,7 +284,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met ReturnValue_t result = OK; size_t sourceNameSize = 0; - const uint8_t* sourceNamePtr = info.getSourceFileName().getValue(&sourceNameSize); + const uint8_t* sourceNamePtr = reader.getSourceFileName().getValue(&sourceNameSize); if (sourceNameSize + 1 > transactionParams.sourceName.size()) { fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "source filename too large"); return FAILED; @@ -291,7 +292,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met std::memcpy(transactionParams.sourceName.data(), sourceNamePtr, sourceNameSize); transactionParams.sourceName[sourceNameSize] = '\0'; size_t destNameSize = 0; - const uint8_t* destNamePtr = info.getDestFileName().getValue(&destNameSize); + const uint8_t* destNamePtr = reader.getDestFileName().getValue(&destNameSize); if (destNameSize + 1 > transactionParams.destName.size()) { fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "dest filename too large"); return FAILED; @@ -350,9 +351,10 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met transactionParams.pduConf.direction = Direction::TOWARDS_SENDER; transactionParams.transactionId.entityId = transactionParams.pduConf.sourceId; transactionParams.transactionId.seqNum = transactionParams.pduConf.seqNum; + transactionParams.fileSize = info.getFileSize(); fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; - MetadataRecvdParams params(transactionParams.transactionId, transactionParams.pduConf.sourceId); - params.fileSize = transactionParams.fileSize.getSize(); + MetadataRecvdParams params(transactionParams.transactionId, transactionParams.pduConf.sourceId, + transactionParams.fileSize); params.destFileName = transactionParams.destName.data(); params.sourceFileName = transactionParams.sourceName.data(); unsigned tlvIdx = 0; diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index dc98d0e0..c9436b35 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -148,12 +148,12 @@ class DestHandler { std::vector tlvVec; std::vector msgToUserVec; + TransactionParams transactionParams; DestHandlerParams destParams; cfdp::FsfwParams fsfwParams; - TransactionParams transactionParams; FsmResult fsmRes; - ReturnValue_t startTransaction(MetadataPduReader& reader, MetadataInfo& info); + ReturnValue_t startTransaction(MetadataPduReader& reader, MetadataGenericInfo& info); ReturnValue_t handleMetadataPdu(const PacketInfo& info); ReturnValue_t handleFileDataPdu(const PacketInfo& info); ReturnValue_t handleEofPdu(const PacketInfo& info); diff --git a/src/fsfw/cfdp/handler/PutRequest.cpp b/src/fsfw/cfdp/handler/PutRequest.cpp index e0cafba1..899cf11b 100644 --- a/src/fsfw/cfdp/handler/PutRequest.cpp +++ b/src/fsfw/cfdp/handler/PutRequest.cpp @@ -14,7 +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)), sourceName(sourceName), destName(destName) {} + : destId(std::move(destId)), sourceName(std::move(sourceName)), destName(std::move(destName)) {} [[nodiscard]] bool cfdp::PutRequest::isMetadataOnly() const { return metadataOnly; } @@ -154,8 +154,9 @@ size_t cfdp::PutRequest::getSerializedSize() const { void cfdp::PutRequest::setSourceAndDestName(cfdp::StringLv &sourceName_, cfdp::StringLv &destName_) { - this->sourceName = sourceName_; - this->destName = destName_; + metadataOnly = false; + this->sourceName = std::move(sourceName_); + this->destName = std::move(destName_); } const cfdp::StringLv &cfdp::PutRequest::getSourceName() const { return sourceName; } diff --git a/src/fsfw/cfdp/handler/ReservedMessageParser.cpp b/src/fsfw/cfdp/handler/ReservedMessageParser.cpp index ab08915b..1165adb1 100644 --- a/src/fsfw/cfdp/handler/ReservedMessageParser.cpp +++ b/src/fsfw/cfdp/handler/ReservedMessageParser.cpp @@ -20,6 +20,8 @@ ReturnValue_t cfdp::ReservedMessageParser::parse(const uint8_t* msgsToUserPtr, const uint8_t* currentPtr = msgsToUserPtr; MessageToUserTlv tlv; size_t deserSize = sizeOfMessages; + cfdp::StringLv sourceFileName; + cfdp::StringLv destFileName; PutRequest putRequest; bool needToSendPutRequest = false; while (currentIdx < sizeOfMessages) { @@ -36,13 +38,11 @@ ReturnValue_t cfdp::ReservedMessageParser::parse(const uint8_t* msgsToUserPtr, return result; } putRequest.setDestId(entityIdLv); - cfdp::StringLv sourceFileName; result = sourceFileName.deSerialize(¤tPtr, &deserSize, SerializeIF::Endianness::NETWORK); if (result != OK) { return result; } - cfdp::StringLv destFileName; result = destFileName.deSerialize(¤tPtr, &deserSize, SerializeIF::Endianness::NETWORK); if (result != OK) { diff --git a/src/fsfw/cfdp/handler/SourceHandler.cpp b/src/fsfw/cfdp/handler/SourceHandler.cpp index b591ed23..cfe4cc23 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.cpp +++ b/src/fsfw/cfdp/handler/SourceHandler.cpp @@ -188,8 +188,9 @@ ReturnValue_t cfdp::SourceHandler::putRequest(PutRequestFull& putRequest, Remote ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() { cfdp::StringLv sourceName(transactionParams.sourceName.data(), transactionParams.sourceNameSize); cfdp::StringLv destName(transactionParams.destName.data(), transactionParams.destNameSize); - auto metadataInfo = MetadataInfo(transactionParams.fileSize, sourceName, destName); - auto metadataPdu = MetadataPduCreator(transactionParams.pduConf, metadataInfo, nullptr, 0); + auto metadataInfo = MetadataGenericInfo(transactionParams.fileSize); + auto metadataPdu = + MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0); ReturnValue_t result = sendGenericPdu(metadataPdu); if (result != OK) { return result; diff --git a/src/fsfw/cfdp/handler/UserBase.h b/src/fsfw/cfdp/handler/UserBase.h index d0ddeaa5..47df48c0 100644 --- a/src/fsfw/cfdp/handler/UserBase.h +++ b/src/fsfw/cfdp/handler/UserBase.h @@ -6,6 +6,7 @@ #include #include "StatusReportIF.h" +#include "fsfw/cfdp/Fss.h" #include "fsfw/cfdp/VarLenFields.h" #include "fsfw/cfdp/tlv/FilestoreResponseTlv.h" #include "fsfw/cfdp/tlv/MessageToUserTlv.h" @@ -27,11 +28,11 @@ struct TransactionFinishedParams { }; struct MetadataRecvdParams { - MetadataRecvdParams(const TransactionId& id, const EntityId& sourceId) - : id(id), sourceId(sourceId) {} + MetadataRecvdParams(const TransactionId& id, const EntityId& sourceId, Fss fileSize) + : id(id), sourceId(sourceId), fileSize(std::move(fileSize)) {} const TransactionId& id; const EntityId& sourceId; - uint64_t fileSize = 0; + Fss fileSize{}; const char* sourceFileName = ""; const char* destFileName = ""; size_t msgsToUserLen = 0; diff --git a/src/fsfw/cfdp/pdu/MetadataInfo.h b/src/fsfw/cfdp/pdu/MetadataGenericInfo.h similarity index 53% rename from src/fsfw/cfdp/pdu/MetadataInfo.h rename to src/fsfw/cfdp/pdu/MetadataGenericInfo.h index eef1447e..e8f89266 100644 --- a/src/fsfw/cfdp/pdu/MetadataInfo.h +++ b/src/fsfw/cfdp/pdu/MetadataGenericInfo.h @@ -9,32 +9,24 @@ #include "fsfw/cfdp/tlv/StringLv.h" #include "fsfw/cfdp/tlv/Tlv.h" -class MetadataInfo { +class MetadataGenericInfo { public: - MetadataInfo(cfdp::Fss& fileSize, cfdp::StringLv& sourceFileName, cfdp::StringLv& destFileName); - MetadataInfo(bool closureRequested, cfdp::ChecksumType checksumType, cfdp::Fss& fileSize, - cfdp::StringLv& sourceFileName, cfdp::StringLv& destFileName); + explicit MetadataGenericInfo(cfdp::Fss fileSize); + MetadataGenericInfo(bool closureRequested, cfdp::ChecksumType checksumType, cfdp::Fss fileSize); - size_t getSerializedSize(bool fssLarge = false); + static size_t getSerializedSize(bool fssLarge = false); [[nodiscard]] cfdp::ChecksumType getChecksumType() const; void setChecksumType(cfdp::ChecksumType checksumType); [[nodiscard]] bool isClosureRequested() const; void setClosureRequested(bool closureRequested = false); - void setDestFileName(cfdp::StringLv& destFileName); - void setSourceFileName(cfdp::StringLv& sourceFileName); - - cfdp::StringLv& getDestFileName(); - cfdp::StringLv& getSourceFileName(); cfdp::Fss& getFileSize(); private: bool closureRequested = false; cfdp::ChecksumType checksumType = cfdp::ChecksumType::NULL_CHECKSUM; - cfdp::Fss& fileSize; - cfdp::StringLv& sourceFileName; - cfdp::StringLv& destFileName; + cfdp::Fss fileSize; }; #endif /* FSFW_SRC_FSFW_CFDP_PDU_METADATAINFO_H_ */ diff --git a/src/fsfw/cfdp/pdu/MetadataInfo.cpp b/src/fsfw/cfdp/pdu/MetadataInfo.cpp index 3ec38f41..efbe620e 100644 --- a/src/fsfw/cfdp/pdu/MetadataInfo.cpp +++ b/src/fsfw/cfdp/pdu/MetadataInfo.cpp @@ -1,50 +1,33 @@ -#include "MetadataInfo.h" +#include "MetadataGenericInfo.h" -MetadataInfo::MetadataInfo(bool closureRequested, cfdp::ChecksumType checksumType, - cfdp::Fss& fileSize, cfdp::StringLv& sourceFileName, - cfdp::StringLv& destFileName) - : MetadataInfo(fileSize, sourceFileName, destFileName) { +MetadataGenericInfo::MetadataGenericInfo(bool closureRequested, cfdp::ChecksumType checksumType, + cfdp::Fss fileSize) + : MetadataGenericInfo(std::move(fileSize)) { this->closureRequested = closureRequested; this->checksumType = checksumType; } -MetadataInfo::MetadataInfo(cfdp::Fss& fileSize, cfdp::StringLv& sourceFileName, - cfdp::StringLv& destFileName) - : fileSize(fileSize), sourceFileName(sourceFileName), destFileName(destFileName) {} +MetadataGenericInfo::MetadataGenericInfo(cfdp::Fss fileSize) : fileSize(std::move(fileSize)) {} -cfdp::ChecksumType MetadataInfo::getChecksumType() const { return checksumType; } +cfdp::ChecksumType MetadataGenericInfo::getChecksumType() const { return checksumType; } -void MetadataInfo::setChecksumType(cfdp::ChecksumType checksumType_) { +void MetadataGenericInfo::setChecksumType(cfdp::ChecksumType checksumType_) { checksumType = checksumType_; } -bool MetadataInfo::isClosureRequested() const { return closureRequested; } +bool MetadataGenericInfo::isClosureRequested() const { return closureRequested; } -void MetadataInfo::setClosureRequested(bool closureRequested_) { +void MetadataGenericInfo::setClosureRequested(bool closureRequested_) { closureRequested = closureRequested_; } -cfdp::StringLv& MetadataInfo::getDestFileName() { return destFileName; } +cfdp::Fss& MetadataGenericInfo::getFileSize() { return fileSize; } -cfdp::Fss& MetadataInfo::getFileSize() { return fileSize; } - -size_t MetadataInfo::getSerializedSize(bool fssLarge) { +size_t MetadataGenericInfo::getSerializedSize(bool fssLarge) { // 1 byte + minimal FSS 4 bytes size_t size = 5; if (fssLarge) { size += 4; } - size += sourceFileName.getSerializedSize(); - size += destFileName.getSerializedSize(); return size; } - -void MetadataInfo::setDestFileName(cfdp::StringLv& destFileName_) { - this->destFileName = destFileName_; -} - -void MetadataInfo::setSourceFileName(cfdp::StringLv& sourceFileName_) { - this->sourceFileName = sourceFileName_; -} - -cfdp::StringLv& MetadataInfo::getSourceFileName() { return sourceFileName; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp b/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp index e31e7903..9f8633cb 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp @@ -1,16 +1,20 @@ #include "MetadataPduCreator.h" -MetadataPduCreator::MetadataPduCreator(PduConfig &conf, MetadataInfo &info, +MetadataPduCreator::MetadataPduCreator(PduConfig &conf, MetadataGenericInfo &info, + cfdp::StringLv &srcFileName, cfdp::StringLv &destFileName, cfdp::Tlv **optionsArray, size_t optionsLen) : FileDirectiveCreator(conf, cfdp::FileDirective::METADATA, 5), info(info), + srcFileName(srcFileName), + destFileName(destFileName), optionsArray(optionsArray), optionsLen(optionsLen) { updateDirectiveFieldLen(); } void MetadataPduCreator::updateDirectiveFieldLen() { - size_t dirFieldLen = info.getSerializedSize(HeaderCreator::getLargeFileFlag()); + size_t dirFieldLen = MetadataGenericInfo::getSerializedSize(HeaderCreator::getLargeFileFlag()) + + srcFileName.getSerializedSize() + destFileName.getSerializedSize(); if (optionsLen > 0 and optionsArray != nullptr) { for (size_t idx = 0; idx < optionsLen; idx++) { dirFieldLen += optionsArray[idx]->getSerializedSize(); @@ -39,11 +43,11 @@ ReturnValue_t MetadataPduCreator::serialize(uint8_t **buffer, size_t *size, size if (result != returnvalue::OK) { return result; } - result = info.getSourceFileName().serialize(buffer, size, maxSize, streamEndianness); + result = srcFileName.serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; } - result = info.getDestFileName().serialize(buffer, size, maxSize, streamEndianness); + result = destFileName.serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; } @@ -58,3 +62,5 @@ ReturnValue_t MetadataPduCreator::serialize(uint8_t **buffer, size_t *size, size } return result; } +const cfdp::StringLv &MetadataPduCreator::getSourceFileName() const { return srcFileName; } +const cfdp::StringLv &MetadataPduCreator::getDestFileName() const { return destFileName; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduCreator.h b/src/fsfw/cfdp/pdu/MetadataPduCreator.h index a6f8331c..e1c931e5 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduCreator.h +++ b/src/fsfw/cfdp/pdu/MetadataPduCreator.h @@ -2,23 +2,28 @@ #define FSFW_CFDP_PDU_METADATAPDUCREATOR_H_ #include "fsfw/cfdp/pdu/FileDirectiveCreator.h" -#include "fsfw/cfdp/pdu/MetadataInfo.h" +#include "fsfw/cfdp/pdu/MetadataGenericInfo.h" class MetadataPduCreator : public FileDirectiveCreator { public: - MetadataPduCreator(PduConfig& conf, MetadataInfo& info, cfdp::Tlv** optionsArray, - size_t optionsLen); + MetadataPduCreator(PduConfig& conf, MetadataGenericInfo& info, cfdp::StringLv& srcFileName, + cfdp::StringLv& destFileName, cfdp::Tlv** optionsArray, size_t optionsLen); void updateDirectiveFieldLen(); [[nodiscard]] size_t getSerializedSize() const override; + const cfdp::StringLv& getSourceFileName() const; + const cfdp::StringLv& getDestFileName() const; + ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const override; using FileDirectiveCreator::serialize; private: - MetadataInfo& info; + MetadataGenericInfo& info; + cfdp::StringLv& srcFileName; + cfdp::StringLv& destFileName; cfdp::Tlv** optionsArray; size_t optionsLen; }; diff --git a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp index 104ef2ae..69a7d4aa 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp +++ b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp @@ -1,7 +1,8 @@ #include "MetadataPduReader.h" -MetadataPduReader::MetadataPduReader(const uint8_t* pduBuf, size_t maxSize, MetadataInfo& info, - cfdp::Tlv* optionsArray, size_t optArrayMaxSize) +MetadataPduReader::MetadataPduReader(const uint8_t* pduBuf, size_t maxSize, + MetadataGenericInfo& info, cfdp::Tlv* optionsArray, + size_t optArrayMaxSize) : FileDirectiveReader(pduBuf, maxSize), info(info), optionArray(optionsArray), @@ -28,11 +29,11 @@ ReturnValue_t MetadataPduReader::parseData() { if (result != returnvalue::OK) { return result; } - result = info.getSourceFileName().deSerialize(&buf, &remSize, endianness); + result = srcFileName.deSerialize(&buf, &remSize, endianness); if (result != returnvalue::OK) { return result; } - result = info.getDestFileName().deSerialize(&buf, &remSize, endianness); + result = destFileName.deSerialize(&buf, &remSize, endianness); if (result != returnvalue::OK) { return result; } @@ -58,3 +59,7 @@ ReturnValue_t MetadataPduReader::parseData() { } size_t MetadataPduReader::getNumberOfParsedOptions() const { return parsedOptions; } + +const cfdp::StringLv& MetadataPduReader::getSourceFileName() const { return srcFileName; } + +const cfdp::StringLv& MetadataPduReader::getDestFileName() const { return destFileName; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduReader.h b/src/fsfw/cfdp/pdu/MetadataPduReader.h index ff5f52a8..89174f2f 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduReader.h +++ b/src/fsfw/cfdp/pdu/MetadataPduReader.h @@ -2,18 +2,24 @@ #define FSFW_CFDP_PDU_METADATAPDUREADER_H_ #include "fsfw/cfdp/pdu/FileDirectiveReader.h" -#include "fsfw/cfdp/pdu/MetadataInfo.h" +#include "fsfw/cfdp/pdu/MetadataGenericInfo.h" class MetadataPduReader : public FileDirectiveReader { public: - MetadataPduReader(const uint8_t* pduBuf, size_t maxSize, MetadataInfo& info, + MetadataPduReader(const uint8_t* pduBuf, size_t maxSize, MetadataGenericInfo& info, cfdp::Tlv* optionsArray, size_t optArrayMaxSize); ReturnValue_t parseData() override; + + const cfdp::StringLv& getSourceFileName() const; + const cfdp::StringLv& getDestFileName() const; + [[nodiscard]] size_t getNumberOfParsedOptions() const; private: - MetadataInfo& info; + cfdp::StringLv srcFileName; + cfdp::StringLv destFileName; + MetadataGenericInfo& info; cfdp::Tlv* optionArray; size_t optionArrayMaxSize; size_t parsedOptions = 0; diff --git a/src/fsfw/cfdp/tlv/Lv.cpp b/src/fsfw/cfdp/tlv/Lv.cpp index 215b9b3b..3bbfb5b9 100644 --- a/src/fsfw/cfdp/tlv/Lv.cpp +++ b/src/fsfw/cfdp/tlv/Lv.cpp @@ -14,23 +14,6 @@ cfdp::Lv::Lv(const std::vector& data) : value(data.data(), data.size(), cfdp::Lv::Lv() : value(static_cast(nullptr), 0, true) {} -cfdp::Lv::Lv(const Lv& other) - : value(other.value.getConstBuffer(), other.value.getSerializedSize() - 1, true) { - if (other.value.getSerializedSize() - 1 > 0) { - zeroLen = false; - } -} - -cfdp::Lv& cfdp::Lv::operator=(const Lv& other) { - size_t otherSize = 0; - auto* otherVal = const_cast(other.getValue(&otherSize)); - if (otherVal == nullptr or otherSize == 0) { - this->zeroLen = true; - } - this->value.setConstBuffer(otherVal, otherSize); - return *this; -} - ReturnValue_t cfdp::Lv::serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const { if (maxSize < 1) { @@ -87,3 +70,26 @@ const uint8_t* cfdp::Lv::getValue(size_t* size) const { } return value.getConstBuffer(); } +cfdp::Lv::Lv(cfdp::Lv&& other) noexcept + : value(other.value.getConstBuffer(), other.value.getSerializedSize() - 1, true) { + if (other.value.getSerializedSize() - 1 > 0) { + zeroLen = false; + } + // Leave other class in intact state. + other.zeroLen = false; + other.value = SerialBufferAdapter(); +} + +cfdp::Lv& cfdp::Lv::operator=(cfdp::Lv&& other) noexcept { + size_t otherSize = 0; + this->zeroLen = false; + auto* otherVal = const_cast(other.getValue(&otherSize)); + if (otherVal == nullptr or otherSize == 0) { + this->zeroLen = true; + } + this->value.setConstBuffer(otherVal, otherSize); + // Leave other class in intact state. + other.zeroLen = false; + other.value = SerialBufferAdapter(); + return *this; +} diff --git a/src/fsfw/cfdp/tlv/Lv.h b/src/fsfw/cfdp/tlv/Lv.h index efabfdef..a17d4617 100644 --- a/src/fsfw/cfdp/tlv/Lv.h +++ b/src/fsfw/cfdp/tlv/Lv.h @@ -18,8 +18,12 @@ class Lv : public SerializeIF { Lv(const uint8_t* value, size_t size); Lv(); - Lv(const Lv&); - Lv& operator=(const Lv&); + // Semantically, this class is a zero-copy helper, so the copy ctor and copy assigment do not + // really make sense here. + Lv(const Lv&) = delete; + Lv& operator=(const Lv&) = delete; + Lv(Lv&&) noexcept; + Lv& operator=(Lv&&) noexcept; 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 752cc4f2..2b6e4a4e 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -55,13 +55,13 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { std::string destNameString = "hello-cpy.txt"; StringLv srcName(srcNameString); StringLv destName(destNameString); - MetadataInfo info(false, checksumType, cfdpFileSize, srcName, destName); + MetadataGenericInfo info(false, checksumType, std::move(cfdpFileSize)); TransactionSeqNum seqNum(UnsignedByteField(1)); conf.sourceId = remoteId; conf.destId = localId; conf.mode = TransmissionMode::UNACKNOWLEDGED; conf.seqNum = seqNum; - MetadataPduCreator metadataCreator(conf, info, nullptr, 0); + MetadataPduCreator metadataCreator(conf, info, srcName, destName, nullptr, 0); REQUIRE(tcStore.getFreeElement(&storeId, metadataCreator.getSerializedSize(), &buf) == OK); REQUIRE(metadataCreator.serialize(buf, serLen, metadataCreator.getSerializedSize()) == OK); PacketInfo packetInfo(metadataCreator.getPduType(), storeId, @@ -81,7 +81,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 == fileLen); + REQUIRE(idMetadataPair.second.fileSize.getSize(nullptr) == fileLen); REQUIRE(strcmp(idMetadataPair.second.destFileName, destName) == 0); REQUIRE(strcmp(idMetadataPair.second.sourceFileName, sourceName) == 0); userMock.metadataRecvd.pop(); diff --git a/unittests/cfdp/handler/testDistributor.cpp b/unittests/cfdp/handler/testDistributor.cpp index 7d38e5c5..f6685c89 100644 --- a/unittests/cfdp/handler/testDistributor.cpp +++ b/unittests/cfdp/handler/testDistributor.cpp @@ -30,9 +30,8 @@ TEST_CASE("CFDP Distributor", "[cfdp][distributor]") { cfdp::StringLv sourceFileName(sourceFileString); std::string destFileString = "hello2.txt"; cfdp::StringLv destFileName(destFileString); - MetadataInfo metadataInfo(false, cfdp::ChecksumType::CRC_32, fileSize, sourceFileName, - destFileName); - MetadataPduCreator creator(pduConf, metadataInfo, nullptr, 0); + MetadataGenericInfo metadataInfo(false, cfdp::ChecksumType::CRC_32, fileSize); + MetadataPduCreator creator(pduConf, metadataInfo, sourceFileName, destFileName, nullptr, 0); uint8_t* dataPtr = nullptr; SECTION("State") { diff --git a/unittests/cfdp/handler/testPutRequest.cpp b/unittests/cfdp/handler/testPutRequest.cpp index e49ec1dd..6dcf5ca8 100644 --- a/unittests/cfdp/handler/testPutRequest.cpp +++ b/unittests/cfdp/handler/testPutRequest.cpp @@ -40,7 +40,8 @@ TEST_CASE("Put Request", "[cfdp]") { PutRequest requestDeserialized; size_t deserLen = putRequest.getSerializedSize(); const uint8_t* deserPtr = buffer.data(); - REQUIRE(requestDeserialized.deSerialize(&deserPtr, &deserLen, SerializeIF::Endianness::NETWORK) == OK); + REQUIRE(requestDeserialized.deSerialize(&deserPtr, &deserLen, + SerializeIF::Endianness::NETWORK) == OK); CHECK(requestDeserialized.getDestId().getWidth() == destId.getWidth()); CHECK(requestDeserialized.getDestId().getValue() == destId.getValue()); size_t totalMsgsSize = 0; diff --git a/unittests/cfdp/handler/testReservedMsgParser.cpp b/unittests/cfdp/handler/testReservedMsgParser.cpp index 2d5b942d..eeb4317e 100644 --- a/unittests/cfdp/handler/testReservedMsgParser.cpp +++ b/unittests/cfdp/handler/testReservedMsgParser.cpp @@ -58,7 +58,8 @@ 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; - // const char* sourceNameStart - // CHECK(putRequest.getSourceName(sourceNameSize)); + size_t sourceNameSize = 0; + auto& sourceNameLv = putRequest.getSourceName(); + sourceNameLv.getString(sourceNameSize); + CHECK(sourceNameSize == srcFileName.size()); } diff --git a/unittests/cfdp/pdu/testMetadataPdu.cpp b/unittests/cfdp/pdu/testMetadataPdu.cpp index ba2fb4da..38b5c11e 100644 --- a/unittests/cfdp/pdu/testMetadataPdu.cpp +++ b/unittests/cfdp/pdu/testMetadataPdu.cpp @@ -22,7 +22,7 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { cfdp::StringLv sourceFileName(firstFileName); cfdp::StringLv destFileName; Fss fileSize(35); - MetadataInfo info(false, ChecksumType::MODULAR, fileSize, sourceFileName, destFileName); + MetadataGenericInfo info(false, ChecksumType::MODULAR, fileSize); FilestoreResponseTlv response(FilestoreActionCode::CREATE_DIRECTORY, FSR_CREATE_NOT_ALLOWED, sourceFileName, nullptr); @@ -37,13 +37,13 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { REQUIRE(options[1]->getSerializedSize() == 5); SECTION("Serialize") { - MetadataPduCreator serializer(pduConf, info, nullptr, 0); + MetadataPduCreator serializer(pduConf, info, sourceFileName, destFileName, nullptr, 0); result = serializer.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK); REQUIRE(result == returnvalue::OK); REQUIRE(serializer.getWholePduSize() == 27); - REQUIRE(info.getSourceFileName().getSerializedSize() == 10); - REQUIRE(info.getDestFileName().getSerializedSize() == 1); - REQUIRE(info.getSerializedSize() == 16); + REQUIRE(serializer.getSourceFileName().getSerializedSize() == 10); + REQUIRE(serializer.getDestFileName().getSerializedSize() == 1); + REQUIRE(info.getSerializedSize() == 5); REQUIRE((mdBuffer[1] << 8 | mdBuffer[2]) == 17); REQUIRE(mdBuffer[10] == FileDirective::METADATA); // no closure requested and checksum type is modular => 0x00 @@ -69,8 +69,8 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { SECTION("Serialize with 2 options") { std::string otherFileName = "hello2.txt"; cfdp::StringLv otherFileNameLv(otherFileName.data(), otherFileName.size()); - info.setSourceFileName(otherFileNameLv); - MetadataPduCreator serializer(pduConf, info, options.data(), options.size()); + MetadataPduCreator serializer(pduConf, info, otherFileNameLv, destFileName, options.data(), + options.size()); info.setChecksumType(cfdp::ChecksumType::CRC_32C); info.setClosureRequested(true); serializer.updateDirectiveFieldLen(); @@ -104,11 +104,10 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { result = serializer.serialize(&buffer, &sz, 46, SerializeIF::Endianness::NETWORK); REQUIRE(result == SerializeIF::BUFFER_TOO_SHORT); } - info.setDestFileName(destFileName); } SECTION("Deserialize") { - MetadataPduCreator serializer(pduConf, info, nullptr, 0); + MetadataPduCreator serializer(pduConf, info, sourceFileName, destFileName, nullptr, 0); result = serializer.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK); REQUIRE(result == returnvalue::OK); @@ -124,12 +123,13 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { } SECTION("Deserialize with 2 options") { - MetadataPduCreator serializer(pduConf, info, options.data(), options.size()); + MetadataPduCreator serializer(pduConf, info, sourceFileName, destFileName, options.data(), + options.size()); info.setChecksumType(cfdp::ChecksumType::CRC_32C); info.setClosureRequested(true); serializer.updateDirectiveFieldLen(); - info.setSourceFileName(sourceFileName); + // info.setSourceFileName(sourceFileName); result = serializer.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK); REQUIRE(result == returnvalue::OK); @@ -165,14 +165,15 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { } SECTION("Can not parse options") { - MetadataPduCreator serializer(pduConf, info, options.data(), options.size()); + MetadataPduCreator serializer(pduConf, info, sourceFileName, destFileName, options.data(), + options.size()); info.setChecksumType(cfdp::ChecksumType::CRC_32C); info.setClosureRequested(true); buffer = mdBuffer.data(); sz = 0; serializer.updateDirectiveFieldLen(); - info.setSourceFileName(sourceFileName); + // info.setSourceFileName(sourceFileName); result = serializer.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK); REQUIRE(result == returnvalue::OK); diff --git a/unittests/cfdp/testLvs.cpp b/unittests/cfdp/testLvs.cpp index 22094568..8415f0d8 100644 --- a/unittests/cfdp/testLvs.cpp +++ b/unittests/cfdp/testLvs.cpp @@ -23,12 +23,6 @@ TEST_CASE("CFDP LV", "[cfdp][lv]") { auto lv = cfdp::Lv(lvRawBuf.data(), 2); REQUIRE(lv.getSerializedSize() == 3); - SECTION("Copy") { - auto lvCopy = cfdp::Lv(lv); - REQUIRE(lvCopy.getSerializedSize() == 3); - REQUIRE(lv.getValue(nullptr) == lvCopy.getValue(nullptr)); - } - serPtr = rawBuf.data(); deserSize = 0; REQUIRE(lv.serialize(&serPtr, &deserSize, rawBuf.size(), SerializeIF::Endianness::NETWORK) == @@ -41,6 +35,16 @@ TEST_CASE("CFDP LV", "[cfdp][lv]") { REQUIRE(sourceIdRaw == 0x0ff0); } + SECTION("Move LV") { + std::array lvRawBuf{}; + serPtr = lvRawBuf.data(); + REQUIRE(sourceId.serialize(&serPtr, &deserSize, lvRawBuf.size(), + SerializeIF::Endianness::NETWORK) == returnvalue::OK); + auto lv = cfdp::Lv(lvRawBuf.data(), 2); + auto lvMovedCopy = cfdp::Lv(std::move(lv)); + REQUIRE(lvMovedCopy.getSerializedSize() == 3); + } + SECTION("Empty Serialization") { auto lvEmpty = Lv(); REQUIRE(lvEmpty.getSerializedSize() == 1);