From 2e4cdb73661426a6909cd95c1b731ddb64379681 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 5 Sep 2022 17:42:56 +0200 Subject: [PATCH] additional filesystem abstractions --- src/fsfw/cfdp/handler/DestHandler.cpp | 62 +++++++++++----------- src/fsfw/cfdp/handler/DestHandler.h | 34 +++++++----- src/fsfw/filesystem/FileSystemArgsIF.h | 2 +- src/fsfw/filesystem/HasFileSystemIF.h | 9 ++++ src/fsfw_hal/host/HostFilesystem.cpp | 14 +++++ src/fsfw_hal/host/HostFilesystem.h | 3 ++ unittests/cfdp/handler/testDestHandler.cpp | 4 +- unittests/mocks/FilesystemMock.cpp | 14 +++++ unittests/mocks/FilesystemMock.h | 6 ++- 9 files changed, 102 insertions(+), 46 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 5142731d..23852272 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -23,12 +23,11 @@ cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) tp.pduConf.direction = cfdp::Direction::TOWARDS_SENDER; } -const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { +const cfdp::DestHandler::FsmResult& cfdp::DestHandler::performStateMachine() { ReturnValue_t result; uint8_t errorIdx = 0; - fsmRes.callStatus = CallStatus::CALL_AFTER_DELAY; - fsmRes.result = OK; - if (step == TransactionStep::IDLE) { + fsmRes.resetOfIteration(); + if (fsmRes.step == TransactionStep::IDLE) { for (auto infoIter = dp.packetListRef.begin(); infoIter != dp.packetListRef.end();) { if (infoIter->pduType == PduType::FILE_DIRECTIVE and infoIter->directiveType == FileDirectives::METADATA) { @@ -42,7 +41,7 @@ const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { } infoIter++; } - if (step == TransactionStep::IDLE) { + if (fsmRes.step == TransactionStep::IDLE) { // To decrease the already high complexity of the software, all packets arriving before // a metadata PDU are deleted. for (auto infoIter = dp.packetListRef.begin(); infoIter != dp.packetListRef.end();) { @@ -52,13 +51,13 @@ const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { dp.packetListRef.clear(); } - if (step != TransactionStep::IDLE) { + if (fsmRes.step != TransactionStep::IDLE) { fsmRes.callStatus = CallStatus::CALL_AGAIN; } return updateFsmRes(errorIdx); } - if (cfdpState == CfdpStates::BUSY_CLASS_1_NACKED) { - if (step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { + if (fsmRes.state == CfdpStates::BUSY_CLASS_1_NACKED) { + if (fsmRes.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); @@ -83,14 +82,14 @@ const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { infoIter++; } } - if (step == TransactionStep::TRANSFER_COMPLETION) { + if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) { result = handleTransferCompletion(); if (result != OK and errorIdx < 3) { fsmRes.errorCodes[errorIdx] = result; errorIdx++; } } - if (step == TransactionStep::SENDING_FINISHED_PDU) { + if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) { result = sendFinishedPdu(); if (result != OK and errorIdx < 3) { fsmRes.errorCodes[errorIdx] = result; @@ -100,7 +99,7 @@ const cfdp::DestFsmResult& cfdp::DestHandler::performStateMachine() { } return updateFsmRes(errorIdx); } - if (cfdpState == CfdpStates::BUSY_CLASS_2_ACKED) { + if (fsmRes.state == CfdpStates::BUSY_CLASS_2_ACKED) { // TODO: Will be implemented at a later stage #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "CFDP state machine for acknowledged mode not implemented yet" << std::endl; @@ -225,11 +224,11 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PacketInfo& info) { } tp.fileSize.setFileSize(fileSizeFromEof, std::nullopt); } - if (step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { - if (cfdpState == CfdpStates::BUSY_CLASS_1_NACKED) { - step = TransactionStep::TRANSFER_COMPLETION; - } else if (cfdpState == CfdpStates::BUSY_CLASS_2_ACKED) { - step = TransactionStep::SENDING_ACK_PDU; + if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { + if (fsmRes.state == CfdpStates::BUSY_CLASS_1_NACKED) { + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + } else if (fsmRes.state == CfdpStates::BUSY_CLASS_2_ACKED) { + fsmRes.step = TransactionStep::SENDING_ACK_PDU; } } return returnvalue::OK; @@ -272,15 +271,15 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, } ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, MetadataInfo& info) { - if (cfdpState != CfdpStates::IDLE) { + if (fsmRes.state != CfdpStates::IDLE) { // According to standard, discard metadata PDU if we are busy return returnvalue::OK; } - step = TransactionStep::TRANSACTION_START; + fsmRes.step = TransactionStep::TRANSACTION_START; if (reader.getTransmissionMode() == TransmissionModes::UNACKNOWLEDGED) { - cfdpState = CfdpStates::BUSY_CLASS_1_NACKED; + fsmRes.state = CfdpStates::BUSY_CLASS_1_NACKED; } else if (reader.getTransmissionMode() == TransmissionModes::ACKNOWLEDGED) { - cfdpState = CfdpStates::BUSY_CLASS_2_ACKED; + fsmRes.state = CfdpStates::BUSY_CLASS_2_ACKED; } tp.checksumType = info.getChecksumType(); tp.closureRequested = info.isClosureRequested(); @@ -313,7 +312,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met #endif return FAILED; } - step = TransactionStep::RECEIVING_FILE_DATA_PDUS; + fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; MetadataRecvdParams params(tp.transactionId, tp.pduConf.sourceId); params.fileSize = tp.fileSize.getSize(); params.destFileName = tp.destName.data(); @@ -324,7 +323,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met return OK; } -cfdp::CfdpStates cfdp::DestHandler::getCfdpState() const { return cfdpState; } +cfdp::CfdpStates cfdp::DestHandler::getCfdpState() const { return fsmRes.state; } ReturnValue_t cfdp::DestHandler::handleTransferCompletion() { ReturnValue_t result; @@ -339,14 +338,14 @@ ReturnValue_t cfdp::DestHandler::handleTransferCompletion() { result = noticeOfCompletion(); if (result != OK) { } - if (cfdpState == CfdpStates::BUSY_CLASS_1_NACKED) { + if (fsmRes.state == CfdpStates::BUSY_CLASS_1_NACKED) { if (tp.closureRequested) { - step = TransactionStep::SENDING_FINISHED_PDU; + fsmRes.step = TransactionStep::SENDING_FINISHED_PDU; } else { finish(); } - } else if (cfdpState == CfdpStates::BUSY_CLASS_2_ACKED) { - step = TransactionStep::SENDING_FINISHED_PDU; + } else if (fsmRes.state == CfdpStates::BUSY_CLASS_2_ACKED) { + fsmRes.step = TransactionStep::SENDING_FINISHED_PDU; } return OK; } @@ -354,8 +353,8 @@ ReturnValue_t cfdp::DestHandler::handleTransferCompletion() { void cfdp::DestHandler::finish() { tp.reset(); dp.packetListRef.clear(); - cfdpState = CfdpStates::IDLE; - step = TransactionStep::IDLE; + fsmRes.state = CfdpStates::IDLE; + fsmRes.step = TransactionStep::IDLE; } ReturnValue_t cfdp::DestHandler::checksumVerification() { @@ -430,12 +429,15 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { // TODO: Error handling and event, this is a non CFDP specific error (most likely store is full) return result; } + fsmRes.packetsSent++; return OK; } -cfdp::DestHandler::TransactionStep cfdp::DestHandler::getTransactionStep() const { return step; } +cfdp::DestHandler::TransactionStep cfdp::DestHandler::getTransactionStep() const { + return fsmRes.step; +} -const cfdp::DestFsmResult& cfdp::DestHandler::updateFsmRes(uint8_t errors) { +const cfdp::DestHandler::FsmResult& cfdp::DestHandler::updateFsmRes(uint8_t errors) { fsmRes.errors = errors; fsmRes.result = OK; if (fsmRes.errors > 0) { diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index 30806153..ba4bcbe3 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -72,14 +72,6 @@ struct FsfwParams { 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 { @@ -90,6 +82,24 @@ class DestHandler { TRANSFER_COMPLETION = 4, SENDING_FINISHED_PDU = 5 }; + + struct FsmResult { + public: + ReturnValue_t result = returnvalue::OK; + CallStatus callStatus = CallStatus::CALL_AFTER_DELAY; + TransactionStep step = TransactionStep::IDLE; + CfdpStates state = CfdpStates::IDLE; + uint32_t packetsSent = 0; + uint8_t errors = 0; + std::array errorCodes = {}; + void resetOfIteration() { + result = returnvalue::OK; + callStatus = CallStatus::CALL_AFTER_DELAY; + packetsSent = 0; + errors = 0; + errorCodes.fill(returnvalue::OK); + } + }; /** * Will be returned if it is advisable to call the state machine operation call again */ @@ -103,7 +113,7 @@ class DestHandler { * - @c returnvalue::OK State machine OK for this execution cycle * - @c CALL_FSM_AGAIN State machine should be called again. */ - const DestFsmResult& performStateMachine(); + const FsmResult& performStateMachine(); ReturnValue_t passPacket(PacketInfo packet); @@ -149,14 +159,12 @@ class DestHandler { RemoteEntityCfg* remoteCfg = nullptr; }; - TransactionStep step = TransactionStep::IDLE; - CfdpStates cfdpState = CfdpStates::IDLE; std::vector tlvVec; std::vector userTlvVec; DestHandlerParams dp; FsfwParams fp; TransactionParams tp; - DestFsmResult fsmRes; + FsmResult fsmRes; ReturnValue_t startTransaction(MetadataPduReader& reader, MetadataInfo& info); ReturnValue_t handleMetadataPdu(const PacketInfo& info); @@ -168,7 +176,7 @@ class DestHandler { ReturnValue_t sendFinishedPdu(); ReturnValue_t noticeOfCompletion(); ReturnValue_t checksumVerification(); - const DestFsmResult& updateFsmRes(uint8_t errors); + const FsmResult& updateFsmRes(uint8_t errors); void finish(); }; diff --git a/src/fsfw/filesystem/FileSystemArgsIF.h b/src/fsfw/filesystem/FileSystemArgsIF.h index f1617f62..656764fa 100644 --- a/src/fsfw/filesystem/FileSystemArgsIF.h +++ b/src/fsfw/filesystem/FileSystemArgsIF.h @@ -7,7 +7,7 @@ */ class FileSystemArgsIF { public: - virtual ~FileSystemArgsIF(){}; + virtual ~FileSystemArgsIF() = default; }; #endif /* FSFW_SRC_FSFW_MEMORY_FILESYSTEMARGS_H_ */ diff --git a/src/fsfw/filesystem/HasFileSystemIF.h b/src/fsfw/filesystem/HasFileSystemIF.h index f206d53c..ee8c5f91 100644 --- a/src/fsfw/filesystem/HasFileSystemIF.h +++ b/src/fsfw/filesystem/HasFileSystemIF.h @@ -73,6 +73,15 @@ class HasFileSystemIF { return MessageQueueIF::NO_QUEUE; } + virtual bool fileExists(FilesystemParams params) = 0; + + /** + * Truncate a file, deleting its contents and setting its size to 0 accordingly. + * @param params + * @return + */ + virtual ReturnValue_t truncateFile(FilesystemParams params) = 0; + /** * @brief Generic function to append to file. * @param fileOpInfo General information: File name, size to write, offset, additional arguments diff --git a/src/fsfw_hal/host/HostFilesystem.cpp b/src/fsfw_hal/host/HostFilesystem.cpp index 20678a8c..6dc099b1 100644 --- a/src/fsfw_hal/host/HostFilesystem.cpp +++ b/src/fsfw_hal/host/HostFilesystem.cpp @@ -144,3 +144,17 @@ ReturnValue_t HostFilesystem::rename(const char *oldPath_, const char *newPath_, } return returnvalue::OK; } + +bool HostFilesystem::fileExists(FilesystemParams params) { + path path(params.path); + return filesystem::exists(path); +} + +ReturnValue_t HostFilesystem::truncateFile(FilesystemParams params) { + path path(params.path); + if (not filesystem::exists(path)) { + return FILE_DOES_NOT_EXIST; + } + ofstream of(path); + return returnvalue::OK; +} diff --git a/src/fsfw_hal/host/HostFilesystem.h b/src/fsfw_hal/host/HostFilesystem.h index c35e0fda..7b865e2d 100644 --- a/src/fsfw_hal/host/HostFilesystem.h +++ b/src/fsfw_hal/host/HostFilesystem.h @@ -8,6 +8,9 @@ class HostFilesystem : public HasFileSystemIF { public: HostFilesystem(); + + bool fileExists(FilesystemParams params) override; + ReturnValue_t truncateFile(FilesystemParams params) override; ReturnValue_t writeToFile(FileOpParams params, const uint8_t *data) override; ReturnValue_t readFromFile(FileOpParams fileOpInfo, uint8_t **buffer, size_t &readSize, size_t maxSize) override; diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp index 1873a081..3fc8f472 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -56,7 +56,7 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { } SECTION("Empty File Transfer") { - const DestFsmResult& res = destHandler.performStateMachine(); + const DestHandler::FsmResult& res = destHandler.performStateMachine(); CHECK(res.result == OK); FileSize size(0); std::string srcNameString = "hello.txt"; @@ -76,6 +76,8 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { packetInfoList.push_back(packetInfo); destHandler.performStateMachine(); CHECK(res.result == OK); + CHECK(res.callStatus == CallStatus::CALL_AGAIN); + destHandler.performStateMachine(); } SECTION("Small File Transfer") {} diff --git a/unittests/mocks/FilesystemMock.cpp b/unittests/mocks/FilesystemMock.cpp index 3d18e3cb..bf0c3bf6 100644 --- a/unittests/mocks/FilesystemMock.cpp +++ b/unittests/mocks/FilesystemMock.cpp @@ -124,3 +124,17 @@ void FilesystemMock::reset() { std::queue empty; std::swap(renameQueue, empty); } + +bool FilesystemMock::fileExists(FilesystemParams params) { + std::string filename(params.path); + auto iter = fileMap.find(filename); + if (iter == fileMap.end()) { + return false; + } + return true; +} + +ReturnValue_t FilesystemMock::truncateFile(FilesystemParams params) { + truncateCalledOnFile = params.path; + return returnvalue::OK; +} diff --git a/unittests/mocks/FilesystemMock.h b/unittests/mocks/FilesystemMock.h index 19d4d38c..74221d70 100644 --- a/unittests/mocks/FilesystemMock.h +++ b/unittests/mocks/FilesystemMock.h @@ -53,8 +53,12 @@ class FilesystemMock : public HasFileSystemIF { std::string newName; }; std::queue renameQueue; - + std::string truncateCalledOnFile; ReturnValue_t feedFile(const std::string &filename, std::ifstream &file); + + bool fileExists(FilesystemParams params) override; + ReturnValue_t truncateFile(FilesystemParams params) override; + ReturnValue_t writeToFile(FileOpParams params, const uint8_t *data) override; ReturnValue_t readFromFile(FileOpParams params, uint8_t **buffer, size_t &readSize, size_t maxSize) override;