From 44a5430555766db0d6bae8a1dca7221c122c80a9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 7 Jan 2021 20:28:03 +0100 Subject: [PATCH 01/19] pool vector improvements --- datapoollocal/LocalPoolVector.tpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index 99e37a87..88ba084a 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -32,10 +32,10 @@ inline ReturnValue_t LocalPoolVector::read( template inline ReturnValue_t LocalPoolVector::readWithoutLock() { if(readWriteMode == pool_rwm_t::VAR_WRITE) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolVector: Invalid read write " - "mode for read() call." << std::endl; -#endif + object_id_t targetObjectId = hkManager->getOwner()->getObjectId(); + reportReadCommitError("LocalPoolVector", + PoolVariableIF::INVALID_READ_WRITE_MODE, true, targetObjectId, + localPoolId); return PoolVariableIF::INVALID_READ_WRITE_MODE; } @@ -64,20 +64,17 @@ inline ReturnValue_t LocalPoolVector::commit( template inline ReturnValue_t LocalPoolVector::commitWithoutLock() { if(readWriteMode == pool_rwm_t::VAR_READ) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolVector: Invalid read write " - "mode for commit call." << std::endl; -#else - sif::warning << "LocalPoolVector: Invalid read write " - "mode for commit call." << std::endl; -#endif + object_id_t targetObjectId = hkManager->getOwner()->getObjectId(); + reportReadCommitError("LocalPoolVector", + PoolVariableIF::INVALID_READ_WRITE_MODE, false, targetObjectId, + localPoolId); return PoolVariableIF::INVALID_READ_WRITE_MODE; } PoolEntry* poolEntry = nullptr; ReturnValue_t result = hkManager->fetchPoolEntry(localPoolId, &poolEntry); if(result != RETURN_OK) { object_id_t targetObjectId = hkManager->getOwner()->getObjectId(); - reportReadCommitError("LocalPoolVector", result, true, targetObjectId, + reportReadCommitError("LocalPoolVector", result, false, targetObjectId, localPoolId); return result; } From d3fbe4a3b96184b1f1620041fd7f433cf902b56e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 7 Jan 2021 20:29:38 +0100 Subject: [PATCH 02/19] local pool variable error handling improved --- datapoollocal/LocalPoolVariable.tpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index 4d472c31..3cf0fc84 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -33,13 +33,10 @@ inline ReturnValue_t LocalPoolVariable::read( template inline ReturnValue_t LocalPoolVariable::readWithoutLock() { if(readWriteMode == pool_rwm_t::VAR_WRITE) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolVariable: Invalid read write " - "mode for read call." << std::endl; -#else - fsfw::printWarning("LocalPoolVariable: Invalid read write " - "mode for read call.\n\r"); -#endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ + object_id_t targetObjectId = hkManager->getOwner()->getObjectId(); + reportReadCommitError("LocalPoolVector", + PoolVariableIF::INVALID_READ_WRITE_MODE, true, targetObjectId, + localPoolId); return PoolVariableIF::INVALID_READ_WRITE_MODE; } @@ -83,13 +80,10 @@ inline ReturnValue_t LocalPoolVariable::commit( template inline ReturnValue_t LocalPoolVariable::commitWithoutLock() { if(readWriteMode == pool_rwm_t::VAR_READ) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalPoolVariable: Invalid read write " - "mode for commit call." << std::endl; -#else - fsfw::printWarning("LocalPoolVariable: Invalid read write " - "mode for commit call.\n\r"); -#endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ + object_id_t targetObjectId = hkManager->getOwner()->getObjectId(); + reportReadCommitError("LocalPoolVector", + PoolVariableIF::INVALID_READ_WRITE_MODE, false, targetObjectId, + localPoolId); return PoolVariableIF::INVALID_READ_WRITE_MODE; } PoolEntry* poolEntry = nullptr; From d3405ee34063eca17a529dcf521ff860d91e5b50 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 7 Jan 2021 20:30:52 +0100 Subject: [PATCH 03/19] whitespace --- datapoollocal/LocalPoolVariable.tpp | 1 + 1 file changed, 1 insertion(+) diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index 3cf0fc84..d9d45d38 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -86,6 +86,7 @@ inline ReturnValue_t LocalPoolVariable::commitWithoutLock() { localPoolId); return PoolVariableIF::INVALID_READ_WRITE_MODE; } + PoolEntry* poolEntry = nullptr; ReturnValue_t result = hkManager->fetchPoolEntry(localPoolId, &poolEntry); if(result != RETURN_OK) { From c1c331e29ef4501d923830cc38a3179d7d7b6332 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 00:10:10 +0100 Subject: [PATCH 04/19] fixed vector test --- datapool/PoolEntry.cpp | 14 +++--------- datapool/PoolEntry.h | 8 +++---- datapoollocal/LocalPoolVector.h | 8 +++++++ datapoollocal/LocalPoolVector.tpp | 7 ++++++ osal/host/Mutex.h | 1 - unittest/tests/datapoollocal/CMakeLists.txt | 1 + .../datapoollocal/LocalPoolVectorTest.cpp | 22 +++++++++++++++++++ 7 files changed, 45 insertions(+), 16 deletions(-) diff --git a/datapool/PoolEntry.cpp b/datapool/PoolEntry.cpp index 5f5f56f2..d21cf2ec 100644 --- a/datapool/PoolEntry.cpp +++ b/datapool/PoolEntry.cpp @@ -6,27 +6,19 @@ #include template -PoolEntry::PoolEntry(std::initializer_list initValue, uint8_t setLength, - bool setValid ) : length(setLength), valid(setValid) { +PoolEntry::PoolEntry(std::initializer_list initValue, bool setValid ): + length(initValue.size()), valid(setValid) { this->address = new T[this->length]; if(initValue.size() == 0) { std::memset(this->address, 0, this->getByteSize()); } - else if (initValue.size() != setLength){ -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "PoolEntry: setLength is not equal to initializer list" - "length! Performing zero initialization with given setLength" - << std::endl; -#endif - std::memset(this->address, 0, this->getByteSize()); - } else { std::copy(initValue.begin(), initValue.end(), this->address); } } template -PoolEntry::PoolEntry( T* initValue, uint8_t setLength, bool setValid ) : +PoolEntry::PoolEntry(T* initValue, uint8_t setLength, bool setValid): length(setLength), valid(setValid) { this->address = new T[this->length]; if (initValue != nullptr) { diff --git a/datapool/PoolEntry.h b/datapool/PoolEntry.h index 033db40d..83f7fc50 100644 --- a/datapool/PoolEntry.h +++ b/datapool/PoolEntry.h @@ -43,16 +43,16 @@ public: * corresponding length should be passed too, otherwise a zero * initialization will be performed with the given setLength. * @param initValue - * Initializer list with values to initialize with, for example {0,0} to - * initialize the two entries to zero. + * Initializer list with values to initialize with, for example {0, 0} to + * initialize the a pool entry of a vector with two entries to 0. * @param setLength * Defines the array length of this entry. Should be equal to the * intializer list length. * @param setValid * Sets the initialization flag. It is invalid by default. */ - PoolEntry(std::initializer_list initValue = {}, uint8_t setLength = 1, - bool setValid = false); + PoolEntry(std::initializer_list initValue = {0}, bool setValid = false); + /** * @brief In the classe's constructor, space is allocated on the heap and * potential init values are copied to that space. diff --git a/datapoollocal/LocalPoolVector.h b/datapoollocal/LocalPoolVector.h index 773a52cc..ce3b1a14 100644 --- a/datapoollocal/LocalPoolVector.h +++ b/datapoollocal/LocalPoolVector.h @@ -139,6 +139,14 @@ public: MutexIF::TimeoutType::WAITING, uint32_t timeoutMs = 20) override; + /** + * @brief This commit call also sets the validity of the pool entry. + * @details + */ + ReturnValue_t commit(bool valid, MutexIF::TimeoutType timeoutType = + MutexIF::TimeoutType::WAITING, + uint32_t timeoutMs = 20); + protected: /** * @brief Like #read, but without a lock protection of the global pool. diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index 88ba084a..ce68982b 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -54,6 +54,13 @@ inline ReturnValue_t LocalPoolVector::readWithoutLock() { return RETURN_OK; } +template +inline ReturnValue_t LocalPoolVector::commit(bool valid, + MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) { + this->setValid(valid); + return commit(timeoutType, timeoutMs); +} + template inline ReturnValue_t LocalPoolVector::commit( MutexIF::TimeoutType timeoutType, uint32_t timeoutMs) { diff --git a/osal/host/Mutex.h b/osal/host/Mutex.h index c0fa19b7..0bd93c8a 100644 --- a/osal/host/Mutex.h +++ b/osal/host/Mutex.h @@ -22,7 +22,6 @@ public: std::timed_mutex* getMutexHandle(); private: - //bool locked = false; std::timed_mutex mutex; }; diff --git a/unittest/tests/datapoollocal/CMakeLists.txt b/unittest/tests/datapoollocal/CMakeLists.txt index 569a8d5f..9d828de5 100644 --- a/unittest/tests/datapoollocal/CMakeLists.txt +++ b/unittest/tests/datapoollocal/CMakeLists.txt @@ -1,3 +1,4 @@ target_sources(${TARGET_NAME} PRIVATE LocalPoolVariableTest.cpp + LocalPoolVectorTest.cpp ) diff --git a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp index c86a2681..188011df 100644 --- a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp @@ -11,6 +11,28 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); + + SECTION("BasicTest") { + // very basic test. + lp_vec_t testVector = lp_vec_t( + objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); + CHECK(testVector.read() == retval::CATCH_OK); + testVector.value[0] = 5; + testVector.value[1] = 232; + testVector.value[2] = 32023; + + REQUIRE(testVector.commit(true) == retval::CATCH_OK); + CHECK(testVector.isValid()); + + testVector.value[0] = 0; + testVector.value[1] = 0; + testVector.value[2] = 0; + + CHECK(testVector.read() == retval::CATCH_OK); + CHECK(testVector.value[0] == 5); + CHECK(testVector.value[1] == 232); + CHECK(testVector.value[2] == 32023); + } } From 8e3f4c81a5d0db9709052012e71824ca019938b9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 00:20:39 +0100 Subject: [PATCH 05/19] pool entry doc correction --- datapool/PoolEntry.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/datapool/PoolEntry.h b/datapool/PoolEntry.h index 83f7fc50..d3d4cd6d 100644 --- a/datapool/PoolEntry.h +++ b/datapool/PoolEntry.h @@ -35,19 +35,17 @@ public: "uint8_t"); /** * @brief In the classe's constructor, space is allocated on the heap and - * potential init values are copied to that space. + * potential initialization values are copied to that space. * @details * Not passing any arguments will initialize an non-array pool entry - * (setLength = 1) with an initial invalid state. - * Please note that if an initializer list is passed, the correct - * corresponding length should be passed too, otherwise a zero - * initialization will be performed with the given setLength. + * with an initial invalid state and the value 0. + * Please note that if an initializer list is passed, the length of the + * initializer list needs to be correct for vector entries because + * required allocated space will be deduced from the initializer list length + * and the pool entry type. * @param initValue * Initializer list with values to initialize with, for example {0, 0} to * initialize the a pool entry of a vector with two entries to 0. - * @param setLength - * Defines the array length of this entry. Should be equal to the - * intializer list length. * @param setValid * Sets the initialization flag. It is invalid by default. */ From 41d8cbda5597c61aac6c965cc14f4fe571fe3263 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 00:22:04 +0100 Subject: [PATCH 06/19] small form changes --- datapool/PoolEntry.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datapool/PoolEntry.h b/datapool/PoolEntry.h index d3d4cd6d..aecfce94 100644 --- a/datapool/PoolEntry.h +++ b/datapool/PoolEntry.h @@ -64,9 +64,9 @@ public: */ PoolEntry(T* initValue, uint8_t setLength = 1, bool setValid = false); - //! Explicitely deleted copy ctor, copying is not allowed! + //! Explicitely deleted copy ctor, copying is not allowed. PoolEntry(const PoolEntry&) = delete; - //! Explicitely deleted copy assignment, copying is not allowed! + //! Explicitely deleted copy assignment, copying is not allowed. PoolEntry& operator=(const PoolEntry&) = delete; /** From 4255176b5c499d3be11947f8b77d9bbcc21bb12c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 01:49:00 +0100 Subject: [PATCH 07/19] more tests, minor bugfix --- datapoollocal/LocalPoolVector.h | 5 +-- datapoollocal/LocalPoolVector.tpp | 18 ++++++---- .../datapoollocal/LocalPoolVectorTest.cpp | 34 ++++++++++++++++++- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/datapoollocal/LocalPoolVector.h b/datapoollocal/LocalPoolVector.h index ce3b1a14..a28fe327 100644 --- a/datapoollocal/LocalPoolVector.h +++ b/datapoollocal/LocalPoolVector.h @@ -100,8 +100,8 @@ public: return vectorSize; } - T& operator [](int i); - const T &operator [](int i) const; + T& operator [](size_t i); + const T &operator [](size_t i) const; virtual ReturnValue_t serialize(uint8_t** buffer, size_t* size, const size_t maxSize, @@ -126,6 +126,7 @@ public: ReturnValue_t read(MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING, uint32_t timeoutMs = 20) override; + /** * @brief The commit call copies the array values back to the data pool. * @details diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index ce68982b..6612a491 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -91,8 +91,8 @@ inline ReturnValue_t LocalPoolVector::commitWithoutLock() { } template -inline T& LocalPoolVector::operator [](int i) { - if(i <= vectorSize) { +inline T& LocalPoolVector::operator [](size_t i) { + if(i < vectorSize) { return value[i]; } // If this happens, I have to set some value. I consider this @@ -100,13 +100,16 @@ inline T& LocalPoolVector::operator [](int i) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "LocalPoolVector: Invalid index. Setting or returning" " last value!" << std::endl; +#else + fsfw::printError("LocalPoolVector: Invalid index. Setting or returning" + " last value!\n\r"); #endif - return value[i]; + return value[vectorSize]; } template -inline const T& LocalPoolVector::operator [](int i) const { - if(i <= vectorSize) { +inline const T& LocalPoolVector::operator [](size_t i) const { + if(i < vectorSize) { return value[i]; } // If this happens, I have to set some value. I consider this @@ -114,8 +117,11 @@ inline const T& LocalPoolVector::operator [](int i) const { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "LocalPoolVector: Invalid index. Setting or returning" " last value!" << std::endl; +#else + fsfw::printError("LocalPoolVector: Invalid index. Setting or returning" + " last value!\n\r"); #endif - return value[i]; + return value[vectorSize]; } template diff --git a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp index 188011df..7e40e898 100644 --- a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp @@ -16,7 +16,7 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { // very basic test. lp_vec_t testVector = lp_vec_t( objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); - CHECK(testVector.read() == retval::CATCH_OK); + REQUIRE(testVector.read() == retval::CATCH_OK); testVector.value[0] = 5; testVector.value[1] = 232; testVector.value[2] = 32023; @@ -32,6 +32,38 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { CHECK(testVector.value[0] == 5); CHECK(testVector.value[1] == 232); CHECK(testVector.value[2] == 32023); + + CHECK(testVector[0] == 5); + + // This is invalid access, so the last value will be set instead. + // (we can't throw exceptions) + testVector[4] = 12; + CHECK(testVector[3] == 12); + CHECK(testVector.commit() == retval::CATCH_OK); + + // Use read-only reference. + const lp_vec_t& roTestVec = testVector; + uint16_t valueOne = roTestVec[0]; + CHECK(valueOne == 5); + + uint16_t lastVal = roTestVec[25]; + CHECK(lastVal == 12); + } + + SECTION("ErrorHandling") { + // not try to use a local pool variable which does not exist + lp_vec_t invalidVector = lp_vec_t( + objects::TEST_LOCAL_POOL_OWNER_BASE, 0xffffffff); + REQUIRE(invalidVector.read() == + static_cast(HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND)); + REQUIRE(invalidVector.commit() == + static_cast(HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND)); + + // now try to access with wrong type + lp_vec_t invalidVector2 = lp_vec_t( + objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); + REQUIRE(invalidVector2.read() == + static_cast(HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT)); } } From 2ef3e0aa7b502b5ac1d40057a4fdc9b324af9fcb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 02:06:27 +0100 Subject: [PATCH 08/19] added option to add CR for printf support --- datapoollocal/LocalDataPoolManager.h | 8 ++++---- datapoollocal/LocalPoolObjectBase.cpp | 11 +++++------ datapoollocal/LocalPoolVector.tpp | 12 ++++++------ serviceinterface/ServiceInterfacePrinter.cpp | 10 +++++++++- serviceinterface/ServiceInterfacePrinter.h | 2 ++ 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index bcb8f0af..7fa480cb 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -377,11 +377,11 @@ ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager::fechPoolEntry: Pool entry " + sif::warning << "HousekeepingManager::fetchPoolEntry: Pool entry " "not found." << std::endl; #else - fsfw::printWarning("HousekeepingManager::fechPoolEntry: Pool entry " - "not found."); + fsfw::printWarning("HousekeepingManager::fetchPoolEntry: Pool entry " + "not found.\n"); #endif return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } @@ -393,7 +393,7 @@ ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, " Pool entry type conflict." << std::endl; #else fsfw::printWarning("HousekeepingManager::fetchPoolEntry:" - " Pool entry type conflict."); + " Pool entry type conflict.\n"); #endif return HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT; } diff --git a/datapoollocal/LocalPoolObjectBase.cpp b/datapoollocal/LocalPoolObjectBase.cpp index b84569b8..bb7822da 100644 --- a/datapoollocal/LocalPoolObjectBase.cpp +++ b/datapoollocal/LocalPoolObjectBase.cpp @@ -110,13 +110,12 @@ void LocalPoolObjectBase::reportReadCommitError(const char* variableType, #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << variableType << ": " << type << " call | " << errMsg - << " | Owner: " << std::hex << std::setw(8) - << std::setfill('0') << objectId << " LPID: 0x" << lpId - << std::dec << std::endl; + << " | Owner: 0x" << std::hex << std::setw(8) + << std::setfill('0') << objectId << std::dec << " LPID: " << lpId + << std::endl; #else - fsfw::printWarning("LocalPoolVariable: %s of local pool variable of " - "object 0x%08x and lp ID 0x%08x failed.\n\r", - type, objectId, lpId); + fsfw::printWarning("%s: %s call | %s | Owner: 0x%08x LPID: %lu\n", + variableType, type, errMsg, objectId, lpId); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif /* FSFW_DISABLE_PRINTOUT == 0 */ } diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index 6612a491..94f3916e 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -98,11 +98,11 @@ inline T& LocalPoolVector::operator [](size_t i) { // If this happens, I have to set some value. I consider this // a configuration error, but I wont exit here. #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalPoolVector: Invalid index. Setting or returning" + sif::warning << "LocalPoolVector: Invalid index. Setting or returning" " last value!" << std::endl; #else - fsfw::printError("LocalPoolVector: Invalid index. Setting or returning" - " last value!\n\r"); + fsfw::printWarning("LocalPoolVector: Invalid index. Setting or returning" + " last value!\n"); #endif return value[vectorSize]; } @@ -115,11 +115,11 @@ inline const T& LocalPoolVector::operator [](size_t i) const { // If this happens, I have to set some value. I consider this // a configuration error, but I wont exit here. #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalPoolVector: Invalid index. Setting or returning" + sif::warning << "LocalPoolVector: Invalid index. Setting or returning" " last value!" << std::endl; #else - fsfw::printError("LocalPoolVector: Invalid index. Setting or returning" - " last value!\n\r"); + fsfw::printWarning("LocalPoolVector: Invalid index. Setting or returning" + " last value!\n"); #endif return value[vectorSize]; } diff --git a/serviceinterface/ServiceInterfacePrinter.cpp b/serviceinterface/ServiceInterfacePrinter.cpp index 15e738a8..cb3a958d 100644 --- a/serviceinterface/ServiceInterfacePrinter.cpp +++ b/serviceinterface/ServiceInterfacePrinter.cpp @@ -10,7 +10,7 @@ fsfw::PrintLevel printLevel = fsfw::PrintLevel::DEBUG; #if defined(WIN32) && FSFW_COLORED_OUTPUT == 1 bool consoleInitialized = false; #endif /* defined(WIN32) && FSFW_COLORED_OUTPUT == 1 */ - +bool addCrAtEnd = false; #if FSFW_DISABLE_PRINTOUT == 0 uint8_t printBuffer[fsfwconfig::FSFW_PRINT_BUFFER_SIZE]; @@ -80,6 +80,10 @@ void fsfwPrint(fsfw::PrintLevel printType, const char* fmt, va_list arg) { len += vsnprintf(bufferPosition + len, sizeof(printBuffer) - len, fmt, arg); + if(addCrAtEnd) { + len += sprintf(bufferPosition + len, "\r"); + } + printf("%s", printBuffer); } @@ -105,6 +109,10 @@ void fsfw::printDebug(const char *fmt, ...) { va_end(args); } +void fsfw::setToAddCrAtEnd(bool addCrAtEnd_) { + addCrAtEnd = addCrAtEnd_; +} + void fsfw::printError(const char *fmt, ...) { va_list args; va_start(args, fmt); diff --git a/serviceinterface/ServiceInterfacePrinter.h b/serviceinterface/ServiceInterfacePrinter.h index a60b6332..1f82ba98 100644 --- a/serviceinterface/ServiceInterfacePrinter.h +++ b/serviceinterface/ServiceInterfacePrinter.h @@ -16,6 +16,8 @@ enum class PrintLevel { void setPrintLevel(PrintLevel printLevel); PrintLevel getPrintLevel(); +void setToAddCrAtEnd(bool addCrAtEnd_); + /** * These functions can be used like the C stdio printf and forward the * supplied formatted string arguments to a printf function. From 3b39c6b6e26fd523b732b14bf00c4c5933cf0ea3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 02:07:37 +0100 Subject: [PATCH 09/19] defaultcfg update --- defaultcfg/fsfwconfig/FSFWConfig.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index 261e3d6d..e251729c 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -9,8 +9,7 @@ //! the C stdio functions can be used alternatively #define FSFW_CPP_OSTREAM_ENABLED 1 -//! More FSFW related printouts. -//! Be careful, this also turns off most diagnostic prinouts! +//! More FSFW related printouts. Useful for development. #define FSFW_ENHANCED_PRINTOUT 0 //! Can be used to completely disable printouts, even the C stdio ones. From 5a8647d36701b9eda4d6b6a29f2d83dd9b5eeeab Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 13:40:13 +0100 Subject: [PATCH 10/19] better returnvalues etc. --- datapool/PoolEntry.cpp | 26 +++++++++++++------ datapool/PoolEntry.h | 35 ++++++++++++++++---------- datapoollocal/LocalDataPoolManager.cpp | 6 ++--- datapoollocal/LocalDataPoolManager.h | 10 +++++--- datapoollocal/LocalPoolVariable.tpp | 22 ++++++++-------- datapoollocal/LocalPoolVector.tpp | 8 +++--- globalfunctions/arrayprinter.cpp | 10 +++++++- 7 files changed, 74 insertions(+), 43 deletions(-) diff --git a/datapool/PoolEntry.cpp b/datapool/PoolEntry.cpp index d21cf2ec..1e99a28c 100644 --- a/datapool/PoolEntry.cpp +++ b/datapool/PoolEntry.cpp @@ -1,6 +1,6 @@ #include "PoolEntry.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include "../globalfunctions/arrayprinter.h" #include #include @@ -62,14 +62,26 @@ bool PoolEntry::getValid() { template void PoolEntry::print() { + const char* validString = nullptr; + if(valid) { + validString = "Valid"; + } + else { + validString = "Invalid"; + } #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "Pool Entry Validity: " << - (this->valid? " (valid) " : " (invalid) ") << std::endl; -#endif - arrayprinter::print(reinterpret_cast(address), length); -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << std::dec << std::endl; + sif::info << "PoolEntry information." << std::endl; + sif::info << "PoolEntry validity: " << validString << std::endl; +#else + fsfw::printInfo("PoolEntry information.\n"); + fsfw::printInfo("PoolEntry validity: %s\n", validString); #endif + arrayprinter::print(reinterpret_cast(address), getByteSize()); +} + +template +inline T* PoolEntry::getDataPtr() { + return this->address; } template diff --git a/datapool/PoolEntry.h b/datapool/PoolEntry.h index aecfce94..30940320 100644 --- a/datapool/PoolEntry.h +++ b/datapool/PoolEntry.h @@ -80,21 +80,16 @@ public: ~PoolEntry(); /** - * @brief This is the address pointing to the allocated memory. + * Return typed pointer to start of data. + * @return */ - T* address; - /** - * @brief This attribute stores the length information. - */ - uint8_t length; - /** - * @brief Here, the validity information for a variable is stored. - * Every entry (single variable or vector) has one valid flag. - */ - uint8_t valid; + T* getDataPtr(); + /** * @brief getSize returns the array size of the entry. - * @details A single parameter has size 1. + * @details + * For non-array pool entries return type size, for vector entries + * return type size times the number of entries. */ uint8_t getSize(); /** @@ -121,8 +116,22 @@ public: * information to the screen. It prints all array entries in a row. */ void print(); - Type getType(); + +private: + /** + * @brief This attribute stores the length information. + */ + uint8_t length; + /** + * @brief Here, the validity information for a variable is stored. + * Every entry (single variable or vector) has one valid flag. + */ + uint8_t valid; + /** + * @brief This is the address pointing to the allocated memory. + */ + T* address; }; #endif /* FSFW_DATAPOOL_POOLENTRY_H_ */ diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 24516aad..db5cd5dd 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -213,7 +213,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolObjectBase* poolObj = owner->getPoolObjectHandle( receiver.dataId.localPoolId); if(poolObj == nullptr) { - return HasReturnvaluesIF::RETURN_FAILED; + return POOLOBJECT_NOT_FOUND; } if (not poolObj->hasChanged()) { @@ -249,7 +249,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolDataSetBase* dataSet = owner->getDataSetHandle( receiver.dataId.sid); if(dataSet == nullptr) { - return HasReturnvaluesIF::RETURN_FAILED; + return DATASET_NOT_FOUND; } if(not dataSet->hasChanged()) { @@ -618,7 +618,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, sif::warning << "HousekeepingManager::generateHousekeepingPacket:" << " Set ID not found or dataset not assigned!" << std::endl; #endif - return HasReturnvaluesIF::RETURN_FAILED; + return DATASET_NOT_FOUND; } store_address_t storeId; diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 7fa480cb..098158da 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -56,11 +56,13 @@ class LocalDataPoolManager { public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::HOUSEKEEPING_MANAGER; - 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(0); - 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); + static constexpr ReturnValue_t WRONG_HK_PACKET_TYPE = MAKE_RETURN_CODE(1); + static constexpr ReturnValue_t REPORTING_STATUS_UNCHANGED = MAKE_RETURN_CODE(2); + static constexpr ReturnValue_t PERIODIC_HELPER_INVALID = MAKE_RETURN_CODE(3); + static constexpr ReturnValue_t POOLOBJECT_NOT_FOUND = MAKE_RETURN_CODE(4); + static constexpr ReturnValue_t DATASET_NOT_FOUND = MAKE_RETURN_CODE(5); /** * This constructor is used by a class which wants to implement diff --git a/datapoollocal/LocalPoolVariable.tpp b/datapoollocal/LocalPoolVariable.tpp index d9d45d38..ef94b620 100644 --- a/datapoollocal/LocalPoolVariable.tpp +++ b/datapoollocal/LocalPoolVariable.tpp @@ -50,16 +50,16 @@ inline ReturnValue_t LocalPoolVariable::readWithoutLock() { } // Actually this should never happen.. - if(poolEntry->address == nullptr) { - result = PoolVariableIF::INVALID_POOL_ENTRY; - object_id_t ownerObjectId = hkManager->getOwner()->getObjectId(); - reportReadCommitError("LocalPoolVariable", result, - false, ownerObjectId, localPoolId); - return result; - } +// if(poolEntry->address == nullptr) { +// result = PoolVariableIF::INVALID_POOL_ENTRY; +// object_id_t ownerObjectId = hkManager->getOwner()->getObjectId(); +// reportReadCommitError("LocalPoolVariable", result, +// false, ownerObjectId, localPoolId); +// return result; +// } - this->value = *(poolEntry->address); - this->valid = poolEntry->valid; + this->value = *(poolEntry->getDataPtr()); + this->valid = poolEntry->getValid(); return RETURN_OK; } @@ -96,8 +96,8 @@ inline ReturnValue_t LocalPoolVariable::commitWithoutLock() { return result; } - *(poolEntry->address) = this->value; - poolEntry->valid = this->valid; + *(poolEntry->getDataPtr()) = this->value; + poolEntry->setValid(this->valid); return RETURN_OK; } diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index 94f3916e..a0e32f62 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -49,8 +49,8 @@ inline ReturnValue_t LocalPoolVector::readWithoutLock() { localPoolId); return result; } - std::memcpy(this->value, poolEntry->address, poolEntry->getByteSize()); - this->valid = poolEntry->valid; + std::memcpy(this->value, poolEntry->getDataPtr(), poolEntry->getByteSize()); + this->valid = poolEntry->getValid(); return RETURN_OK; } @@ -85,8 +85,8 @@ inline ReturnValue_t LocalPoolVector::commitWithoutLock() { localPoolId); return result; } - std::memcpy(poolEntry->address, this->value, poolEntry->getByteSize()); - poolEntry->valid = this->valid; + std::memcpy(poolEntry->getDataPtr(), this->value, poolEntry->getByteSize()); + poolEntry->setValid(this->valid); return RETURN_OK; } diff --git a/globalfunctions/arrayprinter.cpp b/globalfunctions/arrayprinter.cpp index c273b250..ebf21bb8 100644 --- a/globalfunctions/arrayprinter.cpp +++ b/globalfunctions/arrayprinter.cpp @@ -1,5 +1,5 @@ #include "arrayprinter.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include void arrayprinter::print(const uint8_t *data, size_t size, OutputType type, @@ -9,6 +9,8 @@ void arrayprinter::print(const uint8_t *data, size_t size, OutputType type, sif::info << "Printing data with size " << size << ": "; } sif::info << "["; +#else + fsfw::printInfo("Printing data with size %zu: [", size); #endif if(type == OutputType::HEX) { arrayprinter::printHex(data, size, maxCharPerLine); @@ -37,6 +39,8 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, } sif::info << std::dec; sif::info << "]" << std::endl; +#else + // how much memory to reserve for printout? #endif } @@ -54,6 +58,8 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, } } sif::info << "]" << std::endl; +#else + // how much memory to reserve for printout? #endif } @@ -65,5 +71,7 @@ void arrayprinter::printBin(const uint8_t *data, size_t size) { std::bitset<8>(data[i]) << ",\n" << std::flush; } sif::info << "]" << std::endl; +#else + // how much memory to reserve for printout? #endif } From f2ecd6d740763e55e682c5892b8ad3b0df74e627 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 15:10:33 +0100 Subject: [PATCH 11/19] revamed and imroved error handling --- datapoollocal/LocalDataPoolManager.cpp | 187 ++++++++++++++++--------- datapoollocal/LocalDataPoolManager.h | 11 +- 2 files changed, 131 insertions(+), 67 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index db5cd5dd..e5ee499c 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -3,6 +3,7 @@ #include "LocalPoolDataSetBase.h" #include "../housekeeping/HousekeepingPacketUpdate.h" +#include "../serviceinterface/ServiceInterface.h" #include "../housekeeping/HousekeepingSetPacket.h" #include "../housekeeping/AcceptsHkPacketsIF.h" @@ -21,19 +22,17 @@ LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, MessageQueueIF* queueToUse, bool appendValidityBuffer): appendValidityBuffer(appendValidityBuffer) { if(owner == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::LocalDataPoolManager: " - << "Invalid supplied owner!" << std::endl; -#endif + printWarningOrError(ErrorTypes::WARNING_TYPE, + "LocalDataPoolManager", HasReturnvaluesIF::RETURN_FAILED, + "Invalid supplied owner"); return; } this->owner = owner; mutex = MutexFactory::instance()->createMutex(); if(mutex == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::LocalDataPoolManager: " - << "Could not create mutex." << std::endl; -#endif + printWarningOrError(ErrorTypes::ERROR_TYPE, + "LocalDataPoolManager", HasReturnvaluesIF::RETURN_FAILED, + "Could not create mutex"); } hkQueue = queueToUse; @@ -43,21 +42,18 @@ LocalDataPoolManager::~LocalDataPoolManager() {} ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { if(queueToUse == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::initialize: " - << std::hex << "0x" << owner->getObjectId() << ". Supplied " - << "queue invalid!" << std::dec << std::endl; -#endif + // error, all destinations invalid + printWarningOrError(ErrorTypes::ERROR_TYPE, + "initialize", QUEUE_OR_DESTINATION_INVALID); } hkQueue = queueToUse; ipcStore = objectManager->get(objects::IPC_STORE); if(ipcStore == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::initialize: " - << std::hex << "0x" << owner->getObjectId() << ": Could not " - << "set IPC store." <getHkQueue(); } else { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::LocalDataPoolManager: " - << "Default HK destination object is invalid!" << std::endl; -#endif - return HasReturnvaluesIF::RETURN_FAILED; + printWarningOrError(ErrorTypes::ERROR_TYPE, + "initialize", QUEUE_OR_DESTINATION_INVALID); + return QUEUE_OR_DESTINATION_INVALID; } } @@ -95,10 +89,10 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { } return result; } -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager: The map should only be initialized " - << "once!" << std::endl; -#endif + + printWarningOrError(ErrorTypes::WARNING_TYPE, + "initialize", HasReturnvaluesIF::RETURN_FAILED, + "The map should only be initialized once"); return HasReturnvaluesIF::RETURN_OK; } @@ -163,7 +157,9 @@ ReturnValue_t LocalDataPoolManager::handleNotificationUpdate( LocalPoolObjectBase* poolObj = owner->getPoolObjectHandle( receiver.dataId.localPoolId); if(poolObj == nullptr) { - return HasReturnvaluesIF::RETURN_FAILED; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "handleNotificationUpdate", POOLOBJECT_NOT_FOUND); + return POOLOBJECT_NOT_FOUND; } if(poolObj->hasChanged()) { // prepare and send update notification. @@ -183,7 +179,9 @@ ReturnValue_t LocalDataPoolManager::handleNotificationUpdate( LocalPoolDataSetBase* dataSet = owner->getDataSetHandle( receiver.dataId.sid); if(dataSet == nullptr) { - return HasReturnvaluesIF::RETURN_FAILED; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "handleNotificationUpdate", DATASET_NOT_FOUND); + return DATASET_NOT_FOUND; } if(dataSet->hasChanged()) { // prepare and send update notification @@ -213,6 +211,8 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolObjectBase* poolObj = owner->getPoolObjectHandle( receiver.dataId.localPoolId); if(poolObj == nullptr) { + printWarningOrError(ErrorTypes::WARNING_TYPE, + "handleNotificationSnapshot", POOLOBJECT_NOT_FOUND); return POOLOBJECT_NOT_FOUND; } @@ -249,6 +249,8 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolDataSetBase* dataSet = owner->getDataSetHandle( receiver.dataId.sid); if(dataSet == nullptr) { + printWarningOrError(ErrorTypes::WARNING_TYPE, + "handleNotificationSnapshot", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; } @@ -351,11 +353,9 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, AcceptsHkPacketsIF* hkReceiverObject = objectManager->get(packetDestination); if(hkReceiverObject == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::subscribeForPeriodicPacket:" - << " Invalid receiver!"<< std::endl; -#endif - return HasReturnvaluesIF::RETURN_OK; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "subscribeForPeriodicPacket", QUEUE_OR_DESTINATION_INVALID); + return QUEUE_OR_DESTINATION_INVALID; } struct HkReceiver hkReceiver; @@ -383,11 +383,9 @@ ReturnValue_t LocalDataPoolManager::subscribeForUpdatePackets(sid_t sid, AcceptsHkPacketsIF* hkReceiverObject = objectManager->get(packetDestination); if(hkReceiverObject == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::subscribeForPeriodicPacket:" - << " Invalid receiver!"<< std::endl; -#endif - return HasReturnvaluesIF::RETURN_OK; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "subscribeForPeriodicPacket", QUEUE_OR_DESTINATION_INVALID); + return QUEUE_OR_DESTINATION_INVALID; } struct HkReceiver hkReceiver; @@ -591,10 +589,8 @@ ReturnValue_t LocalDataPoolManager::printPoolEntry( lp_id_t localPoolId) { auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "HousekeepingManager::fechPoolEntry:" - << " Pool entry not found." << std::endl; -#endif + printWarningOrError(ErrorTypes::WARNING_TYPE, "printPoolEntry", + HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND); return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } poolIter->second->print(); @@ -614,10 +610,9 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, MessageQueueId_t destination) { if(dataSet == nullptr) { // Configuration error. -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager::generateHousekeepingPacket:" - << " Set ID not found or dataset not assigned!" << std::endl; -#endif + printWarningOrError(ErrorTypes::WARNING_TYPE, + "generateHousekeepingPacket", + DATASET_NOT_FOUND); return DATASET_NOT_FOUND; } @@ -640,12 +635,18 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, } if(hkQueue == nullptr) { - return QUEUE_OR_DESTINATION_NOT_SET; + // error, all destinations invalid + printWarningOrError(ErrorTypes::WARNING_TYPE, + "generateHousekeepingPacket", + QUEUE_OR_DESTINATION_INVALID); + return QUEUE_OR_DESTINATION_INVALID; } if(destination == MessageQueueIF::NO_QUEUE) { if(hkDestinationId == MessageQueueIF::NO_QUEUE) { // error, all destinations invalid - return HasReturnvaluesIF::RETURN_FAILED; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "generateHousekeepingPacket", + QUEUE_OR_DESTINATION_INVALID); } destination = hkDestinationId; } @@ -681,6 +682,13 @@ void LocalDataPoolManager::setNonDiagnosticIntervalFactor( void LocalDataPoolManager::performPeriodicHkGeneration(HkReceiver& receiver) { sid_t sid = receiver.dataId.sid; LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); + if(dataSet == nullptr) { + printWarningOrError(ErrorTypes::WARNING_TYPE, + "performPeriodicHkGeneration", + DATASET_NOT_FOUND); + return; + } + if(not dataSet->getReportingEnabled()) { return; } @@ -699,10 +707,11 @@ void LocalDataPoolManager::performPeriodicHkGeneration(HkReceiver& receiver) { if(result != HasReturnvaluesIF::RETURN_OK) { // configuration error #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "LocalDataPoolManager::performHkOperation:" - << "0x" << std::hex << std::setfill('0') << std::setw(8) - << owner->getObjectId() << " Error generating " - << "HK packet" << std::setfill(' ') << std::dec << std::endl; + sif::warning << "LocalDataPoolManager::performHkOperation: " + << "HK generation failed." << std::endl; +#else + fsfw::printWarning("LocalDataPoolManager::performHkOperation: " + "HK generation failed.\n"); #endif } } @@ -748,11 +757,10 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, // Get and check dataset first. LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); if(dataSet == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager::generateHousekeepingPacket:" - << " Set ID not found" << std::endl; -#endif - return HasReturnvaluesIF::RETURN_FAILED; + printWarningOrError(ErrorTypes::WARNING_TYPE, + "performPeriodicHkGeneration", + DATASET_NOT_FOUND); + return DATASET_NOT_FOUND; } @@ -776,10 +784,9 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, ReturnValue_t result = ipcStore->getFreeElement(&storeId, expectedSize,&storePtr); if(result != HasReturnvaluesIF::RETURN_OK) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "HousekeepingManager::generateHousekeepingPacket: " - << "Could not get free element from IPC store." << std::endl; -#endif + printWarningOrError(ErrorTypes::ERROR_TYPE, + "generateSetStructurePacket", HasReturnvaluesIF::RETURN_FAILED, + "Could not get free element from IPC store."); return result; } @@ -788,10 +795,9 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, result = setPacket.serialize(&storePtr, &size, expectedSize, SerializeIF::Endianness::BIG); if(expectedSize != size) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "HousekeepingManager::generateSetStructurePacket: " - << "Expected size is not equal to serialized size" << std::endl; -#endif + printWarningOrError(ErrorTypes::WARNING_TYPE, + "generateSetStructurePacket", HasReturnvaluesIF::RETURN_FAILED, + "Expected size is not equal to serialized size"); } // Send structure reporting reply. @@ -808,3 +814,52 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, hkQueue->reply(&reply); return result; } + +void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, + const char* functionName, ReturnValue_t error, const char* errorPrint) { + if(errorPrint == nullptr) { + if(error == DATASET_NOT_FOUND) { + errorPrint = "Dataset not found"; + } + else if(error == POOLOBJECT_NOT_FOUND) { + errorPrint = "Pool Object not found"; + } + else if(error == HasReturnvaluesIF::RETURN_FAILED) { + if(errorType == ErrorTypes::WARNING_TYPE) { + errorPrint = "Generic Warning"; + } + else { + errorPrint = "Generic error"; + } + } + else if(error == QUEUE_OR_DESTINATION_INVALID) { + errorPrint = "Queue or destination not set"; + } + else { + errorPrint = "Unknown error"; + } + } + + if(errorType == ErrorTypes::WARNING_TYPE) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "LocalDataPoolManager::" << functionName + << ": Object ID " << std::setw(8) << std::setfill('0') + << std::hex << owner->getObjectId() << " | " << errorPrint + << std::dec << std::setfill(' ') << std::endl; +#else + fsfw::printWarning("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", + owner->getObjectId(), errorPrint); +#endif + } + else if(errorType == ErrorTypes::ERROR_TYPE) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::error << "LocalDataPoolManager::" << functionName + << ": Object ID " << std::setw(8) << std::setfill('0') + << std::hex << owner->getObjectId() << " | " << errorPrint + << std::dec << std::setfill(' ') << std::endl; +#else + fsfw::printError("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", + owner->getObjectId(), errorPrint); +#endif + } +} diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 098158da..6502294e 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -56,7 +56,7 @@ class LocalDataPoolManager { public: static constexpr uint8_t INTERFACE_ID = CLASS_ID::HOUSEKEEPING_MANAGER; - static constexpr ReturnValue_t QUEUE_OR_DESTINATION_NOT_SET = MAKE_RETURN_CODE(0); + static constexpr ReturnValue_t QUEUE_OR_DESTINATION_INVALID = MAKE_RETURN_CODE(0); static constexpr ReturnValue_t WRONG_HK_PACKET_TYPE = MAKE_RETURN_CODE(1); static constexpr ReturnValue_t REPORTING_STATUS_UNCHANGED = MAKE_RETURN_CODE(2); @@ -370,6 +370,15 @@ private: ReturnValue_t& status); ReturnValue_t addUpdateToStore(HousekeepingPacketUpdate& updatePacket, store_address_t& storeId); + + enum class ErrorTypes { + WARNING_TYPE, + ERROR_TYPE + }; + + void printWarningOrError(ErrorTypes errorType, const char* functionName, + ReturnValue_t errorCode = HasReturnvaluesIF::RETURN_FAILED, + const char* errorPrint = nullptr); }; From 541478e4d5f4ad029dd4fd968e10ccc6cc72068d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 15:15:02 +0100 Subject: [PATCH 12/19] another small improvement --- datapoollocal/LocalDataPoolManager.cpp | 8 ++++++-- datapoollocal/LocalDataPoolManager.h | 18 ++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index e5ee499c..d1ab553d 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -3,10 +3,8 @@ #include "LocalPoolDataSetBase.h" #include "../housekeeping/HousekeepingPacketUpdate.h" -#include "../serviceinterface/ServiceInterface.h" #include "../housekeeping/HousekeepingSetPacket.h" #include "../housekeeping/AcceptsHkPacketsIF.h" - #include "../timemanager/CCSDSTime.h" #include "../ipc/MutexFactory.h" #include "../ipc/MutexHelper.h" @@ -835,6 +833,12 @@ void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, else if(error == QUEUE_OR_DESTINATION_INVALID) { errorPrint = "Queue or destination not set"; } + else if(error == HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT) { + errorPrint = "Pool entry type conflict"; + } + else if(error == HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND) { + errorPrint = "Pool entry not found"; + } else { errorPrint = "Unknown error"; } diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 6502294e..730e03e4 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -387,25 +387,15 @@ ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, PoolEntry **poolEntry) { auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager::fetchPoolEntry: Pool entry " - "not found." << std::endl; -#else - fsfw::printWarning("HousekeepingManager::fetchPoolEntry: Pool entry " - "not found.\n"); -#endif + printWarningOrError(ErrorTypes::ERROR_TYPE, "fetchPoolEntry", + HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND); return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } *poolEntry = dynamic_cast< PoolEntry* >(poolIter->second); if(*poolEntry == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "HousekeepingManager::fetchPoolEntry:" - " Pool entry type conflict." << std::endl; -#else - fsfw::printWarning("HousekeepingManager::fetchPoolEntry:" - " Pool entry type conflict.\n"); -#endif + printWarningOrError(ErrorTypes::ERROR_TYPE, "fetchPoolEntry", + HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT); return HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT; } return HasReturnvaluesIF::RETURN_OK; From c0fd981360dbb7d16018e9803a2a9a0f97de32f9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 16:14:11 +0100 Subject: [PATCH 13/19] improved DHB error handling --- datapoollocal/LocalDataPoolManager.cpp | 48 +++--- datapoollocal/LocalDataPoolManager.h | 12 +- devicehandlers/DeviceHandlerBase.cpp | 143 ++++++++++++------ devicehandlers/DeviceHandlerBase.h | 20 ++- serviceinterface/ServiceInterface.h | 1 + serviceinterface/ServiceInterfacePrinter.h | 8 +- serviceinterface/serviceInterfaceDefintions.h | 7 + 7 files changed, 152 insertions(+), 87 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index d1ab553d..0ab6f150 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -20,7 +20,7 @@ LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, MessageQueueIF* queueToUse, bool appendValidityBuffer): appendValidityBuffer(appendValidityBuffer) { if(owner == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "LocalDataPoolManager", HasReturnvaluesIF::RETURN_FAILED, "Invalid supplied owner"); return; @@ -28,7 +28,7 @@ LocalDataPoolManager::LocalDataPoolManager(HasLocalDataPoolIF* owner, this->owner = owner; mutex = MutexFactory::instance()->createMutex(); if(mutex == nullptr) { - printWarningOrError(ErrorTypes::ERROR_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "LocalDataPoolManager", HasReturnvaluesIF::RETURN_FAILED, "Could not create mutex"); } @@ -41,7 +41,7 @@ LocalDataPoolManager::~LocalDataPoolManager() {} ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { if(queueToUse == nullptr) { // error, all destinations invalid - printWarningOrError(ErrorTypes::ERROR_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", QUEUE_OR_DESTINATION_INVALID); } hkQueue = queueToUse; @@ -49,7 +49,7 @@ ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { ipcStore = objectManager->get(objects::IPC_STORE); if(ipcStore == nullptr) { // error, all destinations invalid - printWarningOrError(ErrorTypes::ERROR_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", HasReturnvaluesIF::RETURN_FAILED, "Could not set IPC store."); return HasReturnvaluesIF::RETURN_FAILED; @@ -63,7 +63,7 @@ ReturnValue_t LocalDataPoolManager::initialize(MessageQueueIF* queueToUse) { hkDestinationId = hkPacketReceiver->getHkQueue(); } else { - printWarningOrError(ErrorTypes::ERROR_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", QUEUE_OR_DESTINATION_INVALID); return QUEUE_OR_DESTINATION_INVALID; } @@ -88,7 +88,7 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { return result; } - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "initialize", HasReturnvaluesIF::RETURN_FAILED, "The map should only be initialized once"); return HasReturnvaluesIF::RETURN_OK; @@ -155,7 +155,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationUpdate( LocalPoolObjectBase* poolObj = owner->getPoolObjectHandle( receiver.dataId.localPoolId); if(poolObj == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "handleNotificationUpdate", POOLOBJECT_NOT_FOUND); return POOLOBJECT_NOT_FOUND; } @@ -177,7 +177,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationUpdate( LocalPoolDataSetBase* dataSet = owner->getDataSetHandle( receiver.dataId.sid); if(dataSet == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "handleNotificationUpdate", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; } @@ -209,7 +209,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolObjectBase* poolObj = owner->getPoolObjectHandle( receiver.dataId.localPoolId); if(poolObj == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "handleNotificationSnapshot", POOLOBJECT_NOT_FOUND); return POOLOBJECT_NOT_FOUND; } @@ -247,7 +247,7 @@ ReturnValue_t LocalDataPoolManager::handleNotificationSnapshot( LocalPoolDataSetBase* dataSet = owner->getDataSetHandle( receiver.dataId.sid); if(dataSet == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "handleNotificationSnapshot", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; } @@ -351,7 +351,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForPeriodicPacket(sid_t sid, AcceptsHkPacketsIF* hkReceiverObject = objectManager->get(packetDestination); if(hkReceiverObject == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "subscribeForPeriodicPacket", QUEUE_OR_DESTINATION_INVALID); return QUEUE_OR_DESTINATION_INVALID; } @@ -381,7 +381,7 @@ ReturnValue_t LocalDataPoolManager::subscribeForUpdatePackets(sid_t sid, AcceptsHkPacketsIF* hkReceiverObject = objectManager->get(packetDestination); if(hkReceiverObject == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "subscribeForPeriodicPacket", QUEUE_OR_DESTINATION_INVALID); return QUEUE_OR_DESTINATION_INVALID; } @@ -587,7 +587,7 @@ ReturnValue_t LocalDataPoolManager::printPoolEntry( lp_id_t localPoolId) { auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { - printWarningOrError(ErrorTypes::WARNING_TYPE, "printPoolEntry", + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "printPoolEntry", HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND); return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } @@ -608,7 +608,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, MessageQueueId_t destination) { if(dataSet == nullptr) { // Configuration error. - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; @@ -634,7 +634,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, if(hkQueue == nullptr) { // error, all destinations invalid - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", QUEUE_OR_DESTINATION_INVALID); return QUEUE_OR_DESTINATION_INVALID; @@ -642,7 +642,7 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid, if(destination == MessageQueueIF::NO_QUEUE) { if(hkDestinationId == MessageQueueIF::NO_QUEUE) { // error, all destinations invalid - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "generateHousekeepingPacket", QUEUE_OR_DESTINATION_INVALID); } @@ -681,7 +681,7 @@ void LocalDataPoolManager::performPeriodicHkGeneration(HkReceiver& receiver) { sid_t sid = receiver.dataId.sid; LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); if(dataSet == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "performPeriodicHkGeneration", DATASET_NOT_FOUND); return; @@ -755,7 +755,7 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, // Get and check dataset first. LocalPoolDataSetBase* dataSet = owner->getDataSetHandle(sid); if(dataSet == nullptr) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "performPeriodicHkGeneration", DATASET_NOT_FOUND); return DATASET_NOT_FOUND; @@ -782,7 +782,7 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, ReturnValue_t result = ipcStore->getFreeElement(&storeId, expectedSize,&storePtr); if(result != HasReturnvaluesIF::RETURN_OK) { - printWarningOrError(ErrorTypes::ERROR_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "generateSetStructurePacket", HasReturnvaluesIF::RETURN_FAILED, "Could not get free element from IPC store."); return result; @@ -793,7 +793,7 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, result = setPacket.serialize(&storePtr, &size, expectedSize, SerializeIF::Endianness::BIG); if(expectedSize != size) { - printWarningOrError(ErrorTypes::WARNING_TYPE, + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "generateSetStructurePacket", HasReturnvaluesIF::RETURN_FAILED, "Expected size is not equal to serialized size"); } @@ -813,7 +813,7 @@ ReturnValue_t LocalDataPoolManager::generateSetStructurePacket(sid_t sid, return result; } -void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, +void LocalDataPoolManager::printWarningOrError(fsfw::OutputTypes outputType, const char* functionName, ReturnValue_t error, const char* errorPrint) { if(errorPrint == nullptr) { if(error == DATASET_NOT_FOUND) { @@ -823,7 +823,7 @@ void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, errorPrint = "Pool Object not found"; } else if(error == HasReturnvaluesIF::RETURN_FAILED) { - if(errorType == ErrorTypes::WARNING_TYPE) { + if(outputType == fsfw::OutputTypes::OUT_WARNING) { errorPrint = "Generic Warning"; } else { @@ -844,7 +844,7 @@ void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, } } - if(errorType == ErrorTypes::WARNING_TYPE) { + if(outputType == fsfw::OutputTypes::OUT_WARNING) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "LocalDataPoolManager::" << functionName << ": Object ID " << std::setw(8) << std::setfill('0') @@ -855,7 +855,7 @@ void LocalDataPoolManager::printWarningOrError(ErrorTypes errorType, owner->getObjectId(), errorPrint); #endif } - else if(errorType == ErrorTypes::ERROR_TYPE) { + else if(outputType == fsfw::OutputTypes::OUT_ERROR) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "LocalDataPoolManager::" << functionName << ": Object ID " << std::setw(8) << std::setfill('0') diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 730e03e4..eee4c593 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -371,12 +371,8 @@ private: ReturnValue_t addUpdateToStore(HousekeepingPacketUpdate& updatePacket, store_address_t& storeId); - enum class ErrorTypes { - WARNING_TYPE, - ERROR_TYPE - }; - - void printWarningOrError(ErrorTypes errorType, const char* functionName, + void printWarningOrError(fsfw::OutputTypes outputType, + const char* functionName, ReturnValue_t errorCode = HasReturnvaluesIF::RETURN_FAILED, const char* errorPrint = nullptr); }; @@ -387,14 +383,14 @@ ReturnValue_t LocalDataPoolManager::fetchPoolEntry(lp_id_t localPoolId, PoolEntry **poolEntry) { auto poolIter = localPoolMap.find(localPoolId); if (poolIter == localPoolMap.end()) { - printWarningOrError(ErrorTypes::ERROR_TYPE, "fetchPoolEntry", + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "fetchPoolEntry", HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND); return HasLocalDataPoolIF::POOL_ENTRY_NOT_FOUND; } *poolEntry = dynamic_cast< PoolEntry* >(poolIter->second); if(*poolEntry == nullptr) { - printWarningOrError(ErrorTypes::ERROR_TYPE, "fetchPoolEntry", + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "fetchPoolEntry", HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT); return HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT; } diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 580f8c0d..7ca9922a 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -39,13 +39,8 @@ DeviceHandlerBase::DeviceHandlerBase(object_id_t setObjectId, cookieInfo.state = COOKIE_UNUSED; cookieInfo.pendingCommand = deviceCommandMap.end(); if (comCookie == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase: ObjectID 0x" << std::hex - << std::setw(8) << std::setfill('0') << this->getObjectId() - << std::dec << ": Do not pass nullptr as a cookie, consider " - << std::setfill(' ') << "passing a dummy cookie instead!" - << std::endl; -#endif + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "DeviceHandlerBase", + HasReturnvaluesIF::RETURN_FAILED, "Invalid cookie"); } if (this->fdirInstance == nullptr) { this->fdirInstance = new DeviceHandlerFailureIsolation(setObjectId, @@ -132,30 +127,24 @@ ReturnValue_t DeviceHandlerBase::initialize() { communicationInterface = objectManager->get( deviceCommunicationId); if (communicationInterface == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::initialize: Communication interface " - "invalid." << std::endl; - sif::error << "Make sure it is set up properly and implements" - " DeviceCommunicationIF" << std::endl; -#endif + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", + ObjectManagerIF::CHILD_INIT_FAILED, + "Passed communication IF invalid"); return ObjectManagerIF::CHILD_INIT_FAILED; } result = communicationInterface->initializeInterface(comCookie); if (result != RETURN_OK) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::initialize: Initializing " - "communication interface failed!" << std::endl; -#endif + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", + ObjectManagerIF::CHILD_INIT_FAILED, + "ComIF initialization failed"); return result; } IPCStore = objectManager->get(objects::IPC_STORE); if (IPCStore == nullptr) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::initialize: IPC store not set up in " - "factory." << std::endl; -#endif + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, "initialize", + ObjectManagerIF::CHILD_INIT_FAILED, "IPC Store not set up"); return ObjectManagerIF::CHILD_INIT_FAILED; } @@ -164,11 +153,15 @@ ReturnValue_t DeviceHandlerBase::initialize() { AcceptsDeviceResponsesIF>(rawDataReceiverId); if (rawReceiver == nullptr) { + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, + "initialize", ObjectManagerIF::CHILD_INIT_FAILED, + "Raw receiver object ID set but no valid object found."); #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::initialize: Raw receiver object " - "ID set but no valid object found." << std::endl; sif::error << "Make sure the raw receiver object is set up properly" " and implements AcceptsDeviceResponsesIF" << std::endl; +#else + fsfw::printError("Make sure the raw receiver object is set up " + "properly and implements AcceptsDeviceResponsesIF\n"); #endif return ObjectManagerIF::CHILD_INIT_FAILED; } @@ -178,11 +171,15 @@ ReturnValue_t DeviceHandlerBase::initialize() { if(powerSwitcherId != objects::NO_OBJECT) { powerSwitcher = objectManager->get(powerSwitcherId); if (powerSwitcher == nullptr) { + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, + "initialize", ObjectManagerIF::CHILD_INIT_FAILED, + "Power switcher set but no valid object found."); #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::initialize: Power switcher " - << "object ID set but no valid object found." << std::endl; - sif::error << "Make sure the raw receiver object is set up properly" - << " and implements PowerSwitchIF" << std::endl; + sif::error << "Make sure the power switcher object is set up " + << "properly and implements PowerSwitchIF" << std::endl; +#else + fsfw::printError("Make sure the power switcher object is set up " + "properly and implements PowerSwitchIF\n"); #endif return ObjectManagerIF::CHILD_INIT_FAILED; } @@ -556,17 +553,17 @@ void DeviceHandlerBase::replyReturnvalueToCommand(ReturnValue_t status, void DeviceHandlerBase::replyToCommand(ReturnValue_t status, uint32_t parameter) { -//Check if we reply to a raw command. + // Check if we reply to a raw command. if (cookieInfo.pendingCommand->first == RAW_COMMAND_ID) { if (status == NO_REPLY_EXPECTED) { status = RETURN_OK; } replyReturnvalueToCommand(status, parameter); - //Always delete data from a raw command. + // Always delete data from a raw command. IPCStore->deleteData(storedRawData); return; } -//Check if we were externally commanded. + // Check if we were externally commanded. if (cookieInfo.pendingCommand->second.sendReplyTo != NO_COMMANDER) { MessageQueueId_t queueId = cookieInfo.pendingCommand->second.sendReplyTo; if (status == NO_REPLY_EXPECTED) { @@ -581,15 +578,17 @@ void DeviceHandlerBase::replyToCommand(ReturnValue_t status, void DeviceHandlerBase::replyToReply(DeviceReplyMap::iterator iter, ReturnValue_t status) { -//No need to check if iter exists, as this is checked by callers. If someone else uses the method, add check. + // No need to check if iter exists, as this is checked by callers. + // If someone else uses the method, add check. if (iter->second.command == deviceCommandMap.end()) { //Is most likely periodic reply. Silent return. return; } -//Check if more replies are expected. If so, do nothing. + // Check if more replies are expected. If so, do nothing. DeviceCommandInfo* info = &(iter->second.command->second); if (--info->expectedReplies == 0) { - //Check if it was transition or internal command. Don't send any replies in that case. + // Check if it was transition or internal command. + // Don't send any replies in that case. if (info->sendReplyTo != NO_COMMANDER) { actionHelper.finish(info->sendReplyTo, iter->first, status); } @@ -606,7 +605,7 @@ void DeviceHandlerBase::doSendWrite() { if (result == RETURN_OK) { cookieInfo.state = COOKIE_WRITE_SENT; } else { - //always generate a failure event, so that FDIR knows what's up + // always generate a failure event, so that FDIR knows what's up triggerEvent(DEVICE_SENDING_COMMAND_FAILED, result, cookieInfo.pendingCommand->first); replyToCommand(result); @@ -735,6 +734,9 @@ void DeviceHandlerBase::parseReply(const uint8_t* receivedData, foundId); } if(foundLen == 0) { + printWarningOrError(fsfw::OutputTypes::OUT_ERROR, + "parseReply", ObjectManagerIF::CHILD_INIT_FAILED, + "Power switcher set but no valid object found."); #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "DeviceHandlerBase::parseReply: foundLen is 0!" " Packet parsing will be stuck." << std::endl; @@ -968,7 +970,8 @@ ReturnValue_t DeviceHandlerBase::getStateOfSwitches(void) { } Mode_t DeviceHandlerBase::getBaseMode(Mode_t transitionMode) { -//only child action special modes are handled, as a child should never see any base action modes + // only child action special modes are handled, as a child should + // never see any base action modes if (transitionMode == _MODE_START_UP) { return _MODE_TO_ON; } @@ -1291,12 +1294,11 @@ void DeviceHandlerBase::buildInternalCommand(void) { if (mode == MODE_NORMAL) { result = buildNormalDeviceCommand(&deviceCommandId); if (result == BUSY) { - //so we can track misconfigurations -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << std::hex << getObjectId() - << ": DHB::buildInternalCommand: Busy" << std::dec - << std::endl; -#endif + // so we can track misconfigurations + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, + "buildInternalCommand", + HasReturnvaluesIF::RETURN_FAILED, + "Busy."); result = NOTHING_TO_SEND; //no need to report this } } @@ -1320,13 +1322,13 @@ void DeviceHandlerBase::buildInternalCommand(void) { if (iter == deviceCommandMap.end()) { result = COMMAND_NOT_SUPPORTED; } else if (iter->second.isExecuting) { - //so we can track misconfigurations -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << std::hex << getObjectId() - << ": DHB::buildInternalCommand: Command " - << deviceCommandId << " isExecuting" << std::dec - << std::endl; -#endif + char* output = nullptr; + sprintf(output, "Command %lu is executing", deviceCommandId); + // so we can track misconfigurations + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, + "buildInternalCommand", + HasReturnvaluesIF::RETURN_FAILED, + output); // this is an internal command, no need to report a failure here, // missed reply will track if a reply is too late, otherwise, it's ok return; @@ -1485,3 +1487,48 @@ void DeviceHandlerBase::setNormalDatapoolEntriesInvalid() { } } } + +void DeviceHandlerBase::printWarningOrError(fsfw::OutputTypes errorType, + const char *functionName, ReturnValue_t errorCode, + const char *errorPrint) { + if(errorPrint == nullptr) { + if(errorCode == ObjectManagerIF::CHILD_INIT_FAILED) { + errorPrint = "Initialization error"; + } + if(errorCode == HasReturnvaluesIF::RETURN_FAILED) { + if(errorType == fsfw::OutputTypes::OUT_WARNING) { + errorPrint = "Generic Warning"; + } + else { + errorPrint = "Generic Error"; + } + } + else { + errorPrint = "Unknown error"; + } + } + + if(errorType == fsfw::OutputTypes::OUT_WARNING) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "DeviceHandlerBase::" << functionName << ": Object ID " + << std::hex << std::setw(8) << std::setfill('0') + << this->getObjectId() << " | " << errorPrint << std::dec + << std::setfill(' ') << std::endl; +#else + fsfw::printWarning("DeviceHandlerBase::%s: Object ID 0x%08x | %s\n", + this->getObjectId(), errorPrint); +#endif + } + else if(errorType == fsfw::OutputTypes::OUT_ERROR) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::error << "DeviceHandlerBase::" << functionName << ": Object ID " + << std::hex << std::setw(8) << std::setfill('0') + << this->getObjectId() << " | " << errorPrint << std::dec + << std::setfill(' ') << std::endl; +#else + fsfw::printError("DeviceHandlerBase::%s: Object ID 0x%08x | %s\n", + this->getObjectId(), errorPrint); +#endif + } + +} diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index 24ba0372..4c745b4f 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -6,6 +6,8 @@ #include "DeviceHandlerFailureIsolation.h" #include "DeviceHandlerThermalSet.h" +#include "../serviceinterface/ServiceInterface.h" +#include "../serviceinterface/serviceInterfaceDefintions.h" #include "../objectmanager/SystemObject.h" #include "../tasks/ExecutableObjectIF.h" #include "../returnvalues/HasReturnvaluesIF.h" @@ -1111,7 +1113,7 @@ private: /** * @brief The mode the current transition originated from * - * This is private so the child can not change it and fuck up the timeouts + * This is private so the child can not change it and mess up the timeouts * * IMPORTANT: This is not valid during _MODE_SHUT_DOWN and _MODE_START_UP!! * (it is _MODE_POWER_DOWN during this modes) @@ -1190,7 +1192,8 @@ private: * Check if the RMAP sendWrite action was successful. * * Depending on the result, the following is done - * - if the device command was external commanded, a reply is sent indicating the result + * - if the device command was external commanded, a reply is sent + * indicating the result * - if the action was successful, the reply timout counter is initialized */ void doGetWrite(void); @@ -1206,9 +1209,9 @@ private: /** * Check the getRead reply and the contained data. * - * If data was received scanForReply() and, if successful, handleReply() are called. - * If the current mode is @c MODE_RAW, the received packet is sent to the commanding object - * via commandQueue. + * If data was received scanForReply() and, if successful, handleReply() + * are called. If the current mode is @c MODE_RAW, the received packet + * is sent to the commanding object via commandQueue. */ void doGetRead(void); @@ -1227,7 +1230,7 @@ private: uint32_t *len); /** - * @param modeTo either @c MODE_ON, MODE_NORMAL or MODE_RAW NOTHING ELSE!!! + * @param modeTo either @c MODE_ON, MODE_NORMAL or MODE_RAW, nothing else! */ void setTransition(Mode_t modeTo, Submode_t submodeTo); @@ -1247,6 +1250,11 @@ private: void handleTransitionToOnMode(Mode_t commandedMode, Submode_t commandedSubmode); + + void printWarningOrError(fsfw::OutputTypes errorType, + const char* functionName, + ReturnValue_t errorCode = HasReturnvaluesIF::RETURN_FAILED, + const char* errorPrint = nullptr); }; #endif /* FSFW_DEVICEHANDLERS_DEVICEHANDLERBASE_H_ */ diff --git a/serviceinterface/ServiceInterface.h b/serviceinterface/ServiceInterface.h index 0c7c4383..1f7e5e84 100644 --- a/serviceinterface/ServiceInterface.h +++ b/serviceinterface/ServiceInterface.h @@ -2,6 +2,7 @@ #define FSFW_SERVICEINTERFACE_SERVICEINTERFACE_H_ #include +#include "serviceInterfaceDefintions.h" #if FSFW_CPP_OSTREAM_ENABLED == 1 #include diff --git a/serviceinterface/ServiceInterfacePrinter.h b/serviceinterface/ServiceInterfacePrinter.h index 1f82ba98..9a84f629 100644 --- a/serviceinterface/ServiceInterfacePrinter.h +++ b/serviceinterface/ServiceInterfacePrinter.h @@ -4,7 +4,7 @@ namespace fsfw { -enum class PrintLevel { +enum PrintLevel { NONE = 0, //! Strange error when using just ERROR.. ERROR_TYPE = 1, @@ -13,6 +13,12 @@ enum class PrintLevel { DEBUG = 4 }; +/** + * Set the print level. All print types with a smaller level will be printed + * as well. For example, set to PrintLevel::WARNING to only enable error + * and warning output. + * @param printLevel + */ void setPrintLevel(PrintLevel printLevel); PrintLevel getPrintLevel(); diff --git a/serviceinterface/serviceInterfaceDefintions.h b/serviceinterface/serviceInterfaceDefintions.h index 684c7366..de445907 100644 --- a/serviceinterface/serviceInterfaceDefintions.h +++ b/serviceinterface/serviceInterfaceDefintions.h @@ -3,6 +3,13 @@ namespace fsfw { +enum class OutputTypes { + OUT_INFO, + OUT_DEBUG, + OUT_WARNING, + OUT_ERROR +}; + static const char* const ANSI_COLOR_RED = "\x1b[31m"; static const char* const ANSI_COLOR_GREEN = "\x1b[32m"; static const char* const ANSI_COLOR_YELLOW = "\x1b[33m"; From a9dba82661f6a88eeaa3fd1aab0948bf1919158c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 16:16:17 +0100 Subject: [PATCH 14/19] bugfix --- devicehandlers/DeviceHandlerBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 7ca9922a..1a106844 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1322,8 +1322,8 @@ void DeviceHandlerBase::buildInternalCommand(void) { if (iter == deviceCommandMap.end()) { result = COMMAND_NOT_SUPPORTED; } else if (iter->second.isExecuting) { - char* output = nullptr; - sprintf(output, "Command %lu is executing", deviceCommandId); + char output[36]; + sprintf(output, "Command 0x%08x is executing", deviceCommandId); // so we can track misconfigurations printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "buildInternalCommand", From a1c394143a1b6ba4c80abcb54302ca1020053f69 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 16:18:08 +0100 Subject: [PATCH 15/19] small formatting stuff --- devicehandlers/DeviceHandlerBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 1a106844..835a02b8 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -749,7 +749,8 @@ void DeviceHandlerBase::parseReply(const uint8_t* receivedData, case IGNORE_FULL_PACKET: return; default: - //We need to wait for timeout.. don't know what command failed and who sent it. + // We need to wait for timeout.. don't know what command failed + // and who sent it. replyRawReplyIfnotWiretapped(receivedData, foundLen); triggerEvent(DEVICE_READING_REPLY_FAILED, result, foundLen); break; From 9f15cd697d154d0d464560ce2c26cedee9c87388 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 16:23:57 +0100 Subject: [PATCH 16/19] improved includes --- devicehandlers/DeviceHandlerBase.cpp | 14 ++++++-------- serviceinterface/ServiceInterfacePrinter.h | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 835a02b8..0efdeba0 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -2,7 +2,7 @@ #include "AcceptsDeviceResponsesIF.h" #include "DeviceTmReportingWrapper.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include "../objectmanager/ObjectManager.h" #include "../storagemanager/StorageManagerIF.h" #include "../thermal/ThermalComponentIF.h" @@ -13,9 +13,6 @@ #include "../subsystem/SubsystemBase.h" #include "../datapoollocal/LocalPoolVariable.h" -#include - - object_id_t DeviceHandlerBase::powerSwitcherId = objects::NO_OBJECT; object_id_t DeviceHandlerBase::rawDataReceiverId = objects::NO_OBJECT; object_id_t DeviceHandlerBase::defaultFdirParentId = objects::NO_OBJECT; @@ -720,10 +717,9 @@ void DeviceHandlerBase::parseReply(const uint8_t* receivedData, case RETURN_OK: handleReply(receivedData, foundId, foundLen); if(foundLen == 0) { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "DeviceHandlerBase::parseReply: foundLen is 0!" - " Packet parsing will be stuck." << std::endl; -#endif + printWarningOrError(fsfw::OutputTypes::OUT_WARNING, + "parseReply", ObjectManagerIF::CHILD_INIT_FAILED, + "Found length is one, parsing might be stuck"); } break; case APERIODIC_REPLY: { @@ -1323,6 +1319,7 @@ void DeviceHandlerBase::buildInternalCommand(void) { if (iter == deviceCommandMap.end()) { result = COMMAND_NOT_SUPPORTED; } else if (iter->second.isExecuting) { +#if FSFW_DISABLE_PRINTOUT == 0 char output[36]; sprintf(output, "Command 0x%08x is executing", deviceCommandId); // so we can track misconfigurations @@ -1330,6 +1327,7 @@ void DeviceHandlerBase::buildInternalCommand(void) { "buildInternalCommand", HasReturnvaluesIF::RETURN_FAILED, output); +#endif // this is an internal command, no need to report a failure here, // missed reply will track if a reply is too late, otherwise, it's ok return; diff --git a/serviceinterface/ServiceInterfacePrinter.h b/serviceinterface/ServiceInterfacePrinter.h index 9a84f629..fdff797a 100644 --- a/serviceinterface/ServiceInterfacePrinter.h +++ b/serviceinterface/ServiceInterfacePrinter.h @@ -1,5 +1,5 @@ #if FSFW_DISABLE_PRINTOUT == 0 -#include +#include #endif namespace fsfw { From 7c47b1ce345182c6a69ff2fcaf6e89d81ef8c867 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 20:44:05 +0100 Subject: [PATCH 17/19] 100 percent test coverage for pool vector --- datapoollocal/LocalPoolVector.tpp | 4 +- unittest/tests/datapoollocal/CMakeLists.txt | 1 + unittest/tests/datapoollocal/DataSetTest.cpp | 6 ++- .../datapoollocal/LocalPoolVariableTest.cpp | 2 +- .../datapoollocal/LocalPoolVectorTest.cpp | 52 ++++++++++++++++++- 5 files changed, 59 insertions(+), 6 deletions(-) diff --git a/datapoollocal/LocalPoolVector.tpp b/datapoollocal/LocalPoolVector.tpp index a0e32f62..f311c23d 100644 --- a/datapoollocal/LocalPoolVector.tpp +++ b/datapoollocal/LocalPoolVector.tpp @@ -104,7 +104,7 @@ inline T& LocalPoolVector::operator [](size_t i) { fsfw::printWarning("LocalPoolVector: Invalid index. Setting or returning" " last value!\n"); #endif - return value[vectorSize]; + return value[vectorSize - 1]; } template @@ -121,7 +121,7 @@ inline const T& LocalPoolVector::operator [](size_t i) const { fsfw::printWarning("LocalPoolVector: Invalid index. Setting or returning" " last value!\n"); #endif - return value[vectorSize]; + return value[vectorSize - 1]; } template diff --git a/unittest/tests/datapoollocal/CMakeLists.txt b/unittest/tests/datapoollocal/CMakeLists.txt index 9d828de5..fd7f61b8 100644 --- a/unittest/tests/datapoollocal/CMakeLists.txt +++ b/unittest/tests/datapoollocal/CMakeLists.txt @@ -1,4 +1,5 @@ target_sources(${TARGET_NAME} PRIVATE LocalPoolVariableTest.cpp LocalPoolVectorTest.cpp + DataSetTest.cpp ) diff --git a/unittest/tests/datapoollocal/DataSetTest.cpp b/unittest/tests/datapoollocal/DataSetTest.cpp index ccb7603d..968a0b3d 100644 --- a/unittest/tests/datapoollocal/DataSetTest.cpp +++ b/unittest/tests/datapoollocal/DataSetTest.cpp @@ -2,6 +2,7 @@ #include #include +#include #include TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { @@ -11,9 +12,10 @@ TEST_CASE("LocalDataSet" , "[LocDataSetTest]") { REQUIRE(poolOwner->initializeHkManager() == retval::CATCH_OK); REQUIRE(poolOwner->initializeHkManagerAfterTaskCreation() == retval::CATCH_OK); - + const uint32_t setId = 0; SECTION("BasicTest") { - //StaticLocalDataSet<3> localSet = StaticLocalDataSet<3>() + StaticLocalDataSet<3> localSet = StaticLocalDataSet<3>( + sid_t(objects::TEST_LOCAL_POOL_OWNER_BASE, setId)); } } diff --git a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp index eb5abf35..5f7118ff 100644 --- a/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVariableTest.cpp @@ -23,10 +23,10 @@ TEST_CASE("LocalPoolVariable" , "[LocPoolVarTest]") { REQUIRE(testVariable.commit() == retval::CATCH_OK); REQUIRE(testVariable.read() == retval::CATCH_OK); REQUIRE(testVariable.value == 5); - CHECK(not testVariable.isValid()); testVariable.setValid(true); CHECK(testVariable.isValid()); + CHECK(testVariable.commit(true) == retval::CATCH_OK); testVariable.setReadWriteMode(pool_rwm_t::VAR_READ); CHECK(testVariable.getReadWriteMode() == pool_rwm_t::VAR_READ); diff --git a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp index 7e40e898..cfc2ec2e 100644 --- a/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp +++ b/unittest/tests/datapoollocal/LocalPoolVectorTest.cpp @@ -38,7 +38,7 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { // This is invalid access, so the last value will be set instead. // (we can't throw exceptions) testVector[4] = 12; - CHECK(testVector[3] == 12); + CHECK(testVector[2] == 12); CHECK(testVector.commit() == retval::CATCH_OK); // Use read-only reference. @@ -48,6 +48,42 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { uint16_t lastVal = roTestVec[25]; CHECK(lastVal == 12); + + size_t maxSize = testVector.getSerializedSize(); + CHECK(maxSize == 6); + + uint16_t serializedVector[3]; + uint8_t* vecPtr = reinterpret_cast(serializedVector); + size_t serSize = 0; + REQUIRE(testVector.serialize(&vecPtr, &serSize, + maxSize, SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + + CHECK(serSize == 6); + CHECK(serializedVector[0] == 5); + CHECK(serializedVector[1] == 232); + CHECK(serializedVector[2] == 12); + + maxSize = 1; + REQUIRE(testVector.serialize(&vecPtr, &serSize, + maxSize, SerializeIF::Endianness::MACHINE) == + static_cast(SerializeIF::BUFFER_TOO_SHORT)); + + serializedVector[0] = 16; + serializedVector[1] = 7832; + serializedVector[2] = 39232; + + const uint8_t* constVecPtr = reinterpret_cast( + serializedVector); + REQUIRE(testVector.deSerialize(&constVecPtr, &serSize, + SerializeIF::Endianness::MACHINE) == retval::CATCH_OK); + CHECK(testVector[0] == 16); + CHECK(testVector[1] == 7832); + CHECK(testVector[2] == 39232); + + serSize = 1; + REQUIRE(testVector.deSerialize(&constVecPtr, &serSize, + SerializeIF::Endianness::MACHINE) == + static_cast(SerializeIF::STREAM_TOO_SHORT)); } SECTION("ErrorHandling") { @@ -64,6 +100,20 @@ TEST_CASE("LocalPoolVector" , "[LocPoolVecTest]") { objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id); REQUIRE(invalidVector2.read() == static_cast(HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT)); + REQUIRE(invalidVector2.commit() == + static_cast(HasLocalDataPoolIF::POOL_ENTRY_TYPE_CONFLICT)); + + lp_vec_t writeOnlyVec = lp_vec_t( + objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id, + nullptr, pool_rwm_t::VAR_WRITE); + REQUIRE(writeOnlyVec.read() == + static_cast(PoolVariableIF::INVALID_READ_WRITE_MODE)); + + lp_vec_t readOnlyVec = lp_vec_t( + objects::TEST_LOCAL_POOL_OWNER_BASE, lpool::uint16Vec3Id, + nullptr, pool_rwm_t::VAR_READ); + REQUIRE(readOnlyVec.commit() == + static_cast(PoolVariableIF::INVALID_READ_WRITE_MODE)); } } From 12091ca6ab48df043708b2a63e0b695679f5315d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 20:55:36 +0100 Subject: [PATCH 18/19] enum members renamed, global vars static now --- serviceinterface/ServiceInterfacePrinter.cpp | 30 ++++++++++---------- serviceinterface/ServiceInterfacePrinter.h | 8 +++--- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/serviceinterface/ServiceInterfacePrinter.cpp b/serviceinterface/ServiceInterfacePrinter.cpp index cb3a958d..fff6753b 100644 --- a/serviceinterface/ServiceInterfacePrinter.cpp +++ b/serviceinterface/ServiceInterfacePrinter.cpp @@ -6,11 +6,11 @@ #include #include -fsfw::PrintLevel printLevel = fsfw::PrintLevel::DEBUG; +static fsfw::PrintLevel printLevel = fsfw::PrintLevel::DEBUG_LEVEL; #if defined(WIN32) && FSFW_COLORED_OUTPUT == 1 -bool consoleInitialized = false; +static bool consoleInitialized = false; #endif /* defined(WIN32) && FSFW_COLORED_OUTPUT == 1 */ -bool addCrAtEnd = false; +static bool addCrAtEnd = false; #if FSFW_DISABLE_PRINTOUT == 0 uint8_t printBuffer[fsfwconfig::FSFW_PRINT_BUFFER_SIZE]; @@ -39,31 +39,31 @@ void fsfwPrint(fsfw::PrintLevel printType, const char* fmt, va_list arg) { /* Log message to terminal */ #if FSFW_COLORED_OUTPUT == 1 - if(printType == fsfw::PrintLevel::INFO) { + if(printType == fsfw::PrintLevel::INFO_LEVEL) { len += sprintf(bufferPosition, fsfw::ANSI_COLOR_GREEN); } - else if(printType == fsfw::PrintLevel::DEBUG) { + else if(printType == fsfw::PrintLevel::DEBUG_LEVEL) { len += sprintf(bufferPosition, fsfw::ANSI_COLOR_MAGENTA); } - else if(printType == fsfw::PrintLevel::WARNING) { + else if(printType == fsfw::PrintLevel::WARNING_LEVEL) { len += sprintf(bufferPosition, fsfw::ANSI_COLOR_YELLOW); } - else if(printType == fsfw::PrintLevel::ERROR_TYPE) { + else if(printType == fsfw::PrintLevel::ERROR_LEVEL) { len += sprintf(bufferPosition, fsfw::ANSI_COLOR_RED); } #endif - if (printType == fsfw::PrintLevel::INFO) { + if (printType == fsfw::PrintLevel::INFO_LEVEL) { len += sprintf(bufferPosition + len, "INFO: "); } - if(printType == fsfw::PrintLevel::DEBUG) { + if(printType == fsfw::PrintLevel::DEBUG_LEVEL) { len += sprintf(bufferPosition + len, "DEBUG: "); } - if(printType == fsfw::PrintLevel::WARNING) { + if(printType == fsfw::PrintLevel::WARNING_LEVEL) { len += sprintf(bufferPosition + len, "WARNING: "); } - if(printType == fsfw::PrintLevel::ERROR_TYPE) { + if(printType == fsfw::PrintLevel::ERROR_LEVEL) { len += sprintf(bufferPosition + len, "ERROR: "); } @@ -91,21 +91,21 @@ void fsfwPrint(fsfw::PrintLevel printType, const char* fmt, va_list arg) { void fsfw::printInfo(const char *fmt, ...) { va_list args; va_start(args, fmt); - fsfwPrint(fsfw::PrintLevel::INFO, fmt, args); + fsfwPrint(fsfw::PrintLevel::INFO_LEVEL, fmt, args); va_end(args); } void fsfw::printWarning(const char *fmt, ...) { va_list args; va_start(args, fmt); - fsfwPrint(fsfw::PrintLevel::WARNING, fmt, args); + fsfwPrint(fsfw::PrintLevel::WARNING_LEVEL, fmt, args); va_end(args); } void fsfw::printDebug(const char *fmt, ...) { va_list args; va_start(args, fmt); - fsfwPrint(fsfw::PrintLevel::DEBUG, fmt, args); + fsfwPrint(fsfw::PrintLevel::DEBUG_LEVEL, fmt, args); va_end(args); } @@ -116,7 +116,7 @@ void fsfw::setToAddCrAtEnd(bool addCrAtEnd_) { void fsfw::printError(const char *fmt, ...) { va_list args; va_start(args, fmt); - fsfwPrint(fsfw::PrintLevel::ERROR_TYPE, fmt, args); + fsfwPrint(fsfw::PrintLevel::ERROR_LEVEL, fmt, args); va_end(args); } diff --git a/serviceinterface/ServiceInterfacePrinter.h b/serviceinterface/ServiceInterfacePrinter.h index fdff797a..f49ccbb7 100644 --- a/serviceinterface/ServiceInterfacePrinter.h +++ b/serviceinterface/ServiceInterfacePrinter.h @@ -7,10 +7,10 @@ namespace fsfw { enum PrintLevel { NONE = 0, //! Strange error when using just ERROR.. - ERROR_TYPE = 1, - WARNING = 2, - INFO = 3, - DEBUG = 4 + ERROR_LEVEL = 1, + WARNING_LEVEL = 2, + INFO_LEVEL = 3, + DEBUG_LEVEL = 4 }; /** From 57829faa64f004a48510e057742c4a9fb03a3726 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 8 Jan 2021 21:03:47 +0100 Subject: [PATCH 19/19] added cast --- devicehandlers/DeviceHandlerBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 0efdeba0..4fa7b09c 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1321,7 +1321,8 @@ void DeviceHandlerBase::buildInternalCommand(void) { } else if (iter->second.isExecuting) { #if FSFW_DISABLE_PRINTOUT == 0 char output[36]; - sprintf(output, "Command 0x%08x is executing", deviceCommandId); + sprintf(output, "Command 0x%08x is executing", + static_cast(deviceCommandId)); // so we can track misconfigurations printWarningOrError(fsfw::OutputTypes::OUT_WARNING, "buildInternalCommand",