From 5a3f05fa7996a0261b7ccb6a6b47153dae949e19 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 5 Sep 2022 17:20:29 +0200 Subject: [PATCH] return struct ref instead of code --- src/fsfw/cfdp/handler/DestHandler.cpp | 62 +++++++++++++++------- src/fsfw/cfdp/handler/DestHandler.h | 20 +++++-- unittests/cfdp/handler/testDestHandler.cpp | 14 +++-- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 487ee817..5142731d 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -23,16 +23,19 @@ cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) tp.pduConf.direction = cfdp::Direction::TOWARDS_SENDER; } -ReturnValue_t cfdp::DestHandler::performStateMachine() { +const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { ReturnValue_t result; - ReturnValue_t status = returnvalue::OK; + uint8_t errorIdx = 0; + fsmRes.callStatus = CallStatus::CALL_AFTER_DELAY; + fsmRes.result = OK; if (step == TransactionStep::IDLE) { for (auto infoIter = dp.packetListRef.begin(); infoIter != dp.packetListRef.end();) { if (infoIter->pduType == PduType::FILE_DIRECTIVE and infoIter->directiveType == FileDirectives::METADATA) { result = handleMetadataPdu(*infoIter); - if (result != OK) { - status = result; + if (result != OK and errorIdx < 3) { + fsmRes.errorCodes[errorIdx] = result; + errorIdx++; } // Store data was deleted in PDU handler because a store guard is used dp.packetListRef.erase(infoIter++); @@ -50,17 +53,18 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() { } if (step != TransactionStep::IDLE) { - return CALL_FSM_AGAIN; + fsmRes.callStatus = CallStatus::CALL_AGAIN; } - return status; + return updateFsmRes(errorIdx); } if (cfdpState == CfdpStates::BUSY_CLASS_1_NACKED) { if (step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { for (auto infoIter = dp.packetListRef.begin(); infoIter != dp.packetListRef.end();) { if (infoIter->pduType == PduType::FILE_DATA) { result = handleFileDataPdu(*infoIter); - if (result != OK) { - status = result; + if (result != OK and errorIdx < 3) { + fsmRes.errorCodes[errorIdx] = result; + errorIdx++; } // Store data was deleted in PDU handler because a store guard is used dp.packetListRef.erase(infoIter++); @@ -69,8 +73,9 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() { if (infoIter->pduType == PduType::FILE_DIRECTIVE and infoIter->directiveType == FileDirectives::EOF_DIRECTIVE) { result = handleEofPdu(*infoIter); - if (result != OK) { - status = result; + if (result != OK and errorIdx < 3) { + fsmRes.errorCodes[errorIdx] = result; + errorIdx++; } // Store data was deleted in PDU handler because a store guard is used dp.packetListRef.erase(infoIter++); @@ -80,18 +85,20 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() { } if (step == TransactionStep::TRANSFER_COMPLETION) { result = handleTransferCompletion(); - if (result != OK) { - status = result; + if (result != OK and errorIdx < 3) { + fsmRes.errorCodes[errorIdx] = result; + errorIdx++; } } if (step == TransactionStep::SENDING_FINISHED_PDU) { result = sendFinishedPdu(); - if (result != OK) { - status = result; + if (result != OK and errorIdx < 3) { + fsmRes.errorCodes[errorIdx] = result; + errorIdx++; } finish(); } - return status; + return updateFsmRes(errorIdx); } if (cfdpState == CfdpStates::BUSY_CLASS_2_ACKED) { // TODO: Will be implemented at a later stage @@ -99,7 +106,7 @@ ReturnValue_t cfdp::DestHandler::performStateMachine() { sif::warning << "CFDP state machine for acknowledged mode not implemented yet" << std::endl; #endif } - return OK; + return updateFsmRes(errorIdx); } ReturnValue_t cfdp::DestHandler::passPacket(PacketInfo packet) { @@ -146,7 +153,7 @@ ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PacketInfo& info) { // 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(), + return handleMetadataParseError(result, constAccessorPair.second.data(), constAccessorPair.second.size()); } return startTransaction(reader, metadataInfo); @@ -228,7 +235,8 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PacketInfo& info) { return returnvalue::OK; } -ReturnValue_t cfdp::DestHandler::handleMetadataParseError(const uint8_t* rawData, size_t maxSize) { +ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, + 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 @@ -236,7 +244,7 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(const uint8_t* rawData #else #endif HeaderReader headerReader(rawData, maxSize); - ReturnValue_t result = headerReader.parseData(); + result = headerReader.parseData(); if (result != OK) { // TODO: Now this really should not happen. Warning or error, // yield or cache appropriate returnvalue @@ -296,8 +304,13 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met tp.pduConf.direction = Direction::TOWARDS_SENDER; tp.transactionId.entityId = tp.pduConf.sourceId; tp.transactionId.seqNum = tp.pduConf.seqNum; - if (not dp.remoteCfgTable.getRemoteCfg(tp.pduConf.destId, &tp.remoteCfg)) { + if (not dp.remoteCfgTable.getRemoteCfg(tp.pduConf.sourceId, &tp.remoteCfg)) { // TODO: Warning, event etc. +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler" << __func__ + << ": No remote configuration found for destination ID " + << tp.pduConf.destId.getValue() << std::endl; +#endif return FAILED; } step = TransactionStep::RECEIVING_FILE_DATA_PDUS; @@ -421,3 +434,12 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { } cfdp::DestHandler::TransactionStep cfdp::DestHandler::getTransactionStep() const { return step; } + +const cfdp::DestFsmResult& cfdp::DestHandler::updateFsmRes(uint8_t errors) { + fsmRes.errors = errors; + fsmRes.result = OK; + if (fsmRes.errors > 0) { + fsmRes.result = FAILED; + } + return fsmRes; +} diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index 1b4bed75..30806153 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -70,6 +70,16 @@ struct FsfwParams { StorageManagerIF* tmStore = nullptr; }; +enum class CallStatus { DONE, CALL_AFTER_DELAY, CALL_AGAIN }; + +struct DestFsmResult { + ReturnValue_t result = returnvalue::OK; + CallStatus callStatus = CallStatus::CALL_AFTER_DELAY; + uint32_t packetsSent = 0; + uint8_t errors = 0; + std::array errorCodes = {}; +}; + class DestHandler { public: enum class TransactionStep { @@ -83,7 +93,8 @@ class DestHandler { /** * Will be returned if it is advisable to call the state machine operation call again */ - ReturnValue_t CALL_FSM_AGAIN = returnvalue::makeCode(1, 0); + ReturnValue_t PARTIAL_SUCCESS = returnvalue::makeCode(0, 2); + ReturnValue_t FAILURE = returnvalue::makeCode(0, 3); explicit DestHandler(DestHandlerParams handlerParams, FsfwParams fsfwParams); /** @@ -92,7 +103,7 @@ class DestHandler { * - @c returnvalue::OK State machine OK for this execution cycle * - @c CALL_FSM_AGAIN State machine should be called again. */ - ReturnValue_t performStateMachine(); + const DestFsmResult& performStateMachine(); ReturnValue_t passPacket(PacketInfo packet); @@ -145,16 +156,19 @@ class DestHandler { DestHandlerParams dp; FsfwParams fp; TransactionParams tp; + DestFsmResult fsmRes; 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); + ReturnValue_t handleMetadataParseError(ReturnValue_t result, const uint8_t* rawData, + size_t maxSize); ReturnValue_t handleTransferCompletion(); ReturnValue_t sendFinishedPdu(); ReturnValue_t noticeOfCompletion(); ReturnValue_t checksumVerification(); + const DestFsmResult& updateFsmRes(uint8_t errors); void finish(); }; diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp index 79491bf9..1873a081 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -33,6 +33,9 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { StorageManagerMock tcStore(2, storeCfg); StorageManagerMock tmStore(3, storeCfg); FsfwParams fp(tmReceiver, mqMock, eventReporterMock); + RemoteEntityCfg cfg; + cfg.remoteId = remoteId; + remoteCfgTableMock.addRemoteConfig(cfg); fp.tcStore = &tcStore; fp.tmStore = &tmStore; auto destHandler = DestHandler(dp, fp); @@ -44,13 +47,17 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { } SECTION("Idle State Machine Iteration") { - CHECK(destHandler.performStateMachine() == OK); + auto res = destHandler.performStateMachine(); + CHECK(res.result == OK); + CHECK(res.callStatus == CallStatus::CALL_AFTER_DELAY); + CHECK(res.errors == 0); CHECK(destHandler.getCfdpState() == CfdpStates::IDLE); CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); } SECTION("Empty File Transfer") { - CHECK(destHandler.performStateMachine() == OK); + const DestFsmResult& res = destHandler.performStateMachine(); + CHECK(res.result == OK); FileSize size(0); std::string srcNameString = "hello.txt"; std::string destNameString = "hello-cpy.txt"; @@ -67,7 +74,8 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { CHECK(metadataPdu.serialize(ptr, serLen, metadataPdu.getSerializedSize()) == OK); PacketInfo packetInfo(metadataPdu.getPduType(), metadataPdu.getDirectiveCode(), storeId); packetInfoList.push_back(packetInfo); - //CHECK(destHandler.performStateMachine() == OK); + destHandler.performStateMachine(); + CHECK(res.result == OK); } SECTION("Small File Transfer") {}