From 29b0a352fcc6ddde8c19fca695d5c067ef2d4e30 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 28 Apr 2022 14:26:00 +0200 Subject: [PATCH 01/22] added new functions to add sequences and tables --- src/fsfw/subsystem/Subsystem.cpp | 9 +++++++++ src/fsfw/subsystem/Subsystem.h | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/fsfw/subsystem/Subsystem.cpp b/src/fsfw/subsystem/Subsystem.cpp index a837bf83..8930d0a1 100644 --- a/src/fsfw/subsystem/Subsystem.cpp +++ b/src/fsfw/subsystem/Subsystem.cpp @@ -299,6 +299,11 @@ void Subsystem::replyToCommand(ReturnValue_t status, uint32_t parameter) { } } +ReturnValue_t Subsystem::addSequence(SequenceEntry sequence) { + return addSequence(sequence.table, sequence.mode, sequence.fallbackMode, sequence.inStore, + sequence.preInit); +} + ReturnValue_t Subsystem::addSequence(ArrayList *sequence, Mode_t id, Mode_t fallbackSequence, bool inStore, bool preInit) { ReturnValue_t result; @@ -342,6 +347,10 @@ ReturnValue_t Subsystem::addSequence(ArrayList *sequence, Mode_t return result; } +ReturnValue_t Subsystem::addTable(TableEntry table) { + return addTable(table.table, table.mode, table.inStore, table.preInit); +} + ReturnValue_t Subsystem::addTable(ArrayList *table, Mode_t id, bool inStore, bool preInit) { ReturnValue_t result; diff --git a/src/fsfw/subsystem/Subsystem.h b/src/fsfw/subsystem/Subsystem.h index 2c78c8cd..5b2777e8 100644 --- a/src/fsfw/subsystem/Subsystem.h +++ b/src/fsfw/subsystem/Subsystem.h @@ -11,6 +11,28 @@ #include "SubsystemBase.h" #include "modes/ModeDefinitions.h" +struct TableSequenceBase { + public: + TableSequenceBase(Mode_t mode, ArrayList *table) : mode(mode), table(table){}; + Mode_t mode; + ArrayList *table; + bool inStore = false; + bool preInit = true; +}; + +struct TableEntry : public TableSequenceBase { + public: + TableEntry(Mode_t mode, ArrayList *table) : TableSequenceBase(mode, table){}; +}; + +struct SequenceEntry : public TableSequenceBase { + public: + SequenceEntry(Mode_t mode, ArrayList *table, Mode_t fallbackMode) + : TableSequenceBase(mode, table), fallbackMode(fallbackMode) {} + + Mode_t fallbackMode; +}; + /** * @brief TODO: documentation missing * @details @@ -45,9 +67,11 @@ class Subsystem : public SubsystemBase, public HasModeSequenceIF { uint32_t maxNumberOfTables); virtual ~Subsystem(); + ReturnValue_t addSequence(SequenceEntry sequence); ReturnValue_t addSequence(ArrayList *sequence, Mode_t id, Mode_t fallbackSequence, bool inStore = true, bool preInit = true); + ReturnValue_t addTable(TableEntry table); ReturnValue_t addTable(ArrayList *table, Mode_t id, bool inStore = true, bool preInit = true); From e98857fab4eb8b91c7b1862e35cada5d540f4976 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 28 Apr 2022 14:37:21 +0200 Subject: [PATCH 02/22] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..a45ebab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/559 - Added ETL dependency and improved library dependency management PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/592 +- `Subsystem`: New API to add table and sequence entries ## Fixed From bf2e0f2d73149a16bf3e6e60dc6912698f3995b5 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 28 Apr 2022 16:48:14 +0200 Subject: [PATCH 03/22] added option to change initial submode --- src/fsfw/subsystem/Subsystem.cpp | 6 +++++- src/fsfw/subsystem/Subsystem.h | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/fsfw/subsystem/Subsystem.cpp b/src/fsfw/subsystem/Subsystem.cpp index 8930d0a1..4e9d52f3 100644 --- a/src/fsfw/subsystem/Subsystem.cpp +++ b/src/fsfw/subsystem/Subsystem.cpp @@ -459,6 +459,7 @@ ReturnValue_t Subsystem::initialize() { } mode = initialMode; + submode = initSubmode; return RETURN_OK; } @@ -596,7 +597,10 @@ ReturnValue_t Subsystem::checkObjectConnections() { return RETURN_OK; } -void Subsystem::setInitialMode(Mode_t mode) { initialMode = mode; } +void Subsystem::setInitialMode(Mode_t mode, Submode_t submode) { + this->initialMode = mode; + this->initSubmode = submode; +} void Subsystem::cantKeepMode() { ReturnValue_t result; diff --git a/src/fsfw/subsystem/Subsystem.h b/src/fsfw/subsystem/Subsystem.h index 5b2777e8..84d5f6a7 100644 --- a/src/fsfw/subsystem/Subsystem.h +++ b/src/fsfw/subsystem/Subsystem.h @@ -75,7 +75,7 @@ class Subsystem : public SubsystemBase, public HasModeSequenceIF { ReturnValue_t addTable(ArrayList *table, Mode_t id, bool inStore = true, bool preInit = true); - void setInitialMode(Mode_t mode); + void setInitialMode(Mode_t mode, Submode_t submode = SUBMODE_NONE); virtual ReturnValue_t initialize() override; @@ -114,6 +114,7 @@ class Subsystem : public SubsystemBase, public HasModeSequenceIF { Submode_t targetSubmode; Mode_t initialMode = 0; + Submode_t initSubmode = SUBMODE_NONE; HybridIterator currentSequenceIterator; From 54feb7777023a34b42ee30acfc27314f47616e64 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Mon, 2 May 2022 16:14:23 +0200 Subject: [PATCH 04/22] Proposed fix for gcc and clang --- src/fsfw/container/FixedArrayList.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fsfw/container/FixedArrayList.h b/src/fsfw/container/FixedArrayList.h index 26a73921..1981abe1 100644 --- a/src/fsfw/container/FixedArrayList.h +++ b/src/fsfw/container/FixedArrayList.h @@ -2,6 +2,7 @@ #define FIXEDARRAYLIST_H_ #include +#include #include "ArrayList.h" /** @@ -9,8 +10,8 @@ */ template class FixedArrayList : public ArrayList { -#if !defined(_MSC_VER) && !defined(__clang__) - static_assert(MAX_SIZE <= (std::pow(2, sizeof(count_t) * 8) - 1), +#if !defined(_MSC_VER) + static_assert(MAX_SIZE <= std::numeric_limits::max(), "count_t is not large enough to hold MAX_SIZE"); #endif private: From 3332f68ce79adc42c2ba6d72a84e18b982eb1d3c Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Mon, 2 May 2022 17:22:13 +0200 Subject: [PATCH 05/22] Tested only std::numeric_limits in MSVC --- src/fsfw/container/FixedArrayList.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/fsfw/container/FixedArrayList.h b/src/fsfw/container/FixedArrayList.h index 1981abe1..11882537 100644 --- a/src/fsfw/container/FixedArrayList.h +++ b/src/fsfw/container/FixedArrayList.h @@ -10,10 +10,8 @@ */ template class FixedArrayList : public ArrayList { -#if !defined(_MSC_VER) static_assert(MAX_SIZE <= std::numeric_limits::max(), "count_t is not large enough to hold MAX_SIZE"); -#endif private: T data[MAX_SIZE]; From cb0c80d8dc7510435c15431d5e39c9a9e484948d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 02:22:16 +0200 Subject: [PATCH 06/22] add option and cmake module for lto support --- CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index bfe3da84..d34511e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,13 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE set(FSFW_ETL_LIB_NAME etl) +include(CheckIPOSupported) +check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) +if(NOT IPO_SUPPORTED) + message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") +endif() + +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" ON) option(FSFW_GENERATE_SECTIONS "Generate function and data sections. Required to remove unused code" ON ) From a943e4eebb1aeb3cc2a01e8df7e1069341ac085b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 02:23:20 +0200 Subject: [PATCH 07/22] enable LTO where applicable --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d34511e8..1700236f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -69,6 +69,10 @@ set(FSFW_DUMMY_TGT fsfw-dummy) project(${LIB_FSFW_NAME}) add_library(${LIB_FSFW_NAME}) +if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) + set_property(TARGET ${LIB_FSFW_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) +endif() + if(FSFW_BUILD_UNITTESTS) message(STATUS "Building the FSFW unittests in addition to the static library") # Check whether the user has already installed Catch2 first From a4bd5a2aaaff36e7ad1d207a48533eb42e51439d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:31:03 +0200 Subject: [PATCH 08/22] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..4d713f26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions +- LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether + the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it + and can make good use of it and is usually makes the code faster and/or larger. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information inside `fsfw/version.h` From 637512ad77023a13c15214b5b513535eee4f7f40 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:34:14 +0200 Subject: [PATCH 09/22] changelog update --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d713f26..dc9577f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,8 +44,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions - LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether - the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it - and can make good use of it and is usually makes the code faster and/or larger. + the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it, + can make good use of it and it usually makes the code faster and/or smaller. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information From 4e4820af05d989aa89b1b153cded1267ca56d5ca Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:47:23 +0200 Subject: [PATCH 10/22] bugfix for prepareHealthSetReply function --- src/fsfw/pus/CService201HealthCommanding.cpp | 15 +++++++++------ src/fsfw/pus/CService201HealthCommanding.h | 10 ++++------ src/fsfw/tmtcservices/CommandingServiceBase.h | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/fsfw/pus/CService201HealthCommanding.cpp b/src/fsfw/pus/CService201HealthCommanding.cpp index 4d9806f9..c62791c6 100644 --- a/src/fsfw/pus/CService201HealthCommanding.cpp +++ b/src/fsfw/pus/CService201HealthCommanding.cpp @@ -13,7 +13,7 @@ CService201HealthCommanding::CService201HealthCommanding(object_id_t objectId, u : CommandingServiceBase(objectId, apid, serviceId, numParallelCommands, commandTimeoutSeconds) { } -CService201HealthCommanding::~CService201HealthCommanding() {} +CService201HealthCommanding::~CService201HealthCommanding() = default; ReturnValue_t CService201HealthCommanding::isValidSubservice(uint8_t subservice) { switch (subservice) { @@ -43,8 +43,8 @@ ReturnValue_t CService201HealthCommanding::getMessageQueueAndObject(uint8_t subs } ReturnValue_t CService201HealthCommanding::checkInterfaceAndAcquireMessageQueue( - MessageQueueId_t *messageQueueToSet, object_id_t *objectId) { - HasHealthIF *destination = ObjectManager::instance()->get(*objectId); + MessageQueueId_t *messageQueueToSet, const object_id_t *objectId) { + auto *destination = ObjectManager::instance()->get(*objectId); if (destination == nullptr) { return CommandingServiceBase::INVALID_OBJECT; } @@ -77,6 +77,10 @@ ReturnValue_t CService201HealthCommanding::prepareCommand(CommandMessage *messag HealthMessage::setHealthMessage(message, HealthMessage::HEALTH_ANNOUNCE_ALL); break; } + default: { + // Should never happen, subservice was already checked + result = RETURN_FAILED; + } } return result; } @@ -96,9 +100,8 @@ ReturnValue_t CService201HealthCommanding::handleReply(const CommandMessage *rep // Not used for now, health state already reported by event ReturnValue_t CService201HealthCommanding::prepareHealthSetReply(const CommandMessage *reply) { - prepareHealthSetReply(reply); - uint8_t health = static_cast(HealthMessage::getHealth(reply)); - uint8_t oldHealth = static_cast(HealthMessage::getOldHealth(reply)); + auto health = static_cast(HealthMessage::getHealth(reply)); + auto oldHealth = static_cast(HealthMessage::getOldHealth(reply)); HealthSetReply healthSetReply(health, oldHealth); return sendTmPacket(Subservice::REPLY_HEALTH_SET, &healthSetReply); } diff --git a/src/fsfw/pus/CService201HealthCommanding.h b/src/fsfw/pus/CService201HealthCommanding.h index 5e52ab39..e4ad749f 100644 --- a/src/fsfw/pus/CService201HealthCommanding.h +++ b/src/fsfw/pus/CService201HealthCommanding.h @@ -1,7 +1,7 @@ #ifndef FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_ #define FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_ -#include "../tmtcservices/CommandingServiceBase.h" +#include "fsfw/tmtcservices/CommandingServiceBase.h" /** * @brief Custom PUS service to set health of all objects @@ -21,7 +21,7 @@ class CService201HealthCommanding : public CommandingServiceBase { public: CService201HealthCommanding(object_id_t objectId, uint16_t apid, uint8_t serviceId, uint8_t numParallelCommands = 4, uint16_t commandTimeoutSeconds = 60); - virtual ~CService201HealthCommanding(); + ~CService201HealthCommanding() override; protected: /* CSB abstract function implementations */ @@ -38,10 +38,8 @@ class CService201HealthCommanding : public CommandingServiceBase { bool *isStep) override; private: - ReturnValue_t checkAndAcquireTargetID(object_id_t *objectIdToSet, const uint8_t *tcData, - size_t tcDataLen); - ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet, - object_id_t *objectId); + static ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet, + const object_id_t *objectId); ReturnValue_t prepareHealthSetReply(const CommandMessage *reply); diff --git a/src/fsfw/tmtcservices/CommandingServiceBase.h b/src/fsfw/tmtcservices/CommandingServiceBase.h index 867fc287..4dcad024 100644 --- a/src/fsfw/tmtcservices/CommandingServiceBase.h +++ b/src/fsfw/tmtcservices/CommandingServiceBase.h @@ -166,7 +166,7 @@ class CommandingServiceBase : public SystemObject, * @param objectId Target object ID * @return * - @c RETURN_OK to generate a verification start message - * - @c EXECUTION_COMPELTE Fire-and-forget command. Generate a completion + * - @c EXECUTION_COMPLETE Fire-and-forget command. Generate a completion * verification message. * - @c Anything else rejects the packets and generates a start failure * verification. From e5e163bdbf720cf12d03314a9d11cb0ee010bbf3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:47:56 +0200 Subject: [PATCH 11/22] mark unused function --- src/fsfw/pus/CService201HealthCommanding.cpp | 2 +- src/fsfw/pus/CService201HealthCommanding.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fsfw/pus/CService201HealthCommanding.cpp b/src/fsfw/pus/CService201HealthCommanding.cpp index c62791c6..91ea4e7a 100644 --- a/src/fsfw/pus/CService201HealthCommanding.cpp +++ b/src/fsfw/pus/CService201HealthCommanding.cpp @@ -99,7 +99,7 @@ ReturnValue_t CService201HealthCommanding::handleReply(const CommandMessage *rep } // Not used for now, health state already reported by event -ReturnValue_t CService201HealthCommanding::prepareHealthSetReply(const CommandMessage *reply) { +[[maybe_unused]] ReturnValue_t CService201HealthCommanding::prepareHealthSetReply(const CommandMessage *reply) { auto health = static_cast(HealthMessage::getHealth(reply)); auto oldHealth = static_cast(HealthMessage::getOldHealth(reply)); HealthSetReply healthSetReply(health, oldHealth); diff --git a/src/fsfw/pus/CService201HealthCommanding.h b/src/fsfw/pus/CService201HealthCommanding.h index e4ad749f..29f21695 100644 --- a/src/fsfw/pus/CService201HealthCommanding.h +++ b/src/fsfw/pus/CService201HealthCommanding.h @@ -41,7 +41,7 @@ class CService201HealthCommanding : public CommandingServiceBase { static ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet, const object_id_t *objectId); - ReturnValue_t prepareHealthSetReply(const CommandMessage *reply); + [[maybe_unused]] ReturnValue_t prepareHealthSetReply(const CommandMessage *reply); enum Subservice { //! [EXPORT] : [TC] Set health of target object From 79f17843d82e7b90c4be31c136635ee6eb917a24 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:50:29 +0200 Subject: [PATCH 12/22] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..4a7c6083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixed +- Fix infinite recursion in `prepareHealthSetReply` of PUS Health Service 201. + Is not currently used right now but might be used in the future + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/617 - Small bugfix in STM32 HAL for SPI PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/599 - HAL GPIO: Improved error checking in `LinuxLibgpioIF::configureGpios(...)`. If a GPIO From 16e55a98cee33f205634b9feb1322558dfbb88db Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:57:23 +0200 Subject: [PATCH 13/22] important bugfix for TCP server --- src/fsfw/osal/common/TcpTmTcServer.cpp | 22 +++++++++++++--------- src/fsfw/osal/common/TcpTmTcServer.h | 11 ++++++----- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 2e6910c5..91cb9574 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -19,6 +19,8 @@ #include #elif defined(PLATFORM_UNIX) #include + +#include #endif const std::string TcpTmTcServer::DEFAULT_SERVER_PORT = tcpip::DEFAULT_SERVER_PORT; @@ -29,7 +31,7 @@ TcpTmTcServer::TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, : SystemObject(objectId), tmtcBridgeId(tmtcTcpBridge), receptionMode(receptionMode), - tcpConfig(customTcpServerPort), + tcpConfig(std::move(customTcpServerPort)), receptionBuffer(receptionBufferSize), ringBuffer(ringBufferSize, true) {} @@ -103,7 +105,7 @@ ReturnValue_t TcpTmTcServer::initialize() { TcpTmTcServer::~TcpTmTcServer() { closeSocket(listenerTcpSocket); } -ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { +[[noreturn]] ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { using namespace tcpip; // If a connection is accepted, the corresponding socket will be assigned to the new socket socket_t connSocket = 0; @@ -138,7 +140,6 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { closeSocket(connSocket); connSocket = 0; } - return HasReturnvaluesIF::RETURN_OK; } ReturnValue_t TcpTmTcServer::initializeAfterTaskCreation() { @@ -159,7 +160,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { #endif while (true) { - int retval = recv(connSocket, reinterpret_cast(receptionBuffer.data()), + ssize_t retval = recv(connSocket, reinterpret_cast(receptionBuffer.data()), receptionBuffer.capacity(), tcpConfig.tcpFlags); if (retval == 0) { size_t availableReadData = ringBuffer.getAvailableReadData(); @@ -252,17 +253,17 @@ ReturnValue_t TcpTmTcServer::handleTcReception(uint8_t* spacePacket, size_t pack return result; } -std::string TcpTmTcServer::getTcpPort() const { return tcpConfig.tcpPort; } +const std::string& TcpTmTcServer::getTcpPort() const { return tcpConfig.tcpPort; } -void TcpTmTcServer::setSpacePacketParsingOptions(std::vector validPacketIds) { - this->validPacketIds = validPacketIds; +void TcpTmTcServer::setSpacePacketParsingOptions(std::vector validPacketIds_) { + this->validPacketIds = std::move(validPacketIds_); } TcpTmTcServer::TcpConfig& TcpTmTcServer::getTcpConfigStruct() { return tcpConfig; } ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) { // Access to the FIFO is mutex protected because it is filled by the bridge - MutexGuard(tmtcBridge->mutex, tmtcBridge->timeoutType, tmtcBridge->mutexTimeoutMs); + MutexGuard mg(tmtcBridge->mutex, tmtcBridge->timeoutType, tmtcBridge->mutexTimeoutMs); store_address_t storeId; while ((not tmtcBridge->tmFifo->empty()) and (tmtcBridge->packetSentCounter < tmtcBridge->sentPacketsPerCycle)) { @@ -283,7 +284,7 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) #endif arrayprinter::print(storeAccessor.data(), storeAccessor.size()); } - int retval = send(connSocket, reinterpret_cast(storeAccessor.data()), + ssize_t retval = send(connSocket, reinterpret_cast(storeAccessor.data()), storeAccessor.size(), tcpConfig.tcpTmFlags); if (retval == static_cast(storeAccessor.size())) { // Packet sent, clear FIFO entry @@ -339,6 +340,9 @@ ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { size_t foundSize = 0; size_t readLen = 0; while (readLen < readAmount) { + if(spacePacketParser == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } result = spacePacketParser->parseSpacePackets(bufPtrPtr, readAmount, startIdx, foundSize, readLen); switch (result) { diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index d062a0ce..faf6e0a1 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -17,6 +17,7 @@ #endif #include +#include #include class TcpTmTcBridge; @@ -44,7 +45,7 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb struct TcpConfig { public: - TcpConfig(std::string tcpPort) : tcpPort(tcpPort) {} + explicit TcpConfig(std::string tcpPort) : tcpPort(std::move(tcpPort)) {} /** * Passed to the recv call @@ -84,7 +85,7 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb size_t ringBufferSize = RING_BUFFER_SIZE, std::string customTcpServerPort = DEFAULT_SERVER_PORT, ReceptionModes receptionMode = ReceptionModes::SPACE_PACKETS); - virtual ~TcpTmTcServer(); + ~TcpTmTcServer() override; void enableWiretapping(bool enable); @@ -97,10 +98,10 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb void setSpacePacketParsingOptions(std::vector validPacketIds); ReturnValue_t initialize() override; - ReturnValue_t performOperation(uint8_t opCode) override; + [[noreturn]] ReturnValue_t performOperation(uint8_t opCode) override; ReturnValue_t initializeAfterTaskCreation() override; - std::string getTcpPort() const; + [[nodiscard]] const std::string& getTcpPort() const; protected: StorageManagerIF* tcStore = nullptr; @@ -115,7 +116,7 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb ReceptionModes receptionMode; TcpConfig tcpConfig; - struct sockaddr tcpAddress; + struct sockaddr tcpAddress = {}; socket_t listenerTcpSocket = 0; MessageQueueId_t targetTcDestination = MessageQueueIF::NO_QUEUE; From 6bfdace5128a1b77119260d86e6278b7e57ea190 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 11:00:31 +0200 Subject: [PATCH 14/22] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..c03bf648 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixed +- TCP TMTC Server: `MutexGuard` was not created properly in + `TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent)` call. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/618 - Small bugfix in STM32 HAL for SPI PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/599 - HAL GPIO: Improved error checking in `LinuxLibgpioIF::configureGpios(...)`. If a GPIO From 226f28dc7ba55d8afe1ca626f153a18c3e68d0cf Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 14:53:52 +0200 Subject: [PATCH 15/22] Move some directives up top --- CMakeLists.txt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bfe3da84..3c85114b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,15 @@ cmake_minimum_required(VERSION 3.13) +set(LIB_FSFW_NAME fsfw) +project(${LIB_FSFW_NAME}) + +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 11) + set(CMAKE_CXX_STANDARD_REQUIRED True) +elseif(${CMAKE_CXX_STANDARD} LESS 11) + message(FATAL_ERROR "Compiling the FSFW requires a minimum of C++11 support") +endif() + set(FSFW_VERSION 4) set(FSFW_SUBVERSION 0) set(FSFW_REVISION 0) @@ -55,11 +65,10 @@ option(FSFW_ADD_TMSTORAGE "Compile with tm storage components" OFF) # Contrib sources option(FSFW_ADD_SGP4_PROPAGATOR "Add SGP4 propagator code" OFF) -set(LIB_FSFW_NAME fsfw) + set(FSFW_TEST_TGT fsfw-tests) set(FSFW_DUMMY_TGT fsfw-dummy) -project(${LIB_FSFW_NAME}) add_library(${LIB_FSFW_NAME}) if(FSFW_BUILD_UNITTESTS) @@ -152,12 +161,6 @@ target_include_directories(${LIB_FSFW_NAME} INTERFACE ${CMAKE_CURRENT_BINARY_DIR} ) -if(NOT CMAKE_CXX_STANDARD) - set(CMAKE_CXX_STANDARD 11) - set(CMAKE_CXX_STANDARD_REQUIRED True) -elseif(${CMAKE_CXX_STANDARD} LESS 11) - message(FATAL_ERROR "Compiling the FSFW requires a minimum of C++11 support") -endif() # Backwards comptability if(OS_FSFW AND NOT FSFW_OSAL) From b0d71597f038122ecbb6bb85f3fdd0a77bf842dc Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 14:58:39 +0200 Subject: [PATCH 16/22] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..ae574c15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixed +- Move some CMake directives further up top so they are not ignored + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/621 - Small bugfix in STM32 HAL for SPI PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/599 - HAL GPIO: Improved error checking in `LinuxLibgpioIF::configureGpios(...)`. If a GPIO From 73ff9b97db023f91e4962d9f74e89f667cce1435 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 15:07:46 +0200 Subject: [PATCH 17/22] bump CMAKE_CXX_STANDARD to C++17 --- CHANGELOG.md | 2 ++ CMakeLists.txt | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae574c15..6e913de8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Changes +- Bump C++ required version to C++17. Every project which uses the FSFW and every modern + compiler supports it - HAL Linux SPI: Set the Clock Default State when setting new SPI speed and mode PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/573 diff --git a/CMakeLists.txt b/CMakeLists.txt index 3c85114b..1df1008b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,10 +4,10 @@ set(LIB_FSFW_NAME fsfw) project(${LIB_FSFW_NAME}) if(NOT CMAKE_CXX_STANDARD) - set(CMAKE_CXX_STANDARD 11) + set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED True) -elseif(${CMAKE_CXX_STANDARD} LESS 11) - message(FATAL_ERROR "Compiling the FSFW requires a minimum of C++11 support") +elseif(${CMAKE_CXX_STANDARD} LESS 17) + message(FATAL_ERROR "Compiling the FSFW requires a minimum of C++17 support") endif() set(FSFW_VERSION 4) From 10cc954d276353eb6d981d2408b3c927835b82d6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 15:09:07 +0200 Subject: [PATCH 18/22] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e913de8..a500f27d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Bump C++ required version to C++17. Every project which uses the FSFW and every modern compiler supports it + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/622 - HAL Linux SPI: Set the Clock Default State when setting new SPI speed and mode PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/573 From fd112ed5973bb522a351ed42e1e89995c484d3ad Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 16:07:05 +0200 Subject: [PATCH 19/22] enable lto for test target --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93f8c24f..2f4ffe75 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -106,6 +106,9 @@ if(FSFW_BUILD_UNITTESTS) project(${FSFW_TEST_TGT} CXX C) add_executable(${FSFW_TEST_TGT}) + if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) + set_property(TARGET ${FSFW_TEST_TGT} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) + endif() if(FSFW_TESTS_GEN_COV) message(STATUS "Generating coverage data for the library") From 0fe1b70baedfe250d5d39be3e5545bab3806a9f0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 11:19:29 +0200 Subject: [PATCH 20/22] keep LTO option off by default --- CHANGELOG.md | 10 ++++++++++ CMakeLists.txt | 17 +++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0452e58b..f3523a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it, can make good use of it and it usually makes the code faster and/or smaller. + After some more research: + Enabling LTO will actually cause the compiler to only produce thin LTO by adding + `-flto -fno-fat-lto-objects` to the compiler options. I am not sure this is an ideal choice + because if an application linking against the FSFW does not use LTO, there can be compile + issues (e.g. observed when compiling the FSFW tests without LTO). This is a known issue as + can be seen in the multiple CMake issues for it: + - https://gitlab.kitware.com/cmake/cmake/-/issues/22913, + - https://gitlab.kitware.com/cmake/cmake/-/issues/16808, + - https://gitlab.kitware.com/cmake/cmake/-/issues/21696 + Easiest solution for now: Keep this option OFF by default. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information diff --git a/CMakeLists.txt b/CMakeLists.txt index 9f686d1d..61145e1e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,11 +17,12 @@ set(FSFW_REVISION 0) # Add the cmake folder so the FindSphinx module is found set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH}) +set(FSFW_ETL_LIB_NAME etl) set(FSFW_ETL_LIB_MAJOR_VERSION 20 CACHE STRING - "ETL library major version requirement" + "ETL library major version requirement" ) set(FSFW_ETL_LIB_VERSION ${FSFW_ETL_LIB_MAJOR_VERSION}.27.3 CACHE STRING - "ETL library exact version requirement" + "ETL library exact version requirement" ) set(FSFW_ETL_LINK_TARGET etl::etl) @@ -32,26 +33,26 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE "Catch2 library exact version requirement" ) -set(FSFW_ETL_LIB_NAME etl) - include(CheckIPOSupported) check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) if(NOT IPO_SUPPORTED) message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") endif() -option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" ON) +# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 +# for information which keeping this on by default is problematic +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) option(FSFW_GENERATE_SECTIONS - "Generate function and data sections. Required to remove unused code" ON + "Generate function and data sections. Required to remove unused code" ON ) if(FSFW_GENERATE_SECTIONS) - option(FSFW_REMOVE_UNUSED_CODE "Remove unused code" ON) + option(FSFW_REMOVE_UNUSED_CODE "Remove unused code" ON) endif() option(FSFW_BUILD_UNITTESTS "Build unittest binary in addition to static library" OFF) option(FSFW_BUILD_DOCS "Build documentation with Sphinx and Doxygen" OFF) if(FSFW_BUILD_UNITTESTS) - option(FSFW_TESTS_GEN_COV "Generate coverage data for unittests" ON) + option(FSFW_TESTS_GEN_COV "Generate coverage data for unittests" ON) endif() option(FSFW_WARNING_SHADOW_LOCAL_GCC "Enable -Wshadow=local warning in GCC" ON) From 25775614de4bf6e7d11e70f8b15965247b9a51ee Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 11:56:51 +0200 Subject: [PATCH 21/22] only check IPO support if enabled --- CMakeLists.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61145e1e..e1ab522a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,15 +33,18 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE "Catch2 library exact version requirement" ) +# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 +# for information which keeping this on by default is problematic +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) + +if(FSFW_ENABLE_IPO) include(CheckIPOSupported) check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) if(NOT IPO_SUPPORTED) message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") endif() +endif() -# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 -# for information which keeping this on by default is problematic -option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) option(FSFW_GENERATE_SECTIONS "Generate function and data sections. Required to remove unused code" ON ) From e05c72b062836eb5f9cd4cdc1d35b0a2846653e0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 13:08:14 +0200 Subject: [PATCH 22/22] minor formatting fix --- CMakeLists.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e1ab522a..3cca727e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,13 +36,12 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE # Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 # for information which keeping this on by default is problematic option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) - if(FSFW_ENABLE_IPO) -include(CheckIPOSupported) -check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) -if(NOT IPO_SUPPORTED) - message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") -endif() + include(CheckIPOSupported) + check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) + if(NOT IPO_SUPPORTED) + message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") + endif() endif() option(FSFW_GENERATE_SECTIONS