From 9441b4a70e4cc1aaef9cd5e680b3cd6388b99eaf Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 23 Aug 2022 19:37:30 +0200 Subject: [PATCH] continue dest handler --- src/fsfw/cfdp/handler/DestHandler.cpp | 96 ++++++++++++++++++++++--- src/fsfw/cfdp/handler/DestHandler.h | 25 +++++-- src/fsfw/cfdp/pdu/MetadataInfo.cpp | 39 +++++----- src/fsfw/cfdp/pdu/MetadataInfo.h | 25 ++++--- src/fsfw/events/EventReportingProxyIF.h | 2 +- unittests/cfdp/pdu/testMetadataPdu.cpp | 13 ++-- 6 files changed, 148 insertions(+), 52 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 85ef03f4..2d5c6aa0 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -2,15 +2,31 @@ #include +#include "fsfw/cfdp/pdu/HeaderReader.h" +#include "fsfw/cfdp/pdu/MetadataPduReader.h" #include "fsfw/objectmanager.h" +#include "fsfw/serviceinterface.h" -cfdp::DestHandler::DestHandler(DestHandlerParams params) : p(std::move(params)) {} +using namespace returnvalue; + +cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) + : dp(std::move(params)), fp(fsfwParams), tlvVec(params.maxTlvsInOnePdu) {} ReturnValue_t cfdp::DestHandler::performStateMachine() { switch (step) { case TransactionStep::IDLE: { - for (const auto& info : p.packetListRef) { + ReturnValue_t status = returnvalue::OK; + ReturnValue_t result; + for (const auto& info : dp.packetListRef) { + if (info.pduType == PduType::FILE_DIRECTIVE and + info.directiveType == FileDirectives::METADATA) { + result = handleMetadataPdu(info); + if (result != OK) { + status = result; + } + } } + return status; } case TransactionStep::TRANSACTION_START: break; @@ -27,26 +43,86 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() { } ReturnValue_t cfdp::DestHandler::passPacket(PacketInfo packet) { - if (p.packetListRef.full()) { + if (dp.packetListRef.full()) { return returnvalue::FAILED; } - p.packetListRef.push_back(packet); + dp.packetListRef.push_back(packet); return returnvalue::OK; } ReturnValue_t cfdp::DestHandler::initialize() { - if (p.tmStore == nullptr) { - p.tmStore = ObjectManager::instance()->get(objects::TM_STORE); - if (p.tmStore == nullptr) { + if (fp.tmStore == nullptr) { + fp.tmStore = ObjectManager::instance()->get(objects::TM_STORE); + if (fp.tmStore == nullptr) { return returnvalue::FAILED; } } - if (p.tcStore == nullptr) { - p.tcStore = ObjectManager::instance()->get(objects::TC_STORE); - if (p.tcStore == nullptr) { + if (fp.tcStore == nullptr) { + fp.tcStore = ObjectManager::instance()->get(objects::TC_STORE); + if (fp.tcStore == nullptr) { return returnvalue::FAILED; } } return returnvalue::OK; } +ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PacketInfo& info) { + // Process metadata 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; + } + cfdp::FileSize fileSize; + cfdp::StringLv sourceFileName; + cfdp::StringLv destFileName; + MetadataInfo metadataInfo(fileSize, sourceFileName, destFileName); + cfdp::Tlv* tlvArrayAsPtr = tlvVec.data(); + metadataInfo.setOptionsArray(&tlvArrayAsPtr, std::nullopt, tlvVec.size()); + MetadataPduReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), + metadataInfo); + ReturnValue_t result = reader.parseData(); + // TODO: The standard does not really specify what happens if this kind of error happens + // I think it might be a good idea to cache some sort of error code, which + // is translated into a warning and/or event by an upper layer + if (result != OK) { + return handleMetadataParseError(constAccessorPair.second.data(), + constAccessorPair.second.size()); + } + return result; +} + +ReturnValue_t cfdp::DestHandler::handleMetadataParseError(const uint8_t* rawData, size_t maxSize) { + // TODO: try to extract destination ID for error + // TODO: Invalid metadata PDU. +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "Parsing Metadata PDU failed with code " << result << std::endl; +#else +#endif + HeaderReader headerReader(rawData, maxSize); + ReturnValue_t result = headerReader.parseData(); + if (result != OK) { + // TODO: Now this really should not happen. Warning or error, + // yield or cache appropriate returnvalue +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "Parsing Header failed" << std::endl; +#else +#endif + // TODO: Trigger appropriate event + return result; + } + cfdp::EntityId destId; + headerReader.getDestId(destId); + RemoteEntityCfg* remoteCfg; + if (not dp.remoteCfgTable.getRemoteCfg(destId, &remoteCfg)) { +// TODO: No remote config for dest ID. I consider this a configuration error. +// Warning or error, yield or cache appropriate returnvalue +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "No remote config exists for destination ID" << std::endl; +#else +#endif + // TODO: Trigger appropriate event + } + // TODO: Appropriate returnvalue? + return returnvalue::FAILED; +} diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index c3e30721..d979f09b 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -28,28 +28,34 @@ struct PacketInfo { struct DestHandlerParams { DestHandlerParams(LocalEntityCfg cfg, UserBase& user, RemoteConfigTableIF& remoteCfgTable, - AcceptsTelemetryIF& packetDest, MessageQueueIF& msgQueue, - etl::ilist& packetList) + etl::ilist& packetList, uint8_t maxTlvsInOnePdu) : cfg(std::move(cfg)), user(user), remoteCfgTable(remoteCfgTable), - packetDest(packetDest), - msgQueue(msgQueue), packetListRef(packetList) {} LocalEntityCfg cfg; UserBase& user; RemoteConfigTableIF& remoteCfgTable; + + etl::ilist& packetListRef; + uint8_t maxTlvsInOnePdu; +}; + +struct FsfwParams { + FsfwParams(AcceptsTelemetryIF& packetDest, MessageQueueIF& msgQueue, + EventReportingProxyIF& eventReporter) + : packetDest(packetDest), msgQueue(msgQueue), eventReporter(eventReporter) {} AcceptsTelemetryIF& packetDest; MessageQueueIF& msgQueue; + EventReportingProxyIF& eventReporter; StorageManagerIF* tcStore = nullptr; StorageManagerIF* tmStore = nullptr; - etl::ilist& packetListRef; }; class DestHandler { public: - explicit DestHandler(DestHandlerParams params); + explicit DestHandler(DestHandlerParams handlerParams, FsfwParams fsfwParams); ReturnValue_t performStateMachine(); @@ -57,8 +63,12 @@ class DestHandler { ReturnValue_t initialize(); + ReturnValue_t handleMetadataPdu(const PacketInfo& info); + ReturnValue_t handleMetadataParseError(const uint8_t* rawData, size_t maxSize); + private: - DestHandlerParams p; + DestHandlerParams dp; + FsfwParams fp; enum class TransactionStep { IDLE = 0, TRANSACTION_START = 1, @@ -68,6 +78,7 @@ class DestHandler { SENDING_FINISHED_PDU = 5 }; TransactionStep step = TransactionStep::IDLE; + std::vector tlvVec; }; } // namespace cfdp diff --git a/src/fsfw/cfdp/pdu/MetadataInfo.cpp b/src/fsfw/cfdp/pdu/MetadataInfo.cpp index daa03473..d88bdd87 100644 --- a/src/fsfw/cfdp/pdu/MetadataInfo.cpp +++ b/src/fsfw/cfdp/pdu/MetadataInfo.cpp @@ -1,22 +1,25 @@ #include "MetadataInfo.h" MetadataInfo::MetadataInfo(bool closureRequested, cfdp::ChecksumType checksumType, - cfdp::FileSize& fileSize, cfdp::Lv& sourceFileName, - cfdp::Lv& destFileName) - : closureRequested(closureRequested), - checksumType(checksumType), - fileSize(fileSize), - sourceFileName(sourceFileName), - destFileName(destFileName) {} + cfdp::FileSize& fileSize, cfdp::StringLv& sourceFileName, + cfdp::StringLv& destFileName) + : MetadataInfo(fileSize, sourceFileName, destFileName) { + this->closureRequested = closureRequested; + this->checksumType = checksumType; +} -void MetadataInfo::setOptionsArray(cfdp::Tlv** optionsArray_, const size_t* optionsLen_, - const size_t* maxOptionsLen_) { +MetadataInfo::MetadataInfo(cfdp::FileSize& fileSize, cfdp::StringLv& sourceFileName, + cfdp::StringLv& destFileName) + : fileSize(fileSize), sourceFileName(sourceFileName), destFileName(destFileName) {} + +void MetadataInfo::setOptionsArray(cfdp::Tlv** optionsArray_, std::optional optionsLen_, + std::optional maxOptionsLen_) { this->optionsArray = optionsArray_; - if (maxOptionsLen_ != nullptr) { - this->maxOptionsLen = *maxOptionsLen_; + if (maxOptionsLen_) { + this->maxOptionsLen = maxOptionsLen_.value(); } - if (optionsLen_ != nullptr) { - this->optionsLen = *optionsLen_; + if (optionsLen_) { + this->optionsLen = optionsLen_.value(); } } @@ -32,7 +35,7 @@ void MetadataInfo::setClosureRequested(bool closureRequested_) { closureRequested = closureRequested_; } -cfdp::Lv& MetadataInfo::getDestFileName() { return destFileName; } +cfdp::StringLv& MetadataInfo::getDestFileName() { return destFileName; } cfdp::FileSize& MetadataInfo::getFileSize() { return fileSize; } @@ -81,9 +84,11 @@ size_t MetadataInfo::getSerializedSize(bool fssLarge) { return size; } -void MetadataInfo::setDestFileName(cfdp::Lv& destFileName_) { this->destFileName = destFileName_; } +void MetadataInfo::setDestFileName(cfdp::StringLv& destFileName_) { + this->destFileName = destFileName_; +} -void MetadataInfo::setSourceFileName(cfdp::Lv& sourceFileName_) { +void MetadataInfo::setSourceFileName(cfdp::StringLv& sourceFileName_) { this->sourceFileName = sourceFileName_; } @@ -95,4 +100,4 @@ size_t MetadataInfo::getOptionsLen() const { return optionsLen; } void MetadataInfo::setOptionsLen(size_t optionsLen_) { this->optionsLen = optionsLen_; } -cfdp::Lv& MetadataInfo::getSourceFileName() { return sourceFileName; } +cfdp::StringLv& MetadataInfo::getSourceFileName() { return sourceFileName; } diff --git a/src/fsfw/cfdp/pdu/MetadataInfo.h b/src/fsfw/cfdp/pdu/MetadataInfo.h index f71eb2f4..95f4544a 100644 --- a/src/fsfw/cfdp/pdu/MetadataInfo.h +++ b/src/fsfw/cfdp/pdu/MetadataInfo.h @@ -1,30 +1,35 @@ #ifndef FSFW_SRC_FSFW_CFDP_PDU_METADATAINFO_H_ #define FSFW_SRC_FSFW_CFDP_PDU_METADATAINFO_H_ +#include + #include "fsfw/cfdp/FileSize.h" #include "fsfw/cfdp/definitions.h" #include "fsfw/cfdp/tlv/Lv.h" +#include "fsfw/cfdp/tlv/StringLv.h" #include "fsfw/cfdp/tlv/Tlv.h" class MetadataInfo { public: + MetadataInfo(cfdp::FileSize& fileSize, cfdp::StringLv& sourceFileName, + cfdp::StringLv& destFileName); MetadataInfo(bool closureRequested, cfdp::ChecksumType checksumType, cfdp::FileSize& fileSize, - cfdp::Lv& sourceFileName, cfdp::Lv& destFileName); + cfdp::StringLv& sourceFileName, cfdp::StringLv& destFileName); size_t getSerializedSize(bool fssLarge = false); - void setOptionsArray(cfdp::Tlv** optionsArray, const size_t* optionsLen, - const size_t* maxOptionsLen); + void setOptionsArray(cfdp::Tlv** optionsArray, std::optional optionsLen, + std::optional maxOptionsLen); [[nodiscard]] cfdp::ChecksumType getChecksumType() const; void setChecksumType(cfdp::ChecksumType checksumType); [[nodiscard]] bool isClosureRequested() const; void setClosureRequested(bool closureRequested = false); - void setDestFileName(cfdp::Lv& destFileName); - void setSourceFileName(cfdp::Lv& sourceFileName); + void setDestFileName(cfdp::StringLv& destFileName); + void setSourceFileName(cfdp::StringLv& sourceFileName); - cfdp::Lv& getDestFileName(); - cfdp::Lv& getSourceFileName(); + cfdp::StringLv& getDestFileName(); + cfdp::StringLv& getSourceFileName(); cfdp::FileSize& getFileSize(); [[nodiscard]] bool hasOptions() const; @@ -37,10 +42,10 @@ class MetadataInfo { private: bool closureRequested = false; - cfdp::ChecksumType checksumType; + cfdp::ChecksumType checksumType = cfdp::ChecksumType::NULL_CHECKSUM; cfdp::FileSize& fileSize; - cfdp::Lv& sourceFileName; - cfdp::Lv& destFileName; + cfdp::StringLv& sourceFileName; + cfdp::StringLv& destFileName; cfdp::Tlv** optionsArray = nullptr; size_t optionsLen = 0; diff --git a/src/fsfw/events/EventReportingProxyIF.h b/src/fsfw/events/EventReportingProxyIF.h index c6034c9f..51f8ed1d 100644 --- a/src/fsfw/events/EventReportingProxyIF.h +++ b/src/fsfw/events/EventReportingProxyIF.h @@ -5,7 +5,7 @@ class EventReportingProxyIF { public: - virtual ~EventReportingProxyIF() {} + virtual ~EventReportingProxyIF() = default; virtual void forwardEvent(Event event, uint32_t parameter1 = 0, uint32_t parameter2 = 0) const = 0; diff --git a/unittests/cfdp/pdu/testMetadataPdu.cpp b/unittests/cfdp/pdu/testMetadataPdu.cpp index cfe24fc6..49f1deb4 100644 --- a/unittests/cfdp/pdu/testMetadataPdu.cpp +++ b/unittests/cfdp/pdu/testMetadataPdu.cpp @@ -21,7 +21,7 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { std::string firstFileName = "hello.txt"; cfdp::StringLv sourceFileName(firstFileName); - cfdp::Lv destFileName; + cfdp::StringLv destFileName; FileSize fileSize(35); MetadataInfo info(false, ChecksumType::MODULAR, fileSize, sourceFileName, destFileName); @@ -67,11 +67,10 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { REQUIRE(mdBuffer[26] == 0); std::string otherFileName = "hello2.txt"; - cfdp::Lv otherFileNameLv(reinterpret_cast(otherFileName.data()), - otherFileName.size()); + cfdp::StringLv otherFileNameLv(otherFileName.data(), otherFileName.size()); info.setSourceFileName(otherFileNameLv); size_t sizeOfOptions = options.size(); - info.setOptionsArray(options.data(), &sizeOfOptions, &sizeOfOptions); + info.setOptionsArray(options.data(), sizeOfOptions, sizeOfOptions); REQUIRE(info.getMaxOptionsLen() == 2); info.setMaxOptionsLen(3); REQUIRE(info.getMaxOptionsLen() == 3); @@ -129,7 +128,7 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { } size_t sizeOfOptions = options.size(); size_t maxSize = 4; - info.setOptionsArray(options.data(), &sizeOfOptions, &maxSize); + info.setOptionsArray(options.data(), sizeOfOptions, maxSize); REQUIRE(info.getOptionsLen() == 2); info.setChecksumType(cfdp::ChecksumType::CRC_32C); info.setClosureRequested(true); @@ -166,9 +165,9 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { } mdBuffer[1] = (36 >> 8) & 0xff; mdBuffer[2] = 36 & 0xff; - info.setOptionsArray(nullptr, nullptr, nullptr); + info.setOptionsArray(nullptr, std::nullopt, std::nullopt); REQUIRE(deserializer2.parseData() == cfdp::METADATA_CANT_PARSE_OPTIONS); - info.setOptionsArray(options.data(), &sizeOfOptions, nullptr); + info.setOptionsArray(options.data(), sizeOfOptions, std::nullopt); for (size_t maxSz = 0; maxSz < 46; maxSz++) { MetadataPduReader invalidSzDeser(mdBuffer.data(), maxSz, info); if (not invalidSzDeser.isNull()) {