From 0fc57447199b029ad665cc9f4732c3a33b8526fc Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 24 May 2024 14:29:42 +0200 Subject: [PATCH] Filesystem and CFDP updates --- .clang-format | 1 + .gitignore | 2 +- .idea/codeStyles/Project.xml | 14 - .idea/codeStyles/codeStyleConfig.xml | 5 - CMakeLists.txt | 7 +- src/fsfw/cfdp/CfdpDistributor.h | 2 +- src/fsfw/cfdp/definitions.h | 1 + src/fsfw/cfdp/handler/DestHandler.cpp | 250 +++++++----------- src/fsfw/cfdp/handler/DestHandler.h | 66 +++-- src/fsfw/cfdp/handler/PduPacketIF.h | 18 ++ src/fsfw/cfdp/handler/PduSenderIF.h | 17 ++ src/fsfw/cfdp/handler/SourceHandler.cpp | 144 +++++----- src/fsfw/cfdp/handler/SourceHandler.h | 14 +- src/fsfw/cfdp/handler/defs.h | 36 ++- src/fsfw/cfdp/pdu/AckPduCreator.cpp | 8 + src/fsfw/cfdp/pdu/EofPduCreator.cpp | 6 + src/fsfw/cfdp/pdu/FileDataCreator.cpp | 8 + src/fsfw/cfdp/pdu/FileDataReader.cpp | 6 + src/fsfw/cfdp/pdu/FileDataReader.h | 2 +- src/fsfw/cfdp/pdu/FileDirectiveReader.cpp | 6 + src/fsfw/cfdp/pdu/FileDirectiveReader.h | 2 +- src/fsfw/cfdp/pdu/FinishedPduCreator.cpp | 7 + src/fsfw/cfdp/pdu/HeaderReader.cpp | 24 +- .../pdu/{PduHeaderReader.h => HeaderReader.h} | 5 + src/fsfw/cfdp/pdu/KeepAlivePduCreator.cpp | 13 +- src/fsfw/cfdp/pdu/MetadataPduCreator.cpp | 8 + src/fsfw/cfdp/pdu/MetadataPduReader.cpp | 4 + src/fsfw/cfdp/pdu/NakPduCreator.cpp | 7 + src/fsfw/cfdp/pdu/PromptPduCreator.cpp | 8 + src/fsfw/cfdp/tlv/FilestoreTlvBase.h | 4 +- src/fsfw/cfdp/tlv/TlvIF.h | 2 +- src/fsfw/coordinates/Jgm3Model.h | 12 +- src/fsfw/datapool/PoolDataSetIF.h | 2 +- src/fsfw/datapool/SharedDataSetIF.h | 2 +- src/fsfw/datapoollocal/AccessLocalPoolF.h | 2 +- src/fsfw/datapoollocal/HasLocalDataPoolIF.h | 2 +- src/fsfw/datapoollocal/LocalDataPoolManager.h | 2 +- .../datapoollocal/LocalPoolDataSetBase.cpp | 7 +- .../datapoollocal/LocalPoolObjectBase.cpp | 4 +- src/fsfw/datapoollocal/LocalPoolVariable.h | 2 +- src/fsfw/datapoollocal/LocalPoolVector.h | 2 +- src/fsfw/datapoollocal/MarkChangedIF.h | 2 +- src/fsfw/devicehandlers/CookieIF.h | 2 +- .../DeviceHandlerFailureIsolation.cpp | 4 +- src/fsfw/fdir/FailureIsolationBase.cpp | 3 +- src/fsfw/filesystem/HasFileSystemIF.h | 74 +++--- src/fsfw/housekeeping/HousekeepingSnapshot.h | 8 +- src/fsfw/ipc/CommandMessageIF.h | 2 +- src/fsfw/ipc/definitions.h | 2 +- src/fsfw/objectmanager/ObjectManagerIF.h | 2 +- src/fsfw/osal/common/TcpTmTcBridge.cpp | 4 +- src/fsfw/osal/common/TcpTmTcServer.cpp | 4 +- src/fsfw/pus/Service20ParameterManagement.cpp | 4 +- src/fsfw/pus/Service3Housekeeping.cpp | 4 +- src/fsfw/rmap/RMAPChannelIF.h | 2 +- src/fsfw/storagemanager/LocalPool.cpp | 3 +- src/fsfw/subsystem/Subsystem.cpp | 7 +- src/fsfw/subsystem/Subsystem.h | 4 +- src/fsfw/tasks/ExecutableObjectIF.h | 2 +- src/fsfw/tasks/FixedSlotSequence.cpp | 3 +- src/fsfw/tasks/SemaphoreIF.h | 2 +- src/fsfw/tcdistribution/PusDistributor.cpp | 20 +- src/fsfw/tcdistribution/PusDistributor.h | 5 +- src/fsfw/tcdistribution/definitions.h | 2 +- src/fsfw/timemanager/CCSDSTime.h | 4 +- .../tmtcpacket/ccsds/SpacePacketCreator.cpp | 5 +- src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h | 2 +- src/fsfw/tmtcservices/TmTcBridge.cpp | 23 +- src/fsfw/tmtcservices/TmTcBridge.h | 2 +- src/fsfw_hal/common/gpio/GpioIF.h | 2 +- src/fsfw_hal/common/gpio/gpioDefinitions.h | 2 +- src/fsfw_hal/host/HostFilesystem.cpp | 84 +++--- src/fsfw_hal/host/HostFilesystem.h | 26 +- src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp | 4 +- src/fsfw_hal/linux/i2c/I2cComIF.cpp | 7 +- src/fsfw_hal/linux/spi/SpiComIF.cpp | 10 +- src/fsfw_hal/stm32h7/definitions.h | 4 +- src/fsfw_hal/stm32h7/spi/mspInit.h | 6 +- unittests/cfdp/CMakeLists.txt | 5 +- unittests/cfdp/PduSenderMock.cpp | 1 + unittests/cfdp/PduSenderMock.h | 36 +++ unittests/cfdp/handler/OwnedPduPacket.h | 31 +++ unittests/cfdp/handler/testDestHandler.cpp | 136 ++++------ .../cfdp/handler/testReservedMsgParser.cpp | 1 + unittests/cfdp/handler/testSourceHandler.cpp | 108 ++++---- unittests/cfdp/pdu/testCfdpHeader.cpp | 2 +- unittests/container/TestArrayList.cpp | 4 +- .../container/TestFixedOrderedMultimap.cpp | 6 +- unittests/hal/testFsMock.cpp | 8 +- unittests/hal/testHostFilesystem.cpp | 90 +++---- unittests/mocks/DeviceHandlerMock.cpp | 13 +- unittests/mocks/DeviceHandlerMock.h | 2 + unittests/mocks/FilesystemMock.cpp | 117 ++++---- unittests/mocks/FilesystemMock.h | 44 +-- unittests/tmtcpacket/testCcsdsCreator.cpp | 8 +- 95 files changed, 892 insertions(+), 800 deletions(-) delete mode 100644 .idea/codeStyles/Project.xml delete mode 100644 .idea/codeStyles/codeStyleConfig.xml create mode 100644 src/fsfw/cfdp/handler/PduPacketIF.h create mode 100644 src/fsfw/cfdp/handler/PduSenderIF.h rename src/fsfw/cfdp/pdu/{PduHeaderReader.h => HeaderReader.h} (96%) create mode 100644 unittests/cfdp/PduSenderMock.cpp create mode 100644 unittests/cfdp/PduSenderMock.h create mode 100644 unittests/cfdp/handler/OwnedPduPacket.h diff --git a/.clang-format b/.clang-format index ec53b72b..ada61da2 100644 --- a/.clang-format +++ b/.clang-format @@ -4,4 +4,5 @@ IndentWidth: 2 --- Language: Cpp ColumnLimit: 100 +ReflowComments: true --- diff --git a/.gitignore b/.gitignore index a5a8d2ba..deefd53d 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,7 @@ .idea/* !/.idea/runConfigurations !/.idea/cmake.xml -!/.idea/codeStyles +# !/.idea/codeStyles # Eclipse .cproject diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml deleted file mode 100644 index 0f3b1a4b..00000000 --- a/.idea/codeStyles/Project.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - - \ No newline at end of file diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml deleted file mode 100644 index 79ee123c..00000000 --- a/.idea/codeStyles/codeStyleConfig.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a7319cd..0478cb66 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -140,7 +140,7 @@ option(FSFW_ADD_SGP4_PROPAGATOR "Add SGP4 propagator code" OFF) set(FSFW_TEST_TGT fsfw-tests) set(FSFW_DUMMY_TGT fsfw-dummy) -add_library(${LIB_FSFW_NAME}) +add_library(${LIB_FSFW_NAME} src/fsfw/cfdp/handler/PduPacketIF.h) if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) set_property(TARGET ${LIB_FSFW_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION @@ -175,7 +175,10 @@ if(FSFW_BUILD_TESTS) configure_file(unittests/testcfg/TestsConfig.h.in tests/TestsConfig.h) project(${FSFW_TEST_TGT} CXX C) - add_executable(${FSFW_TEST_TGT}) + add_executable( + ${FSFW_TEST_TGT} + unittests/cfdp/PduSenderMock.cpp unittests/cfdp/PduSenderMock.h + unittests/cfdp/handler/OwnedPduPacket.h) if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) set_property(TARGET ${FSFW_TEST_TGT} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) diff --git a/src/fsfw/cfdp/CfdpDistributor.h b/src/fsfw/cfdp/CfdpDistributor.h index 13d000eb..72ca34a0 100644 --- a/src/fsfw/cfdp/CfdpDistributor.h +++ b/src/fsfw/cfdp/CfdpDistributor.h @@ -4,7 +4,7 @@ #include #include -#include "fsfw/cfdp/pdu/PduHeaderReader.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/returnvalues/returnvalue.h" #include "fsfw/tcdistribution/CfdpPacketChecker.h" #include "fsfw/tcdistribution/TcDistributorBase.h" diff --git a/src/fsfw/cfdp/definitions.h b/src/fsfw/cfdp/definitions.h index d19d8532..adab1d0f 100644 --- a/src/fsfw/cfdp/definitions.h +++ b/src/fsfw/cfdp/definitions.h @@ -38,6 +38,7 @@ static constexpr ReturnValue_t FILESTORE_REQUIRES_SECOND_FILE = static constexpr ReturnValue_t FILESTORE_RESPONSE_CANT_PARSE_FS_MESSAGE = returnvalue::makeCode(CFDP_CLASS_ID, 9); static constexpr ReturnValue_t INVALID_PDU_FORMAT = returnvalue::makeCode(CFDP_CLASS_ID, 10); +static constexpr ReturnValue_t PDU_CHECKSUM_FAILURE = returnvalue::makeCode(CFDP_CLASS_ID, 11); //! Checksum types according to the SANA Checksum Types registry //! https://sanaregistry.org/r/checksum_identifiers/ diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index d9c8793d..0112fed8 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -8,14 +8,18 @@ #include "fsfw/cfdp/pdu/EofPduReader.h" #include "fsfw/cfdp/pdu/FileDataReader.h" #include "fsfw/cfdp/pdu/FinishedPduCreator.h" -#include "fsfw/cfdp/pdu/PduHeaderReader.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/objectmanager.h" #include "fsfw/tmtcservices/TmTcMessage.h" using namespace returnvalue; -cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) - : tlvVec(params.maxTlvsInOnePdu), +cfdp::DestHandler::DestHandler(PduSenderIF& pduSender, size_t pduBufSize, DestHandlerParams params, + FsfwParams fsfwParams) + + : pduBuf(pduBufSize), + pduSender(pduSender), + tlvVec(params.maxTlvsInOnePdu), msgToUserVec(params.maxTlvsInOnePdu), transactionParams(params.maxFilenameLen), destParams(std::move(params)), @@ -23,69 +27,55 @@ cfdp::DestHandler::DestHandler(DestHandlerParams params, FsfwParams fsfwParams) transactionParams.pduConf.direction = cfdp::Direction::TOWARDS_SENDER; } -const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine() { +const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachineNoPacket() { + return stateMachine(std::nullopt); +} +const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine( + const std::optional> optPduPacket) { ReturnValue_t result; uint8_t errorIdx = 0; fsmRes.resetOfIteration(); if (fsmRes.step == TransactionStep::IDLE) { - for (auto infoIter = destParams.packetListRef.begin(); - infoIter != destParams.packetListRef.end();) { - if (infoIter->pduType == PduType::FILE_DIRECTIVE and - infoIter->directiveType == FileDirective::METADATA) { - result = handleMetadataPdu(*infoIter); - checkAndHandleError(result, errorIdx); - // Store data was deleted in PDU handler because a store guard is used - destParams.packetListRef.erase(infoIter++); - } else { - infoIter++; - } + if (!optPduPacket.has_value()) { + return fsmRes; } - 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 = destParams.packetListRef.begin(); - infoIter != destParams.packetListRef.end();) { - fsfwParams.tcStore->deleteData(infoIter->storeId); - infoIter++; - } - destParams.packetListRef.clear(); - } - - if (fsmRes.step != TransactionStep::IDLE) { - fsmRes.callStatus = CallStatus::CALL_AGAIN; + PduPacketIF& pduPacket = *optPduPacket; + if (pduPacket.getPduType() == FILE_DATA or + (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA)) { + fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU; + return fsmRes; } + result = handleMetadataPdu(pduPacket); + checkAndHandleError(result, errorIdx); return updateFsmRes(errorIdx); } + if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) { if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { - for (auto infoIter = destParams.packetListRef.begin(); - infoIter != destParams.packetListRef.end();) { - if (infoIter->pduType == PduType::FILE_DATA) { - result = handleFileDataPdu(*infoIter); - checkAndHandleError(result, errorIdx); - // Store data was deleted in PDU handler because a store guard is used - destParams.packetListRef.erase(infoIter++); - } else if (infoIter->pduType == PduType::FILE_DIRECTIVE and - infoIter->directiveType == FileDirective::EOF_DIRECTIVE) { - // TODO: Support for check timer missing - result = handleEofPdu(*infoIter); - checkAndHandleError(result, errorIdx); - // Store data was deleted in PDU handler because a store guard is used - destParams.packetListRef.erase(infoIter++); - } else { - infoIter++; - } + if (!optPduPacket.has_value()) { + return fsmRes; + } + PduPacketIF& pduPacket = *optPduPacket; + if (pduPacket.getPduType() == FILE_DATA) { + result = handleFileDataPdu(pduPacket); + checkAndHandleError(result, errorIdx); + + } else if (pduPacket.getPduType() == FILE_DIRECTIVE and + *pduPacket.getFileDirective() == EOF_DIRECTIVE) { + // TODO: Support for check timer missing + result = handleEofPdu(pduPacket); + checkAndHandleError(result, errorIdx); } } - if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) { - result = handleTransferCompletion(); - checkAndHandleError(result, errorIdx); - } - if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) { - result = sendFinishedPdu(); - checkAndHandleError(result, errorIdx); - finish(); - } + } + if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) { + result = handleTransferCompletion(); + checkAndHandleError(result, errorIdx); + } + if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) { + result = sendFinishedPdu(); + checkAndHandleError(result, errorIdx); + finish(); return updateFsmRes(errorIdx); } if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) { @@ -97,75 +87,37 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine() { return updateFsmRes(errorIdx); } -ReturnValue_t cfdp::DestHandler::passPacket(PacketInfo packet) { - if (destParams.packetListRef.full()) { - return FAILED; - } - destParams.packetListRef.push_back(packet); - return OK; -} - -ReturnValue_t cfdp::DestHandler::initialize() { - if (fsfwParams.tmStore == nullptr) { - fsfwParams.tmStore = ObjectManager::instance()->get(objects::TM_STORE); - if (fsfwParams.tmStore == nullptr) { - return FAILED; - } - } - - if (fsfwParams.tcStore == nullptr) { - fsfwParams.tcStore = ObjectManager::instance()->get(objects::TC_STORE); - if (fsfwParams.tcStore == nullptr) { - return FAILED; - } - } - - if (fsfwParams.msgQueue == nullptr) { - return FAILED; - } - return OK; -} - -ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PacketInfo& info) { +ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PduPacketIF& pduPacket) { // Process metadata PDU - auto constAccessorPair = fsfwParams.tcStore->getData(info.storeId); - if (constAccessorPair.first != OK) { - // TODO: This is not a CFDP error. Event and/or warning? - return constAccessorPair.first; - } cfdp::StringLv sourceFileName; cfdp::StringLv destFileName; + MetadataGenericInfo metadataInfo(transactionParams.fileSize); - MetadataPduReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), - metadataInfo, tlvVec.data(), tlvVec.size()); + size_t pduSize = 0; + auto rawPdu = pduPacket.getRawPduData(pduSize); + MetadataPduReader reader(rawPdu, pduSize, metadataInfo, tlvVec.data(), tlvVec.size()); 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(result, constAccessorPair.second.data(), - constAccessorPair.second.size()); + return handleMetadataParseError(result, rawPdu, pduSize); } return startTransaction(reader); } -ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const cfdp::PacketInfo& info) { +ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) { + size_t pduSize = 0; + const auto rawPdu = info.getRawPduData(pduSize); // Process file data PDU - auto constAccessorPair = fsfwParams.tcStore->getData(info.storeId); - if (constAccessorPair.first != OK) { - // TODO: This is not a CFDP error. Event and/or warning? - return constAccessorPair.first; - } FileDataInfo fdInfo; - FileDataReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), fdInfo); + FileDataReader reader(rawPdu, pduSize, fdInfo); ReturnValue_t result = reader.parseData(); if (result != OK) { return result; } size_t fileSegmentLen = 0; const uint8_t* fileData = fdInfo.getFileData(&fileSegmentLen); - FileOpParams fileOpParams(transactionParams.destName.data(), fileSegmentLen); - fileOpParams.offset = fdInfo.getOffset().value(); if (destParams.cfg.indicCfg.fileSegmentRecvIndicRequired) { FileSegmentRecvdParams segParams; segParams.offset = fdInfo.getOffset().value(); @@ -177,7 +129,9 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const cfdp::PacketInfo& info) segParams.segmentMetadata = {segMetadata, segmentMetadatLen}; destParams.user.fileSegmentRecvdIndication(segParams); } - result = destParams.user.vfs.writeToFile(fileOpParams, fileData); + sif::printDebug("Writing %d bytes at offset %d\n", fileSegmentLen, fdInfo.getOffset().value()); + result = destParams.user.vfs.writeToFile(transactionParams.destName.data(), + fdInfo.getOffset().value(), fileData, fileSegmentLen); if (result != returnvalue::OK) { // TODO: Proper Error handling #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -191,25 +145,21 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const cfdp::PacketInfo& info) static_cast(fsmRes.step), result); } return result; - } else { - transactionParams.deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE; - transactionParams.vfsErrorCount = 0; } + transactionParams.deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE; + transactionParams.vfsErrorCount = 0; if (fdInfo.getOffset().value() + fileSegmentLen > transactionParams.progress) { transactionParams.progress = fdInfo.getOffset().value() + fileSegmentLen; } return result; } -ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PacketInfo& info) { +ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) { + size_t pduSize = 0; + const auto rawPdu = info.getRawPduData(pduSize); // Process EOF PDU - auto constAccessorPair = fsfwParams.tcStore->getData(info.storeId); - if (constAccessorPair.first != OK) { - // TODO: This is not a CFDP error. Event and/or warning? - return constAccessorPair.first; - } EofInfo eofInfo(nullptr); - EofPduReader reader(constAccessorPair.second.data(), constAccessorPair.second.size(), eofInfo); + EofPduReader reader(rawPdu, pduSize, eofInfo); ReturnValue_t result = reader.parseData(); if (result != OK) { return result; @@ -244,6 +194,7 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "Parsing Metadata PDU failed with code " << result << std::endl; #else + sif::printWarning("cfdp::DestHandler: parsing metadata PDU failed with code %04x\n", result); #endif PduHeaderReader headerReader(rawData, maxSize); result = headerReader.parseData(); @@ -307,24 +258,26 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade // so there is no need to create a file or truncate an existing file if (destNameSize > 0 and sourceNameSize > 0) { transactionParams.metadataOnly = false; - FilesystemParams fparams(transactionParams.destName.data()); // handling to allow only specifying target directory. Example: // Source path /test/hello.txt, dest path /tmp -> dest path /tmp/hello.txt - if (destParams.user.vfs.isDirectory(transactionParams.destName.data())) { + bool isDirectory = false; + + result = destParams.user.vfs.isDirectory(transactionParams.destName.data(), isDirectory); + if (result == returnvalue::OK && isDirectory) { result = tryBuildingAbsoluteDestName(destNameSize); if (result != OK) { return result; } } - if (destParams.user.vfs.fileExists(fparams)) { - result = destParams.user.vfs.truncateFile(fparams); + if (destParams.user.vfs.fileExists(transactionParams.destName.data())) { + result = destParams.user.vfs.truncateFile(transactionParams.destName.data()); if (result != returnvalue::OK) { fileErrorHandler(events::FILESTORE_ERROR, result, "file truncation error"); return FAILED; // TODO: Relevant for filestore rejection error? } } else { - result = destParams.user.vfs.createFile(fparams); + result = destParams.user.vfs.createFile(transactionParams.destName.data()); if (result != OK) { fileErrorHandler(events::FILESTORE_ERROR, result, "file creation error"); return FAILED; @@ -359,6 +312,7 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade transactionParams.checksumType = info.getChecksumType(); transactionParams.closureRequested = info.isClosureRequested(); reader.fillConfig(transactionParams.pduConf); + transactionParams.pduConf.crcFlag = transactionParams.remoteCfg->crcOnTransmission; transactionParams.pduConf.direction = Direction::TOWARDS_SENDER; transactionParams.transactionId.entityId = transactionParams.pduConf.sourceId; transactionParams.transactionId.seqNum = transactionParams.pduConf.seqNum; @@ -368,10 +322,10 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade params.destFileName = transactionParams.destName.data(); params.sourceFileName = transactionParams.sourceName.data(); params.numberOfMsgsToUser = 0; - for (const auto& opt : tlvVec) { - if (opt.getType() == TlvType::MSG_TO_USER) { + for (uint32_t tlvIdx = 0; tlvIdx < reader.getNumberOfParsedOptions(); tlvIdx++) { + if (tlvVec[tlvIdx].getType() == TlvType::MSG_TO_USER) { msgToUserVec[params.numberOfMsgsToUser] = - MessageToUserTlv(opt.getValue(), opt.getLengthField()); + MessageToUserTlv(tlvVec[tlvIdx].getValue(), tlvVec[tlvIdx].getLengthField()); params.numberOfMsgsToUser++; } } @@ -411,10 +365,9 @@ ReturnValue_t cfdp::DestHandler::tryBuildingAbsoluteDestName(size_t destNameSize // A path may only have a maximum of 256 characters in CFDP, so this buffer should be sufficient // for all use-cases. char baseNameBuf[512]{}; - FilesystemParams fparamsSrc(transactionParams.sourceName.data()); size_t baseNameLen = 0; - ReturnValue_t result = destParams.user.vfs.getBaseFilename(fparamsSrc, baseNameBuf, - sizeof(baseNameBuf), baseNameLen); + const ReturnValue_t result = destParams.user.vfs.getBaseFilename( + transactionParams.sourceName.data(), baseNameBuf, sizeof(baseNameBuf), baseNameLen); if (result != returnvalue::OK or baseNameLen == 0) { fileErrorHandler(events::FILENAME_TOO_LARGE_ERROR, 0, "error retrieving source base name"); return FAILED; @@ -442,7 +395,6 @@ void cfdp::DestHandler::fileErrorHandler(Event event, ReturnValue_t result, void cfdp::DestHandler::finish() { transactionParams.reset(); - destParams.packetListRef.clear(); fsmRes.state = CfdpState::IDLE; fsmRes.step = TransactionStep::IDLE; } @@ -451,26 +403,26 @@ ReturnValue_t cfdp::DestHandler::checksumVerification() { std::array buf{}; etl::crc32 crcCalc; uint64_t currentOffset = 0; - FileOpParams params(transactionParams.destName.data(), transactionParams.fileSize.value()); while (currentOffset < transactionParams.fileSize.value()) { - uint64_t readLen; + uint64_t lenToRead; if (currentOffset + buf.size() > transactionParams.fileSize.value()) { - readLen = transactionParams.fileSize.value() - currentOffset; + lenToRead = transactionParams.fileSize.value() - currentOffset; } else { - readLen = buf.size(); + lenToRead = buf.size(); } - if (readLen > 0) { - params.offset = currentOffset; - params.size = readLen; - auto result = destParams.user.vfs.readFromFile(params, buf.data(), buf.size()); + if (lenToRead > 0) { + size_t readLen = 0; + const auto result = + destParams.user.vfs.readFromFile(transactionParams.destName.data(), currentOffset, + lenToRead, buf.data(), readLen, buf.size()); if (result != OK) { // TODO: I think this is a case for a filestore rejection, but it might sense to print // a warning or trigger an event because this should generally not happen return FAILED; } - crcCalc.add(buf.begin(), buf.begin() + readLen); + crcCalc.add(buf.begin(), buf.begin() + lenToRead); } - currentOffset += readLen; + currentOffset += lenToRead; } uint32_t value = crcCalc.value(); @@ -502,19 +454,11 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { FinishedInfo info(transactionParams.conditionCode, transactionParams.deliveryCode, transactionParams.deliveryStatus); FinishPduCreator finishedPdu(transactionParams.pduConf, info); - store_address_t storeId; - uint8_t* dataPtr = nullptr; - ReturnValue_t result = - fsfwParams.tmStore->getFreeElement(&storeId, finishedPdu.getSerializedSize(), &dataPtr); - if (result != OK) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "cfdp::DestHandler:sendFinishedPdu: Getting store slot failed" << std::endl; -#endif - fsfwParams.eventReporter->forwardEvent(events::STORE_ERROR, result, 0); - return result; - } + size_t serLen = 0; - result = finishedPdu.serialize(dataPtr, serLen, finishedPdu.getSerializedSize()); + ReturnValue_t result = + finishedPdu.serialize(pduBuf.data(), serLen, finishedPdu.getSerializedSize()); + checkAndHandleError(result, fsmRes.errors); if (result != OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler::sendFinishedPdu: Serializing Finished PDU failed" @@ -523,13 +467,12 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0); return result; } - TmTcMessage msg(storeId); - result = fsfwParams.msgQueue->sendMessage(fsfwParams.packetDest.getReportReceptionQueue(), &msg); + pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen); if (result != OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler::sendFinishedPdu: Sending PDU failed" << std::endl; #endif - fsfwParams.eventReporter->forwardEvent(events::MSG_QUEUE_ERROR, result, 0); + fsfwParams.eventReporter->forwardEvent(events::PDU_SEND_ERROR, result, 0); return result; } fsmRes.packetsSent++; @@ -560,15 +503,10 @@ void cfdp::DestHandler::checkAndHandleError(ReturnValue_t result, uint8_t& error } } -void cfdp::DestHandler::setMsgQueue(MessageQueueIF& queue) { fsfwParams.msgQueue = &queue; } - void cfdp::DestHandler::setEventReporter(EventReportingProxyIF& reporter) { fsfwParams.eventReporter = &reporter; } const cfdp::DestHandlerParams& cfdp::DestHandler::getDestHandlerParams() const { return destParams; -} - -StorageManagerIF* cfdp::DestHandler::getTmStore() const { return fsfwParams.tmStore; } -StorageManagerIF* cfdp::DestHandler::getTcStore() const { return fsfwParams.tcStore; } +} \ No newline at end of file diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index 0eb7b5e8..4a19c6ca 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -11,6 +11,8 @@ #include "RemoteConfigTableIF.h" #include "UserBase.h" #include "defs.h" +#include "fsfw/cfdp/handler/PduPacketIF.h" +#include "fsfw/cfdp/handler/PduSenderIF.h" #include "fsfw/cfdp/handler/mib.h" #include "fsfw/cfdp/pdu/MetadataPduReader.h" #include "fsfw/cfdp/pdu/PduConfig.h" @@ -28,30 +30,37 @@ using LostSegmentsListBase = etl::iset>; struct DestHandlerParams { DestHandlerParams(LocalEntityCfg cfg, UserBase& user, RemoteConfigTableIF& remoteCfgTable, - PacketInfoListBase& packetList, - // TODO: This container can potentially take tons of space. For a better - // memory efficient implementation, an additional abstraction could be - // be used so users can use uint32_t as the pair type - // TODO: Actually, we can provide a better abstraction via interface, which - // allows using something like a bounded map. This simplifies - // the implementation significantly. + // PacketInfoListBase& packetList, + // TODO: This container can potentially take tons of space. For a better + // memory efficient implementation, an additional abstraction could be + // be used so users can use uint32_t as the pair type + // TODO: Actually, we can provide a better abstraction via interface, which + // allows using something like a bounded map. This simplifies + // the implementation significantly. LostSegmentsListBase& lostSegmentsContainer) : cfg(std::move(cfg)), user(user), remoteCfgTable(remoteCfgTable), - packetListRef(packetList), + // packetListRef(packetList), lostSegmentsContainer(lostSegmentsContainer) {} LocalEntityCfg cfg; UserBase& user; RemoteConfigTableIF& remoteCfgTable; - PacketInfoListBase& packetListRef; + // PacketInfoListBase& packetListRef; LostSegmentsListBase& lostSegmentsContainer; uint8_t maxTlvsInOnePdu = 20; size_t maxFilenameLen = 255; }; +struct PacketInfo { + const uint8_t* rawPdu = nullptr; + size_t pduSize = 0; + PduType pduType = PduType::FILE_DATA; + std::optional directiveType = FileDirective::INVALID_DIRECTIVE; +}; + class DestHandler { public: enum class TransactionStep : uint8_t { @@ -64,17 +73,14 @@ class DestHandler { }; struct FsmResult { - public: + uint32_t packetsSent = 0; ReturnValue_t result = returnvalue::OK; - CallStatus callStatus = CallStatus::CALL_AFTER_DELAY; TransactionStep step = TransactionStep::IDLE; CfdpState state = CfdpState::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); @@ -85,28 +91,30 @@ class DestHandler { */ ReturnValue_t PARTIAL_SUCCESS = returnvalue::makeCode(0, 2); ReturnValue_t FAILURE = returnvalue::makeCode(0, 3); - explicit DestHandler(DestHandlerParams handlerParams, FsfwParams fsfwParams); + + explicit DestHandler(PduSenderIF& pduSender, size_t pduBufSize, DestHandlerParams handlerParams, + FsfwParams fsfwParams); /** + * This is the core function to drive the destination handler. It is also used to insert + * packets into the destination handler. * - * @return - * - @c returnvalue::OK State machine OK for this execution cycle - * - @c CALL_FSM_AGAIN State machine should be called again. + * The state machine should either be called if packets with the appropriate destination ID + * are received and periodically to perform all CFDP related tasks, for example + * checking for timeouts or missed file segments. + * + * The function returns the number of sent PDU packets on success. */ - const FsmResult& stateMachine(); - void setMsgQueue(MessageQueueIF& queue); + const FsmResult& stateMachine(std::optional> optPduPacket); + + const FsmResult& stateMachineNoPacket(); + void setEventReporter(EventReportingProxyIF& reporter); - ReturnValue_t passPacket(PacketInfo packet); - - ReturnValue_t initialize(); - [[nodiscard]] CfdpState getCfdpState() const; [[nodiscard]] TransactionStep getTransactionStep() const; [[nodiscard]] const TransactionId& getTransactionId() const; [[nodiscard]] const DestHandlerParams& getDestHandlerParams() const; - [[nodiscard]] StorageManagerIF* getTcStore() const; - [[nodiscard]] StorageManagerIF* getTmStore() const; private: struct TransactionParams { @@ -149,6 +157,8 @@ class DestHandler { RemoteEntityCfg* remoteCfg = nullptr; }; + std::vector pduBuf; + PduSenderIF& pduSender; std::vector tlvVec; std::vector msgToUserVec; TransactionParams transactionParams; @@ -157,9 +167,9 @@ class DestHandler { FsmResult fsmRes; ReturnValue_t startTransaction(const MetadataPduReader& reader); - ReturnValue_t handleMetadataPdu(const PacketInfo& info); - ReturnValue_t handleFileDataPdu(const PacketInfo& info); - ReturnValue_t handleEofPdu(const PacketInfo& info); + ReturnValue_t handleMetadataPdu(const PduPacketIF& pduPacket); + ReturnValue_t handleFileDataPdu(const PduPacketIF& info); + ReturnValue_t handleEofPdu(const PduPacketIF& info); ReturnValue_t handleMetadataParseError(ReturnValue_t result, const uint8_t* rawData, size_t maxSize); ReturnValue_t handleTransferCompletion(); diff --git a/src/fsfw/cfdp/handler/PduPacketIF.h b/src/fsfw/cfdp/handler/PduPacketIF.h new file mode 100644 index 00000000..c5235dc3 --- /dev/null +++ b/src/fsfw/cfdp/handler/PduPacketIF.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +#include "fsfw/cfdp/definitions.h" + +namespace cfdp { + +class PduPacketIF { + public: + virtual ~PduPacketIF() = default; + + [[nodiscard]] virtual cfdp::PduType getPduType() const = 0; + [[nodiscard]] virtual std::optional getFileDirective() const = 0; + [[nodiscard]] virtual const uint8_t* getRawPduData(size_t& pduLen) const = 0; +}; + +} // namespace cfdp \ No newline at end of file diff --git a/src/fsfw/cfdp/handler/PduSenderIF.h b/src/fsfw/cfdp/handler/PduSenderIF.h new file mode 100644 index 00000000..44e0af69 --- /dev/null +++ b/src/fsfw/cfdp/handler/PduSenderIF.h @@ -0,0 +1,17 @@ +#pragma once + +#include + +#include "fsfw/cfdp/definitions.h" + +namespace cfdp { +class PduSenderIF { + public: + virtual ~PduSenderIF() = default; + + virtual ReturnValue_t sendPdu(cfdp::PduType pduType, + std::optional fileDirective, + const uint8_t* pdu, size_t pduSize) = 0; +}; + +} // namespace cfdp \ No newline at end of file diff --git a/src/fsfw/cfdp/handler/SourceHandler.cpp b/src/fsfw/cfdp/handler/SourceHandler.cpp index f0f89607..7481719c 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.cpp +++ b/src/fsfw/cfdp/handler/SourceHandler.cpp @@ -16,8 +16,12 @@ using namespace returnvalue; -cfdp::SourceHandler::SourceHandler(SourceHandlerParams params, FsfwParams fsfwParams) - : sourceParams(std::move(params)), fsfwParams(fsfwParams) { +cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize, + SourceHandlerParams params, FsfwParams fsfwParams) + : pduSender(pduSender), + pduBuf(pduBufferSize), + sourceParams(std::move(params)), + fsfwParams(fsfwParams) { // The entity ID portion of the transaction ID will always remain fixed. transactionParams.id.entityId = sourceParams.cfg.localId; transactionParams.pduConf.sourceId = sourceParams.cfg.localId; @@ -61,14 +65,14 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { if (result != OK) { addError(result); } - fsmResult.callStatus = CallStatus::CALL_AGAIN; + // fsmResult.callStatus = CallStatus::CALL_AGAIN; return fsmResult; } if (step == TransactionStep::SENDING_FILE_DATA) { bool noFdPdu = false; result = prepareAndSendNextFileDataPdu(noFdPdu); if (result == OK and !noFdPdu) { - fsmResult.callStatus = CallStatus::CALL_AGAIN; + // fsmResult.callStatus = CallStatus::CALL_AGAIN; return fsmResult; } } @@ -82,10 +86,10 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { } if (transactionParams.closureRequested) { step = TransactionStep::WAIT_FOR_FINISH; - fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY; + // fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY; } else { step = TransactionStep::NOTICE_OF_COMPLETION; - fsmResult.callStatus = CallStatus::CALL_AGAIN; + // fsmResult.callStatus = CallStatus::CALL_AGAIN; } return fsmResult; } @@ -104,7 +108,7 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { const cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::stateMachine() { fsmResult.packetsSent = 0; fsmResult.errors = 0; - fsmResult.callStatus = CallStatus::DONE; + // fsmResult.callStatus = CallStatus::DONE; if (state == cfdp::CfdpState::IDLE) { return fsmResult; } @@ -123,25 +127,25 @@ ReturnValue_t cfdp::SourceHandler::checksumGeneration() { std::array buf{}; etl::crc32 crcCalc; uint64_t currentOffset = 0; - FileOpParams params(transactionParams.sourceName.data(), transactionParams.fileSize.value()); while (currentOffset < transactionParams.fileSize.value()) { - uint64_t readLen; + uint64_t lenToRead; if (currentOffset + buf.size() > transactionParams.fileSize.value()) { - readLen = transactionParams.fileSize.value() - currentOffset; + lenToRead = transactionParams.fileSize.value() - currentOffset; } else { - readLen = buf.size(); + lenToRead = buf.size(); } - if (readLen > 0) { - params.offset = currentOffset; - params.size = readLen; - auto result = sourceParams.user.vfs.readFromFile(params, buf.data(), buf.size()); + if (lenToRead > 0) { + size_t readLen = 0; + auto result = + sourceParams.user.vfs.readFromFile(transactionParams.sourceName.data(), currentOffset, + lenToRead, buf.data(), readLen, buf.size()); if (result != OK) { addError(result); return FAILED; } - crcCalc.add(buf.begin(), buf.begin() + readLen); + crcCalc.add(buf.begin(), buf.begin() + lenToRead); } - currentOffset += readLen; + currentOffset += lenToRead; } transactionParams.crc = crcCalc.value(); @@ -169,8 +173,7 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo // operation is safe. transactionParams.sourceName[transactionParams.sourceNameSize] = '\0'; transactionParams.destName[transactionParams.destNameSize] = '\0'; - FilesystemParams params(transactionParams.sourceName.data()); - if (!sourceParams.user.vfs.fileExists(params)) { + if (!sourceParams.user.vfs.fileExists(transactionParams.sourceName.data())) { return FILE_DOES_NOT_EXIST; } if (cfg.maxFileSegmentLen > fileBuf.size() or cfg.maxFileSegmentLen == 0) { @@ -206,7 +209,7 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo } step = TransactionStep::IDLE; uint64_t fileSize = 0; - sourceParams.user.vfs.getFileSize(params, fileSize); + sourceParams.user.vfs.getFileSize(transactionParams.sourceName.data(), fileSize); transactionParams.pduConf.largeFile = false; if (fileSize > UINT32_MAX) { transactionParams.pduConf.largeFile = true; @@ -230,7 +233,8 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() { transactionParams.fileSize); auto metadataPdu = MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0); - ReturnValue_t result = sendGenericPdu(metadataPdu); + ReturnValue_t result = + sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu); if (result != OK) { return result; } @@ -241,7 +245,7 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() { ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDataPdu) { cfdp::Fss offset(transactionParams.progress); - uint64_t readLen; + uint64_t lenToRead; uint64_t fileSize = transactionParams.fileSize.value(); noFileDataPdu = false; if (fileSize == 0) { @@ -251,29 +255,31 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDat return OK; } if (fileSize < transactionParams.remoteCfg.maxFileSegmentLen) { - readLen = transactionParams.fileSize.value(); + lenToRead = transactionParams.fileSize.value(); } else { if (transactionParams.progress + transactionParams.remoteCfg.maxFileSegmentLen > fileSize) { - readLen = fileSize - transactionParams.progress; + lenToRead = fileSize - transactionParams.progress; } else { - readLen = transactionParams.remoteCfg.maxFileSegmentLen; + lenToRead = transactionParams.remoteCfg.maxFileSegmentLen; } } - FileOpParams fileParams(transactionParams.sourceName.data(), readLen); + FileOpParams fileParams(transactionParams.sourceName.data(), lenToRead); fileParams.offset = transactionParams.progress; - ReturnValue_t result = - sourceParams.user.vfs.readFromFile(fileParams, fileBuf.data(), fileBuf.size()); + size_t readLen = 0; + ReturnValue_t result = sourceParams.user.vfs.readFromFile( + transactionParams.sourceName.data(), transactionParams.progress, lenToRead, fileBuf.data(), + readLen, fileBuf.size()); if (result != returnvalue::OK) { addError(result); return result; } - auto fileDataInfo = FileDataInfo(offset, fileBuf.data(), readLen); + auto fileDataInfo = FileDataInfo(offset, fileBuf.data(), lenToRead); auto fileDataPdu = FileDataCreator(transactionParams.pduConf, fileDataInfo); - result = sendGenericPdu(fileDataPdu); + result = sendGenericPdu(PduType::FILE_DATA, std::nullopt, fileDataPdu); if (result != OK) { return result; } - transactionParams.progress += readLen; + transactionParams.progress += lenToRead; if (transactionParams.progress >= fileSize) { // Advance FSM after all file data PDUs were sent. step = TransactionStep::SENDING_EOF; @@ -285,72 +291,44 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendEofPdu() { auto eofInfo = EofInfo(ConditionCode::NO_ERROR, transactionParams.crc, transactionParams.fileSize); auto eofPdu = EofPduCreator(transactionParams.pduConf, eofInfo); - ReturnValue_t result = sendGenericPdu(eofPdu); - if (result != OK) { - return result; - } - return OK; -} - -ReturnValue_t cfdp::SourceHandler::initialize() { - if (fsfwParams.tmStore == nullptr) { - fsfwParams.tmStore = ObjectManager::instance()->get(objects::TM_STORE); - if (fsfwParams.tmStore == nullptr) { - return FAILED; - } - } - - if (fsfwParams.tcStore == nullptr) { - fsfwParams.tcStore = ObjectManager::instance()->get(objects::TC_STORE); - if (fsfwParams.tcStore == nullptr) { - return FAILED; - } - } - - if (fsfwParams.msgQueue == nullptr) { - return FAILED; - } - return OK; -} - -ReturnValue_t cfdp::SourceHandler::sendGenericPdu(const SerializeIF& pdu) { - uint8_t* dataPtr; - store_address_t storeId; ReturnValue_t result = - fsfwParams.tmStore->getFreeElement(&storeId, pdu.getSerializedSize(), &dataPtr); + sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::EOF_DIRECTIVE, eofPdu); if (result != OK) { - addError(result); - fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY; - if (result == StorageManagerIF::DATA_STORAGE_FULL) { - return TM_STORE_FULL; - } return result; } + return OK; +} + +ReturnValue_t cfdp::SourceHandler::initialize() { return OK; } + +ReturnValue_t cfdp::SourceHandler::sendGenericPdu(PduType pduType, + std::optional fileDirective, + const SerializeIF& pdu) { + store_address_t storeId; + if (pdu.getSerializedSize() > pduBuf.size()) { + addError(PDU_BUFFER_TOO_SMALL); + return PDU_BUFFER_TOO_SMALL; + } size_t serializedLen = 0; - result = pdu.serializeBe(dataPtr, serializedLen, pdu.getSerializedSize()); + ReturnValue_t result = pdu.serializeBe(pduBuf.data(), serializedLen, pdu.getSerializedSize()); if (result != OK) { addError(result); return result; } - TmTcMessage tmMsg(storeId); - result = - fsfwParams.msgQueue->sendMessage(fsfwParams.packetDest.getReportReceptionQueue(), &tmMsg); - if (result != OK) { - fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY; - } - if (result == MessageQueueIF::FULL) { - return TARGET_MSG_QUEUE_FULL; - } else if (result == OK) { - fsmResult.packetsSent += 1; - } + pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen); + fsmResult.packetsSent += 1; return result; } ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() { if (sourceParams.cfg.indicCfg.transactionFinishedIndicRequired) { + // TODO: This could still be improved by caching the Finished PDU parameters. + FileDeliveryStatus deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED; + if (transactionParams.closureRequested) { + deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE; + } cfdp::TransactionFinishedParams params(transactionParams.id, ConditionCode::NO_ERROR, - FileDeliveryCode::DATA_COMPLETE, - FileDeliveryStatus::RETAINED_IN_FILESTORE); + FileDeliveryCode::DATA_COMPLETE, deliveryStatus); sourceParams.user.transactionFinishedIndication(params); } return OK; @@ -359,7 +337,7 @@ ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() { ReturnValue_t cfdp::SourceHandler::reset() { step = TransactionStep::IDLE; state = cfdp::CfdpState::IDLE; - fsmResult.callStatus = CallStatus::DONE; + // fsmResult.callStatus = CallStatus::DONE; transactionParams.reset(); return OK; } diff --git a/src/fsfw/cfdp/handler/SourceHandler.h b/src/fsfw/cfdp/handler/SourceHandler.h index d1d3fab2..96f5eb89 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.h +++ b/src/fsfw/cfdp/handler/SourceHandler.h @@ -4,6 +4,7 @@ #include #include +#include "PduSenderIF.h" #include "UserBase.h" #include "defs.h" #include "fsfw/cfdp/Fss.h" @@ -40,14 +41,15 @@ class SourceHandler { struct FsmResult { public: ReturnValue_t result = returnvalue::OK; - CallStatus callStatus = CallStatus::CALL_AFTER_DELAY; - CfdpState state = CfdpState::IDLE; uint32_t packetsSent = 0; + // CallStatus callStatus = CallStatus::CALL_AFTER_DELAY; + CfdpState state = CfdpState::IDLE; uint8_t errors = 0; std::array errorCodes = {}; }; - SourceHandler(SourceHandlerParams params, FsfwParams fsfwParams); + SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize, SourceHandlerParams params, + FsfwParams fsfwParams); [[nodiscard]] CfdpState getState() const; [[nodiscard]] TransactionStep getStep() const; @@ -86,6 +88,8 @@ class SourceHandler { } } transactionParams; + PduSenderIF& pduSender; + std::vector pduBuf; cfdp::CfdpState state = cfdp::CfdpState::IDLE; TransactionStep step = TransactionStep::IDLE; std::array fileBuf{}; @@ -101,7 +105,9 @@ class SourceHandler { ReturnValue_t noticeOfCompletion(); ReturnValue_t reset(); - [[nodiscard]] ReturnValue_t sendGenericPdu(const SerializeIF& pdu); + [[nodiscard]] ReturnValue_t sendGenericPdu(PduType pduType, + std::optional fileDirective, + const SerializeIF& pdu); void addError(ReturnValue_t error); }; diff --git a/src/fsfw/cfdp/handler/defs.h b/src/fsfw/cfdp/handler/defs.h index 4356835d..257ce8b2 100644 --- a/src/fsfw/cfdp/handler/defs.h +++ b/src/fsfw/cfdp/handler/defs.h @@ -14,6 +14,7 @@ enum class CfdpState { IDLE, BUSY_CLASS_1_NACKED, BUSY_CLASS_2_ACKED, SUSPENDED static constexpr uint8_t SSID = SUBSYSTEM_ID::CFDP; static constexpr uint8_t CID = CLASS_ID::CFDP_HANDLER; +/* struct PacketInfo { PacketInfo(PduType type, store_address_t storeId, std::optional directive = std::nullopt) @@ -24,36 +25,27 @@ struct PacketInfo { store_address_t storeId = store_address_t::invalid(); PacketInfo() = default; }; +*/ struct FsfwParams { - FsfwParams(AcceptsTelemetryIF& packetDest, MessageQueueIF* msgQueue, - EventReportingProxyIF* eventReporter, StorageManagerIF& tcStore, - StorageManagerIF& tmStore) - : FsfwParams(packetDest, msgQueue, eventReporter) { - this->tcStore = &tcStore; - this->tmStore = &tmStore; - } - - FsfwParams(AcceptsTelemetryIF& packetDest, MessageQueueIF* msgQueue, - EventReportingProxyIF* eventReporter) - : packetDest(packetDest), msgQueue(msgQueue), eventReporter(eventReporter) {} - AcceptsTelemetryIF& packetDest; - MessageQueueIF* msgQueue; + FsfwParams(EventReportingProxyIF* eventReporter) : eventReporter(eventReporter) {} + // AcceptsTelemetryIF& packetDest; + // uint8_t virtualChannelId; + // MessageQueueIF* msgQueue; EventReportingProxyIF* eventReporter = nullptr; - StorageManagerIF* tcStore = nullptr; - StorageManagerIF* tmStore = nullptr; + // StorageManagerIF* tcStore = nullptr; + // StorageManagerIF* tmStore = nullptr; }; -template -using PacketInfoList = etl::list; -using PacketInfoListBase = etl::ilist; +// template +// using PacketInfoList = etl::list; +// using PacketInfoListBase = etl::ilist; -enum class CallStatus { DONE, CALL_AFTER_DELAY, CALL_AGAIN }; +// enum class CallStatus { DONE, CALL_AFTER_DELAY, CALL_AGAIN }; namespace events { -static constexpr Event STORE_ERROR = event::makeEvent(SSID, 0, severity::LOW); -static constexpr Event MSG_QUEUE_ERROR = event::makeEvent(SSID, 1, severity::LOW); +static constexpr Event PDU_SEND_ERROR = event::makeEvent(SSID, 1, severity::LOW); static constexpr Event SERIALIZATION_ERROR = event::makeEvent(SSID, 2, severity::LOW); static constexpr Event FILESTORE_ERROR = event::makeEvent(SSID, 3, severity::LOW); //! [EXPORT] : [COMMENT] P1: Transaction step ID, P2: 0 for source file name, 1 for dest file name @@ -71,6 +63,8 @@ static constexpr ReturnValue_t DEST_NAME_EMPTY = returnvalue::makeCode(CID, 4); static constexpr ReturnValue_t WRONG_REMOTE_CFG_ENTITY_ID = returnvalue::makeCode(CID, 5); static constexpr ReturnValue_t TARGET_MSG_QUEUE_FULL = returnvalue::makeCode(CID, 6); static constexpr ReturnValue_t TM_STORE_FULL = returnvalue::makeCode(CID, 7); +static constexpr ReturnValue_t DEST_NON_METADATA_PDU_AS_FIRST_PDU = returnvalue::makeCode(CID, 8); +static constexpr ReturnValue_t PDU_BUFFER_TOO_SMALL = returnvalue::makeCode(CID, 9); } // namespace cfdp #endif // FSFW_CFDP_HANDLER_DEFS_H diff --git a/src/fsfw/cfdp/pdu/AckPduCreator.cpp b/src/fsfw/cfdp/pdu/AckPduCreator.cpp index f7623e57..3db17ecb 100644 --- a/src/fsfw/cfdp/pdu/AckPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/AckPduCreator.cpp @@ -1,5 +1,7 @@ #include "AckPduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + AckPduCreator::AckPduCreator(AckInfo &ackInfo, PduConfig &pduConf) : FileDirectiveCreator(pduConf, cfdp::FileDirective::ACK, 2), ackInfo(ackInfo) {} @@ -7,6 +9,7 @@ size_t AckPduCreator::getSerializedSize() const { return FileDirectiveCreator::g ReturnValue_t AckPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -29,5 +32,10 @@ ReturnValue_t AckPduCreator::serialize(uint8_t **buffer, size_t *size, size_t ma **buffer = ackedConditionCode << 4 | transactionStatus; *buffer += 1; *size += 1; + + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return returnvalue::OK; } diff --git a/src/fsfw/cfdp/pdu/EofPduCreator.cpp b/src/fsfw/cfdp/pdu/EofPduCreator.cpp index 12a3e696..0349699d 100644 --- a/src/fsfw/cfdp/pdu/EofPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/EofPduCreator.cpp @@ -1,6 +1,7 @@ #include "EofPduCreator.h" #include "fsfw/FSFW.h" +#include "fsfw/globalfunctions/CRC.h" EofPduCreator::EofPduCreator(PduConfig &conf, EofInfo &info) : FileDirectiveCreator(conf, cfdp::FileDirective::EOF_DIRECTIVE, 9), info(info) { @@ -11,6 +12,7 @@ size_t EofPduCreator::getSerializedSize() const { return FileDirectiveCreator::g ReturnValue_t EofPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -36,5 +38,9 @@ ReturnValue_t EofPduCreator::serialize(uint8_t **buffer, size_t *size, size_t ma if (info.getFaultLoc() != nullptr and info.getConditionCode() != cfdp::ConditionCode::NO_ERROR) { result = info.getFaultLoc()->serialize(buffer, size, maxSize, streamEndianness); } + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return result; } diff --git a/src/fsfw/cfdp/pdu/FileDataCreator.cpp b/src/fsfw/cfdp/pdu/FileDataCreator.cpp index 70779a51..f59d88f9 100644 --- a/src/fsfw/cfdp/pdu/FileDataCreator.cpp +++ b/src/fsfw/cfdp/pdu/FileDataCreator.cpp @@ -2,6 +2,8 @@ #include +#include "fsfw/globalfunctions/CRC.h" + FileDataCreator::FileDataCreator(PduConfig& conf, FileDataInfo& info) : HeaderCreator(conf, cfdp::PduType::FILE_DATA, 0, info.getSegmentMetadataFlag()), info(info) { update(); @@ -18,6 +20,7 @@ ReturnValue_t FileDataCreator::serialize(uint8_t** buffer, size_t* size, size_t if (buffer == nullptr or size == nullptr) { return returnvalue::FAILED; } + const uint8_t* start = *buffer; if (*size + getSerializedSize() > maxSize) { return SerializeIF::BUFFER_TOO_SHORT; } @@ -50,6 +53,11 @@ ReturnValue_t FileDataCreator::serialize(uint8_t** buffer, size_t* size, size_t std::memcpy(*buffer, readOnlyPtr, fileSize); *buffer += fileSize; *size += fileSize; + + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return returnvalue::OK; } diff --git a/src/fsfw/cfdp/pdu/FileDataReader.cpp b/src/fsfw/cfdp/pdu/FileDataReader.cpp index 9155ef28..9ecedca0 100644 --- a/src/fsfw/cfdp/pdu/FileDataReader.cpp +++ b/src/fsfw/cfdp/pdu/FileDataReader.cpp @@ -8,6 +8,9 @@ ReturnValue_t FileDataReader::parseData() { if (result != returnvalue::OK) { return result; } + if (not performCrcCheckIfApplicable()) { + return cfdp::PDU_CHECKSUM_FAILURE; + } size_t currentIdx = PduHeaderReader::getHeaderSize(); const uint8_t* buf = pointers.rawPtr + currentIdx; size_t remSize = PduHeaderReader::getWholePduSize() - currentIdx; @@ -34,6 +37,9 @@ ReturnValue_t FileDataReader::parseData() { if (result != returnvalue::OK) { return result; } + if (getCrcFlag()) { + remSize -= 2; + } if (remSize > 0) { info.setFileData(buf, remSize); } diff --git a/src/fsfw/cfdp/pdu/FileDataReader.h b/src/fsfw/cfdp/pdu/FileDataReader.h index aa962e2b..6fb4dafd 100644 --- a/src/fsfw/cfdp/pdu/FileDataReader.h +++ b/src/fsfw/cfdp/pdu/FileDataReader.h @@ -3,7 +3,7 @@ #include "../definitions.h" #include "FileDataInfo.h" -#include "PduHeaderReader.h" +#include "HeaderReader.h" class FileDataReader : public PduHeaderReader { public: diff --git a/src/fsfw/cfdp/pdu/FileDirectiveReader.cpp b/src/fsfw/cfdp/pdu/FileDirectiveReader.cpp index f5fc6e58..33f1ace3 100644 --- a/src/fsfw/cfdp/pdu/FileDirectiveReader.cpp +++ b/src/fsfw/cfdp/pdu/FileDirectiveReader.cpp @@ -1,5 +1,7 @@ #include "FileDirectiveReader.h" +#include "fsfw/globalfunctions/CRC.h" + FileDirectiveReader::FileDirectiveReader(const uint8_t *pduBuf, size_t maxSize) : PduHeaderReader(pduBuf, maxSize) {} @@ -10,6 +12,10 @@ ReturnValue_t FileDirectiveReader::parseData() { if (result != returnvalue::OK) { return result; } + if (not performCrcCheckIfApplicable()) { + return cfdp::PDU_CHECKSUM_FAILURE; + } + if (this->getPduDataFieldLen() < 1) { return cfdp::INVALID_PDU_DATAFIELD_LEN; } diff --git a/src/fsfw/cfdp/pdu/FileDirectiveReader.h b/src/fsfw/cfdp/pdu/FileDirectiveReader.h index 9e88e006..3415b6d1 100644 --- a/src/fsfw/cfdp/pdu/FileDirectiveReader.h +++ b/src/fsfw/cfdp/pdu/FileDirectiveReader.h @@ -2,7 +2,7 @@ #define FSFW_SRC_FSFW_CFDP_PDU_FILEDIRECTIVEDESERIALIZER_H_ #include "../definitions.h" -#include "fsfw/cfdp/pdu/PduHeaderReader.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" /** * @brief This class is used to deserialize a PDU file directive header from raw memory. diff --git a/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp b/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp index d002e8aa..b2e22856 100644 --- a/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp @@ -1,5 +1,7 @@ #include "FinishedPduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + FinishPduCreator::FinishPduCreator(PduConfig &conf, FinishedInfo &finishInfo) : FileDirectiveCreator(conf, cfdp::FileDirective::FINISH, 0), finishInfo(finishInfo) { updateDirectiveFieldLen(); @@ -13,6 +15,7 @@ void FinishPduCreator::updateDirectiveFieldLen() { ReturnValue_t FinishPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -40,5 +43,9 @@ ReturnValue_t FinishPduCreator::serialize(uint8_t **buffer, size_t *size, size_t if (finishInfo.getFaultLocation(&entityId) == returnvalue::OK) { result = entityId->serialize(buffer, size, maxSize, streamEndianness); } + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return result; } diff --git a/src/fsfw/cfdp/pdu/HeaderReader.cpp b/src/fsfw/cfdp/pdu/HeaderReader.cpp index 4d853920..4e1bcafd 100644 --- a/src/fsfw/cfdp/pdu/HeaderReader.cpp +++ b/src/fsfw/cfdp/pdu/HeaderReader.cpp @@ -1,8 +1,10 @@ +#include "HeaderReader.h" + #include #include -#include "PduHeaderReader.h" +#include "fsfw/globalfunctions/CRC.h" PduHeaderReader::PduHeaderReader(const uint8_t *pduBuf, size_t maxSize) { setReadOnlyData(pduBuf, maxSize); @@ -17,28 +19,29 @@ ReturnValue_t PduHeaderReader::parseData() { } pointers.fixedHeader = reinterpret_cast(const_cast(pointers.rawPtr)); - sourceIdRaw = static_cast(&pointers.fixedHeader->variableFieldsStart); + sourceIdRaw = &pointers.fixedHeader->variableFieldsStart; cfdp::WidthInBytes widthEntityIds = getLenEntityIds(); cfdp::WidthInBytes widthSeqNum = getLenSeqNum(); seqNumRaw = static_cast(sourceIdRaw) + static_cast(widthEntityIds); destIdRaw = static_cast(seqNumRaw) + static_cast(widthSeqNum); if (getWholePduSize() > PduHeaderReader::getHeaderSize()) { - pointers.dataFieldStart = reinterpret_cast(destIdRaw) + widthEntityIds; + pointers.dataFieldStart = static_cast(destIdRaw) + widthEntityIds; } else { pointers.dataFieldStart = nullptr; } + return returnvalue::OK; } -ReturnValue_t PduHeaderReader::setData(uint8_t *dataPtr, size_t maxSize_, void *args) { +ReturnValue_t PduHeaderReader::setData(uint8_t *dataPtr, size_t maxSize, void *args) { if (dataPtr == nullptr) { return returnvalue::FAILED; } - if (maxSize_ < 7) { + if (maxSize < 7) { return SerializeIF::STREAM_TOO_SHORT; } pointers.rawPtr = dataPtr; - maxSize = maxSize_; + this->maxSize = maxSize; return returnvalue::OK; } @@ -166,3 +169,12 @@ void PduHeaderReader::fillConfig(PduConfig &cfg) const { } const uint8_t *PduHeaderReader::getPduDataField() const { return pointers.dataFieldStart; } + +bool PduHeaderReader::performCrcCheckIfApplicable() const { + if (getCrcFlag()) { + if (CRC::crc16ccitt(pointers.rawPtr, getWholePduSize()) != 0) { + return false; + } + } + return true; +} diff --git a/src/fsfw/cfdp/pdu/PduHeaderReader.h b/src/fsfw/cfdp/pdu/HeaderReader.h similarity index 96% rename from src/fsfw/cfdp/pdu/PduHeaderReader.h rename to src/fsfw/cfdp/pdu/HeaderReader.h index d910e0e3..66dd972c 100644 --- a/src/fsfw/cfdp/pdu/PduHeaderReader.h +++ b/src/fsfw/cfdp/pdu/HeaderReader.h @@ -71,6 +71,11 @@ class PduHeaderReader : public RedirectableDataPointerIF, public PduHeaderIF { void getDestId(cfdp::EntityId& destId) const override; void getTransactionSeqNum(cfdp::TransactionSeqNum& seqNum) const override; + /** + * Performs the CRC check on the whole PDU if the CRC flag is set. + */ + [[nodiscard]] bool performCrcCheckIfApplicable() const; + ReturnValue_t deserResult = returnvalue::OK; [[nodiscard]] size_t getMaxSize() const; diff --git a/src/fsfw/cfdp/pdu/KeepAlivePduCreator.cpp b/src/fsfw/cfdp/pdu/KeepAlivePduCreator.cpp index 2afcd427..7c79480d 100644 --- a/src/fsfw/cfdp/pdu/KeepAlivePduCreator.cpp +++ b/src/fsfw/cfdp/pdu/KeepAlivePduCreator.cpp @@ -1,5 +1,7 @@ #include "KeepAlivePduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + KeepAlivePduCreator::KeepAlivePduCreator(PduConfig &conf, cfdp::Fss &progress) : FileDirectiveCreator(conf, cfdp::FileDirective::KEEP_ALIVE, 4), progress(progress) { updateDirectiveFieldLen(); @@ -17,9 +19,18 @@ void KeepAlivePduCreator::updateDirectiveFieldLen() { ReturnValue_t KeepAlivePduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; } - return progress.serialize(this->getLargeFileFlag(), buffer, size, maxSize, streamEndianness); + result = progress.serialize(this->getLargeFileFlag(), buffer, size, maxSize, streamEndianness); + if (result != returnvalue::OK) { + return result; + } + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } + return result; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp b/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp index 9f8633cb..6947f0f8 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/MetadataPduCreator.cpp @@ -1,5 +1,7 @@ #include "MetadataPduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + MetadataPduCreator::MetadataPduCreator(PduConfig &conf, MetadataGenericInfo &info, cfdp::StringLv &srcFileName, cfdp::StringLv &destFileName, cfdp::Tlv **optionsArray, size_t optionsLen) @@ -29,6 +31,7 @@ size_t MetadataPduCreator::getSerializedSize() const { ReturnValue_t MetadataPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -60,6 +63,11 @@ ReturnValue_t MetadataPduCreator::serialize(uint8_t **buffer, size_t *size, size } } } + + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return result; } const cfdp::StringLv &MetadataPduCreator::getSourceFileName() const { return srcFileName; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp index e6160ce8..aaf1e08b 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp +++ b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp @@ -38,6 +38,10 @@ ReturnValue_t MetadataPduReader::parseData() { return result; } + if (getCrcFlag() && remSize == 2) { + return returnvalue::OK; + } + if (remSize > 0) { if (optionArrayMaxSize == 0 or optionArray == nullptr) { return cfdp::METADATA_CANT_PARSE_OPTIONS; diff --git a/src/fsfw/cfdp/pdu/NakPduCreator.cpp b/src/fsfw/cfdp/pdu/NakPduCreator.cpp index 11f3aba9..d9d21344 100644 --- a/src/fsfw/cfdp/pdu/NakPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/NakPduCreator.cpp @@ -1,5 +1,7 @@ #include "NakPduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + NakPduCreator::NakPduCreator(PduConfig &pduConf, NakInfo &nakInfo) : FileDirectiveCreator(pduConf, cfdp::FileDirective::NAK, 0), nakInfo(nakInfo) { updateDirectiveFieldLen(); @@ -13,6 +15,7 @@ size_t NakPduCreator::getSerializedSize() const { return FileDirectiveCreator::g ReturnValue_t NakPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -40,5 +43,9 @@ ReturnValue_t NakPduCreator::serialize(uint8_t **buffer, size_t *size, size_t ma } } } + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return result; } diff --git a/src/fsfw/cfdp/pdu/PromptPduCreator.cpp b/src/fsfw/cfdp/pdu/PromptPduCreator.cpp index f4141fdb..d5d7df02 100644 --- a/src/fsfw/cfdp/pdu/PromptPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/PromptPduCreator.cpp @@ -1,5 +1,7 @@ #include "PromptPduCreator.h" +#include "fsfw/globalfunctions/CRC.h" + PromptPduCreator::PromptPduCreator(PduConfig &conf, cfdp::PromptResponseRequired responseRequired) : FileDirectiveCreator(conf, cfdp::FileDirective::PROMPT, 1), responseRequired(responseRequired) {} @@ -10,6 +12,7 @@ size_t PromptPduCreator::getSerializedSize() const { ReturnValue_t PromptPduCreator::serialize(uint8_t **buffer, size_t *size, size_t maxSize, Endianness streamEndianness) const { + const uint8_t *start = *buffer; ReturnValue_t result = FileDirectiveCreator::serialize(buffer, size, maxSize, streamEndianness); if (result != returnvalue::OK) { return result; @@ -20,5 +23,10 @@ ReturnValue_t PromptPduCreator::serialize(uint8_t **buffer, size_t *size, size_t **buffer = responseRequired << 7; *buffer += 1; *size += 1; + + if (getCrcFlag()) { + uint16_t crc = CRC::crc16ccitt(start, getWholePduSize() - 2); + result = SerializeAdapter::serialize(&crc, buffer, size, maxSize, streamEndianness); + } return result; } diff --git a/src/fsfw/cfdp/tlv/FilestoreTlvBase.h b/src/fsfw/cfdp/tlv/FilestoreTlvBase.h index a5974087..51dc7e29 100644 --- a/src/fsfw/cfdp/tlv/FilestoreTlvBase.h +++ b/src/fsfw/cfdp/tlv/FilestoreTlvBase.h @@ -66,9 +66,9 @@ static constexpr uint8_t FSR_DENY_DIR_NOT_ALLOWED = 0b0010; class FilestoreTlvBase : public TlvIF { public: - explicit FilestoreTlvBase(cfdp::StringLv& firstFileName) : firstFileName(firstFileName){}; + explicit FilestoreTlvBase(cfdp::StringLv& firstFileName) : firstFileName(firstFileName) {}; FilestoreTlvBase(FilestoreActionCode actionCode, cfdp::StringLv& firstFileName) - : actionCode(actionCode), firstFileName(firstFileName){}; + : actionCode(actionCode), firstFileName(firstFileName) {}; ReturnValue_t commonSerialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness, bool isResponse = false, diff --git a/src/fsfw/cfdp/tlv/TlvIF.h b/src/fsfw/cfdp/tlv/TlvIF.h index 317d970a..6d14156f 100644 --- a/src/fsfw/cfdp/tlv/TlvIF.h +++ b/src/fsfw/cfdp/tlv/TlvIF.h @@ -5,7 +5,7 @@ class TlvIF : public SerializeIF { public: - virtual ~TlvIF(){}; + virtual ~TlvIF() {}; virtual uint8_t getLengthField() const = 0; virtual cfdp::TlvType getType() const = 0; diff --git a/src/fsfw/coordinates/Jgm3Model.h b/src/fsfw/coordinates/Jgm3Model.h index e0be20a3..d9a4d665 100644 --- a/src/fsfw/coordinates/Jgm3Model.h +++ b/src/fsfw/coordinates/Jgm3Model.h @@ -29,7 +29,7 @@ class Jgm3Model { lastExecutionTime.tv_sec = 0; lastExecutionTime.tv_usec = 0; } - virtual ~Jgm3Model(){}; + virtual ~Jgm3Model() {}; // double acsNavOrbit(double posECF[3],double velECF[3],timeval gpsTime); @@ -70,10 +70,10 @@ class Jgm3Model { W[n][m] = W[n][m] - (((n + m - 1) / (double)(n - m)) * (pow(Earth::MEAN_RADIUS, 2) / pow(r, 2)) * W[n - 2][m]); } // End of if(n!=(m+1)) - } // End of if(n==m){ - } // End of if(n==0 and m==0) - } // End of for(uint8_t n=0;n<(DEGREE+1);n++) - } // End of for(uint8_t m=0;m<(ORDER+1);m++) + } // End of if(n==m){ + } // End of if(n==0 and m==0) + } // End of for(uint8_t n=0;n<(DEGREE+1);n++) + } // End of for(uint8_t m=0;m<(ORDER+1);m++) // overwrite accel if not properly initialized accel[0] = 0; @@ -106,7 +106,7 @@ class Jgm3Model { accel[1] += partAccel[1]; accel[2] += partAccel[2]; } // End of for(uint8_t n=0;n::LocalPoolVar: The supplied pool " - << "owner is a invalid!" << std::endl; + sif::error << "LocalPoolVar::LocalPoolVar: The supplied pool " << "owner is a invalid!" + << std::endl; #endif return; } diff --git a/src/fsfw/datapoollocal/LocalPoolVariable.h b/src/fsfw/datapoollocal/LocalPoolVariable.h index 76b36503..dc67095c 100644 --- a/src/fsfw/datapoollocal/LocalPoolVariable.h +++ b/src/fsfw/datapoollocal/LocalPoolVariable.h @@ -76,7 +76,7 @@ class LocalPoolVariable : public LocalPoolObjectBase { LocalPoolVariable(gp_id_t globalPoolId, DataSetIF* dataSet = nullptr, pool_rwm_t setReadWriteMode = pool_rwm_t::VAR_READ_WRITE); - virtual ~LocalPoolVariable(){}; + virtual ~LocalPoolVariable() {}; /** * @brief This is the local copy of the data pool entry. diff --git a/src/fsfw/datapoollocal/LocalPoolVector.h b/src/fsfw/datapoollocal/LocalPoolVector.h index d340f2de..ff5943aa 100644 --- a/src/fsfw/datapoollocal/LocalPoolVector.h +++ b/src/fsfw/datapoollocal/LocalPoolVector.h @@ -88,7 +88,7 @@ class LocalPoolVector : public LocalPoolObjectBase { * @details If commit() was not called, the local value is * discarded and not written back to the data pool. */ - ~LocalPoolVector(){}; + ~LocalPoolVector() {}; /** * @brief The operation returns the number of array entries * in this variable. diff --git a/src/fsfw/datapoollocal/MarkChangedIF.h b/src/fsfw/datapoollocal/MarkChangedIF.h index 21667dda..54c2c7f3 100644 --- a/src/fsfw/datapoollocal/MarkChangedIF.h +++ b/src/fsfw/datapoollocal/MarkChangedIF.h @@ -6,7 +6,7 @@ */ class MarkChangedIF { public: - virtual ~MarkChangedIF(){}; + virtual ~MarkChangedIF() {}; virtual bool hasChanged() const = 0; virtual void setChanged(bool changed) = 0; diff --git a/src/fsfw/devicehandlers/CookieIF.h b/src/fsfw/devicehandlers/CookieIF.h index aca5d942..c70b27be 100644 --- a/src/fsfw/devicehandlers/CookieIF.h +++ b/src/fsfw/devicehandlers/CookieIF.h @@ -28,7 +28,7 @@ using address_t = uint32_t; */ class CookieIF { public: - virtual ~CookieIF(){}; + virtual ~CookieIF() {}; }; #endif /* FSFW_DEVICEHANDLER_COOKIE_H_ */ diff --git a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp index aa897769..bdb879df 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp @@ -243,8 +243,8 @@ bool DeviceHandlerFailureIsolation::isFdirInActionOrAreWeFaulty(EventMessage* ev if (owner == nullptr) { // Configuration error. #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerFailureIsolation::" - << "isFdirInActionOrAreWeFaulty: Owner not set!" << std::endl; + sif::error << "DeviceHandlerFailureIsolation::" << "isFdirInActionOrAreWeFaulty: Owner not set!" + << std::endl; #endif return false; } diff --git a/src/fsfw/fdir/FailureIsolationBase.cpp b/src/fsfw/fdir/FailureIsolationBase.cpp index cbf2cc06..5082cf08 100644 --- a/src/fsfw/fdir/FailureIsolationBase.cpp +++ b/src/fsfw/fdir/FailureIsolationBase.cpp @@ -62,8 +62,7 @@ ReturnValue_t FailureIsolationBase::initialize() { ObjectManager::instance()->get(faultTreeParent); if (parentIF == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "FailureIsolationBase::intialize: Parent object " - << "invalid" << std::endl; + sif::error << "FailureIsolationBase::intialize: Parent object " << "invalid" << std::endl; sif::error << "Make sure it implements ConfirmsFailuresIF" << std::endl; #else sif::printError("FailureIsolationBase::intialize: Parent object invalid\n"); diff --git a/src/fsfw/filesystem/HasFileSystemIF.h b/src/fsfw/filesystem/HasFileSystemIF.h index bffc174d..0b6b1a2f 100644 --- a/src/fsfw/filesystem/HasFileSystemIF.h +++ b/src/fsfw/filesystem/HasFileSystemIF.h @@ -58,11 +58,7 @@ class HasFileSystemIF { static constexpr ReturnValue_t DIRECTORY_ALREADY_EXISTS = MAKE_RETURN_CODE(22); static constexpr ReturnValue_t NOT_A_DIRECTORY = MAKE_RETURN_CODE(23); static constexpr ReturnValue_t DIRECTORY_NOT_EMPTY = MAKE_RETURN_CODE(24); - - //! [EXPORT] : P1: Sequence number missing - static constexpr ReturnValue_t SEQUENCE_PACKET_MISSING_WRITE = MAKE_RETURN_CODE(30); - //! [EXPORT] : P1: Sequence number missing - static constexpr ReturnValue_t SEQUENCE_PACKET_MISSING_READ = MAKE_RETURN_CODE(31); + static constexpr ReturnValue_t ELEMENT_DOES_NOT_EXIST = MAKE_RETURN_CODE(25); virtual ~HasFileSystemIF() = default; @@ -75,21 +71,27 @@ class HasFileSystemIF { } // Get the base filename without the full directory path - virtual ReturnValue_t getBaseFilename(FilesystemParams params, char* nameBuf, size_t maxLen, + virtual ReturnValue_t getBaseFilename(const char* path, char* nameBuf, size_t maxLen, size_t& baseNameLen) = 0; - virtual bool isDirectory(const char* path) = 0; + virtual ReturnValue_t isDirectory(const char* path, bool& isDirectory) = 0; - virtual ReturnValue_t getFileSize(FilesystemParams params, uint64_t& fileSize) = 0; + virtual ReturnValue_t getFileSize(const char* path, uint64_t& fileSize, + FileSystemArgsIF* args) = 0; + virtual ReturnValue_t getFileSize(const char* path, uint64_t& fileSize) { + return getFileSize(path, fileSize, nullptr); + } - virtual bool fileExists(FilesystemParams params) = 0; + virtual bool fileExists(const char* path, FileSystemArgsIF* args) = 0; + virtual bool fileExists(const char* path) { return fileExists(path, nullptr); } /** * Truncate a file, deleting its contents and setting its size to 0 accordingly. * @param params * @return */ - virtual ReturnValue_t truncateFile(FilesystemParams params) = 0; + virtual ReturnValue_t truncateFile(const char* path, FileSystemArgsIF* args) = 0; + virtual ReturnValue_t truncateFile(const char* path) { return truncateFile(path, nullptr); } /** * @brief Generic function to write to a file. @@ -100,32 +102,23 @@ class HasFileSystemIF { * @param fileOpInfo General information: File name, size to write, offset, additional arguments * @param data The data to write to the file */ - virtual ReturnValue_t writeToFile(FileOpParams params, const uint8_t* data) = 0; + virtual ReturnValue_t writeToFile(const char* path, size_t offset, const uint8_t* data, + size_t size) = 0; /** * @brief Generic function to read from a file. This variant takes a pointer to a buffer and * performs pointer arithmetic by incrementing the pointer by the read size - * @param fileOpInfo General information: File name, size to write, offset, additional arguments - * @param buffer [in/out] Data will be read into the provided buffer, and the pointer will be - * incremented by the read length - * @param readSize [out] Will be incremented by the read length - * @param maxSize Maximum size of the provided buffer + * @param path [in] file path + * @param offset [in] offset of data to read + * @param sizeToRead [in] size of data to read + * @param buffer [in/out] Data will be read into the provided buffer. + * @param actuallyRead [out] Will be set to the length actually read. + * @param maxSize [in] Maximum size of the provided buffer * @param args * @return */ - virtual ReturnValue_t readFromFile(FileOpParams fileOpInfo, uint8_t** buffer, size_t& readSize, - size_t maxSize) = 0; - /** - * Variant of the @readFromFile which does not perform pointer arithmetic. - * @param fileOpInfo General information: File name, size to write, offset, additional arguments - * @param buf - * @param maxSize - * @return - */ - virtual ReturnValue_t readFromFile(FileOpParams fileOpInfo, uint8_t* buf, size_t maxSize) { - size_t dummy = 0; - return readFromFile(fileOpInfo, &buf, dummy, maxSize); - } + virtual ReturnValue_t readFromFile(const char* path, size_t offset, size_t sizeToRead, + uint8_t* buffer, size_t& actuallyRead, size_t maxSize) = 0; /** * @brief Generic function to create a new file. @@ -136,10 +129,8 @@ class HasFileSystemIF { * @param args Any other arguments which an implementation might require * @return */ - virtual ReturnValue_t createFile(FilesystemParams params) { - return createFile(params, nullptr, 0); - } - virtual ReturnValue_t createFile(FilesystemParams params, const uint8_t* data, size_t size) = 0; + virtual ReturnValue_t createFile(const char* path) { return createFile(path, nullptr, 0); } + virtual ReturnValue_t createFile(const char* path, const uint8_t* data, size_t size) = 0; /** * @brief Generic function to delete a file. @@ -159,9 +150,13 @@ class HasFileSystemIF { * @param args Any other arguments which an implementation might require * @return */ - virtual ReturnValue_t createDirectory(FilesystemParams params, bool createParentDirs) = 0; - virtual ReturnValue_t createDirectory(FilesystemParams params) { - return createDirectory(params, false); + virtual ReturnValue_t createDirectory(const char* path, bool createParentDirs, + FileSystemArgsIF* args) = 0; + virtual ReturnValue_t createDirectory(const char* path, FileSystemArgsIF* args) { + return createDirectory(path, false, args); + } + virtual ReturnValue_t createDirectory(const char* path) { + return createDirectory(path, false, nullptr); } /** @@ -169,9 +164,10 @@ class HasFileSystemIF { * @param repositoryPath * @param args Any other arguments which an implementation might require */ - virtual ReturnValue_t removeDirectory(FilesystemParams params, bool deleteRecurively) = 0; - virtual ReturnValue_t removeDirectory(FilesystemParams params) { - return removeDirectory(params, false); + virtual ReturnValue_t removeDirectory(const char* path, bool deleteRecurively, + FileSystemArgsIF* args) = 0; + virtual ReturnValue_t removeDirectory(const char* path, FileSystemArgsIF* args) { + return removeDirectory(path, false, nullptr); } virtual ReturnValue_t rename(const char* oldPath, const char* newPath) { diff --git a/src/fsfw/housekeeping/HousekeepingSnapshot.h b/src/fsfw/housekeeping/HousekeepingSnapshot.h index 9807795d..7e31c464 100644 --- a/src/fsfw/housekeeping/HousekeepingSnapshot.h +++ b/src/fsfw/housekeeping/HousekeepingSnapshot.h @@ -23,7 +23,7 @@ class HousekeepingSnapshot : public SerializeIF { HousekeepingSnapshot(CCSDSTime::CDS_short* cdsShort, LocalPoolDataSetBase* dataSetPtr) : timeStamp(reinterpret_cast(cdsShort)), timeStampSize(sizeof(CCSDSTime::CDS_short)), - updateData(dataSetPtr){}; + updateData(dataSetPtr) {}; /** * Update packet constructor for datasets. @@ -32,7 +32,7 @@ class HousekeepingSnapshot : public SerializeIF { * @param dataSetPtr Pointer to the dataset instance to deserialize the data into */ HousekeepingSnapshot(uint8_t* timeStamp, size_t timeStampSize, LocalPoolDataSetBase* dataSetPtr) - : timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr){}; + : timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr) {}; /** * Update packet constructor for pool variables. @@ -43,7 +43,7 @@ class HousekeepingSnapshot : public SerializeIF { HousekeepingSnapshot(CCSDSTime::CDS_short* cdsShort, LocalPoolObjectBase* dataSetPtr) : timeStamp(reinterpret_cast(cdsShort)), timeStampSize(sizeof(CCSDSTime::CDS_short)), - updateData(dataSetPtr){}; + updateData(dataSetPtr) {}; /** * Update packet constructor for pool variables. @@ -52,7 +52,7 @@ class HousekeepingSnapshot : public SerializeIF { * @param dataSetPtr */ HousekeepingSnapshot(uint8_t* timeStamp, size_t timeStampSize, LocalPoolObjectBase* dataSetPtr) - : timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr){}; + : timeStamp(timeStamp), timeStampSize(timeStampSize), updateData(dataSetPtr) {}; ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const { diff --git a/src/fsfw/ipc/CommandMessageIF.h b/src/fsfw/ipc/CommandMessageIF.h index e714ca70..7a8cbb22 100644 --- a/src/fsfw/ipc/CommandMessageIF.h +++ b/src/fsfw/ipc/CommandMessageIF.h @@ -37,7 +37,7 @@ class CommandMessageIF { //! Parameter 1 should contain the error code static const Command_t REPLY_REJECTED = MAKE_COMMAND_ID(2); - virtual ~CommandMessageIF(){}; + virtual ~CommandMessageIF() {}; /** * A command message shall have a uint16_t command ID field. diff --git a/src/fsfw/ipc/definitions.h b/src/fsfw/ipc/definitions.h index 150502bb..df4e575f 100644 --- a/src/fsfw/ipc/definitions.h +++ b/src/fsfw/ipc/definitions.h @@ -5,7 +5,7 @@ #include struct MqArgs { - MqArgs(){}; + MqArgs() {}; MqArgs(object_id_t objectId, void* args = nullptr) : objectId(objectId), args(args) {} object_id_t objectId = objects::NO_OBJECT; void* args = nullptr; diff --git a/src/fsfw/objectmanager/ObjectManagerIF.h b/src/fsfw/objectmanager/ObjectManagerIF.h index 81ddd95a..162584c0 100644 --- a/src/fsfw/objectmanager/ObjectManagerIF.h +++ b/src/fsfw/objectmanager/ObjectManagerIF.h @@ -40,7 +40,7 @@ class ObjectManagerIF { /** * @brief This is the empty virtual destructor as requested by C++ interfaces. */ - virtual ~ObjectManagerIF(void){}; + virtual ~ObjectManagerIF(void) {}; /** * @brief With this call, new objects are inserted to the list. * @details The implementation shall return an error code in case the diff --git a/src/fsfw/osal/common/TcpTmTcBridge.cpp b/src/fsfw/osal/common/TcpTmTcBridge.cpp index 80c2ddd1..77f226ce 100644 --- a/src/fsfw/osal/common/TcpTmTcBridge.cpp +++ b/src/fsfw/osal/common/TcpTmTcBridge.cpp @@ -49,8 +49,8 @@ ReturnValue_t TcpTmTcBridge::handleTm() { MutexGuard guard(mutex, timeoutType, mutexTimeoutMs); TmTcMessage message; ReturnValue_t status = returnvalue::OK; - for (ReturnValue_t result = tmTcReceptionQueue->receiveMessage(&message); - result == returnvalue::OK; result = tmTcReceptionQueue->receiveMessage(&message)) { + for (ReturnValue_t result = tmtcReceptionQueue->receiveMessage(&message); + result == returnvalue::OK; result = tmtcReceptionQueue->receiveMessage(&message)) { status = storeDownlinkData(&message); if (status != returnvalue::OK) { break; diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 42417f84..b724aa9a 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -321,8 +321,8 @@ ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { #if FSFW_CPP_OSTREAM_ENABLED == 1 // Possible configuration error, too much data or/and data coming in too fast, // requiring larger buffers - sif::warning << "TcpTmTcServer::handleServerOperation: Ring buffer reached " - << "fill count" << std::endl; + sif::warning << "TcpTmTcServer::handleServerOperation: Ring buffer reached " << "fill count" + << std::endl; #else sif::printWarning( "TcpTmTcServer::handleServerOperation: Ring buffer reached " diff --git a/src/fsfw/pus/Service20ParameterManagement.cpp b/src/fsfw/pus/Service20ParameterManagement.cpp index 87bd5a13..fc9bfbfd 100644 --- a/src/fsfw/pus/Service20ParameterManagement.cpp +++ b/src/fsfw/pus/Service20ParameterManagement.cpp @@ -49,8 +49,8 @@ ReturnValue_t Service20ParameterManagement::checkAndAcquireTargetID(object_id_t* if (SerializeAdapter::deSerialize(objectIdToSet, &tcData, &tcDataLen, SerializeIF::Endianness::BIG) != returnvalue::OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "Service20ParameterManagement::checkAndAcquireTargetID: " - << "Invalid data." << std::endl; + sif::error << "Service20ParameterManagement::checkAndAcquireTargetID: " << "Invalid data." + << std::endl; #else sif::printError( "Service20ParameterManagement::" diff --git a/src/fsfw/pus/Service3Housekeeping.cpp b/src/fsfw/pus/Service3Housekeeping.cpp index 4d1137d1..328f5a25 100644 --- a/src/fsfw/pus/Service3Housekeeping.cpp +++ b/src/fsfw/pus/Service3Housekeeping.cpp @@ -196,8 +196,8 @@ ReturnValue_t Service3Housekeeping::handleReply(const CommandMessage* reply, default: #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "Service3Housekeeping::handleReply: Invalid reply with " - << "reply command " << command << std::endl; + sif::warning << "Service3Housekeeping::handleReply: Invalid reply with " << "reply command " + << command << std::endl; #else sif::printWarning( "Service3Housekeeping::handleReply: Invalid reply with " diff --git a/src/fsfw/rmap/RMAPChannelIF.h b/src/fsfw/rmap/RMAPChannelIF.h index 60d439d0..7fd4f9a3 100644 --- a/src/fsfw/rmap/RMAPChannelIF.h +++ b/src/fsfw/rmap/RMAPChannelIF.h @@ -9,7 +9,7 @@ class RMAPChannelIF { public: - virtual ~RMAPChannelIF(){}; + virtual ~RMAPChannelIF() {}; /** * Reset an RMAP channel * diff --git a/src/fsfw/storagemanager/LocalPool.cpp b/src/fsfw/storagemanager/LocalPool.cpp index 9a4b53a6..f1582136 100644 --- a/src/fsfw/storagemanager/LocalPool.cpp +++ b/src/fsfw/storagemanager/LocalPool.cpp @@ -12,8 +12,7 @@ LocalPool::LocalPool(object_id_t setObjectId, const LocalPoolConfig& poolConfig, spillsToHigherPools(spillsToHigherPools) { if (NUMBER_OF_SUBPOOLS == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalPool::LocalPool: Passed pool configuration is " - << " invalid!" << std::endl; + sif::error << "LocalPool::LocalPool: Passed pool configuration is " << " invalid!" << std::endl; #endif } max_subpools_t index = 0; diff --git a/src/fsfw/subsystem/Subsystem.cpp b/src/fsfw/subsystem/Subsystem.cpp index ad3afb2b..ec7b62b9 100644 --- a/src/fsfw/subsystem/Subsystem.cpp +++ b/src/fsfw/subsystem/Subsystem.cpp @@ -38,10 +38,9 @@ ReturnValue_t Subsystem::checkSequence(HybridIterator iter, if (!existsModeTable(iter->getTableId())) { #if FSFW_CPP_OSTREAM_ENABLED == 1 using namespace std; - sif::warning << "Subsystem::checkSequence: " - << "Object " << setfill('0') << hex << "0x" << setw(8) << getObjectId() - << setw(0) << ": Mode table for mode ID " - << "0x" << setw(8) << iter->getTableId() << " does not exist" << dec << endl; + sif::warning << "Subsystem::checkSequence: " << "Object " << setfill('0') << hex << "0x" + << setw(8) << getObjectId() << setw(0) << ": Mode table for mode ID " << "0x" + << setw(8) << iter->getTableId() << " does not exist" << dec << endl; #endif return TABLE_DOES_NOT_EXIST; } else { diff --git a/src/fsfw/subsystem/Subsystem.h b/src/fsfw/subsystem/Subsystem.h index 20925821..1d25222e 100644 --- a/src/fsfw/subsystem/Subsystem.h +++ b/src/fsfw/subsystem/Subsystem.h @@ -12,7 +12,7 @@ struct TableSequenceBase { public: - TableSequenceBase(Mode_t mode, ArrayList *table) : mode(mode), table(table){}; + TableSequenceBase(Mode_t mode, ArrayList *table) : mode(mode), table(table) {}; Mode_t mode; ArrayList *table; bool inStore = false; @@ -21,7 +21,7 @@ struct TableSequenceBase { struct TableEntry : public TableSequenceBase { public: - TableEntry(Mode_t mode, ArrayList *table) : TableSequenceBase(mode, table){}; + TableEntry(Mode_t mode, ArrayList *table) : TableSequenceBase(mode, table) {}; }; struct SequenceEntry : public TableSequenceBase { diff --git a/src/fsfw/tasks/ExecutableObjectIF.h b/src/fsfw/tasks/ExecutableObjectIF.h index 1e3f19e4..4248019e 100644 --- a/src/fsfw/tasks/ExecutableObjectIF.h +++ b/src/fsfw/tasks/ExecutableObjectIF.h @@ -34,7 +34,7 @@ class ExecutableObjectIF { * a reference to the executing task * @param task_ Pointer to the taskIF of this task */ - virtual void setTaskIF(PeriodicTaskIF* task_){}; + virtual void setTaskIF(PeriodicTaskIF* task_) {}; /** * This function should be called after the object was assigned to a diff --git a/src/fsfw/tasks/FixedSlotSequence.cpp b/src/fsfw/tasks/FixedSlotSequence.cpp index cba7a87a..3774f224 100644 --- a/src/fsfw/tasks/FixedSlotSequence.cpp +++ b/src/fsfw/tasks/FixedSlotSequence.cpp @@ -101,8 +101,7 @@ ReturnValue_t FixedSlotSequence::checkSequence() const { if (result != returnvalue::OK) { // Continue for now but print error output. #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "FixedSlotSequence::checkSequence:" - << " Custom check failed!" << std::endl; + sif::error << "FixedSlotSequence::checkSequence:" << " Custom check failed!" << std::endl; #endif } } diff --git a/src/fsfw/tasks/SemaphoreIF.h b/src/fsfw/tasks/SemaphoreIF.h index 9a692e9b..f9d1a795 100644 --- a/src/fsfw/tasks/SemaphoreIF.h +++ b/src/fsfw/tasks/SemaphoreIF.h @@ -30,7 +30,7 @@ class SemaphoreIF { BLOCKING //!< Block indefinitely until the mutex becomes available. }; - virtual ~SemaphoreIF(){}; + virtual ~SemaphoreIF() {}; static const uint8_t INTERFACE_ID = CLASS_ID::SEMAPHORE_IF; //! Semaphore timeout diff --git a/src/fsfw/tcdistribution/PusDistributor.cpp b/src/fsfw/tcdistribution/PusDistributor.cpp index 7599114b..b7b5e516 100644 --- a/src/fsfw/tcdistribution/PusDistributor.cpp +++ b/src/fsfw/tcdistribution/PusDistributor.cpp @@ -8,8 +8,8 @@ #define PUS_DISTRIBUTOR_DEBUGGING 0 -PusDistributor::PusDistributor(uint16_t setApid, object_id_t setObjectId, - CcsdsDistributorIF* distributor, StorageManagerIF* store_) +PusDistributor::PusDistributor(uint16_t setApid, object_id_t setObjectId, StorageManagerIF* store_, + CcsdsDistributorIF* distributor) : TcDistributorBase(setObjectId), store(store_), checker(setApid, ccsds::PacketType::TC), @@ -122,14 +122,12 @@ ReturnValue_t PusDistributor::initialize() { return ObjectManagerIF::CHILD_INIT_FAILED; } } - if (ccsdsDistributor == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "PUSDistributor::initialize: Packet source invalid" << std::endl; - sif::error << " Make sure it exists and implements CCSDSDistributorIF!" << std::endl; -#else - sif::printError("PusDistributor::initialize: Packet source invalid\n"); - sif::printError("Make sure it exists and implements CcsdsDistributorIF\n"); -#endif + if (ccsdsDistributor != nullptr) { + ReturnValue_t result = + ccsdsDistributor->registerApplication(CcsdsDistributorIF::DestInfo(*this, false)); + if (result != returnvalue::OK) { + return result; + } return ObjectManagerIF::CHILD_INIT_FAILED; } if (verifyChannel == nullptr) { @@ -139,7 +137,7 @@ ReturnValue_t PusDistributor::initialize() { return ObjectManagerIF::CHILD_INIT_FAILED; } } - return ccsdsDistributor->registerApplication(CcsdsDistributorIF::DestInfo(*this, false)); + return SystemObject::initialize(); } void PusDistributor::checkerFailurePrinter() const { diff --git a/src/fsfw/tcdistribution/PusDistributor.h b/src/fsfw/tcdistribution/PusDistributor.h index 206f112a..f10c9d9b 100644 --- a/src/fsfw/tcdistribution/PusDistributor.h +++ b/src/fsfw/tcdistribution/PusDistributor.h @@ -31,8 +31,9 @@ class PusDistributor : public TcDistributorBase, * @param setPacketSource Object ID of the source of TC packets. * Must implement CcsdsDistributorIF. */ - PusDistributor(uint16_t setApid, object_id_t setObjectId, CcsdsDistributorIF* packetSource, - StorageManagerIF* store = nullptr); + PusDistributor(uint16_t setApid, object_id_t setObjectId, StorageManagerIF* store = nullptr, + CcsdsDistributorIF* packetSource = nullptr); + [[nodiscard]] const char* getName() const override; /** * The destructor is empty. diff --git a/src/fsfw/tcdistribution/definitions.h b/src/fsfw/tcdistribution/definitions.h index bec1dd1b..04a7a4a5 100644 --- a/src/fsfw/tcdistribution/definitions.h +++ b/src/fsfw/tcdistribution/definitions.h @@ -27,5 +27,5 @@ static constexpr uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::TMTC_DISTRIBUTION; //! P1: Returnvalue, P2: 0 for TM issues, 1 for TC issues static constexpr Event HANDLE_PACKET_FAILED = event::makeEvent(SUBSYSTEM_ID, 0, severity::LOW); -}; // namespace tmtcdistrib +}; // namespace tmtcdistrib #endif // FSFW_TMTCPACKET_DEFINITIONS_H diff --git a/src/fsfw/timemanager/CCSDSTime.h b/src/fsfw/timemanager/CCSDSTime.h index 03a8ea8f..3d1a25ad 100644 --- a/src/fsfw/timemanager/CCSDSTime.h +++ b/src/fsfw/timemanager/CCSDSTime.h @@ -212,8 +212,8 @@ class CCSDSTime { static uint32_t subsecondsToMicroseconds(uint16_t subseconds); private: - CCSDSTime(){}; - virtual ~CCSDSTime(){}; + CCSDSTime() {}; + virtual ~CCSDSTime() {}; /** * checks a ccs time stream for validity * diff --git a/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp b/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp index 745d5834..550f0e1a 100644 --- a/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp +++ b/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp @@ -43,9 +43,10 @@ ReturnValue_t SpacePacketCreator::serialize(uint8_t **buffer, size_t *size, size void SpacePacketCreator::setCcsdsLenFromTotalDataFieldLen(size_t actualLength) { if (actualLength == 0) { - return; + setDataLenField(actualLength); + } else { + setDataLenField(actualLength - 1); } - setDataLenField(actualLength - 1); } size_t SpacePacketCreator::getSerializedSize() const { return ccsds::HEADER_LEN; } diff --git a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h index f683b363..253cbe1d 100644 --- a/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h +++ b/src/fsfw/tmtcpacket/pus/tm/PusTmCreator.h @@ -22,7 +22,7 @@ struct PusTmSecHeader { struct PusTmParams { PusTmParams() = default; - explicit PusTmParams(PusTmSecHeader secHeader) : secHeader(secHeader){}; + explicit PusTmParams(PusTmSecHeader secHeader) : secHeader(secHeader) {}; PusTmParams(PusTmSecHeader secHeader, const SerializeIF& data) : secHeader(secHeader), sourceData(&data) {} PusTmParams(PusTmSecHeader secHeader, const uint8_t* data, size_t dataLen) diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index 9801b21e..c6249e62 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -17,11 +17,11 @@ TmTcBridge::TmTcBridge(const char* name, object_id_t objectId, object_id_t tcDes { auto mqArgs = MqArgs(objectId, static_cast(this)); - tmTcReceptionQueue = QueueFactory::instance()->createMessageQueue( + tmtcReceptionQueue = QueueFactory::instance()->createMessageQueue( msgQueueDepth, MessageQueueMessage::MAX_MESSAGE_SIZE, &mqArgs); } -TmTcBridge::~TmTcBridge() { QueueFactory::instance()->deleteMessageQueue(tmTcReceptionQueue); } +TmTcBridge::~TmTcBridge() { QueueFactory::instance()->deleteMessageQueue(tmtcReceptionQueue); } void TmTcBridge::setNumberOfSentPacketsPerCycle(uint32_t sentPacketsPerCycle_) { this->sentPacketsPerCycle = sentPacketsPerCycle_; @@ -34,8 +34,7 @@ ReturnValue_t TmTcBridge::setMaxNumberOfPacketsStored(unsigned int maxNumberOfPa } else { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "TmTcBridge::setMaxNumberOfPacketsStored: Number of " - << "packets stored exceeds limits. " - << "Keeping default value." << std::endl; + << "packets stored exceeds limits. " << "Keeping default value." << std::endl; #endif return returnvalue::FAILED; } @@ -70,7 +69,7 @@ ReturnValue_t TmTcBridge::initialize() { tmFifo = new DynamicFIFO(maxNumberOfPacketsStored); - tmTcReceptionQueue->setDefaultDestination(tcDistributor->getRequestQueue()); + tmtcReceptionQueue->setDefaultDestination(tcDistributor->getRequestQueue()); return returnvalue::OK; } @@ -79,15 +78,13 @@ ReturnValue_t TmTcBridge::performOperation(uint8_t operationCode) { result = handleTc(); if (result != returnvalue::OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "TmTcBridge::performOperation: " - << "Error handling TCs" << std::endl; + sif::debug << "TmTcBridge::performOperation: " << "Error handling TCs" << std::endl; #endif } result = handleTm(); if (result != returnvalue::OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "TmTcBridge::performOperation: " - << "Error handling TMs" << std::endl; + sif::debug << "TmTcBridge::performOperation: " << "Error handling TMs" << std::endl; #endif } return result; @@ -124,8 +121,8 @@ ReturnValue_t TmTcBridge::handleTmQueue() { const uint8_t* data = nullptr; size_t size = 0; ReturnValue_t status = returnvalue::OK; - for (ReturnValue_t result = tmTcReceptionQueue->receiveMessage(&message); - result == returnvalue::OK; result = tmTcReceptionQueue->receiveMessage(&message)) { + for (ReturnValue_t result = tmtcReceptionQueue->receiveMessage(&message); + result == returnvalue::OK; result = tmtcReceptionQueue->receiveMessage(&message)) { #if FSFW_VERBOSE_LEVEL >= 3 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "Sent packet counter: " << static_cast(packetSentCounter) << std::endl; @@ -241,7 +238,7 @@ void TmTcBridge::registerCommDisconnect() { } MessageQueueId_t TmTcBridge::getReportReceptionQueue(uint8_t virtualChannel) const { - return tmTcReceptionQueue->getId(); + return tmtcReceptionQueue->getId(); } void TmTcBridge::printData(uint8_t* data, size_t dataLen) { arrayprinter::print(data, dataLen); } @@ -253,7 +250,7 @@ uint32_t TmTcBridge::getIdentifier() const { MessageQueueId_t TmTcBridge::getRequestQueue() const { // Default implementation: Relay TC messages to TC distributor directly. - return tmTcReceptionQueue->getDefaultDestination(); + return tmtcReceptionQueue->getDefaultDestination(); } void TmTcBridge::setFifoToOverwriteOldData(bool overwriteOld) { this->overwriteOld = overwriteOld; } diff --git a/src/fsfw/tmtcservices/TmTcBridge.h b/src/fsfw/tmtcservices/TmTcBridge.h index c87be45c..73d5ce07 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.h +++ b/src/fsfw/tmtcservices/TmTcBridge.h @@ -80,7 +80,7 @@ class TmTcBridge : public AcceptsTelemetryIF, //! Used to send and receive TMTC messages. //! The TmTcMessage class is used to transport messages between tasks. - MessageQueueIF* tmTcReceptionQueue = nullptr; + MessageQueueIF* tmtcReceptionQueue = nullptr; StorageManagerIF* tmStore = nullptr; StorageManagerIF* tcStore = nullptr; diff --git a/src/fsfw_hal/common/gpio/GpioIF.h b/src/fsfw_hal/common/gpio/GpioIF.h index 20ae8a26..3ad5673e 100644 --- a/src/fsfw_hal/common/gpio/GpioIF.h +++ b/src/fsfw_hal/common/gpio/GpioIF.h @@ -15,7 +15,7 @@ class GpioCookie; */ class GpioIF { public: - virtual ~GpioIF(){}; + virtual ~GpioIF() {}; /** * @brief Called by the GPIO using object. diff --git a/src/fsfw_hal/common/gpio/gpioDefinitions.h b/src/fsfw_hal/common/gpio/gpioDefinitions.h index 1f921a57..7e3aa450 100644 --- a/src/fsfw_hal/common/gpio/gpioDefinitions.h +++ b/src/fsfw_hal/common/gpio/gpioDefinitions.h @@ -60,7 +60,7 @@ class GpioBase { gpio::Levels initValue) : gpioType(gpioType), consumer(consumer), direction(direction), initValue(initValue) {} - virtual ~GpioBase(){}; + virtual ~GpioBase() {}; // Can be used to cast GpioBase to a concrete child implementation gpio::GpioTypes gpioType = gpio::GpioTypes::NONE; diff --git a/src/fsfw_hal/host/HostFilesystem.cpp b/src/fsfw_hal/host/HostFilesystem.cpp index 827dda3d..01a3ebb4 100644 --- a/src/fsfw_hal/host/HostFilesystem.cpp +++ b/src/fsfw_hal/host/HostFilesystem.cpp @@ -10,11 +10,12 @@ using namespace std; HostFilesystem::HostFilesystem() = default; -ReturnValue_t HostFilesystem::writeToFile(FileOpParams params, const uint8_t *data) { - if (params.path() == nullptr) { +ReturnValue_t HostFilesystem::writeToFile(const char *path_, size_t offset, const uint8_t *data, + size_t size) { + if (path_ == nullptr) { return returnvalue::FAILED; } - path path(params.path()); + path path(path_); std::error_code e; if (not exists(path, e)) { return HasFileSystemIF::FILE_DOES_NOT_EXIST; @@ -25,17 +26,20 @@ ReturnValue_t HostFilesystem::writeToFile(FileOpParams params, const uint8_t *da if (file.fail()) { return HasFileSystemIF::GENERIC_FILE_ERROR; } - file.seekp(static_cast(params.offset)); - file.write(reinterpret_cast(data), static_cast(params.size)); + file.seekp(static_cast(offset)); + file.write(reinterpret_cast(data), static_cast(size)); return returnvalue::OK; } -ReturnValue_t HostFilesystem::readFromFile(FileOpParams params, uint8_t **buffer, size_t &readSize, - size_t maxSize) { - if (params.path() == nullptr) { +ReturnValue_t HostFilesystem::readFromFile(const char *path_, size_t offset, size_t sizeToRead, + uint8_t *buffer, size_t &actuallyRead, size_t maxSize) { + if (path_ == nullptr) { return returnvalue::FAILED; } - path path(params.path()); + if (sizeToRead > maxSize) { + return SerializeIF::BUFFER_TOO_SHORT; + } + path path(path_); std::error_code e; if (not exists(path, e)) { return HasFileSystemIF::FILE_DOES_NOT_EXIST; @@ -44,23 +48,17 @@ ReturnValue_t HostFilesystem::readFromFile(FileOpParams params, uint8_t **buffer if (file.fail()) { return HasFileSystemIF::GENERIC_FILE_ERROR; } - auto sizeToRead = static_cast(params.size); - file.seekg(static_cast(params.offset)); - if (readSize + sizeToRead > maxSize) { - return SerializeIF::BUFFER_TOO_SHORT; - } - file.read(reinterpret_cast(*buffer), sizeToRead); - readSize += sizeToRead; - *buffer += sizeToRead; + file.seekg(static_cast(offset)); + file.read(reinterpret_cast(buffer), static_cast(sizeToRead)); + actuallyRead = sizeToRead; return returnvalue::OK; } -ReturnValue_t HostFilesystem::createFile(FilesystemParams params, const uint8_t *data, - size_t size) { - if (params.path == nullptr) { +ReturnValue_t HostFilesystem::createFile(const char *path_, const uint8_t *data, size_t size) { + if (path_ == nullptr) { return returnvalue::FAILED; } - path path(params.path); + std::filesystem::path path(path_); std::error_code e; if (exists(path, e)) { return HasFileSystemIF::FILE_ALREADY_EXISTS; @@ -87,11 +85,12 @@ ReturnValue_t HostFilesystem::removeFile(const char *path_, FileSystemArgsIF *ar return HasFileSystemIF::GENERIC_FILE_ERROR; } -ReturnValue_t HostFilesystem::createDirectory(FilesystemParams params, bool createParentDirs) { - if (params.path == nullptr) { +ReturnValue_t HostFilesystem::createDirectory(const char *path, bool createParentDirs, + FileSystemArgsIF *args) { + if (path == nullptr) { return returnvalue::FAILED; } - path dirPath(params.path); + std::filesystem::path dirPath(path); std::error_code e; if (exists(dirPath, e)) { @@ -110,11 +109,12 @@ ReturnValue_t HostFilesystem::createDirectory(FilesystemParams params, bool crea return HasFileSystemIF::GENERIC_DIR_ERROR; } -ReturnValue_t HostFilesystem::removeDirectory(FilesystemParams params, bool deleteRecurively) { - if (params.path == nullptr) { +ReturnValue_t HostFilesystem::removeDirectory(const char *path, bool deleteRecurively, + FileSystemArgsIF *args) { + if (path == nullptr) { return returnvalue::FAILED; } - path dirPath(params.path); + std::filesystem::path dirPath(path); std::error_code e; if (not exists(dirPath, e)) { return HasFileSystemIF::DIRECTORY_DOES_NOT_EXIST; @@ -153,14 +153,14 @@ ReturnValue_t HostFilesystem::rename(const char *oldPath_, const char *newPath_, return returnvalue::OK; } -bool HostFilesystem::fileExists(FilesystemParams params) { - path path(params.path); +bool HostFilesystem::fileExists(const char *path_, FileSystemArgsIF *args) { + std::filesystem::path path(path_); std::error_code e; return filesystem::exists(path, e); } -ReturnValue_t HostFilesystem::truncateFile(FilesystemParams params) { - path path(params.path); +ReturnValue_t HostFilesystem::truncateFile(const char *path_, FileSystemArgsIF *args) { + std::filesystem::path path(path_); std::error_code e; if (not filesystem::exists(path, e)) { return FILE_DOES_NOT_EXIST; @@ -170,12 +170,19 @@ ReturnValue_t HostFilesystem::truncateFile(FilesystemParams params) { return returnvalue::OK; } -bool HostFilesystem::isDirectory(const char *path) { return filesystem::is_directory(path); } +ReturnValue_t HostFilesystem::isDirectory(const char *path, bool &isDirectory) { + isDirectory = filesystem::is_directory(path); + return returnvalue::OK; +} -ReturnValue_t HostFilesystem::getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, +ReturnValue_t HostFilesystem::getBaseFilename(const char *path, char *nameBuf, size_t maxLen, size_t &baseNameLen) { - std::string path(params.path); - std::string baseName = path.substr(path.find_last_of("/\\") + 1); + std::string pathAsStr(path); + size_t lastPos = pathAsStr.find_last_of("/\\"); + if (lastPos == std::string::npos) { + return returnvalue::FAILED; + } + std::string baseName = pathAsStr.substr(pathAsStr.find_last_of("/\\") + 1); if (baseName.size() + 1 > maxLen) { return returnvalue::FAILED; } @@ -185,10 +192,11 @@ ReturnValue_t HostFilesystem::getBaseFilename(FilesystemParams params, char *nam return returnvalue::OK; } -ReturnValue_t HostFilesystem::getFileSize(FilesystemParams params, uint64_t &fileSize) { - if (!fileExists(params)) { +ReturnValue_t HostFilesystem::getFileSize(const char *path, uint64_t &fileSize, + FileSystemArgsIF *args) { + if (!fileExists(path, args)) { return HasFileSystemIF::FILE_DOES_NOT_EXIST; } - fileSize = std::filesystem::file_size(params.path); + fileSize = std::filesystem::file_size(path); return returnvalue::OK; } diff --git a/src/fsfw_hal/host/HostFilesystem.h b/src/fsfw_hal/host/HostFilesystem.h index 5b029abd..2ddea816 100644 --- a/src/fsfw_hal/host/HostFilesystem.h +++ b/src/fsfw_hal/host/HostFilesystem.h @@ -9,19 +9,23 @@ class HostFilesystem : public HasFileSystemIF { public: HostFilesystem(); - ReturnValue_t getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, + ReturnValue_t getBaseFilename(const char *path, char *nameBuf, size_t maxLen, size_t &baseNameLen) override; - virtual ReturnValue_t getFileSize(FilesystemParams params, uint64_t &fileSize) override; - bool isDirectory(const char *path) override; - 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; - ReturnValue_t createFile(FilesystemParams params, const uint8_t *data, size_t size) override; + virtual ReturnValue_t getFileSize(const char *path, uint64_t &fileSize, + FileSystemArgsIF *args) override; + ReturnValue_t isDirectory(const char *path, bool &isDirectory) override; + bool fileExists(const char *path, FileSystemArgsIF *args) override; + ReturnValue_t truncateFile(const char *path, FileSystemArgsIF *args) override; + ReturnValue_t writeToFile(const char *path, size_t offset, const uint8_t *data, + size_t size) override; + ReturnValue_t readFromFile(const char *path, size_t offset, size_t sizeToRead, uint8_t *buffer, + size_t &actuallyRead, size_t maxSize) override; + ReturnValue_t createFile(const char *path, const uint8_t *data, size_t size) override; ReturnValue_t removeFile(const char *path, FileSystemArgsIF *args) override; - ReturnValue_t createDirectory(FilesystemParams params, bool createParentDirs) override; - ReturnValue_t removeDirectory(FilesystemParams params, bool deleteRecurively) override; + ReturnValue_t createDirectory(const char *path, bool createParentDirs, + FileSystemArgsIF *args) override; + ReturnValue_t removeDirectory(const char *path, bool deleteRecurively, + FileSystemArgsIF *args) override; ReturnValue_t rename(const char *oldPath, const char *newPath, FileSystemArgsIF *args) override; std::error_code errorCode; diff --git a/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp b/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp index 701de8f0..df6afa95 100644 --- a/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp +++ b/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp @@ -418,8 +418,8 @@ ReturnValue_t LinuxLibgpioIF::checkForConflictsById(gpioId_t gpioIdToCheck, #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "LinuxLibgpioIF::checkForConflictsRegularGpio: Duplicate GPIO " "definition with ID " - << gpioIdToCheck << " detected. " - << "Duplicate will be removed from map to add" << std::endl; + << gpioIdToCheck << " detected. " << "Duplicate will be removed from map to add" + << std::endl; #else sif::printWarning( "LinuxLibgpioIF::checkForConflictsRegularGpio: Duplicate GPIO definition " diff --git a/src/fsfw_hal/linux/i2c/I2cComIF.cpp b/src/fsfw_hal/linux/i2c/I2cComIF.cpp index 1a85d4d3..436c7f5c 100644 --- a/src/fsfw_hal/linux/i2c/I2cComIF.cpp +++ b/src/fsfw_hal/linux/i2c/I2cComIF.cpp @@ -49,8 +49,7 @@ ReturnValue_t I2cComIF::initializeInterface(CookieIF* cookie) { if (not statusPair.second) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "I2cComIF::initializeInterface: Failed to insert device with address " - << i2cAddress << "to I2C device " - << "map" << std::endl; + << i2cAddress << "to I2C device " << "map" << std::endl; #endif return returnvalue::FAILED; } @@ -91,8 +90,8 @@ ReturnValue_t I2cComIF::sendMessage(CookieIF* cookie, const uint8_t* sendData, s auto i2cDeviceMapIter = i2cDeviceMap.find(i2cAddress); if (i2cDeviceMapIter == i2cDeviceMap.end()) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "I2cComIF::sendMessage: i2cAddress of Cookie not " - << "registered in i2cDeviceMap" << std::endl; + sif::error << "I2cComIF::sendMessage: i2cAddress of Cookie not " << "registered in i2cDeviceMap" + << std::endl; #endif return returnvalue::FAILED; } diff --git a/src/fsfw_hal/linux/spi/SpiComIF.cpp b/src/fsfw_hal/linux/spi/SpiComIF.cpp index f227c685..6787e188 100644 --- a/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -197,9 +197,8 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const if (result == MutexIF::MUTEX_TIMEOUT) { sif::error << "SpiComIF::sendMessage: Lock timeout" << std::endl; } else { - sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " - << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec - << std::endl; + sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " << "0x" << std::hex + << std::setfill('0') << std::setw(4) << result << std::dec << std::endl; } #else sif::printError("SpiComIF::sendMessage: Failed to lock mutex with code %d\n", result); @@ -307,9 +306,8 @@ ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) { if (result != returnvalue::OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " - << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec - << std::endl; + sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " << "0x" << std::hex + << std::setfill('0') << std::setw(4) << result << std::dec << std::endl; #else sif::printError("SpiComIF::sendMessage: Failed to lock mutex with code %d\n", result); #endif diff --git a/src/fsfw_hal/stm32h7/definitions.h b/src/fsfw_hal/stm32h7/definitions.h index 4612d2b7..11a2f26a 100644 --- a/src/fsfw_hal/stm32h7/definitions.h +++ b/src/fsfw_hal/stm32h7/definitions.h @@ -12,10 +12,10 @@ namespace stm32h7 { * and the second entry is the pin number */ struct GpioCfg { - GpioCfg() : port(nullptr), pin(0), altFnc(0){}; + GpioCfg() : port(nullptr), pin(0), altFnc(0) {}; GpioCfg(GPIO_TypeDef* port, uint16_t pin, uint8_t altFnc = 0) - : port(port), pin(pin), altFnc(altFnc){}; + : port(port), pin(pin), altFnc(altFnc) {}; GPIO_TypeDef* port; uint16_t pin; uint8_t altFnc; diff --git a/src/fsfw_hal/stm32h7/spi/mspInit.h b/src/fsfw_hal/stm32h7/spi/mspInit.h index f0658fb9..697f1244 100644 --- a/src/fsfw_hal/stm32h7/spi/mspInit.h +++ b/src/fsfw_hal/stm32h7/spi/mspInit.h @@ -37,7 +37,7 @@ struct MspCfgBase { }; struct MspPollingConfigStruct : public MspCfgBase { - MspPollingConfigStruct() : MspCfgBase(){}; + MspPollingConfigStruct() : MspCfgBase() {}; MspPollingConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, mspCb cleanupCb = nullptr, mspCb setupCb = nullptr) : MspCfgBase(sck, mosi, miso, cleanupCb, setupCb) {} @@ -46,7 +46,7 @@ struct MspPollingConfigStruct : public MspCfgBase { /* A valid instance of this struct must be passed to the MSP initialization function as a void* argument */ struct MspIrqConfigStruct : public MspPollingConfigStruct { - MspIrqConfigStruct() : MspPollingConfigStruct(){}; + MspIrqConfigStruct() : MspPollingConfigStruct() {}; MspIrqConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, mspCb cleanupCb = nullptr, mspCb setupCb = nullptr) : MspPollingConfigStruct(sck, mosi, miso, cleanupCb, setupCb) {} @@ -64,7 +64,7 @@ struct MspIrqConfigStruct : public MspPollingConfigStruct { /* A valid instance of this struct must be passed to the MSP initialization function as a void* argument */ struct MspDmaConfigStruct : public MspIrqConfigStruct { - MspDmaConfigStruct() : MspIrqConfigStruct(){}; + MspDmaConfigStruct() : MspIrqConfigStruct() {}; MspDmaConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, mspCb cleanupCb = nullptr, mspCb setupCb = nullptr) : MspIrqConfigStruct(sck, mosi, miso, cleanupCb, setupCb) {} diff --git a/unittests/cfdp/CMakeLists.txt b/unittests/cfdp/CMakeLists.txt index aba30ede..b028f018 100644 --- a/unittests/cfdp/CMakeLists.txt +++ b/unittests/cfdp/CMakeLists.txt @@ -1,6 +1,7 @@ target_sources( - ${FSFW_TEST_TGT} PRIVATE testCfdp.cpp testOtherTlvs.cpp - testReservedMsgCreator.cpp testTlv.cpp testLvs.cpp) + ${FSFW_TEST_TGT} + PRIVATE testCfdp.cpp testOtherTlvs.cpp testReservedMsgCreator.cpp testTlv.cpp + testLvs.cpp PduSenderMock.cpp) add_subdirectory(handler) add_subdirectory(pdu) diff --git a/unittests/cfdp/PduSenderMock.cpp b/unittests/cfdp/PduSenderMock.cpp new file mode 100644 index 00000000..840e8321 --- /dev/null +++ b/unittests/cfdp/PduSenderMock.cpp @@ -0,0 +1 @@ +#include "PduSenderMock.h" \ No newline at end of file diff --git a/unittests/cfdp/PduSenderMock.h b/unittests/cfdp/PduSenderMock.h new file mode 100644 index 00000000..e2d1b5a9 --- /dev/null +++ b/unittests/cfdp/PduSenderMock.h @@ -0,0 +1,36 @@ +#pragma once +#include +#include + +#include "fsfw/cfdp/handler/PduSenderIF.h" + +struct SentPdu { + cfdp::PduType pduType; + std::optional fileDirective; + std::vector rawPdu; +}; + +class PduSenderMock : public cfdp::PduSenderIF { + public: + ReturnValue_t sendPdu(cfdp::PduType pduType, std::optional fileDirective, + + const uint8_t* pdu, size_t pduSize) override { + SentPdu sentPdu; + sentPdu.pduType = pduType; + sentPdu.fileDirective = fileDirective; + sentPdu.rawPdu = std::vector(pdu, pdu + pduSize); + sentPdus.push_back(sentPdu); + return returnvalue::OK; + } + + std::optional getNextSentPacket() { + if (sentPdus.empty()) { + return std::nullopt; + } + SentPdu nextPdu = sentPdus.front(); + sentPdus.pop_front(); + return nextPdu; + } + + std::deque sentPdus; +}; \ No newline at end of file diff --git a/unittests/cfdp/handler/OwnedPduPacket.h b/unittests/cfdp/handler/OwnedPduPacket.h new file mode 100644 index 00000000..10bb3c14 --- /dev/null +++ b/unittests/cfdp/handler/OwnedPduPacket.h @@ -0,0 +1,31 @@ +#pragma once + +#include +#include + +#include "fsfw/cfdp/definitions.h" +#include "fsfw/cfdp/handler/PduPacketIF.h" + +namespace cfdp { + +class OwnedPduPacket : public PduPacketIF { + public: + OwnedPduPacket(PduType pduType, std::optional fileDirective, const uint8_t* rawPdu, + size_t pduLen) + : pduType(pduType), fileDirective(fileDirective), rawPdu(rawPdu, rawPdu + pduLen) {} + + [[nodiscard]] cfdp::PduType getPduType() const override { return pduType; }; + [[nodiscard]] std::optional getFileDirective() const override { + return fileDirective; + }; + [[nodiscard]] const uint8_t* getRawPduData(size_t& pduLen) const override { + pduLen = rawPdu.size(); + return rawPdu.data(); + } + + PduType pduType; + std::optional fileDirective; + std::vector rawPdu; +}; + +} // namespace cfdp \ No newline at end of file diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp index 4aa2dc32..e94531ce 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -4,6 +4,8 @@ #include #include +#include "OwnedPduPacket.h" +#include "cfdp/PduSenderMock.h" #include "fsfw/cfdp.h" #include "fsfw/cfdp/pdu/EofPduCreator.h" #include "fsfw/cfdp/pdu/FileDataCreator.h" @@ -23,60 +25,49 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { MessageQueueId_t destQueueId = 2; AcceptsTmMock tmReceiver(destQueueId); MessageQueueMock mqMock(destQueueId); - EntityId localId = EntityId(UnsignedByteField(2)); - EntityId remoteId = EntityId(UnsignedByteField(3)); + auto localId = EntityId(UnsignedByteField(2)); + auto remoteId = EntityId(UnsignedByteField(3)); FaultHandlerMock fhMock; LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock); FilesystemMock fsMock; UserMock userMock(fsMock); RemoteConfigTableMock remoteCfgTableMock; - PacketInfoList<64> packetInfoList; LostSegmentsList<128> lostSegmentsList; - DestHandlerParams dp(localEntityCfg, userMock, remoteCfgTableMock, packetInfoList, - lostSegmentsList); + DestHandlerParams dp(localEntityCfg, userMock, remoteCfgTableMock, lostSegmentsList); EventReportingProxyMock eventReporterMock; LocalPool::LocalPoolConfig storeCfg = {{10, 32}, {10, 64}, {10, 128}, {10, 1024}}; - StorageManagerMock tcStore(2, storeCfg); - StorageManagerMock tmStore(3, storeCfg); - FsfwParams fp(tmReceiver, &mqMock, &eventReporterMock); + PduSenderMock senderMock; + FsfwParams fp(&eventReporterMock); RemoteEntityCfg cfg(remoteId); remoteCfgTableMock.addRemoteConfig(cfg); - fp.tcStore = &tcStore; - fp.tmStore = &tmStore; - uint8_t* buf = nullptr; + std::array pduBuf{}; size_t serLen = 0; store_address_t storeId; PduConfig conf; - auto destHandler = DestHandler(dp, fp); - CHECK(destHandler.initialize() == OK); + auto destHandler = DestHandler(senderMock, 4096, dp, fp); auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType) { - std::string srcNameString = "hello.txt"; - std::string destNameString = "hello-cpy.txt"; + const std::string srcNameString = "hello.txt"; + const std::string destNameString = "hello-cpy.txt"; StringLv srcName(srcNameString); StringLv destName(destNameString); MetadataGenericInfo info(false, checksumType, std::move(cfdpFileSize)); - TransactionSeqNum seqNum(UnsignedByteField(1)); + const TransactionSeqNum seqNum(UnsignedByteField(1)); conf.sourceId = remoteId; conf.destId = localId; conf.mode = TransmissionMode::UNACKNOWLEDGED; conf.seqNum = seqNum; - 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, - metadataCreator.getDirectiveCode()); - packetInfoList.push_back(packetInfo); + const MetadataPduCreator metadataCreator(conf, info, srcName, destName, nullptr, 0); + REQUIRE(metadataCreator.serialize(pduBuf.data(), serLen, metadataCreator.getSerializedSize()) == + OK); + return OwnedPduPacket(metadataCreator.getPduType(), metadataCreator.getDirectiveCode(), + pduBuf.data(), serLen); }; auto metadataCheck = [&](const cfdp::DestHandler::FsmResult& res, const char* sourceName, const char* destName, size_t fileLen) { REQUIRE(res.result == OK); - REQUIRE(res.callStatus == CallStatus::CALL_AGAIN); REQUIRE(res.errors == 0); - // Assert that the packet was deleted after handling - REQUIRE(not tcStore.hasDataAtId(storeId)); - REQUIRE(packetInfoList.empty()); REQUIRE(userMock.metadataRecvd.size() == 1); auto& idMetadataPair = userMock.metadataRecvd.back(); REQUIRE(idMetadataPair.first == destHandler.getTransactionId()); @@ -94,10 +85,10 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { auto eofPreparation = [&](Fss cfdpFileSize, uint32_t crc) { EofInfo eofInfo(cfdp::ConditionCode::NO_ERROR, crc, std::move(cfdpFileSize)); EofPduCreator eofCreator(conf, eofInfo); - REQUIRE(tcStore.getFreeElement(&storeId, eofCreator.getSerializedSize(), &buf) == OK); - REQUIRE(eofCreator.serialize(buf, serLen, eofCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(eofCreator.getPduType(), storeId, eofCreator.getDirectiveCode()); - packetInfoList.push_back(packetInfo); + REQUIRE(eofCreator.serialize(pduBuf.data(), serLen, eofCreator.getSerializedSize()) == OK); + OwnedPduPacket packet(eofCreator.getPduType(), eofCreator.getDirectiveCode(), pduBuf.data(), + serLen); + return packet; }; auto eofCheck = [&](const cfdp::DestHandler::FsmResult& res, const TransactionId& id) { @@ -105,9 +96,6 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { REQUIRE(res.state == CfdpState::IDLE); REQUIRE(res.errors == 0); REQUIRE(res.step == DestHandler::TransactionStep::IDLE); - // Assert that the packet was deleted after handling - REQUIRE(not tcStore.hasDataAtId(storeId)); - REQUIRE(packetInfoList.empty()); REQUIRE(userMock.eofRecvdRecvd.size() == 1); auto& eofId = userMock.eofRecvdRecvd.back(); CHECK(eofId == id); @@ -122,10 +110,6 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { REQUIRE(res.result == OK); REQUIRE(res.state == CfdpState::BUSY_CLASS_1_NACKED); REQUIRE(res.step == DestHandler::TransactionStep::RECEIVING_FILE_DATA_PDUS); - for (const auto id : idsToCheck) { - REQUIRE(not tcStore.hasDataAtId(id)); - } - REQUIRE(packetInfoList.empty()); }; SECTION("State") { @@ -134,62 +118,59 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { } SECTION("Idle State Machine Iteration") { - auto res = destHandler.stateMachine(); - CHECK(res.result == OK); - CHECK(res.callStatus == CallStatus::CALL_AFTER_DELAY); - CHECK(res.errors == 0); + auto [packetsSent, result, step, state, errors, errorCodes] = + destHandler.stateMachineNoPacket(); + CHECK(result == OK); + CHECK(errors == 0); CHECK(destHandler.getCfdpState() == CfdpState::IDLE); CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); } SECTION("Empty File Transfer") { - const DestHandler::FsmResult& res = destHandler.stateMachine(); + const DestHandler::FsmResult& res = destHandler.stateMachineNoPacket(); CHECK(res.result == OK); Fss cfdpFileSize(0); - metadataPreparation(cfdpFileSize, ChecksumType::NULL_CHECKSUM); - destHandler.stateMachine(); + auto packet = metadataPreparation(cfdpFileSize, ChecksumType::NULL_CHECKSUM); + destHandler.stateMachine(packet); metadataCheck(res, "hello.txt", "hello-cpy.txt", 0); - destHandler.stateMachine(); - REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); + destHandler.stateMachine(packet); + // REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); auto transactionId = destHandler.getTransactionId(); - eofPreparation(cfdpFileSize, 0); + auto ownedEofPacket = eofPreparation(cfdpFileSize, 0); // After EOF, operation is done because no closure was requested - destHandler.stateMachine(); + destHandler.stateMachine(ownedEofPacket); eofCheck(res, transactionId); } SECTION("Small File Transfer") { - const DestHandler::FsmResult& res = destHandler.stateMachine(); + const DestHandler::FsmResult& res = destHandler.stateMachineNoPacket(); CHECK(res.result == OK); std::string fileData = "hello test data"; etl::crc32 crcCalc; crcCalc.add(fileData.begin(), fileData.end()); uint32_t crc32 = crcCalc.value(); Fss cfdpFileSize(fileData.size()); - metadataPreparation(cfdpFileSize, ChecksumType::CRC_32); - destHandler.stateMachine(); + auto metadataPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32); + destHandler.stateMachine(metadataPacket); metadataCheck(res, "hello.txt", "hello-cpy.txt", fileData.size()); - destHandler.stateMachine(); - REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); + destHandler.stateMachineNoPacket(); auto transactionId = destHandler.getTransactionId(); Fss offset(0); FileDataInfo fdPduInfo(offset, reinterpret_cast(fileData.data()), fileData.size()); FileDataCreator fdPduCreator(conf, fdPduInfo); - REQUIRE(tcStore.getFreeElement(&storeId, fdPduCreator.getSerializedSize(), &buf) == OK); - REQUIRE(fdPduCreator.serialize(buf, serLen, fdPduCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(fdPduCreator.getPduType(), storeId, std::nullopt); - packetInfoList.push_back(packetInfo); - destHandler.stateMachine(); + REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == OK); + OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen); + destHandler.stateMachine(fdPdu); fileDataPduCheck(res, {storeId}); - eofPreparation(cfdpFileSize, crc32); + auto eofPacket = eofPreparation(cfdpFileSize, crc32); // After EOF, operation is done because no closure was requested - destHandler.stateMachine(); + destHandler.stateMachine(eofPacket); eofCheck(res, transactionId); } SECTION("Segmented File Transfer") { - const DestHandler::FsmResult& res = destHandler.stateMachine(); + const DestHandler::FsmResult& res = destHandler.stateMachineNoPacket(); CHECK(res.result == OK); std::random_device dev; std::mt19937 rng(dev()); @@ -202,43 +183,42 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { crcCalc.add(largerFileData.begin(), largerFileData.end()); uint32_t crc32 = crcCalc.value(); Fss cfdpFileSize(largerFileData.size()); - metadataPreparation(cfdpFileSize, ChecksumType::CRC_32); - destHandler.stateMachine(); + auto metaPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32); + destHandler.stateMachine(metaPacket); metadataCheck(res, "hello.txt", "hello-cpy.txt", largerFileData.size()); - destHandler.stateMachine(); - REQUIRE(res.callStatus == CallStatus::CALL_AFTER_DELAY); + destHandler.stateMachineNoPacket(); auto transactionId = destHandler.getTransactionId(); std::vector idsToCheck; { Fss offset(0); - FileDataInfo fdPduInfo(offset, reinterpret_cast(largerFileData.data()), + FileDataInfo fdPduInfo(offset, (largerFileData.data()), largerFileData.size() / 2); FileDataCreator fdPduCreator(conf, fdPduInfo); - REQUIRE(tcStore.getFreeElement(&storeId, fdPduCreator.getSerializedSize(), &buf) == OK); - REQUIRE(fdPduCreator.serialize(buf, serLen, fdPduCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(fdPduCreator.getPduType(), storeId, std::nullopt); + REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == + OK); idsToCheck.push_back(storeId); - packetInfoList.push_back(packetInfo); + OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen); + destHandler.stateMachine(fdPdu); } { Fss offset(512); - FileDataInfo fdPduInfo(offset, reinterpret_cast(largerFileData.data() + 512), + FileDataInfo fdPduInfo(offset, largerFileData.data() + 512, largerFileData.size() / 2); FileDataCreator fdPduCreator(conf, fdPduInfo); - REQUIRE(tcStore.getFreeElement(&storeId, fdPduCreator.getSerializedSize(), &buf) == OK); - REQUIRE(fdPduCreator.serialize(buf, serLen, fdPduCreator.getSerializedSize()) == OK); - PacketInfo packetInfo(fdPduCreator.getPduType(), storeId, std::nullopt); + REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == + OK); idsToCheck.push_back(storeId); - packetInfoList.push_back(packetInfo); + OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen); + destHandler.stateMachine(fdPdu); } - destHandler.stateMachine(); + destHandler.stateMachineNoPacket(); fileDataPduCheck(res, idsToCheck); - eofPreparation(cfdpFileSize, crc32); + auto eofPacket = eofPreparation(cfdpFileSize, crc32); // After EOF, operation is done because no closure was requested - destHandler.stateMachine(); + destHandler.stateMachine(eofPacket); eofCheck(res, transactionId); } } \ No newline at end of file diff --git a/unittests/cfdp/handler/testReservedMsgParser.cpp b/unittests/cfdp/handler/testReservedMsgParser.cpp index e87dc06d..85aad8a2 100644 --- a/unittests/cfdp/handler/testReservedMsgParser.cpp +++ b/unittests/cfdp/handler/testReservedMsgParser.cpp @@ -1,3 +1,4 @@ +#include #include #include "fsfw/cfdp/CfdpMessage.h" diff --git a/unittests/cfdp/handler/testSourceHandler.cpp b/unittests/cfdp/handler/testSourceHandler.cpp index 7fbec9a1..72859db9 100644 --- a/unittests/cfdp/handler/testSourceHandler.cpp +++ b/unittests/cfdp/handler/testSourceHandler.cpp @@ -4,6 +4,7 @@ #include #include +#include "cfdp/PduSenderMock.h" #include "fsfw/cfdp.h" #include "fsfw/cfdp/handler/PutRequest.h" #include "fsfw/cfdp/handler/SourceHandler.h" @@ -27,13 +28,13 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { using namespace cfdp; using namespace returnvalue; using namespace std::filesystem; - const size_t MAX_FILE_SEGMENT_SIZE = 255; + constexpr size_t MAX_FILE_SEGMENT_SIZE = 255; MessageQueueId_t destQueueId = 2; AcceptsTmMock tmReceiver(destQueueId); - MessageQueueMock mqMock(destQueueId); - EntityId localId = EntityId(UnsignedByteField(2)); - EntityId remoteId = EntityId(UnsignedByteField(5)); + // MessageQueueMock mqMock(destQueueId); + auto localId = EntityId(UnsignedByteField(2)); + auto remoteId = EntityId(UnsignedByteField(5)); FaultHandlerMock fhMock; LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock); FilesystemMock fsMock; @@ -41,49 +42,37 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { SeqCountProviderU16 seqCountProvider; SourceHandlerParams dp(localEntityCfg, userMock, seqCountProvider); + PduSenderMock pduSender; EventReportingProxyMock eventReporterMock; LocalPool::LocalPoolConfig storeCfg = {{10, 32}, {10, 64}, {10, 128}, {10, 1024}}; - StorageManagerMock tcStore(2, storeCfg); - StorageManagerMock tmStore(3, storeCfg); - FsfwParams fp(tmReceiver, &mqMock, &eventReporterMock); - fp.tcStore = &tcStore; - fp.tmStore = &tmStore; - auto sourceHandler = SourceHandler(dp, fp); + FsfwParams fp(&eventReporterMock); + auto sourceHandler = SourceHandler(pduSender, 4096, dp, fp); RemoteEntityCfg cfg; cfg.maxFileSegmentLen = MAX_FILE_SEGMENT_SIZE; cfg.remoteId = remoteId; 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)); + fsMock.createFile(srcFileName.c_str()); + cfdp::StringLv srcNameLv(srcFileName.c_str(), srcFileName.length()); FilesystemParams destFileNameFs(destFileName.c_str()); cfdp::StringLv destNameLv(destFileNameFs.path, std::strlen(destFileNameFs.path)); PutRequest putRequest(remoteId, srcNameLv, destNameLv); CHECK(sourceHandler.initialize() == OK); - auto onePduSentCheck = [&](const SourceHandler::FsmResult& fsmResult, TmTcMessage& tmtcMessage, - const uint8_t** pduPtr) { + auto onePduSentCheck = [&](const SourceHandler::FsmResult& fsmResult) { CHECK(fsmResult.errors == 0); CHECK(fsmResult.packetsSent == 1); - CHECK(mqMock.numberOfSentMessages() == 1); - REQUIRE(mqMock.getNextSentMessage(destQueueId, tmtcMessage) == OK); - auto accessor = tmStore.getData(tmtcMessage.getStorageId()); - REQUIRE(accessor.first == OK); - *pduPtr = accessor.second.data(); - return std::move(accessor); }; auto genericMetadataCheck = [&](const SourceHandler::FsmResult& fsmResult, size_t expectedFileSize, uint16_t expectedSeqNum) { CHECK(fsmResult.errors == 0); - CHECK(fsmResult.callStatus == CallStatus::CALL_AGAIN); - TmTcMessage tmtcMessage; - const uint8_t* pduPtr; - auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); - CHECK(accessor.second.size() == 55); + auto optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; + CHECK(rawPdu.size() == 55); MetadataGenericInfo metadataInfo; - MetadataPduReader metadataReader(pduPtr, accessor.second.size(), metadataInfo, nullptr, 0); + MetadataPduReader metadataReader(rawPdu.data(), rawPdu.size(), metadataInfo, nullptr, 0); REQUIRE(metadataReader.parseData() == OK); std::string srcNameRead = metadataReader.getSourceFileName().getString(); CHECK(srcNameRead == srcFileName); @@ -107,20 +96,19 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { } CHECK(metadataInfo.getFileSize().value() == expectedFileSize); CHECK(!metadataInfo.isClosureRequested()); - mqMock.clearMessages(); }; auto genericEofCheck = [&](const SourceHandler::FsmResult& fsmResult, size_t expectedFileSize, uint32_t expectedChecksum, uint16_t expectedSeqNum) { - CHECK(fsmResult.errors == 0); - CHECK(fsmResult.callStatus == CallStatus::CALL_AGAIN); + onePduSentCheck(fsmResult); TmTcMessage tmtcMessage; - const uint8_t* pduPtr; - auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); + auto optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; // 10 byte PDU header, 1 byte directive field, 1 byte condition code, 4 byte checksum, // 4 byte FSS - CHECK(accessor.second.size() == 20); + CHECK(rawPdu.size() == 20); EofInfo eofInfo; - EofPduReader eofReader(pduPtr, accessor.second.size(), eofInfo); + EofPduReader eofReader(rawPdu.data(), rawPdu.size(), eofInfo); REQUIRE(eofReader.parseData() == OK); TransactionSeqNum seqNum; eofReader.getTransactionSeqNum(seqNum); @@ -133,8 +121,7 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { CHECK(eofInfo.getFileSize().value() == expectedFileSize); }; auto genericNoticeOfCompletionCheck = [&](const SourceHandler::FsmResult& fsmResult, - uint16_t expectedSeqNum) { - CHECK(fsmResult.callStatus == CallStatus::DONE); + const uint16_t expectedSeqNum) { CHECK(userMock.finishedRecvd.size() == 1); CHECK(userMock.finishedRecvd.back().first == TransactionId(localId, TransactionSeqNum(cfdp::WidthInBytes::TWO_BYTES, expectedSeqNum))); @@ -166,11 +153,11 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { SECTION("Transfer small file") { uint16_t expectedSeqNum = 0; - fsMock.createFile(srcFileNameFs); + fsMock.createFile(srcFileName.c_str()); std::string fileContent = "hello world\n"; size_t expectedFileSize = fileContent.size(); - FileOpParams params(srcFileName.c_str(), expectedFileSize); - fsMock.writeToFile(params, reinterpret_cast(fileContent.data())); + fsMock.writeToFile(srcFileName.c_str(), 0, reinterpret_cast(fileContent.data()), + expectedFileSize); CHECK(sourceHandler.transactionStart(putRequest, cfg) == OK); const SourceHandler::FsmResult& fsmResult = sourceHandler.stateMachine(); @@ -179,13 +166,15 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { // Verify that a small file data PDU was sent. sourceHandler.stateMachine(); - TmTcMessage tmtcMessage; - const uint8_t* pduPtr; - auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); + + onePduSentCheck(fsmResult); + auto optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; FileDataInfo fdInfo; - FileDataReader fdReader(pduPtr, accessor.second.size(), fdInfo); + FileDataReader fdReader(rawPdu.data(), rawPdu.size(), fdInfo); // 10 byte PDU header, 4 byte offset, 12 bytes file data. - CHECK(accessor.second.size() == 26); + CHECK(rawPdu.size() == 26); CHECK(fdReader.parseData() == OK); CHECK(fdInfo.getOffset().value() == 0); size_t fileSize = 0; @@ -194,7 +183,6 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { CHECK(fileData != nullptr); std::string dataReadBack(reinterpret_cast(fileData), fileSize); CHECK(dataReadBack == fileContent); - mqMock.clearMessages(); sourceHandler.stateMachine(); @@ -220,9 +208,8 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { val = distU8(rng); } size_t expectedFileSize = largerFileData.size(); - fsMock.createFile(srcFileNameFs); - FileOpParams params(srcFileName.c_str(), expectedFileSize); - fsMock.writeToFile(params, reinterpret_cast(largerFileData.data())); + fsMock.createFile(srcFileName.c_str()); + fsMock.writeToFile(srcFileName.c_str(), 0, largerFileData.data(), largerFileData.size()); CHECK(sourceHandler.transactionStart(putRequest, cfg) == OK); const SourceHandler::FsmResult& fsmResult = sourceHandler.stateMachine(); // Verify metadata PDU was sent. @@ -230,15 +217,17 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { // Check first file data PDU. It should have the maximum file segment size. sourceHandler.stateMachine(); - TmTcMessage tmtcMessage; - const uint8_t* pduPtr; + + onePduSentCheck(fsmResult); + auto optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; FileDataInfo fdInfo; { - CHECK(fsmResult.callStatus == CallStatus::CALL_AGAIN); - auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); - FileDataReader fdReader(pduPtr, accessor.second.size(), fdInfo); + // auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); + FileDataReader fdReader(rawPdu.data(), rawPdu.size(), fdInfo); // 10 byte PDU header, 4 byte offset, 255 byte file data - CHECK(accessor.second.size() == 269); + CHECK(rawPdu.size() == 269); CHECK(fdReader.parseData() == OK); CHECK(fdInfo.getOffset().value() == 0); size_t fileSize = 0; @@ -249,16 +238,16 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { CHECK(fileData[i] == largerFileData[i]); } } - mqMock.clearMessages(); // Check second file data PDU. sourceHandler.stateMachine(); { - CHECK(fsmResult.callStatus == CallStatus::CALL_AGAIN); - auto accessor = onePduSentCheck(fsmResult, tmtcMessage, &pduPtr); - FileDataReader fdReader(pduPtr, accessor.second.size(), fdInfo); + optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; + FileDataReader fdReader(rawPdu.data(), rawPdu.size(), fdInfo); // 10 byte PDU header, 4 byte offset, remaining file data (400 - 255 == 145). - CHECK(accessor.second.size() == 10 + 4 + largerFileData.size() - MAX_FILE_SEGMENT_SIZE); + CHECK(rawPdu.size() == 10 + 4 + largerFileData.size() - MAX_FILE_SEGMENT_SIZE); CHECK(fdReader.parseData() == OK); CHECK(fdInfo.getOffset().value() == MAX_FILE_SEGMENT_SIZE); size_t fileDataSize = 0; @@ -269,7 +258,6 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { CHECK(fileData[i] == largerFileData[MAX_FILE_SEGMENT_SIZE + i]); } } - mqMock.clearMessages(); // Check EOF and verify checksum. sourceHandler.stateMachine(); diff --git a/unittests/cfdp/pdu/testCfdpHeader.cpp b/unittests/cfdp/pdu/testCfdpHeader.cpp index 447e3722..ed2be09d 100644 --- a/unittests/cfdp/pdu/testCfdpHeader.cpp +++ b/unittests/cfdp/pdu/testCfdpHeader.cpp @@ -4,7 +4,7 @@ #include "fsfw/cfdp/pdu/FinishedInfo.h" #include "fsfw/cfdp/pdu/FinishedPduCreator.h" #include "fsfw/cfdp/pdu/HeaderCreator.h" -#include "fsfw/cfdp/pdu/PduHeaderReader.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/returnvalues/returnvalue.h" using namespace returnvalue; diff --git a/unittests/container/TestArrayList.cpp b/unittests/container/TestArrayList.cpp index 0c327685..eaf64af0 100644 --- a/unittests/container/TestArrayList.cpp +++ b/unittests/container/TestArrayList.cpp @@ -13,8 +13,8 @@ TEST_CASE("Array List", "[containers]") { ArrayList list(20); struct TestClass { public: - TestClass(){}; - TestClass(uint32_t number1, uint64_t number2) : number1(number1), number2(number2){}; + TestClass() {}; + TestClass(uint32_t number1, uint64_t number2) : number1(number1), number2(number2) {}; uint32_t number1 = -1; uint64_t number2 = -1; bool operator==(const TestClass& other) { diff --git a/unittests/container/TestFixedOrderedMultimap.cpp b/unittests/container/TestFixedOrderedMultimap.cpp index cadb03e6..eedb20fa 100644 --- a/unittests/container/TestFixedOrderedMultimap.cpp +++ b/unittests/container/TestFixedOrderedMultimap.cpp @@ -97,9 +97,9 @@ TEST_CASE("FixedOrderedMultimap Non Trivial Type", "[TestFixedOrderedMultimapNon class TestClass { public: - TestClass(){}; - TestClass(uint32_t number1, uint64_t number2) : number1(number1), number2(number2){}; - ~TestClass(){}; + TestClass() {}; + TestClass(uint32_t number1, uint64_t number2) : number1(number1), number2(number2) {}; + ~TestClass() {}; bool operator==(const TestClass& lhs) { return ((this->number1 == lhs.number1) and (this->number2 == lhs.number2)); diff --git a/unittests/hal/testFsMock.cpp b/unittests/hal/testFsMock.cpp index 2ebcd231..ac69441a 100644 --- a/unittests/hal/testFsMock.cpp +++ b/unittests/hal/testFsMock.cpp @@ -10,7 +10,7 @@ TEST_CASE("Filesystem Mock", "[mocks]") { SECTION("Create File") { FilesystemParams params("hello.txt"); - CHECK(fsMock.createFile(params) == returnvalue::OK); + CHECK(fsMock.createFile("hello.txt") == returnvalue::OK); auto iter = fsMock.fileMap.find("hello.txt"); REQUIRE(iter != fsMock.fileMap.end()); FilesystemMock::FileInfo &stats = iter->second; @@ -21,8 +21,8 @@ TEST_CASE("Filesystem Mock", "[mocks]") { SECTION("Write to File") { std::string testData = "test data"; FileOpParams params("hello.txt", testData.size()); - CHECK(fsMock.writeToFile(params, reinterpret_cast(testData.data())) == - returnvalue::OK); + CHECK(fsMock.writeToFile("hello.txt", 0, reinterpret_cast(testData.data()), + testData.size()) == returnvalue::OK); auto iter = fsMock.fileMap.find("hello.txt"); REQUIRE(iter != fsMock.fileMap.end()); FilesystemMock::FileInfo &stats = iter->second; @@ -38,7 +38,7 @@ TEST_CASE("Filesystem Mock", "[mocks]") { SECTION("Create Directory") { FilesystemParams params("hello"); - CHECK(fsMock.createDirectory(params) == returnvalue::OK); + CHECK(fsMock.createDirectory("hello") == returnvalue::OK); REQUIRE(not fsMock.dirMap.empty()); auto iter = fsMock.dirMap.find("hello"); REQUIRE(iter != fsMock.dirMap.end()); diff --git a/unittests/hal/testHostFilesystem.cpp b/unittests/hal/testHostFilesystem.cpp index e33b30cc..0b1ffc0e 100644 --- a/unittests/hal/testHostFilesystem.cpp +++ b/unittests/hal/testHostFilesystem.cpp @@ -25,15 +25,13 @@ TEST_CASE("Host Filesystem", "[hal][host]") { REQUIRE_NOTHROW(fs::remove_all(dir0)); SECTION("Create file") { - FilesystemParams params(file0.c_str()); - REQUIRE(hostFs.createFile(params) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); REQUIRE(fs::exists(file0)); } SECTION("Remove File") { - FilesystemParams params(file0.c_str()); - REQUIRE(hostFs.createFile(params) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); REQUIRE(fs::exists(file0)); REQUIRE(hostFs.removeFile(file0.c_str()) == returnvalue::OK); @@ -41,23 +39,20 @@ TEST_CASE("Host Filesystem", "[hal][host]") { } SECTION("Create Directory") { - FilesystemParams params(dir0.c_str()); - REQUIRE(hostFs.createDirectory(params) == returnvalue::OK); + REQUIRE(hostFs.createDirectory(dir0.c_str()) == returnvalue::OK); CHECK(fs::is_directory(dir0)); REQUIRE(fs::exists(dir0)); } SECTION("Remove Directory") { - FilesystemParams params(dir0.c_str()); - REQUIRE(hostFs.createDirectory(params) == returnvalue::OK); + REQUIRE(hostFs.createDirectory(dir0.c_str()) == returnvalue::OK); REQUIRE(fs::exists(dir0)); - REQUIRE(hostFs.removeDirectory(params) == returnvalue::OK); + REQUIRE(hostFs.removeDirectory(dir0.c_str(), false, nullptr) == returnvalue::OK); REQUIRE(not fs::exists(dir0)); } SECTION("Rename File") { - FilesystemParams params(file0.c_str()); - REQUIRE(hostFs.createFile(params) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); REQUIRE(fs::exists(file0)); REQUIRE(hostFs.rename(file0.c_str(), file1.c_str()) == returnvalue::OK); @@ -65,12 +60,11 @@ TEST_CASE("Host Filesystem", "[hal][host]") { SECTION("Write To File") { std::string data = "hello world!"; - FileOpParams params(file0.c_str(), data.size()); - REQUIRE(hostFs.createFile(params.fsParams) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); REQUIRE(fs::exists(file0)); - CHECK(hostFs.writeToFile(params, reinterpret_cast(data.c_str())) == - returnvalue::OK); + CHECK(hostFs.writeToFile(file0.c_str(), 0, reinterpret_cast(data.c_str()), + data.size()) == returnvalue::OK); CHECK(fs::file_size(file0) == data.size()); ifstream ifile(file0); std::array readBuf{}; @@ -87,14 +81,13 @@ TEST_CASE("Host Filesystem", "[hal][host]") { for (uint8_t& byte : randData) { byte = distU8(rng); } - FileOpParams params(file0.c_str(), randData.size() - 256); - REQUIRE(hostFs.createFile(params.fsParams) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); REQUIRE(fs::exists(file0)); // Write first file chunk - CHECK(hostFs.writeToFile(params, randData.cbegin()) == returnvalue::OK); - params.offset = 256; - CHECK(hostFs.writeToFile(params, randData.cbegin() + 256) == returnvalue::OK); + CHECK(hostFs.writeToFile(file0.c_str(), 0, randData.cbegin(), randData.size() - 256) == + returnvalue::OK); + CHECK(hostFs.writeToFile(file0.c_str(), 256, randData.cbegin() + 256, 256) == returnvalue::OK); std::ifstream rf(file0, ios::binary); std::array readBack{}; REQUIRE(std::filesystem::file_size(file0) == 512); @@ -106,8 +99,7 @@ TEST_CASE("Host Filesystem", "[hal][host]") { SECTION("Read From File") { std::string data = "hello world!"; - FileOpParams params(file0.c_str(), data.size()); - REQUIRE(hostFs.createFile(params.fsParams) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); CHECK(fs::is_regular_file(file0)); ofstream of(file0); of.write(data.c_str(), static_cast(data.size())); @@ -116,8 +108,10 @@ TEST_CASE("Host Filesystem", "[hal][host]") { REQUIRE(fs::exists(file0)); std::array readBuf{}; uint8_t* readPtr = readBuf.data(); - size_t readSize = 0; - CHECK(hostFs.readFromFile(params, &readPtr, readSize, readBuf.size()) == returnvalue::OK); + size_t readSize = data.size(); + size_t actuallyRead = 0; + CHECK(hostFs.readFromFile(file0.c_str(), 0, readSize, readPtr, actuallyRead, readBuf.size()) == + returnvalue::OK); std::string readBackString(reinterpret_cast(readBuf.data())); CHECK(readSize == data.size()); CHECK(data == readBackString); @@ -125,21 +119,21 @@ TEST_CASE("Host Filesystem", "[hal][host]") { SECTION("Invalid Input does not crash") { FileOpParams params(nullptr, 10); - REQUIRE(hostFs.createFile(params.fsParams) != returnvalue::OK); - REQUIRE(hostFs.createDirectory(params.fsParams) != returnvalue::OK); - REQUIRE(hostFs.createFile(params.fsParams) != returnvalue::OK); - REQUIRE(hostFs.removeDirectory(params.fsParams) != returnvalue::OK); + REQUIRE(hostFs.createFile(nullptr) != returnvalue::OK); + REQUIRE(hostFs.createDirectory(nullptr) != returnvalue::OK); + REQUIRE(hostFs.createFile(nullptr) != returnvalue::OK); + REQUIRE(hostFs.removeDirectory(nullptr, false, nullptr) != returnvalue::OK); REQUIRE(hostFs.removeFile(nullptr) != returnvalue::OK); REQUIRE(hostFs.rename(nullptr, nullptr) != returnvalue::OK); - REQUIRE(hostFs.writeToFile(params, nullptr) != returnvalue::OK); + REQUIRE(hostFs.writeToFile(nullptr, 0, nullptr, 0) != returnvalue::OK); size_t readLen = 0; - REQUIRE(hostFs.readFromFile(params, nullptr, readLen, 20) != returnvalue::OK); + REQUIRE(hostFs.readFromFile(nullptr, 0, 0, nullptr, readLen, 20) != returnvalue::OK); } SECTION("Create File but already exists") { FilesystemParams params(file0.c_str()); - REQUIRE(hostFs.createFile(params) == returnvalue::OK); - REQUIRE(hostFs.createFile(params) == HasFileSystemIF::FILE_ALREADY_EXISTS); + REQUIRE(hostFs.createFile(file0.c_str()) == returnvalue::OK); + REQUIRE(hostFs.createFile(file0.c_str()) == HasFileSystemIF::FILE_ALREADY_EXISTS); } SECTION("Remove File but does not exist") { @@ -148,36 +142,35 @@ TEST_CASE("Host Filesystem", "[hal][host]") { SECTION("Create Directory but already exists") { FileOpParams params(file0.c_str(), 12); - REQUIRE(hostFs.createDirectory(params.fsParams) == returnvalue::OK); - REQUIRE(hostFs.createDirectory(params.fsParams) == HasFileSystemIF::DIRECTORY_ALREADY_EXISTS); + REQUIRE(hostFs.createDirectory(file0.c_str()) == returnvalue::OK); + REQUIRE(hostFs.createDirectory(file0.c_str()) == HasFileSystemIF::DIRECTORY_ALREADY_EXISTS); } SECTION("Remove Directory but does not exist") { FilesystemParams params(dir0.c_str()); - REQUIRE(hostFs.removeDirectory(params) == HasFileSystemIF::DIRECTORY_DOES_NOT_EXIST); + REQUIRE(hostFs.removeDirectory(dir0.c_str(), false, nullptr) == + HasFileSystemIF::DIRECTORY_DOES_NOT_EXIST); } SECTION("Remove Directory but is file") { ofstream of(file0); - FilesystemParams params(file0.c_str()); - REQUIRE(hostFs.removeDirectory(params) == HasFileSystemIF::NOT_A_DIRECTORY); + REQUIRE(hostFs.removeDirectory(file0.c_str(), false, nullptr) == + HasFileSystemIF::NOT_A_DIRECTORY); } SECTION("Read from file but does not exist") { std::string data = "hello world!"; - FileOpParams params(file0.c_str(), data.size()); - std::array readBuf{}; + std::array readBuf{}; uint8_t* readPtr = readBuf.data(); size_t readSize = 0; - CHECK(hostFs.readFromFile(params, &readPtr, readSize, readBuf.size()) == + CHECK(hostFs.readFromFile(file0.c_str(), 0, data.size(), readPtr, readSize, readBuf.size()) == HasFileSystemIF::FILE_DOES_NOT_EXIST); } SECTION("Write to file but does not exist") { std::string data = "hello world!"; - FileOpParams params(file0.c_str(), data.size()); - CHECK(hostFs.writeToFile(params, reinterpret_cast(data.c_str())) == - HasFileSystemIF::FILE_DOES_NOT_EXIST); + CHECK(hostFs.writeToFile(file0.c_str(), 0, reinterpret_cast(data.c_str()), + data.size()) == HasFileSystemIF::FILE_DOES_NOT_EXIST); } SECTION("Remove recursively") { @@ -185,7 +178,7 @@ TEST_CASE("Host Filesystem", "[hal][host]") { ofstream of(fileInDir0); CHECK(fs::is_directory(dir0)); CHECK(fs::is_regular_file(fileInDir0)); - REQUIRE(hostFs.removeDirectory(FilesystemParams(dir0.c_str()), true) == returnvalue::OK); + REQUIRE(hostFs.removeDirectory(dir0.c_str(), true, nullptr) == returnvalue::OK); CHECK(not fs::is_directory(dir0)); CHECK(not fs::is_regular_file(fileInDir0)); } @@ -195,12 +188,12 @@ TEST_CASE("Host Filesystem", "[hal][host]") { ofstream of(fileInDir0); CHECK(fs::is_directory(dir0)); CHECK(fs::is_regular_file(fileInDir0)); - REQUIRE(hostFs.removeDirectory(FilesystemParams(dir0.c_str())) == + REQUIRE(hostFs.removeDirectory(dir0.c_str(), false, nullptr) == HasFileSystemIF::DIRECTORY_NOT_EMPTY); } SECTION("Create directory with parent directory") { - CHECK(hostFs.createDirectory(FilesystemParams(dirWithParent.c_str()), true) == returnvalue::OK); + CHECK(hostFs.createDirectory(dirWithParent.c_str(), true, nullptr) == returnvalue::OK); CHECK(fs::is_directory(dir0)); CHECK(fs::is_directory(dirWithParent)); } @@ -216,10 +209,9 @@ TEST_CASE("Host Filesystem", "[hal][host]") { std::array readBuf{}; uint8_t* readPtr = readBuf.data(); size_t readSize = 0; - CHECK(hostFs.readFromFile(params, &readPtr, readSize, 5) == SerializeIF::BUFFER_TOO_SHORT); - readSize = 10; - CHECK(hostFs.readFromFile(params, &readPtr, readSize, readBuf.size()) == + CHECK(hostFs.readFromFile(file0.c_str(), 0, data.size(), readPtr, readSize, 5) == SerializeIF::BUFFER_TOO_SHORT); + ; } REQUIRE_NOTHROW(fs::remove(file0)); diff --git a/unittests/mocks/DeviceHandlerMock.cpp b/unittests/mocks/DeviceHandlerMock.cpp index 3ad80048..1242d010 100644 --- a/unittests/mocks/DeviceHandlerMock.cpp +++ b/unittests/mocks/DeviceHandlerMock.cpp @@ -4,9 +4,7 @@ DeviceHandlerMock::DeviceHandlerMock(object_id_t objectId, object_id_t deviceCommunication, CookieIF *comCookie, FailureIsolationBase *fdirInstance) - : DeviceHandlerBase(objectId, deviceCommunication, comCookie, fdirInstance) { - setMode(MODE_ON); -} + : DeviceHandlerBase(objectId, deviceCommunication, comCookie, fdirInstance) {} DeviceHandlerMock::~DeviceHandlerMock() = default; @@ -101,3 +99,12 @@ ReturnValue_t DeviceHandlerMock::enablePeriodicReply(DeviceCommandId_t replyId) ReturnValue_t DeviceHandlerMock::disablePeriodicReply(DeviceCommandId_t replyId) { return updatePeriodicReply(false, replyId); } + +ReturnValue_t DeviceHandlerMock::initialize() { + ReturnValue_t result = DeviceHandlerBase::initialize(); + if (result != returnvalue::OK) { + return result; + } + setMode(MODE_ON); + return result; +} diff --git a/unittests/mocks/DeviceHandlerMock.h b/unittests/mocks/DeviceHandlerMock.h index ef1649c3..b659857b 100644 --- a/unittests/mocks/DeviceHandlerMock.h +++ b/unittests/mocks/DeviceHandlerMock.h @@ -21,6 +21,8 @@ class DeviceHandlerMock : public DeviceHandlerBase { ReturnValue_t enablePeriodicReply(DeviceCommandId_t replyId); ReturnValue_t disablePeriodicReply(DeviceCommandId_t replyId); + ReturnValue_t initialize() override; + protected: void doStartUp() override; void doShutDown() override; diff --git a/unittests/mocks/FilesystemMock.cpp b/unittests/mocks/FilesystemMock.cpp index 51c0c686..da728912 100644 --- a/unittests/mocks/FilesystemMock.cpp +++ b/unittests/mocks/FilesystemMock.cpp @@ -1,60 +1,58 @@ #include "FilesystemMock.h" +#include #include #include "fsfw/serialize/SerializeIF.h" -ReturnValue_t FilesystemMock::feedFile(const std::string &filename, std::ifstream &file) { +ReturnValue_t FilesystemMock::feedFile(const std::string& filename, std::ifstream& file) { if (not std::filesystem::exists(filename)) { return returnvalue::FAILED; } size_t fileSize = std::filesystem::file_size(filename); FileOpParams params(filename.c_str(), fileSize); std::vector rawData(fileSize); - file.read(reinterpret_cast(rawData.data()), static_cast(rawData.size())); - createOrAddToFile(params, rawData.data()); + file.read(reinterpret_cast(rawData.data()), static_cast(rawData.size())); + createOrAddToFile(filename.data(), 0, rawData.data(), rawData.size()); return returnvalue::OK; } -ReturnValue_t FilesystemMock::writeToFile(FileOpParams params, const uint8_t *data) { - createOrAddToFile(params, data); +ReturnValue_t FilesystemMock::writeToFile(const char* path, size_t offset, const uint8_t* data, + size_t size) { + createOrAddToFile(path, offset, data, size); return returnvalue::OK; } -ReturnValue_t FilesystemMock::readFromFile(FileOpParams params, uint8_t **buffer, size_t &readSize, - size_t maxSize) { - std::string filename(params.path()); - auto iter = fileMap.find(filename); +ReturnValue_t FilesystemMock::readFromFile(const char* path, size_t offset, size_t size, + uint8_t* buffer, size_t& readSize, size_t maxSize) { + const std::string filename(path); + const auto iter = fileMap.find(filename); if (iter == fileMap.end()) { return HasFileSystemIF::FILE_DOES_NOT_EXIST; - } else { - FileInfo &info = iter->second; - size_t readLen = params.size; - if (params.offset + params.size > info.fileRaw.size()) { - if (params.offset > info.fileRaw.size()) { - return returnvalue::OK; - } - readLen = info.fileRaw.size() - params.offset; - } - if (readSize + readLen > maxSize) { - return SerializeIF::STREAM_TOO_SHORT; - } - std::copy(info.fileRaw.data() + params.offset, info.fileRaw.data() + params.offset + readLen, - *buffer); - *buffer += readLen; - readSize += readLen; } + auto& [fileSegQueue, fileRaw] = iter->second; + size_t readLen = size; + if (offset + size > fileRaw.size()) { + if (offset > fileRaw.size()) { + return returnvalue::OK; + } + readLen = fileRaw.size() - offset; + } + if (readSize + readLen > maxSize) { + return SerializeIF::STREAM_TOO_SHORT; + } + // std::copy(info.fileRaw.data() + offset, info.fileRaw.data() + offset + readLen, *buffer); + std::memcpy(buffer, fileRaw.data() + offset, readLen); + readSize = readLen; return returnvalue::OK; } -ReturnValue_t FilesystemMock::createFile(FilesystemParams params, const uint8_t *data, - size_t size) { - FileOpParams params2(params.path, size); - createOrAddToFile(params2, data); +ReturnValue_t FilesystemMock::createFile(const char* path, const uint8_t* data, size_t size) { + createOrAddToFile(path, 0, data, size); return returnvalue::OK; } -ReturnValue_t FilesystemMock::removeFile(const char *path, FileSystemArgsIF *args) { +ReturnValue_t FilesystemMock::removeFile(const char* path, FileSystemArgsIF* args) { std::string filename(path); auto iter = fileMap.find(filename); if (iter == fileMap.end()) { @@ -65,56 +63,58 @@ ReturnValue_t FilesystemMock::removeFile(const char *path, FileSystemArgsIF *arg } } -ReturnValue_t FilesystemMock::createDirectory(FilesystemParams params, bool createParentDirs) { - std::string dirPath = params.path; +ReturnValue_t FilesystemMock::createDirectory(const char* path, bool createParentDirs, + FileSystemArgsIF* args) { + std::string dirPath = path; dirMap[dirPath].createCallCount++; dirMap[dirPath].wihParentDir.push(createParentDirs); return returnvalue::OK; } -ReturnValue_t FilesystemMock::removeDirectory(FilesystemParams params, bool deleteRecurively) { - std::string dirPath = params.path; +ReturnValue_t FilesystemMock::removeDirectory(const char* path, bool deleteRecurively, + FileSystemArgsIF* args) { + std::string dirPath = path; dirMap[dirPath].delCallCount++; dirMap[dirPath].recursiveDeletion.push(deleteRecurively); return returnvalue::OK; } -ReturnValue_t FilesystemMock::rename(const char *oldPath, const char *newPath, - FileSystemArgsIF *args) { +ReturnValue_t FilesystemMock::rename(const char* oldPath, const char* newPath, + FileSystemArgsIF* args) { renameQueue.emplace(oldPath, newPath); return returnvalue::OK; } -void FilesystemMock::createOrAddToFile(FileOpParams params, const uint8_t *data) { - std::string filename(params.path()); +void FilesystemMock::createOrAddToFile(const char* path, size_t offset, const uint8_t* data, + size_t dataSize) { + std::string filename(path); auto iter = fileMap.find(filename); if (iter == fileMap.end()) { FileSegmentQueue queue; - if (params.size > 0) { - queue.emplace(filename, params.offset, data, params.size); + if (dataSize > 0) { + queue.emplace(filename, offset, data, dataSize); } FileInfo info; info.fileSegQueue = queue; if (data != nullptr) { - info.fileRaw.insert(info.fileRaw.end(), data, data + params.size); + info.fileRaw.insert(info.fileRaw.end(), data, data + dataSize); } fileMap.emplace(filename, info); } else { - FileInfo &info = iter->second; - info.fileSegQueue.emplace(filename, params.offset, data, params.size); + FileInfo& info = iter->second; + info.fileSegQueue.emplace(filename, offset, data, dataSize); if (data == nullptr) { return; } // Easiest case: append data to the end - if (params.offset == info.fileRaw.size()) { - info.fileRaw.insert(info.fileRaw.end(), data, data + params.size); + if (offset == info.fileRaw.size()) { + info.fileRaw.insert(info.fileRaw.end(), data, data + dataSize); } else { - size_t totalNewLen = params.offset + params.size; + size_t totalNewLen = offset + dataSize; if (totalNewLen > info.fileRaw.size()) { - info.fileRaw.resize(params.offset + params.size); + info.fileRaw.resize(offset + dataSize); } - std::copy(data, data + params.size, - info.fileRaw.begin() + static_cast(params.offset)); + std::copy(data, data + dataSize, info.fileRaw.begin() + static_cast(offset)); } } } @@ -126,8 +126,8 @@ void FilesystemMock::reset() { std::swap(renameQueue, empty); } -bool FilesystemMock::fileExists(FilesystemParams params) { - std::string filename(params.path); +bool FilesystemMock::fileExists(const char* path, FileSystemArgsIF* args) { + std::string filename(path); auto iter = fileMap.find(filename); if (iter == fileMap.end()) { return false; @@ -135,20 +135,21 @@ bool FilesystemMock::fileExists(FilesystemParams params) { return true; } -ReturnValue_t FilesystemMock::truncateFile(FilesystemParams params) { - truncateCalledOnFile = params.path; +ReturnValue_t FilesystemMock::truncateFile(const char* path, FileSystemArgsIF* args) { + truncateCalledOnFile = path; return returnvalue::OK; } -ReturnValue_t FilesystemMock::getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, - size_t &baseNameLen) { +ReturnValue_t FilesystemMock::getBaseFilename(const char* path, char* nameBuf, size_t maxLen, + size_t& baseNameLen) { return returnvalue::OK; } -bool FilesystemMock::isDirectory(const char *path) { return false; } +ReturnValue_t FilesystemMock::isDirectory(const char* path, bool& isDirectory) { return false; } -bool FilesystemMock::getFileSize(FilesystemParams params, size_t &fileSize) { - std::string filename(params.path); +ReturnValue_t FilesystemMock::getFileSize(const char* path, uint64_t& fileSize, + FileSystemArgsIF* args) { + std::string filename(path); auto iter = fileMap.find(filename); if (iter != fileMap.end()) { fileSize = iter->second.fileRaw.size(); diff --git a/unittests/mocks/FilesystemMock.h b/unittests/mocks/FilesystemMock.h index b24d5c4c..c5bcdf7a 100644 --- a/unittests/mocks/FilesystemMock.h +++ b/unittests/mocks/FilesystemMock.h @@ -20,7 +20,7 @@ class FilesystemMock : public HasFileSystemIF { public: struct FileWriteInfo { - FileWriteInfo(std::string filename, size_t offset, const uint8_t *data, size_t len) + FileWriteInfo(std::string filename, size_t offset, const uint8_t* data, size_t len) : filename(std::move(filename)), offset(offset) { this->data.insert(this->data.end(), data, data + len); } @@ -54,24 +54,34 @@ class FilesystemMock : public HasFileSystemIF { }; std::queue renameQueue; std::string truncateCalledOnFile; - ReturnValue_t feedFile(const std::string &filename, std::ifstream &file); + ReturnValue_t feedFile(const std::string& filename, std::ifstream& file); - ReturnValue_t getBaseFilename(FilesystemParams params, char *nameBuf, size_t maxLen, - size_t &baseNameLen) override; + ReturnValue_t getBaseFilename(const char* path, char* nameBuf, size_t maxLen, + size_t& baseNameLen) override; - bool isDirectory(const char *path) override; - bool fileExists(FilesystemParams params) override; - ReturnValue_t truncateFile(FilesystemParams params) override; - bool getFileSize(FilesystemParams params, size_t &fileSize) override; + ReturnValue_t isDirectory(const char* path, bool& isDirectory) 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; - ReturnValue_t createFile(FilesystemParams params, const uint8_t *data, size_t size) override; - ReturnValue_t removeFile(const char *path, FileSystemArgsIF *args) override; - ReturnValue_t createDirectory(FilesystemParams params, bool createParentDirs) override; - ReturnValue_t removeDirectory(FilesystemParams params, bool deleteRecurively) override; - ReturnValue_t rename(const char *oldPath, const char *newPath, FileSystemArgsIF *args) override; + bool fileExists(const char* path, FileSystemArgsIF* args) override; + + ReturnValue_t truncateFile(const char* path, FileSystemArgsIF* args) override; + + ReturnValue_t getFileSize(const char* path, uint64_t& fileSize, FileSystemArgsIF* args) override; + + ReturnValue_t writeToFile(const char* path, size_t offset, const uint8_t* data, + size_t size) override; + + ReturnValue_t readFromFile(const char* path, size_t offset, size_t size, uint8_t* buffer, + size_t& readSize, size_t maxSize) override; + + ReturnValue_t createFile(const char* path, const uint8_t* data, size_t size) override; + ReturnValue_t removeFile(const char* path, FileSystemArgsIF* args) override; + + ReturnValue_t createDirectory(const char* path, bool createParentDirs, + FileSystemArgsIF* args) override; + + ReturnValue_t removeDirectory(const char* path, bool deleteRecurively, + FileSystemArgsIF* args) override; + ReturnValue_t rename(const char* oldPath, const char* newPath, FileSystemArgsIF* args) override; void reset(); @@ -80,7 +90,7 @@ class FilesystemMock : public HasFileSystemIF { using HasFileSystemIF::readFromFile; private: - void createOrAddToFile(FileOpParams params, const uint8_t *data); + void createOrAddToFile(const char* path, size_t offset, const uint8_t* data, size_t dataSize); }; #endif // FSFW_MOCKS_FILESYSTEMMOCK_H diff --git a/unittests/tmtcpacket/testCcsdsCreator.cpp b/unittests/tmtcpacket/testCcsdsCreator.cpp index cd9b3e97..a3895c3e 100644 --- a/unittests/tmtcpacket/testCcsdsCreator.cpp +++ b/unittests/tmtcpacket/testCcsdsCreator.cpp @@ -82,13 +82,15 @@ TEST_CASE("CCSDS Creator", "[ccsds-creator]") { CHECK(buf[5] == 0xFF); } - SECTION("Setting data length 0 is ignored") { + SECTION("Setting data length 0") { SpacePacketCreator creator = SpacePacketCreator( ccsds::PacketType::TC, true, 0xFFFF, ccsds::SequenceFlags::FIRST_SEGMENT, 0x34, 0x22); creator.setCcsdsLenFromTotalDataFieldLen(0); - REQUIRE(creator.getPacketDataLen() == 0x22); + // Will result in field length 0. + REQUIRE(creator.getPacketDataLen() == 0); + // Will also result in field length 0. creator.setCcsdsLenFromTotalDataFieldLen(1); - REQUIRE(creator.getPacketDataLen() == 0x00); + REQUIRE(creator.getPacketDataLen() == 0); } SECTION("Invalid APID") {