From 5676969fe368187507d7a3bf90b153719931fdf3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 11 Apr 2021 21:13:43 +0200 Subject: [PATCH 01/10] 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 1c06521ca..74ccb29a2 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 c0c82cf11..3a0f654ed 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 8004c045b..12774a58c 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 01d0bd6c64c68d3a351573203945a35044e341f8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 11 Apr 2021 21:20:45 +0200 Subject: [PATCH 02/10] Small patch for CMakeLists file This adds the additional includes to the interface as well so libraries linking again fsfw get the additional includes as well. --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff060d7be..8ba6a187e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -184,6 +184,7 @@ endif() target_include_directories(${LIB_FSFW_NAME} INTERFACE ${CMAKE_SOURCE_DIR} ${FSFW_CONFIG_PATH_ABSOLUTE} + ${FSFW_ADD_INC_PATHS_ABS} ) # Includes path required to compile FSFW itself as well From e50d0738ab08f75f7284a5788b8d3174d1e2dac7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 11 Apr 2021 21:22:58 +0200 Subject: [PATCH 03/10] bumped version to 1.0.0 --- FSFWVersion.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/FSFWVersion.h b/FSFWVersion.h index 11a608919..df2d49a51 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 b786b53c35dc64fe0743aec68cb8ef9bcfe2fcfb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 11 Apr 2021 21:54:48 +0200 Subject: [PATCH 04/10] added all coverity fixes --- action/ActionHelper.cpp | 5 ----- container/SharedRingBuffer.cpp | 3 +++ container/SharedRingBuffer.h | 23 +++++++++++++---------- datapool/HkSwitchHelper.cpp | 2 +- datapoollocal/LocalDataPoolManager.cpp | 22 ++++++++++++---------- devicehandlers/DeviceHandlerBase.cpp | 9 ++++++--- events/EventMessage.cpp | 2 +- events/EventMessage.h | 2 +- globalfunctions/arrayprinter.cpp | 4 ++-- health/HealthTable.cpp | 24 ++++++++++++++++++++---- osal/FreeRTOS/Clock.cpp | 2 +- osal/common/TcpTmTcServer.cpp | 7 +++++-- osal/host/FixedTimeslotTask.cpp | 10 ++++++---- osal/host/PeriodicTask.cpp | 1 - osal/host/QueueMapManager.cpp | 4 ++++ osal/host/QueueMapManager.h | 2 ++ pus/CService200ModeCommanding.cpp | 3 +-- pus/Service1TelecommandVerification.cpp | 4 +++- pus/Service20ParameterManagement.cpp | 5 ++--- pus/Service5EventReporting.cpp | 4 +++- pus/Service8FunctionManagement.cpp | 6 ++++-- pus/servicepackets/Service8Packets.h | 4 ++-- tmtcservices/TmTcBridge.cpp | 4 +++- 23 files changed, 95 insertions(+), 57 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 4b64a40ca..b2374ed69 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); diff --git a/container/SharedRingBuffer.cpp b/container/SharedRingBuffer.cpp index 5e20bb6fe..fe36341dd 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 66a119ab0..9d6ea56c2 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/datapool/HkSwitchHelper.cpp b/datapool/HkSwitchHelper.cpp index 1a2a25eb9..21e37f59d 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/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 94ee9c264..dbe68ff14 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 */ diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 15eac11f4..531a0642b 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"; } @@ -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) { @@ -1515,7 +1518,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/events/EventMessage.cpp b/events/EventMessage.cpp index bbc41e100..548b4f0f3 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 4d003bd7b..f2f5ffb50 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; }; diff --git a/globalfunctions/arrayprinter.cpp b/globalfunctions/arrayprinter.cpp index 20a64f5b6..0423360b9 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/health/HealthTable.cpp b/health/HealthTable.cpp index 5d720b191..3fed1deb1 100644 --- a/health/HealthTable.cpp +++ b/health/HealthTable.cpp @@ -68,14 +68,30 @@ 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, + result = SerializeAdapter::serialize(&health.first, &pointer, &size, maxSize, SerializeIF::Endianness::BIG); + if(result != HasReturnvaluesIF::RETURN_OK) { + return; + } uint8_t healthValue = health.second; - SerializeAdapter::serialize(&healthValue, &pointer, &size, + result = SerializeAdapter::serialize(&healthValue, &pointer, &size, maxSize, SerializeIF::Endianness::BIG); + if(result != HasReturnvaluesIF::RETURN_OK) { + return; + } } } @@ -86,7 +102,7 @@ ReturnValue_t HealthTable::iterate(HealthEntry *value, bool reset) { mapIterator = healthMap.begin(); } if (mapIterator == healthMap.end()) { - result = HasReturnvaluesIF::RETURN_FAILED; + return HasReturnvaluesIF::RETURN_FAILED; } *value = *mapIterator; mapIterator++; diff --git a/osal/FreeRTOS/Clock.cpp b/osal/FreeRTOS/Clock.cpp index 806edcc7c..c15971fee 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/osal/common/TcpTmTcServer.cpp b/osal/common/TcpTmTcServer.cpp index 296afad8e..08a62ffb9 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); @@ -84,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; @@ -101,6 +102,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { if(clientSocket == INVALID_SOCKET) { handleError(Protocol::TCP, ErrorSources::ACCEPT_CALL, 500); + closeSocket(clientSocket); continue; }; @@ -122,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/host/FixedTimeslotTask.cpp b/osal/host/FixedTimeslotTask.cpp index 3b169b5a2..89daa2786 100644 --- a/osal/host/FixedTimeslotTask.cpp +++ b/osal/host/FixedTimeslotTask.cpp @@ -3,7 +3,7 @@ #include "../../ipc/MutexFactory.h" #include "../../osal/host/Mutex.h" #include "../../osal/host/FixedTimeslotTask.h" -#include "../../serviceinterface/ServiceInterfaceStream.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../tasks/ExecutableObjectIF.h" #include @@ -38,7 +38,6 @@ FixedTimeslotTask::~FixedTimeslotTask(void) { if(mainThread.joinable()) { mainThread.join(); } - delete this; } void FixedTimeslotTask::taskEntryPoint(void* argument) { @@ -119,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; } diff --git a/osal/host/PeriodicTask.cpp b/osal/host/PeriodicTask.cpp index d7abf9d0c..631264b74 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 b50d62dcf..c9100fe9a 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 3610ca638..90c39c2f0 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; diff --git a/pus/CService200ModeCommanding.cpp b/pus/CService200ModeCommanding.cpp index c4e99359b..70caadd10 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; } diff --git a/pus/Service1TelecommandVerification.cpp b/pus/Service1TelecommandVerification.cpp index 9e86c752a..7ce75478b 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/pus/Service20ParameterManagement.cpp b/pus/Service20ParameterManagement.cpp index bc3a9119b..90e966500 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; diff --git a/pus/Service5EventReporting.cpp b/pus/Service5EventReporting.cpp index 29eb7f20e..965a27ad6 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; diff --git a/pus/Service8FunctionManagement.cpp b/pus/Service8FunctionManagement.cpp index a2202abc1..54187a829 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:" diff --git a/pus/servicepackets/Service8Packets.h b/pus/servicepackets/Service8Packets.h index b026edf52..a27cf8bb6 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 diff --git a/tmtcservices/TmTcBridge.cpp b/tmtcservices/TmTcBridge.cpp index f99b90516..dcffac41e 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 06d34efa749dab1e3b76b47a0fa28d7f52cfd0fb Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:33:45 +0200 Subject: [PATCH 05/10] UDP and TCPIP smaller tweaks Using AI_PASSIVE now so the UDP server listens to all addresses (0.0.0.0) instead of just localhost. Also implemented address print function properly --- osal/common/TcpTmTcServer.cpp | 7 ++++-- osal/common/UdpTcPollingTask.cpp | 9 +++++--- osal/common/UdpTmTcBridge.cpp | 22 ++++++++++-------- osal/common/tcpipCommon.cpp | 39 ++++++++++++++++++++++++++++++++ osal/common/tcpipCommon.h | 11 +++++++-- 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/osal/common/TcpTmTcServer.cpp b/osal/common/TcpTmTcServer.cpp index 296afad8e..08a62ffb9 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); @@ -84,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; @@ -101,6 +102,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { if(clientSocket == INVALID_SOCKET) { handleError(Protocol::TCP, ErrorSources::ACCEPT_CALL, 500); + closeSocket(clientSocket); continue; }; @@ -122,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/UdpTcPollingTask.cpp b/osal/common/UdpTcPollingTask.cpp index 759cee057..47f67b295 100644 --- a/osal/common/UdpTcPollingTask.cpp +++ b/osal/common/UdpTcPollingTask.cpp @@ -15,7 +15,7 @@ #endif //! Debugging preprocessor define. -#define FSFW_UDP_RCV_WIRETAPPING_ENABLED 0 +#define FSFW_UDP_RECV_WIRETAPPING_ENABLED 0 UdpTcPollingTask::UdpTcPollingTask(object_id_t objectId, object_id_t tmtcUnixUdpBridge, size_t maxRecvSize, @@ -66,10 +66,13 @@ ReturnValue_t UdpTcPollingTask::performOperation(uint8_t opCode) { tcpip::handleError(tcpip::Protocol::UDP, tcpip::ErrorSources::RECVFROM_CALL, 1000); continue; } -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 +#if FSFW_UDP_RECV_WIRETAPPING_ENABLED == 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 sif::debug << "UdpTcPollingTask::performOperation: " << bytesReceived << " bytes received" << std::endl; +#else #endif +#endif /* FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 */ ReturnValue_t result = handleSuccessfullTcRead(bytesReceived); if(result != HasReturnvaluesIF::RETURN_FAILED) { @@ -84,7 +87,7 @@ ReturnValue_t UdpTcPollingTask::performOperation(uint8_t opCode) { ReturnValue_t UdpTcPollingTask::handleSuccessfullTcRead(size_t bytesRead) { store_address_t storeId; -#if FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 +#if FSFW_UDP_RECV_WIRETAPPING_ENABLED == 1 arrayprinter::print(receptionBuffer.data(), bytesRead); #endif diff --git a/osal/common/UdpTmTcBridge.cpp b/osal/common/UdpTmTcBridge.cpp index 7f3dc9293..ba23f521b 100644 --- a/osal/common/UdpTmTcBridge.cpp +++ b/osal/common/UdpTmTcBridge.cpp @@ -70,6 +70,7 @@ ReturnValue_t UdpTmTcBridge::initialize() { hints.ai_family = AF_INET; hints.ai_socktype = SOCK_DGRAM; hints.ai_protocol = IPPROTO_UDP; + hints.ai_flags = AI_PASSIVE; /* Set up UDP socket: https://en.wikipedia.org/wiki/Getaddrinfo @@ -95,6 +96,10 @@ ReturnValue_t UdpTmTcBridge::initialize() { return HasReturnvaluesIF::RETURN_FAILED; } +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(addrResult->ai_addr); +#endif + retval = bind(serverSocket, addrResult->ai_addr, static_cast(addrResult->ai_addrlen)); if(retval != 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -103,6 +108,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; @@ -120,10 +126,8 @@ ReturnValue_t UdpTmTcBridge::sendTm(const uint8_t *data, size_t dataLen) { /* The target address can be set by different threads so this lock ensures thread-safety */ MutexGuard lock(mutex, timeoutType, mutexTimeoutMs); -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 - char ipAddress [15]; - sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, - &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(&clientAddress); #endif int bytesSent = sendto( @@ -151,13 +155,11 @@ void UdpTmTcBridge::checkAndSetClientAddress(sockaddr& newAddress) { /* The target address can be set by different threads so this lock ensures thread-safety */ MutexGuard lock(mutex, timeoutType, mutexTimeoutMs); -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 - char ipAddress [15]; - sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, - &newAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; - sif::debug << "IP Address Old: " << inet_ntop(AF_INET, - &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(&newAddress); + tcpip::printAddress(&clientAddress); #endif + registerCommConnect(); /* Set new IP address to reply to */ diff --git a/osal/common/tcpipCommon.cpp b/osal/common/tcpipCommon.cpp index 9a5e4647c..551e2a426 100644 --- a/osal/common/tcpipCommon.cpp +++ b/osal/common/tcpipCommon.cpp @@ -1,4 +1,9 @@ #include "tcpipCommon.h" +#include + +#ifdef _WIN32 +#include +#endif void tcpip::determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std::string &protStr, std::string &srcString) { @@ -34,3 +39,37 @@ void tcpip::determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std: srcString = "unknown call"; } } + +void tcpip::printAddress(struct sockaddr* addr) { + char ipAddress[INET6_ADDRSTRLEN] = {}; + const char* stringPtr = NULL; + switch(addr->sa_family) { + case AF_INET: { + struct sockaddr_in *addrIn = reinterpret_cast(addr); + stringPtr = inet_ntop(AF_INET, &(addrIn->sin_addr), ipAddress, INET_ADDRSTRLEN); + break; + } + case AF_INET6: { + struct sockaddr_in6 *addrIn = reinterpret_cast(addr); + stringPtr = inet_ntop(AF_INET6, &(addrIn->sin6_addr), ipAddress, INET6_ADDRSTRLEN); + break; + } + } +#if FSFW_CPP_OSTREAM_ENABLED == 1 + if(stringPtr == NULL) { + sif::debug << "Could not convert IP address to text representation, error code " + << errno << std::endl; + } + else { + sif::debug << "IP Address Sender: " << ipAddress << std::endl; + } +#else + if(stringPtr == NULL) { + sif::printDebug("Could not convert IP address to text representation, error code %d\n", + errno); + } + else { + sif::printDebug("IP Address Sender: %s\n", ipAddress); + } +#endif +} diff --git a/osal/common/tcpipCommon.h b/osal/common/tcpipCommon.h index dc5ada526..22b914dc1 100644 --- a/osal/common/tcpipCommon.h +++ b/osal/common/tcpipCommon.h @@ -4,6 +4,13 @@ #include "../../timemanager/clockDefinitions.h" #include +#ifdef _WIN32 +#include +#else +#include +#include +#endif + namespace tcpip { const char* const DEFAULT_SERVER_PORT = "7301"; @@ -28,8 +35,8 @@ enum class ErrorSources { void determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std::string& protStr, std::string& srcString); +void printAddress(struct sockaddr* addr); + } - - #endif /* FSFW_OSAL_COMMON_TCPIPCOMMON_H_ */ From 547538fbc5aa9b23d1128f01c100ad5a73b9a520 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:38:56 +0200 Subject: [PATCH 06/10] proper MQ implementation --- osal/host/MessageQueue.cpp | 29 ++++++++++------------------- osal/host/MessageQueue.h | 2 +- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 18272a68a..41c55a3df 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; } @@ -106,6 +105,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,21 +130,10 @@ 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) { - targetQueue->messageQueue.push(*mqmMessage); - } - else { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message" - "is not MessageQueueMessage!" << std::endl; -#endif - } - + MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20); + 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 97a9e4915..e965123dc 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 5124f314f497bcfa8758c7124f63c68fb24eb88c Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:50:23 +0200 Subject: [PATCH 07/10] updated distributor modules --- tcdistribution/CCSDSDistributor.cpp | 31 +++++-- tcdistribution/PUSDistributor.cpp | 121 ++++++++++++++++------------ 2 files changed, 92 insertions(+), 60 deletions(-) diff --git a/tcdistribution/CCSDSDistributor.cpp b/tcdistribution/CCSDSDistributor.cpp index b795854f2..62cbfbf2b 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 0 + 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; } diff --git a/tcdistribution/PUSDistributor.cpp b/tcdistribution/PUSDistributor.cpp index 00fd9029a..abdd1f8d3 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 0 + 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 98add88d143105f5c04d17629083d9bab2ee2916 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 13:01:24 +0200 Subject: [PATCH 08/10] Makes linux realtime optional Adds new config variable FSFW_USE_REALTIME_FOR_LINUX --- defaultcfg/fsfwconfig/FSFWConfig.h | 4 ++++ osal/host/PeriodicTask.cpp | 2 +- osal/linux/PosixThread.cpp | 9 ++++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index ed86e6e1f..385808fe4 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -57,6 +57,10 @@ static constexpr size_t FSFW_EVENTMGMR_RANGEMATCHERS = 120; static constexpr uint8_t FSFW_CSB_FIFO_DEPTH = 6; static constexpr size_t FSFW_PRINT_BUFFER_SIZE = 124; + +//! Defines if the real time scheduler for linux should be used. +//! If set to 1 the binary needs "cap_sys_nice=eip" privileges to run +#define FSFW_USE_REALTIME_FOR_LINUX 0 } #endif /* CONFIG_FSFWCONFIG_H_ */ diff --git a/osal/host/PeriodicTask.cpp b/osal/host/PeriodicTask.cpp index 7663d522d..0a99b03a3 100644 --- a/osal/host/PeriodicTask.cpp +++ b/osal/host/PeriodicTask.cpp @@ -19,7 +19,7 @@ PeriodicTask::PeriodicTask(const char *name, TaskPriority setPriority, void (*setDeadlineMissedFunc)()) : started(false), taskName(name), period(setPeriod), deadlineMissedFunc(setDeadlineMissedFunc) { - // It is propably possible to set task priorities by using the native + // It is probably possible to set task priorities by using the native // task handles for Windows / Linux mainThread = std::thread(&PeriodicTask::taskEntryPoint, this, this); #if defined(WIN32) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index bd8e72583..ead32e873 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -184,8 +184,11 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { strerror(status) << std::endl; #endif } - - // TODO FIFO -> This needs root privileges for the process +#ifndef FSFW_USE_REALTIME_FOR_LINUX +#error "Please define FSFW_USE_REALTIME_FOR_LINUX with either 0 or 1" +#endif +#if FSFW_USE_REALTIME_FOR_LINUX == 1 + // FIFO -> This needs root privileges for the process status = pthread_attr_setschedpolicy(&attributes,SCHED_FIFO); if(status != 0){ #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -203,7 +206,7 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { strerror(status) << std::endl; #endif } - +#endif //Set Signal Mask for suspend until startTask is called sigset_t waitSignal; sigemptyset(&waitSignal); From 03095776f16a216edc5d12c0dbc81b7205abb874 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 13:31:15 +0200 Subject: [PATCH 09/10] Added a comment and corrected a mistake --- defaultcfg/fsfwconfig/FSFWConfig.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index 385808fe4..8f82251bd 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -52,15 +52,18 @@ static constexpr size_t FSFW_EVENTMGMR_RANGEMATCHERS = 120; //! Defines the FIFO depth of each commanding service base which //! also determines how many commands a CSB service can handle in one cycle -//! simulataneously. This will increase the required RAM for +//! Simultaneously. This will increase the required RAM for //! each CSB service ! static constexpr uint8_t FSFW_CSB_FIFO_DEPTH = 6; static constexpr size_t FSFW_PRINT_BUFFER_SIZE = 124; //! Defines if the real time scheduler for linux should be used. +//! If set to 0, this will also disable priority settings for linux +//! as most systems will not allow to set nice values without privileges +//! For embedded linux system set this to 1. //! If set to 1 the binary needs "cap_sys_nice=eip" privileges to run -#define FSFW_USE_REALTIME_FOR_LINUX 0 +#define FSFW_USE_REALTIME_FOR_LINUX 1 } #endif /* CONFIG_FSFWCONFIG_H_ */ From 153c44601b484fef84795456fa32c8749e574d55 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 15:04:42 +0200 Subject: [PATCH 10/10] refixed spelling --- defaultcfg/fsfwconfig/FSFWConfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index 8f82251bd..e1e0ec644 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -52,7 +52,7 @@ static constexpr size_t FSFW_EVENTMGMR_RANGEMATCHERS = 120; //! Defines the FIFO depth of each commanding service base which //! also determines how many commands a CSB service can handle in one cycle -//! Simultaneously. This will increase the required RAM for +//! simultaneously. This will increase the required RAM for //! each CSB service ! static constexpr uint8_t FSFW_CSB_FIFO_DEPTH = 6;