From 535b51ef7046b9e056371092737c07069dbb2ff1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:15:52 +0100 Subject: [PATCH] improvements and clarifications --- datapool/PoolDataSetBase.cpp | 7 ++++++- datapool/PoolDataSetBase.h | 15 ++++++++++----- datapoollocal/HasLocalDataPoolIF.h | 3 +++ datapoollocal/LocalDataPoolManager.h | 15 ++++++--------- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index ad1de8e4..63cbe73e 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -35,12 +35,13 @@ ReturnValue_t PoolDataSetBase::registerVariable( ReturnValue_t PoolDataSetBase::read(uint32_t lockTimeout) { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + ReturnValue_t error = result; if (state == States::STATE_SET_UNINITIALISED) { lockDataPool(lockTimeout); for (uint16_t count = 0; count < fillCount; count++) { result = readVariable(count); if(result != RETURN_OK) { - break; + error = result; } } state = States::STATE_SET_WAS_READ; @@ -52,6 +53,10 @@ ReturnValue_t PoolDataSetBase::read(uint32_t lockTimeout) { " member datasets!" << std::endl; result = SET_WAS_ALREADY_READ; } + + if(error != HasReturnvaluesIF::RETURN_OK) { + result = error; + } return result; } diff --git a/datapool/PoolDataSetBase.h b/datapool/PoolDataSetBase.h index 3f52fcd0..18715b1b 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -44,7 +44,11 @@ public: /** * @brief The read call initializes reading out all registered variables. + * It is mandatory to call commit after every read call! * @details + * + * TODO: Write RAII helper so user can not forget to call commit anymore. + * * 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. @@ -52,13 +56,14 @@ public: * the operation is aborted and @c INVALID_PARAMETER_DEFINITION returned. * * The data pool is locked during the whole read operation and - * freed afterwards.The state changes to "was written" after this operation. + * 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 PID, size or type of the - * requested variable is invalid. + * - @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 + * commit() in between */ virtual ReturnValue_t read(uint32_t lockTimeout = MutexIF::BLOCKING) override; @@ -75,7 +80,7 @@ public: * 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. + * 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 diff --git a/datapoollocal/HasLocalDataPoolIF.h b/datapoollocal/HasLocalDataPoolIF.h index 7f1f202e..df855110 100644 --- a/datapoollocal/HasLocalDataPoolIF.h +++ b/datapoollocal/HasLocalDataPoolIF.h @@ -44,6 +44,9 @@ public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::LOCAL_POOL_OWNER_IF; + static constexpr ReturnValue_t POOL_ENTRY_NOT_FOUND = MAKE_RETURN_CODE(0x00); + static constexpr ReturnValue_t POOL_ENTRY_TYPE_CONFLICT = MAKE_RETURN_CODE(0x01); + static constexpr uint32_t INVALID_LPID = localpool::INVALID_LPID; virtual object_id_t getObjectId() const = 0; diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index c4024cf9..cdefa2c2 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -55,14 +55,11 @@ class LocalDataPoolManager { public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::HOUSEKEEPING_MANAGER; - static constexpr ReturnValue_t POOL_ENTRY_NOT_FOUND = MAKE_RETURN_CODE(0x00); - static constexpr ReturnValue_t POOL_ENTRY_TYPE_CONFLICT = MAKE_RETURN_CODE(0x01); + static constexpr ReturnValue_t QUEUE_OR_DESTINATION_NOT_SET = MAKE_RETURN_CODE(0x0); - static constexpr ReturnValue_t QUEUE_OR_DESTINATION_NOT_SET = MAKE_RETURN_CODE(0x02); - - static constexpr ReturnValue_t WRONG_HK_PACKET_TYPE = MAKE_RETURN_CODE(0x03); - static constexpr ReturnValue_t REPORTING_STATUS_UNCHANGED = MAKE_RETURN_CODE(0x04); - static constexpr ReturnValue_t PERIODIC_HELPER_INVALID = MAKE_RETURN_CODE(0x05); + static constexpr ReturnValue_t WRONG_HK_PACKET_TYPE = MAKE_RETURN_CODE(0x01); + static constexpr ReturnValue_t REPORTING_STATUS_UNCHANGED = MAKE_RETURN_CODE(0x02); + static constexpr ReturnValue_t PERIODIC_HELPER_INVALID = MAKE_RETURN_CODE(0x03); /** * This constructor is used by a class which wants to implement @@ -380,14 +377,14 @@ ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, if (poolIter == localPoolMap.end()) { sif::warning << "HousekeepingManager::fechPoolEntry: Pool entry " "not found." << std::endl; - return POOL_ENTRY_NOT_FOUND; + return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } *poolEntry = dynamic_cast< PoolEntry* >(poolIter->second); if(*poolEntry == nullptr) { sif::debug << "HousekeepingManager::fetchPoolEntry:" " Pool entry not found." << std::endl; - return POOL_ENTRY_TYPE_CONFLICT; + return HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT; } return HasReturnvaluesIF::RETURN_OK; }