From 7adb47aecb5f5e0b6e20a46128f44b4f44e4e49a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 Jan 2023 11:55:54 +0100 Subject: [PATCH 1/8] remove duplicate printout --- src/fsfw_hal/linux/i2c/I2cComIF.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/fsfw_hal/linux/i2c/I2cComIF.cpp b/src/fsfw_hal/linux/i2c/I2cComIF.cpp index 1ad19e82..e3e50040 100644 --- a/src/fsfw_hal/linux/i2c/I2cComIF.cpp +++ b/src/fsfw_hal/linux/i2c/I2cComIF.cpp @@ -180,10 +180,6 @@ ReturnValue_t I2cComIF::requestReceiveMessage(CookieIF* cookie, size_t requestLe } #else #endif -#endif -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "I2cComIF::requestReceiveMessage: Read " << readLen << " of " << requestLen - << " bytes" << std::endl; #endif return returnvalue::FAILED; } From ba62c28b64f9ff38fd9e09026e3d0daa62e36dad Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Thu, 12 Jan 2023 15:40:52 +0100 Subject: [PATCH 2/8] adding linux ci and fixing problems --- automation/Jenkinsfile | 96 +++++++++++++------ src/fsfw/events/EventManager.cpp | 6 ++ src/fsfw/events/EventManager.h | 1 + src/fsfw/events/EventManagerIF.h | 1 + src/fsfw/fdir/FailureIsolationBase.cpp | 2 +- src/fsfw/globalfunctions/matching/MatchTree.h | 41 +++++++- src/fsfw/health/HealthTable.cpp | 2 - .../internalerror/InternalErrorReporter.cpp | 7 +- src/fsfw/objectmanager/ObjectManager.cpp | 18 +++- src/fsfw/objectmanager/ObjectManager.h | 10 +- src/fsfw/osal/CMakeLists.txt | 10 +- src/fsfw/osal/linux/MessageQueue.cpp | 2 +- src/fsfw/osal/osal.h.in | 36 +++++++ unittests/CatchSetup.cpp | 5 +- .../devicehandler/DeviceHandlerCommander.cpp | 4 +- unittests/osal/TestSemaphore.cpp | 68 ++++++------- 16 files changed, 230 insertions(+), 79 deletions(-) create mode 100644 src/fsfw/osal/osal.h.in diff --git a/automation/Jenkinsfile b/automation/Jenkinsfile index 27136606..64c32237 100644 --- a/automation/Jenkinsfile +++ b/automation/Jenkinsfile @@ -1,45 +1,87 @@ pipeline { environment { - BUILDDIR = 'cmake-build-tests' + BUILDDIR_HOST = 'cmake-build-tests-host' + BUILDDIR_LINUX = 'cmake-build-tests-linux' DOCDDIR = 'cmake-build-documentation' } agent { docker { image 'fsfw-ci:d6' - args '--network host' + args '--network host --sysctl fs.mqueue.msg_max=100' } } stages { - stage('Clean') { - steps { - sh 'rm -rf $BUILDDIR' - } - } - stage('Configure') { - steps { - dir(BUILDDIR) { - sh 'cmake -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON -DFSFW_CICD_BUILD=ON ..' + stage('Host') { + stages{ + stage('Clean') { + steps { + sh 'rm -rf $BUILDDIR_HOST' + } + } + stage('Configure') { + steps { + dir(BUILDDIR_HOST) { + sh 'cmake -DFSFW_OSAL=host -DFSFW_BUILD_TESTS=ON -DFSFW_CICD_BUILD=ON ..' + } + } + } + stage('Build') { + steps { + dir(BUILDDIR_HOST) { + sh 'cmake --build . -j4' + } + } + } + stage('Unittests') { + steps { + dir(BUILDDIR_HOST) { + sh 'cmake --build . -- fsfw-tests_coverage -j4' + } + } + } + stage('Valgrind') { + steps { + dir(BUILDDIR_HOST) { + sh 'valgrind --leak-check=full --error-exitcode=1 ./fsfw-tests' + } + } } } } - stage('Build') { - steps { - dir(BUILDDIR) { - sh 'cmake --build . -j4' + stage('Linux') { + stages{ + stage('Clean') { + steps { + sh 'rm -rf $BUILDDIR_LINUX' + } } - } - } - stage('Unittests') { - steps { - dir(BUILDDIR) { - sh 'cmake --build . -- fsfw-tests_coverage -j4' + stage('Configure') { + steps { + dir(BUILDDIR_LINUX) { + sh 'cmake -DFSFW_OSAL=linux -DFSFW_BUILD_TESTS=ON -DFSFW_CICD_BUILD=ON ..' + } + } } - } - } - stage('Valgrind') { - steps { - dir(BUILDDIR) { - sh 'valgrind --leak-check=full --error-exitcode=1 ./fsfw-tests' + stage('Build') { + steps { + dir(BUILDDIR_LINUX) { + sh 'cmake --build . -j4' + } + } + } + stage('Unittests') { + steps { + dir(BUILDDIR_LINUX) { + sh 'cmake --build . -- fsfw-tests_coverage -j4' + } + } + } + stage('Valgrind') { + steps { + dir(BUILDDIR_LINUX) { + sh 'valgrind --leak-check=full --error-exitcode=1 ./fsfw-tests' + } + } } } } diff --git a/src/fsfw/events/EventManager.cpp b/src/fsfw/events/EventManager.cpp index f4466817..8bd26282 100644 --- a/src/fsfw/events/EventManager.cpp +++ b/src/fsfw/events/EventManager.cpp @@ -23,6 +23,7 @@ EventManager::EventManager(object_id_t setObjectId) } EventManager::~EventManager() { + listenerList.clear(); QueueFactory::instance()->deleteMessageQueue(eventReportQueue); MutexFactory::instance()->deleteMutex(mutex); } @@ -61,9 +62,14 @@ ReturnValue_t EventManager::registerListener(MessageQueueId_t listener, if (!result.second) { return returnvalue::FAILED; } + return returnvalue::OK; } +ReturnValue_t EventManager::unregisterListener(MessageQueueId_t listener) { + return listenerList.erase(listener) == 1 ? returnvalue::OK : returnvalue::FAILED; +} + ReturnValue_t EventManager::subscribeToEvent(MessageQueueId_t listener, EventId_t event) { return subscribeToEventRange(listener, event); } diff --git a/src/fsfw/events/EventManager.h b/src/fsfw/events/EventManager.h index 70245f5d..0e6dace0 100644 --- a/src/fsfw/events/EventManager.h +++ b/src/fsfw/events/EventManager.h @@ -31,6 +31,7 @@ class EventManager : public EventManagerIF, public ExecutableObjectIF, public Sy MessageQueueId_t getEventReportQueue(); ReturnValue_t registerListener(MessageQueueId_t listener, bool forwardAllButSelected = false); + ReturnValue_t unregisterListener(MessageQueueId_t listener) override; ReturnValue_t subscribeToEvent(MessageQueueId_t listener, EventId_t event); ReturnValue_t subscribeToAllEventsFrom(MessageQueueId_t listener, object_id_t object); ReturnValue_t subscribeToEventRange(MessageQueueId_t listener, EventId_t idFrom = 0, diff --git a/src/fsfw/events/EventManagerIF.h b/src/fsfw/events/EventManagerIF.h index adb61f2b..12014090 100644 --- a/src/fsfw/events/EventManagerIF.h +++ b/src/fsfw/events/EventManagerIF.h @@ -18,6 +18,7 @@ class EventManagerIF { virtual ReturnValue_t registerListener(MessageQueueId_t listener, bool forwardAllButSelected = false) = 0; + virtual ReturnValue_t unregisterListener(MessageQueueId_t listener) = 0; virtual ReturnValue_t subscribeToEvent(MessageQueueId_t listener, EventId_t event) = 0; virtual ReturnValue_t subscribeToAllEventsFrom(MessageQueueId_t listener, object_id_t object) = 0; virtual ReturnValue_t unsubscribeFromAllEvents(MessageQueueId_t listener, object_id_t object) = 0; diff --git a/src/fsfw/fdir/FailureIsolationBase.cpp b/src/fsfw/fdir/FailureIsolationBase.cpp index 28df16d8..0d3b0214 100644 --- a/src/fsfw/fdir/FailureIsolationBase.cpp +++ b/src/fsfw/fdir/FailureIsolationBase.cpp @@ -23,7 +23,7 @@ FailureIsolationBase::~FailureIsolationBase() { #endif return; } - manager->unsubscribeFromAllEvents(eventQueue->getId(), ownerId); + manager->unregisterListener(eventQueue->getId()); QueueFactory::instance()->deleteMessageQueue(eventQueue); } diff --git a/src/fsfw/globalfunctions/matching/MatchTree.h b/src/fsfw/globalfunctions/matching/MatchTree.h index 0c31cf2a..69f660d3 100644 --- a/src/fsfw/globalfunctions/matching/MatchTree.h +++ b/src/fsfw/globalfunctions/matching/MatchTree.h @@ -24,7 +24,7 @@ class MatchTree : public SerializeableMatcherIF, public BinaryTree>(root.element), maxDepth(maxDepth) {} MatchTree() : BinaryTree>(), maxDepth(-1) {} - virtual ~MatchTree() {} + virtual ~MatchTree() { clear(); } virtual bool match(T number) override { return matchesTree(number); } bool matchesTree(T number) { iterator iter = this->begin(); @@ -176,6 +176,45 @@ class MatchTree : public SerializeableMatcherIF, public BinaryTree>::rootNode; + + if (localRoot == nullptr) { + return; + } + + Node* node = localRoot->left; + + while (true) { + if (node->left != nullptr) { + node = node->left; + continue; + } + if (node->right != nullptr) { + node = node->right; + continue; + } + if (node->parent == nullptr) { + // this is the root node with no children + if (node->value != nullptr) { + cleanUpElement(iterator(node)); + } + return; + } + // leaf + { + Node* parent = node->parent; + if (parent->left == node) { + parent->left = nullptr; + } else { + parent->right = nullptr; + } + cleanUpElement(iterator(node)); + node = parent; + } + } + } + virtual ReturnValue_t cleanUpElement(iterator position) { return returnvalue::OK; } bool matchSubtree(iterator iter, T number) { diff --git a/src/fsfw/health/HealthTable.cpp b/src/fsfw/health/HealthTable.cpp index 5fb45fb9..3fcf7f77 100644 --- a/src/fsfw/health/HealthTable.cpp +++ b/src/fsfw/health/HealthTable.cpp @@ -6,8 +6,6 @@ HealthTable::HealthTable(object_id_t objectid) : SystemObject(objectid) { mutex = MutexFactory::instance()->createMutex(); - ; - mapIterator = healthMap.begin(); } diff --git a/src/fsfw/internalerror/InternalErrorReporter.cpp b/src/fsfw/internalerror/InternalErrorReporter.cpp index b3b77366..fb2dc8a6 100644 --- a/src/fsfw/internalerror/InternalErrorReporter.cpp +++ b/src/fsfw/internalerror/InternalErrorReporter.cpp @@ -7,14 +7,17 @@ InternalErrorReporter::InternalErrorReporter(object_id_t setObjectId, uint32_t messageQueueDepth) : SystemObject(setObjectId), - commandQueue(QueueFactory::instance()->createMessageQueue(messageQueueDepth)), poolManager(this, commandQueue), internalErrorSid(setObjectId, InternalErrorDataset::ERROR_SET_ID), internalErrorDataset(this) { + commandQueue = QueueFactory::instance()->createMessageQueue(messageQueueDepth); mutex = MutexFactory::instance()->createMutex(); } -InternalErrorReporter::~InternalErrorReporter() { MutexFactory::instance()->deleteMutex(mutex); } +InternalErrorReporter::~InternalErrorReporter() { + MutexFactory::instance()->deleteMutex(mutex); + QueueFactory::instance()->deleteMessageQueue(commandQueue); +} void InternalErrorReporter::setDiagnosticPrintout(bool enable) { this->diagnosticPrintout = enable; diff --git a/src/fsfw/objectmanager/ObjectManager.cpp b/src/fsfw/objectmanager/ObjectManager.cpp index ddf5ab80..29c1b2fc 100644 --- a/src/fsfw/objectmanager/ObjectManager.cpp +++ b/src/fsfw/objectmanager/ObjectManager.cpp @@ -23,9 +23,17 @@ void ObjectManager::setObjectFactoryFunction(produce_function_t objFactoryFunc, ObjectManager::ObjectManager() = default; +void ObjectManager::clear() { + if (objManagerInstance != nullptr) { + delete objManagerInstance; + objManagerInstance = nullptr; + } +} + ObjectManager::~ObjectManager() { - for (auto const& iter : objectList) { - delete iter.second; + teardown = true; + for (auto iter = objectList.begin(); iter != objectList.end(); iter = objectList.erase(iter)) { + delete iter->second; } } @@ -53,6 +61,12 @@ ReturnValue_t ObjectManager::insert(object_id_t id, SystemObjectIF* object) { } ReturnValue_t ObjectManager::remove(object_id_t id) { + // this function is called during destruction of System Objects + // disabeld for teardown to avoid iterator invalidation and + // double free + if (teardown) { + return returnvalue::OK; + } if (this->getSystemObject(id) != nullptr) { this->objectList.erase(id); #if FSFW_CPP_OSTREAM_ENABLED == 1 diff --git a/src/fsfw/objectmanager/ObjectManager.h b/src/fsfw/objectmanager/ObjectManager.h index e0e74792..355f5d0a 100644 --- a/src/fsfw/objectmanager/ObjectManager.h +++ b/src/fsfw/objectmanager/ObjectManager.h @@ -24,12 +24,17 @@ class ObjectManager : public ObjectManagerIF { using produce_function_t = void (*)(void* args); /** - * Returns the single instance of TaskFactory. + * Returns the single instance of ObjectManager. * The implementation of #instance is found in its subclasses. * Thus, we choose link-time variability of the instance. */ static ObjectManager* instance(); + /** + * Deletes the single instance of ObjectManager + */ + static void clear(); + void setObjectFactoryFunction(produce_function_t prodFunc, void* args); template @@ -66,6 +71,9 @@ class ObjectManager : public ObjectManagerIF { */ std::map objectList; static ObjectManager* objManagerInstance; + // used when the OM itself is deleted to modify behaviour of remove() + // to avoid iterator invalidation and double free + bool teardown = false; }; // Documentation can be found in the class method declaration above diff --git a/src/fsfw/osal/CMakeLists.txt b/src/fsfw/osal/CMakeLists.txt index 50fd6102..a4097649 100644 --- a/src/fsfw/osal/CMakeLists.txt +++ b/src/fsfw/osal/CMakeLists.txt @@ -1,10 +1,13 @@ # Check the OS_FSFW variable if(FSFW_OSAL MATCHES "freertos") add_subdirectory(freertos) + set(FSFW_OSAL_FREERTOS 1) elseif(FSFW_OSAL MATCHES "rtems") add_subdirectory(rtems) + set(FSFW_OSAL_RTEMS 1) elseif(FSFW_OSAL MATCHES "linux") add_subdirectory(linux) + set(FSFW_OSAL_LINUX 1) elseif(FSFW_OSAL MATCHES "host") add_subdirectory(host) if(WIN32) @@ -13,16 +16,17 @@ elseif(FSFW_OSAL MATCHES "host") # We still need to pull in some Linux specific sources target_sources(${LIB_FSFW_NAME} PUBLIC linux/tcpipHelpers.cpp) endif() - + set(FSFW_OSAL_HOST 1) else() - message(WARNING "The OS_FSFW variable was not set. Assuming host OS..") # Not set. Assumuing this is a host build, try to determine host OS if(WIN32) add_subdirectory(host) add_subdirectory(windows) + set(FSFW_OSAL_HOST 1) elseif(UNIX) add_subdirectory(linux) + set(FSFW_OSAL_LINUX 1) else() # MacOS or other OSes have not been tested yet / are not supported. message(FATAL_ERROR "The host OS could not be determined! Aborting.") @@ -31,3 +35,5 @@ else() endif() add_subdirectory(common) + +configure_file(osal.h.in ${CMAKE_BINARY_DIR}/fsfw/osal/osal.h) diff --git a/src/fsfw/osal/linux/MessageQueue.cpp b/src/fsfw/osal/linux/MessageQueue.cpp index b2cca3f1..c08e3775 100644 --- a/src/fsfw/osal/linux/MessageQueue.cpp +++ b/src/fsfw/osal/linux/MessageQueue.cpp @@ -21,7 +21,7 @@ MessageQueue::MessageQueue(uint32_t messageDepth, size_t maxMessageSize, MqArgs* attributes.mq_msgsize = maxMessageSize; attributes.mq_flags = 0; // Flags are ignored on Linux during mq_open // Set the name of the queue. The slash is mandatory! - sprintf(name, "/FSFW_MQ%u\n", queueCounter++); + sprintf(name, "/FSFW_MQ%u", queueCounter++); // Create a nonblocking queue if the name is available (the queue is read // and writable for the owner as well as the group) diff --git a/src/fsfw/osal/osal.h.in b/src/fsfw/osal/osal.h.in new file mode 100644 index 00000000..8e48c31a --- /dev/null +++ b/src/fsfw/osal/osal.h.in @@ -0,0 +1,36 @@ +#pragma once + +namespace osal { + enum osalTarget{ + HOST, + LINUX, + WINDOWS, + FREERTOS, + RTEMS, + }; + +#cmakedefine FSFW_OSAL_HOST +#cmakedefine FSFW_OSAL_LINUX +#cmakedefine FSFW_OSAL_WINDOWS +#cmakedefine FSFW_OSAL_FREERTOS +#cmakedefine FSFW_OSAL_RTEMS + + + constexpr osalTarget getTarget() { +#ifdef FSFW_OSAL_HOST + return HOST; +#endif +#ifdef FSFW_OSAL_LINUX + return LINUX; +#endif +#ifdef FSFW_OSAL_WINDOWS + return WINDOWS; +#endif +#ifdef FSFW_OSAL_FREERTOS + return FREERTOS; +#endif +#ifdef FSFW_OSAL_RTEMS + return RTEMS; +#endif + } +}; \ No newline at end of file diff --git a/unittests/CatchSetup.cpp b/unittests/CatchSetup.cpp index 4f2a4a54..fc5bb3f5 100644 --- a/unittests/CatchSetup.cpp +++ b/unittests/CatchSetup.cpp @@ -31,4 +31,7 @@ int customSetup() { return 0; } -int customTeardown() { return 0; } +int customTeardown() { + ObjectManager::clear(); + return 0; +} diff --git a/unittests/devicehandler/DeviceHandlerCommander.cpp b/unittests/devicehandler/DeviceHandlerCommander.cpp index 03ff992c..835faf2b 100644 --- a/unittests/devicehandler/DeviceHandlerCommander.cpp +++ b/unittests/devicehandler/DeviceHandlerCommander.cpp @@ -9,7 +9,9 @@ DeviceHandlerCommander::DeviceHandlerCommander(object_id_t objectId) QUEUE_SIZE, MessageQueueMessage::MAX_MESSAGE_SIZE, &mqArgs); } -DeviceHandlerCommander::~DeviceHandlerCommander() {} +DeviceHandlerCommander::~DeviceHandlerCommander() { + QueueFactory::instance()->deleteMessageQueue(commandQueue); +} ReturnValue_t DeviceHandlerCommander::performOperation(uint8_t operationCode) { readCommandQueue(); diff --git a/unittests/osal/TestSemaphore.cpp b/unittests/osal/TestSemaphore.cpp index 376b08db..1d8758b1 100644 --- a/unittests/osal/TestSemaphore.cpp +++ b/unittests/osal/TestSemaphore.cpp @@ -1,46 +1,38 @@ - -#ifdef LINUX - -/* +#include #include #include -#include "catch.hpp" -#include "core/CatchDefinitions.h" +#include -TEST_CASE("Binary Semaphore Test" , "[BinSemaphore]") { - //perform set-up here - SemaphoreIF* binSemaph = SemaphoreFactory::instance()-> - createBinarySemaphore(); - REQUIRE(binSemaph != nullptr); - SECTION("Simple Test") { - // set-up is run for each section - REQUIRE(binSemaph->getSemaphoreCounter() == 1); - REQUIRE(binSemaph->release() == - static_cast(SemaphoreIF::SEMAPHORE_NOT_OWNED)); - REQUIRE(binSemaph->acquire(SemaphoreIF::POLLING) == - result::OK); - { - // not precise enough on linux.. should use clock instead.. - //Stopwatch stopwatch(false); - //REQUIRE(binSemaph->acquire(SemaphoreIF::TimeoutType::WAITING, 5) == - // SemaphoreIF::SEMAPHORE_TIMEOUT); - //dur_millis_t time = stopwatch.stop(); - //CHECK(time == 5); - } - REQUIRE(binSemaph->getSemaphoreCounter() == 0); - REQUIRE(binSemaph->release() == result::OK); - } - SemaphoreFactory::instance()->deleteSemaphore(binSemaph); - // perform tear-down here +// binary semaphores currently only supported on linux +#ifdef FSFW_OSAL_LINUX + +TEST_CASE("Binary Semaphore Test", "[BinSemaphore]") { + // perform set-up here + SemaphoreIF* binSemaph = SemaphoreFactory::instance()->createBinarySemaphore(); + REQUIRE(binSemaph != nullptr); + SECTION("Simple Test") { + // set-up is run for each section + REQUIRE(binSemaph->getSemaphoreCounter() == 1); + REQUIRE(binSemaph->release() == static_cast(SemaphoreIF::SEMAPHORE_NOT_OWNED)); + REQUIRE(binSemaph->acquire(SemaphoreIF::POLLING) == returnvalue::OK); + { + // not precise enough on linux.. should use clock instead.. + // Stopwatch stopwatch(false); + // REQUIRE(binSemaph->acquire(SemaphoreIF::TimeoutType::WAITING, 5) == + // SemaphoreIF::SEMAPHORE_TIMEOUT); + // dur_millis_t time = stopwatch.stop(); + // CHECK(time == 5); + } + REQUIRE(binSemaph->getSemaphoreCounter() == 0); + REQUIRE(binSemaph->release() == returnvalue::OK); + } + SemaphoreFactory::instance()->deleteSemaphore(binSemaph); + // perform tear-down here } - -TEST_CASE("Counting Semaphore Test" , "[CountingSemaph]") { - SECTION("Simple Test") { - - } +TEST_CASE("Counting Semaphore Test", "[CountingSemaph]") { + SECTION("Simple Test") {} } -*/ -#endif +#endif \ No newline at end of file From 99d8c845f2690a55a6436e31fb0f50a21a285ff0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 30 Jan 2023 14:05:39 +0100 Subject: [PATCH 3/8] allow using SO_REUSEADDR and SO_REUSEPORT on TCP server --- src/fsfw/osal/common/TcpTmTcServer.cpp | 15 ++++++++++++--- src/fsfw/osal/common/TcpTmTcServer.h | 26 ++++++++++++++++++++------ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index dff959ba..903d8752 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -26,12 +26,12 @@ const std::string TcpTmTcServer::DEFAULT_SERVER_PORT = tcpip::DEFAULT_SERVER_PORT; TcpTmTcServer::TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, - size_t receptionBufferSize, size_t ringBufferSize, - std::string customTcpServerPort, ReceptionModes receptionMode) + TcpTmTcServer::TcpConfig cfg, size_t receptionBufferSize, + size_t ringBufferSize, ReceptionModes receptionMode) : SystemObject(objectId), tmtcBridgeId(tmtcTcpBridge), receptionMode(receptionMode), - tcpConfig(std::move(customTcpServerPort)), + tcpConfig(cfg), receptionBuffer(receptionBufferSize), ringBuffer(ringBufferSize, true) {} @@ -91,6 +91,15 @@ ReturnValue_t TcpTmTcServer::initialize() { return returnvalue::FAILED; } + if (tcpConfig.reuseAddr) { + unsigned int enable = 1; + setsockopt(listenerTcpSocket, SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(enable)); + } + if (tcpConfig.reusePort) { + unsigned int enable = 1; + setsockopt(listenerTcpSocket, SOL_SOCKET, SO_REUSEPORT, &enable, sizeof(enable)); + } + // Bind to the address found by getaddrinfo retval = bind(listenerTcpSocket, addrResult->ai_addr, static_cast(addrResult->ai_addrlen)); if (retval == SOCKET_ERROR) { diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index 0e2182a5..8668d845 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -41,11 +41,11 @@ class SpacePacketParser; */ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableObjectIF { public: - enum class ReceptionModes { SPACE_PACKETS }; - struct TcpConfig { public: - explicit TcpConfig(std::string tcpPort) : tcpPort(std::move(tcpPort)) {} + TcpConfig(bool reuseAddr, bool reusePort) : reuseAddr(reuseAddr), reusePort(reusePort) {} + TcpConfig(std::string tcpPort, bool reuseAddr, bool reusePort) + : tcpPort(std::move(tcpPort)), reuseAddr(reuseAddr), reusePort(reusePort) {} /** * Passed to the recv call @@ -63,8 +63,23 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb */ int tcpTmFlags = 0; - const std::string tcpPort; + /** + * Sets the SO_REUSEADDR option on the socket. See + * https://man7.org/linux/man-pages/man7/socket.7.html for more details. This option is + * especially useful in a debugging and development environment where an OBSW image might be + * re-flashed oftentimes and where all incoming telecommands are received on a dedicated TCP + * port. + */ + bool reuseAddr = false; + /** + * Sets the SO_REUSEPORT option on the socket. See + * https://man7.org/linux/man-pages/man7/socket.7.html for more details. + */ + bool reusePort = false; + + std::string tcpPort = DEFAULT_SERVER_PORT; }; + enum class ReceptionModes { SPACE_PACKETS }; static const std::string DEFAULT_SERVER_PORT; @@ -80,10 +95,9 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb * size will be the Ethernet MTU size * @param customTcpServerPort The user can specify another port than the default (7301) here. */ - TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, + TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, TcpTmTcServer::TcpConfig cfg, size_t receptionBufferSize = RING_BUFFER_SIZE, size_t ringBufferSize = RING_BUFFER_SIZE, - std::string customTcpServerPort = DEFAULT_SERVER_PORT, ReceptionModes receptionMode = ReceptionModes::SPACE_PACKETS); ~TcpTmTcServer() override; From b646717a7610d430f999e4582a439a17eafaadc9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 30 Jan 2023 14:11:00 +0100 Subject: [PATCH 4/8] bump changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f738032f..bc0fa937 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Added +- `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. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/722 - `DleParser` helper class to parse DLE encoded packets from a byte stream. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/711 - `UioMapper` is able to resolve symlinks now. From fe7197846748040bbe4ab4c3cdd15d6cbf58cc43 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 28 Jan 2023 14:31:32 +0100 Subject: [PATCH 5/8] improve srv20 error messages --- src/fsfw/pus/Service20ParameterManagement.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fsfw/pus/Service20ParameterManagement.cpp b/src/fsfw/pus/Service20ParameterManagement.cpp index e12d47bc..87bd5a13 100644 --- a/src/fsfw/pus/Service20ParameterManagement.cpp +++ b/src/fsfw/pus/Service20ParameterManagement.cpp @@ -69,14 +69,14 @@ ReturnValue_t Service20ParameterManagement::checkInterfaceAndAcquireMessageQueue #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "Service20ParameterManagement::checkInterfaceAndAcquire" << "MessageQueue: Can't access object" << std::endl; - sif::error << "Object ID: " << std::hex << objectId << std::dec << std::endl; - sif::error << "Make sure it implements ReceivesParameterMessagesIF!" << std::endl; + sif::error << "Object ID: 0x" << std::hex << *objectId << std::dec << std::endl; + sif::error << "Make sure it implements ReceivesParameterMessagesIF" << std::endl; #else sif::printError( "Service20ParameterManagement::checkInterfaceAndAcquire" "MessageQueue: Can't access object\n"); sif::printError("Object ID: 0x%08x\n", *objectId); - sif::printError("Make sure it implements ReceivesParameterMessagesIF!\n"); + sif::printError("Make sure it implements ReceivesParameterMessagesIF\n"); #endif return CommandingServiceBase::INVALID_OBJECT; From 3656662d8881bb8b8e200a412dad4a1867ce759c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 20 Jan 2023 11:10:05 +0100 Subject: [PATCH 6/8] small tweak, gain factory was always them same --- src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp index a32153eb..ce215a64 100644 --- a/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp +++ b/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp @@ -329,8 +329,8 @@ ReturnValue_t MgmRM3100Handler::handleDataReadout(const uint8_t *packet) { // Now scale to physical value in microtesla float fieldStrengthX = fieldStrengthRawX * scaleFactorX; - float fieldStrengthY = fieldStrengthRawY * scaleFactorX; - float fieldStrengthZ = fieldStrengthRawZ * scaleFactorX; + float fieldStrengthY = fieldStrengthRawY * scaleFactorY; + float fieldStrengthZ = fieldStrengthRawZ * scaleFactorZ; if (periodicPrintout) { if (debugDivider.checkAndIncrement()) { From eb223dae880266fdbff7f26e700e14c819ad8f8b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 30 Jan 2023 14:20:28 +0100 Subject: [PATCH 7/8] bump changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f738032f..e379a6f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixes +- 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 - TC Scheduler Service 11: Add size and CRC check for contained TC. - Only delete health table entry in `HealthHelper` destructor if health table was set. From 9b05e8f27406e0bc1df4db6dde6ff85fbc9ea8e8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 30 Jan 2023 14:24:28 +0100 Subject: [PATCH 8/8] re-order fields in TcpConfig --- src/fsfw/osal/common/TcpTmTcServer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index 8668d845..3d182827 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -63,6 +63,8 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb */ int tcpTmFlags = 0; + std::string tcpPort = DEFAULT_SERVER_PORT; + /** * Sets the SO_REUSEADDR option on the socket. See * https://man7.org/linux/man-pages/man7/socket.7.html for more details. This option is @@ -77,7 +79,6 @@ class TcpTmTcServer : public SystemObject, public TcpIpBase, public ExecutableOb */ bool reusePort = false; - std::string tcpPort = DEFAULT_SERVER_PORT; }; enum class ReceptionModes { SPACE_PACKETS };