From 7e7b3bbbc9eccfb8610e712116c1398ce877b23e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:06:34 +0100 Subject: [PATCH 01/30] time stamper empty ctor --- src/fsfw/timemanager/CdsShortTimeStamper.cpp | 2 ++ src/fsfw/timemanager/CdsShortTimeStamper.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/fsfw/timemanager/CdsShortTimeStamper.cpp b/src/fsfw/timemanager/CdsShortTimeStamper.cpp index 8fb33f12..aa259029 100644 --- a/src/fsfw/timemanager/CdsShortTimeStamper.cpp +++ b/src/fsfw/timemanager/CdsShortTimeStamper.cpp @@ -4,6 +4,8 @@ #include "fsfw/timemanager/Clock.h" +CdsShortTimeStamper::CdsShortTimeStamper() : SystemObject(0, false) {} + CdsShortTimeStamper::CdsShortTimeStamper(object_id_t objectId) : SystemObject(objectId) {} ReturnValue_t CdsShortTimeStamper::serialize(uint8_t **buffer, size_t *size, size_t maxSize, diff --git a/src/fsfw/timemanager/CdsShortTimeStamper.h b/src/fsfw/timemanager/CdsShortTimeStamper.h index 244d54b6..a16a07b6 100644 --- a/src/fsfw/timemanager/CdsShortTimeStamper.h +++ b/src/fsfw/timemanager/CdsShortTimeStamper.h @@ -18,6 +18,7 @@ class CdsShortTimeStamper : public TimeWriterIF, public TimeReaderIF, public SystemObject { public: static constexpr size_t TIMESTAMP_LEN = 7; + CdsShortTimeStamper(); /** * @brief Default constructor which also registers the time stamper as a * system object so it can be found with the #objectManager. From b22d4393002f6d5a72bfc24c7ca6be98a1cccfe1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:10:11 +0100 Subject: [PATCH 02/30] bump changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c5cf023..0c70147e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Added +- Empty constructor for `CdsShortTimeStamper` which does not do an object manager registration. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/730 - `TcpTmTcServer`: Allow setting the `SO_REUSEADDR` and `SO_REUSEPORT` option on the TCP server. CTOR prototype has changed and expects an explicit TCP configuration struct to be passed. From c8e065a713fcb7d10efa1a13a770af4ce76e9404 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:36:42 +0100 Subject: [PATCH 03/30] comment tweak to event parser can read everything --- src/fsfw/health/HasHealthIF.h | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/fsfw/health/HasHealthIF.h b/src/fsfw/health/HasHealthIF.h index 77a1c12f..16666bbc 100644 --- a/src/fsfw/health/HasHealthIF.h +++ b/src/fsfw/health/HasHealthIF.h @@ -16,26 +16,24 @@ class HasHealthIF { }; static const uint8_t INTERFACE_ID = CLASS_ID::HAS_HEALTH_IF; - static const ReturnValue_t OBJECT_NOT_HEALTHY = MAKE_RETURN_CODE(1); - static const ReturnValue_t INVALID_HEALTH_STATE = MAKE_RETURN_CODE(2); + static constexpr ReturnValue_t OBJECT_NOT_HEALTHY = returnvalue::makeCode(INTERFACE_ID, 1); + static constexpr ReturnValue_t INVALID_HEALTH_STATE = returnvalue::makeCode(INTERFACE_ID, 2); + static constexpr ReturnValue_t IS_EXTERNALLY_CONTROLLED = returnvalue::makeCode(INTERFACE_ID, 3); static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::SYSTEM_MANAGER_1; + //! P1: New Health, P2: Old Health static const Event HEALTH_INFO = MAKE_EVENT(6, severity::INFO); static const Event CHILD_CHANGED_HEALTH = MAKE_EVENT(7, severity::INFO); static const Event CHILD_PROBLEMS = MAKE_EVENT(8, severity::LOW); - static const Event OVERWRITING_HEALTH = - MAKE_EVENT(9, severity::LOW); //!< Assembly overwrites health information of children to keep - //!< satellite alive. - static const Event TRYING_RECOVERY = - MAKE_EVENT(10, severity::MEDIUM); //!< Someone starts a recovery of a component (typically - //!< power-cycle). No parameters. - static const Event RECOVERY_STEP = - MAKE_EVENT(11, severity::MEDIUM); //!< Recovery is ongoing. Comes twice during recovery. P1: - //!< 0 for the first, 1 for the second event. P2: 0 - static const Event RECOVERY_DONE = MAKE_EVENT( - 12, - severity::MEDIUM); //!< Recovery was completed. Not necessarily successful. No parameters. - + //! Assembly overwrites health information of children to keep satellite alive. + static const Event OVERWRITING_HEALTH = MAKE_EVENT(9, severity::LOW); + //! Someone starts a recovery of a component (typically power-cycle). No parameters. + static const Event TRYING_RECOVERY = MAKE_EVENT(10, severity::MEDIUM); + //! Recovery is ongoing. Comes twice during recovery. + //! P1: 0 for the first, 1 for the second event. P2: 0 + static const Event RECOVERY_STEP = MAKE_EVENT(11, severity::MEDIUM); + //! Recovery was completed. Not necessarily successful. No parameters. + static const Event RECOVERY_DONE = MAKE_EVENT(12, severity::MEDIUM); virtual ~HasHealthIF() {} virtual MessageQueueId_t getCommandQueue() const = 0; From c2e6a22deca2033ed79276f4dceb515aa87f4bd4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:39:43 +0100 Subject: [PATCH 04/30] important bugfix for RM3100 --- src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp index ce215a64..4becd420 100644 --- a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp +++ b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp @@ -169,7 +169,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const case (RM3100::CONFIGURE_CYCLE_COUNT): case (RM3100::CONFIGURE_TMRC): { // We can only check whether write was successful with read operation - if (mode == _MODE_START_UP) { + if (getMode() == _MODE_START_UP) { commandExecuted = true; } break; @@ -192,7 +192,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const if (packet[1] == tmrcRegValue) { commandExecuted = true; // Reading TMRC was commanded. Trigger event to inform ground - if (mode != _MODE_START_UP) { + if (getMode() != _MODE_START_UP) { triggerEvent(tmrcSet, tmrcRegValue, 0); } } else { @@ -211,7 +211,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const return DeviceHandlerIF::DEVICE_REPLY_INVALID; } // Reading TMRC was commanded. Trigger event to inform ground - if (mode != _MODE_START_UP) { + if (getMode() != _MODE_START_UP) { uint32_t eventParam1 = (cycleCountX << 16) | cycleCountY; triggerEvent(cycleCountersSet, eventParam1, cycleCountZ); } @@ -325,7 +325,7 @@ ReturnValue_t MgmRM3100Handler::handleDataReadout(const uint8_t *packet) { // trickery here to calculate the raw values first int32_t fieldStrengthRawX = ((packet[1] << 24) | (packet[2] << 16) | (packet[3] << 8)) >> 8; int32_t fieldStrengthRawY = ((packet[4] << 24) | (packet[5] << 16) | (packet[6] << 8)) >> 8; - int32_t fieldStrengthRawZ = ((packet[7] << 24) | (packet[8] << 16) | (packet[3] << 8)) >> 8; + int32_t fieldStrengthRawZ = ((packet[7] << 24) | (packet[8] << 16) | (packet[9] << 8)) >> 8; // Now scale to physical value in microtesla float fieldStrengthX = fieldStrengthRawX * scaleFactorX; From 5adf89b9110fac3367735419a2aa82fb434f0297 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:41:42 +0100 Subject: [PATCH 05/30] changelog update --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecc585d6..96b2df80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - HAL MGM3100 Handler: Use axis specific gain/scaling factors. Previously, only the X scaling factor was used. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/724 +- Bugfix for RM3100 MGM sensors. Z value was previously calculated + with bytes of the X value. - DHB `setNormalDatapoolEntriesInvalid`: The default implementation did not set the validity to false correctly because the `read` and `write` calls were missing. - PUS TMTC creator module: Sequence flags were set to continuation segment (0b00) instead From f39054edd4a90833903190af7d9400ec5a2335b1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:45:29 +0100 Subject: [PATCH 06/30] introduce warning switch --- src/fsfw/tmtcservices/TmTcBridge.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index f22d70d6..9ff58766 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -16,7 +16,9 @@ TmTcBridge::TmTcBridge(const char* name, object_id_t objectId, object_id_t tcDes tcDestination(tcDestination) { - tmTcReceptionQueue = QueueFactory::instance()->createMessageQueue(TMTC_RECEPTION_QUEUE_DEPTH); + auto mqArgs = MqArgs(objectId, static_cast(this)); + tmTcReceptionQueue = QueueFactory::instance()->createMessageQueue( + TMTC_RECEPTION_QUEUE_DEPTH, MessageQueueMessage::MAX_MESSAGE_SIZE, &mqArgs); } TmTcBridge::~TmTcBridge() { QueueFactory::instance()->deleteMessageQueue(tmTcReceptionQueue); } @@ -35,7 +37,7 @@ ReturnValue_t TmTcBridge::setNumberOfSentPacketsPerCycle(uint8_t sentPacketsPerC } } -ReturnValue_t TmTcBridge::setMaxNumberOfPacketsStored(uint8_t maxNumberOfPacketsStored) { +ReturnValue_t TmTcBridge::setMaxNumberOfPacketsStored(unsigned int maxNumberOfPacketsStored) { if (maxNumberOfPacketsStored <= LIMIT_DOWNLINK_PACKETS_STORED) { this->maxNumberOfPacketsStored = maxNumberOfPacketsStored; return returnvalue::OK; @@ -171,15 +173,18 @@ ReturnValue_t TmTcBridge::storeDownlinkData(TmTcMessage* message) { } if (tmFifo->full()) { + if (warningSwitch) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "TmTcBridge::storeDownlinkData: TM downlink max. number " - "of stored packet IDs reached!" - << std::endl; + sif::warning << "TmTcBridge::storeDownlinkData: TM downlink max. number " + "of stored packet IDs reached!" + << std::endl; #else - sif::printWarning( - "TmTcBridge::storeDownlinkData: TM downlink max. number " - "of stored packet IDs reached!\n"); + sif::printWarning( + "TmTcBridge::storeDownlinkData: TM downlink max. number " + "of stored packet IDs reached!\n"); #endif + warningSwitch = false; + } if (overwriteOld) { tmFifo->retrieve(&storeId); tmStore->deleteData(storeId); @@ -221,6 +226,7 @@ ReturnValue_t TmTcBridge::handleStoredTm() { packetSentCounter++; if (tmFifo->empty()) { + warningSwitch = true; tmStored = false; } tmStore->deleteData(storeId); From 40a9e12416ce0d174e8402d97f155dd603fc670b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:47:40 +0100 Subject: [PATCH 07/30] 1000 is a bit much --- src/fsfw/tmtcservices/TmTcBridge.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fsfw/tmtcservices/TmTcBridge.h b/src/fsfw/tmtcservices/TmTcBridge.h index ed4d254e..3df3419c 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.h +++ b/src/fsfw/tmtcservices/TmTcBridge.h @@ -17,7 +17,7 @@ class TmTcBridge : public AcceptsTelemetryIF, public: static constexpr uint8_t TMTC_RECEPTION_QUEUE_DEPTH = 20; static constexpr uint8_t LIMIT_STORED_DATA_SENT_PER_CYCLE = 15; - static constexpr uint8_t LIMIT_DOWNLINK_PACKETS_STORED = 200; + static constexpr unsigned int LIMIT_DOWNLINK_PACKETS_STORED = 500; static constexpr uint8_t DEFAULT_STORED_DATA_SENT_PER_CYCLE = 5; static constexpr uint8_t DEFAULT_DOWNLINK_PACKETS_STORED = 10; @@ -42,7 +42,7 @@ class TmTcBridge : public AcceptsTelemetryIF, * @return -@c returnvalue::OK if value was set successfully * -@c returnvalue::FAILED otherwise, stored value stays the same */ - ReturnValue_t setMaxNumberOfPacketsStored(uint8_t maxNumberOfPacketsStored); + ReturnValue_t setMaxNumberOfPacketsStored(unsigned int maxNumberOfPacketsStored); /** * This will set up the bridge to overwrite old data in the FIFO. @@ -91,6 +91,7 @@ class TmTcBridge : public AcceptsTelemetryIF, //! by default, so telemetry will be handled immediately. bool communicationLinkUp = true; bool tmStored = false; + bool warningSwitch = true; bool overwriteOld = true; uint8_t packetSentCounter = 0; @@ -152,7 +153,7 @@ class TmTcBridge : public AcceptsTelemetryIF, */ DynamicFIFO* tmFifo = nullptr; uint8_t sentPacketsPerCycle = DEFAULT_STORED_DATA_SENT_PER_CYCLE; - uint8_t maxNumberOfPacketsStored = DEFAULT_DOWNLINK_PACKETS_STORED; + unsigned int maxNumberOfPacketsStored = DEFAULT_DOWNLINK_PACKETS_STORED; }; #endif /* FSFW_TMTCSERVICES_TMTCBRIDGE_H_ */ From 134d908f2651648c9f9f3d053f6a1961ebb5d0e5 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 7 Feb 2023 12:52:18 +0100 Subject: [PATCH 08/30] that stuff is not in upstream yet.. --- src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp index 4becd420..307d0a55 100644 --- a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp +++ b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp @@ -169,7 +169,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const case (RM3100::CONFIGURE_CYCLE_COUNT): case (RM3100::CONFIGURE_TMRC): { // We can only check whether write was successful with read operation - if (getMode() == _MODE_START_UP) { + if (mode == _MODE_START_UP) { commandExecuted = true; } break; @@ -192,7 +192,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const if (packet[1] == tmrcRegValue) { commandExecuted = true; // Reading TMRC was commanded. Trigger event to inform ground - if (getMode() != _MODE_START_UP) { + if (mode != _MODE_START_UP) { triggerEvent(tmrcSet, tmrcRegValue, 0); } } else { @@ -211,7 +211,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const return DeviceHandlerIF::DEVICE_REPLY_INVALID; } // Reading TMRC was commanded. Trigger event to inform ground - if (getMode() != _MODE_START_UP) { + if (mode != _MODE_START_UP) { uint32_t eventParam1 = (cycleCountX << 16) | cycleCountY; triggerEvent(cycleCountersSet, eventParam1, cycleCountZ); } From 6f05d6b7b0ee0e6bc86986ec55491d42cde1cf93 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 8 Feb 2023 20:38:32 +0100 Subject: [PATCH 09/30] possiible leak fixes --- src/fsfw/tmtcservices/TmTcBridge.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index 9ff58766..ba851a85 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -145,13 +145,17 @@ ReturnValue_t TmTcBridge::handleTmQueue() { #endif /* FSFW_VERBOSE_LEVEL >= 3 */ if (communicationLinkUp == false or packetSentCounter >= sentPacketsPerCycle) { - storeDownlinkData(&message); + ReturnValue_t result = storeDownlinkData(&message); + if (result != returnvalue::OK) { + tmStore->deleteData(message.getStorageId()); + } continue; } result = tmStore->getData(message.getStorageId(), &data, &size); if (result != returnvalue::OK) { status = result; + tmStore->deleteData(message.getStorageId()); continue; } @@ -159,9 +163,9 @@ ReturnValue_t TmTcBridge::handleTmQueue() { if (result != returnvalue::OK) { status = result; } else { - tmStore->deleteData(message.getStorageId()); packetSentCounter++; } + tmStore->deleteData(message.getStorageId()); } return status; } From 1fffcc2229c3cc2fea52c9c3e6829ed99a70e7f1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 8 Feb 2023 20:38:32 +0100 Subject: [PATCH 10/30] possiible leak fixes --- src/fsfw/tmtcservices/TmTcBridge.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fsfw/tmtcservices/TmTcBridge.cpp b/src/fsfw/tmtcservices/TmTcBridge.cpp index f22d70d6..bed2c911 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.cpp +++ b/src/fsfw/tmtcservices/TmTcBridge.cpp @@ -143,13 +143,17 @@ ReturnValue_t TmTcBridge::handleTmQueue() { #endif /* FSFW_VERBOSE_LEVEL >= 3 */ if (communicationLinkUp == false or packetSentCounter >= sentPacketsPerCycle) { - storeDownlinkData(&message); + ReturnValue_t result = storeDownlinkData(&message); + if (result != returnvalue::OK) { + tmStore->deleteData(message.getStorageId()); + } continue; } result = tmStore->getData(message.getStorageId(), &data, &size); if (result != returnvalue::OK) { status = result; + tmStore->deleteData(message.getStorageId()); continue; } @@ -157,9 +161,9 @@ ReturnValue_t TmTcBridge::handleTmQueue() { if (result != returnvalue::OK) { status = result; } else { - tmStore->deleteData(message.getStorageId()); packetSentCounter++; } + tmStore->deleteData(message.getStorageId()); } return status; } From 000df85556bb2bfc4e648f1e0324f71e240afd90 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 8 Feb 2023 21:24:00 +0100 Subject: [PATCH 11/30] bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecc585d6..add39a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixes +- Memory leak fixes for the TCP/IP TMTC bridge. - `Service9TimeManagement`: Fix the time dump at the `SET_TIME` subservice: Include clock timeval seconds instead of uptime. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/726 From f1b0ca7cffd53ef86ec94ebc6db084ffcee8c8fd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 8 Feb 2023 21:26:37 +0100 Subject: [PATCH 12/30] add PR link --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index add39a3f..c7f0ce52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixes - Memory leak fixes for the TCP/IP TMTC bridge. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/737 - `Service9TimeManagement`: Fix the time dump at the `SET_TIME` subservice: Include clock timeval seconds instead of uptime. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/726 From 8c712441ab659d33663908e694216d437e5921d6 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Thu, 9 Feb 2023 11:34:58 +0100 Subject: [PATCH 13/30] Making fetch Catch2 quiet as well. --- CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9025537a..33d55351 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -153,12 +153,12 @@ if(FSFW_BUILD_TESTS) "${MSG_PREFIX} Building the FSFW unittests in addition to the static library" ) # Check whether the user has already installed Catch2 first - find_package(Catch2 ${FSFW_CATCH2_LIB_MAJOR_VERSION}) + find_package(Catch2 ${FSFW_CATCH2_LIB_MAJOR_VERSION} QUIET) # Not installed, so use FetchContent to download and provide Catch2 if(NOT Catch2_FOUND) message( STATUS - "${MSG_PREFIX} Catch2 installation not found. Downloading Catch2 library with FetchContent" + "${MSG_PREFIX} Catch2 installation not found. Downloading Catch2 library with FetchContent." ) include(FetchContent) @@ -201,8 +201,8 @@ find_package(${FSFW_ETL_LIB_NAME} ${FSFW_ETL_LIB_MAJOR_VERSION} CONFIG QUIET) if(NOT ${FSFW_ETL_LIB_NAME}_FOUND) message( STATUS - "${MSG_PREFIX} No ETL installation was found with find_package. Installing and providing " - "etl with FindPackage") + "${MSG_PREFIX} ETL installation not found. Downloading ETL with FetchContent." + ) include(FetchContent) FetchContent_Declare( From 010509efb442bf0d075b280afb5a74d8c84d1cb3 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Thu, 9 Feb 2023 13:50:16 +0100 Subject: [PATCH 14/30] removed unneeded find_package parameter for etl --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 33d55351..56601aaa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -196,7 +196,7 @@ message( ) # Check whether the user has already installed ETL first -find_package(${FSFW_ETL_LIB_NAME} ${FSFW_ETL_LIB_MAJOR_VERSION} CONFIG QUIET) +find_package(${FSFW_ETL_LIB_NAME} ${FSFW_ETL_LIB_MAJOR_VERSION} QUIET) # Not installed, so use FetchContent to download and provide etl if(NOT ${FSFW_ETL_LIB_NAME}_FOUND) message( From 3a2393885f6228502737adb50923e73d99756a42 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Thu, 9 Feb 2023 15:44:39 +0100 Subject: [PATCH 15/30] more style --- src/fsfw/pus/CService200ModeCommanding.cpp | 20 ++++++++------------ src/fsfw/pus/CService200ModeCommanding.h | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/fsfw/pus/CService200ModeCommanding.cpp b/src/fsfw/pus/CService200ModeCommanding.cpp index a82e7f5c..15745a37 100644 --- a/src/fsfw/pus/CService200ModeCommanding.cpp +++ b/src/fsfw/pus/CService200ModeCommanding.cpp @@ -19,7 +19,7 @@ ReturnValue_t CService200ModeCommanding::isValidSubservice(uint8_t subservice) { switch (subservice) { case (Subservice::COMMAND_MODE_COMMAND): case (Subservice::COMMAND_MODE_READ): - case (Subservice::COMMAND_MODE_ANNCOUNCE): + case (Subservice::COMMAND_MODE_ANNOUNCE): return returnvalue::OK; default: return AcceptsTelecommandsIF::INVALID_SUBSERVICE; @@ -53,6 +53,7 @@ ReturnValue_t CService200ModeCommanding::checkInterfaceAndAcquireMessageQueue( ReturnValue_t CService200ModeCommanding::prepareCommand(CommandMessage *message, uint8_t subservice, const uint8_t *tcData, size_t tcDataLen, uint32_t *state, object_id_t objectId) { + bool recursive = false; switch (subservice) { case (Subservice::COMMAND_MODE_COMMAND): { ModePacket modeCommandPacket; @@ -66,22 +67,17 @@ ReturnValue_t CService200ModeCommanding::prepareCommand(CommandMessage *message, modeCommandPacket.getMode(), modeCommandPacket.getSubmode()); return returnvalue::OK; } - case (Subservice::COMMAND_MODE_ANNCOUNCE): - case (Subservice::COMMAND_MODE_ANNOUNCE_RECURSIVELY): { - bool recursive = true; - if (subservice == Subservice::COMMAND_MODE_ANNCOUNCE) { - recursive = false; - } + case (Subservice::COMMAND_MODE_ANNOUNCE_RECURSIVELY): + recursive = true; + [[fallthrough]]; + case (Subservice::COMMAND_MODE_ANNOUNCE): ModeMessage::setModeAnnounceMessage(*message, recursive); return EXECUTION_COMPLETE; - } - case (Subservice::COMMAND_MODE_READ): { + case (Subservice::COMMAND_MODE_READ): ModeMessage::setModeReadMessage(*message); return returnvalue::OK; - } - default: { + default: return CommandingServiceBase::INVALID_SUBSERVICE; - } } } diff --git a/src/fsfw/pus/CService200ModeCommanding.h b/src/fsfw/pus/CService200ModeCommanding.h index 830e5950..cf2baf7e 100644 --- a/src/fsfw/pus/CService200ModeCommanding.h +++ b/src/fsfw/pus/CService200ModeCommanding.h @@ -52,7 +52,7 @@ class CService200ModeCommanding : public CommandingServiceBase { COMMAND_MODE_READ = 3, //!< [EXPORT] : [COMMAND] Trigger an ModeInfo Event. //! This command does NOT have a reply - COMMAND_MODE_ANNCOUNCE = 4, + COMMAND_MODE_ANNOUNCE = 4, //!< [EXPORT] : [COMMAND] Trigger a ModeInfo Event and to send this //! command to every child. This command does NOT have a reply. COMMAND_MODE_ANNOUNCE_RECURSIVELY = 5, From b27998585912a7a2cfa0b08dae4260f0dcd8cdd3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 16:42:38 +0100 Subject: [PATCH 16/30] refactor DHB: Bugfix for thermal module --- src/fsfw/devicehandlers/DeviceHandlerBase.cpp | 14 ++++++-------- src/fsfw/devicehandlers/DeviceHandlerBase.h | 8 ++++---- src/fsfw/devicehandlers/DeviceHandlerIF.h | 6 ++++++ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index b2505344..acb9e1d2 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -58,12 +58,7 @@ void DeviceHandlerBase::setHkDestination(object_id_t hkDestination) { this->hkDestination = hkDestination; } -void DeviceHandlerBase::setThermalStateRequestPoolIds(lp_id_t thermalStatePoolId, - lp_id_t heaterRequestPoolId, - uint32_t thermalSetId) { - thermalSet = - new DeviceHandlerThermalSet(this, thermalSetId, thermalStatePoolId, heaterRequestPoolId); -} +void DeviceHandlerBase::setUpThermalModule(ThermalStateCfg cfg) { this->thermalStateCfg = cfg; } DeviceHandlerBase::~DeviceHandlerBase() { if (comCookie != nullptr) { @@ -1494,9 +1489,12 @@ ReturnValue_t DeviceHandlerBase::initializeAfterTaskCreation() { } this->poolManager.initializeAfterTaskCreation(); - if (setStartupImmediately) { - startTransition(MODE_ON, getInitialSubmode()); + if (thermalStateCfg.has_value()) { + ThermalStateCfg& val = thermalStateCfg.value(); + thermalSet = new DeviceHandlerThermalSet(this, val.thermalSetId, val.thermalStatePoolId, + val.thermalRequestPoolId); } + startTransition(MODE_ON, getInitialSubmode()); return returnvalue::OK; } diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.h b/src/fsfw/devicehandlers/DeviceHandlerBase.h index b06815d1..b96506f5 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.h +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.h @@ -2,6 +2,7 @@ #define FSFW_DEVICEHANDLERS_DEVICEHANDLERBASE_H_ #include +#include #include "DeviceCommunicationIF.h" #include "DeviceHandlerFailureIsolation.h" @@ -166,10 +167,7 @@ class DeviceHandlerBase : public DeviceHandlerIF, * @param thermalStatePoolId * @param thermalRequestPoolId */ - void setThermalStateRequestPoolIds( - lp_id_t thermalStatePoolId = DeviceHandlerIF::DEFAULT_THERMAL_STATE_POOL_ID, - lp_id_t thermalRequestPoolId = DeviceHandlerIF::DEFAULT_THERMAL_HEATING_REQUEST_POOL_ID, - uint32_t thermalSetId = DeviceHandlerIF::DEFAULT_THERMAL_SET_ID); + void setUpThermalModule(ThermalStateCfg cfg); ReturnValue_t connectModeTreeParent(HasModeTreeChildrenIF &parent) override; ModeTreeChildIF &getModeTreeChildIF() override; @@ -931,6 +929,8 @@ class DeviceHandlerBase : public DeviceHandlerIF, //! Object which may be the root cause of an identified fault. static object_id_t defaultFdirParentId; + std::optional thermalStateCfg; + /** * @brief Send a reply to a received device handler command. * diff --git a/src/fsfw/devicehandlers/DeviceHandlerIF.h b/src/fsfw/devicehandlers/DeviceHandlerIF.h index 474c3a75..5e1fdd2f 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerIF.h +++ b/src/fsfw/devicehandlers/DeviceHandlerIF.h @@ -136,4 +136,10 @@ class DeviceHandlerIF { virtual MessageQueueId_t getCommandQueue() const = 0; }; +struct ThermalStateCfg { + lp_id_t thermalStatePoolId = DeviceHandlerIF::DEFAULT_THERMAL_STATE_POOL_ID; + lp_id_t thermalRequestPoolId = DeviceHandlerIF::DEFAULT_THERMAL_HEATING_REQUEST_POOL_ID; + uint32_t thermalSetId = DeviceHandlerIF::DEFAULT_THERMAL_SET_ID; +}; + #endif /* FSFW_DEVICEHANDLERS_DEVICEHANDLERIF_H_ */ From 1841f929442d7945fd0c9401de4accc72031003d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 17:17:42 +0100 Subject: [PATCH 17/30] important bugfix for thermal set --- src/fsfw/devicehandlers/DeviceHandlerBase.cpp | 19 ++++++++-------- .../devicehandlers/DeviceHandlerThermalSet.h | 22 +++++++------------ 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index acb9e1d2..b725b186 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -1472,11 +1472,11 @@ void DeviceHandlerBase::performOperationHook() {} ReturnValue_t DeviceHandlerBase::initializeLocalDataPool(localpool::DataPool& localDataPoolMap, LocalDataPoolManager& poolManager) { - if (thermalSet != nullptr) { - localDataPoolMap.emplace(thermalSet->thermalStatePoolId, - new PoolEntry); - localDataPoolMap.emplace(thermalSet->heaterRequestPoolId, - new PoolEntry); + if (thermalStateCfg.has_value()) { + localDataPoolMap.emplace(thermalStateCfg.value().thermalStatePoolId, + new PoolEntry()); + localDataPoolMap.emplace(thermalStateCfg.value().thermalRequestPoolId, + new PoolEntry()); } return returnvalue::OK; } @@ -1490,11 +1490,12 @@ ReturnValue_t DeviceHandlerBase::initializeAfterTaskCreation() { this->poolManager.initializeAfterTaskCreation(); if (thermalStateCfg.has_value()) { - ThermalStateCfg& val = thermalStateCfg.value(); - thermalSet = new DeviceHandlerThermalSet(this, val.thermalSetId, val.thermalStatePoolId, - val.thermalRequestPoolId); + ThermalStateCfg& cfg = thermalStateCfg.value(); + thermalSet = new DeviceHandlerThermalSet(this, cfg); + } + if (setStartupImmediately) { + startTransition(MODE_ON, getInitialSubmode()); } - startTransition(MODE_ON, getInitialSubmode()); return returnvalue::OK; } diff --git a/src/fsfw/devicehandlers/DeviceHandlerThermalSet.h b/src/fsfw/devicehandlers/DeviceHandlerThermalSet.h index 944d7c0f..49ebd5f4 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerThermalSet.h +++ b/src/fsfw/devicehandlers/DeviceHandlerThermalSet.h @@ -7,27 +7,21 @@ class DeviceHandlerThermalSet : public StaticLocalDataSet<2> { public: - DeviceHandlerThermalSet( - HasLocalDataPoolIF* hkOwner, uint32_t setId = DeviceHandlerIF::DEFAULT_THERMAL_SET_ID, - lp_id_t thermalStateId = DeviceHandlerIF::DEFAULT_THERMAL_STATE_POOL_ID, - lp_id_t heaterRequestId = DeviceHandlerIF::DEFAULT_THERMAL_HEATING_REQUEST_POOL_ID) - : DeviceHandlerThermalSet(hkOwner->getObjectId(), setId, thermalStateId, heaterRequestId) {} + DeviceHandlerThermalSet(HasLocalDataPoolIF* hkOwner, ThermalStateCfg cfg) + : DeviceHandlerThermalSet(hkOwner->getObjectId(), cfg) {} - DeviceHandlerThermalSet( - object_id_t deviceHandler, uint32_t setId = DeviceHandlerIF::DEFAULT_THERMAL_SET_ID, - lp_id_t thermalStateId = DeviceHandlerIF::DEFAULT_THERMAL_STATE_POOL_ID, - lp_id_t thermalStateRequestId = DeviceHandlerIF::DEFAULT_THERMAL_HEATING_REQUEST_POOL_ID) - : StaticLocalDataSet(sid_t(deviceHandler, setId)), - thermalStatePoolId(thermalStateId), - heaterRequestPoolId(thermalStateRequestId) {} + DeviceHandlerThermalSet(object_id_t deviceHandler, ThermalStateCfg cfg) + : StaticLocalDataSet(sid_t(deviceHandler, cfg.thermalSetId)), + thermalStatePoolId(cfg.thermalStatePoolId), + heaterRequestPoolId(cfg.thermalRequestPoolId) {} const lp_id_t thermalStatePoolId; const lp_id_t heaterRequestPoolId; lp_var_t thermalState = - lp_var_t(thermalStatePoolId, sid.objectId, this); + lp_var_t(sid.objectId, thermalStatePoolId, this); lp_var_t heaterRequest = - lp_var_t(heaterRequestPoolId, sid.objectId, this); + lp_var_t(sid.objectId, heaterRequestPoolId, this); }; #endif /* FSFW_DEVICEHANDLERS_DEVICEHANDLERTHERMALSET_H_ */ From f4d188c36f0f08c94d678048a979261d373e7642 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:12:43 +0100 Subject: [PATCH 18/30] that time margin check is possible broken --- src/fsfw/pus/Service11TelecommandScheduling.tpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/pus/Service11TelecommandScheduling.tpp b/src/fsfw/pus/Service11TelecommandScheduling.tpp index 2ad11277..d0728780 100644 --- a/src/fsfw/pus/Service11TelecommandScheduling.tpp +++ b/src/fsfw/pus/Service11TelecommandScheduling.tpp @@ -160,7 +160,7 @@ inline ReturnValue_t Service11TelecommandScheduling::doInsertActivi // (See requirement for Time margin) timeval tNow = {}; Clock::getClock_timeval(&tNow); - if (timestamp - tNow.tv_sec <= RELEASE_TIME_MARGIN_SECONDS) { + if (timestamp >= tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "Service11TelecommandScheduling::doInsertActivity: Release time too close to " "current time" From 820a7f059c05d4b2f2012536bfe9482c1625961d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:13:58 +0100 Subject: [PATCH 19/30] logic error --- src/fsfw/pus/Service11TelecommandScheduling.tpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/pus/Service11TelecommandScheduling.tpp b/src/fsfw/pus/Service11TelecommandScheduling.tpp index d0728780..2a20e822 100644 --- a/src/fsfw/pus/Service11TelecommandScheduling.tpp +++ b/src/fsfw/pus/Service11TelecommandScheduling.tpp @@ -160,7 +160,7 @@ inline ReturnValue_t Service11TelecommandScheduling::doInsertActivi // (See requirement for Time margin) timeval tNow = {}; Clock::getClock_timeval(&tNow); - if (timestamp >= tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { + if (timestamp < tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "Service11TelecommandScheduling::doInsertActivity: Release time too close to " "current time" From d93486a340907fb682e916ec702e1be4eb05e15f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:12:43 +0100 Subject: [PATCH 20/30] that time margin check is possible broken --- src/fsfw/pus/Service11TelecommandScheduling.tpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/pus/Service11TelecommandScheduling.tpp b/src/fsfw/pus/Service11TelecommandScheduling.tpp index 540f6c68..d507e354 100644 --- a/src/fsfw/pus/Service11TelecommandScheduling.tpp +++ b/src/fsfw/pus/Service11TelecommandScheduling.tpp @@ -160,7 +160,7 @@ inline ReturnValue_t Service11TelecommandScheduling::doInsertActivi // (See requirement for Time margin) timeval tNow = {}; Clock::getClock_timeval(&tNow); - if (timestamp - tNow.tv_sec <= RELEASE_TIME_MARGIN_SECONDS) { + if (timestamp >= tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "Service11TelecommandScheduling::doInsertActivity: Release time too close to " "current time" From b9b076aa4c08639e5a2dfb8caf0ec8cb764b048a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:13:58 +0100 Subject: [PATCH 21/30] logic error --- src/fsfw/pus/Service11TelecommandScheduling.tpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/pus/Service11TelecommandScheduling.tpp b/src/fsfw/pus/Service11TelecommandScheduling.tpp index d507e354..090d0fd6 100644 --- a/src/fsfw/pus/Service11TelecommandScheduling.tpp +++ b/src/fsfw/pus/Service11TelecommandScheduling.tpp @@ -160,7 +160,7 @@ inline ReturnValue_t Service11TelecommandScheduling::doInsertActivi // (See requirement for Time margin) timeval tNow = {}; Clock::getClock_timeval(&tNow); - if (timestamp >= tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { + if (timestamp < tNow.tv_sec + RELEASE_TIME_MARGIN_SECONDS) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "Service11TelecommandScheduling::doInsertActivity: Release time too close to " "current time" From 341a66c2651b884667191b51c8246dad898d0f70 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:22:00 +0100 Subject: [PATCH 22/30] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7788808..7b2dcfc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixes +- Bugfix in `Service11TelecommandScheduling` which allowed commands + time tagged in the past to be inserted. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/738 - `CService200ModeManagement`: Various bugfixes which lead to now execution complete being generated on mode announcements, duplicate mode reply generated on announce commands, and the mode read subservice not working properly. From 14a92b3d89e37d50ccd46b250826cac293185d68 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 9 Feb 2023 18:28:16 +0100 Subject: [PATCH 23/30] typo --- src/fsfw/pus/CService200ModeCommanding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/pus/CService200ModeCommanding.cpp b/src/fsfw/pus/CService200ModeCommanding.cpp index 77488585..0dbdedfe 100644 --- a/src/fsfw/pus/CService200ModeCommanding.cpp +++ b/src/fsfw/pus/CService200ModeCommanding.cpp @@ -19,7 +19,7 @@ ReturnValue_t CService200ModeCommanding::isValidSubservice(uint8_t subservice) { switch (subservice) { case (Subservice::COMMAND_MODE_COMMAND): case (Subservice::COMMAND_MODE_READ): - case (Subservice::COMMAND_MODE_ANNCOUNCE): + case (Subservice::COMMAND_MODE_ANNOUNCE): case (Subservice::COMMAND_MODE_ANNOUNCE_RECURSIVELY): return returnvalue::OK; default: From d302ba71858edfa15834ff8b28d6cce2c2cbbb84 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 10 Feb 2023 13:51:16 +0100 Subject: [PATCH 24/30] afmt --- src/fsfw/devicehandlers/DeviceHandlerBase.cpp | 2 +- src/fsfw/devicehandlers/DeviceHandlerBase.h | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index b725b186..55eb1b19 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -58,7 +58,7 @@ void DeviceHandlerBase::setHkDestination(object_id_t hkDestination) { this->hkDestination = hkDestination; } -void DeviceHandlerBase::setUpThermalModule(ThermalStateCfg cfg) { this->thermalStateCfg = cfg; } +void DeviceHandlerBase::enableThermalModule(ThermalStateCfg cfg) { this->thermalStateCfg = cfg; } DeviceHandlerBase::~DeviceHandlerBase() { if (comCookie != nullptr) { diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.h b/src/fsfw/devicehandlers/DeviceHandlerBase.h index b96506f5..5e05c3a8 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.h +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.h @@ -164,10 +164,8 @@ class DeviceHandlerBase : public DeviceHandlerIF, * The device handler will then take care of creating local pool entries * for the device thermal state and device heating request. * Custom local pool IDs can be assigned as well. - * @param thermalStatePoolId - * @param thermalRequestPoolId */ - void setUpThermalModule(ThermalStateCfg cfg); + void enableThermalModule(ThermalStateCfg cfg); ReturnValue_t connectModeTreeParent(HasModeTreeChildrenIF &parent) override; ModeTreeChildIF &getModeTreeChildIF() override; From 7fae6cbd6db588d69fc00198e4b2a9d8a7c12f59 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 12 Feb 2023 20:06:32 +0100 Subject: [PATCH 25/30] added missing CS unlock --- src/fsfw_hal/linux/spi/SpiComIF.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw_hal/linux/spi/SpiComIF.cpp b/src/fsfw_hal/linux/spi/SpiComIF.cpp index 11db7cfe..2f84275e 100644 --- a/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -214,6 +214,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const sif::printWarning("SpiComIF::sendMessage: Pulling low CS pin failed"); #endif #endif + csMutex->unlockMutex(); return result; } } else { From 8b4f73a97b15f27e314932538d468707c57f965a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 12 Feb 2023 21:07:11 +0100 Subject: [PATCH 26/30] better error msg --- src/fsfw_hal/linux/spi/SpiComIF.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/fsfw_hal/linux/spi/SpiComIF.cpp b/src/fsfw_hal/linux/spi/SpiComIF.cpp index 2f84275e..059025c8 100644 --- a/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -195,9 +195,13 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const if (result != returnvalue::OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " - << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec - << std::endl; + if (result == MutexIF::MUTEX_TIMEOUT) { + sif::error << "SpiComIF::sendMessage: Lock timeout" << std::endl; + } else { + sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code " + << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec + << std::endl; + } #else sif::printError("SpiComIF::sendMessage: Failed to lock mutex with code %d\n", result); #endif From dac2d210b597adfaf45bd5ae6a4c027599927601 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 13 Feb 2023 00:36:54 +0100 Subject: [PATCH 27/30] updates for thermal module in DHB --- src/fsfw/devicehandlers/DeviceHandlerBase.cpp | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index 55eb1b19..ec627142 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -222,12 +222,11 @@ ReturnValue_t DeviceHandlerBase::initialize() { fillCommandAndReplyMap(); if (thermalSet != nullptr) { + PoolReadGuard pg(thermalSet); // Set temperature target state to NON_OP. - result = thermalSet->read(); - if (result == returnvalue::OK) { + if (pg.getReadResult() == returnvalue::OK) { thermalSet->heaterRequest.value = ThermalComponentIF::STATE_REQUEST_NON_OPERATIONAL; thermalSet->heaterRequest.setValid(true); - thermalSet->commit(); } } @@ -589,12 +588,12 @@ void DeviceHandlerBase::setMode(Mode_t newMode, uint8_t newSubmode) { Clock::getUptime(&timeoutStart); if (mode == MODE_OFF and thermalSet != nullptr) { - ReturnValue_t result = thermalSet->read(); - if (result == returnvalue::OK) { + PoolReadGuard pg(thermalSet); + if (pg.getReadResult() == returnvalue::OK) { if (thermalSet->heaterRequest.value != ThermalComponentIF::STATE_REQUEST_IGNORE) { thermalSet->heaterRequest.value = ThermalComponentIF::STATE_REQUEST_NON_OPERATIONAL; } - thermalSet->heaterRequest.commit(PoolVariableIF::VALID); + thermalSet->heaterRequest.setValid(true); } } /* TODO: This will probably be done by the LocalDataPoolManager now */ @@ -1083,8 +1082,8 @@ ReturnValue_t DeviceHandlerBase::checkModeCommand(Mode_t commandedMode, Submode_ // Do not check thermal state for MODE_RAW if ((mode == MODE_OFF) and ((commandedMode == MODE_ON) or (commandedMode == MODE_NORMAL)) and (thermalSet != nullptr)) { - ReturnValue_t result = thermalSet->read(); - if (result == returnvalue::OK) { + PoolReadGuard pg(thermalSet); + if (pg.getReadResult() == returnvalue::OK) { if ((thermalSet->heaterRequest.value != ThermalComponentIF::STATE_REQUEST_IGNORE) and (not ThermalComponentIF::isOperational(thermalSet->thermalState.value))) { triggerEvent(ThermalComponentIF::TEMP_NOT_IN_OP_RANGE, thermalSet->thermalState.value); @@ -1145,11 +1144,10 @@ void DeviceHandlerBase::handleTransitionToOnMode(Mode_t commandedMode, Submode_t childTransitionDelay = getTransitionDelayMs(_MODE_START_UP, MODE_ON); triggerEvent(CHANGING_MODE, commandedMode, commandedSubmode); if (thermalSet != nullptr) { - ReturnValue_t result = thermalSet->read(); - if (result == returnvalue::OK) { + PoolReadGuard pg(thermalSet); + if (pg.getReadResult() == returnvalue::OK) { if (thermalSet->heaterRequest != ThermalComponentIF::STATE_REQUEST_IGNORE) { thermalSet->heaterRequest = ThermalComponentIF::STATE_REQUEST_OPERATIONAL; - thermalSet->commit(); } } } From f0b8457ba2d9a34a42b10314c3cdccfd46ebf168 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 13 Feb 2023 13:51:52 +0100 Subject: [PATCH 28/30] bugfix for SPI - Set transfer length to 0 for failed transfers --- src/fsfw_hal/linux/spi/SpiComIF.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fsfw_hal/linux/spi/SpiComIF.cpp b/src/fsfw_hal/linux/spi/SpiComIF.cpp index 059025c8..ea25e837 100644 --- a/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -179,12 +179,11 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const uint32_t spiSpeed = 0; spiCookie->getSpiParameters(spiMode, spiSpeed, nullptr); setSpiSpeedAndMode(fileDescriptor, spiMode, spiSpeed); - spiCookie->assignWriteBuffer(sendData); - spiCookie->setTransferSize(sendLen); bool fullDuplex = spiCookie->isFullDuplex(); gpioId_t gpioId = spiCookie->getChipSelectPin(); bool csLockManual = spiCookie->getCsLockManual(); + spiCookie->setTransferSize(0); MutexIF::TimeoutType csType; dur_millis_t csTimeout = 0; @@ -225,11 +224,15 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const updateLinePolarity(fileDescriptor); } + spiCookie->assignWriteBuffer(sendData); + spiCookie->setTransferSize(sendLen); + /* Execute transfer */ if (fullDuplex) { /* Initiate a full duplex SPI transfer. */ retval = ioctl(fileDescriptor, SPI_IOC_MESSAGE(1), spiCookie->getTransferStructHandle()); if (retval < 0) { + spiCookie->setTransferSize(0); utility::handleIoctlError("SpiComIF::sendMessage: ioctl error."); result = FULL_DUPLEX_TRANSFER_FAILED; } @@ -239,6 +242,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const } else { /* We write with a blocking half-duplex transfer here */ if (write(fileDescriptor, sendData, sendLen) != static_cast(sendLen)) { + spiCookie->setTransferSize(0); #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "SpiComIF::sendMessage: Half-Duplex write operation failed!" << std::endl; From d256ede8c1d8e7a746d3a56d45313d2b863e0b28 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 14 Feb 2023 10:33:22 +0100 Subject: [PATCH 29/30] fix cppcheck lint --- src/fsfw/osal/common/TcpIpBase.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/fsfw/osal/common/TcpIpBase.cpp b/src/fsfw/osal/common/TcpIpBase.cpp index 486a5171..3e760f0e 100644 --- a/src/fsfw/osal/common/TcpIpBase.cpp +++ b/src/fsfw/osal/common/TcpIpBase.cpp @@ -41,6 +41,7 @@ int TcpIpBase::closeSocket(socket_t socket) { #elif defined(PLATFORM_UNIX) return close(socket); #endif + return -1; } int TcpIpBase::getLastSocketError() { @@ -49,4 +50,5 @@ int TcpIpBase::getLastSocketError() { #elif defined(PLATFORM_UNIX) return errno; #endif + return 0; } From 9de6c4b3aa20ee63c28051d486be8a12df147f22 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 14 Feb 2023 14:33:02 +0100 Subject: [PATCH 30/30] printout corrections --- src/fsfw/version.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fsfw/version.cpp b/src/fsfw/version.cpp index 050187a9..27d44c03 100644 --- a/src/fsfw/version.cpp +++ b/src/fsfw/version.cpp @@ -1,6 +1,7 @@ #include "version.h" #include +#include #include "fsfw/FSFWVersion.h" @@ -20,7 +21,7 @@ fsfw::Version::Version(int major, int minor, int revision, const char* addInfo) void fsfw::Version::getVersion(char* str, size_t maxLen) const { size_t len = snprintf(str, maxLen, "%d.%d.%d", major, minor, revision); - if (addInfo != nullptr) { + if (addInfo != nullptr and std::strcmp(addInfo, "") != 0) { snprintf(str + len, maxLen - len, "-%s", addInfo); } } @@ -30,7 +31,7 @@ namespace fsfw { #if FSFW_CPP_OSTREAM_ENABLED == 1 std::ostream& operator<<(std::ostream& os, const Version& v) { os << v.major << "." << v.minor << "." << v.revision; - if (v.addInfo != nullptr) { + if (v.addInfo != nullptr and std::strcmp(v.addInfo, "") != 0) { os << "-" << v.addInfo; } return os;