From a66fc5339606495fd5222a2bbcf6160647289bf8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 15:31:03 +0200 Subject: [PATCH 01/10] config define moved, better warning --- defaultcfg/fsfwconfig/FSFWConfig.h | 13 +++++++------ osal/linux/PosixThread.cpp | 10 +++++++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index e1e0ec644..fe18a2f43 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -40,6 +40,13 @@ //! Specify whether a special mode store is used for Subsystem components. #define FSFW_USE_MODESTORE 0 +//! 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 1 + namespace fsfwconfig { //! Default timestamp size. The default timestamp will be an eight byte CDC //! short timestamp. @@ -58,12 +65,6 @@ 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 1 } #endif /* CONFIG_FSFWCONFIG_H_ */ diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index f1cff9925..72adfb140 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -223,8 +223,16 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { status = pthread_create(&thread,&attributes,fnc_,arg_); if(status != 0){ #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "Posix Thread create failed with: " << + sif::error << "PosixThread::createTask: Failed with: " << strerror(status) << std::endl; + sif::error << "For FSFW_USE_REALTIME_FOR_LINUX == 1 make sure to call " << + "\"all sudo setcap 'cap_sys_nice=eip'\" on the application or set " + "/etc/security/limit.conf" << std::endl; +#else + sif::printError("PosixThread::createTask: Create failed with: %s\n", strerror(status)); + sif::printError("For FSFW_USE_REALTIME_FOR_LINUX == 1 make sure to call " + "\"all sudo setcap 'cap_sys_nice=eip'\" on the application or set " + "/etc/security/limit.conf\n"); #endif } From cc8c3aef3d3047c4a6692f24513459043a3c9cfd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:18:39 +0200 Subject: [PATCH 02/10] removed unused interface --- tmtcpacket/pus/TmPacketIF.h | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 tmtcpacket/pus/TmPacketIF.h diff --git a/tmtcpacket/pus/TmPacketIF.h b/tmtcpacket/pus/TmPacketIF.h deleted file mode 100644 index bd66523b8..000000000 --- a/tmtcpacket/pus/TmPacketIF.h +++ /dev/null @@ -1,8 +0,0 @@ -#ifndef FSFW_TMTCPACKET_PUS_TMPACKETIF_H_ -#define FSFW_TMTCPACKET_PUS_TMPACKETIF_H_ - - - - - -#endif /* FSFW_TMTCPACKET_PUS_TMPACKETIF_H_ */ From 3f91803422a97a448a23da550a05cae1564f900b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 18:35:11 +0200 Subject: [PATCH 03/10] fixes from code review --- defaultcfg/fsfwconfig/FSFWConfig.h | 4 ++-- tmtcpacket/pus/TmPacketStoredPusC.h | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index 2e5a7aae7..1abdab4cd 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -50,8 +50,8 @@ #define FSFW_USE_REALTIME_FOR_LINUX 1 namespace fsfwconfig { -//! Default timestamp size. The default timestamp will be an eight byte CDC -//! short timestamp. + +//! Default timestamp size. The default timestamp will be an seven byte CDC short timestamp. static constexpr uint8_t FSFW_MISSION_TIMESTAMP_SIZE = 7; //! Configure the allocated pool sizes for the event manager. diff --git a/tmtcpacket/pus/TmPacketStoredPusC.h b/tmtcpacket/pus/TmPacketStoredPusC.h index 83478a942..883dc0ffd 100644 --- a/tmtcpacket/pus/TmPacketStoredPusC.h +++ b/tmtcpacket/pus/TmPacketStoredPusC.h @@ -5,7 +5,7 @@ #include /** - * This class generates a ECSS PUS A Telemetry packet within a given + * This class generates a ECSS PUS C Telemetry packet within a given * intermediate storage. * As most packets are passed between tasks with the help of a storage * anyway, it seems logical to create a Packet-In-Storage access class @@ -42,6 +42,9 @@ public: * @param headerData The header Data of the Application field, * will be copied in front of data * @param headerSize The size of the headerDataF + * @param destinationId Destination ID containing the application process ID as specified + * by PUS C + * @param timeRefField 4 bit time reference field as specified by PUS C */ TmPacketStoredPusC( uint16_t apid, uint8_t service, uint8_t subservice, uint16_t packetCounter = 0, const uint8_t* data = nullptr, From 87fee9bd0e8e59d762ae7885fb6918505072201c Mon Sep 17 00:00:00 2001 From: "Jakob.Meier" Date: Sun, 25 Apr 2021 11:40:04 +0200 Subject: [PATCH 04/10] dhb multiple replies support --- devicehandlers/DeviceHandlerBase.cpp | 36 ++++++++++++++++++---------- devicehandlers/DeviceHandlerBase.h | 18 ++++++++++++-- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 531a0642b..4e4570167 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -308,6 +308,14 @@ void DeviceHandlerBase::doStateMachine() { uint32_t currentUptime; Clock::getUptime(¤tUptime); if (currentUptime - timeoutStart >= childTransitionDelay) { +#if FSFW_VERBOSE_LEVEL >= 1 + char printout[60]; + sprintf(printout, "Transition timeout (%lu) occured !", + static_cast(childTransitionDelay)); + /* Very common configuration error, so print it */ + printWarningOrError(sif::OutputTypes::OUT_WARNING, "doStateMachine", + RETURN_FAILED, printout); +#endif triggerEvent(MODE_TRANSITION_FAILED, childTransitionFailure, 0); setMode(transitionSourceMode, transitionSourceSubMode); break; @@ -448,6 +456,15 @@ ReturnValue_t DeviceHandlerBase::insertInCommandMap(DeviceCommandId_t deviceComm } } +size_t DeviceHandlerBase::getNextReplyLength(DeviceCommandId_t commandId){ + DeviceReplyIter iter = deviceReplyMap.find(commandId); + if(iter != deviceReplyMap.end()) { + return iter->second.replyLen; + }else{ + return 0; + } +} + ReturnValue_t DeviceHandlerBase::updateReplyMapEntry(DeviceCommandId_t deviceReply, uint16_t delayCycles, uint16_t maxDelayCycles, bool periodic) { auto replyIter = deviceReplyMap.find(deviceReply); @@ -638,16 +655,12 @@ void DeviceHandlerBase::doGetWrite() { void DeviceHandlerBase::doSendRead() { ReturnValue_t result; - size_t requestLen = 0; + size_t replyLen = 0; if(cookieInfo.pendingCommand != deviceCommandMap.end()) { - DeviceReplyIter iter = deviceReplyMap.find( - cookieInfo.pendingCommand->first); - if(iter != deviceReplyMap.end()) { - requestLen = iter->second.replyLen; - } + replyLen = getNextReplyLength(cookieInfo.pendingCommand->first); } - result = communicationInterface->requestReceiveMessage(comCookie, requestLen); + result = communicationInterface->requestReceiveMessage(comCookie, replyLen); if (result == RETURN_OK) { cookieInfo.state = COOKIE_READ_SENT; @@ -1501,10 +1514,9 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, if(errorType == sif::OutputTypes::OUT_WARNING) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "DeviceHandlerBase::" << functionName << ": Object ID " - << std::hex << std::setw(8) << std::setfill('0') - << this->getObjectId() << " | " << errorPrint << std::dec - << std::setfill(' ') << std::endl; + sif::warning << "DeviceHandlerBase::" << functionName << ": Object ID 0x" << std::hex << + std::setw(8) << std::setfill('0') << this->getObjectId() << " | " << errorPrint << + std::dec << std::setfill(' ') << std::endl; #else sif::printWarning("DeviceHandlerBase::%s: Object ID 0x%08x | %s\n", functionName, this->getObjectId(), errorPrint); @@ -1512,7 +1524,7 @@ void DeviceHandlerBase::printWarningOrError(sif::OutputTypes errorType, } else if(errorType == sif::OutputTypes::OUT_ERROR) { #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "DeviceHandlerBase::" << functionName << ": Object ID " + sif::error << "DeviceHandlerBase::" << functionName << ": Object ID 0x" << std::hex << std::setw(8) << std::setfill('0') << this->getObjectId() << " | " << errorPrint << std::dec << std::setfill(' ') << std::endl; diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index d61b0407d..496c08ffd 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -471,13 +471,27 @@ protected: ReturnValue_t insertInReplyMap(DeviceCommandId_t deviceCommand, uint16_t maxDelayCycles, LocalPoolDataSetBase* dataSet = nullptr, size_t replyLen = 0, bool periodic = false); + /** - * @brief A simple command to add a command to the commandList. + * @brief A simple command to add a command to the commandList. * @param deviceCommand The command to add * @return - @c RETURN_OK when the command was successfully inserted, * - @c RETURN_FAILED else. */ ReturnValue_t insertInCommandMap(DeviceCommandId_t deviceCommand); + + /** + * @brief This function returns the reply length of the next reply to read. + * + * @param deviceCommand The command which triggered the device reply. + * + * @details The default implementation assumes only one reply is triggered by the command. In + * case the command triggers multiple replies (e.g. one acknowledgment, one data, + * and one execution status reply), this function can be overwritten to get the + * reply length of the next reply to read. + */ + virtual size_t getNextReplyLength(DeviceCommandId_t deviceCommand); + /** * @brief This is a helper method to facilitate updating entries * in the reply map. @@ -953,7 +967,7 @@ protected: * - NO_REPLY_EXPECTED if there was no reply found. This is not an * error case as many commands do not expect a reply. */ - virtual ReturnValue_t enableReplyInReplyMap(DeviceCommandMap::iterator cmd, + virtual ReturnValue_t enableReplyInReplyMap(DeviceCommandMap::iterator command, uint8_t expectedReplies = 1, bool useAlternateId = false, DeviceCommandId_t alternateReplyID = 0); From 818634755d21e908c3e54e53ceb7907ac3999401 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Apr 2021 15:47:49 +0200 Subject: [PATCH 05/10] checking returnvalue now --- parameters/ParameterHelper.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 24d0b2b1f..e80c2c47f 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -65,6 +65,9 @@ ReturnValue_t ParameterHelper::handleParameterMessage(CommandMessage *message) { ParameterWrapper ownerWrapper; result = owner->getParameter(domain, uniqueIdentifier, &ownerWrapper, &streamWrapper, linearIndex); + if(result != HasReturnvaluesIF::RETURN_OK) { + return result; + } result = ownerWrapper.copyFrom(&streamWrapper, linearIndex); if (result != HasReturnvaluesIF::RETURN_OK) { From 054de9781b28b511167cfc2a9d21c79831eef2b9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 27 Apr 2021 14:17:27 +0200 Subject: [PATCH 06/10] avoid duplicate printout --- devicehandlers/DeviceHandlerBase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 4e4570167..1623a1ac4 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -308,11 +308,11 @@ void DeviceHandlerBase::doStateMachine() { uint32_t currentUptime; Clock::getUptime(¤tUptime); if (currentUptime - timeoutStart >= childTransitionDelay) { -#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_VERBOSE_LEVEL >= 1 && FSFW_OBJ_EVENT_TRANSLATION == 0 char printout[60]; sprintf(printout, "Transition timeout (%lu) occured !", static_cast(childTransitionDelay)); - /* Very common configuration error, so print it */ + /* Common configuration error for development, so print it */ printWarningOrError(sif::OutputTypes::OUT_WARNING, "doStateMachine", RETURN_FAILED, printout); #endif From 097244bf8b429d1b107343a3dbd77fa537a9c86d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 29 Apr 2021 19:51:38 +0200 Subject: [PATCH 07/10] use timestamp size from FSFWConfig.h --- timemanager/TimeStamperIF.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/timemanager/TimeStamperIF.h b/timemanager/TimeStamperIF.h index 57b7f0149..534cc734f 100644 --- a/timemanager/TimeStamperIF.h +++ b/timemanager/TimeStamperIF.h @@ -2,6 +2,7 @@ #define FSFW_TIMEMANAGER_TIMESTAMPERIF_H_ #include "../returnvalues/HasReturnvaluesIF.h" +#include /** * A class implementing this IF provides facilities to add a time stamp to the @@ -16,8 +17,7 @@ public: //! This is a mission-specific constant and determines the total //! size reserved for timestamps. - //! TODO: Default define in FSFWConfig ? - static const uint8_t MISSION_TIMESTAMP_SIZE = 8; + static const uint8_t MISSION_TIMESTAMP_SIZE = fsfwconfig::FSFW_MISSION_TIMESTAMP_SIZE; virtual ReturnValue_t addTimeStamp(uint8_t* buffer, const uint8_t maxSize) = 0; virtual ~TimeStamperIF() {} From 8b17c40aa61537a70ce947b1db8bf338dea2ae16 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 11 May 2021 15:02:04 +0200 Subject: [PATCH 08/10] Added Network Byte Order --- serialize/SerializeIF.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/serialize/SerializeIF.h b/serialize/SerializeIF.h index d72218f0a..dfd854e3a 100644 --- a/serialize/SerializeIF.h +++ b/serialize/SerializeIF.h @@ -19,7 +19,10 @@ class SerializeIF { public: enum class Endianness : uint8_t { - BIG, LITTLE, MACHINE + BIG, + LITTLE, + MACHINE, + NETWORK = BIG // Added for convenience like htons on sockets }; static const uint8_t INTERFACE_ID = CLASS_ID::SERIALIZE_IF; From d807998f4de768be499aa3a48f9bf2d577057667 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 11 May 2021 15:25:38 +0200 Subject: [PATCH 09/10] Added Missing includes in Host Osal Updated correct defaults for Host MessageQueues --- osal/host/MessageQueue.cpp | 2 ++ osal/host/MessageQueue.h | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 41c55a3df..4e2862716 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -5,6 +5,8 @@ #include "../../ipc/MutexFactory.h" #include "../../ipc/MutexGuard.h" +#include + MessageQueue::MessageQueue(size_t messageDepth, size_t maxMessageSize): messageSize(maxMessageSize), messageDepth(messageDepth) { queueLock = MutexFactory::instance()->createMutex(); diff --git a/osal/host/MessageQueue.h b/osal/host/MessageQueue.h index e965123dc..1c9b5e331 100644 --- a/osal/host/MessageQueue.h +++ b/osal/host/MessageQueue.h @@ -217,15 +217,15 @@ private: * @brief The class stores the queue id it got assigned. * If initialization fails, the queue id is set to zero. */ - MessageQueueId_t mqId = 0; + MessageQueueId_t mqId = MessageQueueIF::NO_QUEUE; size_t messageSize = 0; size_t messageDepth = 0; MutexIF* queueLock; bool defaultDestinationSet = false; - MessageQueueId_t defaultDestination = 0; - MessageQueueId_t lastPartner = 0; + MessageQueueId_t defaultDestination = MessageQueueIF::NO_QUEUE; + MessageQueueId_t lastPartner = MessageQueueIF::NO_QUEUE; }; #endif /* FRAMEWORK_OSAL_HOST_MESSAGEQUEUE_H_ */ From 9d0155d9ae58425bf982cd4712be3b7d83efc87b Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 11 May 2021 15:30:49 +0200 Subject: [PATCH 10/10] Added a returnvalue --- osal/host/MessageQueue.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 4e2862716..a779bdcb8 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -128,8 +128,7 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, internalErrorReporter->queueMessageNotSent(); } } - // TODO: Better returnvalue - return HasReturnvaluesIF::RETURN_FAILED; + return MessageQueueIF::DESTINATION_INVALID; } if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20);