From b7f3eff74230fa89fc02cc28a368460b32cfb80d Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Fri, 18 Feb 2022 19:08:06 +0100 Subject: [PATCH 01/14] 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 68693c40..079a5548 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 e7088e2c..98ed2af3 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 98711139..a946b81c 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 fc73f7b5..a83e2f7f 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 00000000..d49ce006 --- /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 00000000..d394166e --- /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 00000000..dd5bca12 --- /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 232ffb94..365df379 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 02/14] 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 d394166e..1103037d 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 2f1b92300996c61a5e9ef8c2ba4e738eb477a0b2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:06:00 +0100 Subject: [PATCH 03/14] this module should not be needed anymore --- src/fsfw/controller/ControllerBase.h | 1 - src/fsfw/datapool/HkSwitchHelper.cpp | 67 ---------------------------- src/fsfw/datapool/HkSwitchHelper.h | 44 ------------------ 3 files changed, 112 deletions(-) delete mode 100644 src/fsfw/datapool/HkSwitchHelper.cpp delete mode 100644 src/fsfw/datapool/HkSwitchHelper.h diff --git a/src/fsfw/controller/ControllerBase.h b/src/fsfw/controller/ControllerBase.h index db75982c..7032f817 100644 --- a/src/fsfw/controller/ControllerBase.h +++ b/src/fsfw/controller/ControllerBase.h @@ -1,7 +1,6 @@ #ifndef FSFW_CONTROLLER_CONTROLLERBASE_H_ #define FSFW_CONTROLLER_CONTROLLERBASE_H_ -#include "fsfw/datapool/HkSwitchHelper.h" #include "fsfw/health/HasHealthIF.h" #include "fsfw/health/HealthHelper.h" #include "fsfw/modes/HasModesIF.h" diff --git a/src/fsfw/datapool/HkSwitchHelper.cpp b/src/fsfw/datapool/HkSwitchHelper.cpp deleted file mode 100644 index 7f6ffd17..00000000 --- a/src/fsfw/datapool/HkSwitchHelper.cpp +++ /dev/null @@ -1,67 +0,0 @@ -#include "fsfw/datapool/HkSwitchHelper.h" - -#include "fsfw/ipc/QueueFactory.h" - -HkSwitchHelper::HkSwitchHelper(EventReportingProxyIF* eventProxy) - : commandActionHelper(this), eventProxy(eventProxy) { - actionQueue = QueueFactory::instance()->createMessageQueue(); -} - -HkSwitchHelper::~HkSwitchHelper() { QueueFactory::instance()->deleteMessageQueue(actionQueue); } - -ReturnValue_t HkSwitchHelper::initialize() { - ReturnValue_t result = commandActionHelper.initialize(); - - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } - - return result; -} - -ReturnValue_t HkSwitchHelper::performOperation(uint8_t operationCode) { - CommandMessage command; - while (actionQueue->receiveMessage(&command) == HasReturnvaluesIF::RETURN_OK) { - ReturnValue_t result = commandActionHelper.handleReply(&command); - if (result == HasReturnvaluesIF::RETURN_OK) { - continue; - } - command.setToUnknownCommand(); - actionQueue->reply(&command); - } - - return HasReturnvaluesIF::RETURN_OK; -} - -void HkSwitchHelper::stepSuccessfulReceived(ActionId_t actionId, uint8_t step) {} - -void HkSwitchHelper::stepFailedReceived(ActionId_t actionId, uint8_t step, - ReturnValue_t returnCode) { - eventProxy->forwardEvent(SWITCHING_TM_FAILED, returnCode, actionId); -} - -void HkSwitchHelper::dataReceived(ActionId_t actionId, const uint8_t* data, uint32_t size) {} - -void HkSwitchHelper::completionSuccessfulReceived(ActionId_t actionId) {} - -void HkSwitchHelper::completionFailedReceived(ActionId_t actionId, ReturnValue_t returnCode) { - eventProxy->forwardEvent(SWITCHING_TM_FAILED, returnCode, actionId); -} - -ReturnValue_t HkSwitchHelper::switchHK(SerializeIF* sids, bool enable) { - // ActionId_t action = HKService::DISABLE_HK; - // if (enable) { - // action = HKService::ENABLE_HK; - // } - // - // ReturnValue_t result = commandActionHelper.commandAction( - // objects::PUS_HK_SERVICE, action, sids); - // - // if (result != HasReturnvaluesIF::RETURN_OK) { - // eventProxy->forwardEvent(SWITCHING_TM_FAILED, result); - // } - // return result; - return HasReturnvaluesIF::RETURN_OK; -} - -MessageQueueIF* HkSwitchHelper::getCommandQueuePtr() { return actionQueue; } diff --git a/src/fsfw/datapool/HkSwitchHelper.h b/src/fsfw/datapool/HkSwitchHelper.h deleted file mode 100644 index a0becd81..00000000 --- a/src/fsfw/datapool/HkSwitchHelper.h +++ /dev/null @@ -1,44 +0,0 @@ -#ifndef FRAMEWORK_DATAPOOL_HKSWITCHHELPER_H_ -#define FRAMEWORK_DATAPOOL_HKSWITCHHELPER_H_ - -#include "fsfw/action/CommandsActionsIF.h" -#include "fsfw/events/EventReportingProxyIF.h" -#include "fsfw/tasks/ExecutableObjectIF.h" - -// TODO this class violations separation between mission and framework -// but it is only a transitional solution until the Datapool is -// implemented decentrally - -class HkSwitchHelper : public ExecutableObjectIF, public CommandsActionsIF { - public: - static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::HK; - static const Event SWITCHING_TM_FAILED = - MAKE_EVENT(1, severity::LOW); //!< Commanding the HK Service failed, p1: error code, p2 - //!< action: 0 disable / 1 enable - - HkSwitchHelper(EventReportingProxyIF* eventProxy); - virtual ~HkSwitchHelper(); - - ReturnValue_t initialize(); - - virtual ReturnValue_t performOperation(uint8_t operationCode = 0); - - ReturnValue_t switchHK(SerializeIF* sids, bool enable); - - virtual void setTaskIF(PeriodicTaskIF* task_){}; - - protected: - virtual void stepSuccessfulReceived(ActionId_t actionId, uint8_t step); - virtual void stepFailedReceived(ActionId_t actionId, uint8_t step, ReturnValue_t returnCode); - virtual void dataReceived(ActionId_t actionId, const uint8_t* data, uint32_t size); - virtual void completionSuccessfulReceived(ActionId_t actionId); - virtual void completionFailedReceived(ActionId_t actionId, ReturnValue_t returnCode); - virtual MessageQueueIF* getCommandQueuePtr(); - - private: - CommandActionHelper commandActionHelper; - MessageQueueIF* actionQueue; - EventReportingProxyIF* eventProxy; -}; - -#endif /* FRAMEWORK_DATAPOOL_HKSWITCHHELPER_H_ */ From 6fb64f9ada241b1d0567d363ef4340c3f1bd7f3e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:11:54 +0100 Subject: [PATCH 04/14] removed source from cmakelists.txt --- src/fsfw/datapool/CMakeLists.txt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/fsfw/datapool/CMakeLists.txt b/src/fsfw/datapool/CMakeLists.txt index 0d53e1ba..be4606aa 100644 --- a/src/fsfw/datapool/CMakeLists.txt +++ b/src/fsfw/datapool/CMakeLists.txt @@ -1,6 +1,4 @@ -target_sources(${LIB_FSFW_NAME} - PRIVATE - HkSwitchHelper.cpp - PoolDataSetBase.cpp - PoolEntry.cpp +target_sources(${LIB_FSFW_NAME} PRIVATE + PoolDataSetBase.cpp + PoolEntry.cpp ) \ No newline at end of file From 025f79fcb46cb5200ac6310dadc33e411bf20c12 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:16:14 +0100 Subject: [PATCH 05/14] apply clang format --- src/fsfw/globalfunctions/CRC.cpp | 2 +- src/fsfw/osal/linux/PeriodicPosixTask.h | 7 ++++--- src/fsfw/osal/rtems/PeriodicTask.h | 4 ++-- src/fsfw/rmap/RMAP.h | 8 ++++---- src/fsfw/rmap/RMAPChannelIF.h | 12 ++++++------ 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/fsfw/globalfunctions/CRC.cpp b/src/fsfw/globalfunctions/CRC.cpp index 6b8140c5..033920d0 100644 --- a/src/fsfw/globalfunctions/CRC.cpp +++ b/src/fsfw/globalfunctions/CRC.cpp @@ -117,7 +117,7 @@ uint16_t CRC::crc16ccitt(uint8_t const input[], uint32_t length, uint16_t starti // { // if (xor_out[i] == true) // crc_value = crc_value + pow(2,(15 -i)); // reverse CrC result before - //Final XOR + // Final XOR // } // // crc_value = 0;// for debug mode diff --git a/src/fsfw/osal/linux/PeriodicPosixTask.h b/src/fsfw/osal/linux/PeriodicPosixTask.h index 3c9a3a0d..1c3a52c7 100644 --- a/src/fsfw/osal/linux/PeriodicPosixTask.h +++ b/src/fsfw/osal/linux/PeriodicPosixTask.h @@ -65,9 +65,10 @@ class PeriodicPosixTask : public PosixThread, public PeriodicTaskIF { /** * @brief The function containing the actual functionality of the task. * @details The method sets and starts - * the task's period, then enters a loop that is repeated indefinitely. Within the - * loop, all performOperation methods of the added objects are called. Afterwards the task will be - * blocked until the next period. On missing the deadline, the deadlineMissedFunction is executed. + * the task's period, then enters a loop that is repeated indefinitely. Within + * the loop, all performOperation methods of the added objects are called. Afterwards the task + * will be blocked until the next period. On missing the deadline, the deadlineMissedFunction is + * executed. */ virtual void taskFunctionality(void); /** diff --git a/src/fsfw/osal/rtems/PeriodicTask.h b/src/fsfw/osal/rtems/PeriodicTask.h index ff8617fc..119329f2 100644 --- a/src/fsfw/osal/rtems/PeriodicTask.h +++ b/src/fsfw/osal/rtems/PeriodicTask.h @@ -13,8 +13,8 @@ class ExecutableObjectIF; * @brief This class represents a specialized task for periodic activities of multiple objects. * * @details MultiObjectTask is an extension to ObjectTask in the way that it is able to execute - * multiple objects that implement the ExecutableObjectIF interface. The objects - * must be added prior to starting the task. + * multiple objects that implement the ExecutableObjectIF interface. The + * objects must be added prior to starting the task. * @author baetz * @ingroup task_handling */ diff --git a/src/fsfw/rmap/RMAP.h b/src/fsfw/rmap/RMAP.h index 42ee1ac5..7c654262 100644 --- a/src/fsfw/rmap/RMAP.h +++ b/src/fsfw/rmap/RMAP.h @@ -169,8 +169,8 @@ class RMAP : public HasReturnvaluesIF { * @param buffer the data to write * @param length length of data * @return - * - @c COMMAND_NULLPOINTER datalen was != 0 but data was == NULL in - * write command + * - @c COMMAND_NULLPOINTER datalen was != 0 but data was == NULL + * in write command * - return codes of RMAPChannelIF::sendCommand() */ static ReturnValue_t sendWriteCommand(RMAPCookie *cookie, const uint8_t *buffer, size_t length); @@ -205,8 +205,8 @@ class RMAP : public HasReturnvaluesIF { * @param cookie to cookie to read from * @param expLength the expected maximum length of the reply * @return - * - @c COMMAND_NULLPOINTER datalen was != 0 but data was == NULL in - * write command, or nullpointer in read command + * - @c COMMAND_NULLPOINTER datalen was != 0 but data was == NULL + * in write command, or nullpointer in read command * - return codes of RMAPChannelIF::sendCommand() */ static ReturnValue_t sendReadCommand(RMAPCookie *cookie, uint32_t expLength); diff --git a/src/fsfw/rmap/RMAPChannelIF.h b/src/fsfw/rmap/RMAPChannelIF.h index 20dfd5f8..7dab07c1 100644 --- a/src/fsfw/rmap/RMAPChannelIF.h +++ b/src/fsfw/rmap/RMAPChannelIF.h @@ -75,10 +75,10 @@ class RMAPChannelIF { * - @c RETURN_OK * - @c COMMAND_NO_DESCRIPTORS_AVAILABLE no descriptors available for sending * command; command was not sent - * - @c COMMAND_BUFFER_FULL no receiver buffer available for expected len; - * command was not sent - * - @c COMMAND_TOO_BIG the data that was to be sent was too long for the hw - * to handle (write command) or the expected len was bigger than maximal expected len (read + * - @c COMMAND_BUFFER_FULL no receiver buffer available for expected + * len; command was not sent + * - @c COMMAND_TOO_BIG the data that was to be sent was too long for the + * hw to handle (write command) or the expected len was bigger than maximal expected len (read * command) command was not sent * - @c COMMAND_CHANNEL_DEACTIVATED the channel has no port set * - @c NOT_SUPPORTED if you dont feel like @@ -97,8 +97,8 @@ class RMAPChannelIF { * - @c REPLY_NO_REPLY no reply was received * - @c REPLY_NOT_SENT command was not sent, implies no reply * - @c REPLY_NOT_YET_SENT command is still waiting to be sent - * - @c WRITE_REPLY_INTERFACE_BUSY Interface is busy (transmission buffer still - * being processed) + * - @c WRITE_REPLY_INTERFACE_BUSY Interface is busy (transmission buffer + * still being processed) * - @c WRITE_REPLY_TRANSMISSION_ERROR Interface encountered errors during last * operation, data could not be processed. (transmission error) * - @c WRITE_REPLY_INVALID_DATA Invalid data (amount / value) From 6739890d5368d1e6ecf3e1cb743111113fb384d8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:19:49 +0100 Subject: [PATCH 06/14] add i2c wiretapping option --- hal/src/fsfw_hal/linux/i2c/I2cComIF.cpp | 26 +++++++++++++++++++------ src/fsfw/FSFW.h.in | 5 +++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/hal/src/fsfw_hal/linux/i2c/I2cComIF.cpp b/hal/src/fsfw_hal/linux/i2c/I2cComIF.cpp index dc23542d..4f53dc1f 100644 --- a/hal/src/fsfw_hal/linux/i2c/I2cComIF.cpp +++ b/hal/src/fsfw_hal/linux/i2c/I2cComIF.cpp @@ -1,4 +1,13 @@ -#include "fsfw_hal/linux/i2c/I2cComIF.h" +#include "I2cComIF.h" + +#include "fsfw/FSFW.h" +#include "fsfw/serviceinterface.h" +#include "fsfw_hal/linux/UnixFileGuard.h" +#include "fsfw_hal/linux/utility.h" + +#if FSFW_HAL_I2C_WIRETAPPING == 1 +#include "fsfw/globalfunctions/arrayprinter.h" +#endif #include #include @@ -8,11 +17,6 @@ #include -#include "fsfw/FSFW.h" -#include "fsfw/serviceinterface.h" -#include "fsfw_hal/linux/UnixFileGuard.h" -#include "fsfw_hal/linux/utility.h" - I2cComIF::I2cComIF(object_id_t objectId) : SystemObject(objectId) {} I2cComIF::~I2cComIF() {} @@ -112,6 +116,11 @@ ReturnValue_t I2cComIF::sendMessage(CookieIF* cookie, const uint8_t* sendData, s #endif return HasReturnvaluesIF::RETURN_FAILED; } + +#if FSFW_HAL_I2C_WIRETAPPING == 1 + sif::info << "Sent I2C data to bus " << deviceFile << ":" << std::endl; + arrayprinter::print(sendData, sendLen); +#endif return HasReturnvaluesIF::RETURN_OK; } @@ -176,6 +185,11 @@ ReturnValue_t I2cComIF::requestReceiveMessage(CookieIF* cookie, size_t requestLe return HasReturnvaluesIF::RETURN_FAILED; } +#if FSFW_HAL_I2C_WIRETAPPING == 1 + sif::info << "I2C read bytes from bus " << deviceFile << ":" << std::endl; + arrayprinter::print(replyBuffer, requestLen); +#endif + i2cDeviceMapIter->second.replyLen = requestLen; return HasReturnvaluesIF::RETURN_OK; } diff --git a/src/fsfw/FSFW.h.in b/src/fsfw/FSFW.h.in index 88ad10cf..28e0c0bf 100644 --- a/src/fsfw/FSFW.h.in +++ b/src/fsfw/FSFW.h.in @@ -69,4 +69,9 @@ #define FSFW_HAL_LIS3MDL_MGM_DEBUG 0 #endif /* FSFW_HAL_LIS3MDL_MGM_DEBUG */ +// Can be used for low-level debugging of the I2C bus +#ifndef FSFW_HAL_I2C_WIRETAPPING +#define FSFW_HAL_I2C_WIRETAPPING 0 +#endif + #endif /* FSFW_FSFW_H_ */ From d119479c0a793708d4858e2bef6674814c5600ac Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:22:20 +0100 Subject: [PATCH 07/14] update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 335c0f7b..e061b6f8 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] + +## Additions + +- Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 + # [v4.0.0] ## Additions From 68ace0b74a03dc772457c4d8332f15cf167f8b5d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 11:23:29 +0100 Subject: [PATCH 08/14] update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 335c0f7b..62882082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +# [v5.0.0] + +## Removed + +- Removed the `HkSwitchHelper`. This module should not be needed anymore, now that the local + datapools have been implemented + # [v4.0.0] ## Additions From a50b52df5137ecae86f0c2a8db21210ec507cece Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 22 Feb 2022 13:37:28 +0100 Subject: [PATCH 09/14] 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 9057de31..cab13207 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 db66b671..62151164 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 1103037d..1e63a57b 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 365df379..11c0739b 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 10/14] 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 98ed2af3..08d37feb 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 62151164..d328fb82 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 1e63a57b..a7f5e109 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 dd5bca12..ebd9a2e7 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 11/14] 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 cab13207..acfa23c5 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 fdc8a3d4f761dc5887e97542caf65d03645d31c0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 22 Feb 2022 14:02:03 +0100 Subject: [PATCH 12/14] display run commands in helper script --- scripts/helper.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/scripts/helper.py b/scripts/helper.py index 68693c40..09599d3d 100755 --- a/scripts/helper.py +++ b/scripts/helper.py @@ -97,11 +97,11 @@ def handle_docs_type(args, build_dir_list: list): build_directory = determine_build_dir(build_dir_list) os.chdir(build_directory) if args.build: - os.system("cmake --build . -j") + cmd_runner("cmake --build . -j") if args.open: if not os.path.isfile("docs/sphinx/index.html"): # try again.. - os.system("cmake --build . -j") + cmd_runner("cmake --build . -j") if not os.path.isfile("docs/sphinx/index.html"): print( "No Sphinx documentation file detected. " @@ -147,21 +147,21 @@ def handle_tests_type(args, build_dir_list: list): # 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") + cmd_runner("valgrind --leak-check=full ./fsfw-tests") os.chdir("..") def create_tests_build_cfg(): os.mkdir(UNITTEST_FOLDER_NAME) os.chdir(UNITTEST_FOLDER_NAME) - os.system("cmake -DFSFW_OSAL=host -DFSFW_BUILD_UNITTESTS=ON ..") + cmd_runner("cmake -DFSFW_OSAL=host -DFSFW_BUILD_UNITTESTS=ON ..") os.chdir("..") def create_docs_build_cfg(): os.mkdir(DOCS_FOLDER_NAME) os.chdir(DOCS_FOLDER_NAME) - os.system("cmake -DFSFW_OSAL=host -DFSFW_BUILD_DOCS=ON ..") + cmd_runner("cmake -DFSFW_OSAL=host -DFSFW_BUILD_DOCS=ON ..") os.chdir("..") @@ -184,7 +184,7 @@ def check_for_cmake_build_dir(build_dir_list: list) -> list: def perform_lcov_operation(directory: str, chdir: bool): if chdir: os.chdir(directory) - os.system("cmake --build . -- fsfw-tests_coverage -j") + cmd_runner("cmake --build . -- fsfw-tests_coverage -j") def determine_build_dir(build_dir_list: List[str]): @@ -206,5 +206,10 @@ def determine_build_dir(build_dir_list: List[str]): return build_directory +def cmd_runner(cmd: str): + print(f"Executing command: {cmd}") + os.system(cmd) + + if __name__ == "__main__": main() From 2cb254a556b3f5da550fec9e479b56de6834274b Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Wed, 23 Feb 2022 11:53:48 +0100 Subject: [PATCH 13/14] 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 08d37feb..fa16ec3f 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 a946b81c..549be4bd 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 14/14] 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 9eb8e7d7..61bb52e7 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 a7f5e109..92c3ff22 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);