From fc4da433d504efec43814e54dd91c62ed91b13c2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 13:49:42 +0100 Subject: [PATCH 01/18] using winsock2 now --- globalfunctions/timevalOperations.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/globalfunctions/timevalOperations.h b/globalfunctions/timevalOperations.h index 46f763b9..db68f330 100644 --- a/globalfunctions/timevalOperations.h +++ b/globalfunctions/timevalOperations.h @@ -4,7 +4,7 @@ #include #ifdef WIN32 -#include +#include #else #include #endif From 59eab8866d8cd8c7f34c710ba6eec8e9845caab2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 17:16:44 +0100 Subject: [PATCH 02/18] using traits again --- serviceinterface/ServiceInterfaceBuffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index e69538d1..c5d5b258 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -30,7 +30,7 @@ protected: //! This is called when buffer becomes full. If //! buffer is not used, then this is called every //! time when characters are put to stream. - int overflow(int c = std::ostream::traits_type::eof()) override; + int overflow(int c = Traits::eof()) override; //! This function is called when stream is flushed, //! for example when std::endl is put to stream. From 71ee86a6884762e40b66fdafb700f9cce65b8cf7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 17:18:31 +0100 Subject: [PATCH 03/18] reverted sif buffer changes --- serviceinterface/ServiceInterfaceBuffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serviceinterface/ServiceInterfaceBuffer.h b/serviceinterface/ServiceInterfaceBuffer.h index e69538d1..c5d5b258 100644 --- a/serviceinterface/ServiceInterfaceBuffer.h +++ b/serviceinterface/ServiceInterfaceBuffer.h @@ -30,7 +30,7 @@ protected: //! This is called when buffer becomes full. If //! buffer is not used, then this is called every //! time when characters are put to stream. - int overflow(int c = std::ostream::traits_type::eof()) override; + int overflow(int c = Traits::eof()) override; //! This function is called when stream is flushed, //! for example when std::endl is put to stream. From 68fe923a014533e1097465e0b279aaaff9e34f86 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 24 Dec 2020 02:03:33 +0100 Subject: [PATCH 04/18] renamed class enums --- datapool/PoolDataSetBase.cpp | 12 ++++++------ datapool/PoolDataSetBase.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index cb2348f7..49495a88 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -11,7 +11,7 @@ PoolDataSetBase::~PoolDataSetBase() {} ReturnValue_t PoolDataSetBase::registerVariable( PoolVariableIF *variable) { - if (state != States::DATA_SET_UNINITIALISED) { + if (state != States::STATE_SET_UNINITIALISED) { sif::error << "DataSet::registerVariable: " "Call made in wrong position." << std::endl; return DataSetIF::DATA_SET_UNINITIALISED; @@ -33,7 +33,7 @@ ReturnValue_t PoolDataSetBase::registerVariable( ReturnValue_t PoolDataSetBase::read(uint32_t lockTimeout) { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - if (state == States::DATA_SET_UNINITIALISED) { + if (state == States::STATE_SET_UNINITIALISED) { lockDataPool(lockTimeout); for (uint16_t count = 0; count < fillCount; count++) { result = readVariable(count); @@ -41,7 +41,7 @@ ReturnValue_t PoolDataSetBase::read(uint32_t lockTimeout) { break; } } - state = States::DATA_SET_WAS_READ; + state = States::STATE_SET_WAS_READ; unlockDataPool(); } else { @@ -80,7 +80,7 @@ ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) { } ReturnValue_t PoolDataSetBase::commit(uint32_t lockTimeout) { - if (state == States::DATA_SET_WAS_READ) { + if (state == States::STATE_SET_WAS_READ) { handleAlreadyReadDatasetCommit(lockTimeout); return HasReturnvaluesIF::RETURN_OK; } @@ -99,7 +99,7 @@ void PoolDataSetBase::handleAlreadyReadDatasetCommit(uint32_t lockTimeout) { registeredVariables[count]->commitWithoutLock(); } } - state = States::DATA_SET_UNINITIALISED; + state = States::STATE_SET_UNINITIALISED; unlockDataPool(); } @@ -121,7 +121,7 @@ ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit(uint32_t lockTimeout) { } } } - state = States::DATA_SET_UNINITIALISED; + state = States::STATE_SET_UNINITIALISED; unlockDataPool(); return result; } diff --git a/datapool/PoolDataSetBase.h b/datapool/PoolDataSetBase.h index a8931d62..66a1924c 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -124,14 +124,14 @@ protected: * States of the seet. */ enum class States { - DATA_SET_UNINITIALISED, //!< DATA_SET_UNINITIALISED - DATA_SET_WAS_READ //!< DATA_SET_WAS_READ + 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::DATA_SET_UNINITIALISED; + States state = States::STATE_SET_UNINITIALISED; /** * @brief This array represents all pool variables registered in this set. From 9eec75df263f3c184a279281a793b520c5daed81 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 25 Dec 2020 01:49:44 +0100 Subject: [PATCH 05/18] linking against thread lib --- osal/host/CMakeLists.txt | 2 +- osal/linux/CMakeLists.txt | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osal/host/CMakeLists.txt b/osal/host/CMakeLists.txt index aa32990b..ee6b4cdb 100644 --- a/osal/host/CMakeLists.txt +++ b/osal/host/CMakeLists.txt @@ -16,6 +16,6 @@ if(UNIX) target_link_libraries(${LIB_FSFW_NAME} PRIVATE rt - pthread + ${CMAKE_THREAD_LIBS_INIT} ) endif() \ No newline at end of file diff --git a/osal/linux/CMakeLists.txt b/osal/linux/CMakeLists.txt index c0096e42..e9a2935b 100644 --- a/osal/linux/CMakeLists.txt +++ b/osal/linux/CMakeLists.txt @@ -18,8 +18,8 @@ target_sources(${LIB_FSFW_NAME} Timer.cpp ) -target_link_libraries(${LIB_FSFW_NAME} - PRIVATE - rt - pthread +target_link_libraries(${LIB_FSFW_NAME} PRIVATE + ${CMAKE_THREAD_LIBS_INIT} + rt ) + From b469340ef45fc63c65069874e413c9aeeab198b3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 18:06:56 +0100 Subject: [PATCH 06/18] bugfix --- datapoollocal/LocalPoolVariable.tpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index 48649ad5..fb129635 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -40,11 +40,11 @@ inline ReturnValue_t LocalPoolVariable::readWithoutLock() { PoolEntry* poolEntry = nullptr; ReturnValue_t result = hkManager->fetchPoolEntry(localPoolId, &poolEntry); - if(result != RETURN_OK and poolEntry != nullptr) { + if(result != RETURN_OK or poolEntry == nullptr) { sif::error << "PoolVector: Read of local pool variable of object " - "0x" << std::hex << std::setw(8) << std::setfill('0') << - hkManager->getOwner() << " and lp ID 0x" << localPoolId << - std::dec << " failed.\n" << std::flush; + << std::hex << std::setw(8) << std::setfill('0') + << hkManager->getOwner() << " and lp ID " << localPoolId + << std::dec << " failed." << std::setfill(' ') << std::endl; return result; } this->value = *(poolEntry->address); @@ -62,7 +62,7 @@ inline ReturnValue_t LocalPoolVariable::commit(dur_millis_t lockTimeout) { template inline ReturnValue_t LocalPoolVariable::commitWithoutLock() { if(readWriteMode == pool_rwm_t::VAR_READ) { - sif::debug << "LocalPoolVar: Invalid read write " + sif::debug << "LocalPoolVariable: Invalid read write " "mode for commit() call." << std::endl; return PoolVariableIF::INVALID_READ_WRITE_MODE; } From e35c2cd604658ac83850aa27891543e461f3a1e9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 23:16:33 +0100 Subject: [PATCH 07/18] hk manager optional now --- datapoollocal/LocalPoolDataSetBase.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 640956db..dc81a612 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -34,13 +34,9 @@ LocalPoolDataSetBase::LocalPoolDataSetBase(sid_t sid, PoolDataSetBase(registeredVariablesArray, maxNumberOfVariables) { HasLocalDataPoolIF* hkOwner = objectManager->get( sid.objectId); - if(hkOwner == nullptr) { - // Configuration error. - sif::error << "LocalPoolDataSetBase::LocalPoolDataSetBase: Owner " - << "invalid!" << std::endl; - return; + if(hkOwner != nullptr) { + hkManager = hkOwner->getHkManagerHandle(); } - hkManager = hkOwner->getHkManagerHandle(); this->sid = sid; mutex = MutexFactory::instance()->createMutex(); @@ -50,8 +46,11 @@ LocalPoolDataSetBase::~LocalPoolDataSetBase() { } ReturnValue_t LocalPoolDataSetBase::lockDataPool(uint32_t timeoutMs) { - MutexIF* mutex = hkManager->getMutexHandle(); - return mutex->lockMutex(MutexIF::TimeoutType::WAITING, timeoutMs); + if(hkManager != nullptr) { + MutexIF* mutex = hkManager->getMutexHandle(); + return mutex->lockMutex(MutexIF::TimeoutType::WAITING, timeoutMs); + } + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t LocalPoolDataSetBase::serializeWithValidityBuffer(uint8_t **buffer, @@ -127,8 +126,11 @@ ReturnValue_t LocalPoolDataSetBase::deSerializeWithValidityBuffer( } ReturnValue_t LocalPoolDataSetBase::unlockDataPool() { - MutexIF* mutex = hkManager->getMutexHandle(); - return mutex->unlockMutex(); + if(hkManager != nullptr) { + MutexIF* mutex = hkManager->getMutexHandle(); + return mutex->unlockMutex(); + } + return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t LocalPoolDataSetBase::serializeLocalPoolIds(uint8_t** buffer, From 76696e34be711277fdbdd07575380f1e2b294a17 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 23:57:23 +0100 Subject: [PATCH 08/18] its possible to protect every read/commit now --- datapool/PoolDataSetBase.cpp | 31 +++++++++++++++++++++++--- datapool/PoolDataSetBase.h | 13 +++++++++++ datapoollocal/LocalPoolDataSetBase.cpp | 10 +++++++-- datapoollocal/LocalPoolDataSetBase.h | 13 ++++++++++- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 49495a88..ad1de8e4 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -9,6 +9,8 @@ PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, PoolDataSetBase::~PoolDataSetBase() {} + + ReturnValue_t PoolDataSetBase::registerVariable( PoolVariableIF *variable) { if (state != States::STATE_SET_UNINITIALISED) { @@ -71,7 +73,13 @@ ReturnValue_t PoolDataSetBase::readVariable(uint16_t count) { registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) { - result = registeredVariables[count]->readWithoutLock(); + if(protectEveryReadCommitCall) { + result = registeredVariables[count]->read(mutexTimeout); + } + else { + result = registeredVariables[count]->readWithoutLock(); + } + if(result != HasReturnvaluesIF::RETURN_OK) { result = INVALID_PARAMETER_DEFINITION; } @@ -96,7 +104,12 @@ void PoolDataSetBase::handleAlreadyReadDatasetCommit(uint32_t lockTimeout) { != PoolVariableIF::VAR_READ && registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) { - registeredVariables[count]->commitWithoutLock(); + if(protectEveryReadCommitCall) { + registeredVariables[count]->commit(mutexTimeout); + } + else { + registeredVariables[count]->commitWithoutLock(); + } } } state = States::STATE_SET_UNINITIALISED; @@ -111,7 +124,13 @@ ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit(uint32_t lockTimeout) { == PoolVariableIF::VAR_WRITE && registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) { - registeredVariables[count]->commitWithoutLock(); + if(protectEveryReadCommitCall) { + result = registeredVariables[count]->commit(mutexTimeout); + } + else { + result = registeredVariables[count]->commitWithoutLock(); + } + } else if (registeredVariables[count]->getDataPoolId() != PoolVariableIF::NO_PARAMETER) { if (result != COMMITING_WITHOUT_READING) { @@ -172,3 +191,9 @@ size_t PoolDataSetBase::getSerializedSize() const { void PoolDataSetBase::setContainer(PoolVariableIF **variablesContainer) { this->registeredVariables = variablesContainer; } + +void PoolDataSetBase::setReadCommitProtectionBehaviour( + bool protectEveryReadCommit, uint32_t mutexTimeout) { + this->protectEveryReadCommitCall = protectEveryReadCommit; + this->mutexTimeout = mutexTimeout; +} diff --git a/datapool/PoolDataSetBase.h b/datapool/PoolDataSetBase.h index 66a1924c..3f52fcd0 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -89,6 +89,7 @@ public: * @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 @@ -114,6 +115,15 @@ public: SerializeIF::Endianness streamEndianness) override; protected: + + /** + * Can be used to individually protect every read and commit call. + * @param protectEveryReadCommit + * @param mutexTimeout + */ + void setReadCommitProtectionBehaviour(bool protectEveryReadCommit, + uint32_t mutexTimeout = 20); + /** * @brief The fill_count attribute ensures that the variables * register in the correct array position and that the maximum @@ -144,6 +154,9 @@ protected: void setContainer(PoolVariableIF** variablesContainer); private: + bool protectEveryReadCommitCall = false; + uint32_t mutexTimeout = 20; + ReturnValue_t readVariable(uint16_t count); void handleAlreadyReadDatasetCommit(uint32_t lockTimeout); ReturnValue_t handleUnreadDatasetCommit(uint32_t lockTimeout); diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index dc81a612..9ecad218 100644 --- a/datapoollocal/LocalPoolDataSetBase.cpp +++ b/datapoollocal/LocalPoolDataSetBase.cpp @@ -8,7 +8,7 @@ LocalPoolDataSetBase::LocalPoolDataSetBase(HasLocalDataPoolIF *hkOwner, uint32_t setId, PoolVariableIF** registeredVariablesArray, - const size_t maxNumberOfVariables, bool noPeriodicHandling): + const size_t maxNumberOfVariables, bool periodicHandling): PoolDataSetBase(registeredVariablesArray, maxNumberOfVariables) { if(hkOwner == nullptr) { // Configuration error. @@ -23,7 +23,7 @@ LocalPoolDataSetBase::LocalPoolDataSetBase(HasLocalDataPoolIF *hkOwner, mutex = MutexFactory::instance()->createMutex(); // Data creators get a periodic helper for periodic HK data generation. - if(not noPeriodicHandling) { + if(periodicHandling) { periodicHelper = new PeriodicHousekeepingHelper(this); } } @@ -278,3 +278,9 @@ void LocalPoolDataSetBase::setValidity(bool valid, bool setEntriesRecursively) { } this->valid = valid; } + +void LocalPoolDataSetBase::setReadCommitProtectionBehaviour( + bool protectEveryReadCommit, uint32_t mutexTimeout) { + PoolDataSetBase::setReadCommitProtectionBehaviour(protectEveryReadCommit, + mutexTimeout); +} diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index aa155bf1..7eabd368 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -54,7 +54,7 @@ public: */ LocalPoolDataSetBase(HasLocalDataPoolIF *hkOwner, uint32_t setId, PoolVariableIF** registeredVariablesArray, - const size_t maxNumberOfVariables, bool noPeriodicHandling = false); + const size_t maxNumberOfVariables, bool periodicHandling = true); /** * @brief Constructor for users of local pool data. @@ -77,6 +77,16 @@ public: */ ~LocalPoolDataSetBase(); + /** + * If the data is pulled from different local data pools, every read and + * commit call should be mutex protected for thread safety. + * This can be specified with the second parameter. + * @param dataCreator + * @param protectEveryReadCommit + */ + void setReadCommitProtectionBehaviour(bool protectEveryReadCommit, + uint32_t mutexTimeout = 20); + void setValidityBufferGeneration(bool withValidityBuffer); sid_t getSid() const; @@ -128,6 +138,7 @@ public: protected: sid_t sid; + uint32_t mutexTimeout = 20; MutexIF* mutex = nullptr; bool diagnostic = false; From 639dbee8a342593f6b765b0efff09f69606d3563 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 27 Dec 2020 00:21:39 +0100 Subject: [PATCH 09/18] form stuff --- datapoollocal/LocalPoolVariable.tpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index fb129635..a685ea8d 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -11,14 +11,14 @@ inline LocalPoolVariable::LocalPoolVariable(HasLocalDataPoolIF* hkOwner, LocalPoolObjectBase(poolId, hkOwner, dataSet, setReadWriteMode) {} template -inline LocalPoolVariable::LocalPoolVariable(object_id_t poolOwner, lp_id_t poolId, - DataSetIF *dataSet, pool_rwm_t setReadWriteMode): +inline LocalPoolVariable::LocalPoolVariable(object_id_t poolOwner, + lp_id_t poolId, DataSetIF *dataSet, pool_rwm_t setReadWriteMode): LocalPoolObjectBase(poolOwner, poolId, dataSet, setReadWriteMode) {} template -inline LocalPoolVariable::LocalPoolVariable(gp_id_t globalPoolId, DataSetIF *dataSet, - pool_rwm_t setReadWriteMode): +inline LocalPoolVariable::LocalPoolVariable(gp_id_t globalPoolId, + DataSetIF *dataSet, pool_rwm_t setReadWriteMode): LocalPoolObjectBase(globalPoolId.objectId, globalPoolId.localPoolId, dataSet, setReadWriteMode){} From 535b51ef7046b9e056371092737c07069dbb2ff1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:15:52 +0100 Subject: [PATCH 10/18] 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; } From f852c774f8e41a6ebf00b5cb6e5e24a815345837 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:52:48 +0100 Subject: [PATCH 11/18] accessor test --- datapool/PoolDataSetIF.h | 6 ++-- datapool/PoolVariableIF.h | 40 ++----------------------- datapool/ReadCommitIF.h | 31 +++++++++++++++++++ datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/PoolReadHelper.h | 41 ++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 42 deletions(-) create mode 100644 datapool/ReadCommitIF.h create mode 100644 datapoollocal/PoolReadHelper.h diff --git a/datapool/PoolDataSetIF.h b/datapool/PoolDataSetIF.h index aa45fa54..99c06cfd 100644 --- a/datapool/PoolDataSetIF.h +++ b/datapool/PoolDataSetIF.h @@ -1,19 +1,17 @@ #ifndef FSFW_DATAPOOL_POOLDATASETIF_H_ #define FSFW_DATAPOOL_POOLDATASETIF_H_ +#include "ReadCommitIF.h" #include "DataSetIF.h" /** * @brief Extendes the DataSetIF by adding abstract functions to lock * and unlock a data pool and read/commit semantics. */ -class PoolDataSetIF: public DataSetIF { +class PoolDataSetIF: public DataSetIF, public ReadCommitIF { public: virtual~ PoolDataSetIF() {}; - virtual ReturnValue_t read(dur_millis_t lockTimeout) = 0; - virtual ReturnValue_t commit(dur_millis_t lockTimeout) = 0; - /** * @brief Most underlying data structures will have a pool like structure * and will require a lock and unlock mechanism to ensure diff --git a/datapool/PoolVariableIF.h b/datapool/PoolVariableIF.h index cd15f744..9740fc12 100644 --- a/datapool/PoolVariableIF.h +++ b/datapool/PoolVariableIF.h @@ -3,6 +3,7 @@ #include "../returnvalues/HasReturnvaluesIF.h" #include "../serialize/SerializeIF.h" +#include "ReadCommitIF.h" /** * @brief This interface is used to control data pool @@ -17,9 +18,9 @@ * @author Bastian Baetz * @ingroup data_pool */ -class PoolVariableIF : public SerializeIF { +class PoolVariableIF : public SerializeIF, + public ReadCommitIF { friend class PoolDataSetBase; - friend class GlobDataSet; friend class LocalPoolDataSetBase; public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::POOL_VARIABLE_IF; @@ -57,41 +58,6 @@ public: */ virtual void setValid(bool validity) = 0; - /** - * @brief The commit call shall write back a newly calculated local - * value to the data pool. - * @details - * It is assumed that these calls are implemented in a thread-safe manner! - */ - virtual ReturnValue_t commit(uint32_t lockTimeout) = 0; - /** - * @brief The read call shall read the value of this parameter from - * the data pool and store the content locally. - * @details - * It is assumbed that these calls are implemented in a thread-safe manner! - */ - virtual ReturnValue_t read(uint32_t lockTimeout) = 0; - -protected: - - /** - * @brief Same as commit with the difference that comitting will be - * performed without a lock - * @return - * This can be used if the lock protection is handled externally - * to avoid the overhead of locking and unlocking consecutively. - * Declared protected to avoid free public usage. - */ - virtual ReturnValue_t readWithoutLock() = 0; - /** - * @brief Same as commit with the difference that comitting will be - * performed without a lock - * @return - * This can be used if the lock protection is handled externally - * to avoid the overhead of locking and unlocking consecutively. - * Declared protected to avoid free public usage. - */ - virtual ReturnValue_t commitWithoutLock() = 0; }; using pool_rwm_t = PoolVariableIF::ReadWriteMode_t; diff --git a/datapool/ReadCommitIF.h b/datapool/ReadCommitIF.h new file mode 100644 index 00000000..0cdce371 --- /dev/null +++ b/datapool/ReadCommitIF.h @@ -0,0 +1,31 @@ +#ifndef FSFW_DATAPOOL_READCOMMITIF_H_ +#define FSFW_DATAPOOL_READCOMMITIF_H_ + +#include + +/** + * @brief Common interface for all software objects which employ read-commit + * semantics. + */ +class ReadCommitIF { +public: + virtual ~ReadCommitIF() {} + virtual ReturnValue_t read(uint32_t mutexTimeout) = 0; + virtual ReturnValue_t commit(uint32_t mutexTimeout) = 0; + +protected: + + //! 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(20); + } + + virtual ReturnValue_t commitWithoutLock() { + return commit(20); + } +}; + + +#endif /* FSFW_DATAPOOL_READCOMMITIF_H_ */ diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index f649a362..5f0d15d3 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -577,7 +577,7 @@ ReturnValue_t LocalDataPoolManager::printPoolEntry( if (poolIter == localPoolMap.end()) { sif::debug << "HousekeepingManager::fechPoolEntry:" << " Pool entry not found." << std::endl; - return POOL_ENTRY_NOT_FOUND; + return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } poolIter->second->print(); return HasReturnvaluesIF::RETURN_OK; diff --git a/datapoollocal/PoolReadHelper.h b/datapoollocal/PoolReadHelper.h new file mode 100644 index 00000000..4fe506ee --- /dev/null +++ b/datapoollocal/PoolReadHelper.h @@ -0,0 +1,41 @@ +#ifndef FSFW_DATAPOOLLOCAL_POOLREADHELPER_H_ +#define FSFW_DATAPOOLLOCAL_POOLREADHELPER_H_ + +#include +#include + +/** + * @brief Helper class to read data sets or pool variables + */ +class PoolReadHelper { +public: + PoolReadHelper(ReadCommitIF* readObject, uint32_t mutexTimeout = 20): + readObject(readObject), mutexTimeout(mutexTimeout) { + if(readObject != nullptr) { + readResult = readObject->read(mutexTimeout); +#if FSFW_PRINT_VERBOSITY_LEVEL == 1 + sif::error << "PoolReadHelper: Read failed!" << std::endl; +#endif + } + } + + ReturnValue_t getReadResult() const { + return readResult; + } + + ~PoolReadHelper() { + if(readObject != nullptr) { + readObject->commit(mutexTimeout); + } + + } + +private: + ReadCommitIF* readObject = nullptr; + ReturnValue_t readResult = HasReturnvaluesIF::RETURN_OK; + uint32_t mutexTimeout = 20; +}; + + + +#endif /* FSFW_DATAPOOLLOCAL_POOLREADHELPER_H_ */ From 76403ce0aba4ae4b68d2011a40310f7573296d62 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:53:59 +0100 Subject: [PATCH 12/18] renaming --- datapoollocal/LocalDataPoolManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index f649a362..5f0d15d3 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -577,7 +577,7 @@ ReturnValue_t LocalDataPoolManager::printPoolEntry( if (poolIter == localPoolMap.end()) { sif::debug << "HousekeepingManager::fechPoolEntry:" << " Pool entry not found." << std::endl; - return POOL_ENTRY_NOT_FOUND; + return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } poolIter->second->print(); return HasReturnvaluesIF::RETURN_OK; From ed473d0a1bbd6bc80c3526eb2a02b8586bbfeec7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 02:04:45 +0100 Subject: [PATCH 13/18] bugfix --- datapoollocal/LocalPoolDataSetBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datapoollocal/LocalPoolDataSetBase.h b/datapoollocal/LocalPoolDataSetBase.h index 7eabd368..d9b6a221 100644 --- a/datapoollocal/LocalPoolDataSetBase.h +++ b/datapoollocal/LocalPoolDataSetBase.h @@ -192,7 +192,7 @@ protected: */ ReturnValue_t unlockDataPool() override; - LocalDataPoolManager* hkManager; + LocalDataPoolManager* hkManager = nullptr; /** * Set n-th bit of a byte, with n being the position from 0 From e24e50b5eea21840a1f74f451baffddc96637ae4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 12:58:05 +0100 Subject: [PATCH 14/18] removed todo --- datapool/PoolDataSetBase.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/datapool/PoolDataSetBase.h b/datapool/PoolDataSetBase.h index 18715b1b..7c1d84a4 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -46,9 +46,6 @@ 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. From 39887db8d2b457aeb5edc0801c136d44e6d8258c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 12:59:25 +0100 Subject: [PATCH 15/18] removed newline --- datapool/PoolDataSetBase.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index 63cbe73e..8f073359 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -10,7 +10,6 @@ PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, PoolDataSetBase::~PoolDataSetBase() {} - ReturnValue_t PoolDataSetBase::registerVariable( PoolVariableIF *variable) { if (state != States::STATE_SET_UNINITIALISED) { From b126149215b8f1c9536f8a9c19ea63705f7ed7c6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 16:49:43 +0100 Subject: [PATCH 16/18] removed abi flags, are part of cross compile file now --- CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 02849ca4..3cc5ffd5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -139,5 +139,4 @@ target_include_directories(${LIB_FSFW_NAME} PRIVATE target_compile_options(${LIB_FSFW_NAME} PRIVATE ${WARNING_FLAGS} ${COMPILER_FLAGS} - ${ABI_FLAGS} ) From d268c6d96cf22cb7317c87b80f648b81c8afb1db Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 16:59:39 +0100 Subject: [PATCH 17/18] moved includes to allow c inclusion --- defaultcfg/fsfwconfig/OBSWConfig.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/defaultcfg/fsfwconfig/OBSWConfig.h b/defaultcfg/fsfwconfig/OBSWConfig.h index 8ad2cb67..6ed8ea2c 100644 --- a/defaultcfg/fsfwconfig/OBSWConfig.h +++ b/defaultcfg/fsfwconfig/OBSWConfig.h @@ -3,11 +3,12 @@ #include "OBSWVersion.h" +#ifdef __cplusplus + #include "objects/systemObjectList.h" #include "events/subsystemIdRanges.h" #include "returnvalues/classIds.h" -#ifdef __cplusplus namespace config { #endif From bc475f0a9a794d75b90317ade116d7e3ada7aa9b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 17:48:26 +0100 Subject: [PATCH 18/18] bugfixes --- osal/host/CMakeLists.txt | 2 ++ osal/linux/CMakeLists.txt | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/osal/host/CMakeLists.txt b/osal/host/CMakeLists.txt index ee6b4cdb..367f721e 100644 --- a/osal/host/CMakeLists.txt +++ b/osal/host/CMakeLists.txt @@ -13,6 +13,8 @@ target_sources(${LIB_FSFW_NAME} ) if(UNIX) + find_package(Threads REQUIRED) + target_link_libraries(${LIB_FSFW_NAME} PRIVATE rt diff --git a/osal/linux/CMakeLists.txt b/osal/linux/CMakeLists.txt index e9a2935b..474e548b 100644 --- a/osal/linux/CMakeLists.txt +++ b/osal/linux/CMakeLists.txt @@ -18,8 +18,14 @@ target_sources(${LIB_FSFW_NAME} Timer.cpp ) +find_package(Threads REQUIRED) + target_link_libraries(${LIB_FSFW_NAME} PRIVATE ${CMAKE_THREAD_LIBS_INIT} rt ) +target_link_libraries(${LIB_FSFW_NAME} INTERFACE + ${CMAKE_THREAD_LIBS_INIT} +) +