From b7f3eff74230fa89fc02cc28a368460b32cfb80d Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Fri, 18 Feb 2022 19:08:06 +0100 Subject: [PATCH 1/8] WIP unit tests --- scripts/helper.py | 4 -- .../internalerror/InternalErrorReporter.cpp | 3 + .../internalerror/InternalErrorReporter.h | 6 +- tests/src/fsfw_tests/unit/CMakeLists.txt | 1 + .../unit/internalerror/CMakeLists.txt | 3 + .../TestInternalErrorReporter.cpp | 71 +++++++++++++++++++ .../unit/mocks/PeriodicTaskIFMock.h | 45 ++++++++++++ .../fsfw_tests/unit/osal/TestMessageQueue.cpp | 21 ++++++ 8 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 tests/src/fsfw_tests/unit/internalerror/CMakeLists.txt create mode 100644 tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp create mode 100644 tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h diff --git a/scripts/helper.py b/scripts/helper.py index 68693c40f..079a5548c 100755 --- a/scripts/helper.py +++ b/scripts/helper.py @@ -143,10 +143,6 @@ def handle_tests_type(args, build_dir_list: list): if which("valgrind") is None: print("Please install valgrind first") sys.exit(1) - if os.path.split(os.getcwd())[1] != UNITTEST_FOLDER_NAME: - # If we are in a different directory we try to switch into it but - # this might fail - os.chdir(UNITTEST_FOLDER_NAME) os.system("valgrind --leak-check=full ./fsfw-tests") os.chdir("..") diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index e7088e2cf..98ed2af32 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -57,6 +57,9 @@ ReturnValue_t InternalErrorReporter::performOperation(uint8_t opCode) { internalErrorDataset.storeHits.value += newStoreHits; internalErrorDataset.tmHits.value += newTmHits; internalErrorDataset.setValidity(true, true); + if((newQueueHits != 0) or (newStoreHits !=0) or (newTmHits != 0)){ + internalErrorDataset.setChanged(true); + } } } diff --git a/src/fsfw/internalerror/InternalErrorReporter.h b/src/fsfw/internalerror/InternalErrorReporter.h index 987111396..a946b81c1 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.h +++ b/src/fsfw/internalerror/InternalErrorReporter.h @@ -46,11 +46,11 @@ class InternalErrorReporter : public SystemObject, virtual ReturnValue_t initializeAfterTaskCreation() override; virtual ReturnValue_t performOperation(uint8_t opCode) override; - virtual void queueMessageNotSent(); + virtual void queueMessageNotSent() override; - virtual void lostTm(); + virtual void lostTm() override; - virtual void storeFull(); + virtual void storeFull() override; virtual void setTaskIF(PeriodicTaskIF* task) override; diff --git a/tests/src/fsfw_tests/unit/CMakeLists.txt b/tests/src/fsfw_tests/unit/CMakeLists.txt index fc73f7b50..a83e2f7f8 100644 --- a/tests/src/fsfw_tests/unit/CMakeLists.txt +++ b/tests/src/fsfw_tests/unit/CMakeLists.txt @@ -23,3 +23,4 @@ add_subdirectory(timemanager) add_subdirectory(tmtcpacket) add_subdirectory(cfdp) add_subdirectory(hal) +add_subdirectory(internalerror) diff --git a/tests/src/fsfw_tests/unit/internalerror/CMakeLists.txt b/tests/src/fsfw_tests/unit/internalerror/CMakeLists.txt new file mode 100644 index 000000000..d49ce0067 --- /dev/null +++ b/tests/src/fsfw_tests/unit/internalerror/CMakeLists.txt @@ -0,0 +1,3 @@ +target_sources(${FSFW_TEST_TGT} PRIVATE + TestInternalErrorReporter.cpp +) diff --git a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp new file mode 100644 index 000000000..d394166e2 --- /dev/null +++ b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp @@ -0,0 +1,71 @@ +#include +#include +#include +#include +#include + +#include +#include + +#include "fsfw/action/ActionMessage.h" +#include "fsfw/ipc/CommandMessage.h" +#include "fsfw/ipc/MessageQueueMessage.h" +#include "fsfw/objectmanager/frameworkObjects.h" +#include "fsfw_tests/unit/CatchDefinitions.h" +#include "fsfw_tests/unit/mocks/PeriodicTaskIFMock.h" + +TEST_CASE("Internal Error Reporter", "[TestInternalError]") { + PeriodicTaskMock task(10); + ObjectManagerIF* manager = ObjectManager::instance(); + if(manager == nullptr){ + FAIL(); + } + InternalErrorReporter* internalErrorReporter = + dynamic_cast(ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER)); + task.addComponent(objects::INTERNAL_ERROR_REPORTER); + MessageQueueIF* testQueue = QueueFactory::instance()->createMessageQueue(1); + MessageQueueIF* hkQueue = QueueFactory::instance()->createMessageQueue(1); + internalErrorReporter->getSubscriptionInterface()-> + subscribeForSetUpdateMessage(InternalErrorDataset::ERROR_SET_ID, objects::NO_OBJECT, hkQueue->getId(), true); + StorageManagerIF* ipcStore = ObjectManager::instance()->get(objects::IPC_STORE); + SECTION("MessageQueueFull"){ + CommandMessage message; + ActionMessage::setCompletionReply(&message, 10, true); + auto result = hkQueue->sendMessage(testQueue->getId(), &message); + REQUIRE(result == retval::CATCH_OK); + internalErrorReporter->performOperation(0); + { + CommandMessage hkMessage; + result = hkQueue->receiveMessage(&hkMessage); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); + store_address_t storeAddress; + gp_id_t gpid = HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); + REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); + InternalErrorDataset dataset(objects::NO_OBJECT); + CCSDSTime::CDS_short time; + ConstAccessorPair data = ipcStore->getData(storeAddress); + HousekeepingSnapshot hkSnapshot(&time, &dataset); + const uint8_t* buffer = data.second.data(); + size_t size = data.second.size(); + hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + } + result = hkQueue->sendMessage(testQueue->getId(), &message); + REQUIRE(result == MessageQueueIF::FULL); + { + internalErrorReporter->performOperation(0); + CommandMessage hkMessage; + result = hkQueue->receiveMessage(&hkMessage); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); + store_address_t storeAddress; + gp_id_t gpid = HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); + REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); + + ConstAccessorPair data = ipcStore->getData(storeAddress); + REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); + } + } + delete testQueue; + delete hkQueue; +} \ No newline at end of file diff --git a/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h b/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h new file mode 100644 index 000000000..dd5bca124 --- /dev/null +++ b/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h @@ -0,0 +1,45 @@ +#ifndef FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ +#define FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ + +#include +#include + +class PeriodicTaskMock: public PeriodicTaskIF{ +public: + PeriodicTaskMock(uint32_t period = 5):period(period){ + + } + /** + * @brief A virtual destructor as it is mandatory for interfaces. + */ + virtual ~PeriodicTaskMock() {} + /** + * @brief With the startTask method, a created task can be started + * for the first time. + */ + virtual ReturnValue_t startTask() override{ + return HasReturnvaluesIF::RETURN_OK; + }; + + virtual ReturnValue_t addComponent(object_id_t object) override{ + ExecutableObjectIF* executableObject = ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER); + if(executableObject == nullptr){ + return HasReturnvaluesIF::RETURN_FAILED; + } + executableObject->setTaskIF(this); + executableObject->initializeAfterTaskCreation(); + return HasReturnvaluesIF::RETURN_OK; + }; + + virtual ReturnValue_t sleepFor(uint32_t ms) override{ + return HasReturnvaluesIF::RETURN_OK; + }; + + virtual uint32_t getPeriodMs() const override{ + return period; + }; + uint32_t period; +}; + + +#endif // FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ \ No newline at end of file diff --git a/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp b/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp index 232ffb94b..365df379b 100644 --- a/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp +++ b/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp @@ -35,4 +35,25 @@ TEST_CASE("MessageQueue Basic Test", "[TestMq]") { senderId = testReceiverMq->getLastPartner(); CHECK(senderId == testSenderMqId); } + SECTION("Test Full") { + auto result = testSenderMq->sendMessage(testReceiverMqId, &testMessage); + REQUIRE(result == retval::CATCH_OK); + result = testSenderMq->sendMessage(testReceiverMqId, &testMessage); + REQUIRE(result == MessageQueueIF::FULL); + // We try another message + result = testSenderMq->sendMessage(testReceiverMqId, &testMessage); + REQUIRE(result == MessageQueueIF::FULL); + MessageQueueMessage recvMessage; + result = testReceiverMq->receiveMessage(&recvMessage); + REQUIRE(result == retval::CATCH_OK); + CHECK(recvMessage.getData()[0] == 42); + result = testSenderMq->sendMessage(testReceiverMqId, &testMessage); + REQUIRE(result == retval::CATCH_OK); + result = testReceiverMq->receiveMessage(&recvMessage); + REQUIRE(result == retval::CATCH_OK); + CHECK(recvMessage.getData()[0] == 42); + } + // We have to clear MQs ourself ATM + delete testSenderMq; + delete testReceiverMq; } From 45ea09291a76c8d609a0315fa7f1ba59ef2b64a4 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Fri, 18 Feb 2022 19:57:36 +0100 Subject: [PATCH 2/8] Still test for InternalError Reporter --- .../internalerror/TestInternalErrorReporter.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp index d394166e2..1103037d8 100644 --- a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp +++ b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,7 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { auto result = hkQueue->sendMessage(testQueue->getId(), &message); REQUIRE(result == retval::CATCH_OK); internalErrorReporter->performOperation(0); + uint32_t queueHits = 0; { CommandMessage hkMessage; result = hkQueue->receiveMessage(&hkMessage); @@ -45,10 +47,13 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { InternalErrorDataset dataset(objects::NO_OBJECT); CCSDSTime::CDS_short time; ConstAccessorPair data = ipcStore->getData(storeAddress); + REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); HousekeepingSnapshot hkSnapshot(&time, &dataset); const uint8_t* buffer = data.second.data(); size_t size = data.second.size(); - hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + queueHits = dataset.queueHits.value; } result = hkQueue->sendMessage(testQueue->getId(), &message); REQUIRE(result == MessageQueueIF::FULL); @@ -64,6 +69,14 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { ConstAccessorPair data = ipcStore->getData(storeAddress); REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); + CCSDSTime::CDS_short time; + InternalErrorDataset dataset(objects::NO_OBJECT); + HousekeepingSnapshot hkSnapshot(&time, &dataset); + const uint8_t* buffer = data.second.data(); + size_t size = data.second.size(); + result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + REQUIRE(dataset.queueHits == (queueHits + 1)); } } delete testQueue; From 9c2ceb4a9f5dbe2517a224572846fa7a996ddcac Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:29:23 +0100 Subject: [PATCH 3/8] one shot flag not static anymore --- CHANGELOG.md | 6 ++++++ tests/src/fsfw_tests/integration/task/TestTask.cpp | 1 - tests/src/fsfw_tests/integration/task/TestTask.h | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 335c0f7bb..ac5f004ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +# [v5.0.0] + +## Changes + +- `oneShotAction` flag in the `TestTask` class is not static anymore + # [v4.0.0] ## Additions diff --git a/tests/src/fsfw_tests/integration/task/TestTask.cpp b/tests/src/fsfw_tests/integration/task/TestTask.cpp index 65e444e38..765f780e6 100644 --- a/tests/src/fsfw_tests/integration/task/TestTask.cpp +++ b/tests/src/fsfw_tests/integration/task/TestTask.cpp @@ -3,7 +3,6 @@ #include #include -bool TestTask::oneShotAction = true; MutexIF* TestTask::testLock = nullptr; TestTask::TestTask(object_id_t objectId) : SystemObject(objectId), testMode(testModes::A) { diff --git a/tests/src/fsfw_tests/integration/task/TestTask.h b/tests/src/fsfw_tests/integration/task/TestTask.h index 557b50b21..cd630ee3a 100644 --- a/tests/src/fsfw_tests/integration/task/TestTask.h +++ b/tests/src/fsfw_tests/integration/task/TestTask.h @@ -29,7 +29,7 @@ class TestTask : public SystemObject, public ExecutableObjectIF, public HasRetur bool testFlag = false; private: - static bool oneShotAction; + bool oneShotAction = true; static MutexIF* testLock; StorageManagerIF* IPCStore; }; From a50b52df5137ecae86f0c2a8db21210ec507cece Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 22 Feb 2022 13:37:28 +0100 Subject: [PATCH 4/8] Fixed an issue in host OSAL and added more coverage to IER --- .../datapoollocal/LocalDataPoolManager.cpp | 2 +- src/fsfw/osal/host/MessageQueue.cpp | 7 +++++ .../TestInternalErrorReporter.cpp | 30 +++++++++++++++---- .../fsfw_tests/unit/osal/TestMessageQueue.cpp | 4 +-- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp index 9057de31b..cab13207a 100644 --- a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp +++ b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp @@ -84,7 +84,7 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { return result; } - printWarningOrError(sif::OutputTypes::OUT_WARNING, "initialize", HasReturnvaluesIF::RETURN_FAILED, + printWarningOrError(sif::OutputTypes::OUT_WARNING, "initializeHousekeepingPoolEntriesOnce", HasReturnvaluesIF::RETURN_FAILED, "The map should only be initialized once"); return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/osal/host/MessageQueue.cpp b/src/fsfw/osal/host/MessageQueue.cpp index db66b6715..621511646 100644 --- a/src/fsfw/osal/host/MessageQueue.cpp +++ b/src/fsfw/osal/host/MessageQueue.cpp @@ -125,6 +125,13 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, memcpy(targetQueue->messageQueue.back().data(), message->getBuffer(), message->getMaximumMessageSize()); } else { + if (not ignoreFault) { + InternalErrorReporterIF* internalErrorReporter = + ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER); + if (internalErrorReporter != nullptr) { + internalErrorReporter->queueMessageNotSent(); + } + } return MessageQueueIF::FULL; } return HasReturnvaluesIF::RETURN_OK; diff --git a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp index 1103037d8..1e63a57b9 100644 --- a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp +++ b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp @@ -23,6 +23,9 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { } InternalErrorReporter* internalErrorReporter = dynamic_cast(ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER)); + if(internalErrorReporter == nullptr){ + FAIL(); + } task.addComponent(objects::INTERNAL_ERROR_REPORTER); MessageQueueIF* testQueue = QueueFactory::instance()->createMessageQueue(1); MessageQueueIF* hkQueue = QueueFactory::instance()->createMessageQueue(1); @@ -34,8 +37,13 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { ActionMessage::setCompletionReply(&message, 10, true); auto result = hkQueue->sendMessage(testQueue->getId(), &message); REQUIRE(result == retval::CATCH_OK); - internalErrorReporter->performOperation(0); uint32_t queueHits = 0; + uint32_t lostTm = 0; + uint32_t storeHits = 0; + /* We don't know if another test caused a queue Hit so we will enforce one, + then remeber the queueHit count and force another hit */ + internalErrorReporter->queueMessageNotSent(); + internalErrorReporter->performOperation(0); { CommandMessage hkMessage; result = hkQueue->receiveMessage(&hkMessage); @@ -44,7 +52,8 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { store_address_t storeAddress; gp_id_t gpid = HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); - InternalErrorDataset dataset(objects::NO_OBJECT); + // We need the object ID of the reporter here (NO_OBJECT) + InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); CCSDSTime::CDS_short time; ConstAccessorPair data = ipcStore->getData(storeAddress); REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); @@ -53,10 +62,15 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { size_t size = data.second.size(); result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + // Remember the amount of queueHits before to see the increase queueHits = dataset.queueHits.value; + lostTm = dataset.tmHits.value; + storeHits = dataset.storeHits.value; } result = hkQueue->sendMessage(testQueue->getId(), &message); REQUIRE(result == MessageQueueIF::FULL); + internalErrorReporter->lostTm(); + internalErrorReporter->storeFull(); { internalErrorReporter->performOperation(0); CommandMessage hkMessage; @@ -70,15 +84,19 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { ConstAccessorPair data = ipcStore->getData(storeAddress); REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); CCSDSTime::CDS_short time; - InternalErrorDataset dataset(objects::NO_OBJECT); + // We need the object ID of the reporter here (NO_OBJECT) + InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); HousekeepingSnapshot hkSnapshot(&time, &dataset); const uint8_t* buffer = data.second.data(); size_t size = data.second.size(); result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); REQUIRE(result == HasReturnvaluesIF::RETURN_OK); - REQUIRE(dataset.queueHits == (queueHits + 1)); + // Test that we had one more queueHit + REQUIRE(dataset.queueHits.value == (queueHits + 1)); + REQUIRE(dataset.tmHits.value == (lostTm + 1)); + REQUIRE(dataset.storeHits.value == (storeHits + 1)); } } - delete testQueue; - delete hkQueue; + QueueFactory::instance()->deleteMessageQueue(testQueue); + QueueFactory::instance()->deleteMessageQueue(hkQueue); } \ No newline at end of file diff --git a/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp b/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp index 365df379b..11c0739b1 100644 --- a/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp +++ b/tests/src/fsfw_tests/unit/osal/TestMessageQueue.cpp @@ -54,6 +54,6 @@ TEST_CASE("MessageQueue Basic Test", "[TestMq]") { CHECK(recvMessage.getData()[0] == 42); } // We have to clear MQs ourself ATM - delete testSenderMq; - delete testReceiverMq; + QueueFactory::instance()->deleteMessageQueue(testSenderMq); + QueueFactory::instance()->deleteMessageQueue(testReceiverMq); } From 4862edfdb5dec18e34e0678930f25aeacd2ed409 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 22 Feb 2022 13:42:56 +0100 Subject: [PATCH 5/8] Clang format --- .../internalerror/InternalErrorReporter.cpp | 2 +- src/fsfw/osal/host/MessageQueue.cpp | 4 +- .../TestInternalErrorReporter.cpp | 168 +++++++++--------- .../unit/mocks/PeriodicTaskIFMock.h | 34 ++-- 4 files changed, 101 insertions(+), 107 deletions(-) diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index 98ed2af32..08d37feb8 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -57,7 +57,7 @@ ReturnValue_t InternalErrorReporter::performOperation(uint8_t opCode) { internalErrorDataset.storeHits.value += newStoreHits; internalErrorDataset.tmHits.value += newTmHits; internalErrorDataset.setValidity(true, true); - if((newQueueHits != 0) or (newStoreHits !=0) or (newTmHits != 0)){ + if ((newQueueHits != 0) or (newStoreHits != 0) or (newTmHits != 0)) { internalErrorDataset.setChanged(true); } } diff --git a/src/fsfw/osal/host/MessageQueue.cpp b/src/fsfw/osal/host/MessageQueue.cpp index 621511646..d328fb820 100644 --- a/src/fsfw/osal/host/MessageQueue.cpp +++ b/src/fsfw/osal/host/MessageQueue.cpp @@ -125,13 +125,13 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, memcpy(targetQueue->messageQueue.back().data(), message->getBuffer(), message->getMaximumMessageSize()); } else { - if (not ignoreFault) { + if (not ignoreFault) { InternalErrorReporterIF* internalErrorReporter = ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER); if (internalErrorReporter != nullptr) { internalErrorReporter->queueMessageNotSent(); } - } + } return MessageQueueIF::FULL; } return HasReturnvaluesIF::RETURN_OK; diff --git a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp index 1e63a57b9..a7f5e1092 100644 --- a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp +++ b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp @@ -1,9 +1,9 @@ +#include +#include #include #include -#include #include #include -#include #include #include @@ -16,87 +16,89 @@ #include "fsfw_tests/unit/mocks/PeriodicTaskIFMock.h" TEST_CASE("Internal Error Reporter", "[TestInternalError]") { - PeriodicTaskMock task(10); - ObjectManagerIF* manager = ObjectManager::instance(); - if(manager == nullptr){ - FAIL(); + PeriodicTaskMock task(10); + ObjectManagerIF* manager = ObjectManager::instance(); + if (manager == nullptr) { + FAIL(); + } + InternalErrorReporter* internalErrorReporter = dynamic_cast( + ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER)); + if (internalErrorReporter == nullptr) { + FAIL(); + } + task.addComponent(objects::INTERNAL_ERROR_REPORTER); + MessageQueueIF* testQueue = QueueFactory::instance()->createMessageQueue(1); + MessageQueueIF* hkQueue = QueueFactory::instance()->createMessageQueue(1); + internalErrorReporter->getSubscriptionInterface()->subscribeForSetUpdateMessage( + InternalErrorDataset::ERROR_SET_ID, objects::NO_OBJECT, hkQueue->getId(), true); + StorageManagerIF* ipcStore = ObjectManager::instance()->get(objects::IPC_STORE); + SECTION("MessageQueueFull") { + CommandMessage message; + ActionMessage::setCompletionReply(&message, 10, true); + auto result = hkQueue->sendMessage(testQueue->getId(), &message); + REQUIRE(result == retval::CATCH_OK); + uint32_t queueHits = 0; + uint32_t lostTm = 0; + uint32_t storeHits = 0; + /* We don't know if another test caused a queue Hit so we will enforce one, + then remeber the queueHit count and force another hit */ + internalErrorReporter->queueMessageNotSent(); + internalErrorReporter->performOperation(0); + { + CommandMessage hkMessage; + result = hkQueue->receiveMessage(&hkMessage); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); + store_address_t storeAddress; + gp_id_t gpid = + HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); + REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); + // We need the object ID of the reporter here (NO_OBJECT) + InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); + CCSDSTime::CDS_short time; + ConstAccessorPair data = ipcStore->getData(storeAddress); + REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); + HousekeepingSnapshot hkSnapshot(&time, &dataset); + const uint8_t* buffer = data.second.data(); + size_t size = data.second.size(); + result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + // Remember the amount of queueHits before to see the increase + queueHits = dataset.queueHits.value; + lostTm = dataset.tmHits.value; + storeHits = dataset.storeHits.value; } - InternalErrorReporter* internalErrorReporter = - dynamic_cast(ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER)); - if(internalErrorReporter == nullptr){ - FAIL(); + result = hkQueue->sendMessage(testQueue->getId(), &message); + REQUIRE(result == MessageQueueIF::FULL); + internalErrorReporter->lostTm(); + internalErrorReporter->storeFull(); + { + internalErrorReporter->performOperation(0); + CommandMessage hkMessage; + result = hkQueue->receiveMessage(&hkMessage); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); + store_address_t storeAddress; + gp_id_t gpid = + HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); + REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); + + ConstAccessorPair data = ipcStore->getData(storeAddress); + REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); + CCSDSTime::CDS_short time; + // We need the object ID of the reporter here (NO_OBJECT) + InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); + HousekeepingSnapshot hkSnapshot(&time, &dataset); + const uint8_t* buffer = data.second.data(); + size_t size = data.second.size(); + result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + // Test that we had one more queueHit + REQUIRE(dataset.queueHits.value == (queueHits + 1)); + REQUIRE(dataset.tmHits.value == (lostTm + 1)); + REQUIRE(dataset.storeHits.value == (storeHits + 1)); } - task.addComponent(objects::INTERNAL_ERROR_REPORTER); - MessageQueueIF* testQueue = QueueFactory::instance()->createMessageQueue(1); - MessageQueueIF* hkQueue = QueueFactory::instance()->createMessageQueue(1); - internalErrorReporter->getSubscriptionInterface()-> - subscribeForSetUpdateMessage(InternalErrorDataset::ERROR_SET_ID, objects::NO_OBJECT, hkQueue->getId(), true); - StorageManagerIF* ipcStore = ObjectManager::instance()->get(objects::IPC_STORE); - SECTION("MessageQueueFull"){ - CommandMessage message; - ActionMessage::setCompletionReply(&message, 10, true); - auto result = hkQueue->sendMessage(testQueue->getId(), &message); - REQUIRE(result == retval::CATCH_OK); - uint32_t queueHits = 0; - uint32_t lostTm = 0; - uint32_t storeHits = 0; - /* We don't know if another test caused a queue Hit so we will enforce one, - then remeber the queueHit count and force another hit */ - internalErrorReporter->queueMessageNotSent(); - internalErrorReporter->performOperation(0); - { - CommandMessage hkMessage; - result = hkQueue->receiveMessage(&hkMessage); - REQUIRE(result == HasReturnvaluesIF::RETURN_OK); - REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); - store_address_t storeAddress; - gp_id_t gpid = HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); - REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); - // We need the object ID of the reporter here (NO_OBJECT) - InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); - CCSDSTime::CDS_short time; - ConstAccessorPair data = ipcStore->getData(storeAddress); - REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); - HousekeepingSnapshot hkSnapshot(&time, &dataset); - const uint8_t* buffer = data.second.data(); - size_t size = data.second.size(); - result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); - REQUIRE(result == HasReturnvaluesIF::RETURN_OK); - // Remember the amount of queueHits before to see the increase - queueHits = dataset.queueHits.value; - lostTm = dataset.tmHits.value; - storeHits = dataset.storeHits.value; - } - result = hkQueue->sendMessage(testQueue->getId(), &message); - REQUIRE(result == MessageQueueIF::FULL); - internalErrorReporter->lostTm(); - internalErrorReporter->storeFull(); - { - internalErrorReporter->performOperation(0); - CommandMessage hkMessage; - result = hkQueue->receiveMessage(&hkMessage); - REQUIRE(result == HasReturnvaluesIF::RETURN_OK); - REQUIRE(hkMessage.getCommand() == HousekeepingMessage::UPDATE_SNAPSHOT_SET); - store_address_t storeAddress; - gp_id_t gpid = HousekeepingMessage::getUpdateSnapshotVariableCommand(&hkMessage, &storeAddress); - REQUIRE(gpid.objectId == objects::INTERNAL_ERROR_REPORTER); - - ConstAccessorPair data = ipcStore->getData(storeAddress); - REQUIRE(data.first == HasReturnvaluesIF::RETURN_OK); - CCSDSTime::CDS_short time; - // We need the object ID of the reporter here (NO_OBJECT) - InternalErrorDataset dataset(objects::INTERNAL_ERROR_REPORTER); - HousekeepingSnapshot hkSnapshot(&time, &dataset); - const uint8_t* buffer = data.second.data(); - size_t size = data.second.size(); - result = hkSnapshot.deSerialize(&buffer, &size, SerializeIF::Endianness::MACHINE); - REQUIRE(result == HasReturnvaluesIF::RETURN_OK); - // Test that we had one more queueHit - REQUIRE(dataset.queueHits.value == (queueHits + 1)); - REQUIRE(dataset.tmHits.value == (lostTm + 1)); - REQUIRE(dataset.storeHits.value == (storeHits + 1)); - } - } - QueueFactory::instance()->deleteMessageQueue(testQueue); - QueueFactory::instance()->deleteMessageQueue(hkQueue); + } + QueueFactory::instance()->deleteMessageQueue(testQueue); + QueueFactory::instance()->deleteMessageQueue(hkQueue); } \ No newline at end of file diff --git a/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h b/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h index dd5bca124..ebd9a2e70 100644 --- a/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h +++ b/tests/src/fsfw_tests/unit/mocks/PeriodicTaskIFMock.h @@ -1,14 +1,12 @@ #ifndef FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ #define FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ -#include #include +#include -class PeriodicTaskMock: public PeriodicTaskIF{ -public: - PeriodicTaskMock(uint32_t period = 5):period(period){ - - } +class PeriodicTaskMock : public PeriodicTaskIF { + public: + PeriodicTaskMock(uint32_t period = 5) : period(period) {} /** * @brief A virtual destructor as it is mandatory for interfaces. */ @@ -17,29 +15,23 @@ public: * @brief With the startTask method, a created task can be started * for the first time. */ - virtual ReturnValue_t startTask() override{ - return HasReturnvaluesIF::RETURN_OK; - }; + virtual ReturnValue_t startTask() override { return HasReturnvaluesIF::RETURN_OK; }; - virtual ReturnValue_t addComponent(object_id_t object) override{ - ExecutableObjectIF* executableObject = ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER); - if(executableObject == nullptr){ - return HasReturnvaluesIF::RETURN_FAILED; + virtual ReturnValue_t addComponent(object_id_t object) override { + ExecutableObjectIF* executableObject = + ObjectManager::instance()->get(objects::INTERNAL_ERROR_REPORTER); + if (executableObject == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; } executableObject->setTaskIF(this); executableObject->initializeAfterTaskCreation(); return HasReturnvaluesIF::RETURN_OK; }; - virtual ReturnValue_t sleepFor(uint32_t ms) override{ - return HasReturnvaluesIF::RETURN_OK; - }; + virtual ReturnValue_t sleepFor(uint32_t ms) override { return HasReturnvaluesIF::RETURN_OK; }; - virtual uint32_t getPeriodMs() const override{ - return period; - }; + virtual uint32_t getPeriodMs() const override { return period; }; uint32_t period; }; - -#endif // FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ \ No newline at end of file +#endif // FSFW_UNITTEST_TESTS_MOCKS_PERIODICTASKMOCK_H_ \ No newline at end of file From e5e85bcff97266f0ff46eca5bd461c323dc62424 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 22 Feb 2022 13:43:25 +0100 Subject: [PATCH 6/8] still clang --- src/fsfw/datapoollocal/LocalDataPoolManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp index cab13207a..acfa23c55 100644 --- a/src/fsfw/datapoollocal/LocalDataPoolManager.cpp +++ b/src/fsfw/datapoollocal/LocalDataPoolManager.cpp @@ -84,8 +84,8 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { return result; } - printWarningOrError(sif::OutputTypes::OUT_WARNING, "initializeHousekeepingPoolEntriesOnce", HasReturnvaluesIF::RETURN_FAILED, - "The map should only be initialized once"); + printWarningOrError(sif::OutputTypes::OUT_WARNING, "initializeHousekeepingPoolEntriesOnce", + HasReturnvaluesIF::RETURN_FAILED, "The map should only be initialized once"); return HasReturnvaluesIF::RETURN_OK; } From 2cb254a556b3f5da550fec9e479b56de6834274b Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Wed, 23 Feb 2022 11:53:48 +0100 Subject: [PATCH 7/8] Removed unused code --- .../internalerror/InternalErrorReporter.cpp | 24 ------------------- .../internalerror/InternalErrorReporter.h | 3 --- 2 files changed, 27 deletions(-) diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index 08d37feb8..fa16ec3f6 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -80,14 +80,6 @@ uint32_t InternalErrorReporter::getAndResetQueueHits() { return value; } -uint32_t InternalErrorReporter::getQueueHits() { - uint32_t value; - mutex->lockMutex(timeoutType, timeoutMs); - value = queueHits; - mutex->unlockMutex(); - return value; -} - void InternalErrorReporter::incrementQueueHits() { mutex->lockMutex(timeoutType, timeoutMs); queueHits++; @@ -103,14 +95,6 @@ uint32_t InternalErrorReporter::getAndResetTmHits() { return value; } -uint32_t InternalErrorReporter::getTmHits() { - uint32_t value; - mutex->lockMutex(timeoutType, timeoutMs); - value = tmHits; - mutex->unlockMutex(); - return value; -} - void InternalErrorReporter::incrementTmHits() { mutex->lockMutex(timeoutType, timeoutMs); tmHits++; @@ -128,14 +112,6 @@ uint32_t InternalErrorReporter::getAndResetStoreHits() { return value; } -uint32_t InternalErrorReporter::getStoreHits() { - uint32_t value; - mutex->lockMutex(timeoutType, timeoutMs); - value = storeHits; - mutex->unlockMutex(); - return value; -} - void InternalErrorReporter::incrementStoreHits() { mutex->lockMutex(timeoutType, timeoutMs); storeHits++; diff --git a/src/fsfw/internalerror/InternalErrorReporter.h b/src/fsfw/internalerror/InternalErrorReporter.h index a946b81c1..549be4bdc 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.h +++ b/src/fsfw/internalerror/InternalErrorReporter.h @@ -74,15 +74,12 @@ class InternalErrorReporter : public SystemObject, uint32_t storeHits = 0; uint32_t getAndResetQueueHits(); - uint32_t getQueueHits(); void incrementQueueHits(); uint32_t getAndResetTmHits(); - uint32_t getTmHits(); void incrementTmHits(); uint32_t getAndResetStoreHits(); - uint32_t getStoreHits(); void incrementStoreHits(); }; From d6508e23b610dfc2541fe77e38758c28e80c6ff5 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Wed, 23 Feb 2022 12:12:49 +0100 Subject: [PATCH 8/8] Added more coverage and Documentation --- .../internalerror/InternalErrorReporterIF.h | 24 ++++++++++++++----- .../TestInternalErrorReporter.cpp | 14 +++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/fsfw/internalerror/InternalErrorReporterIF.h b/src/fsfw/internalerror/InternalErrorReporterIF.h index 9eb8e7d7e..61bb52e7b 100644 --- a/src/fsfw/internalerror/InternalErrorReporterIF.h +++ b/src/fsfw/internalerror/InternalErrorReporterIF.h @@ -1,22 +1,34 @@ #ifndef INTERNALERRORREPORTERIF_H_ #define INTERNALERRORREPORTERIF_H_ +/** + * @brief Interface which is used to report internal errors like full message queues or stores. + * @details + * This interface smust be used for the InteralErrorReporter object. + * It should be used to indicate that there was a Problem with Queues or Stores. + * + * It can be used to report missing Telemetry which could not be sent due to a internal problem. + * + */ class InternalErrorReporterIF { public: virtual ~InternalErrorReporterIF() {} - /** - * Thread safe + * @brief Function to be called if a message queue could not be sent. + * @details OSAL Implementations should call this function to indicate that + * a message was lost. + * + * Implementations are required to be Thread safe */ virtual void queueMessageNotSent() = 0; - /** - * Thread safe + * @brief Function to be called if Telemetry could not be sent + * @details Implementations must be Thread safe */ virtual void lostTm() = 0; - /** - * Thread safe + * @brief Function to be called if a onboard storage is full + * @details Implementations must be Thread safe */ virtual void storeFull() = 0; }; diff --git a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp index a7f5e1092..92c3ff22d 100644 --- a/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp +++ b/tests/src/fsfw_tests/unit/internalerror/TestInternalErrorReporter.cpp @@ -98,6 +98,20 @@ TEST_CASE("Internal Error Reporter", "[TestInternalError]") { REQUIRE(dataset.tmHits.value == (lostTm + 1)); REQUIRE(dataset.storeHits.value == (storeHits + 1)); } + // Complete Coverage + internalErrorReporter->setDiagnosticPrintout(true); + internalErrorReporter->setMutexTimeout(MutexIF::TimeoutType::BLOCKING, 0); + { + // Message Queue Id + MessageQueueId_t id = internalErrorReporter->getCommandQueue(); + REQUIRE(id != MessageQueueIF::NO_QUEUE); + CommandMessage message; + sid_t sid(objects::INTERNAL_ERROR_REPORTER, InternalErrorDataset::ERROR_SET_ID); + HousekeepingMessage::setToggleReportingCommand(&message, sid, true, false); + result = hkQueue->sendMessage(id, &message); + REQUIRE(result == HasReturnvaluesIF::RETURN_OK); + internalErrorReporter->performOperation(0); + } } QueueFactory::instance()->deleteMessageQueue(testQueue); QueueFactory::instance()->deleteMessageQueue(hkQueue);