From 9998de086fc2e469dd370b4b36dcb854c5d233d8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 8 Feb 2021 14:20:36 +0100 Subject: [PATCH 01/20] added printouts for action helper --- action/ActionHelper.cpp | 36 +++++++++++++++++++++++------------- action/ActionHelper.h | 3 ++- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 23c9d732..e32be68b 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -3,6 +3,7 @@ #include "../ipc/MessageQueueSenderIF.h" #include "../objectmanager/ObjectManagerIF.h" +#include "../serviceinterface/ServiceInterface.h" ActionHelper::ActionHelper(HasActionsIF* setOwner, MessageQueueIF* useThisQueue) : @@ -86,13 +87,20 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, uint8_t *dataPtr; size_t maxSize = data->getSerializedSize(); if (maxSize == 0) { - //No error, there's simply nothing to report. + /* No error, there's simply nothing to report. */ return HasReturnvaluesIF::RETURN_OK; } size_t size = 0; ReturnValue_t result = ipcStore->getFreeElement(&storeAddress, maxSize, &dataPtr); if (result != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "ActionHelper::reportData: Getting free element from IPC store failed!" << + std::endl; +#else + sif::printWarning("ActionHelper::reportData: Getting free element from IPC " + "store failed!\n"); +#endif return result; } result = data->serialize(&dataPtr, &size, maxSize, @@ -101,14 +109,13 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, ipcStore->deleteData(storeAddress); return result; } - // We don't need to report the objectId, as we receive REQUESTED data - // before the completion success message. - // True aperiodic replies need to be reported with - // another dedicated message. + + /* We don't need to report the objectId, as we receive REQUESTED data before the completion + success message. True aperiodic replies need to be reported with another dedicated message. */ ActionMessage::setDataReply(&reply, replyId, storeAddress); - // If the sender needs to be hidden, for example to handle packet - // as unrequested reply, this will be done here. + /* If the sender needs to be hidden, for example to handle packet + as unrequested reply, this will be done here. */ if (hideSender) { result = MessageQueueSenderIF::sendMessage(reportTo, &reply); } @@ -132,6 +139,11 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, store_address_t storeAddress; ReturnValue_t result = ipcStore->addData(&storeAddress, data, dataSize); if (result != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "ActionHelper::reportData: Adding data to IPC store failed!" << std::endl; +#else + sif::printWarning("ActionHelper::reportData: Adding data to IPC store failed!\n"); +#endif return result; } @@ -140,14 +152,12 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, return result; } - // We don't need to report the objectId, as we receive REQUESTED data - // before the completion success message. - // True aperiodic replies need to be reported with - // another dedicated message. + /* We don't need to report the objectId, as we receive REQUESTED data before the completion + success message. True aperiodic replies need to be reported with another dedicated message. */ ActionMessage::setDataReply(&reply, replyId, storeAddress); - // If the sender needs to be hidden, for example to handle packet - // as unrequested reply, this will be done here. + /* If the sender needs to be hidden, for example to handle packet + as unrequested reply, this will be done here. */ if (hideSender) { result = MessageQueueSenderIF::sendMessage(reportTo, &reply); } diff --git a/action/ActionHelper.h b/action/ActionHelper.h index ae971d38..35ac41d1 100644 --- a/action/ActionHelper.h +++ b/action/ActionHelper.h @@ -101,7 +101,8 @@ public: protected: //! Increase of value of this per step static const uint8_t STEP_OFFSET = 1; - HasActionsIF* owner;//!< Pointer to the owner + //! Pointer to the owner + HasActionsIF* owner; //! Queue to be used as response sender, has to be set in ctor or with //! setQueueToUse MessageQueueIF* queueToUse; From c5ee2260d14008c2afd7f260ee86240011efb156 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 12:59:37 +0100 Subject: [PATCH 02/20] renamed abstract function, removed plural --- datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/LocalDataPoolManager.h | 2 +- datapoollocal/ProvidesDataPoolSubscriptionIF.h | 2 +- unittest/tests/datapoollocal/LocalPoolOwnerBase.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 44769163..01c5bb9a 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -380,7 +380,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, } -ReturnValue_t LocalDataPoolManager::subscribeForUpdatePackets(sid_t sid, +ReturnValue_t LocalDataPoolManager::subscribeForUpdatePacket(sid_t sid, bool isDiagnostics, bool reportingEnabled, object_id_t packetDestination) { AcceptsHkPacketsIF* hkReceiverObject = diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 05c6adfd..a53e45b5 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -137,7 +137,7 @@ public: * @param packetDestination * @return */ - ReturnValue_t subscribeForUpdatePackets(sid_t sid, bool reportingEnabled, + ReturnValue_t subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, bool isDiagnostics, object_id_t packetDestination = defaultHkDestination) override; diff --git a/datapoollocal/ProvidesDataPoolSubscriptionIF.h b/datapoollocal/ProvidesDataPoolSubscriptionIF.h index ead479cc..642e8ee1 100644 --- a/datapoollocal/ProvidesDataPoolSubscriptionIF.h +++ b/datapoollocal/ProvidesDataPoolSubscriptionIF.h @@ -33,7 +33,7 @@ public: * @param packetDestination * @return */ - virtual ReturnValue_t subscribeForUpdatePackets(sid_t sid, + virtual ReturnValue_t subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, bool isDiagnostics, object_id_t packetDestination) = 0; diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 876ae5f5..68bccbb8 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -174,7 +174,7 @@ public: } ReturnValue_t subscribeWrapperSetUpdateHk(bool diagnostics = false) { - return poolManager.subscribeForUpdatePackets(lpool::testSid, diagnostics, + return poolManager.subscribeForUpdatePacket(lpool::testSid, diagnostics, false, objects::HK_RECEIVER_MOCK); } From f45d19a9619ad787ee00efd6f46934f67b4785f0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 13:06:55 +0100 Subject: [PATCH 03/20] better documentation --- housekeeping/HousekeepingPacketDownlink.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/housekeeping/HousekeepingPacketDownlink.h b/housekeeping/HousekeepingPacketDownlink.h index ae0cc988..aeea16c3 100644 --- a/housekeeping/HousekeepingPacketDownlink.h +++ b/housekeeping/HousekeepingPacketDownlink.h @@ -10,7 +10,11 @@ * which are destined to be downlinked into the store. * @details * The housekeeping packets are stored into the IPC store and forwarded - * to the designated housekeeping handler. + * to the designated housekeeping handler. The packet will consist of the following fields + * - SID (8 byte): Structure ID, with the first 4 bytes being the object ID and the last four + * bytes being the set ID + * - Housekeeping Data: The rest of the packet will be the serialized housekeeping data. A validity + * buffer might be appended at the end, depending on the set configuration. */ class HousekeepingPacketDownlink: public SerialLinkedListAdapter { public: From 788dbe4eca1bd8f2daa212ec14af95a6b333b4d9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 13:56:48 +0100 Subject: [PATCH 04/20] removed plural --- datapoollocal/LocalDataPoolManager.cpp | 4 ++-- datapoollocal/LocalDataPoolManager.h | 4 ++-- datapoollocal/ProvidesDataPoolSubscriptionIF.h | 4 ++-- unittest/tests/datapoollocal/LocalPoolManagerTest.cpp | 2 +- unittest/tests/datapoollocal/LocalPoolOwnerBase.h | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 01c5bb9a..19e23588 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -410,7 +410,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForUpdatePacket(sid_t sid, return HasReturnvaluesIF::RETURN_OK; } -ReturnValue_t LocalDataPoolManager::subscribeForSetUpdateMessages( +ReturnValue_t LocalDataPoolManager::subscribeForSetUpdateMessage( const uint32_t setId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) { struct HkReceiver hkReceiver; @@ -431,7 +431,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForSetUpdateMessages( return HasReturnvaluesIF::RETURN_OK; } -ReturnValue_t LocalDataPoolManager::subscribeForVariableUpdateMessages( +ReturnValue_t LocalDataPoolManager::subscribeForVariableUpdateMessage( const lp_id_t localPoolId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) { struct HkReceiver hkReceiver; diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index a53e45b5..fdfd9d23 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -155,7 +155,7 @@ public: * Otherwise, only an notification message is sent. * @return */ - ReturnValue_t subscribeForSetUpdateMessages(const uint32_t setId, + ReturnValue_t subscribeForSetUpdateMessage(const uint32_t setId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) override; @@ -174,7 +174,7 @@ public: * Otherwise, only an notification message is sent. * @return */ - ReturnValue_t subscribeForVariableUpdateMessages(const lp_id_t localPoolId, + ReturnValue_t subscribeForVariableUpdateMessage(const lp_id_t localPoolId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) override; diff --git a/datapoollocal/ProvidesDataPoolSubscriptionIF.h b/datapoollocal/ProvidesDataPoolSubscriptionIF.h index 642e8ee1..3394c98b 100644 --- a/datapoollocal/ProvidesDataPoolSubscriptionIF.h +++ b/datapoollocal/ProvidesDataPoolSubscriptionIF.h @@ -52,7 +52,7 @@ public: * Otherwise, only an notification message is sent. * @return */ - virtual ReturnValue_t subscribeForSetUpdateMessages(const uint32_t setId, + virtual ReturnValue_t subscribeForSetUpdateMessage(const uint32_t setId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) = 0; @@ -70,7 +70,7 @@ public: * only an notification message is sent. * @return */ - virtual ReturnValue_t subscribeForVariableUpdateMessages( + virtual ReturnValue_t subscribeForVariableUpdateMessage( const lp_id_t localPoolId, object_id_t destinationObject, MessageQueueId_t targetQueueId, diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index c8b10442..e621f4d8 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -160,7 +160,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(messageSent.getCommand() == static_cast( HousekeepingMessage::UPDATE_NOTIFICATION_VARIABLE)); /* Now subscribe for the dataset update (HK and update) again with subscription interface */ - REQUIRE(subscriptionIF->subscribeForSetUpdateMessages(lpool::testSetId, + REQUIRE(subscriptionIF->subscribeForSetUpdateMessage(lpool::testSetId, objects::NO_OBJECT, objects::HK_RECEIVER_MOCK, false) == retval::CATCH_OK); REQUIRE(poolOwner->subscribeWrapperSetUpdateHk() == retval::CATCH_OK); diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 68bccbb8..acbb2b73 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -164,12 +164,12 @@ public: } ReturnValue_t subscribeWrapperSetUpdate() { - return poolManager.subscribeForSetUpdateMessages(lpool::testSetId, + return poolManager.subscribeForSetUpdateMessage(lpool::testSetId, objects::NO_OBJECT, objects::HK_RECEIVER_MOCK, false); } ReturnValue_t subscribeWrapperSetUpdateSnapshot() { - return poolManager.subscribeForSetUpdateMessages(lpool::testSetId, + return poolManager.subscribeForSetUpdateMessage(lpool::testSetId, objects::NO_OBJECT, objects::HK_RECEIVER_MOCK, true); } @@ -179,7 +179,7 @@ public: } ReturnValue_t subscribeWrapperVariableUpdate(lp_id_t localPoolId) { - return poolManager.subscribeForVariableUpdateMessages(localPoolId, + return poolManager.subscribeForVariableUpdateMessage(localPoolId, MessageQueueIF::NO_QUEUE, objects::HK_RECEIVER_MOCK, false); } From 110159eea1dfcbe74d9a91f6d8c2546d3a3b5019 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 13:57:58 +0100 Subject: [PATCH 05/20] formatting --- .../ProvidesDataPoolSubscriptionIF.h | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/datapoollocal/ProvidesDataPoolSubscriptionIF.h b/datapoollocal/ProvidesDataPoolSubscriptionIF.h index 3394c98b..1c34099a 100644 --- a/datapoollocal/ProvidesDataPoolSubscriptionIF.h +++ b/datapoollocal/ProvidesDataPoolSubscriptionIF.h @@ -17,12 +17,8 @@ public: * 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 subscribeForPeriodicPacket(sid_t sid, bool enableReporting, + float collectionInterval, bool isDiagnostics, object_id_t packetDestination) = 0; /** * @brief Subscribe for the generation of packets if the dataset * is marked as changed. @@ -33,11 +29,8 @@ public: * @param packetDestination * @return */ - virtual ReturnValue_t subscribeForUpdatePacket(sid_t sid, - bool reportingEnabled, - bool isDiagnostics, - object_id_t packetDestination) = 0; - + virtual ReturnValue_t subscribeForUpdatePacket(sid_t sid, bool reportingEnabled, + bool isDiagnostics, object_id_t packetDestination) = 0; /** * @brief Subscribe for a notification message which will be sent * if a dataset has changed. @@ -55,7 +48,6 @@ public: virtual ReturnValue_t subscribeForSetUpdateMessage(const uint32_t setId, object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) = 0; - /** * @brief Subscribe for an notification message which will be sent if a * pool variable has changed. @@ -70,12 +62,9 @@ public: * only an notification message is sent. * @return */ - virtual ReturnValue_t subscribeForVariableUpdateMessage( - const lp_id_t localPoolId, - object_id_t destinationObject, - MessageQueueId_t targetQueueId, + virtual ReturnValue_t subscribeForVariableUpdateMessage(const lp_id_t localPoolId, + object_id_t destinationObject, MessageQueueId_t targetQueueId, bool generateSnapshot) = 0; - }; #endif /* FSFW_DATAPOOLLOCAL_PROVIDESDATAPOOLSUBSCRIPTION_H_ */ From ea6ee7e79c546454ae60ef6c498a42bf5ef761ac Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 14:08:30 +0100 Subject: [PATCH 06/20] added instructions on how to retrieve the interface --- datapoollocal/HasLocalDataPoolIF.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index 05570175..3c31c5cd 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -23,11 +23,20 @@ class LocalPoolObjectBase; * @details * Any class implementing this interface shall also have a LocalDataPoolManager member class which * contains the actual pool data structure and exposes the public interface for it. + * * The local data pool can be accessed using helper classes by using the * LocalPoolVariable, LocalPoolVector or LocalDataSet classes. Every local pool variable can * be uniquely identified by a global pool ID (gp_id_t) and every dataset tied * to a pool manager can be uniqely identified by a global structure ID (sid_t). * + * All software objects which want to use the local pool of another object shall also use this + * interface, for example to get a handle to the subscription interface. + * For example, the following line of code can be used to retrieve the interface + * + * HasLocalDataPoolIF* poolIF = objectManager->get(objects::SOME_OBJECT); + * if(poolIF != nullptr) { + * doSomething() + * } */ class HasLocalDataPoolIF { friend class HasLocalDpIFManagerAttorney; From fcff06c83fe6d8c0bcd864b9276b039fe6845f04 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 27 Feb 2021 14:09:44 +0100 Subject: [PATCH 07/20] some more details --- datapoollocal/HasLocalDataPoolIF.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index 3c31c5cd..49f2db7d 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -30,7 +30,8 @@ class LocalPoolObjectBase; * to a pool manager can be uniqely identified by a global structure ID (sid_t). * * All software objects which want to use the local pool of another object shall also use this - * interface, for example to get a handle to the subscription interface. + * interface, for example to get a handle to the subscription interface. The interface + * can be retrieved using the object manager, provided the target object is a SystemObject. * For example, the following line of code can be used to retrieve the interface * * HasLocalDataPoolIF* poolIF = objectManager->get(objects::SOME_OBJECT); From a65211be51be8d2c998396444649edb4eeef1719 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 13:48:53 +0100 Subject: [PATCH 08/20] new attorney for ReadCommitIF --- datapool/PoolDataSetBase.cpp | 315 ++++++++++++++++---------------- datapool/PoolDataSetBase.h | 241 ++++++++++++------------ datapool/PoolVariableIF.h | 75 ++++---- datapool/ReadCommitIF.h | 8 +- datapool/ReadCommitIFAttorney.h | 32 ++++ 5 files changed, 354 insertions(+), 317 deletions(-) create mode 100644 datapool/ReadCommitIFAttorney.h diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 2fd51966..1474aff2 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -1,5 +1,8 @@ #include "PoolDataSetBase.h" +#include "ReadCommitIFAttorney.h" + #include "../serviceinterface/ServiceInterfaceStream.h" + #include PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, @@ -12,61 +15,61 @@ PoolDataSetBase::~PoolDataSetBase() {} ReturnValue_t PoolDataSetBase::registerVariable( - PoolVariableIF *variable) { - if (state != States::STATE_SET_UNINITIALISED) { + PoolVariableIF *variable) { + if (state != States::STATE_SET_UNINITIALISED) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "Call made in wrong position." << std::endl; + sif::error << "DataSet::registerVariable: " + "Call made in wrong position." << std::endl; #endif - return DataSetIF::DATA_SET_UNINITIALISED; - } - if (variable == nullptr) { + return DataSetIF::DATA_SET_UNINITIALISED; + } + if (variable == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "Pool variable is nullptr." << std::endl; + sif::error << "DataSet::registerVariable: " + "Pool variable is nullptr." << std::endl; #endif - return DataSetIF::POOL_VAR_NULL; - } - if (fillCount >= maxFillCount) { + return DataSetIF::POOL_VAR_NULL; + } + if (fillCount >= maxFillCount) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "DataSet is full." << std::endl; + sif::error << "DataSet::registerVariable: " + "DataSet is full." << std::endl; #endif - return DataSetIF::DATA_SET_FULL; - } - registeredVariables[fillCount] = variable; - fillCount++; - return HasReturnvaluesIF::RETURN_OK; + return DataSetIF::DATA_SET_FULL; + } + registeredVariables[fillCount] = variable; + fillCount++; + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t PoolDataSetBase::read(MutexIF::TimeoutType timeoutType, - uint32_t lockTimeout) { - ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - ReturnValue_t error = result; - if (state == States::STATE_SET_UNINITIALISED) { - lockDataPool(timeoutType, lockTimeout); - for (uint16_t count = 0; count < fillCount; count++) { - result = readVariable(count); - if(result != RETURN_OK) { - error = result; - } - } - state = States::STATE_SET_WAS_READ; - unlockDataPool(); - } - else { + uint32_t lockTimeout) { + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + ReturnValue_t error = result; + if (state == States::STATE_SET_UNINITIALISED) { + lockDataPool(timeoutType, lockTimeout); + for (uint16_t count = 0; count < fillCount; count++) { + result = readVariable(count); + if(result != RETURN_OK) { + error = result; + } + } + state = States::STATE_SET_WAS_READ; + unlockDataPool(); + } + else { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::read(): " - "Call made in wrong position. Don't forget to commit" - " member datasets!" << std::endl; + sif::error << "DataSet::read(): " + "Call made in wrong position. Don't forget to commit" + " member datasets!" << std::endl; #endif - result = SET_WAS_ALREADY_READ; - } + result = SET_WAS_ALREADY_READ; + } - if(error != HasReturnvaluesIF::RETURN_OK) { - result = error; - } - return result; + if(error != HasReturnvaluesIF::RETURN_OK) { + result = error; + } + return result; } uint16_t PoolDataSetBase::getFillCount() const { @@ -74,144 +77,144 @@ uint16_t PoolDataSetBase::getFillCount() const { } ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) { - ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - if(registeredVariables[count] == nullptr) { - // configuration error. - return HasReturnvaluesIF::RETURN_FAILED; - } + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + if(registeredVariables[count] == nullptr) { + /* Configuration error. */ + return HasReturnvaluesIF::RETURN_FAILED; + } - // These checks are often performed by the respective - // variable implementation too, but I guess a double check does not hurt. - if (registeredVariables[count]->getReadWriteMode() != - PoolVariableIF::VAR_WRITE and - registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) - { - if(protectEveryReadCommitCall) { - result = registeredVariables[count]->read( - timeoutTypeForSingleVars, - mutexTimeoutForSingleVars); - } - else { - result = registeredVariables[count]->readWithoutLock(); - } + /* These checks are often performed by the respective variable implementation too, but I guess + a double check does not hurt. */ + if (registeredVariables[count]->getReadWriteMode() != + PoolVariableIF::VAR_WRITE and + registeredVariables[count]->getDataPoolId() + != PoolVariableIF::NO_PARAMETER) + { + if(protectEveryReadCommitCall) { + result = registeredVariables[count]->read( + timeoutTypeForSingleVars, + mutexTimeoutForSingleVars); + } + else { + result = ReadCommitIFAttorney::readWithoutLock(registeredVariables[count]); + } - if(result != HasReturnvaluesIF::RETURN_OK) { - result = INVALID_PARAMETER_DEFINITION; - } - } - return result; + if(result != HasReturnvaluesIF::RETURN_OK) { + result = INVALID_PARAMETER_DEFINITION; + } + } + return result; } ReturnValue_t PoolDataSetBase::commit(MutexIF::TimeoutType timeoutType, - uint32_t lockTimeout) { - if (state == States::STATE_SET_WAS_READ) { - handleAlreadyReadDatasetCommit(timeoutType, lockTimeout); - return HasReturnvaluesIF::RETURN_OK; - } - else { - return handleUnreadDatasetCommit(timeoutType, lockTimeout); - } + uint32_t lockTimeout) { + if (state == States::STATE_SET_WAS_READ) { + handleAlreadyReadDatasetCommit(timeoutType, lockTimeout); + return HasReturnvaluesIF::RETURN_OK; + } + else { + return handleUnreadDatasetCommit(timeoutType, lockTimeout); + } } void PoolDataSetBase::handleAlreadyReadDatasetCommit( - MutexIF::TimeoutType timeoutType, uint32_t lockTimeout) { - lockDataPool(timeoutType, lockTimeout); - for (uint16_t count = 0; count < fillCount; count++) { - if (registeredVariables[count]->getReadWriteMode() - != PoolVariableIF::VAR_READ - && registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) { - if(protectEveryReadCommitCall) { - registeredVariables[count]->commit( - timeoutTypeForSingleVars, - mutexTimeoutForSingleVars); - } - else { - registeredVariables[count]->commitWithoutLock(); - } - } - } - state = States::STATE_SET_UNINITIALISED; - unlockDataPool(); + MutexIF::TimeoutType timeoutType, uint32_t lockTimeout) { + lockDataPool(timeoutType, lockTimeout); + for (uint16_t count = 0; count < fillCount; count++) { + if (registeredVariables[count]->getReadWriteMode() + != PoolVariableIF::VAR_READ + && registeredVariables[count]->getDataPoolId() + != PoolVariableIF::NO_PARAMETER) { + if(protectEveryReadCommitCall) { + registeredVariables[count]->commit( + timeoutTypeForSingleVars, + mutexTimeoutForSingleVars); + } + else { + ReadCommitIFAttorney::commitWithoutLock(registeredVariables[count]); + } + } + } + state = States::STATE_SET_UNINITIALISED; + unlockDataPool(); } ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit( - MutexIF::TimeoutType timeoutType, uint32_t lockTimeout) { - ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - lockDataPool(timeoutType, lockTimeout); - for (uint16_t count = 0; count < fillCount; count++) { - if (registeredVariables[count]->getReadWriteMode() - == PoolVariableIF::VAR_WRITE - && registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) { - if(protectEveryReadCommitCall) { - result = registeredVariables[count]->commit( - timeoutTypeForSingleVars, - mutexTimeoutForSingleVars); - } - else { - result = registeredVariables[count]->commitWithoutLock(); - } + MutexIF::TimeoutType timeoutType, uint32_t lockTimeout) { + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + lockDataPool(timeoutType, lockTimeout); + for (uint16_t count = 0; count < fillCount; count++) { + if (registeredVariables[count]->getReadWriteMode() + == PoolVariableIF::VAR_WRITE + && registeredVariables[count]->getDataPoolId() + != PoolVariableIF::NO_PARAMETER) { + if(protectEveryReadCommitCall) { + result = registeredVariables[count]->commit( + timeoutTypeForSingleVars, + mutexTimeoutForSingleVars); + } + else { + result = registeredVariables[count]->commitWithoutLock(); + } - } else if (registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) { - if (result != COMMITING_WITHOUT_READING) { + } else if (registeredVariables[count]->getDataPoolId() + != PoolVariableIF::NO_PARAMETER) { + if (result != COMMITING_WITHOUT_READING) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::commit(): commit-without-read call made " - "with non write-only variable." << std::endl; + sif::error << "DataSet::commit(): commit-without-read call made " + "with non write-only variable." << std::endl; #endif - result = COMMITING_WITHOUT_READING; - } - } - } - state = States::STATE_SET_UNINITIALISED; - unlockDataPool(); - return result; + result = COMMITING_WITHOUT_READING; + } + } + } + state = States::STATE_SET_UNINITIALISED; + unlockDataPool(); + return result; } ReturnValue_t PoolDataSetBase::lockDataPool(MutexIF::TimeoutType timeoutType, - uint32_t lockTimeout) { - return HasReturnvaluesIF::RETURN_OK; + uint32_t lockTimeout) { + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t PoolDataSetBase::unlockDataPool() { - return HasReturnvaluesIF::RETURN_OK; + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t PoolDataSetBase::serialize(uint8_t** buffer, size_t* size, - const size_t maxSize, SerializeIF::Endianness streamEndianness) const { - ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; - for (uint16_t count = 0; count < fillCount; count++) { - result = registeredVariables[count]->serialize(buffer, size, maxSize, - streamEndianness); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } - } - return result; + const size_t maxSize, SerializeIF::Endianness streamEndianness) const { + ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; + for (uint16_t count = 0; count < fillCount; count++) { + result = registeredVariables[count]->serialize(buffer, size, maxSize, + streamEndianness); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } + } + return result; } ReturnValue_t PoolDataSetBase::deSerialize(const uint8_t** buffer, size_t* size, SerializeIF::Endianness streamEndianness) { - ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; - for (uint16_t count = 0; count < fillCount; count++) { - result = registeredVariables[count]->deSerialize(buffer, size, - streamEndianness); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } - } - return result; + ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; + for (uint16_t count = 0; count < fillCount; count++) { + result = registeredVariables[count]->deSerialize(buffer, size, + streamEndianness); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } + } + return result; } size_t PoolDataSetBase::getSerializedSize() const { - uint32_t size = 0; - for (uint16_t count = 0; count < fillCount; count++) { - size += registeredVariables[count]->getSerializedSize(); - } - return size; + uint32_t size = 0; + for (uint16_t count = 0; count < fillCount; count++) { + size += registeredVariables[count]->getSerializedSize(); + } + return size; } void PoolDataSetBase::setContainer(PoolVariableIF **variablesContainer) { @@ -219,13 +222,13 @@ void PoolDataSetBase::setContainer(PoolVariableIF **variablesContainer) { } PoolVariableIF** PoolDataSetBase::getContainer() const { - return registeredVariables; + return registeredVariables; } void PoolDataSetBase::setReadCommitProtectionBehaviour( - bool protectEveryReadCommit, MutexIF::TimeoutType timeoutType, - uint32_t mutexTimeout) { - this->protectEveryReadCommitCall = protectEveryReadCommit; - this->timeoutTypeForSingleVars = timeoutType; - this->mutexTimeoutForSingleVars = mutexTimeout; + bool protectEveryReadCommit, MutexIF::TimeoutType timeoutType, + uint32_t mutexTimeout) { + this->protectEveryReadCommitCall = protectEveryReadCommit; + this->timeoutTypeForSingleVars = timeoutType; + this->mutexTimeoutForSingleVars = mutexTimeout; } diff --git a/datapool/PoolDataSetBase.h b/datapool/PoolDataSetBase.h index ab895455..043c860a 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -29,98 +29,99 @@ * @author Bastian Baetz * @ingroup data_pool */ -class PoolDataSetBase: public PoolDataSetIF, - public SerializeIF, - public HasReturnvaluesIF { +class PoolDataSetBase: + public PoolDataSetIF, + public SerializeIF, + public HasReturnvaluesIF { public: - /** - * @brief Creates an empty dataset. Use registerVariable or - * supply a pointer to this dataset to PoolVariable - * initializations to register pool variables. - */ - PoolDataSetBase(PoolVariableIF** registeredVariablesArray, const size_t maxFillCount); + /** + * @brief Creates an empty dataset. Use registerVariable or + * supply a pointer to this dataset to PoolVariable + * initializations to register pool variables. + */ + PoolDataSetBase(PoolVariableIF** registeredVariablesArray, const size_t maxFillCount); - /* Forbidden for now */ - PoolDataSetBase(const PoolDataSetBase& otherSet) = delete; - const PoolDataSetBase& operator=(const PoolDataSetBase& otherSet) = delete; + /* Forbidden for now */ + PoolDataSetBase(const PoolDataSetBase& otherSet) = delete; + const PoolDataSetBase& operator=(const PoolDataSetBase& otherSet) = delete; - virtual~ PoolDataSetBase(); + virtual~ PoolDataSetBase(); - /** - * @brief The read call initializes reading out all registered variables. - * It is mandatory to call commit after every read call! - * @details - * It iterates through the list of registered variables and calls all read() - * functions of the registered pool variables (which read out their values - * from the data pool) which are not write-only. - * In case of an error (e.g. a wrong data type, or an invalid data pool id), - * the operation is aborted and @c INVALID_PARAMETER_DEFINITION returned. - * - * The data pool is locked during the whole read operation and - * freed afterwards. It is mandatory to call commit after a read call, - * even if the read operation is not successful! - * @return - * - @c RETURN_OK if all variables were read successfully. - * - @c INVALID_PARAMETER_DEFINITION if a pool entry does not exist or there - * is a type conflict. - * - @c SET_WAS_ALREADY_READ if read() is called twice without calling - * commit() in between - */ - virtual ReturnValue_t read(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, - uint32_t lockTimeout = 20) override; - /** - * @brief The commit call initializes writing back the registered variables. - * @details - * It iterates through the list of registered variables and calls the - * commit() method of the remaining registered variables (which write back - * their values to the pool). - * - * The data pool is locked during the whole commit operation and - * freed afterwards. The state changes to "was committed" after this operation. - * - * If the set does contain at least one variable which is not write-only - * commit() can only be called after read(). If the set only contains - * variables which are write only, commit() can be called without a - * preceding read() call. Every read call must be followed by a commit call! - * @return - @c RETURN_OK if all variables were read successfully. - * - @c COMMITING_WITHOUT_READING if set was not read yet and - * contains non write-only variables - */ - virtual ReturnValue_t commit(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, - uint32_t lockTimeout = 20) override; + /** + * @brief The read call initializes reading out all registered variables. + * It is mandatory to call commit after every read call! + * @details + * It iterates through the list of registered variables and calls all read() + * functions of the registered pool variables (which read out their values + * from the data pool) which are not write-only. + * In case of an error (e.g. a wrong data type, or an invalid data pool id), + * the operation is aborted and @c INVALID_PARAMETER_DEFINITION returned. + * + * The data pool is locked during the whole read operation and + * freed afterwards. It is mandatory to call commit after a read call, + * even if the read operation is not successful! + * @return + * - @c RETURN_OK if all variables were read successfully. + * - @c INVALID_PARAMETER_DEFINITION if a pool entry does not exist or there + * is a type conflict. + * - @c SET_WAS_ALREADY_READ if read() is called twice without calling + * commit() in between + */ + virtual ReturnValue_t read(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + uint32_t lockTimeout = 20) override; + /** + * @brief The commit call initializes writing back the registered variables. + * @details + * It iterates through the list of registered variables and calls the + * commit() method of the remaining registered variables (which write back + * their values to the pool). + * + * The data pool is locked during the whole commit operation and + * freed afterwards. The state changes to "was committed" after this operation. + * + * If the set does contain at least one variable which is not write-only + * commit() can only be called after read(). If the set only contains + * variables which are write only, commit() can be called without a + * preceding read() call. Every read call must be followed by a commit call! + * @return - @c RETURN_OK if all variables were read successfully. + * - @c COMMITING_WITHOUT_READING if set was not read yet and + * contains non write-only variables + */ + virtual ReturnValue_t commit(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + uint32_t lockTimeout = 20) override; - /** - * Register the passed pool variable instance into the data set. - * @param variable - * @return - */ - virtual ReturnValue_t registerVariable( PoolVariableIF* variable) override; + /** + * Register the passed pool variable instance into the data set. + * @param variable + * @return + */ + virtual ReturnValue_t registerVariable( PoolVariableIF* variable) override; - /** - * Provides the means to lock the underlying data structure to ensure - * thread-safety. Default implementation is empty - * @return Always returns -@c RETURN_OK - */ - virtual ReturnValue_t lockDataPool( - MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, - uint32_t timeoutMs = 20) override; - /** - * Provides the means to unlock the underlying data structure to ensure - * thread-safety. Default implementation is empty - * @return Always returns -@c RETURN_OK - */ - virtual ReturnValue_t unlockDataPool() override; + /** + * Provides the means to lock the underlying data structure to ensure + * thread-safety. Default implementation is empty + * @return Always returns -@c RETURN_OK + */ + virtual ReturnValue_t lockDataPool( + MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + uint32_t timeoutMs = 20) override; + /** + * Provides the means to unlock the underlying data structure to ensure + * thread-safety. Default implementation is empty + * @return Always returns -@c RETURN_OK + */ + virtual ReturnValue_t unlockDataPool() override; - virtual uint16_t getFillCount() const; + virtual uint16_t getFillCount() const; - /* SerializeIF implementations */ - virtual ReturnValue_t serialize(uint8_t** buffer, size_t* size, - const size_t maxSize, - SerializeIF::Endianness streamEndianness) const override; - virtual size_t getSerializedSize() const override; - virtual ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size, - SerializeIF::Endianness streamEndianness) override; + /* SerializeIF implementations */ + virtual ReturnValue_t serialize(uint8_t** buffer, size_t* size, + const size_t maxSize, + SerializeIF::Endianness streamEndianness) const override; + virtual size_t getSerializedSize() const override; + virtual ReturnValue_t deSerialize(const uint8_t** buffer, size_t* size, + SerializeIF::Endianness streamEndianness) override; /** * Can be used to individually protect every read and commit call. @@ -132,48 +133,48 @@ public: uint32_t mutexTimeout = 20); protected: - /** - * @brief The fill_count attribute ensures that the variables - * register in the correct array position and that the maximum - * number of variables is not exceeded. - */ - uint16_t fillCount = 0; - /** - * States of the seet. - */ - enum class States { - STATE_SET_UNINITIALISED, //!< DATA_SET_UNINITIALISED - STATE_SET_WAS_READ //!< DATA_SET_WAS_READ - }; - /** - * @brief state manages the internal state of the data set, - * which is important e.g. for the behavior on destruction. - */ - States state = States::STATE_SET_UNINITIALISED; + /** + * @brief The fill_count attribute ensures that the variables + * register in the correct array position and that the maximum + * number of variables is not exceeded. + */ + uint16_t fillCount = 0; + /** + * States of the seet. + */ + enum class States { + STATE_SET_UNINITIALISED, //!< DATA_SET_UNINITIALISED + STATE_SET_WAS_READ //!< DATA_SET_WAS_READ + }; + /** + * @brief state manages the internal state of the data set, + * which is important e.g. for the behavior on destruction. + */ + States state = States::STATE_SET_UNINITIALISED; - /** - * @brief This array represents all pool variables registered in this set. - * Child classes can use a static or dynamic container to create - * an array of registered variables and assign the first entry here. - */ - PoolVariableIF** registeredVariables = nullptr; - const size_t maxFillCount = 0; + /** + * @brief This array represents all pool variables registered in this set. + * Child classes can use a static or dynamic container to create + * an array of registered variables and assign the first entry here. + */ + PoolVariableIF** registeredVariables = nullptr; + const size_t maxFillCount = 0; - void setContainer(PoolVariableIF** variablesContainer); - PoolVariableIF** getContainer() const; + void setContainer(PoolVariableIF** variablesContainer); + PoolVariableIF** getContainer() const; private: - bool protectEveryReadCommitCall = false; - MutexIF::TimeoutType timeoutTypeForSingleVars = MutexIF::TimeoutType::WAITING; - uint32_t mutexTimeoutForSingleVars = 20; + bool protectEveryReadCommitCall = false; + MutexIF::TimeoutType timeoutTypeForSingleVars = MutexIF::TimeoutType::WAITING; + uint32_t mutexTimeoutForSingleVars = 20; - ReturnValue_t readVariable(uint16_t count); - void handleAlreadyReadDatasetCommit( - MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, - uint32_t timeoutMs = 20); - ReturnValue_t handleUnreadDatasetCommit( - MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, - uint32_t timeoutMs = 20); + ReturnValue_t readVariable(uint16_t count); + void handleAlreadyReadDatasetCommit( + MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + uint32_t timeoutMs = 20); + ReturnValue_t handleUnreadDatasetCommit( + MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, + uint32_t timeoutMs = 20); }; #endif /* FSFW_DATAPOOL_POOLDATASETBASE_H_ */ diff --git a/datapool/PoolVariableIF.h b/datapool/PoolVariableIF.h index 444e19d0..dead6844 100644 --- a/datapool/PoolVariableIF.h +++ b/datapool/PoolVariableIF.h @@ -1,9 +1,10 @@ #ifndef FSFW_DATAPOOL_POOLVARIABLEIF_H_ #define FSFW_DATAPOOL_POOLVARIABLEIF_H_ +#include "ReadCommitIF.h" #include "../returnvalues/HasReturnvaluesIF.h" #include "../serialize/SerializeIF.h" -#include "ReadCommitIF.h" + /** * @brief This interface is used to control data pool @@ -18,46 +19,46 @@ * @author Bastian Baetz * @ingroup data_pool */ -class PoolVariableIF : public SerializeIF, - public ReadCommitIF { - friend class PoolDataSetBase; - friend class LocalPoolDataSetBase; +class PoolVariableIF : + public SerializeIF, + public ReadCommitIF { + public: - static constexpr uint8_t INTERFACE_ID = CLASS_ID::POOL_VARIABLE_IF; - static constexpr ReturnValue_t INVALID_READ_WRITE_MODE = MAKE_RETURN_CODE(0xA0); - static constexpr ReturnValue_t INVALID_POOL_ENTRY = MAKE_RETURN_CODE(0xA1); + static constexpr uint8_t INTERFACE_ID = CLASS_ID::POOL_VARIABLE_IF; + static constexpr ReturnValue_t INVALID_READ_WRITE_MODE = MAKE_RETURN_CODE(0xA0); + static constexpr ReturnValue_t INVALID_POOL_ENTRY = MAKE_RETURN_CODE(0xA1); - static constexpr bool VALID = 1; - static constexpr bool INVALID = 0; - static constexpr uint32_t NO_PARAMETER = 0xffffffff; + static constexpr bool VALID = 1; + static constexpr bool INVALID = 0; + static constexpr uint32_t NO_PARAMETER = 0xffffffff; - enum ReadWriteMode_t { - VAR_READ, VAR_WRITE, VAR_READ_WRITE - }; + enum ReadWriteMode_t { + VAR_READ, VAR_WRITE, VAR_READ_WRITE + }; - /** - * @brief This is an empty virtual destructor, - * as it is proposed for C++ interfaces. - */ - virtual ~PoolVariableIF() {} - /** - * @brief This method returns if the variable is write-only, - * read-write or read-only. - */ - virtual ReadWriteMode_t getReadWriteMode() const = 0; - /** - * @brief This operation shall return the data pool id of the variable. - */ - virtual uint32_t getDataPoolId() const = 0; - /** - * @brief With this call, the valid information of the - * variable is returned. - */ - virtual bool isValid() const = 0; - /** - * @brief With this call, the valid information of the variable is set. - */ - virtual void setValid(bool validity) = 0; + /** + * @brief This is an empty virtual destructor, + * as it is proposed for C++ interfaces. + */ + virtual ~PoolVariableIF() {} + /** + * @brief This method returns if the variable is write-only, + * read-write or read-only. + */ + virtual ReadWriteMode_t getReadWriteMode() const = 0; + /** + * @brief This operation shall return the data pool id of the variable. + */ + virtual uint32_t getDataPoolId() const = 0; + /** + * @brief With this call, the valid information of the + * variable is returned. + */ + virtual bool isValid() const = 0; + /** + * @brief With this call, the valid information of the variable is set. + */ + virtual void setValid(bool validity) = 0; }; diff --git a/datapool/ReadCommitIF.h b/datapool/ReadCommitIF.h index e6355e82..d8bc5a66 100644 --- a/datapool/ReadCommitIF.h +++ b/datapool/ReadCommitIF.h @@ -9,6 +9,7 @@ * semantics. */ class ReadCommitIF { + friend class ReadCommitIFAttorney; public: virtual ~ReadCommitIF() {} virtual ReturnValue_t read(MutexIF::TimeoutType timeoutType, @@ -16,11 +17,10 @@ public: virtual ReturnValue_t commit(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) = 0; -protected: +public: - //! Optional and protected because this is interesting for classes grouping - //! members with commit and read semantics where the lock is only necessary - //! once. + /* Optional and protected because this is interesting for classes grouping members with commit + and read semantics where the lock is only necessary once. */ virtual ReturnValue_t readWithoutLock() { return read(MutexIF::TimeoutType::WAITING, 20); } diff --git a/datapool/ReadCommitIFAttorney.h b/datapool/ReadCommitIFAttorney.h new file mode 100644 index 00000000..0291cf5c --- /dev/null +++ b/datapool/ReadCommitIFAttorney.h @@ -0,0 +1,32 @@ +#ifndef FSFW_DATAPOOL_READCOMMITIFATTORNEY_H_ +#define FSFW_DATAPOOL_READCOMMITIFATTORNEY_H_ + +#include +#include + +/** + * @brief This class determines which members are allowed to access protected members + * of the ReadCommitIF. + */ +class ReadCommitIFAttorney { +private: + static ReturnValue_t readWithoutLock(ReadCommitIF* readCommitIF) { + if(readCommitIF == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + return readCommitIF->readWithoutLock(); + } + + static ReturnValue_t commitWithoutLock(ReadCommitIF* readCommitIF) { + if(readCommitIF == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + return readCommitIF->commitWithoutLock(); + } + + friend class PoolDataSetBase; +}; + + + +#endif /* FSFW_DATAPOOL_READCOMMITIFATTORNEY_H_ */ From 36039266ee703e86d828a8edf499895bc2d7529a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 13:52:07 +0100 Subject: [PATCH 09/20] some small formatting stuff --- datapool/PoolDataSetBase.cpp | 42 ++++++++++++++---------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 1474aff2..176676ce 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -14,8 +14,7 @@ PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, PoolDataSetBase::~PoolDataSetBase() {} -ReturnValue_t PoolDataSetBase::registerVariable( - PoolVariableIF *variable) { +ReturnValue_t PoolDataSetBase::registerVariable(PoolVariableIF *variable) { if (state != States::STATE_SET_UNINITIALISED) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "DataSet::registerVariable: " @@ -59,10 +58,12 @@ ReturnValue_t PoolDataSetBase::read(MutexIF::TimeoutType timeoutType, } else { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::read(): " - "Call made in wrong position. Don't forget to commit" + sif::error << "DataSet::read(): Call made in wrong position. Don't forget to commit" " member datasets!" << std::endl; -#endif +#else + sif::printError("DataSet::read(): Call made in wrong position. Don't forget to commit" + " member datasets!\n"); +#endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ result = SET_WAS_ALREADY_READ; } @@ -85,14 +86,10 @@ ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) { /* These checks are often performed by the respective variable implementation too, but I guess a double check does not hurt. */ - if (registeredVariables[count]->getReadWriteMode() != - PoolVariableIF::VAR_WRITE and - registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) - { + if (registeredVariables[count]->getReadWriteMode() != PoolVariableIF::VAR_WRITE and + registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) { if(protectEveryReadCommitCall) { - result = registeredVariables[count]->read( - timeoutTypeForSingleVars, + result = registeredVariables[count]->read(timeoutTypeForSingleVars, mutexTimeoutForSingleVars); } else { @@ -121,13 +118,10 @@ void PoolDataSetBase::handleAlreadyReadDatasetCommit( MutexIF::TimeoutType timeoutType, uint32_t lockTimeout) { lockDataPool(timeoutType, lockTimeout); for (uint16_t count = 0; count < fillCount; count++) { - if (registeredVariables[count]->getReadWriteMode() - != PoolVariableIF::VAR_READ - && registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) { + if ((registeredVariables[count]->getReadWriteMode() != PoolVariableIF::VAR_READ) and + (registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER)) { if(protectEveryReadCommitCall) { - registeredVariables[count]->commit( - timeoutTypeForSingleVars, + registeredVariables[count]->commit(timeoutTypeForSingleVars, mutexTimeoutForSingleVars); } else { @@ -144,13 +138,10 @@ ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit( ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; lockDataPool(timeoutType, lockTimeout); for (uint16_t count = 0; count < fillCount; count++) { - if (registeredVariables[count]->getReadWriteMode() - == PoolVariableIF::VAR_WRITE - && registeredVariables[count]->getDataPoolId() - != PoolVariableIF::NO_PARAMETER) { + if ((registeredVariables[count]->getReadWriteMode() == PoolVariableIF::VAR_WRITE) and + (registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER)) { if(protectEveryReadCommitCall) { - result = registeredVariables[count]->commit( - timeoutTypeForSingleVars, + result = registeredVariables[count]->commit(timeoutTypeForSingleVars, mutexTimeoutForSingleVars); } else { @@ -187,8 +178,7 @@ ReturnValue_t PoolDataSetBase::serialize(uint8_t** buffer, size_t* size, const size_t maxSize, SerializeIF::Endianness streamEndianness) const { ReturnValue_t result = HasReturnvaluesIF::RETURN_FAILED; for (uint16_t count = 0; count < fillCount; count++) { - result = registeredVariables[count]->serialize(buffer, size, maxSize, - streamEndianness); + result = registeredVariables[count]->serialize(buffer, size, maxSize, streamEndianness); if (result != HasReturnvaluesIF::RETURN_OK) { return result; } From 35d8453b485b394b0add0883c3117cd37dbc7598 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 13:56:16 +0100 Subject: [PATCH 10/20] fixes for unit tests --- datapool/PoolDataSetBase.cpp | 2 +- osal/host/Clock.cpp | 2 +- osal/host/QueueMapManager.cpp | 1 + unittest/tests/datapoollocal/DataSetTest.cpp | 3 +-- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 176676ce..88470152 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -1,7 +1,7 @@ #include "PoolDataSetBase.h" #include "ReadCommitIFAttorney.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include diff --git a/osal/host/Clock.cpp b/osal/host/Clock.cpp index f0543c79..c097f619 100644 --- a/osal/host/Clock.cpp +++ b/osal/host/Clock.cpp @@ -1,4 +1,4 @@ -#include "../../serviceinterface/ServiceInterfaceStream.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../timemanager/Clock.h" #include diff --git a/osal/host/QueueMapManager.cpp b/osal/host/QueueMapManager.cpp index 9fb8b7a3..2a54f813 100644 --- a/osal/host/QueueMapManager.cpp +++ b/osal/host/QueueMapManager.cpp @@ -1,5 +1,6 @@ #include "QueueMapManager.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../ipc/MutexFactory.h" #include "../../ipc/MutexHelper.h" diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 5c2428ae..370a1e27 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -14,8 +14,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { == retval::CATCH_OK); const uint32_t setId = 0; SECTION("BasicTest") { - StaticLocalDataSet<3> localSet = StaticLocalDataSet<3>( - sid_t(objects::TEST_LOCAL_POOL_OWNER_BASE, setId)); + StaticLocalDataSet<3> localSet(sid_t(objects::TEST_LOCAL_POOL_OWNER_BASE, setId)); } } From d79f0e1172fbf6c6702084777815660c573bceec Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:04:31 +0100 Subject: [PATCH 11/20] some more bugfixes for tests --- datapoollocal/LocalPoolObjectBase.cpp | 4 ++- unittest/tests/datapoollocal/DataSetTest.cpp | 4 +-- .../tests/datapoollocal/LocalPoolOwnerBase.h | 30 +++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/datapoollocal/LocalPoolObjectBase.cpp b/datapoollocal/LocalPoolObjectBase.cpp index 5b9bd34f..7bc91dcb 100644 --- a/datapoollocal/LocalPoolObjectBase.cpp +++ b/datapoollocal/LocalPoolObjectBase.cpp @@ -1,10 +1,12 @@ #include "LocalPoolObjectBase.h" #include "LocalDataPoolManager.h" -#include "internal/HasLocalDpIFUserAttorney.h" +#include "AccessLocalPoolF.h" #include "HasLocalDataPoolIF.h" +#include "internal/HasLocalDpIFUserAttorney.h" #include "../objectmanager/ObjectManagerIF.h" + LocalPoolObjectBase::LocalPoolObjectBase(lp_id_t poolId, HasLocalDataPoolIF* hkOwner, DataSetIF* dataSet, pool_rwm_t setReadWriteMode): localPoolId(poolId), readWriteMode(setReadWriteMode) { diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 370a1e27..6ccb5122 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -12,9 +12,9 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); - const uint32_t setId = 0; + //const uint32_t setId = 0; SECTION("BasicTest") { - StaticLocalDataSet<3> localSet(sid_t(objects::TEST_LOCAL_POOL_OWNER_BASE, setId)); + LocalPoolStaticTestDataSet localSet; } } diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index acbb2b73..6b66f1f0 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -31,6 +31,23 @@ static const gp_id_t uint64Vec2Id = gp_id_t(objects::TEST_LOCAL_POOL_OWNER_BASE, } +class LocalPoolStaticTestDataSet: public StaticLocalDataSet<3> { +public: + LocalPoolStaticTestDataSet(): + StaticLocalDataSet(lpool::testSid) { + } + + LocalPoolStaticTestDataSet(HasLocalDataPoolIF* owner, uint32_t setId): + StaticLocalDataSet(owner, setId) { + } + + lp_var_t localPoolVarUint8 = lp_var_t(lpool::uint8VarGpid, this); + lp_var_t localPoolVarFloat = lp_var_t(lpool::floatVarGpid, this); + lp_vec_t localPoolUint16Vec = lp_vec_t(lpool::uint16Vec3Gpid, this); + +private: +}; + class LocalPoolTestDataSet: public LocalDataSet { public: LocalPoolTestDataSet(): @@ -41,19 +58,6 @@ public: LocalDataSet(owner, setId, lpool::dataSetMaxVariables) { } -// ReturnValue_t assignPointers() { -// PoolVariableIF** rawVarArray = getContainer(); -// localPoolVarUint8 = dynamic_cast*>(rawVarArray[0]); -// localPoolVarFloat = dynamic_cast*>(rawVarArray[1]); -// localPoolUint16Vec = dynamic_cast*>( -// rawVarArray[2]); -// if(localPoolVarUint8 == nullptr or localPoolVarFloat == nullptr or -// localPoolUint16Vec == nullptr) { -// return HasReturnvaluesIF::RETURN_FAILED; -// } -// return HasReturnvaluesIF::RETURN_OK; -// } - lp_var_t localPoolVarUint8 = lp_var_t(lpool::uint8VarGpid, this); lp_var_t localPoolVarFloat = lp_var_t(lpool::floatVarGpid, this); lp_vec_t localPoolUint16Vec = lp_vec_t(lpool::uint16Vec3Gpid, this); From 828115a566f69cfe99f64872f448585eab62ba28 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:35:10 +0100 Subject: [PATCH 12/20] test bugfixes and new reset function --- unittest/tests/datapoollocal/DataSetTest.cpp | 44 +++++++++++++++++- .../datapoollocal/LocalPoolManagerTest.cpp | 2 +- .../tests/datapoollocal/LocalPoolOwnerBase.h | 46 +++++++++++++++++-- .../datapoollocal/LocalPoolVariableTest.cpp | 2 + 4 files changed, 87 insertions(+), 7 deletions(-) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 6ccb5122..7ba56877 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -1,8 +1,11 @@ #include "LocalPoolOwnerBase.h" #include +#include + #include #include +#include #include TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { @@ -12,10 +15,49 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); + LocalPoolStaticTestDataSet localSet; //const uint32_t setId = 0; SECTION("BasicTest") { - LocalPoolStaticTestDataSet localSet; + { + PoolReadHelper readHelper(&localSet); + REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); + CHECK(not localSet.isValid()); + CHECK(localSet.localPoolVarUint8.value == 0); + CHECK(not localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0)); + CHECK(not localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolUint16Vec.value[0] == 0); + CHECK(localSet.localPoolUint16Vec.value[1] == 0); + CHECK(localSet.localPoolUint16Vec.value[2] == 0); + CHECK(not localSet.localPoolVarUint8.isValid()); + + localSet.localPoolVarUint8 = 232; + localSet.localPoolVarFloat = -2324.322; + localSet.localPoolUint16Vec.value[0] = 232; + localSet.localPoolUint16Vec.value[1] = 23923; + localSet.localPoolUint16Vec.value[2] = 1; + localSet.setValidity(true, true); + } + + { + PoolReadHelper readHelper(&localSet); + REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); + CHECK(localSet.isValid()); + CHECK(localSet.localPoolVarUint8.value == 232); + CHECK(localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolVarFloat.value == Catch::Approx(-2324.322)); + CHECK(localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolUint16Vec.value[0] == 232); + CHECK(localSet.localPoolUint16Vec.value[1] == 23923); + CHECK(localSet.localPoolUint16Vec.value[2] == 1); + CHECK(localSet.localPoolVarUint8.isValid()); + } + } + + /* we need to reset the subscription list because the pool owner + is a global object. */ + CHECK(poolOwner->reset() == retval::CATCH_OK); } diff --git a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp index e621f4d8..a10b4499 100644 --- a/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolManagerTest.cpp @@ -192,7 +192,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { /* we need to reset the subscription list because the pool owner is a global object. */ - poolOwner->resetSubscriptionList(); + CHECK(poolOwner->reset() == retval::CATCH_OK); mqMock->clearMessages(true); } diff --git a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h index 6b66f1f0..bb090e3b 100644 --- a/unittest/tests/datapoollocal/LocalPoolOwnerBase.h +++ b/unittest/tests/datapoollocal/LocalPoolOwnerBase.h @@ -10,6 +10,7 @@ #include #include #include +#include "../../../datapool/PoolReadHelper.h" namespace lpool { static constexpr lp_id_t uint8VarId = 0; @@ -187,6 +188,43 @@ public: MessageQueueIF::NO_QUEUE, objects::HK_RECEIVER_MOCK, false); } + ReturnValue_t reset() { + resetSubscriptionList(); + ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; + { + PoolReadHelper readHelper(&dataset); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + dataset.localPoolVarUint8.value = 0; + dataset.localPoolVarFloat.value = 0.0; + dataset.localPoolUint16Vec.value[0] = 0; + dataset.localPoolUint16Vec.value[1] = 0; + dataset.localPoolUint16Vec.value[2] = 0; + dataset.setValidity(false, true); + } + + { + PoolReadHelper readHelper(&testUint32); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + testUint32.value = 0; + testUint32.setValid(false); + } + + { + PoolReadHelper readHelper(&testInt64Vec); + if(readHelper.getReadResult() != HasReturnvaluesIF::RETURN_OK) { + status = readHelper.getReadResult(); + } + testInt64Vec.value[0] = 0; + testInt64Vec.value[1] = 0; + testInt64Vec.setValid(false); + } + return status; + } + void resetSubscriptionList() { poolManager.clearReceiversList(); } @@ -195,14 +233,12 @@ public: LocalPoolTestDataSet dataset; private: - lp_var_t testUint8 = lp_var_t(this, lpool::uint8VarId, - &dataset); - lp_var_t testFloat = lp_var_t(this, lpool::floatVarId, - &dataset); + lp_var_t testUint8 = lp_var_t(this, lpool::uint8VarId); + lp_var_t testFloat = lp_var_t(this, lpool::floatVarId); lp_var_t testUint32 = lp_var_t(this, lpool::uint32VarId); lp_vec_t testUint16Vec = lp_vec_t(this, - lpool::uint16Vec3Id, &dataset); + lpool::uint16Vec3Id); lp_vec_t testInt64Vec = lp_vec_t(this, lpool::int64Vec2Id); diff --git a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp index eb99e58d..e5a5d364 100644 --- a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp @@ -118,6 +118,8 @@ TEST_CASE("LocalPoolVariable" , "[LocPoolVarTest]") { lpool::uint8VarId); } + CHECK(poolOwner->reset() == retval::CATCH_OK); + } From fb5a1b93fce6c3e107050395d31303021928d79b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:38:01 +0100 Subject: [PATCH 13/20] unneeded variable removed --- unittest/tests/datapoollocal/DataSetTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 7ba56877..18503f05 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -16,7 +16,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); LocalPoolStaticTestDataSet localSet; - //const uint32_t setId = 0; + SECTION("BasicTest") { { PoolReadHelper readHelper(&localSet); From 68415853b5040733ff9f5d2739dd1bc704769bd1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:41:43 +0100 Subject: [PATCH 14/20] read commit IF functions protected again --- datapool/PoolDataSetBase.cpp | 5 ++++- datapool/ReadCommitIF.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 88470152..e7572b27 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -93,6 +93,7 @@ ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) { mutexTimeoutForSingleVars); } else { + /* The readWithoutLock function is protected, so we use the attorney here */ result = ReadCommitIFAttorney::readWithoutLock(registeredVariables[count]); } @@ -125,6 +126,7 @@ void PoolDataSetBase::handleAlreadyReadDatasetCommit( mutexTimeoutForSingleVars); } else { + /* The commitWithoutLock function is protected, so we use the attorney here */ ReadCommitIFAttorney::commitWithoutLock(registeredVariables[count]); } } @@ -145,7 +147,8 @@ ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit( mutexTimeoutForSingleVars); } else { - result = registeredVariables[count]->commitWithoutLock(); + /* The commitWithoutLock function is protected, so we use the attorney here */ + ReadCommitIFAttorney::commitWithoutLock(registeredVariables[count]); } } else if (registeredVariables[count]->getDataPoolId() diff --git a/datapool/ReadCommitIF.h b/datapool/ReadCommitIF.h index d8bc5a66..3ad2b3c0 100644 --- a/datapool/ReadCommitIF.h +++ b/datapool/ReadCommitIF.h @@ -17,7 +17,7 @@ public: virtual ReturnValue_t commit(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) = 0; -public: +protected: /* Optional and protected because this is interesting for classes grouping members with commit and read semantics where the lock is only necessary once. */ From 16566a5690159a84a88bdfb5f8a276feb1050da3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:45:09 +0100 Subject: [PATCH 15/20] nullptr check added --- datapoollocal/LocalPoolObjectBase.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/datapoollocal/LocalPoolObjectBase.cpp b/datapoollocal/LocalPoolObjectBase.cpp index 7bc91dcb..a64ed2b4 100644 --- a/datapoollocal/LocalPoolObjectBase.cpp +++ b/datapoollocal/LocalPoolObjectBase.cpp @@ -95,6 +95,10 @@ void LocalPoolObjectBase::setReadWriteMode(pool_rwm_t newReadWriteMode) { void LocalPoolObjectBase::reportReadCommitError(const char* variableType, ReturnValue_t error, bool read, object_id_t objectId, lp_id_t lpId) { #if FSFW_DISABLE_PRINTOUT == 0 + const char* variablePrintout = variableType; + if(variablePrintout == nullptr) { + variablePrintout = "Unknown Type"; + } const char* type = nullptr; if(read) { type = "read"; @@ -121,12 +125,12 @@ void LocalPoolObjectBase::reportReadCommitError(const char* variableType, } #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << variableType << ": " << type << " call | " << errMsg << " | Owner: 0x" + sif::warning << variablePrintout << ": " << type << " call | " << errMsg << " | Owner: 0x" << std::hex << std::setw(8) << std::setfill('0') << objectId << std::dec << " LPID: " << lpId << std::endl; #else sif::printWarning("%s: %s call | %s | Owner: 0x%08x LPID: %lu\n", - variableType, type, errMsg, objectId, lpId); + variablePrintout, type, errMsg, objectId, lpId); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif /* FSFW_DISABLE_PRINTOUT == 0 */ } From 304773f7a7dbf84ab98c2b4c62c60f792cd07f1d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 14:54:03 +0100 Subject: [PATCH 16/20] added some failure test cases --- datapool/DataSetIF.h | 14 ++++++-------- datapool/PoolDataSetBase.cpp | 15 +++++++++------ unittest/tests/datapoollocal/DataSetTest.cpp | 8 ++++++++ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/datapool/DataSetIF.h b/datapool/DataSetIF.h index a6634a5c..ea1a7126 100644 --- a/datapool/DataSetIF.h +++ b/datapool/DataSetIF.h @@ -18,15 +18,13 @@ class PoolVariableIF; class DataSetIF { public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::DATA_SET_CLASS; - static constexpr ReturnValue_t INVALID_PARAMETER_DEFINITION = - MAKE_RETURN_CODE( 0x01 ); - static constexpr ReturnValue_t SET_WAS_ALREADY_READ = MAKE_RETURN_CODE( 0x02 ); - static constexpr ReturnValue_t COMMITING_WITHOUT_READING = - MAKE_RETURN_CODE(0x03); + static constexpr ReturnValue_t INVALID_PARAMETER_DEFINITION = MAKE_RETURN_CODE(1); + static constexpr ReturnValue_t SET_WAS_ALREADY_READ = MAKE_RETURN_CODE(2); + static constexpr ReturnValue_t COMMITING_WITHOUT_READING = MAKE_RETURN_CODE(3); - static constexpr ReturnValue_t DATA_SET_UNINITIALISED = MAKE_RETURN_CODE( 0x04 ); - static constexpr ReturnValue_t DATA_SET_FULL = MAKE_RETURN_CODE( 0x05 ); - static constexpr ReturnValue_t POOL_VAR_NULL = MAKE_RETURN_CODE( 0x06 ); + static constexpr ReturnValue_t DATA_SET_UNINITIALISED = MAKE_RETURN_CODE(4); + static constexpr ReturnValue_t DATA_SET_FULL = MAKE_RETURN_CODE(5); + static constexpr ReturnValue_t POOL_VAR_NULL = MAKE_RETURN_CODE(6); /** * @brief This is an empty virtual destructor, diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index e7572b27..bdca22c3 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -17,22 +17,25 @@ PoolDataSetBase::~PoolDataSetBase() {} ReturnValue_t PoolDataSetBase::registerVariable(PoolVariableIF *variable) { if (state != States::STATE_SET_UNINITIALISED) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "Call made in wrong position." << std::endl; + sif::error << "DataSet::registerVariable: Call made in wrong position." << std::endl; +#else + sif::printError("DataSet::registerVariable: Call made in wrong position."); #endif return DataSetIF::DATA_SET_UNINITIALISED; } if (variable == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "Pool variable is nullptr." << std::endl; + sif::error << "DataSet::registerVariable: Pool variable is nullptr." << std::endl; +#else + sif::printError("DataSet::registerVariable: Pool variable is nullptr.\n"); #endif return DataSetIF::POOL_VAR_NULL; } if (fillCount >= maxFillCount) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DataSet::registerVariable: " - "DataSet is full." << std::endl; + sif::error << "DataSet::registerVariable: DataSet is full." << std::endl; +#else + sif::printError("DataSet::registerVariable: DataSet is full.\n"); #endif return DataSetIF::DATA_SET_FULL; } diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 18503f05..d829e775 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -53,6 +53,14 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(localSet.localPoolVarUint8.isValid()); } + /* Common fault test cases */ + LocalPoolObjectBase* variableHandle = poolOwner->getPoolObjectHandle(lpool::uint32VarId); + CHECK(variableHandle != nullptr); + CHECK(localSet.registerVariable(variableHandle) == + static_cast(DataSetIF::DATA_SET_FULL)); + variableHandle = nullptr; + REQUIRE(localSet.registerVariable(variableHandle) == + static_cast(DataSetIF::POOL_VAR_NULL)); } /* we need to reset the subscription list because the pool owner From 50ba3773809a170b858f4eece60f69ad1c65c7af Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 15:34:04 +0100 Subject: [PATCH 17/20] more set tests and new function to suppress commits --- datapool/PoolReadHelper.h | 13 ++++- datapoollocal/LocalPoolDataSetBase.cpp | 2 +- unittest/tests/datapoollocal/DataSetTest.cpp | 55 ++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/datapool/PoolReadHelper.h b/datapool/PoolReadHelper.h index 5c3153bb..9825e83c 100644 --- a/datapool/PoolReadHelper.h +++ b/datapool/PoolReadHelper.h @@ -32,8 +32,18 @@ public: return readResult; } + /** + * @brief Can be used to suppress commit on destruction. + */ + void setNoCommitMode(bool commit) { + this->noCommit = commit; + } + + /** + * @brief Default destructor which will take care of commiting changed values. + */ ~PoolReadHelper() { - if(readObject != nullptr) { + if(readObject != nullptr and not noCommit) { readObject->commit(timeoutType, mutexTimeout); } @@ -42,6 +52,7 @@ public: private: ReadCommitIF* readObject = nullptr; ReturnValue_t readResult = HasReturnvaluesIF::RETURN_OK; + bool noCommit = false; MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING; uint32_t mutexTimeout = 20; }; diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index d1d95d8a..9f23fc6c 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -173,7 +173,7 @@ ReturnValue_t LocalPoolDataSetBase::unlockDataPool() { ReturnValue_t LocalPoolDataSetBase::serializeLocalPoolIds(uint8_t** buffer, size_t* size, size_t maxSize,SerializeIF::Endianness streamEndianness, bool serializeFillCount) const { - // Serialize as uint8_t + /* Serialize fill count as uint8_t */ uint8_t fillCount = this->fillCount; if(serializeFillCount) { SerializeAdapter::serialize(&fillCount, buffer, size, maxSize, diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index d829e775..eb8e9b60 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -18,7 +18,40 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { LocalPoolStaticTestDataSet localSet; SECTION("BasicTest") { + /* Test some basic functions */ + CHECK(localSet.getLocalPoolIdsSerializedSize(false) == 3 * sizeof(lp_id_t)); + CHECK(localSet.getLocalPoolIdsSerializedSize(true) == + 3 * sizeof(lp_id_t) + sizeof(uint8_t)); + CHECK(localSet.getSid() == lpool::testSid); + CHECK(localSet.getCreatorObjectId() == objects::TEST_LOCAL_POOL_OWNER_BASE); + size_t maxSize = localSet.getLocalPoolIdsSerializedSize(true); + uint8_t localPoolIdBuff[maxSize]; + /* Skip size field */ + lp_id_t* lpIds = reinterpret_cast(localPoolIdBuff + 1); + size_t serSize = 0; + uint8_t *localPoolIdBuffPtr = reinterpret_cast(localPoolIdBuff); + + /* Test local pool ID serialization */ + CHECK(localSet.serializeLocalPoolIds(&localPoolIdBuffPtr, &serSize, + maxSize, SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + CHECK(serSize == maxSize); + CHECK(localPoolIdBuff[0] == 3); + CHECK(lpIds[0] == localSet.localPoolVarUint8.getDataPoolId()); + CHECK(lpIds[1] == localSet.localPoolVarFloat.getDataPoolId()); + CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId()); + /* Now serialize without fill count */ + lpIds = reinterpret_cast(localPoolIdBuff); + localPoolIdBuffPtr = localPoolIdBuff; + serSize = 0; + CHECK(localSet.serializeLocalPoolIds(&localPoolIdBuffPtr, &serSize, + maxSize, SerializeIF::Endianness::MACHINE, false) == retval::CATCH_OK); + CHECK(serSize == maxSize - sizeof(uint8_t)); + CHECK(lpIds[0] == localSet.localPoolVarUint8.getDataPoolId()); + CHECK(lpIds[1] == localSet.localPoolVarFloat.getDataPoolId()); + CHECK(lpIds[2] == localSet.localPoolUint16Vec.getDataPoolId()); + { + /* Test read operation. Values should be all zeros */ PoolReadHelper readHelper(&localSet); REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); CHECK(not localSet.isValid()); @@ -31,6 +64,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(localSet.localPoolUint16Vec.value[2] == 0); CHECK(not localSet.localPoolVarUint8.isValid()); + /* Now set new values, commit should be done by read helper automatically */ localSet.localPoolVarUint8 = 232; localSet.localPoolVarFloat = -2324.322; localSet.localPoolUint16Vec.value[0] = 232; @@ -51,6 +85,27 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(localSet.localPoolUint16Vec.value[1] == 23923); CHECK(localSet.localPoolUint16Vec.value[2] == 1); CHECK(localSet.localPoolVarUint8.isValid()); + + localSet.setValidityBufferGeneration(false); + maxSize = localSet.getSerializedSize(); + CHECK(maxSize == sizeof(uint8_t) + sizeof(uint16_t) * 3 + sizeof(float)); + serSize = 0; + uint8_t buffer[maxSize]; + uint8_t* buffPtr = buffer; + CHECK(localSet.serialize(&buffPtr, &serSize, maxSize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + uint8_t rawUint8 = buffer[0]; + CHECK(rawUint8 == 232); + float rawFloat = 0.0; + std::memcpy(&rawFloat, buffer + sizeof(uint8_t), sizeof(float)); + CHECK(rawFloat == Catch::Approx(-2324.322)); + + uint16_t rawUint16Vec[3]; + std::memcpy(&rawUint16Vec, buffer + sizeof(uint8_t) + sizeof(float), + 3 * sizeof(uint16_t)); + CHECK(rawUint16Vec[0] == 232); + CHECK(rawUint16Vec[1] == 23923); + CHECK(rawUint16Vec[2] == 1); } /* Common fault test cases */ From ffce336801d157c2ee9df2dc6feb682a5d125228 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 15:44:05 +0100 Subject: [PATCH 18/20] set tests continued --- unittest/tests/datapoollocal/DataSetTest.cpp | 35 ++++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index eb8e9b60..929e5d76 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -73,24 +73,32 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { localSet.setValidity(true, true); } + /* Zero out some values for next test */ + localSet.localPoolVarUint8 = 0; + localSet.localPoolVarFloat = 0; + { + /* Now we read again and check whether our zeroed values were overwritten with + the values in the pool */ PoolReadHelper readHelper(&localSet); REQUIRE(readHelper.getReadResult() == retval::CATCH_OK); CHECK(localSet.isValid()); CHECK(localSet.localPoolVarUint8.value == 232); CHECK(localSet.localPoolVarUint8.isValid()); CHECK(localSet.localPoolVarFloat.value == Catch::Approx(-2324.322)); - CHECK(localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolVarFloat.isValid()); CHECK(localSet.localPoolUint16Vec.value[0] == 232); CHECK(localSet.localPoolUint16Vec.value[1] == 23923); CHECK(localSet.localPoolUint16Vec.value[2] == 1); - CHECK(localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolUint16Vec.isValid()); + /* Now we serialize these values into a buffer without the validity buffer */ localSet.setValidityBufferGeneration(false); maxSize = localSet.getSerializedSize(); CHECK(maxSize == sizeof(uint8_t) + sizeof(uint16_t) * 3 + sizeof(float)); serSize = 0; - uint8_t buffer[maxSize]; + /* Already reserve additional space for validity buffer, will be needed later */ + uint8_t buffer[maxSize + 1]; uint8_t* buffPtr = buffer; CHECK(localSet.serialize(&buffPtr, &serSize, maxSize, SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); @@ -106,6 +114,27 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(rawUint16Vec[0] == 232); CHECK(rawUint16Vec[1] == 23923); CHECK(rawUint16Vec[2] == 1); + + size_t sizeToDeserialize = maxSize; + /* Now we zeros out the raw entries and deserialize back into the dataset */ + std::memset(buffer, 0, sizeof(buffer)); + const uint8_t* constBuffPtr = buffer; + CHECK(localSet.deSerialize(&constBuffPtr, &sizeToDeserialize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + /* Check whether deserialization was successfull */ + CHECK(localSet.localPoolVarUint8.value == 0); + CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0)); + CHECK(localSet.localPoolVarUint8.value == 0); + CHECK(localSet.localPoolUint16Vec.value[0] == 0); + CHECK(localSet.localPoolUint16Vec.value[1] == 0); + CHECK(localSet.localPoolUint16Vec.value[2] == 0); + /* Validity should be unchanged */ + CHECK(localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolVarFloat.isValid()); + CHECK(localSet.localPoolUint16Vec.isValid()); + + /* Now we do the same process but with the validity buffer */ + localSet.setValidityBufferGeneration(true); } /* Common fault test cases */ From 714f11f1170ac3669e155c9fdd68632349464d1d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 16:17:07 +0100 Subject: [PATCH 19/20] more tests and validity buffer bugfix --- datapoollocal/LocalPoolDataSetBase.cpp | 77 ++++++++++---------- datapoollocal/LocalPoolDataSetBase.h | 14 ++-- unittest/tests/datapoollocal/DataSetTest.cpp | 33 +++++++++ 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 9f23fc6c..085ae227 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -96,22 +96,22 @@ ReturnValue_t LocalPoolDataSetBase::serializeWithValidityBuffer(uint8_t **buffer SerializeIF::Endianness streamEndianness) const { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; uint8_t validityMaskSize = std::ceil(static_cast(fillCount)/8.0); - uint8_t validityMask[validityMaskSize]; + uint8_t validityMask[validityMaskSize] = {}; uint8_t validBufferIndex = 0; uint8_t validBufferIndexBit = 0; for (uint16_t count = 0; count < fillCount; count++) { if(registeredVariables[count]->isValid()) { - // set validity buffer here. - this->bitSetter(validityMask + validBufferIndex, - validBufferIndexBit); - if(validBufferIndexBit == 7) { - validBufferIndex ++; - validBufferIndexBit = 0; - } - else { - validBufferIndexBit ++; - } + /* Set bit at correct position */ + this->bitSetter(validityMask + validBufferIndex, validBufferIndexBit); } + if(validBufferIndexBit == 7) { + validBufferIndex ++; + validBufferIndexBit = 0; + } + else { + validBufferIndexBit ++; + } + result = registeredVariables[count]->serialize(buffer, size, maxSize, streamEndianness); if (result != HasReturnvaluesIF::RETURN_OK) { @@ -246,21 +246,6 @@ ReturnValue_t LocalPoolDataSetBase::serialize(uint8_t **buffer, size_t *size, } } -void LocalPoolDataSetBase::bitSetter(uint8_t* byte, uint8_t position) const { - if(position > 7) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolDataSetBase::bitSetter: Invalid position!" - << std::endl; -#else - sif::printWarning("LocalPoolDataSetBase::bitSetter: " - "Invalid position!\n\r"); -#endif - return; - } - uint8_t shiftNumber = position + (7 - 2 * position); - *byte |= 1 << shiftNumber; -} - void LocalPoolDataSetBase::setDiagnostic(bool isDiagnostics) { this->diagnostic = isDiagnostics; } @@ -296,19 +281,6 @@ sid_t LocalPoolDataSetBase::getSid() const { return sid; } -bool LocalPoolDataSetBase::bitGetter(const uint8_t* byte, - uint8_t position) const { - if(position > 7) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "Pool Raw Access: Bit setting invalid position" - << std::endl; -#endif - return false; - } - uint8_t shiftNumber = position + (7 - 2 * position); - return *byte & (1 << shiftNumber); -} - bool LocalPoolDataSetBase::isValid() const { return this->valid; } @@ -328,3 +300,30 @@ object_id_t LocalPoolDataSetBase::getCreatorObjectId() { } return objects::NO_OBJECT; } + +void LocalPoolDataSetBase::bitSetter(uint8_t* byte, uint8_t position) { + if(position > 7) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "LocalPoolDataSetBase::bitSetter: Invalid position!" + << std::endl; +#else + sif::printWarning("LocalPoolDataSetBase::bitSetter: " + "Invalid position!\n\r"); +#endif + return; + } + uint8_t shiftNumber = position + (7 - 2 * position); + *byte |= 1 << shiftNumber; +} + +bool LocalPoolDataSetBase::bitGetter(const uint8_t* byte, uint8_t position) { + if(position > 7) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::debug << "Pool Raw Access: Bit setting invalid position" + << std::endl; +#endif + return false; + } + uint8_t shiftNumber = position + (7 - 2 * position); + return *byte & (1 << shiftNumber); +} diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index ca907431..c676285c 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -160,6 +160,13 @@ public: object_id_t getCreatorObjectId(); + /* Static helper functions for manipulating validity buffers */ + /** + * Set n-th bit of a byte, with n being the position from 0 + * (most significant bit) to 7 (least significant bit) + */ + static void bitSetter(uint8_t* byte, uint8_t position); + static bool bitGetter(const uint8_t* byte, uint8_t position); protected: sid_t sid; //! This mutex is used if the data is created by one object only. @@ -218,13 +225,6 @@ protected: */ ReturnValue_t unlockDataPool() override; - /** - * Set n-th bit of a byte, with n being the position from 0 - * (most significant bit) to 7 (least significant bit) - */ - void bitSetter(uint8_t* byte, uint8_t position) const; - bool bitGetter(const uint8_t* byte, uint8_t position) const; - PeriodicHousekeepingHelper* periodicHelper = nullptr; LocalDataPoolManager* poolManager = nullptr; diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 929e5d76..7b2632c2 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -134,7 +134,39 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(localSet.localPoolUint16Vec.isValid()); /* Now we do the same process but with the validity buffer */ + localSet.localPoolVarUint8 = 232; + localSet.localPoolVarFloat = -2324.322; + localSet.localPoolUint16Vec.value[0] = 232; + localSet.localPoolUint16Vec.value[1] = 23923; + localSet.localPoolUint16Vec.value[2] = 1; + localSet.localPoolVarUint8.setValid(true); + localSet.localPoolVarFloat.setValid(false); + localSet.localPoolUint16Vec.setValid(true); localSet.setValidityBufferGeneration(true); + maxSize = localSet.getSerializedSize(); + CHECK(maxSize == sizeof(uint8_t) + sizeof(uint16_t) * 3 + sizeof(float) + 1); + serSize = 0; + buffPtr = buffer; + CHECK(localSet.serialize(&buffPtr, &serSize, maxSize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + CHECK(rawUint8 == 232); + std::memcpy(&rawFloat, buffer + sizeof(uint8_t), sizeof(float)); + CHECK(rawFloat == Catch::Approx(-2324.322)); + + std::memcpy(&rawUint16Vec, buffer + sizeof(uint8_t) + sizeof(float), + 3 * sizeof(uint16_t)); + CHECK(rawUint16Vec[0] == 232); + CHECK(rawUint16Vec[1] == 23923); + CHECK(rawUint16Vec[2] == 1); + /* We can do it like this because the buffer only has one byte for + less than 8 variables */ + uint8_t validityByte = buffer[sizeof(buffer) - 1]; + CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 0) == true); + CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 1) == false); + CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 2) == true); + + /* Now we manipulate the validity buffer for the deserialization */ + } /* Common fault test cases */ @@ -145,6 +177,7 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { variableHandle = nullptr; REQUIRE(localSet.registerVariable(variableHandle) == static_cast(DataSetIF::POOL_VAR_NULL)); + } /* we need to reset the subscription list because the pool owner From 940bbf47e425dc0bdd70d1cba1c95a981d4d2db9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 28 Feb 2021 16:34:11 +0100 Subject: [PATCH 20/20] set deser test complete new bitutility file --- datapoollocal/LocalPoolDataSetBase.cpp | 31 ++---------------- datapoollocal/LocalPoolDataSetBase.h | 7 ----- globalfunctions/CMakeLists.txt | 3 +- globalfunctions/bitutility.cpp | 33 ++++++++++++++++++++ globalfunctions/bitutility.h | 18 +++++++++++ unittest/tests/datapoollocal/DataSetTest.cpp | 29 ++++++++++++++--- 6 files changed, 81 insertions(+), 40 deletions(-) create mode 100644 globalfunctions/bitutility.cpp create mode 100644 globalfunctions/bitutility.h diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 085ae227..75a3ab7c 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -3,6 +3,7 @@ #include "internal/HasLocalDpIFUserAttorney.h" #include "../serviceinterface/ServiceInterface.h" +#include "../globalfunctions/bitutility.h" #include "../datapoollocal/LocalDataPoolManager.h" #include "../housekeeping/PeriodicHousekeepingHelper.h" #include "../serialize/SerializeAdapter.h" @@ -102,7 +103,7 @@ ReturnValue_t LocalPoolDataSetBase::serializeWithValidityBuffer(uint8_t **buffer for (uint16_t count = 0; count < fillCount; count++) { if(registeredVariables[count]->isValid()) { /* Set bit at correct position */ - this->bitSetter(validityMask + validBufferIndex, validBufferIndexBit); + bitutil::bitSet(validityMask + validBufferIndex, validBufferIndexBit); } if(validBufferIndexBit == 7) { validBufferIndex ++; @@ -148,7 +149,7 @@ ReturnValue_t LocalPoolDataSetBase::deSerializeWithValidityBuffer( uint8_t validBufferIndexBit = 0; for (uint16_t count = 0; count < fillCount; count++) { // set validity buffer here. - bool nextVarValid = this->bitGetter(*buffer + + bool nextVarValid = bitutil::bitGet(*buffer + validBufferIndex, validBufferIndexBit); registeredVariables[count]->setValid(nextVarValid); @@ -301,29 +302,3 @@ object_id_t LocalPoolDataSetBase::getCreatorObjectId() { return objects::NO_OBJECT; } -void LocalPoolDataSetBase::bitSetter(uint8_t* byte, uint8_t position) { - if(position > 7) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolDataSetBase::bitSetter: Invalid position!" - << std::endl; -#else - sif::printWarning("LocalPoolDataSetBase::bitSetter: " - "Invalid position!\n\r"); -#endif - return; - } - uint8_t shiftNumber = position + (7 - 2 * position); - *byte |= 1 << shiftNumber; -} - -bool LocalPoolDataSetBase::bitGetter(const uint8_t* byte, uint8_t position) { - if(position > 7) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "Pool Raw Access: Bit setting invalid position" - << std::endl; -#endif - return false; - } - uint8_t shiftNumber = position + (7 - 2 * position); - return *byte & (1 << shiftNumber); -} diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index c676285c..8a3a6339 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -160,13 +160,6 @@ public: object_id_t getCreatorObjectId(); - /* Static helper functions for manipulating validity buffers */ - /** - * Set n-th bit of a byte, with n being the position from 0 - * (most significant bit) to 7 (least significant bit) - */ - static void bitSetter(uint8_t* byte, uint8_t position); - static bool bitGetter(const uint8_t* byte, uint8_t position); protected: sid_t sid; //! This mutex is used if the data is created by one object only. diff --git a/globalfunctions/CMakeLists.txt b/globalfunctions/CMakeLists.txt index 2b3dcf8e..5ccd3c4c 100644 --- a/globalfunctions/CMakeLists.txt +++ b/globalfunctions/CMakeLists.txt @@ -7,6 +7,7 @@ target_sources(${LIB_FSFW_NAME} PeriodicOperationDivider.cpp timevalOperations.cpp Type.cpp + bitutility.cpp ) -add_subdirectory(math) \ No newline at end of file +add_subdirectory(math) diff --git a/globalfunctions/bitutility.cpp b/globalfunctions/bitutility.cpp new file mode 100644 index 00000000..5cc92dac --- /dev/null +++ b/globalfunctions/bitutility.cpp @@ -0,0 +1,33 @@ +#include "bitutility.h" + +void bitutil::bitSet(uint8_t *byte, uint8_t position) { + if(position > 7) { + return; + } + uint8_t shiftNumber = position + (7 - 2 * position); + *byte |= 1 << shiftNumber; +} + +void bitutil::bitToggle(uint8_t *byte, uint8_t position) { + if(position > 7) { + return; + } + uint8_t shiftNumber = position + (7 - 2 * position); + *byte ^= 1 << shiftNumber; +} + +void bitutil::bitClear(uint8_t *byte, uint8_t position) { + if(position > 7) { + return; + } + uint8_t shiftNumber = position + (7 - 2 * position); + *byte &= ~(1 << shiftNumber); +} + +bool bitutil::bitGet(const uint8_t *byte, uint8_t position) { + if(position > 7) { + return false; + } + uint8_t shiftNumber = position + (7 - 2 * position); + return *byte & (1 << shiftNumber); +} diff --git a/globalfunctions/bitutility.h b/globalfunctions/bitutility.h new file mode 100644 index 00000000..1fc1290d --- /dev/null +++ b/globalfunctions/bitutility.h @@ -0,0 +1,18 @@ +#ifndef FSFW_GLOBALFUNCTIONS_BITUTIL_H_ +#define FSFW_GLOBALFUNCTIONS_BITUTIL_H_ + +#include + +namespace bitutil { + +/* Helper functions for manipulating the individual bits of a byte. +Position refers to n-th bit of a byte, going from 0 (most significant bit) to +7 (least significant bit) */ +void bitSet(uint8_t* byte, uint8_t position); +void bitToggle(uint8_t* byte, uint8_t position); +void bitClear(uint8_t* byte, uint8_t position); +bool bitGet(const uint8_t* byte, uint8_t position); + +} + +#endif /* FSFW_GLOBALFUNCTIONS_BITUTIL_H_ */ diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index 7b2632c2..56134595 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -6,6 +6,8 @@ #include #include #include +#include + #include TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { @@ -160,12 +162,31 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { CHECK(rawUint16Vec[2] == 1); /* We can do it like this because the buffer only has one byte for less than 8 variables */ - uint8_t validityByte = buffer[sizeof(buffer) - 1]; - CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 0) == true); - CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 1) == false); - CHECK(LocalPoolDataSetBase::bitGetter(&validityByte, 2) == true); + uint8_t* validityByte = buffer + sizeof(buffer) - 1; + CHECK(bitutil::bitGet(validityByte, 0) == true); + CHECK(bitutil::bitGet(validityByte, 1) == false); + CHECK(bitutil::bitGet(validityByte, 2) == true); /* Now we manipulate the validity buffer for the deserialization */ + bitutil::bitClear(validityByte, 0); + bitutil::bitSet(validityByte, 1); + bitutil::bitClear(validityByte, 2); + /* Zero out everything except validity buffer */ + std::memset(buffer, 0, sizeof(buffer) - 1); + sizeToDeserialize = maxSize; + constBuffPtr = buffer; + CHECK(localSet.deSerialize(&constBuffPtr, &sizeToDeserialize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + /* Check whether deserialization was successfull */ + CHECK(localSet.localPoolVarUint8.value == 0); + CHECK(localSet.localPoolVarFloat.value == Catch::Approx(0.0)); + CHECK(localSet.localPoolVarUint8.value == 0); + CHECK(localSet.localPoolUint16Vec.value[0] == 0); + CHECK(localSet.localPoolUint16Vec.value[1] == 0); + CHECK(localSet.localPoolUint16Vec.value[2] == 0); + CHECK(not localSet.localPoolVarUint8.isValid()); + CHECK(localSet.localPoolVarFloat.isValid()); + CHECK(not localSet.localPoolUint16Vec.isValid()); }