From e98857fab4eb8b91c7b1862e35cada5d540f4976 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 28 Apr 2022 14:37:21 +0200 Subject: [PATCH 01/19] 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 02/19] 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 03/19] 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 04/19] 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 4e4820af05d989aa89b1b153cded1267ca56d5ca Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:47:23 +0200 Subject: [PATCH 05/19] 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 06/19] 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 07/19] 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 08/19] 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 09/19] 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 10/19] 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 11/19] 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 12/19] 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 13/19] 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 29c3a4376058bb392edd9ef25623723f000058a4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 10:54:13 +0200 Subject: [PATCH 14/19] getter functions for speed and mode --- hal/src/fsfw_hal/linux/spi/SpiComIF.cpp | 14 ++++++++++++++ hal/src/fsfw_hal/linux/spi/SpiComIF.h | 1 + 2 files changed, 15 insertions(+) diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp index dcf92b5d..db694613 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -410,3 +410,17 @@ void SpiComIF::setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed) utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Updating SPI default clock failed"); } } + +void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes &mode, uint32_t &speed) const { + uint8_t tmpMode = 0; + int retval = ioctl(spiFd, SPI_IOC_RD_MODE, &tmpMode); + if (retval != 0) { + utility::handleIoctlError("SpiComIF::getSpiSpeedAndMode: Reading SPI mode failed"); + } + mode = static_cast(tmpMode); + + retval = ioctl(spiFd, SPI_IOC_RD_MAX_SPEED_HZ, &speed); + if (retval != 0) { + utility::handleIoctlError("SpiComIF::getSpiSpeedAndMode: Getting SPI speed failed"); + } +} diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.h b/hal/src/fsfw_hal/linux/spi/SpiComIF.h index 357afa2f..ded47e04 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.h @@ -59,6 +59,7 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { GpioIF* getGpioInterface(); void setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed); + void getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& speed) const; void performSpiWiretapping(SpiCookie* spiCookie); ReturnValue_t getReadBuffer(address_t spiAddress, uint8_t** buffer); From 0e880de0d08d05b14125a5af785930aeff636d1b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 10:54:39 +0200 Subject: [PATCH 15/19] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84fb2ca1..856580cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions +- Linux HAL: Getter functions for SPI speed and mode. - 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 e06c457743991b2a0c2d84de9e169508ac428aa8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 11:11:39 +0200 Subject: [PATCH 16/19] Cache SPI device name in ComIF - Architecturally, this makes a lot more sense because each ComIF should be responsible for one SPI bus --- hal/src/fsfw_hal/linux/UnixFileGuard.cpp | 2 +- hal/src/fsfw_hal/linux/UnixFileGuard.h | 2 +- hal/src/fsfw_hal/linux/spi/SpiComIF.cpp | 17 +++++------ hal/src/fsfw_hal/linux/spi/SpiComIF.h | 6 ++-- hal/src/fsfw_hal/linux/spi/SpiCookie.cpp | 29 +++++++++---------- hal/src/fsfw_hal/linux/spi/SpiCookie.h | 17 +++++------ src/fsfw/container/FixedArrayList.h | 1 + src/fsfw/devicehandlers/DeviceHandlerBase.cpp | 3 +- src/fsfw/devicehandlers/DeviceHandlerBase.h | 6 ++-- src/fsfw/osal/common/TcpTmTcServer.cpp | 6 ++-- src/fsfw/osal/freertos/FixedTimeslotTask.cpp | 2 +- src/fsfw/osal/linux/FixedTimeslotTask.cpp | 2 +- src/fsfw/osal/rtems/FixedTimeslotTask.cpp | 2 +- src/fsfw/pus/CService201HealthCommanding.cpp | 3 +- src/fsfw/pus/CService201HealthCommanding.h | 2 +- src/fsfw/timemanager/Countdown.cpp | 3 +- 16 files changed, 49 insertions(+), 54 deletions(-) diff --git a/hal/src/fsfw_hal/linux/UnixFileGuard.cpp b/hal/src/fsfw_hal/linux/UnixFileGuard.cpp index 41293815..3e916ba2 100644 --- a/hal/src/fsfw_hal/linux/UnixFileGuard.cpp +++ b/hal/src/fsfw_hal/linux/UnixFileGuard.cpp @@ -6,7 +6,7 @@ #include "fsfw/FSFW.h" #include "fsfw/serviceinterface.h" -UnixFileGuard::UnixFileGuard(std::string device, int* fileDescriptor, int flags, +UnixFileGuard::UnixFileGuard(const std::string& device, int* fileDescriptor, int flags, std::string diagnosticPrefix) : fileDescriptor(fileDescriptor) { if (fileDescriptor == nullptr) { diff --git a/hal/src/fsfw_hal/linux/UnixFileGuard.h b/hal/src/fsfw_hal/linux/UnixFileGuard.h index d94234b6..04f379d6 100644 --- a/hal/src/fsfw_hal/linux/UnixFileGuard.h +++ b/hal/src/fsfw_hal/linux/UnixFileGuard.h @@ -15,7 +15,7 @@ class UnixFileGuard { static constexpr ReturnValue_t OPEN_FILE_FAILED = 1; - UnixFileGuard(std::string device, int* fileDescriptor, int flags, + UnixFileGuard(const std::string& device, int* fileDescriptor, int flags, std::string diagnosticPrefix = ""); virtual ~UnixFileGuard(); diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp index db694613..da95bfcb 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -15,8 +15,8 @@ #include "fsfw_hal/linux/spi/SpiCookie.h" #include "fsfw_hal/linux/utility.h" -SpiComIF::SpiComIF(object_id_t objectId, GpioIF* gpioComIF) - : SystemObject(objectId), gpioComIF(gpioComIF) { +SpiComIF::SpiComIF(object_id_t objectId, std::string devname, GpioIF* gpioComIF) + : SystemObject(objectId), gpioComIF(gpioComIF), dev(std::move(devname)) { if (gpioComIF == nullptr) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -85,8 +85,7 @@ ReturnValue_t SpiComIF::initializeInterface(CookieIF* cookie) { spiCookie->getSpiParameters(spiMode, spiSpeed, ¶ms); int fileDescriptor = 0; - UnixFileGuard fileHelper(spiCookie->getSpiDevice(), &fileDescriptor, O_RDWR, - "SpiComIF::initializeInterface"); + UnixFileGuard fileHelper(dev, &fileDescriptor, O_RDWR, "SpiComIF::initializeInterface"); if (fileHelper.getOpenResult() != HasReturnvaluesIF::RETURN_OK) { return fileHelper.getOpenResult(); } @@ -182,8 +181,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const int retval = 0; /* Prepare transfer */ int fileDescriptor = 0; - std::string device = spiCookie->getSpiDevice(); - UnixFileGuard fileHelper(device, &fileDescriptor, O_RDWR, "SpiComIF::sendMessage"); + UnixFileGuard fileHelper(dev, &fileDescriptor, O_RDWR, "SpiComIF::sendMessage"); if (fileHelper.getOpenResult() != HasReturnvaluesIF::RETURN_OK) { return OPENING_FILE_FAILED; } @@ -278,9 +276,8 @@ ReturnValue_t SpiComIF::requestReceiveMessage(CookieIF* cookie, size_t requestLe ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) { ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; - std::string device = spiCookie->getSpiDevice(); int fileDescriptor = 0; - UnixFileGuard fileHelper(device, &fileDescriptor, O_RDWR, "SpiComIF::requestReceiveMessage"); + UnixFileGuard fileHelper(dev, &fileDescriptor, O_RDWR, "SpiComIF::requestReceiveMessage"); if (fileHelper.getOpenResult() != HasReturnvaluesIF::RETURN_OK) { return OPENING_FILE_FAILED; } @@ -411,7 +408,7 @@ void SpiComIF::setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed) } } -void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes &mode, uint32_t &speed) const { +void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& speed) const { uint8_t tmpMode = 0; int retval = ioctl(spiFd, SPI_IOC_RD_MODE, &tmpMode); if (retval != 0) { @@ -424,3 +421,5 @@ void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes &mode, uint32_t &spee utility::handleIoctlError("SpiComIF::getSpiSpeedAndMode: Getting SPI speed failed"); } } + +const std::string& SpiComIF::getSpiDev() const { return dev; } diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.h b/hal/src/fsfw_hal/linux/spi/SpiComIF.h index ded47e04..881af8c4 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.h @@ -32,7 +32,7 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { static constexpr ReturnValue_t HALF_DUPLEX_TRANSFER_FAILED = HasReturnvaluesIF::makeReturnCode(spiRetvalId, 2); - SpiComIF(object_id_t objectId, GpioIF* gpioComIF); + SpiComIF(object_id_t objectId, std::string devname, GpioIF* gpioComIF); ReturnValue_t initializeInterface(CookieIF* cookie) override; ReturnValue_t sendMessage(CookieIF* cookie, const uint8_t* sendData, size_t sendLen) override; @@ -60,6 +60,8 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { GpioIF* getGpioInterface(); void setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed); void getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& speed) const; + + const std::string& getSpiDev() const; void performSpiWiretapping(SpiCookie* spiCookie); ReturnValue_t getReadBuffer(address_t spiAddress, uint8_t** buffer); @@ -71,7 +73,7 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { }; GpioIF* gpioComIF = nullptr; - + std::string dev = ""; MutexIF* spiMutex = nullptr; MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING; uint32_t timeoutMs = 20; diff --git a/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp b/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp index c94fcdf1..85f96f28 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp @@ -1,26 +1,25 @@ #include "SpiCookie.h" -SpiCookie::SpiCookie(address_t spiAddress, gpioId_t chipSelect, std::string spiDev, - const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed) - : SpiCookie(spi::SpiComIfModes::REGULAR, spiAddress, chipSelect, spiDev, maxSize, spiMode, - spiSpeed, nullptr, nullptr) {} - -SpiCookie::SpiCookie(address_t spiAddress, std::string spiDev, const size_t maxSize, +SpiCookie::SpiCookie(address_t spiAddress, gpioId_t chipSelect, const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed) - : SpiCookie(spiAddress, gpio::NO_GPIO, spiDev, maxSize, spiMode, spiSpeed) {} + : SpiCookie(spi::SpiComIfModes::REGULAR, spiAddress, chipSelect, maxSize, spiMode, spiSpeed, + nullptr, nullptr) {} -SpiCookie::SpiCookie(address_t spiAddress, gpioId_t chipSelect, std::string spiDev, - const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed, +SpiCookie::SpiCookie(address_t spiAddress, const size_t maxSize, spi::SpiModes spiMode, + uint32_t spiSpeed) + : SpiCookie(spiAddress, gpio::NO_GPIO, maxSize, spiMode, spiSpeed) {} + +SpiCookie::SpiCookie(address_t spiAddress, gpioId_t chipSelect, const size_t maxSize, + spi::SpiModes spiMode, uint32_t spiSpeed, spi::send_callback_function_t callback, void* args) - : SpiCookie(spi::SpiComIfModes::CALLBACK, spiAddress, chipSelect, spiDev, maxSize, spiMode, - spiSpeed, callback, args) {} + : SpiCookie(spi::SpiComIfModes::CALLBACK, spiAddress, chipSelect, maxSize, spiMode, spiSpeed, + callback, args) {} SpiCookie::SpiCookie(spi::SpiComIfModes comIfMode, address_t spiAddress, gpioId_t chipSelect, - std::string spiDev, const size_t maxSize, spi::SpiModes spiMode, - uint32_t spiSpeed, spi::send_callback_function_t callback, void* args) + const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed, + spi::send_callback_function_t callback, void* args) : spiAddress(spiAddress), chipSelectPin(chipSelect), - spiDevice(spiDev), comIfMode(comIfMode), maxSize(maxSize), spiMode(spiMode), @@ -50,8 +49,6 @@ size_t SpiCookie::getMaxBufferSize() const { return maxSize; } address_t SpiCookie::getSpiAddress() const { return spiAddress; } -std::string SpiCookie::getSpiDevice() const { return spiDevice; } - void SpiCookie::setThreeWireSpi(bool enable) { uncommonParameters.threeWireSpi = enable; } void SpiCookie::setLsbFirst(bool enable) { uncommonParameters.lsbFirst = enable; } diff --git a/hal/src/fsfw_hal/linux/spi/SpiCookie.h b/hal/src/fsfw_hal/linux/spi/SpiCookie.h index 5f4bf2d5..d6d4078f 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiCookie.h +++ b/hal/src/fsfw_hal/linux/spi/SpiCookie.h @@ -29,23 +29,22 @@ class SpiCookie : public CookieIF { * @param spiDev * @param maxSize */ - SpiCookie(address_t spiAddress, gpioId_t chipSelect, std::string spiDev, const size_t maxSize, - spi::SpiModes spiMode, uint32_t spiSpeed); + SpiCookie(address_t spiAddress, gpioId_t chipSelect, const size_t maxSize, spi::SpiModes spiMode, + uint32_t spiSpeed); /** * Like constructor above, but without a dedicated GPIO CS. Can be used for hardware * slave select or if CS logic is performed with decoders. */ - SpiCookie(address_t spiAddress, std::string spiDev, const size_t maxReplySize, - spi::SpiModes spiMode, uint32_t spiSpeed); + SpiCookie(address_t spiAddress, const size_t maxReplySize, spi::SpiModes spiMode, + uint32_t spiSpeed); /** * Use the callback mode of the SPI communication interface. The user can pass the callback * function here or by using the setter function #setCallbackMode */ - SpiCookie(address_t spiAddress, gpioId_t chipSelect, std::string spiDev, const size_t maxSize, - spi::SpiModes spiMode, uint32_t spiSpeed, spi::send_callback_function_t callback, - void* args); + SpiCookie(address_t spiAddress, gpioId_t chipSelect, const size_t maxSize, spi::SpiModes spiMode, + uint32_t spiSpeed, spi::send_callback_function_t callback, void* args); /** * Get the callback function @@ -55,7 +54,6 @@ class SpiCookie : public CookieIF { void getCallback(spi::send_callback_function_t* callback, void** args); address_t getSpiAddress() const; - std::string getSpiDevice() const; gpioId_t getChipSelectPin() const; size_t getMaxBufferSize() const; @@ -154,12 +152,11 @@ class SpiCookie : public CookieIF { * @param args */ SpiCookie(spi::SpiComIfModes comIfMode, address_t spiAddress, gpioId_t chipSelect, - std::string spiDev, const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed, + const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed, spi::send_callback_function_t callback, void* args); address_t spiAddress; gpioId_t chipSelectPin; - std::string spiDevice; spi::SpiComIfModes comIfMode; diff --git a/src/fsfw/container/FixedArrayList.h b/src/fsfw/container/FixedArrayList.h index 11882537..fc8be393 100644 --- a/src/fsfw/container/FixedArrayList.h +++ b/src/fsfw/container/FixedArrayList.h @@ -12,6 +12,7 @@ template class FixedArrayList : public ArrayList { static_assert(MAX_SIZE <= std::numeric_limits::max(), "count_t is not large enough to hold MAX_SIZE"); + private: T data[MAX_SIZE]; diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index 52178683..69baf54f 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -1401,8 +1401,7 @@ uint8_t DeviceHandlerBase::getReplyDelayCycles(DeviceCommandId_t deviceCommand) DeviceReplyMap::iterator iter = deviceReplyMap.find(deviceCommand); if (iter == deviceReplyMap.end()) { return 0; - } - else if (iter->second.countdown != nullptr) { + } else if (iter->second.countdown != nullptr) { return 0; } return iter->second.delayCycles; diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.h b/src/fsfw/devicehandlers/DeviceHandlerBase.h index c67aed27..0a273675 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.h +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.h @@ -801,7 +801,7 @@ class DeviceHandlerBase : public DeviceHandlerIF, DeviceCommandMap::iterator command; //! Instead of using delayCycles to specify the maximum time to wait for the device reply, it //! is also possible specify a countdown - Countdown* countdown = nullptr; + Countdown *countdown = nullptr; //! will be set to true when reply is enabled bool active = false; }; @@ -1269,13 +1269,13 @@ class DeviceHandlerBase : public DeviceHandlerIF, /** * @brief Handles disabling of replies which use a timeout to detect missed replies. */ - void disableTimeoutControlledReply(DeviceReplyInfo* info); + void disableTimeoutControlledReply(DeviceReplyInfo *info); /** * @brief Handles disabling of replies which use a number of maximum delay cycles to detect * missed replies. */ - void disableDelayCyclesControlledReply(DeviceReplyInfo* info); + void disableDelayCyclesControlledReply(DeviceReplyInfo *info); /** * Retrive data from the #IPCStore. diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 91cb9574..b9089245 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -161,7 +161,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { while (true) { ssize_t retval = recv(connSocket, reinterpret_cast(receptionBuffer.data()), - receptionBuffer.capacity(), tcpConfig.tcpFlags); + receptionBuffer.capacity(), tcpConfig.tcpFlags); if (retval == 0) { size_t availableReadData = ringBuffer.getAvailableReadData(); if (availableReadData > lastRingBufferSize) { @@ -285,7 +285,7 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) arrayprinter::print(storeAccessor.data(), storeAccessor.size()); } ssize_t retval = send(connSocket, reinterpret_cast(storeAccessor.data()), - storeAccessor.size(), tcpConfig.tcpTmFlags); + storeAccessor.size(), tcpConfig.tcpTmFlags); if (retval == static_cast(storeAccessor.size())) { // Packet sent, clear FIFO entry tmtcBridge->tmFifo->pop(); @@ -340,7 +340,7 @@ ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { size_t foundSize = 0; size_t readLen = 0; while (readLen < readAmount) { - if(spacePacketParser == nullptr) { + if (spacePacketParser == nullptr) { return HasReturnvaluesIF::RETURN_FAILED; } result = diff --git a/src/fsfw/osal/freertos/FixedTimeslotTask.cpp b/src/fsfw/osal/freertos/FixedTimeslotTask.cpp index e8823f8a..87d262f3 100644 --- a/src/fsfw/osal/freertos/FixedTimeslotTask.cpp +++ b/src/fsfw/osal/freertos/FixedTimeslotTask.cpp @@ -47,7 +47,7 @@ void FixedTimeslotTask::missedDeadlineCounter() { if (FixedTimeslotTask::deadlineMissedCount % 10 == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "PST missed " << FixedTimeslotTask::deadlineMissedCount << " deadlines" - << std::endl; + << std::endl; #endif } } diff --git a/src/fsfw/osal/linux/FixedTimeslotTask.cpp b/src/fsfw/osal/linux/FixedTimeslotTask.cpp index 37b958de..a6337fb0 100644 --- a/src/fsfw/osal/linux/FixedTimeslotTask.cpp +++ b/src/fsfw/osal/linux/FixedTimeslotTask.cpp @@ -88,7 +88,7 @@ void FixedTimeslotTask::missedDeadlineCounter() { if (FixedTimeslotTask::deadlineMissedCount % 10 == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "PST missed " << FixedTimeslotTask::deadlineMissedCount << " deadlines" - << std::endl; + << std::endl; #endif } } diff --git a/src/fsfw/osal/rtems/FixedTimeslotTask.cpp b/src/fsfw/osal/rtems/FixedTimeslotTask.cpp index 745b8d70..d83a4d4a 100644 --- a/src/fsfw/osal/rtems/FixedTimeslotTask.cpp +++ b/src/fsfw/osal/rtems/FixedTimeslotTask.cpp @@ -51,7 +51,7 @@ void FixedTimeslotTask::missedDeadlineCounter() { if (FixedTimeslotTask::deadlineMissedCount % 10 == 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "PST missed " << FixedTimeslotTask::deadlineMissedCount << " deadlines" - << std::endl; + << std::endl; #endif } } diff --git a/src/fsfw/pus/CService201HealthCommanding.cpp b/src/fsfw/pus/CService201HealthCommanding.cpp index f6c49cd3..644e0d7c 100644 --- a/src/fsfw/pus/CService201HealthCommanding.cpp +++ b/src/fsfw/pus/CService201HealthCommanding.cpp @@ -97,7 +97,8 @@ ReturnValue_t CService201HealthCommanding::handleReply(const CommandMessage *rep } // Not used for now, health state already reported by event -[[maybe_unused]] 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 7ffa06d2..71b7caa0 100644 --- a/src/fsfw/pus/CService201HealthCommanding.h +++ b/src/fsfw/pus/CService201HealthCommanding.h @@ -39,7 +39,7 @@ class CService201HealthCommanding : public CommandingServiceBase { private: static ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet, - const object_id_t *objectId); + const object_id_t *objectId); [[maybe_unused]] ReturnValue_t prepareHealthSetReply(const CommandMessage *reply); diff --git a/src/fsfw/timemanager/Countdown.cpp b/src/fsfw/timemanager/Countdown.cpp index 327995f8..334883ae 100644 --- a/src/fsfw/timemanager/Countdown.cpp +++ b/src/fsfw/timemanager/Countdown.cpp @@ -3,8 +3,7 @@ Countdown::Countdown(uint32_t initialTimeout, bool startImmediately) : timeout(initialTimeout) { if (startImmediately) { setTimeout(initialTimeout); - } - else { + } else { timeout = initialTimeout; } } From 56e4fca06f9644d857493ddfd27d8788b48a47a9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 11:24:06 +0200 Subject: [PATCH 17/19] Some improvements - Rename mutex to csMutex to better represent its purpose - Move the empty transfer to update the line polarity to separate function --- hal/src/fsfw_hal/linux/spi/SpiComIF.cpp | 35 ++++++++++++++----------- hal/src/fsfw_hal/linux/spi/SpiComIF.h | 17 +++++++++++- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp index da95bfcb..12d95f0d 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -27,7 +27,7 @@ SpiComIF::SpiComIF(object_id_t objectId, std::string devname, GpioIF* gpioComIF) #endif /* FSFW_VERBOSE_LEVEL >= 1 */ } - spiMutex = MutexFactory::instance()->createMutex(); + csMutex = MutexFactory::instance()->createMutex(); } ReturnValue_t SpiComIF::initializeInterface(CookieIF* cookie) { @@ -197,7 +197,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const /* Pull SPI CS low. For now, no support for active high given */ if (gpioId != gpio::NO_GPIO) { - result = spiMutex->lockMutex(timeoutType, timeoutMs); + result = csMutex->lockMutex(timeoutType, timeoutMs); if (result != RETURN_OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -208,6 +208,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const #endif return result; } + updateLinePolarity(fileDescriptor); ReturnValue_t result = gpioComIF->pullLow(gpioId); if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_VERBOSE_LEVEL >= 1 @@ -219,6 +220,8 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const #endif return result; } + } else { + updateLinePolarity(fileDescriptor); } /* Execute transfer */ @@ -248,7 +251,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const if (gpioId != gpio::NO_GPIO) { gpioComIF->pullHigh(gpioId); - result = spiMutex->unlockMutex(); + result = csMutex->unlockMutex(); if (result != RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "SpiComIF::sendMessage: Failed to unlock mutex" << std::endl; @@ -291,7 +294,7 @@ ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) { gpioId_t gpioId = spiCookie->getChipSelectPin(); if (gpioId != gpio::NO_GPIO) { - result = spiMutex->lockMutex(timeoutType, timeoutMs); + result = csMutex->lockMutex(timeoutType, timeoutMs); if (result != RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "SpiComIF::getSendSuccess: Failed to lock mutex" << std::endl; @@ -314,7 +317,7 @@ ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) { if (gpioId != gpio::NO_GPIO) { gpioComIF->pullHigh(gpioId); - result = spiMutex->unlockMutex(); + result = csMutex->unlockMutex(); if (result != RETURN_OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "SpiComIF::getSendSuccess: Failed to unlock mutex" << std::endl; @@ -350,7 +353,7 @@ MutexIF* SpiComIF::getMutex(MutexIF::TimeoutType* timeoutType, uint32_t* timeout if (timeoutMs != nullptr) { *timeoutMs = this->timeoutMs; } - return spiMutex; + return csMutex; } void SpiComIF::performSpiWiretapping(SpiCookie* spiCookie) { @@ -398,14 +401,6 @@ void SpiComIF::setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed) if (retval != 0) { utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Setting SPI speed failed"); } - // This updates the SPI clock default polarity. Only setting the mode does not update - // the line state, which can be an issue on mode switches because the clock line will - // switch the state after the chip select is pulled low - clockUpdateTransfer.len = 0; - retval = ioctl(spiFd, SPI_IOC_MESSAGE(1), &clockUpdateTransfer); - if (retval != 0) { - utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Updating SPI default clock failed"); - } } void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& speed) const { @@ -422,4 +417,14 @@ void SpiComIF::getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& spee } } -const std::string& SpiComIF::getSpiDev() const { return dev; } +const std::string& SpiComIF::getSpiDev() const { + return dev; +} + +void SpiComIF::updateLinePolarity(int spiFd) { + clockUpdateTransfer.len = 0; + int retval = ioctl(spiFd, SPI_IOC_MESSAGE(1), &clockUpdateTransfer); + if (retval != 0) { + utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Updating SPI default clock failed"); + } +} diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.h b/hal/src/fsfw_hal/linux/spi/SpiComIF.h index 881af8c4..aca94765 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.h @@ -59,6 +59,17 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { GpioIF* getGpioInterface(); void setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed); + + /** + * This updates the SPI clock default polarity. Only setting the mode does not update + * the line state, which can be an issue on mode switches because the clock line will + * switch the state after the chip select is pulled low. + * + * It is recommended to call this function after #setSpiSpeedAndMode if the SPI bus + * has multiple SPI devices with different speed and SPI modes attached. + * @param spiFd + */ + void updateLinePolarity(int spiFd); void getSpiSpeedAndMode(int spiFd, spi::SpiModes& mode, uint32_t& speed) const; const std::string& getSpiDev() const; @@ -74,7 +85,11 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { GpioIF* gpioComIF = nullptr; std::string dev = ""; - MutexIF* spiMutex = nullptr; + /** + * Protects the chip select operations. Lock when GPIO is pulled low, unlock after it was + * pulled high + */ + MutexIF* csMutex = nullptr; MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING; uint32_t timeoutMs = 20; spi_ioc_transfer clockUpdateTransfer = {}; From ab2d7ca98fbfbb862e129ea57b65b712e3dec589 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 11:29:28 +0200 Subject: [PATCH 18/19] update changelog and docs --- CHANGELOG.md | 13 +++++++++++-- hal/src/fsfw_hal/linux/spi/SpiComIF.h | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34f40ed3..2019fdcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,8 +60,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions -- Linux HAL: Getter functions for SPI speed and mode. -- 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` PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/559 @@ -69,6 +67,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/). PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/592 - `Subsystem`: New API to add table and sequence entries +## HAL + +- SPI: Cache the SPI device in the communication interface. Architecturally, this makes a + lot more sense because each ComIF should be responsible for one SPI bus. +- SPI: Move the empty transfer to update the line polarity to separate function. This means + it is not automatically called when calling the setter function for SPI speed and mode. + The user should call this function after locking the CS mutex if multiple SPI devices with + differing speeds and modes are attached to one bus. +- SPI: Getter functions for SPI speed and mode. +- I2C: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1. + ## Fixed - TCP TMTC Server: `MutexGuard` was not created properly in diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.h b/hal/src/fsfw_hal/linux/spi/SpiComIF.h index aca94765..3d15009d 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.h @@ -65,8 +65,8 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject { * the line state, which can be an issue on mode switches because the clock line will * switch the state after the chip select is pulled low. * - * It is recommended to call this function after #setSpiSpeedAndMode if the SPI bus - * has multiple SPI devices with different speed and SPI modes attached. + * It is recommended to call this function after #setSpiSpeedAndMode and after locking the + * CS mutex if the SPI bus has multiple SPI devices with different speed and SPI modes attached. * @param spiFd */ void updateLinePolarity(int spiFd); From 0a97077a0e0f5e1c4859d9c867a74f277405c00c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 11 May 2022 15:42:52 +0200 Subject: [PATCH 19/19] hotfix --- src/fsfw/tmtcservices/SourceSequenceCounter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/tmtcservices/SourceSequenceCounter.h b/src/fsfw/tmtcservices/SourceSequenceCounter.h index 6f2778a4..00f4021e 100644 --- a/src/fsfw/tmtcservices/SourceSequenceCounter.h +++ b/src/fsfw/tmtcservices/SourceSequenceCounter.h @@ -5,7 +5,7 @@ class SourceSequenceCounter { private: - uint16_t sequenceCount; + uint16_t sequenceCount = 0; public: SourceSequenceCounter() : sequenceCount(0) {}