From fc4da433d504efec43814e54dd91c62ed91b13c2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 13:49:42 +0100 Subject: [PATCH 01/24] 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 -- 2.34.1 From 59eab8866d8cd8c7f34c710ba6eec8e9845caab2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 17:16:44 +0100 Subject: [PATCH 02/24] 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. -- 2.34.1 From 71ee86a6884762e40b66fdafb700f9cce65b8cf7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 21 Dec 2020 17:18:31 +0100 Subject: [PATCH 03/24] 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. -- 2.34.1 From 68fe923a014533e1097465e0b279aaaff9e34f86 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 24 Dec 2020 02:03:33 +0100 Subject: [PATCH 04/24] 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. -- 2.34.1 From 9eec75df263f3c184a279281a793b520c5daed81 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 25 Dec 2020 01:49:44 +0100 Subject: [PATCH 05/24] 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 ) + -- 2.34.1 From b469340ef45fc63c65069874e413c9aeeab198b3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 18:06:56 +0100 Subject: [PATCH 06/24] 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; } -- 2.34.1 From e35c2cd604658ac83850aa27891543e461f3a1e9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 23:16:33 +0100 Subject: [PATCH 07/24] 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, -- 2.34.1 From 76696e34be711277fdbdd07575380f1e2b294a17 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 26 Dec 2020 23:57:23 +0100 Subject: [PATCH 08/24] 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; -- 2.34.1 From 639dbee8a342593f6b765b0efff09f69606d3563 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 27 Dec 2020 00:21:39 +0100 Subject: [PATCH 09/24] 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){} -- 2.34.1 From 535b51ef7046b9e056371092737c07069dbb2ff1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:15:52 +0100 Subject: [PATCH 10/24] 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; } -- 2.34.1 From f852c774f8e41a6ebf00b5cb6e5e24a815345837 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:52:48 +0100 Subject: [PATCH 11/24] 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_ */ -- 2.34.1 From 76403ce0aba4ae4b68d2011a40310f7573296d62 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 01:53:59 +0100 Subject: [PATCH 12/24] 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; -- 2.34.1 From ed473d0a1bbd6bc80c3526eb2a02b8586bbfeec7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 02:04:45 +0100 Subject: [PATCH 13/24] 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 -- 2.34.1 From e24e50b5eea21840a1f74f451baffddc96637ae4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 12:58:05 +0100 Subject: [PATCH 14/24] 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. -- 2.34.1 From 39887db8d2b457aeb5edc0801c136d44e6d8258c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 12:59:25 +0100 Subject: [PATCH 15/24] 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) { -- 2.34.1 From 37463b660b626133c53376fedfb98646a82b11be Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 27 Dec 2020 14:14:38 +0100 Subject: [PATCH 16/24] restructured test folder and enabled cmake v3 support --- unittest/CMakeLists.txt | 2 ++ unittest/internal/CMakeLists.txt | 7 +++++++ unittest/internal/osal/CMakeLists.txt | 5 +++++ unittest/internal/serialize/CMakeLists.txt | 3 +++ unittest/tests/CMakeLists.txt | 6 ++++++ unittest/tests/action/CMakeLists.txt | 3 +++ unittest/tests/action/TestActionHelper.cpp | 7 +++++-- unittest/tests/action/TestActionHelper.h | 2 +- unittest/tests/container/CMakeLists.txt | 10 ++++++++++ unittest/tests/container/RingBufferTest.cpp | 6 +++--- unittest/tests/container/TestArrayList.cpp | 8 ++++---- unittest/tests/container/TestDynamicFifo.cpp | 11 +++++------ unittest/tests/container/TestFifo.cpp | 11 +++++------ unittest/tests/container/TestFixedArrayList.cpp | 9 ++++----- unittest/tests/container/TestFixedMap.cpp | 8 ++++---- .../tests/container/TestFixedOrderedMultimap.cpp | 8 ++++---- unittest/tests/osal/CMakeLists.txt | 4 ++++ unittest/tests/osal/TestMessageQueue.cpp | 10 ++++++---- unittest/tests/serialize/CMakeLists.txt | 5 +++++ .../tests/serialize/TestSerialBufferAdapter.cpp | 8 +++++--- .../tests/serialize/TestSerialLinkedPacket.cpp | 9 ++++++--- .../tests/serialize/TestSerialLinkedPacket.h | 2 +- unittest/tests/serialize/TestSerialization.cpp | 16 +++++++++------- unittest/tests/storagemanager/CMakeLists.txt | 4 ++++ unittest/tests/storagemanager/TestPool.cpp | 6 ++---- unittest/user/CMakeLists.txt | 1 + unittest/user/testcfg/CMakeLists.txt | 11 +++++++++++ unittest/{ => user}/testcfg/FSFWConfig.h | 0 unittest/{ => user}/testcfg/Makefile-FSFW-Tests | 0 unittest/{ => user}/testcfg/TestsConfig.h | 0 .../testcfg/cdatapool/dataPoolInit.cpp | 0 .../{ => user}/testcfg/cdatapool/dataPoolInit.h | 0 .../testcfg/devices/logicalAddresses.cpp | 0 .../testcfg/devices/logicalAddresses.h | 0 .../testcfg/devices/powerSwitcherList.cpp | 0 .../testcfg/devices/powerSwitcherList.h | 0 .../testcfg/events/subsystemIdRanges.h | 0 .../testcfg/ipc/MissionMessageTypes.cpp | 0 .../{ => user}/testcfg/ipc/MissionMessageTypes.h | 0 .../testcfg/objects/systemObjectList.h | 0 .../pollingsequence/PollingSequenceFactory.cpp | 0 .../pollingsequence/PollingSequenceFactory.h | 0 .../{ => user}/testcfg/returnvalues/classIds.h | 0 unittest/{ => user}/testcfg/testcfg.mk | 0 unittest/{ => user}/testcfg/tmtc/apid.h | 0 unittest/{ => user}/testcfg/tmtc/pusIds.h | 0 .../{ => user}/testtemplate/TestTemplate.cpp | 0 unittest/user/unittest/core/CMakeLists.txt | 7 +++++++ .../unittest}/core/CatchDefinitions.cpp | 0 .../{ => user/unittest}/core/CatchDefinitions.h | 0 .../{ => user/unittest}/core/CatchFactory.cpp | 0 unittest/{ => user/unittest}/core/CatchFactory.h | 0 .../{ => user/unittest}/core/CatchRunner.cpp | 0 unittest/{ => user/unittest}/core/CatchSetup.cpp | 0 unittest/{ => user/unittest}/core/core.mk | 0 unittest/{ => user/unittest}/core/printChar.cpp | 0 unittest/{ => user/unittest}/core/printChar.h | 0 unittest/{ => user}/unlockRealtime.sh | 0 58 files changed, 132 insertions(+), 57 deletions(-) create mode 100644 unittest/CMakeLists.txt create mode 100644 unittest/internal/CMakeLists.txt create mode 100644 unittest/internal/osal/CMakeLists.txt create mode 100644 unittest/internal/serialize/CMakeLists.txt create mode 100644 unittest/tests/CMakeLists.txt create mode 100644 unittest/tests/action/CMakeLists.txt create mode 100644 unittest/tests/container/CMakeLists.txt create mode 100644 unittest/tests/osal/CMakeLists.txt create mode 100644 unittest/tests/serialize/CMakeLists.txt create mode 100644 unittest/tests/storagemanager/CMakeLists.txt create mode 100644 unittest/user/CMakeLists.txt create mode 100644 unittest/user/testcfg/CMakeLists.txt rename unittest/{ => user}/testcfg/FSFWConfig.h (100%) rename unittest/{ => user}/testcfg/Makefile-FSFW-Tests (100%) rename unittest/{ => user}/testcfg/TestsConfig.h (100%) rename unittest/{ => user}/testcfg/cdatapool/dataPoolInit.cpp (100%) rename unittest/{ => user}/testcfg/cdatapool/dataPoolInit.h (100%) rename unittest/{ => user}/testcfg/devices/logicalAddresses.cpp (100%) rename unittest/{ => user}/testcfg/devices/logicalAddresses.h (100%) rename unittest/{ => user}/testcfg/devices/powerSwitcherList.cpp (100%) rename unittest/{ => user}/testcfg/devices/powerSwitcherList.h (100%) rename unittest/{ => user}/testcfg/events/subsystemIdRanges.h (100%) rename unittest/{ => user}/testcfg/ipc/MissionMessageTypes.cpp (100%) rename unittest/{ => user}/testcfg/ipc/MissionMessageTypes.h (100%) rename unittest/{ => user}/testcfg/objects/systemObjectList.h (100%) rename unittest/{ => user}/testcfg/pollingsequence/PollingSequenceFactory.cpp (100%) rename unittest/{ => user}/testcfg/pollingsequence/PollingSequenceFactory.h (100%) rename unittest/{ => user}/testcfg/returnvalues/classIds.h (100%) rename unittest/{ => user}/testcfg/testcfg.mk (100%) rename unittest/{ => user}/testcfg/tmtc/apid.h (100%) rename unittest/{ => user}/testcfg/tmtc/pusIds.h (100%) rename unittest/{ => user}/testtemplate/TestTemplate.cpp (100%) create mode 100644 unittest/user/unittest/core/CMakeLists.txt rename unittest/{ => user/unittest}/core/CatchDefinitions.cpp (100%) rename unittest/{ => user/unittest}/core/CatchDefinitions.h (100%) rename unittest/{ => user/unittest}/core/CatchFactory.cpp (100%) rename unittest/{ => user/unittest}/core/CatchFactory.h (100%) rename unittest/{ => user/unittest}/core/CatchRunner.cpp (100%) rename unittest/{ => user/unittest}/core/CatchSetup.cpp (100%) rename unittest/{ => user/unittest}/core/core.mk (100%) rename unittest/{ => user/unittest}/core/printChar.cpp (100%) rename unittest/{ => user/unittest}/core/printChar.h (100%) rename unittest/{ => user}/unlockRealtime.sh (100%) diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt new file mode 100644 index 00000000..119ef243 --- /dev/null +++ b/unittest/CMakeLists.txt @@ -0,0 +1,2 @@ +add_subdirectory(internal) +add_subdirectory(tests) \ No newline at end of file diff --git a/unittest/internal/CMakeLists.txt b/unittest/internal/CMakeLists.txt new file mode 100644 index 00000000..9c24317f --- /dev/null +++ b/unittest/internal/CMakeLists.txt @@ -0,0 +1,7 @@ +target_sources(${TARGET_NAME} PRIVATE + InternalUnitTester.cpp + UnittDefinitions.cpp +) + +add_subdirectory(osal) +add_subdirectory(serialize) \ No newline at end of file diff --git a/unittest/internal/osal/CMakeLists.txt b/unittest/internal/osal/CMakeLists.txt new file mode 100644 index 00000000..c6f1eb95 --- /dev/null +++ b/unittest/internal/osal/CMakeLists.txt @@ -0,0 +1,5 @@ +target_sources(${TARGET_NAME} PRIVATE + IntTestMq.cpp + IntTestMutex.cpp + IntTestSemaphore.cpp +) diff --git a/unittest/internal/serialize/CMakeLists.txt b/unittest/internal/serialize/CMakeLists.txt new file mode 100644 index 00000000..e8dc5717 --- /dev/null +++ b/unittest/internal/serialize/CMakeLists.txt @@ -0,0 +1,3 @@ +target_sources(${TARGET_NAME} PRIVATE + IntTestSerialization.cpp +) diff --git a/unittest/tests/CMakeLists.txt b/unittest/tests/CMakeLists.txt new file mode 100644 index 00000000..97074417 --- /dev/null +++ b/unittest/tests/CMakeLists.txt @@ -0,0 +1,6 @@ +add_subdirectory(action) +add_subdirectory(container) +add_subdirectory(osal) +add_subdirectory(serialize) +add_subdirectory(storagemanager) + diff --git a/unittest/tests/action/CMakeLists.txt b/unittest/tests/action/CMakeLists.txt new file mode 100644 index 00000000..0339000f --- /dev/null +++ b/unittest/tests/action/CMakeLists.txt @@ -0,0 +1,3 @@ +target_sources(${TARGET_NAME} PRIVATE + TestActionHelper.cpp +) diff --git a/unittest/tests/action/TestActionHelper.cpp b/unittest/tests/action/TestActionHelper.cpp index 944ad705..3109eba1 100644 --- a/unittest/tests/action/TestActionHelper.cpp +++ b/unittest/tests/action/TestActionHelper.cpp @@ -1,8 +1,11 @@ #include "TestActionHelper.h" + +#include + #include #include -#include -#include "../../core/CatchDefinitions.h" + +#include TEST_CASE( "Action Helper" , "[ActionHelper]") { diff --git a/unittest/tests/action/TestActionHelper.h b/unittest/tests/action/TestActionHelper.h index 4ba417eb..610fb09e 100644 --- a/unittest/tests/action/TestActionHelper.h +++ b/unittest/tests/action/TestActionHelper.h @@ -3,7 +3,7 @@ #include #include -#include +#include #include diff --git a/unittest/tests/container/CMakeLists.txt b/unittest/tests/container/CMakeLists.txt new file mode 100644 index 00000000..966c5834 --- /dev/null +++ b/unittest/tests/container/CMakeLists.txt @@ -0,0 +1,10 @@ +target_sources(${TARGET_NAME} PRIVATE + RingBufferTest.cpp + TestArrayList.cpp + TestDynamicFifo.cpp + TestFifo.cpp + TestFixedArrayList.cpp + TestFixedMap.cpp + TestFixedOrderedMultimap.cpp + TestPlacementFactory.cpp +) diff --git a/unittest/tests/container/RingBufferTest.cpp b/unittest/tests/container/RingBufferTest.cpp index 9c1c8a23..32a2502d 100644 --- a/unittest/tests/container/RingBufferTest.cpp +++ b/unittest/tests/container/RingBufferTest.cpp @@ -1,7 +1,7 @@ -#include "../../core/CatchDefinitions.h" -#include "../../container/SimpleRingBuffer.h" +#include +#include -#include +#include #include TEST_CASE("Ring Buffer Test" , "[RingBufferTest]") { diff --git a/unittest/tests/container/TestArrayList.cpp b/unittest/tests/container/TestArrayList.cpp index 2f884276..1fd330b6 100644 --- a/unittest/tests/container/TestArrayList.cpp +++ b/unittest/tests/container/TestArrayList.cpp @@ -1,8 +1,8 @@ -#include "../../container/ArrayList.h" -#include "../../returnvalues/HasReturnvaluesIF.h" +#include +#include -#include -#include "../../core/CatchDefinitions.h" +#include +#include /** * @brief Array List test diff --git a/unittest/tests/container/TestDynamicFifo.cpp b/unittest/tests/container/TestDynamicFifo.cpp index bb19131e..759fcc66 100644 --- a/unittest/tests/container/TestDynamicFifo.cpp +++ b/unittest/tests/container/TestDynamicFifo.cpp @@ -1,10 +1,9 @@ +#include "../../../container/DynamicFIFO.h" +#include "../../../container/FIFO.h" +#include "../../../returnvalues/HasReturnvaluesIF.h" -#include "../../container/DynamicFIFO.h" -#include "../../container/FIFO.h" -#include "../../returnvalues/HasReturnvaluesIF.h" - -#include -#include +#include +#include TEST_CASE( "Dynamic Fifo Tests", "[TestDynamicFifo]") { INFO("Dynamic Fifo Tests"); diff --git a/unittest/tests/container/TestFifo.cpp b/unittest/tests/container/TestFifo.cpp index bd727e00..f7bfb1de 100644 --- a/unittest/tests/container/TestFifo.cpp +++ b/unittest/tests/container/TestFifo.cpp @@ -1,10 +1,9 @@ +#include "../../../container/DynamicFIFO.h" +#include "../../../container/FIFO.h" +#include "../../../returnvalues/HasReturnvaluesIF.h" -#include "../../container/DynamicFIFO.h" -#include "../../container/FIFO.h" -#include "../../returnvalues/HasReturnvaluesIF.h" - -#include -#include "../../core/CatchDefinitions.h" +#include +#include TEST_CASE( "Static Fifo Tests", "[TestFifo]") { INFO("Fifo Tests"); diff --git a/unittest/tests/container/TestFixedArrayList.cpp b/unittest/tests/container/TestFixedArrayList.cpp index 5a1bd280..b2baa3e7 100644 --- a/unittest/tests/container/TestFixedArrayList.cpp +++ b/unittest/tests/container/TestFixedArrayList.cpp @@ -1,9 +1,8 @@ -#include "../../core/CatchDefinitions.h" +#include "../../../container/FixedArrayList.h" +#include "../../../returnvalues/HasReturnvaluesIF.h" -#include "../../container/FixedArrayList.h" -#include "../../returnvalues/HasReturnvaluesIF.h" - -#include +#include +#include TEST_CASE( "FixedArrayList Tests", "[TestFixedArrayList]") { diff --git a/unittest/tests/container/TestFixedMap.cpp b/unittest/tests/container/TestFixedMap.cpp index da0c84e3..f93065b6 100644 --- a/unittest/tests/container/TestFixedMap.cpp +++ b/unittest/tests/container/TestFixedMap.cpp @@ -1,8 +1,8 @@ -#include "../../container/FixedMap.h" -#include "../../returnvalues/HasReturnvaluesIF.h" +#include "../../../container/FixedMap.h" +#include "../../../returnvalues/HasReturnvaluesIF.h" -#include -#include "../../core/CatchDefinitions.h" +#include +#include template class FixedMap; diff --git a/unittest/tests/container/TestFixedOrderedMultimap.cpp b/unittest/tests/container/TestFixedOrderedMultimap.cpp index e625b559..b337a9c8 100644 --- a/unittest/tests/container/TestFixedOrderedMultimap.cpp +++ b/unittest/tests/container/TestFixedOrderedMultimap.cpp @@ -1,8 +1,8 @@ -#include "../../container/FixedOrderedMultimap.h" -#include "../../returnvalues/HasReturnvaluesIF.h" +#include "../../../container/FixedOrderedMultimap.h" +#include "../../../returnvalues/HasReturnvaluesIF.h" -#include -#include "../../core/CatchDefinitions.h" +#include +#include TEST_CASE( "FixedOrderedMultimap Tests", "[TestFixedOrderedMultimap]") { INFO("FixedOrderedMultimap Tests"); diff --git a/unittest/tests/osal/CMakeLists.txt b/unittest/tests/osal/CMakeLists.txt new file mode 100644 index 00000000..5ca5e400 --- /dev/null +++ b/unittest/tests/osal/CMakeLists.txt @@ -0,0 +1,4 @@ +target_sources(${TARGET_NAME} PRIVATE + TestMessageQueue.cpp + TestSemaphore.cpp +) diff --git a/unittest/tests/osal/TestMessageQueue.cpp b/unittest/tests/osal/TestMessageQueue.cpp index 441d32e7..5360974f 100644 --- a/unittest/tests/osal/TestMessageQueue.cpp +++ b/unittest/tests/osal/TestMessageQueue.cpp @@ -1,8 +1,10 @@ -#include "../../ipc/MessageQueueIF.h" -#include "../../ipc/QueueFactory.h" -#include +#include "../../../ipc/MessageQueueIF.h" +#include "../../../ipc/QueueFactory.h" + +#include +#include + #include -#include "../../core/CatchDefinitions.h" TEST_CASE("MessageQueue Basic Test","[TestMq]") { MessageQueueIF* testSenderMq = diff --git a/unittest/tests/serialize/CMakeLists.txt b/unittest/tests/serialize/CMakeLists.txt new file mode 100644 index 00000000..5a9d9a0f --- /dev/null +++ b/unittest/tests/serialize/CMakeLists.txt @@ -0,0 +1,5 @@ +target_sources(${TARGET_NAME} PRIVATE + TestSerialBufferAdapter.cpp + TestSerialization.cpp + TestSerialLinkedPacket.cpp +) diff --git a/unittest/tests/serialize/TestSerialBufferAdapter.cpp b/unittest/tests/serialize/TestSerialBufferAdapter.cpp index 07cd3f9c..778f857e 100644 --- a/unittest/tests/serialize/TestSerialBufferAdapter.cpp +++ b/unittest/tests/serialize/TestSerialBufferAdapter.cpp @@ -1,7 +1,9 @@ -#include "../../serialize/SerialBufferAdapter.h" +#include "../../../serialize/SerialBufferAdapter.h" -#include -#include "../../core/CatchDefinitions.h" +#include +#include + +#include static bool test_value_bool = true; diff --git a/unittest/tests/serialize/TestSerialLinkedPacket.cpp b/unittest/tests/serialize/TestSerialLinkedPacket.cpp index fbe48894..83bdf857 100644 --- a/unittest/tests/serialize/TestSerialLinkedPacket.cpp +++ b/unittest/tests/serialize/TestSerialLinkedPacket.cpp @@ -1,9 +1,12 @@ #include "TestSerialLinkedPacket.h" -#include "../../core/CatchDefinitions.h" +#include -#include "../../globalfunctions/arrayprinter.h" +#include "../../../globalfunctions/arrayprinter.h" -#include +#include +#include + +#include TEST_CASE("Serial Linked Packet" , "[SerLinkPacket]") { diff --git a/unittest/tests/serialize/TestSerialLinkedPacket.h b/unittest/tests/serialize/TestSerialLinkedPacket.h index 6c720577..71f258f9 100644 --- a/unittest/tests/serialize/TestSerialLinkedPacket.h +++ b/unittest/tests/serialize/TestSerialLinkedPacket.h @@ -37,7 +37,7 @@ public: return buffer.entry.getConstBuffer(); } - const size_t getBufferLength() { + size_t getBufferLength() { return buffer.getSerializedSize(); } diff --git a/unittest/tests/serialize/TestSerialization.cpp b/unittest/tests/serialize/TestSerialization.cpp index 6e31170a..2877d09a 100644 --- a/unittest/tests/serialize/TestSerialization.cpp +++ b/unittest/tests/serialize/TestSerialization.cpp @@ -1,8 +1,10 @@ -#include "../../serialize/SerializeAdapter.h" +#include "../../../serialize/SerializeAdapter.h" + +#include +#include +#include -#include "catch.hpp" #include -#include "../../core/CatchDefinitions.h" static bool test_value_bool = true; static uint8_t tv_uint8 {5}; @@ -119,10 +121,10 @@ TEST_CASE("Auto Serialize Adapter testing", "[single-file]") { REQUIRE(tv_int16 == -829); REQUIRE(tv_int32 == -2312); - REQUIRE(tv_float == Approx(8.214921)); - REQUIRE(tv_double == Approx(9.2132142141e8)); - REQUIRE(tv_sfloat == Approx(-922.2321321)); - REQUIRE(tv_sdouble == Approx(-2.2421e19)); + REQUIRE(tv_float == Catch::Approx(8.214921)); + REQUIRE(tv_double == Catch::Approx(9.2132142141e8)); + REQUIRE(tv_sfloat == Catch::Approx(-922.2321321)); + REQUIRE(tv_sdouble == Catch::Approx(-2.2421e19)); } } diff --git a/unittest/tests/storagemanager/CMakeLists.txt b/unittest/tests/storagemanager/CMakeLists.txt new file mode 100644 index 00000000..ed7be7d5 --- /dev/null +++ b/unittest/tests/storagemanager/CMakeLists.txt @@ -0,0 +1,4 @@ +target_sources(${TARGET_NAME} PRIVATE + TestNewAccessor.cpp + TestPool.cpp +) diff --git a/unittest/tests/storagemanager/TestPool.cpp b/unittest/tests/storagemanager/TestPool.cpp index daeb6bb5..dcfb6c03 100644 --- a/unittest/tests/storagemanager/TestPool.cpp +++ b/unittest/tests/storagemanager/TestPool.cpp @@ -1,10 +1,8 @@ -#include "CatchDefinitions.h" - #include #include -#include -#include +#include +#include #include diff --git a/unittest/user/CMakeLists.txt b/unittest/user/CMakeLists.txt new file mode 100644 index 00000000..ad6d4787 --- /dev/null +++ b/unittest/user/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(core) diff --git a/unittest/user/testcfg/CMakeLists.txt b/unittest/user/testcfg/CMakeLists.txt new file mode 100644 index 00000000..dbf0256f --- /dev/null +++ b/unittest/user/testcfg/CMakeLists.txt @@ -0,0 +1,11 @@ +target_sources(${TARGET_NAME} + PRIVATE + ipc/MissionMessageTypes.cpp + pollingsequence/PollingSequenceFactory.cpp +) + +# Add include paths for the executable +target_include_directories(${TARGET_NAME} + PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR} +) diff --git a/unittest/testcfg/FSFWConfig.h b/unittest/user/testcfg/FSFWConfig.h similarity index 100% rename from unittest/testcfg/FSFWConfig.h rename to unittest/user/testcfg/FSFWConfig.h diff --git a/unittest/testcfg/Makefile-FSFW-Tests b/unittest/user/testcfg/Makefile-FSFW-Tests similarity index 100% rename from unittest/testcfg/Makefile-FSFW-Tests rename to unittest/user/testcfg/Makefile-FSFW-Tests diff --git a/unittest/testcfg/TestsConfig.h b/unittest/user/testcfg/TestsConfig.h similarity index 100% rename from unittest/testcfg/TestsConfig.h rename to unittest/user/testcfg/TestsConfig.h diff --git a/unittest/testcfg/cdatapool/dataPoolInit.cpp b/unittest/user/testcfg/cdatapool/dataPoolInit.cpp similarity index 100% rename from unittest/testcfg/cdatapool/dataPoolInit.cpp rename to unittest/user/testcfg/cdatapool/dataPoolInit.cpp diff --git a/unittest/testcfg/cdatapool/dataPoolInit.h b/unittest/user/testcfg/cdatapool/dataPoolInit.h similarity index 100% rename from unittest/testcfg/cdatapool/dataPoolInit.h rename to unittest/user/testcfg/cdatapool/dataPoolInit.h diff --git a/unittest/testcfg/devices/logicalAddresses.cpp b/unittest/user/testcfg/devices/logicalAddresses.cpp similarity index 100% rename from unittest/testcfg/devices/logicalAddresses.cpp rename to unittest/user/testcfg/devices/logicalAddresses.cpp diff --git a/unittest/testcfg/devices/logicalAddresses.h b/unittest/user/testcfg/devices/logicalAddresses.h similarity index 100% rename from unittest/testcfg/devices/logicalAddresses.h rename to unittest/user/testcfg/devices/logicalAddresses.h diff --git a/unittest/testcfg/devices/powerSwitcherList.cpp b/unittest/user/testcfg/devices/powerSwitcherList.cpp similarity index 100% rename from unittest/testcfg/devices/powerSwitcherList.cpp rename to unittest/user/testcfg/devices/powerSwitcherList.cpp diff --git a/unittest/testcfg/devices/powerSwitcherList.h b/unittest/user/testcfg/devices/powerSwitcherList.h similarity index 100% rename from unittest/testcfg/devices/powerSwitcherList.h rename to unittest/user/testcfg/devices/powerSwitcherList.h diff --git a/unittest/testcfg/events/subsystemIdRanges.h b/unittest/user/testcfg/events/subsystemIdRanges.h similarity index 100% rename from unittest/testcfg/events/subsystemIdRanges.h rename to unittest/user/testcfg/events/subsystemIdRanges.h diff --git a/unittest/testcfg/ipc/MissionMessageTypes.cpp b/unittest/user/testcfg/ipc/MissionMessageTypes.cpp similarity index 100% rename from unittest/testcfg/ipc/MissionMessageTypes.cpp rename to unittest/user/testcfg/ipc/MissionMessageTypes.cpp diff --git a/unittest/testcfg/ipc/MissionMessageTypes.h b/unittest/user/testcfg/ipc/MissionMessageTypes.h similarity index 100% rename from unittest/testcfg/ipc/MissionMessageTypes.h rename to unittest/user/testcfg/ipc/MissionMessageTypes.h diff --git a/unittest/testcfg/objects/systemObjectList.h b/unittest/user/testcfg/objects/systemObjectList.h similarity index 100% rename from unittest/testcfg/objects/systemObjectList.h rename to unittest/user/testcfg/objects/systemObjectList.h diff --git a/unittest/testcfg/pollingsequence/PollingSequenceFactory.cpp b/unittest/user/testcfg/pollingsequence/PollingSequenceFactory.cpp similarity index 100% rename from unittest/testcfg/pollingsequence/PollingSequenceFactory.cpp rename to unittest/user/testcfg/pollingsequence/PollingSequenceFactory.cpp diff --git a/unittest/testcfg/pollingsequence/PollingSequenceFactory.h b/unittest/user/testcfg/pollingsequence/PollingSequenceFactory.h similarity index 100% rename from unittest/testcfg/pollingsequence/PollingSequenceFactory.h rename to unittest/user/testcfg/pollingsequence/PollingSequenceFactory.h diff --git a/unittest/testcfg/returnvalues/classIds.h b/unittest/user/testcfg/returnvalues/classIds.h similarity index 100% rename from unittest/testcfg/returnvalues/classIds.h rename to unittest/user/testcfg/returnvalues/classIds.h diff --git a/unittest/testcfg/testcfg.mk b/unittest/user/testcfg/testcfg.mk similarity index 100% rename from unittest/testcfg/testcfg.mk rename to unittest/user/testcfg/testcfg.mk diff --git a/unittest/testcfg/tmtc/apid.h b/unittest/user/testcfg/tmtc/apid.h similarity index 100% rename from unittest/testcfg/tmtc/apid.h rename to unittest/user/testcfg/tmtc/apid.h diff --git a/unittest/testcfg/tmtc/pusIds.h b/unittest/user/testcfg/tmtc/pusIds.h similarity index 100% rename from unittest/testcfg/tmtc/pusIds.h rename to unittest/user/testcfg/tmtc/pusIds.h diff --git a/unittest/testtemplate/TestTemplate.cpp b/unittest/user/testtemplate/TestTemplate.cpp similarity index 100% rename from unittest/testtemplate/TestTemplate.cpp rename to unittest/user/testtemplate/TestTemplate.cpp diff --git a/unittest/user/unittest/core/CMakeLists.txt b/unittest/user/unittest/core/CMakeLists.txt new file mode 100644 index 00000000..d78f45f1 --- /dev/null +++ b/unittest/user/unittest/core/CMakeLists.txt @@ -0,0 +1,7 @@ +target_sources(${TARGET_NAME} PRIVATE + CatchDefinitions.cpp + CatchFactory.cpp + CatchRunner.cpp + CatchSetup.cpp + printChar.cpp +) diff --git a/unittest/core/CatchDefinitions.cpp b/unittest/user/unittest/core/CatchDefinitions.cpp similarity index 100% rename from unittest/core/CatchDefinitions.cpp rename to unittest/user/unittest/core/CatchDefinitions.cpp diff --git a/unittest/core/CatchDefinitions.h b/unittest/user/unittest/core/CatchDefinitions.h similarity index 100% rename from unittest/core/CatchDefinitions.h rename to unittest/user/unittest/core/CatchDefinitions.h diff --git a/unittest/core/CatchFactory.cpp b/unittest/user/unittest/core/CatchFactory.cpp similarity index 100% rename from unittest/core/CatchFactory.cpp rename to unittest/user/unittest/core/CatchFactory.cpp diff --git a/unittest/core/CatchFactory.h b/unittest/user/unittest/core/CatchFactory.h similarity index 100% rename from unittest/core/CatchFactory.h rename to unittest/user/unittest/core/CatchFactory.h diff --git a/unittest/core/CatchRunner.cpp b/unittest/user/unittest/core/CatchRunner.cpp similarity index 100% rename from unittest/core/CatchRunner.cpp rename to unittest/user/unittest/core/CatchRunner.cpp diff --git a/unittest/core/CatchSetup.cpp b/unittest/user/unittest/core/CatchSetup.cpp similarity index 100% rename from unittest/core/CatchSetup.cpp rename to unittest/user/unittest/core/CatchSetup.cpp diff --git a/unittest/core/core.mk b/unittest/user/unittest/core/core.mk similarity index 100% rename from unittest/core/core.mk rename to unittest/user/unittest/core/core.mk diff --git a/unittest/core/printChar.cpp b/unittest/user/unittest/core/printChar.cpp similarity index 100% rename from unittest/core/printChar.cpp rename to unittest/user/unittest/core/printChar.cpp diff --git a/unittest/core/printChar.h b/unittest/user/unittest/core/printChar.h similarity index 100% rename from unittest/core/printChar.h rename to unittest/user/unittest/core/printChar.h diff --git a/unittest/unlockRealtime.sh b/unittest/user/unlockRealtime.sh similarity index 100% rename from unittest/unlockRealtime.sh rename to unittest/user/unlockRealtime.sh -- 2.34.1 From d8d2f207e172ec8fd4f20104112f38fe176231b9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 27 Dec 2020 14:20:26 +0100 Subject: [PATCH 17/24] cleaned up includes --- unittest/tests/container/TestDynamicFifo.cpp | 6 +++--- unittest/tests/container/TestFifo.cpp | 6 +++--- unittest/tests/container/TestFixedMap.cpp | 4 ++-- unittest/tests/osal/TestMessageQueue.cpp | 4 ++-- unittest/tests/serialize/TestSerialBufferAdapter.cpp | 2 +- unittest/tests/serialize/TestSerialLinkedPacket.cpp | 2 +- unittest/tests/serialize/TestSerialization.cpp | 2 +- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/unittest/tests/container/TestDynamicFifo.cpp b/unittest/tests/container/TestDynamicFifo.cpp index 759fcc66..2b572d52 100644 --- a/unittest/tests/container/TestDynamicFifo.cpp +++ b/unittest/tests/container/TestDynamicFifo.cpp @@ -1,6 +1,6 @@ -#include "../../../container/DynamicFIFO.h" -#include "../../../container/FIFO.h" -#include "../../../returnvalues/HasReturnvaluesIF.h" +#include +#include +#include #include #include diff --git a/unittest/tests/container/TestFifo.cpp b/unittest/tests/container/TestFifo.cpp index f7bfb1de..fbcd40cc 100644 --- a/unittest/tests/container/TestFifo.cpp +++ b/unittest/tests/container/TestFifo.cpp @@ -1,6 +1,6 @@ -#include "../../../container/DynamicFIFO.h" -#include "../../../container/FIFO.h" -#include "../../../returnvalues/HasReturnvaluesIF.h" +#include +#include +#include #include #include diff --git a/unittest/tests/container/TestFixedMap.cpp b/unittest/tests/container/TestFixedMap.cpp index f93065b6..297171ca 100644 --- a/unittest/tests/container/TestFixedMap.cpp +++ b/unittest/tests/container/TestFixedMap.cpp @@ -1,5 +1,5 @@ -#include "../../../container/FixedMap.h" -#include "../../../returnvalues/HasReturnvaluesIF.h" +#include +#include #include #include diff --git a/unittest/tests/osal/TestMessageQueue.cpp b/unittest/tests/osal/TestMessageQueue.cpp index 5360974f..e33b7240 100644 --- a/unittest/tests/osal/TestMessageQueue.cpp +++ b/unittest/tests/osal/TestMessageQueue.cpp @@ -1,5 +1,5 @@ -#include "../../../ipc/MessageQueueIF.h" -#include "../../../ipc/QueueFactory.h" +#include +#include #include #include diff --git a/unittest/tests/serialize/TestSerialBufferAdapter.cpp b/unittest/tests/serialize/TestSerialBufferAdapter.cpp index 778f857e..1938746d 100644 --- a/unittest/tests/serialize/TestSerialBufferAdapter.cpp +++ b/unittest/tests/serialize/TestSerialBufferAdapter.cpp @@ -1,4 +1,4 @@ -#include "../../../serialize/SerialBufferAdapter.h" +#include #include #include diff --git a/unittest/tests/serialize/TestSerialLinkedPacket.cpp b/unittest/tests/serialize/TestSerialLinkedPacket.cpp index 83bdf857..b90ae9f8 100644 --- a/unittest/tests/serialize/TestSerialLinkedPacket.cpp +++ b/unittest/tests/serialize/TestSerialLinkedPacket.cpp @@ -1,7 +1,7 @@ #include "TestSerialLinkedPacket.h" #include -#include "../../../globalfunctions/arrayprinter.h" +#include #include #include diff --git a/unittest/tests/serialize/TestSerialization.cpp b/unittest/tests/serialize/TestSerialization.cpp index 2877d09a..3de581ec 100644 --- a/unittest/tests/serialize/TestSerialization.cpp +++ b/unittest/tests/serialize/TestSerialization.cpp @@ -1,4 +1,4 @@ -#include "../../../serialize/SerializeAdapter.h" +#include #include #include -- 2.34.1 From f60f02c5b84033a63b94b80925ed2f8e1a64953d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sun, 27 Dec 2020 14:53:37 +0100 Subject: [PATCH 18/24] updated readme --- unittest/README.md | 62 +++---------- unittest/user/CMakeLists.txt | 166 ++++++++++++++++++++++++++++++++++- 2 files changed, 176 insertions(+), 52 deletions(-) diff --git a/unittest/README.md b/unittest/README.md index 8a787c07..d6a4bb85 100644 --- a/unittest/README.md +++ b/unittest/README.md @@ -1,59 +1,19 @@ ## FSFW Testing -This repository contains testing and unit testing components. -[Catch2](https://github.com/catchorg/Catch2) has been used as a framework, -and these unit tests can only be run on a linux host machine. -The makefile with default settings creates the unit test binary which can be -run in the terminal or in eclipse. +This folder contains testing and unit testing components. ### Instructions -To run the fsfw unittests in the project, perform following steps: +The easiest way to run the unittest contained in this folder is to follow +the steps in the [test repository](https://egit.irs.uni-stuttgart.de/fsfw/fsfw_tests). +This is recommended even if the goal is to set up a custom test repository to have +a starting point. -1. Copy the testcfg folder the project root (folder containing the FSFW). -2. There is a makefile inside the testcfg folder which can be used to have - a starting point to compile the unit tests. Copy that Makefile to the project - root -3. Create a folder named catch2 (can have other name which requires Makefile - adaption) and copy the Catch2 header files there (NOTE: CMake support - not enabled yet!) +To set up a custom test repository or project, following steps can be performed: + +1. Copy the user folder content into the project root. +2. Clone [Catch2](https://github.com/catchorg/Catch2) in the project root. +3. Use the `CMakeLists.txt` as a starting point to add tests and build the test + executable. -### Eclipse CDT settings - -The default eclipse terminal has issues displaying the colors used -when running the unit test binary by catch2. To fix this issue, -install the ANSI Escape In Console package from the eclipse marketplace. - -### GCOV integration - -GCOV has been integrated as a code coverage tool. -It can be enabled by adding `GCOV=1` to the build process as an additional argument. -Coverage data will be provided in form of .gcno and .gcda files. -These can be displayed in eclipse by looking -for a .gcno or .gcda file in the \_obj folder, double-clicking it -and picking the right source-binary. This will generate -information about which lines of a file have run, provided it is open in -eclipse. - -### LCOV integration - -The files generated by GCOV can also be processed by the tool LCOV. -On ubuntu, the tool can be installed with the following command: - -```sh -sudo apt-get install lcov -```` - -After that, the tool can be run by building the unit tests with `GCOV=1`, -running them at least one time and then executing the `lcov.sh` script. - -### Adding unit tests - -The catch unit tests are located in unittest/testfw. To add new unit tests, -add them to the UnitTestCatch.cpp file or add a new source file which -includes catch.hpp. - -For writing basics tests, the [assertion documentation](https://github.com/catchorg/Catch2/blob/master/docs/assertions.md#top) -or the existing examples are a good guideliens. -For more advanced tests, refer to the [catch2 documentation](https://github.com/catchorg/Catch2/blob/master/docs/Readme.md#top). diff --git a/unittest/user/CMakeLists.txt b/unittest/user/CMakeLists.txt index ad6d4787..6ec314c3 100644 --- a/unittest/user/CMakeLists.txt +++ b/unittest/user/CMakeLists.txt @@ -1 +1,165 @@ -add_subdirectory(core) +################################################################################ +# CMake support for the Flight Software Framework Tests +# +# Developed in an effort to replace Make with a modern build system. +# +# Author: R. Mueller +################################################################################ + +################################################################################ +# Pre-Project preparation +################################################################################ +cmake_minimum_required(VERSION 3.13) + +# set(CMAKE_VERBOSE TRUE) + +set(CMAKE_SCRIPT_PATH "${CMAKE_CURRENT_SOURCE_DIR}/buildsystem/cmake") + +# Tests can be built with the Host OSAL or with the Linux OSAL. +if(NOT OS_FSFW) + set(OS_FSFW host CACHE STRING "OS for the FSFW.") +endif() + +option(CUSTOM_UNITTEST_RUNNER + "Specify whether custom main or Catch2 main is used" TRUE +) + +# Perform steps like loading toolchain files where applicable. +#include(${CMAKE_SCRIPT_PATH}/PreProjectConfig.cmake) +#pre_project_config() + +# Project Name +project(fsfw_tests C CXX) + +################################################################################ +# Pre-Sources preparation +################################################################################ + +# Specify the C++ standard +set(CMAKE_CXX_STANDARD 11) +set(CMAKE_CXX_STANDARD_REQUIRED True) + +# Set names and variables +set(TARGET_NAME ${CMAKE_PROJECT_NAME}) +if(CUSTOM_UNITTEST_RUNNER) + set(CATCH2_TARGET Catch2) +else() + set(CATCH2_TARGET Catch2WithMain) +endif() +set(LIB_FSFW_NAME fsfw) + +# Set path names +set(FSFW_PATH fsfw) +set(CATCH2_PATH Catch2) +set(FSFW_TESTS_PATH fsfw/unittest) +set(TEST_SETUP_PATH unittest) + +# Analyse different OS and architecture/target options and +# determine BSP_PATH + +# FreeRTOS +if(${OS_FSFW} STREQUAL linux) + add_definitions(-DUNIX -DLINUX) + find_package(Threads REQUIRED) +# Hosted +else() + if(WIN32) + add_definitions(-DWIN32) + elseif(UNIX) + find_package(Threads REQUIRED) + add_definitions(-DUNIX -DLINUX) + endif() +endif() + +set(FSFW_CONFIG_PATH testcfg) + +################################################################################ +# Executable and Sources +################################################################################ + +# Add executable +add_executable(${TARGET_NAME}) + +# Add subdirectories +add_subdirectory(${FSFW_PATH}) +add_subdirectory(${CATCH2_PATH}) +add_subdirectory(${FSFW_CONFIG_PATH}) +add_subdirectory(${FSFW_TESTS_PATH}) +add_subdirectory(${TEST_SETUP_PATH}) + +################################################################################ +# Post-Sources preparation +################################################################################ + +# Add libraries for all sources. +target_link_libraries(${TARGET_NAME} PRIVATE + ${LIB_FSFW_NAME} + ${CATCH2_TARGET} +) + +# Add include paths for all sources. +target_include_directories(${TARGET_NAME} PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${FSFW_CONFIG_PATH} +) + +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set(WARNING_FLAGS + -Wall + -Wextra + -Wshadow=local + -Wimplicit-fallthrough=1 + -Wno-unused-parameter + -Wno-psabi + ) + + # Remove unused sections. + target_compile_options(${TARGET_NAME} PRIVATE + "-ffunction-sections" + "-fdata-sections" + ) + + # Removed unused sections. + target_link_options(${TARGET_NAME} PRIVATE + "-Wl,--gc-sections" + ) + +elseif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + set(COMPILER_FLAGS "/permissive-") +endif() + +if(CMAKE_VERBOSE) + message(STATUS "Warning flags: ${WARNING_FLAGS}") +endif() + + +# Compile options for all sources. +target_compile_options(${TARGET_NAME} PRIVATE + $<$:${WARNING_FLAGS} ${COMPILER_FLAGS}> + $<$:${WARNING_FLAGS} ${COMPILER_FLAGS}> + ${ABI_FLAGS} +) + +if(NOT CMAKE_SIZE) + set(CMAKE_SIZE size) + if(WIN32) + set(FILE_SUFFIX ".exe") + endif() +endif() + +add_custom_command( + TARGET ${TARGET_NAME} + POST_BUILD + COMMAND echo "Build directory: ${CMAKE_BINARY_DIR}" + COMMAND echo "Target OSAL: ${OS_FSFW}" + COMMAND echo "Target Build Type: ${CMAKE_BUILD_TYPE}" + COMMAND ${CMAKE_SIZE} ${TARGET_NAME}${FILE_SUFFIX} +) + +include (${CMAKE_CURRENT_SOURCE_DIR}/buildsystem/cmake/BuildType.cmake) +set_build_type() + + + + + -- 2.34.1 From 254b1437e978f7f17a57a07876f3ac7668898ef0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 27 Dec 2020 15:15:51 +0100 Subject: [PATCH 19/24] repaired windows unittests --- unittest/internal/osal/IntTestMq.cpp | 6 +++--- unittest/internal/osal/IntTestMutex.cpp | 6 +++--- unittest/internal/osal/IntTestSemaphore.cpp | 11 ++++++----- unittest/internal/serialize/IntTestSerialization.cpp | 8 ++++---- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/unittest/internal/osal/IntTestMq.cpp b/unittest/internal/osal/IntTestMq.cpp index 9917285f..fc8e963e 100644 --- a/unittest/internal/osal/IntTestMq.cpp +++ b/unittest/internal/osal/IntTestMq.cpp @@ -1,8 +1,8 @@ #include "IntTestMq.h" -#include "../UnittDefinitions.h" +#include -#include "../../../ipc/MessageQueueIF.h" -#include "../../../ipc/QueueFactory.h" +#include +#include #include diff --git a/unittest/internal/osal/IntTestMutex.cpp b/unittest/internal/osal/IntTestMutex.cpp index d2f8b962..3316de74 100644 --- a/unittest/internal/osal/IntTestMutex.cpp +++ b/unittest/internal/osal/IntTestMutex.cpp @@ -1,10 +1,10 @@ #include "IntTestMutex.h" -#include "../../../ipc/MutexFactory.h" -#include "../UnittDefinitions.h" +#include +#include #if defined(hosted) -#include "../../osal/hosted/Mutex.h" +#include #include #include #endif diff --git a/unittest/internal/osal/IntTestSemaphore.cpp b/unittest/internal/osal/IntTestSemaphore.cpp index 6d2719d5..5a82f78c 100644 --- a/unittest/internal/osal/IntTestSemaphore.cpp +++ b/unittest/internal/osal/IntTestSemaphore.cpp @@ -1,10 +1,11 @@ #include "IntTestSemaphore.h" -#include "../UnittDefinitions.h" +#include -#include "../../../tasks/SemaphoreFactory.h" -#include "../../../serviceinterface/ServiceInterfaceStream.h" -#include "../../../timemanager/Stopwatch.h" +#include +#include +#include +#include void testsemaph::testBinSemaph() { std::string id = "[BinSemaphore]"; @@ -138,7 +139,7 @@ void testsemaph::testCountingSemaphImplementation(SemaphoreIF* countingSemaph, // attempt to take when count is 0, measure time result = countingSemaph->acquire(SemaphoreIF::TimeoutType::WAITING, 10); dur_millis_t time = stopwatch.stop(); - if(abs(time - 10) > 1) { + if(std::abs(static_cast(time - 10)) > 1) { unitt::put_error(id); } } diff --git a/unittest/internal/serialize/IntTestSerialization.cpp b/unittest/internal/serialize/IntTestSerialization.cpp index 793661c5..69d82942 100644 --- a/unittest/internal/serialize/IntTestSerialization.cpp +++ b/unittest/internal/serialize/IntTestSerialization.cpp @@ -1,9 +1,9 @@ #include "IntTestSerialization.h" -#include "../UnittDefinitions.h" +#include -#include "../../../serialize/SerializeElement.h" -#include "../../../serialize/SerialBufferAdapter.h" -#include "../../../serialize/SerializeIF.h" +#include +#include +#include #include -- 2.34.1 From f9bcc835d7e170c97b153422ea80f2a2306dbe16 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 28 Dec 2020 01:17:03 +0100 Subject: [PATCH 20/24] include updated --- unittest/tests/container/TestFixedArrayList.cpp | 4 ++-- unittest/tests/container/TestFixedOrderedMultimap.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/unittest/tests/container/TestFixedArrayList.cpp b/unittest/tests/container/TestFixedArrayList.cpp index b2baa3e7..1a85f30d 100644 --- a/unittest/tests/container/TestFixedArrayList.cpp +++ b/unittest/tests/container/TestFixedArrayList.cpp @@ -1,5 +1,5 @@ -#include "../../../container/FixedArrayList.h" -#include "../../../returnvalues/HasReturnvaluesIF.h" +#include +#include #include #include diff --git a/unittest/tests/container/TestFixedOrderedMultimap.cpp b/unittest/tests/container/TestFixedOrderedMultimap.cpp index b337a9c8..a47d6efb 100644 --- a/unittest/tests/container/TestFixedOrderedMultimap.cpp +++ b/unittest/tests/container/TestFixedOrderedMultimap.cpp @@ -1,5 +1,5 @@ -#include "../../../container/FixedOrderedMultimap.h" -#include "../../../returnvalues/HasReturnvaluesIF.h" +#include +#include #include #include -- 2.34.1 From b126149215b8f1c9536f8a9c19ea63705f7ed7c6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 16:49:43 +0100 Subject: [PATCH 21/24] 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} ) -- 2.34.1 From d268c6d96cf22cb7317c87b80f648b81c8afb1db Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 16:59:39 +0100 Subject: [PATCH 22/24] 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 -- 2.34.1 From bc475f0a9a794d75b90317ade116d7e3ada7aa9b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 17:48:26 +0100 Subject: [PATCH 23/24] 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} +) + -- 2.34.1 From 91cb061da941f610d2c7001ed2e47d64eaa6872f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 28 Dec 2020 17:55:19 +0100 Subject: [PATCH 24/24] datapool updates, fixes, pool read helper --- datapool/PoolDataSetBase.cpp | 49 ++++++++++++++++++++------ datapool/PoolDataSetBase.h | 31 +++++++++++----- datapool/PoolDataSetIF.h | 6 ++-- datapool/PoolVariableIF.h | 40 ++------------------- datapool/ReadCommitIF.h | 31 ++++++++++++++++ datapoollocal/HasLocalDataPoolIF.h | 3 ++ datapoollocal/LocalDataPoolManager.cpp | 2 +- datapoollocal/LocalDataPoolManager.h | 15 ++++---- datapoollocal/LocalPoolDataSetBase.cpp | 32 ++++++++++------- datapoollocal/LocalPoolDataSetBase.h | 15 ++++++-- datapoollocal/LocalPoolVariable.tpp | 18 +++++----- datapoollocal/PoolReadHelper.h | 41 +++++++++++++++++++++ 12 files changed, 191 insertions(+), 92 deletions(-) create mode 100644 datapool/ReadCommitIF.h create mode 100644 datapoollocal/PoolReadHelper.h diff --git a/datapool/PoolDataSetBase.cpp b/datapool/PoolDataSetBase.cpp index cb2348f7..8f073359 100644 --- a/datapool/PoolDataSetBase.cpp +++ b/datapool/PoolDataSetBase.cpp @@ -9,9 +9,10 @@ PoolDataSetBase::PoolDataSetBase(PoolVariableIF** registeredVariablesArray, 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,15 +34,16 @@ ReturnValue_t PoolDataSetBase::registerVariable( ReturnValue_t PoolDataSetBase::read(uint32_t lockTimeout) { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - if (state == States::DATA_SET_UNINITIALISED) { + 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::DATA_SET_WAS_READ; + state = States::STATE_SET_WAS_READ; unlockDataPool(); } else { @@ -50,6 +52,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; } @@ -71,7 +77,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; } @@ -80,7 +92,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; } @@ -96,10 +108,15 @@ 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::DATA_SET_UNINITIALISED; + state = States::STATE_SET_UNINITIALISED; unlockDataPool(); } @@ -111,7 +128,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) { @@ -121,7 +144,7 @@ ReturnValue_t PoolDataSetBase::handleUnreadDatasetCommit(uint32_t lockTimeout) { } } } - state = States::DATA_SET_UNINITIALISED; + state = States::STATE_SET_UNINITIALISED; unlockDataPool(); return result; } @@ -172,3 +195,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 a8931d62..7c1d84a4 100644 --- a/datapool/PoolDataSetBase.h +++ b/datapool/PoolDataSetBase.h @@ -44,6 +44,7 @@ public: /** * @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 @@ -52,13 +53,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 +77,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 @@ -89,6 +91,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 +117,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 @@ -124,14 +136,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. @@ -144,6 +156,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/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/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.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/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; } diff --git a/datapoollocal/LocalPoolDataSetBase.cpp b/datapoollocal/LocalPoolDataSetBase.cpp index 640956db..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); } } @@ -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, @@ -276,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..d9b6a221 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; @@ -181,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 diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index 48649ad5..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){} @@ -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; } 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_ */ -- 2.34.1