From bdbe0cc9da5edcaa8b01af4f6462a2f46d4628bd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 16 Sep 2022 16:27:57 +0200 Subject: [PATCH 1/8] pass message queue externally --- src/fsfw/cfdp/handler/CfdpHandler.cpp | 13 ++++++------- src/fsfw/cfdp/handler/CfdpHandler.h | 8 +++++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/fsfw/cfdp/handler/CfdpHandler.cpp b/src/fsfw/cfdp/handler/CfdpHandler.cpp index 1ea5501b..9d20cc5e 100644 --- a/src/fsfw/cfdp/handler/CfdpHandler.cpp +++ b/src/fsfw/cfdp/handler/CfdpHandler.cpp @@ -11,15 +11,14 @@ using namespace cfdp; CfdpHandler::CfdpHandler(const FsfwHandlerParams& fsfwParams, const CfdpHandlerCfg& cfdpCfg) : SystemObject(fsfwParams.objectId), + msgQueue(fsfwParams.msgQueue), destHandler( DestHandlerParams(LocalEntityCfg(cfdpCfg.id, cfdpCfg.indicCfg, cfdpCfg.faultHandler), cfdpCfg.userHandler, cfdpCfg.remoteCfgProvider, cfdpCfg.packetInfoList, cfdpCfg.lostSegmentsList), FsfwParams(fsfwParams.packetDest, nullptr, this, fsfwParams.tcStore, - fsfwParams.tmStore)) { - // TODO: Make queue params configurable, or better yet, expect it to be passed externally - msgQueue = QueueFactory::instance()->createMessageQueue(); - destHandler.setMsgQueue(*msgQueue); + fsfwParams.tmStore)) { + destHandler.setMsgQueue(msgQueue); } [[nodiscard]] const char* CfdpHandler::getName() const { return "CFDP Handler"; } @@ -28,7 +27,7 @@ CfdpHandler::CfdpHandler(const FsfwHandlerParams& fsfwParams, const CfdpHandlerC return destHandler.getDestHandlerParams().cfg.localId.getValue(); } -[[nodiscard]] MessageQueueId_t CfdpHandler::getRequestQueue() const { return msgQueue->getId(); } +[[nodiscard]] MessageQueueId_t CfdpHandler::getRequestQueue() const { return msgQueue.getId(); } ReturnValue_t CfdpHandler::initialize() { ReturnValue_t result = destHandler.initialize(); @@ -47,8 +46,8 @@ ReturnValue_t CfdpHandler::performOperation(uint8_t operationCode) { ReturnValue_t status; ReturnValue_t result = OK; TmTcMessage tmtcMsg; - for (status = msgQueue->receiveMessage(&tmtcMsg); status == returnvalue::OK; - status = msgQueue->receiveMessage(&tmtcMsg)) { + for (status = msgQueue.receiveMessage(&tmtcMsg); status == returnvalue::OK; + status = msgQueue.receiveMessage(&tmtcMsg)) { result = handleCfdpPacket(tmtcMsg); if (result != OK) { status = result; diff --git a/src/fsfw/cfdp/handler/CfdpHandler.h b/src/fsfw/cfdp/handler/CfdpHandler.h index d7472905..147ffc70 100644 --- a/src/fsfw/cfdp/handler/CfdpHandler.h +++ b/src/fsfw/cfdp/handler/CfdpHandler.h @@ -11,13 +11,15 @@ struct FsfwHandlerParams { FsfwHandlerParams(object_id_t objectId, HasFileSystemIF& vfs, AcceptsTelemetryIF& packetDest, - StorageManagerIF& tcStore, StorageManagerIF& tmStore) - : objectId(objectId), vfs(vfs), packetDest(packetDest), tcStore(tcStore), tmStore(tmStore) {} + StorageManagerIF& tcStore, StorageManagerIF& tmStore, MessageQueueIF& msgQueue) + : objectId(objectId), vfs(vfs), packetDest(packetDest), tcStore(tcStore), tmStore(tmStore), + msgQueue(msgQueue) {} object_id_t objectId{}; HasFileSystemIF& vfs; AcceptsTelemetryIF& packetDest; StorageManagerIF& tcStore; StorageManagerIF& tmStore; + MessageQueueIF& msgQueue; }; struct CfdpHandlerCfg { @@ -54,7 +56,7 @@ class CfdpHandler : public SystemObject, public ExecutableObjectIF, public Accep ReturnValue_t performOperation(uint8_t operationCode) override; private: - MessageQueueIF* msgQueue = nullptr; + MessageQueueIF& msgQueue; cfdp::DestHandler destHandler; StorageManagerIF* tcStore = nullptr; StorageManagerIF* tmStore = nullptr; From e893e73f867efe7471371fa79ff1cf01e561aa31 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 12:19:31 +0200 Subject: [PATCH 2/8] add first CFDP events --- src/fsfw/cfdp/handler/CMakeLists.txt | 5 +++-- src/fsfw/cfdp/handler/CfdpHandler.cpp | 2 +- src/fsfw/cfdp/handler/CfdpHandler.h | 6 +++++- src/fsfw/cfdp/handler/DestHandler.cpp | 13 ++++++++++++- src/fsfw/cfdp/handler/defs.h | 12 +++++++++++- src/fsfw/events/fwSubsystemIdRanges.h | 1 + unittests/mocks/cfdp/FaultHandlerMock.h | 2 +- 7 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/fsfw/cfdp/handler/CMakeLists.txt b/src/fsfw/cfdp/handler/CMakeLists.txt index 2e7dceef..d96ce91c 100644 --- a/src/fsfw/cfdp/handler/CMakeLists.txt +++ b/src/fsfw/cfdp/handler/CMakeLists.txt @@ -1,2 +1,3 @@ -target_sources(${LIB_FSFW_NAME} PRIVATE SourceHandler.cpp DestHandler.cpp - FaultHandlerBase.cpp UserBase.cpp CfdpHandler.cpp) +target_sources( + ${LIB_FSFW_NAME} PRIVATE SourceHandler.cpp DestHandler.cpp + FaultHandlerBase.cpp UserBase.cpp CfdpHandler.cpp) diff --git a/src/fsfw/cfdp/handler/CfdpHandler.cpp b/src/fsfw/cfdp/handler/CfdpHandler.cpp index 9d20cc5e..902097b6 100644 --- a/src/fsfw/cfdp/handler/CfdpHandler.cpp +++ b/src/fsfw/cfdp/handler/CfdpHandler.cpp @@ -17,7 +17,7 @@ CfdpHandler::CfdpHandler(const FsfwHandlerParams& fsfwParams, const CfdpHandlerC cfdpCfg.userHandler, cfdpCfg.remoteCfgProvider, cfdpCfg.packetInfoList, cfdpCfg.lostSegmentsList), FsfwParams(fsfwParams.packetDest, nullptr, this, fsfwParams.tcStore, - fsfwParams.tmStore)) { + fsfwParams.tmStore)) { destHandler.setMsgQueue(msgQueue); } diff --git a/src/fsfw/cfdp/handler/CfdpHandler.h b/src/fsfw/cfdp/handler/CfdpHandler.h index 147ffc70..2de9f7dd 100644 --- a/src/fsfw/cfdp/handler/CfdpHandler.h +++ b/src/fsfw/cfdp/handler/CfdpHandler.h @@ -12,7 +12,11 @@ struct FsfwHandlerParams { FsfwHandlerParams(object_id_t objectId, HasFileSystemIF& vfs, AcceptsTelemetryIF& packetDest, StorageManagerIF& tcStore, StorageManagerIF& tmStore, MessageQueueIF& msgQueue) - : objectId(objectId), vfs(vfs), packetDest(packetDest), tcStore(tcStore), tmStore(tmStore), + : objectId(objectId), + vfs(vfs), + packetDest(packetDest), + tcStore(tcStore), + tmStore(tmStore), msgQueue(msgQueue) {} object_id_t objectId{}; HasFileSystemIF& vfs; diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 8049cc0a..13b93a36 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -427,22 +427,33 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { ReturnValue_t result = fp.tcStore->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 // TODO: Error handling and event, this is a non CFDP specific error (most likely store is full) return result; } size_t serLen = 0; result = finishedPdu.serialize(dataPtr, serLen, finishedPdu.getSerializedSize()); if (result != OK) { - // TODO: Error printout, this really should not happen +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler::sendFinishedPdu: Serializing Finished PDU failed" + << std::endl; +#endif + // TODO: Event, non CFDP specific error return result; } TmTcMessage msg(storeId); result = fp.msgQueue->sendMessage(fp.packetDest.getReportReceptionQueue(), &msg); if (result != OK) { // TODO: Error handling and event, this is a non CFDP specific error (most likely store is full) +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler::sendFinishedPdu: Sending PDU failed" << std::endl; +#endif return result; } fsmRes.packetsSent++; + sif::info << "Sent finished PDU" << std::endl; return OK; } diff --git a/src/fsfw/cfdp/handler/defs.h b/src/fsfw/cfdp/handler/defs.h index 9e837a96..190fb67d 100644 --- a/src/fsfw/cfdp/handler/defs.h +++ b/src/fsfw/cfdp/handler/defs.h @@ -5,5 +5,15 @@ namespace cfdp { enum class CfdpStates { IDLE, BUSY_CLASS_1_NACKED, BUSY_CLASS_2_ACKED, SUSPENDED }; -} +static constexpr uint8_t SSID = SUBSYSTEM_ID::CFDP; + +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 SERIALIZATION_ERROR = event::makeEvent(SSID, 2, severity::LOW); + +} // namespace events + +} // namespace cfdp #endif // FSFW_CFDP_HANDLER_DEFS_H diff --git a/src/fsfw/events/fwSubsystemIdRanges.h b/src/fsfw/events/fwSubsystemIdRanges.h index d8e4ade6..574ea070 100644 --- a/src/fsfw/events/fwSubsystemIdRanges.h +++ b/src/fsfw/events/fwSubsystemIdRanges.h @@ -33,6 +33,7 @@ enum : uint8_t { PUS_SERVICE_23 = 103, MGM_LIS3MDL = 106, MGM_RM3100 = 107, + CFDP = 108, FW_SUBSYSTEM_ID_RANGE }; diff --git a/unittests/mocks/cfdp/FaultHandlerMock.h b/unittests/mocks/cfdp/FaultHandlerMock.h index 1c59485c..5e094509 100644 --- a/unittests/mocks/cfdp/FaultHandlerMock.h +++ b/unittests/mocks/cfdp/FaultHandlerMock.h @@ -17,7 +17,7 @@ class FaultHandlerMock : public FaultHandlerBase { void noticeOfSuspensionCb(TransactionId& id, ConditionCode code) override; void noticeOfCancellationCb(TransactionId& id, ConditionCode code) override; - void abandonCb(TransactionId& id,ConditionCode code) override; + void abandonCb(TransactionId& id, ConditionCode code) override; void ignoreCb(TransactionId& id, ConditionCode code) override; FaultInfo& getFhInfo(FaultHandlerCode fhCode); From 79c38b45df4647a8748c97a491467472275a6a51 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 12:20:26 +0200 Subject: [PATCH 3/8] events for FSFW specific errors --- src/fsfw/cfdp/handler/DestHandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 13b93a36..92b4341a 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -430,7 +430,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler:sendFinishedPdu: Getting store slot failed" << std::endl; #endif - // TODO: Error handling and event, this is a non CFDP specific error (most likely store is full) + fp.eventReporter->forwardEvent(events::STORE_ERROR, result, 0); return result; } size_t serLen = 0; @@ -440,7 +440,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { sif::warning << "cfdp::DestHandler::sendFinishedPdu: Serializing Finished PDU failed" << std::endl; #endif - // TODO: Event, non CFDP specific error + fp.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0); return result; } TmTcMessage msg(storeId); From 9f81926aeccf65149db02bf58d67326e7ca10c63 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 12:22:01 +0200 Subject: [PATCH 4/8] some more basic error handling --- src/fsfw/cfdp/handler/DestHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 92b4341a..2e59179b 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -446,10 +446,10 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { TmTcMessage msg(storeId); result = fp.msgQueue->sendMessage(fp.packetDest.getReportReceptionQueue(), &msg); if (result != OK) { - // TODO: Error handling and event, this is a non CFDP specific error (most likely store is full) #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler::sendFinishedPdu: Sending PDU failed" << std::endl; #endif + fp.eventReporter->forwardEvent(events::MSG_QUEUE_ERROR, result, 0); return result; } fsmRes.packetsSent++; From 14a8924a83dc15e29cd4bf8c6ac495c8729e4828 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 14:01:21 +0200 Subject: [PATCH 5/8] size check bugfix --- src/fsfw/cfdp/pdu/FinishedPduCreator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp b/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp index 8ac22e0a..d002e8aa 100644 --- a/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp +++ b/src/fsfw/cfdp/pdu/FinishedPduCreator.cpp @@ -17,7 +17,7 @@ ReturnValue_t FinishPduCreator::serialize(uint8_t **buffer, size_t *size, size_t if (result != returnvalue::OK) { return result; } - if (*size + 1 >= maxSize) { + if (*size + 1 > maxSize) { return SerializeIF::BUFFER_TOO_SHORT; } **buffer = finishInfo.getConditionCode() << 4 | finishInfo.getDeliveryCode() << 2 | From 1aa062df7fc43135b8e45115eef45d45afca5a81 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 16:27:41 +0200 Subject: [PATCH 6/8] const specifier for AcceptsTelemetryIF --- src/fsfw/cfdp/handler/DestHandler.cpp | 2 +- src/fsfw/tmtcservices/AcceptsTelemetryIF.h | 4 ++-- src/fsfw/tmtcservices/TmTcBridge.cpp | 2 +- src/fsfw/tmtcservices/TmTcBridge.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 2e59179b..86219fb7 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -425,7 +425,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { store_address_t storeId; uint8_t* dataPtr = nullptr; ReturnValue_t result = - fp.tcStore->getFreeElement(&storeId, finishedPdu.getSerializedSize(), &dataPtr); + fp.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; diff --git a/src/fsfw/tmtcservices/AcceptsTelemetryIF.h b/src/fsfw/tmtcservices/AcceptsTelemetryIF.h index c3e3eff3..0c5621c8 100644 --- a/src/fsfw/tmtcservices/AcceptsTelemetryIF.h +++ b/src/fsfw/tmtcservices/AcceptsTelemetryIF.h @@ -21,9 +21,9 @@ class AcceptsTelemetryIF { * receiving message queue. * @return The telemetry reception message queue id. */ - virtual MessageQueueId_t getReportReceptionQueue(uint8_t virtualChannel) = 0; + [[nodiscard]] virtual MessageQueueId_t getReportReceptionQueue(uint8_t virtualChannel) const = 0; - virtual MessageQueueId_t getReportReceptionQueue() { return getReportReceptionQueue(0); } + [[nodiscard]] virtual MessageQueueId_t getReportReceptionQueue() const { return getReportReceptionQueue(0); } }; #endif /* FSFW_TMTCSERVICES_ACCEPTSTELEMETRYIF_H_ */ diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index 0e9f0da4..f22d70d6 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -240,7 +240,7 @@ void TmTcBridge::registerCommDisconnect() { } } -MessageQueueId_t TmTcBridge::getReportReceptionQueue(uint8_t virtualChannel) { +MessageQueueId_t TmTcBridge::getReportReceptionQueue(uint8_t virtualChannel) const { return tmTcReceptionQueue->getId(); } diff --git a/src/fsfw/tmtcservices/TmTcBridge.h b/src/fsfw/tmtcservices/TmTcBridge.h index 4b90d1d5..d74c612b 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.h +++ b/src/fsfw/tmtcservices/TmTcBridge.h @@ -65,7 +65,7 @@ class TmTcBridge : public AcceptsTelemetryIF, ReturnValue_t performOperation(uint8_t operationCode = 0) override; /** AcceptsTelemetryIF override */ - MessageQueueId_t getReportReceptionQueue(uint8_t virtualChannel = 0) override; + MessageQueueId_t getReportReceptionQueue(uint8_t virtualChannel = 0) const override; /** AcceptsTelecommandsIF override */ uint32_t getIdentifier() const override; From 1e43296f2b127093c8ba5e18164fb361b6df7bb3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 17:24:46 +0200 Subject: [PATCH 7/8] missing validity check --- src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp b/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp index b8607079..745d5834 100644 --- a/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp +++ b/src/fsfw/tmtcpacket/ccsds/SpacePacketCreator.cpp @@ -14,6 +14,7 @@ SpacePacketCreator::SpacePacketCreator(ccsds::PacketType packetType, bool secHea : params(SpacePacketParams(PacketId(packetType, secHeaderFlag, apid), PacketSeqCtrl(seqFlags, seqCount), dataLen)) { params.version = version; + checkFieldValidity(); } uint16_t SpacePacketCreator::getPacketIdRaw() const { return params.packetId.raw(); } From 009700ce801641921fd713c7fe668e27d8a7c653 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 17 Oct 2022 17:29:10 +0200 Subject: [PATCH 8/8] remove info printout --- src/fsfw/cfdp/handler/DestHandler.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 86219fb7..b4d4af09 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -453,7 +453,6 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { return result; } fsmRes.packetsSent++; - sif::info << "Sent finished PDU" << std::endl; return OK; }