From 3429918f5e35ed9ed942c946fdf0d19ed6379f26 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 30 Mar 2021 00:25:22 +0200 Subject: [PATCH 01/35] fixed cmakelists for linux --- osal/CMakeLists.txt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/osal/CMakeLists.txt b/osal/CMakeLists.txt index e4f1de7c..6cda0c88 100644 --- a/osal/CMakeLists.txt +++ b/osal/CMakeLists.txt @@ -10,11 +10,6 @@ elseif(${OS_FSFW} STREQUAL "host") if (WIN32) add_subdirectory(windows) elseif(UNIX) - target_sources(${LIB_FSFW_NAME} - PUBLIC - linux/TcUnixUdpPollingTask.cpp - linux/TmTcUnixUdpBridge.cpp - ) endif () else() From 435c6e641021ce450ba88b1cb57c960a5548f784 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 30 Mar 2021 00:37:10 +0200 Subject: [PATCH 02/35] bugfix for linux/host --- osal/CMakeLists.txt | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/osal/CMakeLists.txt b/osal/CMakeLists.txt index 6cda0c88..76b939b1 100644 --- a/osal/CMakeLists.txt +++ b/osal/CMakeLists.txt @@ -1,30 +1,34 @@ # Check the OS_FSFW variable if(${OS_FSFW} STREQUAL "freertos") - add_subdirectory(FreeRTOS) + add_subdirectory(FreeRTOS) elseif(${OS_FSFW} STREQUAL "rtems") - add_subdirectory(rtems) + add_subdirectory(rtems) elseif(${OS_FSFW} STREQUAL "linux") - add_subdirectory(linux) + add_subdirectory(linux) elseif(${OS_FSFW} STREQUAL "host") - add_subdirectory(host) - if (WIN32) - add_subdirectory(windows) - elseif(UNIX) - endif () + add_subdirectory(host) + if (WIN32) + add_subdirectory(windows) + elseif(UNIX) + # We still need to pull in some Linux specific sources + target_sources(${LIB_FSFW_NAME} PUBLIC + linux/tcpipHelpers.cpp + ) + endif () 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) - elseif (UNIX) - add_subdirectory(linux) - else () - # MacOS or other OSes have not been tested yet / are not supported. - message(FATAL_ERROR "The host OS could not be determined! Aborting.") - endif() + 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) + elseif (UNIX) + add_subdirectory(linux) + else () + # MacOS or other OSes have not been tested yet / are not supported. + message(FATAL_ERROR "The host OS could not be determined! Aborting.") + endif() endif() From d1a256cbf66223fe47dcb1c7b8c825b6fcfa943c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 31 Mar 2021 19:53:58 +0200 Subject: [PATCH 03/35] added missing include --- osal/linux/tcpipHelpers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/osal/linux/tcpipHelpers.cpp b/osal/linux/tcpipHelpers.cpp index 4c1b9a78..3e8f6009 100644 --- a/osal/linux/tcpipHelpers.cpp +++ b/osal/linux/tcpipHelpers.cpp @@ -1,5 +1,6 @@ #include "../common/tcpipHelpers.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../tasks/TaskFactory.h" #include From 0e76333d3315f2278397935d220c20f64b6e838a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 7 Apr 2021 12:03:28 +0200 Subject: [PATCH 04/35] minor improvements for PUS distributor --- tcdistribution/PUSDistributor.cpp | 121 +++++++++++++++++------------- 1 file changed, 68 insertions(+), 53 deletions(-) diff --git a/tcdistribution/PUSDistributor.cpp b/tcdistribution/PUSDistributor.cpp index 00fd9029..0a95c5ef 100644 --- a/tcdistribution/PUSDistributor.cpp +++ b/tcdistribution/PUSDistributor.cpp @@ -1,22 +1,24 @@ #include "CCSDSDistributorIF.h" #include "PUSDistributor.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include "../tmtcpacket/pus/TcPacketStored.h" #include "../tmtcservices/PusVerificationReport.h" +#define PUS_DISTRIBUTOR_DEBUGGING 1 + PUSDistributor::PUSDistributor(uint16_t setApid, object_id_t setObjectId, - object_id_t setPacketSource) : - TcDistributor(setObjectId), checker(setApid), verifyChannel(), - tcStatus(RETURN_FAILED), packetSource(setPacketSource) {} + object_id_t setPacketSource) : + TcDistributor(setObjectId), checker(setApid), verifyChannel(), + tcStatus(RETURN_FAILED), packetSource(setPacketSource) {} PUSDistributor::~PUSDistributor() {} PUSDistributor::TcMqMapIter PUSDistributor::selectDestination() { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - // sif:: debug << "PUSDistributor::handlePacket received: " - // << this->current_packet_id.store_index << ", " - // << this->current_packet_id.packet_index << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && PUS_DISTRIBUTOR_DEBUGGING == 1 + store_address_t storeId = this->currentMessage.getStorageId()); + sif:: debug << "PUSDistributor::handlePacket received: " << storeId.poolIndex << ", " << + storeId.packetIndex << std::endl; #endif TcMqMapIter queueMapIt = this->queueMap.end(); if(this->currentPacket == nullptr) { @@ -25,15 +27,17 @@ PUSDistributor::TcMqMapIter PUSDistributor::selectDestination() { this->currentPacket->setStoreAddress(this->currentMessage.getStorageId()); if (currentPacket->getWholeData() != nullptr) { tcStatus = checker.checkPacket(currentPacket); -#ifdef DEBUG if(tcStatus != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "PUSDistributor::handlePacket: Packet format " - << "invalid, code "<< static_cast(tcStatus) - << std::endl; + sif::debug << "PUSDistributor::handlePacket: Packet format invalid, code " << + static_cast(tcStatus) << std::endl; +#else + sif::printDebug("PUSDistributor::handlePacket: Packet format invalid, code %d\n", + static_cast(tcStatus)); +#endif #endif } -#endif uint32_t queue_id = currentPacket->getService(); queueMapIt = this->queueMap.find(queue_id); } @@ -43,11 +47,12 @@ PUSDistributor::TcMqMapIter PUSDistributor::selectDestination() { if (queueMapIt == this->queueMap.end()) { tcStatus = DESTINATION_NOT_FOUND; -#ifdef DEBUG +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::debug << "PUSDistributor::handlePacket: Destination not found, " - << "code "<< static_cast(tcStatus) << std::endl; -#endif + sif::debug << "PUSDistributor::handlePacket: Destination not found" << std::endl; +#else + sif::printDebug("PUSDistributor::handlePacket: Destination not found\n"); +#endif /* !FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif } @@ -62,46 +67,54 @@ PUSDistributor::TcMqMapIter PUSDistributor::selectDestination() { ReturnValue_t PUSDistributor::registerService(AcceptsTelecommandsIF* service) { - uint16_t serviceId = service->getIdentifier(); + uint16_t serviceId = service->getIdentifier(); +#if PUS_DISTRIBUTOR_DEBUGGING == 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - // sif::info << "Service ID: " << (int)serviceId << std::endl; + sif::info << "Service ID: " << static_cast(serviceId) << std::endl; +#else + sif::printInfo("Service ID: %d\n", static_cast(serviceId)); #endif - MessageQueueId_t queue = service->getRequestQueue(); - auto returnPair = queueMap.emplace(serviceId, queue); - if (not returnPair.second) { +#endif + MessageQueueId_t queue = service->getRequestQueue(); + auto returnPair = queueMap.emplace(serviceId, queue); + if (not returnPair.second) { +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "PUSDistributor::registerService: Service ID already" - " exists in map." << std::endl; + sif::error << "PUSDistributor::registerService: Service ID already" + " exists in map" << std::endl; +#else + sif::printError("PUSDistributor::registerService: Service ID already exists in map\n"); #endif - return SERVICE_ID_ALREADY_EXISTS; - } - return HasReturnvaluesIF::RETURN_OK; +#endif + return SERVICE_ID_ALREADY_EXISTS; + } + return HasReturnvaluesIF::RETURN_OK; } MessageQueueId_t PUSDistributor::getRequestQueue() { - return tcQueue->getId(); + return tcQueue->getId(); } ReturnValue_t PUSDistributor::callbackAfterSending(ReturnValue_t queueStatus) { - if (queueStatus != RETURN_OK) { - tcStatus = queueStatus; - } - if (tcStatus != RETURN_OK) { - this->verifyChannel.sendFailureReport(tc_verification::ACCEPTANCE_FAILURE, - currentPacket, tcStatus); - // A failed packet is deleted immediately after reporting, - // otherwise it will block memory. - currentPacket->deletePacket(); - return RETURN_FAILED; - } else { - this->verifyChannel.sendSuccessReport(tc_verification::ACCEPTANCE_SUCCESS, - currentPacket); - return RETURN_OK; - } + if (queueStatus != RETURN_OK) { + tcStatus = queueStatus; + } + if (tcStatus != RETURN_OK) { + this->verifyChannel.sendFailureReport(tc_verification::ACCEPTANCE_FAILURE, + currentPacket, tcStatus); + // A failed packet is deleted immediately after reporting, + // otherwise it will block memory. + currentPacket->deletePacket(); + return RETURN_FAILED; + } else { + this->verifyChannel.sendSuccessReport(tc_verification::ACCEPTANCE_SUCCESS, + currentPacket); + return RETURN_OK; + } } uint16_t PUSDistributor::getIdentifier() { - return checker.getApid(); + return checker.getApid(); } ReturnValue_t PUSDistributor::initialize() { @@ -111,15 +124,17 @@ ReturnValue_t PUSDistributor::initialize() { return ObjectManagerIF::CHILD_INIT_FAILED; } - CCSDSDistributorIF* ccsdsDistributor = - objectManager->get(packetSource); - if (ccsdsDistributor == nullptr) { + CCSDSDistributorIF* ccsdsDistributor = + objectManager->get(packetSource); + if (ccsdsDistributor == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "PUSDistributor::initialize: Packet source invalid." - << " Make sure it exists and implements CCSDSDistributorIF!" - << std::endl; + sif::error << "PUSDistributor::initialize: Packet source invalid" << std::endl; + sif::error << " Make sure it exists and implements CCSDSDistributorIF!" << std::endl; +#else + sif::printError("PUSDistributor::initialize: Packet source invalid\n"); + sif::printError("Make sure it exists and implements CCSDSDistributorIF\n"); #endif - return RETURN_FAILED; - } - return ccsdsDistributor->registerApplication(this); + return RETURN_FAILED; + } + return ccsdsDistributor->registerApplication(this); } From 7322a7d0f53e0ae72b69cac1a4abffbe74543365 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 7 Apr 2021 12:09:06 +0200 Subject: [PATCH 05/35] improvements for ccsds distributor --- tcdistribution/CCSDSDistributor.cpp | 31 ++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tcdistribution/CCSDSDistributor.cpp b/tcdistribution/CCSDSDistributor.cpp index b795854f..f3864066 100644 --- a/tcdistribution/CCSDSDistributor.cpp +++ b/tcdistribution/CCSDSDistributor.cpp @@ -1,8 +1,10 @@ #include "CCSDSDistributor.h" -#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterface.h" #include "../tmtcpacket/SpacePacketBase.h" +#define CCSDS_DISTRIBUTOR_DEBUGGING 1 + CCSDSDistributor::CCSDSDistributor(uint16_t setDefaultApid, object_id_t setObjectId): TcDistributor(setObjectId), defaultApid( setDefaultApid ) { @@ -11,26 +13,36 @@ CCSDSDistributor::CCSDSDistributor(uint16_t setDefaultApid, CCSDSDistributor::~CCSDSDistributor() {} TcDistributor::TcMqMapIter CCSDSDistributor::selectDestination() { +#if CCSDS_DISTRIBUTOR_DEBUGGING == 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 -// sif::debug << "CCSDSDistributor::selectDestination received: " << -// this->currentMessage.getStorageId().pool_index << ", " << -// this->currentMessage.getStorageId().packet_index << std::endl; + sif::debug << "CCSDSDistributor::selectDestination received: " << + this->currentMessage.getStorageId().poolIndex << ", " << + this->currentMessage.getStorageId().packetIndex << std::endl; +#else + sif::printDebug("CCSDSDistributor::selectDestination received: %d, %d\n", + currentMessage.getStorageId().poolIndex, currentMessage.getStorageId().packetIndex); +#endif #endif const uint8_t* packet = nullptr; size_t size = 0; ReturnValue_t result = this->tcStore->getData(currentMessage.getStorageId(), &packet, &size ); if(result != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "CCSDSDistributor::selectDestination: Getting data from" " store failed!" << std::endl; +#else + sif::printError("CCSDSDistributor::selectDestination: Getting data from" + " store failed!\n"); +#endif #endif } SpacePacketBase currentPacket(packet); -#if FSFW_CPP_OSTREAM_ENABLED == 1 -// sif:: info << "CCSDSDistributor::selectDestination has packet with APID " -// << std::hex << currentPacket.getAPID() << std::dec << std::endl; +#if FSFW_CPP_OSTREAM_ENABLED == 1 && CCSDS_DISTRIBUTOR_DEBUGGING == 1 + sif::info << "CCSDSDistributor::selectDestination has packet with APID " << std::hex << + currentPacket.getAPID() << std::dec << std::endl; #endif TcMqMapIter position = this->queueMap.find(currentPacket.getAPID()); if ( position != this->queueMap.end() ) { @@ -76,9 +88,14 @@ ReturnValue_t CCSDSDistributor::initialize() { ReturnValue_t status = this->TcDistributor::initialize(); this->tcStore = objectManager->get( objects::TC_STORE ); if (this->tcStore == nullptr) { +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "CCSDSDistributor::initialize: Could not initialize" " TC store!" << std::endl; +#else + sif::printError("CCSDSDistributor::initialize: Could not initialize" + " TC store!\n"); +#endif #endif status = RETURN_FAILED; } From 3ff54844153aa81cbb0c5c6b5546be4bd4a30904 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 7 Apr 2021 13:44:03 +0200 Subject: [PATCH 06/35] disabled debugging mode --- tcdistribution/CCSDSDistributor.cpp | 2 +- tcdistribution/PUSDistributor.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tcdistribution/CCSDSDistributor.cpp b/tcdistribution/CCSDSDistributor.cpp index f3864066..62cbfbf2 100644 --- a/tcdistribution/CCSDSDistributor.cpp +++ b/tcdistribution/CCSDSDistributor.cpp @@ -3,7 +3,7 @@ #include "../serviceinterface/ServiceInterface.h" #include "../tmtcpacket/SpacePacketBase.h" -#define CCSDS_DISTRIBUTOR_DEBUGGING 1 +#define CCSDS_DISTRIBUTOR_DEBUGGING 0 CCSDSDistributor::CCSDSDistributor(uint16_t setDefaultApid, object_id_t setObjectId): diff --git a/tcdistribution/PUSDistributor.cpp b/tcdistribution/PUSDistributor.cpp index 0a95c5ef..abdd1f8d 100644 --- a/tcdistribution/PUSDistributor.cpp +++ b/tcdistribution/PUSDistributor.cpp @@ -5,7 +5,7 @@ #include "../tmtcpacket/pus/TcPacketStored.h" #include "../tmtcservices/PusVerificationReport.h" -#define PUS_DISTRIBUTOR_DEBUGGING 1 +#define PUS_DISTRIBUTOR_DEBUGGING 0 PUSDistributor::PUSDistributor(uint16_t setApid, object_id_t setObjectId, object_id_t setPacketSource) : From 43ddb445737575a8847e895aea279a57f4e0a6a1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 7 Apr 2021 22:12:01 +0200 Subject: [PATCH 07/35] fixed temperature sensor object --- thermal/TemperatureSensor.h | 54 +++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index 2b1fb1f0..ceb8a861 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -1,11 +1,14 @@ #ifndef TEMPERATURESENSOR_H_ #define TEMPERATURESENSOR_H_ -#include "../thermal/AbstractTemperatureSensor.h" -#include "../datapoolglob/GlobalDataSet.h" -#include "../datapoolglob/GlobalPoolVariable.h" +#include "tcsDefinitions.h" +#include "AbstractTemperatureSensor.h" + +#include "../datapoollocal/LocalPoolDataSetBase.h" +#include "../datapoollocal/LocalPoolVariable.h" #include "../monitoring/LimitMonitor.h" + /** * @brief This building block handles non-linear value conversion and * range checks for analog temperature sensors. @@ -57,27 +60,26 @@ public: /** * Instantiate Temperature Sensor Object. - * @param setObjectid objectId of the sensor object - * @param inputValue Input value which is converted to a temperature - * @param poolVariable Pool Variable to store the temperature value - * @param vectorIndex Vector Index for the sensor monitor - * @param parameters Calculation parameters, temperature limits, gradient limit - * @param datapoolId Datapool ID of the output temperature - * @param outputSet Output dataset for the output temperature to fetch it with read() + * @param setObjectid objectId of the sensor object + * @param inputValue Pointer to input value which is converted to a temperature + * @param variableGpid Global Pool ID of the output value + * @param inputVariable Input variable handle + * @param vectorIndex Vector Index for the sensor monitor + * @param parameters Calculation parameters, temperature limits, gradient limit + * @param outputSet Output dataset for the output temperature to fetch it with read() * @param thermalModule respective thermal module, if it has one */ TemperatureSensor(object_id_t setObjectid, - inputType *inputValue, PoolVariableIF *poolVariable, - uint8_t vectorIndex, uint32_t datapoolId, Parameters parameters = {0, 0, 0, 0, 0, 0}, - GlobDataSet *outputSet = NULL, ThermalModuleIF *thermalModule = NULL) : + inputType *inputValue, gp_id_t variableGpid, PoolVariableIF* inputVariable, + uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, + LocalPoolDataSetBase *outputSet = NULL, ThermalModuleIF *thermalModule = NULL) : AbstractTemperatureSensor(setObjectid, thermalModule), parameters(parameters), - inputValue(inputValue), poolVariable(poolVariable), - outputTemperature(datapoolId, outputSet, PoolVariableIF::VAR_WRITE), - sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, - GlobalDataPool::poolIdAndPositionToPid(poolVariable->getDataPoolId(), vectorIndex), + inputValue(inputValue), poolVariable(inputVariable), + outputTemperature(variableGpid, outputSet, PoolVariableIF::VAR_WRITE), + sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, poolVariable, DEFAULT_CONFIRMATION_COUNT, parameters.lowerLimit, parameters.upperLimit, TEMP_SENSOR_LOW, TEMP_SENSOR_HIGH), - oldTemperature(20), uptimeOfOldTemperature( { INVALID_TEMPERATURE, 0 }) { + oldTemperature(20), uptimeOfOldTemperature({ thermal::INVALID_TEMPERATURE, 0 }) { } @@ -98,7 +100,7 @@ protected: private: void setInvalid() { - outputTemperature = INVALID_TEMPERATURE; + outputTemperature = thermal::INVALID_TEMPERATURE; outputTemperature.setValid(false); uptimeOfOldTemperature.tv_sec = INVALID_UPTIME; sensorMonitor.setToInvalid(); @@ -108,11 +110,11 @@ protected: UsedParameters parameters; - inputType * inputValue; + inputType* inputValue; - PoolVariableIF *poolVariable; + PoolVariableIF* poolVariable; - gp_float_t outputTemperature; + lp_var_t outputTemperature; LimitMonitor sensorMonitor; @@ -120,8 +122,8 @@ protected: timeval uptimeOfOldTemperature; void doChildOperation() { - if (!poolVariable->isValid() - || !healthHelper.healthTable->isHealthy(getObjectId())) { + if ((not poolVariable->isValid()) or + (not healthHelper.healthTable->isHealthy(getObjectId()))) { setInvalid(); return; } @@ -152,13 +154,13 @@ protected: } } - //Check is done against raw limits. SHOULDDO: Why? Using �C would be more easy to handle. + //Check is done against raw limits. SHOULDDO: Why? Using C would be more easy to handle. sensorMonitor.doCheck(outputTemperature.value); if (sensorMonitor.isOutOfLimits()) { uptimeOfOldTemperature.tv_sec = INVALID_UPTIME; outputTemperature.setValid(PoolVariableIF::INVALID); - outputTemperature = INVALID_TEMPERATURE; + outputTemperature = thermal::INVALID_TEMPERATURE; } else { oldTemperature = outputTemperature; uptimeOfOldTemperature = uptime; From 80aab5f461fa0f462aff4743c95413299642ce06 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 15:25:22 +0200 Subject: [PATCH 08/35] super evil bug --- osal/linux/MessageQueue.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index 60d15dee..b40ff29b 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -190,13 +190,14 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { return HasReturnvaluesIF::RETURN_FAILED; } return HasReturnvaluesIF::RETURN_OK; - }else if(status==0){ + } + else if(status==0) { //Success but no message received return MessageQueueIF::EMPTY; } else { //No message was received. Keep lastPartner anyway, I might send //something later. But still, delete packet content. - memset(message->getData(), 0, message->getMaximumMessageSize()); + memset(message->getBuffer(), 0, message->getMaximumMessageSize()); switch(errno){ case EAGAIN: //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages From 6983ddc3e022bfcf1e78354998e4577b20e146b0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 15:27:03 +0200 Subject: [PATCH 09/35] super evil bugfix --- osal/linux/MessageQueue.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index 60d15dee..b40ff29b 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -190,13 +190,14 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { return HasReturnvaluesIF::RETURN_FAILED; } return HasReturnvaluesIF::RETURN_OK; - }else if(status==0){ + } + else if(status==0) { //Success but no message received return MessageQueueIF::EMPTY; } else { //No message was received. Keep lastPartner anyway, I might send //something later. But still, delete packet content. - memset(message->getData(), 0, message->getMaximumMessageSize()); + memset(message->getBuffer(), 0, message->getMaximumMessageSize()); switch(errno){ case EAGAIN: //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages From 9ee1bd15c4f82e10edc8aba250686ed70f1699ae Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 15:38:36 +0200 Subject: [PATCH 10/35] now fixed properly --- ipc/MessageQueueMessage.cpp | 4 ++++ ipc/MessageQueueMessage.h | 1 + ipc/MessageQueueMessageIF.h | 1 + osal/linux/MessageQueue.cpp | 7 ++++--- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index e97778c3..1958af54 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -86,3 +86,7 @@ size_t MessageQueueMessage::getMaximumMessageSize() const { return this->MAX_MESSAGE_SIZE; } +size_t MessageQueueMessage::getMaximumDataSize() const { + return this->MAX_DATA_SIZE; +} + diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 5234f64f..111056ca 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -139,6 +139,7 @@ public: virtual void setMessageSize(size_t messageSize) override; virtual size_t getMinimumMessageSize() const override; virtual size_t getMaximumMessageSize() const override; + virtual size_t getMaximumDataSize() const override; /** * @brief This is a debug method that prints the content. diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 33e01e7d..893c30b5 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -72,6 +72,7 @@ public: virtual void setMessageSize(size_t messageSize) = 0; virtual size_t getMinimumMessageSize() const = 0; virtual size_t getMaximumMessageSize() const = 0; + virtual size_t getMaximumDataSize() const = 0; }; diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index b40ff29b..8004c045 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -191,13 +191,14 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { } return HasReturnvaluesIF::RETURN_OK; } - else if(status==0) { + else if (status==0) { //Success but no message received return MessageQueueIF::EMPTY; - } else { + } + else { //No message was received. Keep lastPartner anyway, I might send //something later. But still, delete packet content. - memset(message->getBuffer(), 0, message->getMaximumMessageSize()); + memset(message->getData(), 0, message->getMaximumDataSize()); switch(errno){ case EAGAIN: //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages From 23e3f2f34f1b68998a8aad6c01691d2e9c97ed65 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 15:39:23 +0200 Subject: [PATCH 11/35] now fixed properly --- ipc/MessageQueueMessage.cpp | 4 ++++ ipc/MessageQueueMessage.h | 1 + ipc/MessageQueueMessageIF.h | 1 + osal/linux/MessageQueue.cpp | 7 ++++--- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index e97778c3..1958af54 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -86,3 +86,7 @@ size_t MessageQueueMessage::getMaximumMessageSize() const { return this->MAX_MESSAGE_SIZE; } +size_t MessageQueueMessage::getMaximumDataSize() const { + return this->MAX_DATA_SIZE; +} + diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 5234f64f..111056ca 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -139,6 +139,7 @@ public: virtual void setMessageSize(size_t messageSize) override; virtual size_t getMinimumMessageSize() const override; virtual size_t getMaximumMessageSize() const override; + virtual size_t getMaximumDataSize() const override; /** * @brief This is a debug method that prints the content. diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 33e01e7d..893c30b5 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -72,6 +72,7 @@ public: virtual void setMessageSize(size_t messageSize) = 0; virtual size_t getMinimumMessageSize() const = 0; virtual size_t getMaximumMessageSize() const = 0; + virtual size_t getMaximumDataSize() const = 0; }; diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index b40ff29b..8004c045 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -191,13 +191,14 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { } return HasReturnvaluesIF::RETURN_OK; } - else if(status==0) { + else if (status==0) { //Success but no message received return MessageQueueIF::EMPTY; - } else { + } + else { //No message was received. Keep lastPartner anyway, I might send //something later. But still, delete packet content. - memset(message->getBuffer(), 0, message->getMaximumMessageSize()); + memset(message->getData(), 0, message->getMaximumDataSize()); switch(errno){ case EAGAIN: //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages From e799e45198c19793d99a530e0d496f89d08107b3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 16:20:24 +0200 Subject: [PATCH 12/35] bugfix for RTEMS --- osal/rtems/MessageQueue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/rtems/MessageQueue.cpp b/osal/rtems/MessageQueue.cpp index bfaf3569..717b80dd 100644 --- a/osal/rtems/MessageQueue.cpp +++ b/osal/rtems/MessageQueue.cpp @@ -61,7 +61,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { } else { //No message was received. Keep lastPartner anyway, I might send something later. //But still, delete packet content. - memset(message->getData(), 0, message->getMaximumMessageSize()); + memset(message->getData(), 0, message->getMaximumDataSize()); } return convertReturnCode(status); } From 10c3483fe51bfaac7f385332fb0fbc3d9ff1f57e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 16:20:59 +0200 Subject: [PATCH 13/35] bugfix for RTEMS --- osal/rtems/MessageQueue.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/rtems/MessageQueue.cpp b/osal/rtems/MessageQueue.cpp index bfaf3569..717b80dd 100644 --- a/osal/rtems/MessageQueue.cpp +++ b/osal/rtems/MessageQueue.cpp @@ -61,7 +61,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { } else { //No message was received. Keep lastPartner anyway, I might send something later. //But still, delete packet content. - memset(message->getData(), 0, message->getMaximumMessageSize()); + memset(message->getData(), 0, message->getMaximumDataSize()); } return convertReturnCode(status); } From 38d929c2a8df4451129bad3b464f3119fffe96ab Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 18:48:44 +0200 Subject: [PATCH 14/35] coverity fixes --- osal/common/TcpTmTcServer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/osal/common/TcpTmTcServer.cpp b/osal/common/TcpTmTcServer.cpp index 296afad8..2bfd4876 100644 --- a/osal/common/TcpTmTcServer.cpp +++ b/osal/common/TcpTmTcServer.cpp @@ -70,6 +70,7 @@ ReturnValue_t TcpTmTcServer::initialize() { #endif freeaddrinfo(addrResult); handleError(Protocol::TCP, ErrorSources::BIND_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } freeaddrinfo(addrResult); From f988271be4dbae63963755871451dc95d3085efd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 18:53:22 +0200 Subject: [PATCH 15/35] coverity fixes --- container/SharedRingBuffer.cpp | 3 +++ container/SharedRingBuffer.h | 23 +++++++++++++---------- osal/common/TcpTmTcServer.cpp | 6 ++++-- osal/common/UdpTmTcBridge.cpp | 1 + 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/container/SharedRingBuffer.cpp b/container/SharedRingBuffer.cpp index 5e20bb6f..fe36341d 100644 --- a/container/SharedRingBuffer.cpp +++ b/container/SharedRingBuffer.cpp @@ -17,6 +17,9 @@ SharedRingBuffer::SharedRingBuffer(object_id_t objectId, uint8_t *buffer, mutex = MutexFactory::instance()->createMutex(); } +SharedRingBuffer::~SharedRingBuffer() { + MutexFactory::instance()->deleteMutex(mutex); +} void SharedRingBuffer::setToUseReceiveSizeFIFO(size_t fifoDepth) { this->fifoDepth = fifoDepth; diff --git a/container/SharedRingBuffer.h b/container/SharedRingBuffer.h index 66a119ab..9d6ea56c 100644 --- a/container/SharedRingBuffer.h +++ b/container/SharedRingBuffer.h @@ -26,6 +26,18 @@ public: */ SharedRingBuffer(object_id_t objectId, const size_t size, bool overwriteOld, size_t maxExcessBytes); + /** + * This constructor takes an external buffer with the specified size. + * @param buffer + * @param size + * @param overwriteOld + * If the ring buffer is overflowing at a write operartion, the oldest data + * will be overwritten. + */ + SharedRingBuffer(object_id_t objectId, uint8_t* buffer, const size_t size, + bool overwriteOld, size_t maxExcessBytes); + + virtual~ SharedRingBuffer(); /** * @brief This function can be used to add an optional FIFO to the class @@ -37,16 +49,7 @@ public: */ void setToUseReceiveSizeFIFO(size_t fifoDepth); - /** - * This constructor takes an external buffer with the specified size. - * @param buffer - * @param size - * @param overwriteOld - * If the ring buffer is overflowing at a write operartion, the oldest data - * will be overwritten. - */ - SharedRingBuffer(object_id_t objectId, uint8_t* buffer, const size_t size, - bool overwriteOld, size_t maxExcessBytes); + /** * Unless a read-only constant value is read, all operations on the diff --git a/osal/common/TcpTmTcServer.cpp b/osal/common/TcpTmTcServer.cpp index 2bfd4876..08a62ffb 100644 --- a/osal/common/TcpTmTcServer.cpp +++ b/osal/common/TcpTmTcServer.cpp @@ -85,8 +85,8 @@ TcpTmTcServer::~TcpTmTcServer() { 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 clientSocket; - sockaddr clientSockAddr; + socket_t clientSocket = 0; + sockaddr clientSockAddr = {}; socklen_t connectorSockAddrLen = 0; int retval = 0; @@ -102,6 +102,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { if(clientSocket == INVALID_SOCKET) { handleError(Protocol::TCP, ErrorSources::ACCEPT_CALL, 500); + closeSocket(clientSocket); continue; }; @@ -123,6 +124,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { /* Done, shut down connection */ retval = shutdown(clientSocket, SHUT_SEND); + closeSocket(clientSocket); } return HasReturnvaluesIF::RETURN_OK; } diff --git a/osal/common/UdpTmTcBridge.cpp b/osal/common/UdpTmTcBridge.cpp index 7f3dc929..798be6f5 100644 --- a/osal/common/UdpTmTcBridge.cpp +++ b/osal/common/UdpTmTcBridge.cpp @@ -103,6 +103,7 @@ ReturnValue_t UdpTmTcBridge::initialize() { #endif freeaddrinfo(addrResult); tcpip::handleError(tcpip::Protocol::UDP, tcpip::ErrorSources::BIND_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } freeaddrinfo(addrResult); return HasReturnvaluesIF::RETURN_OK; From 00d9a4f3ed44b1bc929271f93f126105aa866616 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 18:57:24 +0200 Subject: [PATCH 16/35] more coverity fixes --- osal/host/FixedTimeslotTask.cpp | 1 - osal/host/PeriodicTask.cpp | 1 - osal/host/QueueMapManager.cpp | 4 ++++ osal/host/QueueMapManager.h | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/osal/host/FixedTimeslotTask.cpp b/osal/host/FixedTimeslotTask.cpp index 3b169b5a..ee149922 100644 --- a/osal/host/FixedTimeslotTask.cpp +++ b/osal/host/FixedTimeslotTask.cpp @@ -38,7 +38,6 @@ FixedTimeslotTask::~FixedTimeslotTask(void) { if(mainThread.joinable()) { mainThread.join(); } - delete this; } void FixedTimeslotTask::taskEntryPoint(void* argument) { diff --git a/osal/host/PeriodicTask.cpp b/osal/host/PeriodicTask.cpp index d7abf9d0..631264b7 100644 --- a/osal/host/PeriodicTask.cpp +++ b/osal/host/PeriodicTask.cpp @@ -38,7 +38,6 @@ PeriodicTask::~PeriodicTask(void) { if(mainThread.joinable()) { mainThread.join(); } - delete this; } void PeriodicTask::taskEntryPoint(void* argument) { diff --git a/osal/host/QueueMapManager.cpp b/osal/host/QueueMapManager.cpp index b50d62dc..c9100fe9 100644 --- a/osal/host/QueueMapManager.cpp +++ b/osal/host/QueueMapManager.cpp @@ -10,6 +10,10 @@ QueueMapManager::QueueMapManager() { mapLock = MutexFactory::instance()->createMutex(); } +QueueMapManager::~QueueMapManager() { + MutexFactory::instance()->deleteMutex(mapLock); +} + QueueMapManager* QueueMapManager::instance() { if (mqManagerInstance == nullptr){ mqManagerInstance = new QueueMapManager(); diff --git a/osal/host/QueueMapManager.h b/osal/host/QueueMapManager.h index 3610ca63..90c39c2f 100644 --- a/osal/host/QueueMapManager.h +++ b/osal/host/QueueMapManager.h @@ -36,6 +36,8 @@ public: private: //! External instantiation is forbidden. QueueMapManager(); + ~QueueMapManager(); + uint32_t queueCounter = 0; MutexIF* mapLock; QueueMap queueMap; From bddd8720b2898d785f6647d9d383ff65c3d2c4e0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 18:59:48 +0200 Subject: [PATCH 17/35] another coverity fix --- osal/host/FixedTimeslotTask.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osal/host/FixedTimeslotTask.cpp b/osal/host/FixedTimeslotTask.cpp index ee149922..016b1404 100644 --- a/osal/host/FixedTimeslotTask.cpp +++ b/osal/host/FixedTimeslotTask.cpp @@ -118,8 +118,11 @@ ReturnValue_t FixedTimeslotTask::addSlot(object_id_t componentId, } #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "Component " << std::hex << componentId << - " not found, not adding it to pst" << std::endl; + sif::error << "Component " << std::hex << "0x" << componentId << "not found, " + "not adding it to PST.." << std::dec << std::endl; +#else + sif::printError("Component 0x%08x not found, not adding it to PST..\n", + static_cast(componentId)); #endif return HasReturnvaluesIF::RETURN_FAILED; } From efb7c8760a864e45e5686ce450fb8f7b06dc7500 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:01:06 +0200 Subject: [PATCH 18/35] coverity action helper fix --- action/ActionHelper.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 4b64a40c..b2374ed6 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -147,11 +147,6 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, return result; } - if (result != HasReturnvaluesIF::RETURN_OK) { - ipcStore->deleteData(storeAddress); - return result; - } - /* We don't need to report the objectId, as we receive REQUESTED data before the completion success message. True aperiodic replies need to be reported with another dedicated message. */ ActionMessage::setDataReply(&reply, replyId, storeAddress); From b30a3aaa382c136df69329e16bae1dfac5f82aa1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:07:03 +0200 Subject: [PATCH 19/35] coverity fix --- osal/host/MessageQueue.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 18272a68..359ce8ec 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -106,6 +106,9 @@ bool MessageQueue::isDefaultDestinationSet() const { ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { + if(message == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } message->setSender(sentFrom); if(message->getMessageSize() > message->getMaximumMessageSize()) { // Actually, this should never happen or an error will be emitted @@ -128,12 +131,12 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, return HasReturnvaluesIF::RETURN_FAILED; } if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { - MutexGuard mutexLock(targetQueue->queueLock, - MutexIF::TimeoutType::WAITING, 20); - // not ideal, works for now though. - MessageQueueMessage* mqmMessage = - dynamic_cast(message); - if(message != nullptr) { + MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20); + // TODO: Would be nice to support other message types, but this would require + // create the message on the heap from an upper layer and simply storing + // MessageQueueMessageIF pointers in the queue. + MessageQueueMessage* mqmMessage = dynamic_cast(message); + if(mqmMessage != nullptr) { targetQueue->messageQueue.push(*mqmMessage); } else { From 8f4ab6d7edd963166e9f4f0845ff85c9882aeb5a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:07:51 +0200 Subject: [PATCH 20/35] coverity: initialize member --- serialize/SerializeElement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serialize/SerializeElement.h b/serialize/SerializeElement.h index 47080292..d41098d8 100644 --- a/serialize/SerializeElement.h +++ b/serialize/SerializeElement.h @@ -57,7 +57,7 @@ public: return &entry; } - T entry; + T entry = 0; }; #endif /* FSFW_SERIALIZE_SERIALIZEELEMENT_H_ */ From dd367bf083422f24f0a2e9aea4ea8b9f213499ce Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:09:00 +0200 Subject: [PATCH 21/35] coverity: initialize entry --- serialize/SerializeElement.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serialize/SerializeElement.h b/serialize/SerializeElement.h index d41098d8..ba34393d 100644 --- a/serialize/SerializeElement.h +++ b/serialize/SerializeElement.h @@ -25,7 +25,7 @@ public: } SerializeElement() : - LinkedElement(this) { + LinkedElement(this), entry(0) { } ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize, @@ -57,7 +57,7 @@ public: return &entry; } - T entry = 0; + T entry; }; #endif /* FSFW_SERIALIZE_SERIALIZEELEMENT_H_ */ From e6a1a7cc2d5d5f17f02c70e8bb677d75cde5712e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:10:49 +0200 Subject: [PATCH 22/35] coverity fix for event message --- events/EventMessage.cpp | 2 +- events/EventMessage.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/events/EventMessage.cpp b/events/EventMessage.cpp index bbc41e10..548b4f0f 100644 --- a/events/EventMessage.cpp +++ b/events/EventMessage.cpp @@ -109,6 +109,6 @@ bool EventMessage::isClearedEventMessage() { return getEvent() == INVALID_EVENT; } -size_t EventMessage::getMinimumMessageSize() { +size_t EventMessage::getMinimumMessageSize() const { return EVENT_MESSAGE_SIZE; } diff --git a/events/EventMessage.h b/events/EventMessage.h index 4d003bd7..f2f5ffb5 100644 --- a/events/EventMessage.h +++ b/events/EventMessage.h @@ -45,7 +45,7 @@ public: protected: static const Event INVALID_EVENT = 0; - virtual size_t getMinimumMessageSize(); + virtual size_t getMinimumMessageSize() const override; }; From 905d525aa27069e9948861879a5e104a35e40a4b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 19:12:21 +0200 Subject: [PATCH 23/35] small fix DHB --- devicehandlers/DeviceHandlerBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 15eac11f..4a0b3582 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1483,7 +1483,7 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, if(errorCode == ObjectManagerIF::CHILD_INIT_FAILED) { errorPrint = "Initialization error"; } - if(errorCode == HasReturnvaluesIF::RETURN_FAILED) { + else if(errorCode == HasReturnvaluesIF::RETURN_FAILED) { if(errorType == sif::OutputTypes::OUT_WARNING) { errorPrint = "Generic Warning"; } From 2b84ab018c03774e6a8caff9bb87e9844498ec71 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 20:53:54 +0200 Subject: [PATCH 24/35] implemented mq properly --- osal/host/MessageQueue.cpp | 24 ++++++------------------ osal/host/MessageQueue.h | 2 +- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 359ce8ec..41c55a3d 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -64,9 +64,8 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { return MessageQueueIF::EMPTY; } MutexGuard mutexLock(queueLock, MutexIF::TimeoutType::WAITING, 20); - MessageQueueMessage* currentMessage = &messageQueue.front(); - std::copy(currentMessage->getBuffer(), - currentMessage->getBuffer() + messageSize, message->getBuffer()); + std::copy(messageQueue.front().data(), messageQueue.front().data() + messageSize, + message->getBuffer()); messageQueue.pop(); // The last partner is the first uint32_t field in the message this->lastPartner = message->getSender(); @@ -80,7 +79,7 @@ MessageQueueId_t MessageQueue::getLastPartner() const { ReturnValue_t MessageQueue::flush(uint32_t* count) { *count = messageQueue.size(); // Clears the queue. - messageQueue = std::queue(); + messageQueue = std::queue>(); return HasReturnvaluesIF::RETURN_OK; } @@ -132,20 +131,9 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, } if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20); - // TODO: Would be nice to support other message types, but this would require - // create the message on the heap from an upper layer and simply storing - // MessageQueueMessageIF pointers in the queue. - MessageQueueMessage* mqmMessage = dynamic_cast(message); - if(mqmMessage != nullptr) { - targetQueue->messageQueue.push(*mqmMessage); - } - else { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message" - "is not MessageQueueMessage!" << std::endl; -#endif - } - + targetQueue->messageQueue.push(std::vector(message->getMaximumMessageSize())); + memcpy(targetQueue->messageQueue.back().data(), message->getBuffer(), + message->getMaximumMessageSize()); } else { return MessageQueueIF::FULL; diff --git a/osal/host/MessageQueue.h b/osal/host/MessageQueue.h index 97a9e491..e965123d 100644 --- a/osal/host/MessageQueue.h +++ b/osal/host/MessageQueue.h @@ -212,7 +212,7 @@ protected: //static ReturnValue_t handleSendResult(BaseType_t result, bool ignoreFault); private: - std::queue messageQueue; + std::queue> messageQueue; /** * @brief The class stores the queue id it got assigned. * If initialization fails, the queue id is set to zero. From c1f4ae08fb4e1c76592735b1dd0741fda14a772d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 22:28:44 +0200 Subject: [PATCH 25/35] typo fix --- ipc/MessageQueueIF.h | 2 +- osal/FreeRTOS/MessageQueue.cpp | 2 +- osal/linux/MessageQueue.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ipc/MessageQueueIF.h b/ipc/MessageQueueIF.h index 1c06521c..74ccb29a 100644 --- a/ipc/MessageQueueIF.h +++ b/ipc/MessageQueueIF.h @@ -27,7 +27,7 @@ public: //! Returned if a reply method was called without partner static const ReturnValue_t NO_REPLY_PARTNER = MAKE_RETURN_CODE(3); //! Returned if the target destination is invalid. - static constexpr ReturnValue_t DESTINVATION_INVALID = MAKE_RETURN_CODE(4); + static constexpr ReturnValue_t DESTINATION_INVALID = MAKE_RETURN_CODE(4); virtual ~MessageQueueIF() {} /** diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index c0c82cf1..3a0f654e 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -135,7 +135,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, QueueHandle_t destination = nullptr; if(sendTo == MessageQueueIF::NO_QUEUE or sendTo == 0x00) { - return MessageQueueIF::DESTINVATION_INVALID; + return MessageQueueIF::DESTINATION_INVALID; } else { destination = reinterpret_cast(sendTo); diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index 8004c045..12774a58 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -373,7 +373,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, <<"mq_send to: " << sendTo << " sent from " << sentFrom << std::endl; #endif - return DESTINVATION_INVALID; + return DESTINATION_INVALID; } case EINTR: //The call was interrupted by a signal. From d92a20a669d5918422833aaf09b9934a59d8d789 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 8 Apr 2021 22:49:36 +0200 Subject: [PATCH 26/35] coverity --- datapool/HkSwitchHelper.cpp | 2 +- globalfunctions/arrayprinter.cpp | 4 ++-- osal/FreeRTOS/Clock.cpp | 2 +- pus/Service5EventReporting.cpp | 4 +++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/datapool/HkSwitchHelper.cpp b/datapool/HkSwitchHelper.cpp index 1a2a25eb..21e37f59 100644 --- a/datapool/HkSwitchHelper.cpp +++ b/datapool/HkSwitchHelper.cpp @@ -7,7 +7,7 @@ HkSwitchHelper::HkSwitchHelper(EventReportingProxyIF* eventProxy) : } HkSwitchHelper::~HkSwitchHelper() { - // TODO Auto-generated destructor stub + QueueFactory::instance()->deleteMessageQueue(actionQueue); } ReturnValue_t HkSwitchHelper::initialize() { diff --git a/globalfunctions/arrayprinter.cpp b/globalfunctions/arrayprinter.cpp index 20a64f5b..0423360b 100644 --- a/globalfunctions/arrayprinter.cpp +++ b/globalfunctions/arrayprinter.cpp @@ -51,7 +51,7 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, #else // General format: 0x01, 0x02, 0x03 so it is number of chars times 6 // plus line break plus small safety margin. - char printBuffer[(size + 1) * 7 + 1]; + char printBuffer[(size + 1) * 7 + 1] = {}; size_t currentPos = 0; for(size_t i = 0; i < size; i++) { // To avoid buffer overflows. @@ -94,7 +94,7 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, #else // General format: 32, 243, -12 so it is number of chars times 5 // plus line break plus small safety margin. - char printBuffer[(size + 1) * 5 + 1]; + char printBuffer[(size + 1) * 5 + 1] = {}; size_t currentPos = 0; for(size_t i = 0; i < size; i++) { // To avoid buffer overflows. diff --git a/osal/FreeRTOS/Clock.cpp b/osal/FreeRTOS/Clock.cpp index 806edcc7..c15971fe 100644 --- a/osal/FreeRTOS/Clock.cpp +++ b/osal/FreeRTOS/Clock.cpp @@ -111,7 +111,7 @@ ReturnValue_t Clock::getDateAndTime(TimeOfDay_t* time) { ReturnValue_t Clock::convertTimeOfDayToTimeval(const TimeOfDay_t* from, timeval* to) { - struct tm time_tm; + struct tm time_tm = {}; time_tm.tm_year = from->year - 1900; time_tm.tm_mon = from->month - 1; diff --git a/pus/Service5EventReporting.cpp b/pus/Service5EventReporting.cpp index 29eb7f20..965a27ad 100644 --- a/pus/Service5EventReporting.cpp +++ b/pus/Service5EventReporting.cpp @@ -15,7 +15,9 @@ Service5EventReporting::Service5EventReporting(object_id_t objectId, eventQueue = QueueFactory::instance()->createMessageQueue(messageQueueDepth); } -Service5EventReporting::~Service5EventReporting(){} +Service5EventReporting::~Service5EventReporting() { + QueueFactory::instance()->deleteMessageQueue(eventQueue); +} ReturnValue_t Service5EventReporting::performService() { EventMessage message; From 40cc3c383bca725d162638fa9885ec3f4d83f8fb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 9 Apr 2021 00:45:04 +0200 Subject: [PATCH 27/35] this should work --- serialize/SerializeElement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serialize/SerializeElement.h b/serialize/SerializeElement.h index ba34393d..db66f9cc 100644 --- a/serialize/SerializeElement.h +++ b/serialize/SerializeElement.h @@ -25,7 +25,7 @@ public: } SerializeElement() : - LinkedElement(this), entry(0) { + LinkedElement(this), entry() { } ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize, From d3c3a9147a875f34a4fe95358b5e41cecfa4093f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 9 Apr 2021 08:29:56 +0200 Subject: [PATCH 28/35] bumped version number --- FSFWVersion.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/FSFWVersion.h b/FSFWVersion.h index 11a60891..df2d49a5 100644 --- a/FSFWVersion.h +++ b/FSFWVersion.h @@ -3,9 +3,9 @@ const char* const FSFW_VERSION_NAME = "ASTP"; -#define FSFW_VERSION 0 -#define FSFW_SUBVERSION 0 -#define FSFW_REVISION 1 +#define FSFW_VERSION 1 +#define FSFW_SUBVERSION 0 +#define FSFW_REVISION 0 From b8c7a6570915df561df260ec6e83b03521b9114e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 9 Apr 2021 08:59:32 +0200 Subject: [PATCH 29/35] coverity fix --- datapoollocal/LocalDataPoolManager.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 94ee9c26..dbe68ff1 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -909,27 +909,29 @@ void LocalDataPoolManager::printWarningOrError(sif::OutputTypes outputType, errorPrint = "Unknown error"; } } + object_id_t objectId = 0xffffffff; + if(owner != nullptr) { + objectId = owner->getObjectId(); + } if(outputType == sif::OutputTypes::OUT_WARNING) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "LocalDataPoolManager::" << functionName - << ": Object ID 0x" << std::setw(8) << std::setfill('0') - << std::hex << owner->getObjectId() << " | " << errorPrint - << std::dec << std::setfill(' ') << std::endl; + sif::warning << "LocalDataPoolManager::" << functionName << ": Object ID 0x" << + std::setw(8) << std::setfill('0') << std::hex << objectId << " | " << errorPrint << + std::dec << std::setfill(' ') << std::endl; #else sif::printWarning("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", - functionName, owner->getObjectId(), errorPrint); + functionName, objectId, errorPrint); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } else if(outputType == sif::OutputTypes::OUT_ERROR) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "LocalDataPoolManager::" << functionName - << ": Object ID 0x" << std::setw(8) << std::setfill('0') - << std::hex << owner->getObjectId() << " | " << errorPrint - << std::dec << std::setfill(' ') << std::endl; + sif::error << "LocalDataPoolManager::" << functionName << ": Object ID 0x" << + std::setw(8) << std::setfill('0') << std::hex << objectId << " | " << errorPrint << + std::dec << std::setfill(' ') << std::endl; #else sif::printError("LocalDataPoolManager::%s: Object ID 0x%08x | %s\n", - functionName, owner->getObjectId(), errorPrint); + functionName, objectId, errorPrint); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ } #endif /* #if FSFW_VERBOSE_LEVEL >= 1 */ From d9a0a4f2ea75f62035287c3e75f2272b995d77b5 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 9 Apr 2021 09:14:42 +0200 Subject: [PATCH 30/35] coverity --- pus/Service1TelecommandVerification.cpp | 4 +++- tmtcservices/TmTcBridge.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pus/Service1TelecommandVerification.cpp b/pus/Service1TelecommandVerification.cpp index 9e86c752..7ce75478 100644 --- a/pus/Service1TelecommandVerification.cpp +++ b/pus/Service1TelecommandVerification.cpp @@ -15,7 +15,9 @@ Service1TelecommandVerification::Service1TelecommandVerification( tmQueue = QueueFactory::instance()->createMessageQueue(messageQueueDepth); } -Service1TelecommandVerification::~Service1TelecommandVerification() {} +Service1TelecommandVerification::~Service1TelecommandVerification() { + QueueFactory::instance()->deleteMessageQueue(tmQueue); +} MessageQueueId_t Service1TelecommandVerification::getVerificationQueue(){ return tmQueue->getId(); diff --git a/tmtcservices/TmTcBridge.cpp b/tmtcservices/TmTcBridge.cpp index f99b9051..dcffac41 100644 --- a/tmtcservices/TmTcBridge.cpp +++ b/tmtcservices/TmTcBridge.cpp @@ -16,7 +16,9 @@ TmTcBridge::TmTcBridge(object_id_t objectId, object_id_t tcDestination, createMessageQueue(TMTC_RECEPTION_QUEUE_DEPTH); } -TmTcBridge::~TmTcBridge() {} +TmTcBridge::~TmTcBridge() { + QueueFactory::instance()->deleteMessageQueue(tmTcReceptionQueue); +} ReturnValue_t TmTcBridge::setNumberOfSentPacketsPerCycle( uint8_t sentPacketsPerCycle) { From e0d39b1feb04aafdb3fbe9e57a2f84ed9923300a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 9 Apr 2021 09:17:11 +0200 Subject: [PATCH 31/35] coverity --- pus/Service8FunctionManagement.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pus/Service8FunctionManagement.cpp b/pus/Service8FunctionManagement.cpp index a2202abc..54187a82 100644 --- a/pus/Service8FunctionManagement.cpp +++ b/pus/Service8FunctionManagement.cpp @@ -53,12 +53,14 @@ ReturnValue_t Service8FunctionManagement::checkInterfaceAndAcquireMessageQueue( ReturnValue_t Service8FunctionManagement::prepareCommand( CommandMessage* message, uint8_t subservice, const uint8_t* tcData, size_t tcDataLen, uint32_t* state, object_id_t objectId) { - return prepareDirectCommand(dynamic_cast(message), - tcData, tcDataLen); + return prepareDirectCommand(message, tcData, tcDataLen); } ReturnValue_t Service8FunctionManagement::prepareDirectCommand( CommandMessage *message, const uint8_t *tcData, size_t tcDataLen) { + if(message == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } if(tcDataLen < sizeof(object_id_t) + sizeof(ActionId_t)) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::debug << "Service8FunctionManagement::prepareDirectCommand:" From 6db0725aa4293cfafdf9cdea87d701a3892cd2eb Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 9 Apr 2021 15:39:48 +0200 Subject: [PATCH 32/35] coverity fix --- health/HealthTable.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/health/HealthTable.cpp b/health/HealthTable.cpp index 5d720b19..8b19d8e3 100644 --- a/health/HealthTable.cpp +++ b/health/HealthTable.cpp @@ -68,8 +68,18 @@ void HealthTable::printAll(uint8_t* pointer, size_t maxSize) { MutexGuard(mutex, timeoutType, mutexTimeoutMs); size_t size = 0; uint16_t count = healthMap.size(); - SerializeAdapter::serialize(&count, + ReturnValue_t result = SerializeAdapter::serialize(&count, &pointer, &size, maxSize, SerializeIF::Endianness::BIG); + if(result != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "HealthTable::printAll: Serialization of health table failed" << std::endl; +#else + sif::printWarning("HealthTable::printAll: Serialization of health table failed\n"); +#endif +#endif /* FSFW_VERBOSE_LEVEL >= 1 */ + return; + } for (const auto& health: healthMap) { SerializeAdapter::serialize(&health.first, &pointer, &size, maxSize, SerializeIF::Endianness::BIG); From 316310e99301790d9674d69e3c17e9ff782a34e9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 9 Apr 2021 15:49:33 +0200 Subject: [PATCH 33/35] coverity --- devicehandlers/DeviceHandlerBase.cpp | 2 +- pus/CService200ModeCommanding.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 4a0b3582..8c8919ce 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1515,7 +1515,7 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, << std::setfill(' ') << std::endl; #else sif::printError("DeviceHandlerBase::%s: Object ID 0x%08x | %s\n", - this->getObjectId(), errorPrint); + functionName, this->getObjectId(), errorPrint); #endif } diff --git a/pus/CService200ModeCommanding.cpp b/pus/CService200ModeCommanding.cpp index c4e99359..70caadd1 100644 --- a/pus/CService200ModeCommanding.cpp +++ b/pus/CService200ModeCommanding.cpp @@ -61,8 +61,7 @@ ReturnValue_t CService200ModeCommanding::prepareCommand( return result; } - ModeMessage::setModeMessage(dynamic_cast(message), - ModeMessage::CMD_MODE_COMMAND, modeCommandPacket.getMode(), + ModeMessage::setModeMessage(message, ModeMessage::CMD_MODE_COMMAND, modeCommandPacket.getMode(), modeCommandPacket.getSubmode()); return result; } From 2e417c787d8fa38f9ff0bc9a73b44f900f796c75 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 9 Apr 2021 16:14:14 +0200 Subject: [PATCH 34/35] coverity --- devicehandlers/DeviceHandlerBase.cpp | 5 ++++- pus/Service20ParameterManagement.cpp | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 8c8919ce..531a0642 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1495,6 +1495,9 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, errorPrint = "Unknown error"; } } + if(functionName == nullptr) { + functionName = "unknown function"; + } if(errorType == sif::OutputTypes::OUT_WARNING) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -1504,7 +1507,7 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, << std::setfill(' ') << std::endl; #else sif::printWarning("DeviceHandlerBase::%s: Object ID 0x%08x | %s\n", - this->getObjectId(), errorPrint); + functionName, this->getObjectId(), errorPrint); #endif } else if(errorType == sif::OutputTypes::OUT_ERROR) { diff --git a/pus/Service20ParameterManagement.cpp b/pus/Service20ParameterManagement.cpp index bc3a9119..90e96650 100644 --- a/pus/Service20ParameterManagement.cpp +++ b/pus/Service20ParameterManagement.cpp @@ -75,9 +75,8 @@ ReturnValue_t Service20ParameterManagement::checkInterfaceAndAcquireMessageQueue #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("Object ID: 0x%08x\n", *objectId); + sif::printError("Make sure it implements ReceivesParameterMessagesIF!\n"); #endif return CommandingServiceBase::INVALID_OBJECT; From 83e7dbb1f0ab91fa7068e628d66d70633afd5238 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 9 Apr 2021 16:17:21 +0200 Subject: [PATCH 35/35] Coveritx fixes --- pus/servicepackets/Service8Packets.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pus/servicepackets/Service8Packets.h b/pus/servicepackets/Service8Packets.h index b026edf5..a27cf8bb 100644 --- a/pus/servicepackets/Service8Packets.h +++ b/pus/servicepackets/Service8Packets.h @@ -43,8 +43,8 @@ public: private: DirectCommand(const DirectCommand &command); - object_id_t objectId; - ActionId_t actionId; + object_id_t objectId = 0; + ActionId_t actionId = 0; uint32_t parametersSize; //!< [EXPORT] : [IGNORE] const uint8_t * parameterBuffer; //!< [EXPORT] : [MAXSIZE] 65535 Bytes