diff --git a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp index ca22c904..67c2b4b8 100644 --- a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp +++ b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp @@ -14,6 +14,7 @@ #include "internal/HasLocalDpIFManagerAttorney.h" #include "internal/LocalPoolDataSetAttorney.h" +// TODO: Get rid of this. This should be a constructor argument, not something hardcoded in any way object_id_t LocalDataPoolManager::defaultHkDestination = objects::PUS_SERVICE_3_HOUSEKEEPING; LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, MessageQueueIF* queueToUse, @@ -325,28 +326,32 @@ void LocalDataPoolManager::resetHkUpdateResetHelper() { } } -ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, bool enableReporting, - float collectionInterval, - bool isDiagnostics, - object_id_t packetDestination) { +ReturnValue_t LocalDataPoolManager::subscribeForRegularPeriodicPacket( + subdp::RegularHkPeriodicParams params) { + return subscribeForPeriodicPacket(params); +} + +ReturnValue_t LocalDataPoolManager::subscribeForDiagPeriodicPacket( + subdp::DiagnosticsHkPeriodicParams params) { + return subscribeForPeriodicPacket(params); +} + +ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(subdp::ParamsBase& params) { struct HkReceiver hkReceiver; - hkReceiver.dataId.sid = sid; + hkReceiver.dataId.sid = params.sid; hkReceiver.reportingType = ReportingType::PERIODIC; hkReceiver.dataType = DataType::DATA_SET; - if (packetDestination != objects::NO_OBJECT) { - auto* receivedHkIF = ObjectManager::instance()->get(packetDestination); - if (receivedHkIF->getHkQueue() == MessageQueueIF::NO_QUEUE) { - hkReceiver.destinationQueue = hkDestinationId; - } else { - hkReceiver.destinationQueue = receivedHkIF->getHkQueue(); - } + if (params.receiver == MessageQueueIF::NO_QUEUE) { + hkReceiver.destinationQueue = hkDestinationId; + } else { + hkReceiver.destinationQueue = params.receiver; } - LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); + LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, params.sid); if (dataSet != nullptr) { - LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, enableReporting); - LocalPoolDataSetAttorney::setDiagnostic(*dataSet, isDiagnostics); - LocalPoolDataSetAttorney::initializePeriodicHelper(*dataSet, collectionInterval, + LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, params.enableReporting); + LocalPoolDataSetAttorney::setDiagnostic(*dataSet, params.isDiagnostics()); + LocalPoolDataSetAttorney::initializePeriodicHelper(*dataSet, params.collectionInterval, owner->getPeriodicOperationFrequency()); } @@ -354,26 +359,30 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, bool e return returnvalue::OK; } -ReturnValue_t LocalDataPoolManager::subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, - bool isDiagnostics, - object_id_t packetDestination) { +ReturnValue_t LocalDataPoolManager::subscribeForRegularUpdatePacket( + subdp::RegularHkUpdateParams params) { + return subscribeForUpdatePacket(params); +} +ReturnValue_t LocalDataPoolManager::subscribeForDiagUpdatePacket( + subdp::DiagnosticsHkUpdateParams params) { + return subscribeForUpdatePacket(params); +} + +ReturnValue_t LocalDataPoolManager::subscribeForUpdatePacket(subdp::ParamsBase& params) { struct HkReceiver hkReceiver; - hkReceiver.dataId.sid = sid; + hkReceiver.dataId.sid = params.sid; hkReceiver.reportingType = ReportingType::UPDATE_HK; hkReceiver.dataType = DataType::DATA_SET; - if (packetDestination != objects::NO_OBJECT) { - auto* receivedHkIF = ObjectManager::instance()->get(packetDestination); - if (receivedHkIF->getHkQueue() == MessageQueueIF::NO_QUEUE) { - hkReceiver.destinationQueue = hkDestinationId; - } else { - hkReceiver.destinationQueue = receivedHkIF->getHkQueue(); - } + if (params.receiver == MessageQueueIF::NO_QUEUE) { + hkReceiver.destinationQueue = hkDestinationId; + } else { + hkReceiver.destinationQueue = params.receiver; } - LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, sid); + LocalPoolDataSetBase* dataSet = HasLocalDpIFManagerAttorney::getDataSetHandle(owner, params.sid); if (dataSet != nullptr) { LocalPoolDataSetAttorney::setReportingEnabled(*dataSet, true); - LocalPoolDataSetAttorney::setDiagnostic(*dataSet, isDiagnostics); + LocalPoolDataSetAttorney::setDiagnostic(*dataSet, params.isDiagnostics()); } hkReceivers.push_back(hkReceiver); diff --git a/src/fsfw/datapoollocal/LocalDataPoolManager.h b/src/fsfw/datapoollocal/LocalDataPoolManager.h index bbbc6b72..2f1476ab 100644 --- a/src/fsfw/datapoollocal/LocalDataPoolManager.h +++ b/src/fsfw/datapoollocal/LocalDataPoolManager.h @@ -241,13 +241,13 @@ class LocalDataPoolManager : public ProvidesDataPoolSubscriptionIF, public Acces MutexIF* getMutexHandle(); LocalDataPoolManager* getPoolManagerHandle() override; - ReturnValue_t subscribeForPeriodicPacket( - sid_t sid, bool enableReporting, float collectionInterval, bool isDiagnostics, - object_id_t packetDestination = objects::NO_OBJECT) override; + ReturnValue_t subscribeForRegularPeriodicPacket(subdp::RegularHkPeriodicParams params) override; + ReturnValue_t subscribeForDiagPeriodicPacket(subdp::DiagnosticsHkPeriodicParams params) override; + ReturnValue_t subscribeForPeriodicPacket(subdp::ParamsBase& params); - ReturnValue_t subscribeForUpdatePacket( - sid_t sid, bool reportingEnabled, bool isDiagnostics, - object_id_t packetDestination = objects::NO_OBJECT) override; + ReturnValue_t subscribeForRegularUpdatePacket(subdp::RegularHkUpdateParams params) override; + ReturnValue_t subscribeForDiagUpdatePacket(subdp::DiagnosticsHkUpdateParams params) override; + ReturnValue_t subscribeForUpdatePacket(subdp::ParamsBase& params); protected: ReturnValue_t subscribeForPeriodicPacket(subdp::ParamsBase& params); diff --git a/src/fsfw/datapoollocal/ProvidesDataPoolSubscriptionIF.h b/src/fsfw/datapoollocal/ProvidesDataPoolSubscriptionIF.h index eb4e55b7..baf91796 100644 --- a/src/fsfw/datapoollocal/ProvidesDataPoolSubscriptionIF.h +++ b/src/fsfw/datapoollocal/ProvidesDataPoolSubscriptionIF.h @@ -1,24 +1,90 @@ #ifndef FSFW_DATAPOOLLOCAL_PROVIDESDATAPOOLSUBSCRIPTION_H_ #define FSFW_DATAPOOLLOCAL_PROVIDESDATAPOOLSUBSCRIPTION_H_ -#include "../ipc/messageQueueDefinitions.h" -#include "../returnvalues/returnvalue.h" +#include "fsfw/housekeeping/AcceptsHkPacketsIF.h" +#include "fsfw/ipc/MessageQueueIF.h" +#include "fsfw/ipc/messageQueueDefinitions.h" +#include "fsfw/returnvalues/returnvalue.h" #include "localPoolDefinitions.h" +namespace subdp { + +struct ParamsBase { + ParamsBase(sid_t sid, bool enableReporting, float collectionInterval, bool diagnostics) + : sid(sid), + enableReporting(enableReporting), + collectionInterval(collectionInterval), + diagnostics(diagnostics) {} + + [[nodiscard]] bool isDiagnostics() const { return diagnostics; } + + sid_t sid; + bool enableReporting; + float collectionInterval; + MessageQueueId_t receiver = MessageQueueIF::NO_QUEUE; + + protected: + bool diagnostics; +}; + +struct RegularHkPeriodicParams : public ParamsBase { + RegularHkPeriodicParams(sid_t sid, bool enableReporting, float collectionInterval) + : ParamsBase(sid, enableReporting, collectionInterval, false) {} +}; + +struct DiagnosticsHkPeriodicParams : public ParamsBase { + DiagnosticsHkPeriodicParams(sid_t sid, bool enableReporting, float collectionInterval) + : ParamsBase(sid, enableReporting, collectionInterval, true) {} +}; + +struct RegularHkUpdateParams : public ParamsBase { + RegularHkUpdateParams(sid_t sid, bool enableReporting) + : ParamsBase(sid, enableReporting, 0.0, false) {} +}; + +struct DiagnosticsHkUpdateParams : public ParamsBase { + DiagnosticsHkUpdateParams(sid_t sid, bool enableReporting) + : ParamsBase(sid, enableReporting, 0.0, true) {} +}; +} // namespace subdp + class ProvidesDataPoolSubscriptionIF { public: - virtual ~ProvidesDataPoolSubscriptionIF(){}; - + virtual ~ProvidesDataPoolSubscriptionIF() = default; /** - * @brief Subscribe for the generation of periodic packets. + * @brief Subscribe for the generation of periodic packets. Used for regular HK packets * @details * This subscription mechanism will generally be used by the data creator * to generate housekeeping packets which are downlinked directly. * @return */ - virtual ReturnValue_t subscribeForPeriodicPacket(sid_t sid, bool enableReporting, - float collectionInterval, bool isDiagnostics, - object_id_t packetDestination) = 0; + virtual ReturnValue_t subscribeForRegularPeriodicPacket( + subdp::RegularHkPeriodicParams params) = 0; + /** + * @brief Subscribe for the generation of periodic packets. Used for diagnostic packets + * @details + * This subscription mechanism will generally be used by the data creator + * to generate housekeeping packets which are downlinked directly. + * @return + */ + virtual ReturnValue_t subscribeForDiagPeriodicPacket( + subdp::DiagnosticsHkPeriodicParams params) = 0; + + [[deprecated( + "Please use the new API which takes all arguments as one wrapper " + "struct")]] virtual ReturnValue_t + subscribeForPeriodicPacket(sid_t sid, bool enableReporting, float collectionInterval, + bool isDiagnostics, + object_id_t packetDestination = objects::NO_OBJECT) { + if (isDiagnostics) { + subdp::DiagnosticsHkPeriodicParams params(sid, enableReporting, collectionInterval); + return subscribeForDiagPeriodicPacket(params); + } else { + subdp::RegularHkPeriodicParams params(sid, enableReporting, collectionInterval); + return subscribeForRegularPeriodicPacket(params); + } + } + /** * @brief Subscribe for the generation of packets if the dataset * is marked as changed. @@ -29,9 +95,28 @@ class ProvidesDataPoolSubscriptionIF { * @param packetDestination * @return */ - virtual ReturnValue_t subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, - bool isDiagnostics, - object_id_t packetDestination) = 0; + virtual ReturnValue_t subscribeForRegularUpdatePacket(subdp::RegularHkUpdateParams params) = 0; + virtual ReturnValue_t subscribeForDiagUpdatePacket(subdp::DiagnosticsHkUpdateParams params) = 0; + + // virtual ReturnValue_t + // subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, bool isDiagnostics) { + // return subscribeForUpdatePacket(sid, reportingEnabled, isDiagnostics, objects::NO_OBJECT); + // } + + [[deprecated( + "Please use the new API which takes all arguments as one wrapper " + "struct")]] virtual ReturnValue_t + subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, bool isDiagnostics, + object_id_t packetDestination = objects::NO_OBJECT) { + if (isDiagnostics) { + subdp::DiagnosticsHkUpdateParams params(sid, reportingEnabled); + return subscribeForDiagUpdatePacket(params); + } else { + subdp::RegularHkUpdateParams params(sid, reportingEnabled); + return subscribeForRegularUpdatePacket(params); + } + } + /** * @brief Subscribe for a notification message which will be sent * if a dataset has changed. @@ -46,8 +131,7 @@ class ProvidesDataPoolSubscriptionIF { * Otherwise, only an notification message is sent. * @return */ - virtual ReturnValue_t subscribeForSetUpdateMessage(const uint32_t setId, - object_id_t destinationObject, + virtual ReturnValue_t subscribeForSetUpdateMessage(uint32_t setId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) = 0; /** @@ -64,7 +148,7 @@ class ProvidesDataPoolSubscriptionIF { * only an notification message is sent. * @return */ - virtual ReturnValue_t subscribeForVariableUpdateMessage(const lp_id_t localPoolId, + virtual ReturnValue_t subscribeForVariableUpdateMessage(lp_id_t localPoolId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) = 0; diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index 3275389a..96d12de4 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -130,9 +130,9 @@ ReturnValue_t InternalErrorReporter::initializeLocalDataPool(localpool::DataPool localDataPoolMap.emplace(errorPoolIds::TM_HITS, &tmHitsEntry); localDataPoolMap.emplace(errorPoolIds::QUEUE_HITS, &queueHitsEntry); localDataPoolMap.emplace(errorPoolIds::STORE_HITS, &storeHitsEntry); - poolManager.subscribeForPeriodicPacket( + poolManager.subscribeForDiagPeriodicPacket(subdp::DiagnosticsHkPeriodicParams( internalErrorSid, false, - static_cast(getPeriodicOperationFrequency()) / static_cast(1000.0), true); + static_cast(getPeriodicOperationFrequency()) / static_cast(1000.0))); internalErrorDataset.setValidity(true, true); return returnvalue::OK; } diff --git a/src/fsfw_hal/devicehandlers/MgmLIS3MDLHandler.cpp b/src/fsfw_hal/devicehandlers/MgmLIS3MDLHandler.cpp index 1336276e..d925285f 100644 --- a/src/fsfw_hal/devicehandlers/MgmLIS3MDLHandler.cpp +++ b/src/fsfw_hal/devicehandlers/MgmLIS3MDLHandler.cpp @@ -469,7 +469,7 @@ ReturnValue_t MgmLIS3MDLHandler::initializeLocalDataPool(localpool::DataPool &lo LocalDataPoolManager &poolManager) { localDataPoolMap.emplace(MGMLIS3MDL::FIELD_STRENGTHS, &mgmXYZ); localDataPoolMap.emplace(MGMLIS3MDL::TEMPERATURE_CELCIUS, &temperature); - poolManager.subscribeForPeriodicPacket(dataset.getSid(), false, 10.0, false); + poolManager.subscribeForRegularPeriodicPacket({dataset.getSid(), false, 10.0}); return returnvalue::OK; } diff --git a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp index f1629d89..a32153eb 100644 --- a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp +++ b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp @@ -310,7 +310,7 @@ void MgmRM3100Handler::modeChanged() { internalState = InternalState::NONE; } ReturnValue_t MgmRM3100Handler::initializeLocalDataPool(localpool::DataPool &localDataPoolMap, LocalDataPoolManager &poolManager) { localDataPoolMap.emplace(RM3100::FIELD_STRENGTHS, &mgmXYZ); - poolManager.subscribeForPeriodicPacket(primaryDataset.getSid(), false, 10.0, false); + poolManager.subscribeForRegularPeriodicPacket({primaryDataset.getSid(), false, 10.0}); return returnvalue::OK; } diff --git a/unittests/datapoollocal/CMakeLists.txt b/unittests/datapoollocal/CMakeLists.txt index 4a7ed61a..016645fd 100644 --- a/unittests/datapoollocal/CMakeLists.txt +++ b/unittests/datapoollocal/CMakeLists.txt @@ -2,5 +2,5 @@ target_sources(${FSFW_TEST_TGT} PRIVATE testLocalPoolVariable.cpp testLocalPoolVector.cpp testDataSet.cpp - testLocalPoolManager.cpp + testLocalPoolManager.cpp ) diff --git a/unittests/mocks/LocalPoolOwnerBase.h b/unittests/mocks/LocalPoolOwnerBase.h index 9938c517..f1d1225f 100644 --- a/unittests/mocks/LocalPoolOwnerBase.h +++ b/unittests/mocks/LocalPoolOwnerBase.h @@ -11,7 +11,6 @@ #include #include "fsfw/datapool/PoolEntry.h" -#include "fsfw/housekeeping/AcceptsHkPacketsIF.h" #include "mocks/MessageQueueMock.h" #include "tests/TestsConfig.h" @@ -105,7 +104,8 @@ class LocalPoolOwnerBase : public SystemObject, public HasLocalDataPoolIF { } ReturnValue_t subscribePeriodicHk(bool enableReporting) { - return poolManager.subscribeForPeriodicPacket(lpool::testSid, enableReporting, 0.2, false); + return poolManager.subscribeForRegularPeriodicPacket( + subdp::RegularHkPeriodicParams(lpool::testSid, enableReporting, 0.2)); } ReturnValue_t subscribeWrapperSetUpdate(MessageQueueId_t receiverId) { @@ -121,9 +121,17 @@ class LocalPoolOwnerBase : public SystemObject, public HasLocalDataPoolIF { ReturnValue_t subscribeWrapperSetUpdateHk(bool diagnostics = false, AcceptsHkPacketsIF* receiver = nullptr) { if (diagnostics) { - return poolManager.subscribeForUpdatePacket(lpool::testSid, true, true); + auto params = subdp::DiagnosticsHkUpdateParams(lpool::testSid, true); + if (receiver != nullptr) { + params.receiver = receiver->getHkQueue(); + } + return poolManager.subscribeForDiagUpdatePacket(params); } else { - return poolManager.subscribeForUpdatePacket(lpool::testSid, true, false); + auto params = subdp::RegularHkUpdateParams(lpool::testSid, true); + if (receiver != nullptr) { + params.receiver = receiver->getHkQueue(); + } + return poolManager.subscribeForRegularUpdatePacket(params); } }