From 1965a0e33b76e2bf317db48dee3b3592552403ff Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 15:47:33 +0200 Subject: [PATCH 01/31] linux important bugfix and general improvements --- ipc/QueueFactory.h | 4 +- osal/FreeRTOS/QueueFactory.cpp | 4 +- osal/linux/FixedTimeslotTask.cpp | 7 +- osal/linux/FixedTimeslotTask.h | 37 ++++++--- osal/linux/MessageQueue.cpp | 130 +++++++++++++++++++++---------- osal/linux/MessageQueue.h | 36 +++++---- osal/linux/PeriodicPosixTask.cpp | 16 ++-- osal/linux/PeriodicPosixTask.h | 13 ++++ osal/linux/PosixThread.cpp | 43 +++++++--- osal/linux/PosixThread.h | 32 ++++---- osal/linux/QueueFactory.cpp | 16 ++-- osal/linux/TaskFactory.cpp | 16 +++- osal/rtems/QueueFactory.cpp | 6 +- 13 files changed, 239 insertions(+), 121 deletions(-) diff --git a/ipc/QueueFactory.h b/ipc/QueueFactory.h index c385b15d..6e7c4a26 100644 --- a/ipc/QueueFactory.h +++ b/ipc/QueueFactory.h @@ -18,8 +18,8 @@ public: */ static QueueFactory* instance(); - MessageQueueIF* createMessageQueue(uint32_t message_depth = 3, - uint32_t max_message_size = MessageQueueMessage::MAX_MESSAGE_SIZE); + MessageQueueIF* createMessageQueue(uint32_t messageDepth = 3, + size_t maxMessageSize = MessageQueueMessage::MAX_MESSAGE_SIZE); void deleteMessageQueue(MessageQueueIF* queue); private: diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index eaf245d3..a0128918 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -25,8 +25,8 @@ QueueFactory::~QueueFactory() { } MessageQueueIF* QueueFactory::createMessageQueue(uint32_t message_depth, - uint32_t max_message_size) { - return new MessageQueue(message_depth, max_message_size); + size_t maxMessageSize) { + return new MessageQueue(message_depth, maxMessageSize); } void QueueFactory::deleteMessageQueue(MessageQueueIF* queue) { diff --git a/osal/linux/FixedTimeslotTask.cpp b/osal/linux/FixedTimeslotTask.cpp index 098753a7..e5c68f9d 100644 --- a/osal/linux/FixedTimeslotTask.cpp +++ b/osal/linux/FixedTimeslotTask.cpp @@ -1,10 +1,7 @@ #include -#include -#include -#include -#include #include +#include uint32_t FixedTimeslotTask::deadlineMissedCount = 0; const size_t PeriodicTaskIF::MINIMUM_STACK_SIZE = PTHREAD_STACK_MIN; @@ -23,7 +20,7 @@ void* FixedTimeslotTask::taskEntryPoint(void* arg) { FixedTimeslotTask *originalTask(reinterpret_cast(arg)); //The task's functionality is called. originalTask->taskFunctionality(); - return NULL; + return nullptr; } ReturnValue_t FixedTimeslotTask::startTask() { diff --git a/osal/linux/FixedTimeslotTask.h b/osal/linux/FixedTimeslotTask.h index 6350f347..916e6d6c 100644 --- a/osal/linux/FixedTimeslotTask.h +++ b/osal/linux/FixedTimeslotTask.h @@ -8,7 +8,20 @@ class FixedTimeslotTask: public FixedTimeslotTaskIF, public PosixThread { public: - FixedTimeslotTask(const char* name_, int priority_, size_t stackSize_, uint32_t periodMs_); + /** + * Create a generic periodic task. + * @param name_ + * Name, maximum allowed size of linux is 16 chars, everything else will + * be truncated. + * @param priority_ + * Real-time priority, ranges from 1 to 99 for Linux. + * See: https://man7.org/linux/man-pages/man7/sched.7.html + * @param stackSize_ + * @param period_ + * @param deadlineMissedFunc_ + */ + FixedTimeslotTask(const char* name_, int priority_, size_t stackSize_, + uint32_t periodMs_); virtual ~FixedTimeslotTask(); virtual ReturnValue_t startTask(); @@ -17,7 +30,9 @@ public: virtual uint32_t getPeriodMs() const; - virtual ReturnValue_t addSlot(object_id_t componentId, uint32_t slotTimeMs, int8_t executionStep); + virtual ReturnValue_t addSlot(object_id_t componentId, uint32_t slotTimeMs, + int8_t executionStep); + virtual ReturnValue_t checkSequence() const; /** @@ -34,11 +49,10 @@ public: protected: /** * @brief This function holds the main functionality of the thread. - * - * - * @details Holding the main functionality of the task, this method is most important. - * It links the functionalities provided by FixedSlotSequence with the OS's System Calls - * to keep the timing of the periods. + * @details + * Holding the main functionality of the task, this method is most important. + * It links the functionalities provided by FixedSlotSequence with the + * OS's System Calls to keep the timing of the periods. */ virtual void taskFunctionality(); @@ -46,8 +60,13 @@ private: /** * @brief This is the entry point in a new thread. * - * @details This method, that is the entry point in the new thread and calls taskFunctionality of the child class. - * Needs a valid pointer to the derived class. + * @details + * This method, that is the entry point in the new thread and calls + * taskFunctionality of the child class. Needs a valid pointer to the + * derived class. + * + * The void* returnvalue is not used yet but could be used to return + * arbitrary data. */ static void* taskEntryPoint(void* arg); FixedSlotSequence pst; diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index edabe946..86d1e0d9 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -1,56 +1,36 @@ #include -#include /* For O_* constants */ -#include /* For mode constants */ -#include -#include -#include #include +#include -MessageQueue::MessageQueue(size_t message_depth, size_t max_message_size) : - id(0), lastPartner(0), defaultDestination(NO_QUEUE) { +#include /* For O_* constants */ +#include /* For mode constants */ +#include +#include + + +MessageQueue::MessageQueue(uint32_t messageDepth, size_t maxMessageSize): id(0), + lastPartner(0), defaultDestination(NO_QUEUE) { //debug << "MessageQueue::MessageQueue: Creating a queue" << std::endl; mq_attr attributes; this->id = 0; //Set attributes attributes.mq_curmsgs = 0; - attributes.mq_maxmsg = message_depth; - attributes.mq_msgsize = max_message_size; + attributes.mq_maxmsg = messageDepth; + attributes.mq_msgsize = maxMessageSize; attributes.mq_flags = 0; //Flags are ignored on Linux during mq_open - //Set the name of the queue - sprintf(name, "/Q%u\n", queueCounter++); + //Set the name of the queue. The slash is mandatory! + sprintf(name, "/FSFW_MQ%u\n", queueCounter++); - //Create a nonblocking queue if the name is available (the queue is Read and - // writable for the owner as well as the group) - mqd_t tempId = mq_open(name, O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL, - S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP | S_IROTH | S_IWOTH, &attributes); + // Create a nonblocking queue if the name is available (the queue is read + // and writable for the owner as well as the group) + int oflag = O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL; + mode_t mode = S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP | S_IROTH | S_IWOTH; + mqd_t tempId = mq_open(name, oflag, mode, &attributes); if (tempId == -1) { - //An error occured during open - //We need to distinguish if it is caused by an already created queue - if (errno == EEXIST) { - //There's another queue with the same name - //We unlink the other queue - int status = mq_unlink(name); - if (status != 0) { - sif::error << "mq_unlink Failed with status: " << strerror(errno) - << std::endl; - } else { - //Successful unlinking, try to open again - mqd_t tempId = mq_open(name, - O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL, - S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP, &attributes); - if (tempId != -1) { - //Successful mq_open - this->id = tempId; - return; - } - } - } - //Failed either the first time or the second time - sif::error << "MessageQueue::MessageQueue: Creating Queue " << std::hex - << name << std::dec << " failed with status: " - << strerror(errno) << std::endl; - } else { + handleError(&attributes, messageDepth); + } + else { //Successful mq_open call this->id = tempId; } @@ -69,6 +49,68 @@ MessageQueue::~MessageQueue() { } } +ReturnValue_t MessageQueue::handleError(mq_attr* attributes, + uint32_t messageDepth) { + switch(errno) { + case(EINVAL): { + sif::error << "MessageQueue::MessageQueue: Invalid name or attributes" + " for message size" << std::endl; + size_t defaultMqMaxMsg = 0; + if(std::ifstream("/proc/sys/fs/mqueue/msg_max",std::ios::in) >> + defaultMqMaxMsg and defaultMqMaxMsg < messageDepth) { + // See: https://www.man7.org/linux/man-pages/man3/mq_open.3.html + // This happens if the msg_max value is not large enough + // It is ignored if the executable is run in privileged mode. + // Run the unlockRealtime script or grant the mode manully by using: + // sudo setcap 'CAP_SYS_RESOURCE=+ep' + + // Permanent solution (EventManager has mq depth of 80): + // echo msg_max | sudo tee /proc/sys/fs/mqueue/msg_max + sif::error << "MessageQueue::MessageQueue: Default MQ size " + << defaultMqMaxMsg << " is too small for requested size " + << messageDepth << std::endl; + } + break; + } + case(EEXIST): { + // An error occured during open + // We need to distinguish if it is caused by an already created queue + if (errno == EEXIST) { + //There's another queue with the same name + //We unlink the other queue + int status = mq_unlink(name); + if (status != 0) { + sif::error << "mq_unlink Failed with status: " << strerror(errno) + << std::endl; + } + else { + // Successful unlinking, try to open again + mqd_t tempId = mq_open(name, + O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL, + S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP, attributes); + if (tempId != -1) { + //Successful mq_open + this->id = tempId; + return HasReturnvaluesIF::RETURN_OK; + } + } + } + break; + } + + default: + // Failed either the first time or the second time + sif::error << "MessageQueue::MessageQueue: Creating Queue " << std::hex + << name << std::dec << " failed with status: " + << strerror(errno) << std::endl; + + } + return HasReturnvaluesIF::RETURN_FAILED; + + + +} + ReturnValue_t MessageQueue::sendMessage(MessageQueueId_t sendTo, MessageQueueMessage* message, bool ignoreFault) { return sendMessageFrom(sendTo, message, this->getId(), false); @@ -265,7 +307,11 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, << strerror(errno) << " in mq_send" << std::endl; /*NO BREAK*/ case EMSGSIZE: - //The msg_len is greater than the msgsize associated with the specified queue. + // The msg_len is greater than the msgsize associated with + //the specified queue. + sif::error << "MessageQueue::sendMessage: Size error [" << + strerror(errno) << "] in mq_send" << std::endl; + /*NO BREAK*/ default: return HasReturnvaluesIF::RETURN_FAILED; } diff --git a/osal/linux/MessageQueue.h b/osal/linux/MessageQueue.h index b8285dc5..2808d36e 100644 --- a/osal/linux/MessageQueue.h +++ b/osal/linux/MessageQueue.h @@ -4,21 +4,26 @@ #include #include #include + +#include /** - * @brief This class manages sending and receiving of message queue messages. + * @brief This class manages sending and receiving of message queue messages. * - * @details Message queues are used to pass asynchronous messages between processes. - * They work like post boxes, where all incoming messages are stored in FIFO - * order. This class creates a new receiving queue and provides methods to fetch - * received messages. Being a child of MessageQueueSender, this class also provides - * methods to send a message to a user-defined or a default destination. In addition - * it also provides a reply method to answer to the queue it received its last message - * from. - * The MessageQueue should be used as "post box" for a single owning object. So all - * message queue communication is "n-to-one". - * For creating the queue, as well as sending and receiving messages, the class makes - * use of the operating system calls provided. - * \ingroup message_queue + * @details + * Message queues are used to pass asynchronous messages between processes. + * They work like post boxes, where all incoming messages are stored in FIFO + * order. This class creates a new receiving queue and provides methods to fetch + * received messages. Being a child of MessageQueueSender, this class also + * provides methods to send a message to a user-defined or a default destination. + * In addition it also provides a reply method to answer to the queue it + * received its last message from. + * + * The MessageQueue should be used as "post box" for a single owning object. + * So all message queue communication is "n-to-one". + * + * The creation of message queues, as well as sending and receiving messages, + * makes use of the operating system calls provided. + * @ingroup message_queue */ class MessageQueue : public MessageQueueIF { friend class MessageQueueSenderIF; @@ -35,7 +40,8 @@ public: * @param max_message_size With this parameter, the maximum message size can be adjusted. * This should be left default. */ - MessageQueue( size_t message_depth = 3, size_t max_message_size = MessageQueueMessage::MAX_MESSAGE_SIZE ); + MessageQueue(uint32_t messageDepth = 3, + size_t maxMessageSize = MessageQueueMessage::MAX_MESSAGE_SIZE ); /** * @brief The destructor deletes the formerly created message queue. * @details This is accomplished by using the delete call provided by the operating system. @@ -168,6 +174,8 @@ private: char name[5]; static uint16_t queueCounter; + + ReturnValue_t handleError(mq_attr* attributes, uint32_t messageDepth); }; #endif /* MESSAGEQUEUE_H_ */ diff --git a/osal/linux/PeriodicPosixTask.cpp b/osal/linux/PeriodicPosixTask.cpp index b754c3f4..df5bbf93 100644 --- a/osal/linux/PeriodicPosixTask.cpp +++ b/osal/linux/PeriodicPosixTask.cpp @@ -3,9 +3,10 @@ #include #include -PeriodicPosixTask::PeriodicPosixTask(const char* name_, int priority_, size_t stackSize_, uint32_t period_, void(deadlineMissedFunc_)()):PosixThread(name_,priority_,stackSize_),objectList(),started(false),periodMs(period_),deadlineMissedFunc( - deadlineMissedFunc_) { - +PeriodicPosixTask::PeriodicPosixTask(const char* name_, int priority_, + size_t stackSize_, uint32_t period_, void(deadlineMissedFunc_)()): + PosixThread(name_,priority_,stackSize_),objectList(),started(false), + periodMs(period_),deadlineMissedFunc(deadlineMissedFunc_) { } PeriodicPosixTask::~PeriodicPosixTask() { @@ -37,7 +38,8 @@ ReturnValue_t PeriodicPosixTask::sleepFor(uint32_t ms) { ReturnValue_t PeriodicPosixTask::startTask(void){ started = true; - createTask(&taskEntryPoint,this); + //sif::info << stackSize << std::endl; + PosixThread::createTask(&taskEntryPoint,this); return HasReturnvaluesIF::RETURN_OK; } @@ -56,9 +58,11 @@ void PeriodicPosixTask::taskFunctionality(void){ char name[20] = {0}; int status = pthread_getname_np(pthread_self(),name,sizeof(name)); if(status==0){ - sif::error << "ObjectTask: " << name << " Deadline missed." << std::endl; + sif::error << "PeriodicPosixTask " << name << ": Deadline " + "missed." << std::endl; }else{ - sif::error << "ObjectTask: X Deadline missed. " << status << std::endl; + sif::error << "PeriodicPosixTask X: Deadline missed. " << + status << std::endl; } if (this->deadlineMissedFunc != NULL) { this->deadlineMissedFunc(); diff --git a/osal/linux/PeriodicPosixTask.h b/osal/linux/PeriodicPosixTask.h index 43647eda..85de0d59 100644 --- a/osal/linux/PeriodicPosixTask.h +++ b/osal/linux/PeriodicPosixTask.h @@ -9,9 +9,22 @@ class PeriodicPosixTask: public PosixThread, public PeriodicTaskIF { public: + /** + * Create a generic periodic task. + * @param name_ + * Name, maximum allowed size of linux is 16 chars, everything else will + * be truncated. + * @param priority_ + * Real-time priority, ranges from 1 to 99 for Linux. + * See: https://man7.org/linux/man-pages/man7/sched.7.html + * @param stackSize_ + * @param period_ + * @param deadlineMissedFunc_ + */ PeriodicPosixTask(const char* name_, int priority_, size_t stackSize_, uint32_t period_, void(*deadlineMissedFunc_)()); virtual ~PeriodicPosixTask(); + /** * @brief The method to start the task. * @details The method starts the task with the respective system call. diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index 899700f0..24545f6f 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -1,8 +1,12 @@ #include +#include #include #include -#include +PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): + thread(0),priority(priority_),stackSize(stackSize_) { + strncpy(name,name_,16); +} PosixThread::~PosixThread() { //No deletion and no free of Stack Pointer @@ -113,12 +117,6 @@ uint64_t PosixThread::getCurrentMonotonicTimeMs(){ return currentTime_ms; } -PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): - thread(0),priority(priority_),stackSize(stackSize_) { - strcpy(name,name_); -} - - void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { //sif::debug << "PosixThread::createTask" << std::endl; @@ -135,14 +133,24 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { sif::error << "Posix Thread attribute init failed with: " << strerror(status) << std::endl; } - void* sp; - status = posix_memalign(&sp, sysconf(_SC_PAGESIZE), stackSize); + void* stackPointer; + status = posix_memalign(&stackPointer, sysconf(_SC_PAGESIZE), stackSize); if(status != 0){ - sif::error << "Posix Thread stack init failed with: " << + sif::error << "PosixThread::createTask: Stack init failed with: " << strerror(status) << std::endl; + if(errno == ENOMEM) { + uint64_t stackMb = stackSize/10e6; + sif::error << "PosixThread::createTask: Insufficient memory for" + " the requested " << stackMb << " MB" << std::endl; + } + else if(errno == EINVAL) { + sif::error << "PosixThread::createTask: Wrong alignment argument!" + << std::endl; + } + return; } - status = pthread_attr_setstack(&attributes, sp, stackSize); + status = pthread_attr_setstack(&attributes, stackPointer, stackSize); if(status != 0){ sif::error << "Posix Thread attribute setStack failed with: " << strerror(status) << std::endl; @@ -188,8 +196,19 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { status = pthread_setname_np(thread,name); if(status != 0){ - sif::error << "Posix Thread setname failed with: " << + sif::error << "PosixThread::createTask: setname failed with: " << strerror(status) << std::endl; + if(status == ERANGE) { + sif::error << "PosixThread::createTask: Task name length longer" + " than 16 chars. Truncating.." << std::endl; + name[15] = '\0'; + status = pthread_setname_np(thread,name); + if(status != 0){ + sif::error << "PosixThread::createTask: Setting name" + " did not work.." << std::endl; + } + } + } status = pthread_attr_destroy(&attributes); diff --git a/osal/linux/PosixThread.h b/osal/linux/PosixThread.h index d96c4156..07233fe5 100644 --- a/osal/linux/PosixThread.h +++ b/osal/linux/PosixThread.h @@ -1,12 +1,11 @@ #ifndef FRAMEWORK_OSAL_LINUX_POSIXTHREAD_H_ #define FRAMEWORK_OSAL_LINUX_POSIXTHREAD_H_ -#include -#include -#include -#include -#include #include +#include +#include +#include +#include class PosixThread { public: @@ -54,21 +53,24 @@ protected: pthread_t thread; /** - * @brief Function that has to be called by derived class because the derived class pointer has to be valid as argument - * @details This function creates a pthread with the given parameters. As the function requires a pointer to the derived object - * it has to be called after the this pointer of the derived object is valid. Sets the taskEntryPoint as - * function to be called by new a thread. - * @param name_ Name of the task - * @param priority_ Priority of the task according to POSIX - * @param stackSize_ Size of the stack attached to that task - * @param arg_ argument of the taskEntryPoint function, needs to be this pointer of derived class + * @brief Function that has to be called by derived class because the + * derived class pointer has to be valid as argument + * @details + * This function creates a pthread with the given parameters. As the + * function requires a pointer to the derived object it has to be called + * after the this pointer of the derived object is valid. + * Sets the taskEntryPoint as function to be called by new a thread. + * @param fnc_ Function which will be executed by the thread. + * @param arg_ + * argument of the taskEntryPoint function, needs to be this pointer + * of derived class */ void createTask(void* (*fnc_)(void*),void* arg_); private: - char name[10]; + char name[16]; int priority; - size_t stackSize; + size_t stackSize = 0; }; #endif /* FRAMEWORK_OSAL_LINUX_POSIXTHREAD_H_ */ diff --git a/osal/linux/QueueFactory.cpp b/osal/linux/QueueFactory.cpp index fc4c9026..268d0b99 100644 --- a/osal/linux/QueueFactory.cpp +++ b/osal/linux/QueueFactory.cpp @@ -5,16 +5,18 @@ #include #include -QueueFactory* QueueFactory::factoryInstance = NULL; +QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom,bool ignoreFault) { - return MessageQueue::sendMessageFromMessageQueue(sendTo,message,sentFrom,ignoreFault); + MessageQueueMessage* message, MessageQueueId_t sentFrom, + bool ignoreFault) { + return MessageQueue::sendMessageFromMessageQueue(sendTo,message, + sentFrom,ignoreFault); } QueueFactory* QueueFactory::instance() { - if (factoryInstance == NULL) { + if (factoryInstance == nullptr) { factoryInstance = new QueueFactory; } return factoryInstance; @@ -26,9 +28,9 @@ QueueFactory::QueueFactory() { QueueFactory::~QueueFactory() { } -MessageQueueIF* QueueFactory::createMessageQueue(uint32_t message_depth, - uint32_t max_message_size) { - return new MessageQueue(message_depth, max_message_size); +MessageQueueIF* QueueFactory::createMessageQueue(uint32_t messageDepth, + size_t maxMessageSize) { + return new MessageQueue(messageDepth, maxMessageSize); } void QueueFactory::deleteMessageQueue(MessageQueueIF* queue) { diff --git a/osal/linux/TaskFactory.cpp b/osal/linux/TaskFactory.cpp index 44f46d90..219630a7 100644 --- a/osal/linux/TaskFactory.cpp +++ b/osal/linux/TaskFactory.cpp @@ -13,12 +13,20 @@ TaskFactory* TaskFactory::instance() { return TaskFactory::factoryInstance; } -PeriodicTaskIF* TaskFactory::createPeriodicTask(TaskName name_,TaskPriority taskPriority_,TaskStackSize stackSize_,TaskPeriod periodInSeconds_,TaskDeadlineMissedFunction deadLineMissedFunction_) { - return static_cast(new PeriodicPosixTask(name_, taskPriority_,stackSize_,periodInSeconds_ * 1000,deadLineMissedFunction_)); +PeriodicTaskIF* TaskFactory::createPeriodicTask(TaskName name_, + TaskPriority taskPriority_,TaskStackSize stackSize_, + TaskPeriod periodInSeconds_, + TaskDeadlineMissedFunction deadLineMissedFunction_) { + return new PeriodicPosixTask(name_, taskPriority_,stackSize_, + periodInSeconds_ * 1000, deadLineMissedFunction_); } -FixedTimeslotTaskIF* TaskFactory::createFixedTimeslotTask(TaskName name_,TaskPriority taskPriority_,TaskStackSize stackSize_,TaskPeriod periodInSeconds_,TaskDeadlineMissedFunction deadLineMissedFunction_) { - return static_cast(new FixedTimeslotTask(name_, taskPriority_,stackSize_,periodInSeconds_*1000)); +FixedTimeslotTaskIF* TaskFactory::createFixedTimeslotTask(TaskName name_, + TaskPriority taskPriority_,TaskStackSize stackSize_, + TaskPeriod periodInSeconds_, + TaskDeadlineMissedFunction deadLineMissedFunction_) { + return new FixedTimeslotTask(name_, taskPriority_,stackSize_, + periodInSeconds_*1000); } ReturnValue_t TaskFactory::deleteTask(PeriodicTaskIF* task) { diff --git a/osal/rtems/QueueFactory.cpp b/osal/rtems/QueueFactory.cpp index 0f2e0ea9..8cc9905c 100644 --- a/osal/rtems/QueueFactory.cpp +++ b/osal/rtems/QueueFactory.cpp @@ -49,9 +49,9 @@ QueueFactory::QueueFactory() { QueueFactory::~QueueFactory() { } -MessageQueueIF* QueueFactory::createMessageQueue(uint32_t message_depth, - uint32_t max_message_size) { - return new MessageQueue(message_depth, max_message_size); +MessageQueueIF* QueueFactory::createMessageQueue(uint32_t messageDepth, + size_t maxMessageSize) { + return new MessageQueue(messageDepth, maxMessageSize); } void QueueFactory::deleteMessageQueue(MessageQueueIF* queue) { From 6ad36ceb24fef5bcc52ae8b5405f74042caba4bf Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 16:24:33 +0200 Subject: [PATCH 02/31] variable for max name len --- osal/linux/PosixThread.cpp | 2 +- osal/linux/PosixThread.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index 24545f6f..3845123c 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -5,7 +5,7 @@ PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): thread(0),priority(priority_),stackSize(stackSize_) { - strncpy(name,name_,16); + strncpy(name, name_, PTHREAD_MAX_NAMELEN); } PosixThread::~PosixThread() { diff --git a/osal/linux/PosixThread.h b/osal/linux/PosixThread.h index 07233fe5..e9d31728 100644 --- a/osal/linux/PosixThread.h +++ b/osal/linux/PosixThread.h @@ -9,6 +9,7 @@ class PosixThread { public: + static constexpr uint8_t PTHREAD_MAX_NAMELEN = 16; PosixThread(const char* name_, int priority_, size_t stackSize_); virtual ~PosixThread(); /** @@ -54,7 +55,7 @@ protected: /** * @brief Function that has to be called by derived class because the - * derived class pointer has to be valid as argument + * derived class pointer has to be valid as argument. * @details * This function creates a pthread with the given parameters. As the * function requires a pointer to the derived object it has to be called @@ -68,7 +69,7 @@ protected: void createTask(void* (*fnc_)(void*),void* arg_); private: - char name[16]; + char name[PTHREAD_MAX_NAMELEN]; int priority; size_t stackSize = 0; }; From be066755c0c2e7465149cecac58bab1e1ab49ea7 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 14:39:08 +0200 Subject: [PATCH 03/31] even safer and more efficient alternative --- osal/linux/PosixThread.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index 3845123c..c63dde9f 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -5,7 +5,8 @@ PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): thread(0),priority(priority_),stackSize(stackSize_) { - strncpy(name, name_, PTHREAD_MAX_NAMELEN); + name[0] = '\0'; + strncat(name, name_, PTHREAD_MAX_NAMELE - 1); } PosixThread::~PosixThread() { From 92f493f13a9c1b5685c0e4e500854aeb3aeef677 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 16:48:21 +0200 Subject: [PATCH 04/31] name correction --- osal/linux/PosixThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index c63dde9f..5750471d 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -6,7 +6,7 @@ PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): thread(0),priority(priority_),stackSize(stackSize_) { name[0] = '\0'; - strncat(name, name_, PTHREAD_MAX_NAMELE - 1); + strncat(name, name_, PTHREAD_MAX_NAMELEN - 1); } PosixThread::~PosixThread() { From c602e30a63278d92ac2951b6452756dea31da539 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 18:45:55 +0200 Subject: [PATCH 05/31] std:: added --- osal/linux/PosixThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index 5750471d..7cf1d9bc 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -6,7 +6,7 @@ PosixThread::PosixThread(const char* name_, int priority_, size_t stackSize_): thread(0),priority(priority_),stackSize(stackSize_) { name[0] = '\0'; - strncat(name, name_, PTHREAD_MAX_NAMELEN - 1); + std::strncat(name, name_, PTHREAD_MAX_NAMELEN - 1); } PosixThread::~PosixThread() { From bf1b86e809a5375fc80031606675e233446c560b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 6 Jun 2020 18:48:28 +0200 Subject: [PATCH 06/31] small form stuff --- osal/linux/PosixThread.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index 7cf1d9bc..bc3e62ed 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -60,10 +60,6 @@ void PosixThread::resume(){ pthread_kill(thread,SIGUSR1); } - - - - bool PosixThread::delayUntil(uint64_t* const prevoiusWakeTime_ms, const uint64_t delayTime_ms) { uint64_t nextTimeToWake_ms; @@ -163,7 +159,7 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { strerror(status) << std::endl; } -//TODO FIFO -> This needs root privileges for the process + // TODO FIFO -> This needs root privileges for the process status = pthread_attr_setschedpolicy(&attributes,SCHED_FIFO); if(status != 0){ sif::error << "Posix Thread attribute schedule policy failed with: " << @@ -209,7 +205,6 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { " did not work.." << std::endl; } } - } status = pthread_attr_destroy(&attributes); From ea2cd7c1a0b4f50e534e0640236dc3a349b4bcdd Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 01:07:08 +0200 Subject: [PATCH 07/31] some fixes, doc improvements --- osal/linux/MessageQueue.cpp | 102 +++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 42 deletions(-) diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index 86d1e0d9..a9c140c6 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -56,16 +56,23 @@ ReturnValue_t MessageQueue::handleError(mq_attr* attributes, sif::error << "MessageQueue::MessageQueue: Invalid name or attributes" " for message size" << std::endl; size_t defaultMqMaxMsg = 0; + // Not POSIX conformant, but should work for all UNIX systems. + // Just an additional helpful printout :-) if(std::ifstream("/proc/sys/fs/mqueue/msg_max",std::ios::in) >> defaultMqMaxMsg and defaultMqMaxMsg < messageDepth) { // See: https://www.man7.org/linux/man-pages/man3/mq_open.3.html // This happens if the msg_max value is not large enough // It is ignored if the executable is run in privileged mode. - // Run the unlockRealtime script or grant the mode manully by using: + // Run the unlockRealtime script or grant the mode manually by using: // sudo setcap 'CAP_SYS_RESOURCE=+ep' - // Permanent solution (EventManager has mq depth of 80): - // echo msg_max | sudo tee /proc/sys/fs/mqueue/msg_max + // Persistent solution for session: + // echo | sudo tee /proc/sys/fs/mqueue/msg_max + + // Permanent solution: + // sudo nano /etc/sysctl.conf + // Append at end: fs/mqueue/msg_max = + // Apply changes with: sudo sysctl -p sif::error << "MessageQueue::MessageQueue: Default MQ size " << defaultMqMaxMsg << " is too small for requested size " << messageDepth << std::endl; @@ -75,24 +82,22 @@ ReturnValue_t MessageQueue::handleError(mq_attr* attributes, case(EEXIST): { // An error occured during open // We need to distinguish if it is caused by an already created queue - if (errno == EEXIST) { - //There's another queue with the same name - //We unlink the other queue - int status = mq_unlink(name); - if (status != 0) { - sif::error << "mq_unlink Failed with status: " << strerror(errno) - << std::endl; - } - else { - // Successful unlinking, try to open again - mqd_t tempId = mq_open(name, - O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL, - S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP, attributes); - if (tempId != -1) { - //Successful mq_open - this->id = tempId; - return HasReturnvaluesIF::RETURN_OK; - } + //There's another queue with the same name + //We unlink the other queue + int status = mq_unlink(name); + if (status != 0) { + sif::error << "mq_unlink Failed with status: " << strerror(errno) + << std::endl; + } + else { + // Successful unlinking, try to open again + mqd_t tempId = mq_open(name, + O_NONBLOCK | O_RDWR | O_CREAT | O_EXCL, + S_IWUSR | S_IREAD | S_IWGRP | S_IRGRP, attributes); + if (tempId != -1) { + //Successful mq_open + this->id = tempId; + return HasReturnvaluesIF::RETURN_OK; } } break; @@ -150,12 +155,13 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { //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. + //No message was received. Keep lastPartner anyway, I might send + //something later. But still, delete packet content. memset(message->getData(), 0, message->MAX_DATA_SIZE); switch(errno){ case EAGAIN: - //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages currently on the specified queue. + //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages + //currently on the specified queue. return MessageQueueIF::EMPTY; case EBADF: //mqdes doesn't represent a valid queue open for reading. @@ -165,9 +171,12 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { case EINVAL: /* * This value indicates one of the following: - * * The pointer to the buffer for storing the received message, msg_ptr, is NULL. - * * The number of bytes requested, msg_len is less than zero. - * * msg_len is anything other than the mq_msgsize of the specified queue, and the QNX extended option MQ_READBUF_DYNAMIC hasn't been set in the queue's mq_flags. + * - The pointer to the buffer for storing the received message, + * msg_ptr, is NULL. + * - The number of bytes requested, msg_len is less than zero. + * - msg_len is anything other than the mq_msgsize of the specified + * queue, and the QNX extended option MQ_READBUF_DYNAMIC hasn't + * been set in the queue's mq_flags. */ sif::error << "MessageQueue::receive: configuration error " << strerror(errno) << std::endl; @@ -175,8 +184,12 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { case EMSGSIZE: /* * This value indicates one of the following: - * * the QNX extended option MQ_READBUF_DYNAMIC hasn't been set, and the given msg_len is shorter than the mq_msgsize for the given queue. - * * the extended option MQ_READBUF_DYNAMIC has been set, but the given msg_len is too short for the message that would have been received. + * - the QNX extended option MQ_READBUF_DYNAMIC hasn't been set, + * and the given msg_len is shorter than the mq_msgsize for + * the given queue. + * - the extended option MQ_READBUF_DYNAMIC has been set, but the + * given msg_len is too short for the message that would have + * been received. */ sif::error << "MessageQueue::receive: configuration error " << strerror(errno) << std::endl; @@ -224,9 +237,10 @@ ReturnValue_t MessageQueue::flush(uint32_t* count) { case EINVAL: /* * This value indicates one of the following: - * * mq_attr is NULL. - * * MQ_MULT_NOTIFY had been set for this queue, and the given mq_flags includes a 0 in the MQ_MULT_NOTIFY bit. Once MQ_MULT_NOTIFY has been turned on, it may never be turned off. - * + * - mq_attr is NULL. + * - MQ_MULT_NOTIFY had been set for this queue, and the given + * mq_flags includes a 0 in the MQ_MULT_NOTIFY bit. Once + * MQ_MULT_NOTIFY has been turned on, it may never be turned off. */ default: return HasReturnvaluesIF::RETURN_FAILED; @@ -275,7 +289,8 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, //TODO: Check if we're in ISR. if (result != 0) { if(!ignoreFault){ - InternalErrorReporterIF* internalErrorReporter = objectManager->get( + InternalErrorReporterIF* internalErrorReporter = + objectManager->get( objects::INTERNAL_ERROR_REPORTER); if (internalErrorReporter != NULL) { internalErrorReporter->queueMessageNotSent(); @@ -283,10 +298,13 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, } switch(errno){ case EAGAIN: - //The O_NONBLOCK flag was set when opening the queue, or the MQ_NONBLOCK flag was set in its attributes, and the specified queue is full. + //The O_NONBLOCK flag was set when opening the queue, or the + //MQ_NONBLOCK flag was set in its attributes, and the + //specified queue is full. return MessageQueueIF::FULL; case EBADF: - //mq_des doesn't represent a valid message queue descriptor, or mq_des wasn't opened for writing. + //mq_des doesn't represent a valid message queue descriptor, + //or mq_des wasn't opened for writing. sif::error << "MessageQueue::sendMessage: Configuration error " << strerror(errno) << " in mq_send mqSendTo: " << sendTo << " sent from " << sentFrom << std::endl; @@ -296,13 +314,13 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, case EINVAL: /* * This value indicates one of the following: - * * msg_ptr is NULL. - * * msg_len is negative. - * * msg_prio is greater than MQ_PRIO_MAX. - * * msg_prio is less than 0. - * * MQ_PRIO_RESTRICT is set in the mq_attr of mq_des, - * and msg_prio is greater than the priority of the calling process. - * */ + * - msg_ptr is NULL. + * - msg_len is negative. + * - msg_prio is greater than MQ_PRIO_MAX. + * - msg_prio is less than 0. + * - MQ_PRIO_RESTRICT is set in the mq_attr of mq_des, and + * msg_prio is greater than the priority of the calling process. + */ sif::error << "MessageQueue::sendMessage: Configuration error " << strerror(errno) << " in mq_send" << std::endl; /*NO BREAK*/ From 4dd037550774263fe9941e96a9212ea5b084531a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 01:10:40 +0200 Subject: [PATCH 08/31] 0 replaced by MQIF:NO_QUEUE --- 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 a9c140c6..a0749ded 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -9,8 +9,9 @@ #include -MessageQueue::MessageQueue(uint32_t messageDepth, size_t maxMessageSize): id(0), - lastPartner(0), defaultDestination(NO_QUEUE) { +MessageQueue::MessageQueue(uint32_t messageDepth, size_t maxMessageSize): + id(MessageQueueIF::NO_QUEUE),lastPartner(MessageQueueIF::NO_QUEUE), + defaultDestination(MessageQueueIF::NO_QUEUE) { //debug << "MessageQueue::MessageQueue: Creating a queue" << std::endl; mq_attr attributes; this->id = 0; From 109fdad8b36ca1fe99f0ebad89fb28b04fa7d197 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 15:34:35 +0200 Subject: [PATCH 09/31] size check for message queue --- osal/FreeRTOS/MessageQueue.cpp | 11 +++++++++-- osal/FreeRTOS/MessageQueue.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index d44b0bc0..cf6130f8 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -7,7 +7,8 @@ // As a first step towards this, introduces system context variable which needs // to be switched manually // Haven't found function to find system context. -MessageQueue::MessageQueue(size_t messageDepth, size_t maxMessageSize) { +MessageQueue::MessageQueue(size_t messageDepth, size_t maxMessageSize): + maxMessageSize(maxMessageSize) { handle = xQueueCreate(messageDepth, maxMessageSize); if (handle == NULL) { sif::error << "MessageQueue: Creation failed" << std::endl; @@ -120,10 +121,16 @@ bool MessageQueue::isDefaultDestinationSet() const { // static core function to send messages. ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF *message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault, CallContext callContext) { message->setSender(sentFrom); BaseType_t result; + if(message->getMaximumMessageSize() > maxMessageSize) { + sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size" + "too large for queue!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(callContext == CallContext::TASK) { result = xQueueSendToBack(reinterpret_cast(sendTo), static_cast(message->getBuffer()), 0); diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index c13a8a20..b7e52bb3 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -204,6 +204,7 @@ private: QueueHandle_t handle; MessageQueueId_t defaultDestination = 0; MessageQueueId_t lastPartner = 0; + const size_t maxMessageSize; //!< Stores the current system context CallContext callContext = CallContext::TASK; }; From 3b2fa978e1b86ccf1b9c113ab958dcaf81e63b4a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 19:06:10 +0200 Subject: [PATCH 10/31] replaced break with continue --- osal/FreeRTOS/FixedTimeslotTask.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index f3a65fcd..5d16a408 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -118,7 +118,7 @@ void FixedTimeslotTask::taskFunctionality() { this->deadlineMissedFunc(); } // Continue immediately, no need to wait. - break; + continue; } // we need to wait before executing the current slot From 5734a0a0e948df50c57f9d589a2eddac479bb70b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 19:22:52 +0200 Subject: [PATCH 11/31] some fixes necessary to perform size check --- action/ActionHelper.cpp | 3 ++- datalinklayer/MapPacketExtraction.cpp | 3 ++- ipc/MessageQueueSenderIF.h | 2 +- monitoring/LimitViolationReporter.cpp | 3 ++- osal/FreeRTOS/MessageQueue.cpp | 8 ++++---- osal/FreeRTOS/MessageQueue.h | 5 +++-- osal/FreeRTOS/QueueFactory.cpp | 6 +++--- tmtcservices/VerificationReporter.cpp | 12 ++++++++---- 8 files changed, 25 insertions(+), 17 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 0df5f2d4..026df4e1 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -101,7 +101,8 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, //TODO Service Implementation sucks at the moment if (hideSender){ - result = MessageQueueSenderIF::sendMessage(reportTo, &reply); + result = MessageQueueSenderIF::sendMessage(reportTo, &reply, + MessageQueueMessage::MAX_MESSAGE_SIZE); } else { result = queueToUse->sendMessage(reportTo, &reply); } diff --git a/datalinklayer/MapPacketExtraction.cpp b/datalinklayer/MapPacketExtraction.cpp index 4ea45e89..87b46556 100644 --- a/datalinklayer/MapPacketExtraction.cpp +++ b/datalinklayer/MapPacketExtraction.cpp @@ -117,7 +117,8 @@ ReturnValue_t MapPacketExtraction::sendCompletePacket(uint8_t* data, ReturnValue_t status = this->packetStore->addData(&store_id, data, size); if (status == RETURN_OK) { TmTcMessage message(store_id); - status = MessageQueueSenderIF::sendMessage(tcQueueId,&message); + status = MessageQueueSenderIF::sendMessage(tcQueueId,&message, + MessageQueueMessage::MAX_MESSAGE_SIZE); } return status; } diff --git a/ipc/MessageQueueSenderIF.h b/ipc/MessageQueueSenderIF.h index 3172454f..912f4d7e 100644 --- a/ipc/MessageQueueSenderIF.h +++ b/ipc/MessageQueueSenderIF.h @@ -15,7 +15,7 @@ public: * Must be implemented by a subclass. */ static ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, + MessageQueueMessageIF* message, size_t maxMessageSize, MessageQueueId_t sentFrom = MessageQueueMessageIF::NO_QUEUE, bool ignoreFault=false); private: diff --git a/monitoring/LimitViolationReporter.cpp b/monitoring/LimitViolationReporter.cpp index 1378d754..c6e2b5db 100644 --- a/monitoring/LimitViolationReporter.cpp +++ b/monitoring/LimitViolationReporter.cpp @@ -34,7 +34,8 @@ ReturnValue_t LimitViolationReporter::sendLimitViolationReport(const SerializeIF MessageQueueMessage message; CommandMessage report(&message); MonitoringMessage::setLimitViolationReport(&report, storeId); - return MessageQueueSenderIF::sendMessage(reportQueue, &report); + return MessageQueueSenderIF::sendMessage(reportQueue, &report, + MessageQueueMessage::MAX_MESSAGE_SIZE); } ReturnValue_t LimitViolationReporter::checkClassLoaded() { diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index cf6130f8..0280869d 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -50,7 +50,7 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { - return sendMessageFromMessageQueue(sendTo, message, sentFrom, + return sendMessageFromMessageQueue(sendTo, message, maxMessageSize, sentFrom, ignoreFault, callContext); } @@ -121,11 +121,11 @@ bool MessageQueue::isDefaultDestinationSet() const { // static core function to send messages. ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom, - bool ignoreFault, CallContext callContext) { + MessageQueueMessageIF* message, size_t maxSize, + MessageQueueId_t sentFrom, bool ignoreFault, CallContext callContext) { message->setSender(sentFrom); BaseType_t result; - if(message->getMaximumMessageSize() > maxMessageSize) { + if(message->getMaximumMessageSize() > maxSize) { sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size" "too large for queue!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index b7e52bb3..e87e884f 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -194,8 +194,9 @@ protected: * @param context Specify whether call is made from task or from an ISR. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, - bool ignoreFault=false, CallContext callContext = CallContext::TASK); + MessageQueueMessageIF* message, size_t maxSize, + MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault=false, + CallContext callContext = CallContext::TASK); static ReturnValue_t handleSendResult(BaseType_t result, bool ignoreFault); diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index beb4969b..ec678890 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -7,9 +7,9 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom, - bool ignoreFault) { - return MessageQueue::sendMessageFromMessageQueue(sendTo,message, + MessageQueueMessageIF* message, size_t maxSize, + MessageQueueId_t sentFrom, bool ignoreFault) { + return MessageQueue::sendMessageFromMessageQueue(sendTo,message, maxSize, sentFrom,ignoreFault); } diff --git a/tmtcservices/VerificationReporter.cpp b/tmtcservices/VerificationReporter.cpp index b0247944..a54c1f8d 100644 --- a/tmtcservices/VerificationReporter.cpp +++ b/tmtcservices/VerificationReporter.cpp @@ -22,7 +22,8 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, current_packet->getAcknowledgeFlags(), current_packet->getPacketId(), current_packet->getPacketSequenceControl(), 0, set_step); - ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); + ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, + &message, MessageQueueMessage::MAX_MESSAGE_SIZE); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendSuccessReport: Error writing " "to queue. Code: " << std::hex << (uint16_t) status << std::endl; @@ -37,7 +38,8 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, } PusVerificationMessage message(set_report_id, ackFlags, tcPacketId, tcSequenceControl, 0, set_step); - ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); + ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, + &message, MessageQueueMessage::MAX_MESSAGE_SIZE); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendSuccessReport: Error writing " "to queue. Code: " << std::hex << (uint16_t) status << std::endl; @@ -55,7 +57,8 @@ void VerificationReporter::sendFailureReport(uint8_t report_id, current_packet->getPacketId(), current_packet->getPacketSequenceControl(), error_code, step, parameter1, parameter2); - ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); + ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, + &message, MessageQueueMessage::MAX_MESSAGE_SIZE); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendFailureReport Error writing to queue. Code: " @@ -72,7 +75,8 @@ void VerificationReporter::sendFailureReport(uint8_t report_id, } PusVerificationMessage message(report_id, ackFlags, tcPacketId, tcSequenceControl, error_code, step, parameter1, parameter2); - ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); + ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, + &message, MessageQueueMessage::MAX_MESSAGE_SIZE); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendFailureReport Error writing to queue. Code: " From dadc867d9ed44d82782d2f3f66cc0f54b50bfe00 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 20:18:13 +0200 Subject: [PATCH 12/31] adapted MessageQueueSenderIF function calls --- events/EventManager.cpp | 3 ++- events/EventManagerIF.h | 3 ++- health/HealthHelper.cpp | 6 ++++-- modes/ModeHelper.cpp | 11 +++++++---- osal/FreeRTOS/FixedTimeslotTask.cpp | 16 +++++++--------- osal/FreeRTOS/MessageQueue.cpp | 12 ++++++++++-- osal/FreeRTOS/QueueFactory.cpp | 5 +++++ parameters/ParameterHelper.cpp | 6 ++++-- tmtcpacket/pus/TmPacketStored.cpp | 3 ++- 9 files changed, 43 insertions(+), 22 deletions(-) diff --git a/events/EventManager.cpp b/events/EventManager.cpp index 30f5ed71..a5d14569 100644 --- a/events/EventManager.cpp +++ b/events/EventManager.cpp @@ -52,7 +52,8 @@ void EventManager::notifyListeners(EventMessage* message) { for (auto iter = listenerList.begin(); iter != listenerList.end(); ++iter) { if (iter->second.match(message)) { MessageQueueSenderIF::sendMessage(iter->first, message, - message->getSender()); + MessageQueueMessage::MAX_MESSAGE_SIZE, + message->getSender()); } } unlockMutex(); diff --git a/events/EventManagerIF.h b/events/EventManagerIF.h index bc0de432..053e320b 100644 --- a/events/EventManagerIF.h +++ b/events/EventManagerIF.h @@ -44,7 +44,8 @@ public: eventmanagerQueue = eventmanager->getEventReportQueue(); } } - MessageQueueSenderIF::sendMessage(eventmanagerQueue, message, sentFrom); + MessageQueueSenderIF::sendMessage(eventmanagerQueue, message, + MessageQueueMessage::MAX_MESSAGE_SIZE, sentFrom); } }; diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index df259b52..935bf5e3 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -70,7 +70,8 @@ void HealthHelper::informParent(HasHealthIF::HealthState health, HealthMessage::setHealthMessage(&information, HealthMessage::HEALTH_INFO, health, oldHealth); if (MessageQueueSenderIF::sendMessage(parentQueue, &information, - owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { + MessageQueueMessage::MAX_MESSAGE_SIZE, + owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::informParent: sending health reply failed." << std::endl; } @@ -90,7 +91,8 @@ void HealthHelper::handleSetHealthCommand(CommandMessage* command) { reply.setReplyRejected(result, command->getCommand()); } if (MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { + MessageQueueMessage::MAX_MESSAGE_SIZE, + owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::handleHealthCommand: sending health " "reply failed." << std::endl; diff --git a/modes/ModeHelper.cpp b/modes/ModeHelper.cpp index b79082af..f636c0bf 100644 --- a/modes/ModeHelper.cpp +++ b/modes/ModeHelper.cpp @@ -28,7 +28,8 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { if (result != HasReturnvaluesIF::RETURN_OK) { ModeMessage::cantReachMode(&reply, result); MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - owner->getCommandQueue()); + MessageQueueMessage::MAX_MESSAGE_SIZE, + owner->getCommandQueue()); break; } //Free to start transition @@ -50,7 +51,8 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_REPLY, mode, submode); MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - owner->getCommandQueue()); + MessageQueueMessage::MAX_MESSAGE_SIZE, + owner->getCommandQueue()); } break; case ModeMessage::CMD_MODE_ANNOUNCE: @@ -95,7 +97,7 @@ void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, ownerMode, ownerSubmode); } MessageQueueSenderIF::sendMessage(theOneWhoCommandedAMode, &reply, - owner->getCommandQueue()); + MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()); } } @@ -109,7 +111,8 @@ void ModeHelper::sendModeInfoMessage(Mode_t ownerMode, ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_INFO, ownerMode, ownerSubmode); MessageQueueSenderIF::sendMessage(parentQueueId, &reply, - owner->getCommandQueue()); + MessageQueueMessage::MAX_MESSAGE_SIZE, + owner->getCommandQueue()); } } diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 5d16a408..4d4ec996 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -105,11 +105,14 @@ void FixedTimeslotTask::taskFunctionality() { //The component for this slot is executed and the next one is chosen. this->pst.executeAndAdvance(); if (not pst.slotFollowsImmediately()) { + // Get the interval till execution of the next slot. + intervalMs = this->pst.getIntervalToPreviousSlotMs(); + interval = pdMS_TO_TICKS(intervalMs); + /* If all operations are finished and the difference of the * current time minus the last wake time is larger than the * expected wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= - pdMS_TO_TICKS(this->pst.getIntervalToPreviousSlotMs())) { + if(xTaskGetTickCount() - xLastWakeTime >= interval) { #ifdef DEBUG sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << " missed deadline!\n" << std::flush; @@ -117,14 +120,9 @@ void FixedTimeslotTask::taskFunctionality() { if(deadlineMissedFunc != nullptr) { this->deadlineMissedFunc(); } - // Continue immediately, no need to wait. - continue; } - - // we need to wait before executing the current slot - //this gives us the time to wait: - intervalMs = this->pst.getIntervalToPreviousSlotMs(); - interval = pdMS_TO_TICKS(intervalMs); + // Wait for the interval. This exits immediately if a deadline was + // missed while also updating the last wake time. vTaskDelayUntil(&xLastWakeTime, interval); } } diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index 0280869d..c9adf229 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -80,6 +80,12 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, } ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { + if(message->getMaximumMessageSize() < maxMessageSize) { + sif::error << "MessageQueue::receiveMessage: Message size " + << message->getMaximumMessageSize() << + " too small to receive data!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } BaseType_t result = xQueueReceive(handle,reinterpret_cast( message->getBuffer()), 0); if (result == pdPASS){ @@ -126,8 +132,10 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, message->setSender(sentFrom); BaseType_t result; if(message->getMaximumMessageSize() > maxSize) { - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size" - "too large for queue!" << std::endl; + sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size " + << message->getMaximumMessageSize() << " too large for queue" + " with max. message size " << maxSize << "!" + << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index ec678890..d754b2b9 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -9,6 +9,11 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, MessageQueueMessageIF* message, size_t maxSize, MessageQueueId_t sentFrom, bool ignoreFault) { + if(maxSize == 0) { + sif::error << "MessageQueueSenderIF::sendMessage: Max Size is 0!" + << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } return MessageQueue::sendMessageFromMessageQueue(sendTo,message, maxSize, sentFrom,ignoreFault); } diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 8b91020f..68ff5ab0 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -107,7 +107,8 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, ParameterMessage::setParameterDumpReply(&reply, id, address); - MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); + MessageQueueSenderIF::sendMessage(to, &reply, + MessageQueueMessage::MAX_MESSAGE_SIZE, ownerQueueId); return HasReturnvaluesIF::RETURN_OK; } @@ -129,5 +130,6 @@ void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, MessageQueueMessage message; CommandMessage reply(&message); reply.setReplyRejected(reason, initialCommand); - MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); + MessageQueueSenderIF::sendMessage(to, &reply, + MessageQueueMessage::MAX_MESSAGE_SIZE, ownerQueueId); } diff --git a/tmtcpacket/pus/TmPacketStored.cpp b/tmtcpacket/pus/TmPacketStored.cpp index 42d0d038..d589db8d 100644 --- a/tmtcpacket/pus/TmPacketStored.cpp +++ b/tmtcpacket/pus/TmPacketStored.cpp @@ -117,7 +117,8 @@ ReturnValue_t TmPacketStored::sendPacket(MessageQueueId_t destination, return HasReturnvaluesIF::RETURN_FAILED; } TmTcMessage tmMessage(getStoreAddress()); - ReturnValue_t result = MessageQueueSenderIF::sendMessage(destination, &tmMessage, sentFrom); + ReturnValue_t result = MessageQueueSenderIF::sendMessage(destination, + &tmMessage, MessageQueueMessage::MAX_MESSAGE_SIZE, sentFrom); if (result != HasReturnvaluesIF::RETURN_OK) { deletePacket(); if (doErrorReporting) { From 6a7f47e06d95c91f8f9885c38f0716894735a10a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 23:46:44 +0200 Subject: [PATCH 13/31] doc fix --- osal/FreeRTOS/FixedTimeslotTask.cpp | 56 ++++++++++++++++++++--------- osal/FreeRTOS/FixedTimeslotTask.h | 6 +++- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.cpp b/osal/FreeRTOS/FixedTimeslotTask.cpp index 4d4ec996..57c927f9 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.cpp +++ b/osal/FreeRTOS/FixedTimeslotTask.cpp @@ -82,7 +82,7 @@ ReturnValue_t FixedTimeslotTask::checkSequence() const { void FixedTimeslotTask::taskFunctionality() { // A local iterator for the Polling Sequence Table is created to find the // start time for the first entry. - SlotListIter slotListIter = pst.current; + auto slotListIter = pst.current; //The start time for the first entry is read. uint32_t intervalMs = slotListIter->pollingTimeMs; @@ -90,9 +90,9 @@ void FixedTimeslotTask::taskFunctionality() { TickType_t xLastWakeTime; /* The xLastWakeTime variable needs to be initialized with the current tick - count. Note that this is the only time the variable is written to explicitly. - After this assignment, xLastWakeTime is updated automatically internally within - vTaskDelayUntil(). */ + count. Note that this is the only time the variable is written to + explicitly. After this assignment, xLastWakeTime is updated automatically + internally within vTaskDelayUntil(). */ xLastWakeTime = xTaskGetTickCount(); // wait for first entry's start time @@ -109,18 +109,8 @@ void FixedTimeslotTask::taskFunctionality() { intervalMs = this->pst.getIntervalToPreviousSlotMs(); interval = pdMS_TO_TICKS(intervalMs); - /* If all operations are finished and the difference of the - * current time minus the last wake time is larger than the - * expected wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= interval) { -#ifdef DEBUG - sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << - " missed deadline!\n" << std::flush; -#endif - if(deadlineMissedFunc != nullptr) { - this->deadlineMissedFunc(); - } - } + checkMissedDeadline(xLastWakeTime, interval); + // Wait for the interval. This exits immediately if a deadline was // missed while also updating the last wake time. vTaskDelayUntil(&xLastWakeTime, interval); @@ -128,6 +118,40 @@ void FixedTimeslotTask::taskFunctionality() { } } +void FixedTimeslotTask::checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval) { + /* Check whether deadline was missed while also taking overflows + * into account. Drawing this on paper with a timeline helps to understand + * it. */ + TickType_t currentTickCount = xTaskGetTickCount(); + TickType_t timeToWake = xLastWakeTime + interval; + // Tick count has overflown + if(currentTickCount < xLastWakeTime) { + // Time to wake has overflown as well. If the tick count + // is larger than the time to wake, a deadline was missed. + if(timeToWake < xLastWakeTime and + currentTickCount > timeToWake) { + handleMissedDeadline(); + } + } + // No tick count overflow. If the timeToWake has not overflown + // and the current tick count is larger than the time to wake, + // a deadline was missed. + else if(timeToWake > xLastWakeTime and currentTickCount > timeToWake) { + handleMissedDeadline(); + } +} + +void FixedTimeslotTask::handleMissedDeadline() { +#ifdef DEBUG + sif::warning << "FixedTimeslotTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } +} + ReturnValue_t FixedTimeslotTask::sleepFor(uint32_t ms) { vTaskDelay(pdMS_TO_TICKS(ms)); return HasReturnvaluesIF::RETURN_OK; diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index 183fad20..32289f4b 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -12,7 +12,7 @@ class FixedTimeslotTask: public FixedTimeslotTaskIF { public: /** - * Keep in Mind that you need to call before this vTaskStartScheduler()! + * Keep in Mind that you need to call before vTaskStartScheduler()! * A lot of task parameters are set in "FreeRTOSConfig.h". * @param name Name of the task, lenght limited by configMAX_TASK_NAME_LEN * @param setPriority Number of priorities specified by @@ -89,6 +89,10 @@ protected: * OS's System Calls to keep the timing of the periods. */ void taskFunctionality(void); + + void checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval); + void handleMissedDeadline(); }; #endif /* POLLINGTASK_H_ */ From a82dbcbd501a6c035430f8394d5eaebe06480918 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 22 Jun 2020 23:47:18 +0200 Subject: [PATCH 14/31] minor doc correction --- osal/FreeRTOS/FixedTimeslotTask.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osal/FreeRTOS/FixedTimeslotTask.h b/osal/FreeRTOS/FixedTimeslotTask.h index 32289f4b..27d08190 100644 --- a/osal/FreeRTOS/FixedTimeslotTask.h +++ b/osal/FreeRTOS/FixedTimeslotTask.h @@ -12,7 +12,7 @@ class FixedTimeslotTask: public FixedTimeslotTaskIF { public: /** - * Keep in Mind that you need to call before vTaskStartScheduler()! + * Keep in mind that you need to call before vTaskStartScheduler()! * A lot of task parameters are set in "FreeRTOSConfig.h". * @param name Name of the task, lenght limited by configMAX_TASK_NAME_LEN * @param setPriority Number of priorities specified by From 847292ea30e3c71678b0cf994e6436caad71a463 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 01:10:19 +0200 Subject: [PATCH 15/31] added overflow checking for periodic task --- osal/FreeRTOS/PeriodicTask.cpp | 47 +++++++++++++++++++++++++--------- osal/FreeRTOS/PeriodicTask.h | 4 +++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index 43928f7c..fa49150e 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -74,18 +74,7 @@ void PeriodicTask::taskFunctionality() { object->performOperation(); } - /* If all operations are finished and the difference of the - * current time minus the last wake time is larger than the - * wait period, a deadline was missed. */ - if(xTaskGetTickCount() - xLastWakeTime >= xPeriod) { -#ifdef DEBUG - sif::warning << "PeriodicTask: " << pcTaskGetName(NULL) << - " missed deadline!\n" << std::flush; -#endif - if(deadlineMissedFunc != nullptr) { - this->deadlineMissedFunc(); - } - } + checkMissedDeadline(xLastWakeTime, xPeriod); vTaskDelayUntil(&xLastWakeTime, xPeriod); @@ -110,3 +99,37 @@ ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { uint32_t PeriodicTask::getPeriodMs() const { return period * 1000; } + +void PeriodicTask::checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval) { + /* Check whether deadline was missed while also taking overflows + * into account. Drawing this on paper with a timeline helps to understand + * it. */ + TickType_t currentTickCount = xTaskGetTickCount(); + TickType_t timeToWake = xLastWakeTime + interval; + // Tick count has overflown + if(currentTickCount < xLastWakeTime) { + // Time to wake has overflown as well. If the tick count + // is larger than the time to wake, a deadline was missed. + if(timeToWake < xLastWakeTime and + currentTickCount > timeToWake) { + handleMissedDeadline(); + } + } + // No tick count overflow. If the timeToWake has not overflown + // and the current tick count is larger than the time to wake, + // a deadline was missed. + else if(timeToWake > xLastWakeTime and currentTickCount > timeToWake) { + handleMissedDeadline(); + } +} + +void PeriodicTask::handleMissedDeadline() { +#ifdef DEBUG + sif::warning << "PeriodicTask: " << pcTaskGetName(NULL) << + " missed deadline!\n" << std::flush; +#endif + if(deadlineMissedFunc != nullptr) { + this->deadlineMissedFunc(); + } +} diff --git a/osal/FreeRTOS/PeriodicTask.h b/osal/FreeRTOS/PeriodicTask.h index a580f519..09aa6bc7 100644 --- a/osal/FreeRTOS/PeriodicTask.h +++ b/osal/FreeRTOS/PeriodicTask.h @@ -116,6 +116,10 @@ protected: * On missing the deadline, the deadlineMissedFunction is executed. */ void taskFunctionality(void); + + void checkMissedDeadline(const TickType_t xLastWakeTime, + const TickType_t interval); + void handleMissedDeadline(); }; #endif /* PERIODICTASK_H_ */ From f7d55a8a37559bf9717b49e414c19db99c5113e9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 01:15:35 +0200 Subject: [PATCH 16/31] equal to pull request now --- osal/FreeRTOS/PeriodicTask.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osal/FreeRTOS/PeriodicTask.cpp b/osal/FreeRTOS/PeriodicTask.cpp index fa49150e..170eb0a4 100644 --- a/osal/FreeRTOS/PeriodicTask.cpp +++ b/osal/FreeRTOS/PeriodicTask.cpp @@ -89,10 +89,11 @@ ReturnValue_t PeriodicTask::addComponent(object_id_t object, bool setTaskIF) { "it implement ExecutableObjectIF" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } - if(setTaskIF) { - newObject->setTaskIF(this); - } objectList.push_back(newObject); + + if(setTaskIF) { + newObject->setTaskIF(this); + } return HasReturnvaluesIF::RETURN_OK; } From 64a02c55ba9c0f067e64b460b66750b613a31526 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 01:37:25 +0200 Subject: [PATCH 17/31] linux fixed, size checks added --- osal/linux/MessageQueue.cpp | 50 +++++++++++++++++++++++++------- osal/linux/MessageQueue.h | 6 ++-- osal/linux/PeriodicPosixTask.cpp | 14 +++++---- osal/linux/PeriodicPosixTask.h | 6 ++-- osal/linux/QueueFactory.cpp | 6 ++-- 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index 605dea2a..c4b24a13 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -11,7 +11,8 @@ MessageQueue::MessageQueue(uint32_t messageDepth, size_t maxMessageSize): id(MessageQueueIF::NO_QUEUE),lastPartner(MessageQueueIF::NO_QUEUE), - defaultDestination(MessageQueueIF::NO_QUEUE) { + defaultDestination(MessageQueueIF::NO_QUEUE), + maxMessageSize(maxMessageSize) { //debug << "MessageQueue::MessageQueue: Creating a queue" << std::endl; mq_attr attributes; this->id = 0; @@ -142,6 +143,19 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, } ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { + if(message == nullptr) { + sif::error << "MessageQueue::receiveMessage: Message is " + "nullptr!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + + if(message->getMaximumMessageSize() < maxMessageSize) { + sif::error << "MessageQueue::receiveMessage: Message size " + << message->getMaximumMessageSize() << + " too small to receive data!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + unsigned int messagePriority = 0; int status = mq_receive(id,reinterpret_cast(message->getBuffer()), message->getMaximumMessageSize(),&messagePriority); @@ -258,18 +272,20 @@ void MessageQueue::setDefaultDestination(MessageQueueId_t defaultDestination) { this->defaultDestination = defaultDestination; } -ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom, - bool ignoreFault) { - return sendMessageFromMessageQueue(sendTo,message,sentFrom,ignoreFault); - -} - ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFrom(defaultDestination, message, sentFrom, ignoreFault); } + +ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, + bool ignoreFault) { + return sendMessageFromMessageQueue(sendTo,message, maxMessageSize, + sentFrom,ignoreFault); + +} + MessageQueueId_t MessageQueue::getDefaultDestination() const { return this->defaultDestination; } @@ -281,8 +297,22 @@ bool MessageQueue::isDefaultDestinationSet() const { uint16_t MessageQueue::queueCounter = 0; ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF *message, MessageQueueId_t sentFrom, - bool ignoreFault) { + MessageQueueMessageIF *message, size_t maxSize, + MessageQueueId_t sentFrom, bool ignoreFault) { + if(message == nullptr) { + sif::error << "MessageQueue::sendMessageFromMessageQueue: Message is " + "nullptr!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + + if(message->getMaximumMessageSize() > maxSize) { + sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size " + << message->getMaximumMessageSize() << " too large for queue" + " with max. message size " << maxSize << "!" + << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + message->setSender(sentFrom); int result = mq_send(sendTo, reinterpret_cast(message->getBuffer()), diff --git a/osal/linux/MessageQueue.h b/osal/linux/MessageQueue.h index 91066bf7..22725d18 100644 --- a/osal/linux/MessageQueue.h +++ b/osal/linux/MessageQueue.h @@ -149,7 +149,8 @@ protected: * \param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, + MessageQueueMessageIF* message, size_t maxSize, + MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault=false); private: /** @@ -176,9 +177,10 @@ private: /** * The name of the message queue, stored for unlinking */ - char name[5]; + char name[16]; static uint16_t queueCounter; + const size_t maxMessageSize; ReturnValue_t handleError(mq_attr* attributes, uint32_t messageDepth); }; diff --git a/osal/linux/PeriodicPosixTask.cpp b/osal/linux/PeriodicPosixTask.cpp index fe8ac19c..db4005b0 100644 --- a/osal/linux/PeriodicPosixTask.cpp +++ b/osal/linux/PeriodicPosixTask.cpp @@ -31,10 +31,11 @@ ReturnValue_t PeriodicPosixTask::addComponent(object_id_t object, "it implements ExecutableObjectIF" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } + objectList.push_back(newObject); + if(setTaskIF) { newObject->setTaskIF(this); } - objectList.push_back(newObject); return HasReturnvaluesIF::RETURN_OK; } @@ -43,14 +44,14 @@ ReturnValue_t PeriodicPosixTask::sleepFor(uint32_t ms) { } -ReturnValue_t PeriodicPosixTask::startTask(void){ +ReturnValue_t PeriodicPosixTask::startTask(void) { started = true; //sif::info << stackSize << std::endl; PosixThread::createTask(&taskEntryPoint,this); return HasReturnvaluesIF::RETURN_OK; } -void PeriodicPosixTask::taskFunctionality(void){ +void PeriodicPosixTask::taskFunctionality(void) { if(!started){ suspend(); } @@ -64,14 +65,15 @@ void PeriodicPosixTask::taskFunctionality(void){ if(!PosixThread::delayUntil(&lastWakeTime,periodMs)){ char name[20] = {0}; int status = pthread_getname_np(pthread_self(),name,sizeof(name)); - if(status==0){ + if(status == 0){ sif::error << "PeriodicPosixTask " << name << ": Deadline " "missed." << std::endl; - }else{ + } + else { sif::error << "PeriodicPosixTask X: Deadline missed. " << status << std::endl; } - if (this->deadlineMissedFunc != NULL) { + if (this->deadlineMissedFunc != nullptr) { this->deadlineMissedFunc(); } } diff --git a/osal/linux/PeriodicPosixTask.h b/osal/linux/PeriodicPosixTask.h index 1d62a565..9b477e36 100644 --- a/osal/linux/PeriodicPosixTask.h +++ b/osal/linux/PeriodicPosixTask.h @@ -32,7 +32,7 @@ public: * The address of the task object is passed as an argument * to the system call. */ - ReturnValue_t startTask(void); + ReturnValue_t startTask(void) override; /** * Adds an object to the list of objects to be executed. * The objects are executed in the order added. @@ -42,9 +42,9 @@ public: ReturnValue_t addComponent(object_id_t object, bool setTaskIF = true) override; - uint32_t getPeriodMs() const; + uint32_t getPeriodMs() const override; - ReturnValue_t sleepFor(uint32_t ms); + ReturnValue_t sleepFor(uint32_t ms) override; private: typedef std::vector ObjectList; //!< Typedef for the List of objects. diff --git a/osal/linux/QueueFactory.cpp b/osal/linux/QueueFactory.cpp index 8bd9d52c..95264a42 100644 --- a/osal/linux/QueueFactory.cpp +++ b/osal/linux/QueueFactory.cpp @@ -9,10 +9,10 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom, - bool ignoreFault) { + MessageQueueMessageIF* message, size_t maxSize, + MessageQueueId_t sentFrom, bool ignoreFault) { return MessageQueue::sendMessageFromMessageQueue(sendTo,message, - sentFrom,ignoreFault); + maxSize, sentFrom,ignoreFault); } QueueFactory* QueueFactory::instance() { From c0beef44630f991e7307727bbcca6adc81bb48a0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 10:27:44 +0200 Subject: [PATCH 18/31] some include guards, todo comments --- devicehandlers/DeviceHandlerFailureIsolation.cpp | 2 +- devicehandlers/DeviceHandlerFailureIsolation.h | 1 + fdir/ConfirmsFailuresIF.h | 1 + fdir/FailureIsolationBase.cpp | 3 ++- health/HealthTable.h | 4 ++-- health/HealthTableIF.h | 8 +++++--- ipc/MessageQueueSenderIF.h | 1 - returnvalues/HasReturnvaluesIF.h | 4 ++-- 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/devicehandlers/DeviceHandlerFailureIsolation.cpp b/devicehandlers/DeviceHandlerFailureIsolation.cpp index 1df2afa0..46e73cd8 100644 --- a/devicehandlers/DeviceHandlerFailureIsolation.cpp +++ b/devicehandlers/DeviceHandlerFailureIsolation.cpp @@ -137,7 +137,7 @@ void DeviceHandlerFailureIsolation::decrementFaultCounters() { void DeviceHandlerFailureIsolation::handleRecovery(Event reason) { clearFaultCounters(); - if (!recoveryCounter.incrementAndCheck()) { + if (not recoveryCounter.incrementAndCheck()) { startRecovery(reason); } else { setFaulty(reason); diff --git a/devicehandlers/DeviceHandlerFailureIsolation.h b/devicehandlers/DeviceHandlerFailureIsolation.h index 379edae3..4343d88b 100644 --- a/devicehandlers/DeviceHandlerFailureIsolation.h +++ b/devicehandlers/DeviceHandlerFailureIsolation.h @@ -31,6 +31,7 @@ protected: bool hasPowerConfirmation = false; MessageQueueId_t powerConfirmation; static object_id_t powerConfirmationId; + // TODO: Are those hardcoded value? How can they be changed. static const uint32_t MAX_REBOOT = 1; static const uint32_t REBOOT_TIME_MS = 180000; static const uint32_t MAX_STRANGE_REPLIES = 10; diff --git a/fdir/ConfirmsFailuresIF.h b/fdir/ConfirmsFailuresIF.h index 99cd212a..460749ee 100644 --- a/fdir/ConfirmsFailuresIF.h +++ b/fdir/ConfirmsFailuresIF.h @@ -4,6 +4,7 @@ #include #include +// TODO: Documentation. class ConfirmsFailuresIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::HANDLES_FAILURES_IF; diff --git a/fdir/FailureIsolationBase.cpp b/fdir/FailureIsolationBase.cpp index 46114467..018fbe6a 100644 --- a/fdir/FailureIsolationBase.cpp +++ b/fdir/FailureIsolationBase.cpp @@ -9,7 +9,8 @@ FailureIsolationBase::FailureIsolationBase(object_id_t owner, object_id_t parent, uint8_t messageDepth, uint8_t parameterDomainBase) : eventQueue(NULL), ownerId(owner), owner(NULL), faultTreeParent(parent), parameterDomainBase(parameterDomainBase) { - eventQueue = QueueFactory::instance()->createMessageQueue(messageDepth, EventMessage::EVENT_MESSAGE_SIZE); + eventQueue = QueueFactory::instance()->createMessageQueue(messageDepth, + EventMessage::EVENT_MESSAGE_SIZE); } FailureIsolationBase::~FailureIsolationBase() { diff --git a/health/HealthTable.h b/health/HealthTable.h index 32a7eee2..194ad259 100644 --- a/health/HealthTable.h +++ b/health/HealthTable.h @@ -1,5 +1,5 @@ -#ifndef HEALTHTABLE_H_ -#define HEALTHTABLE_H_ +#ifndef FRAMEWORK_HEALTH_HEALTHTABLE_H_ +#define FRAMEWORK_HEALTH_HEALTHTABLE_H_ #include #include diff --git a/health/HealthTableIF.h b/health/HealthTableIF.h index 6fdfc2c0..c8d2c74a 100644 --- a/health/HealthTableIF.h +++ b/health/HealthTableIF.h @@ -1,5 +1,5 @@ -#ifndef HEALTHTABLEIF_H_ -#define HEALTHTABLEIF_H_ +#ifndef FRAMEWORK_HEALTH_HEALTHTABLEIF_H_ +#define FRAMEWORK_HEALTH_HEALTHTABLEIF_H_ #include #include @@ -8,6 +8,8 @@ class HealthTableIF: public ManagesHealthIF { + // TODO: This is in the mission folder and not in the framework folder. + // delete it? friend class HealthCommandingService; public: virtual ~HealthTableIF() { @@ -23,4 +25,4 @@ protected: virtual ReturnValue_t iterate(std::pair *value, bool reset = false) = 0; }; -#endif /* HEALTHTABLEIF_H_ */ +#endif /* FRAMEWORK_HEALTH_HEALTHTABLEIF_H_ */ diff --git a/ipc/MessageQueueSenderIF.h b/ipc/MessageQueueSenderIF.h index 912f4d7e..8bc14948 100644 --- a/ipc/MessageQueueSenderIF.h +++ b/ipc/MessageQueueSenderIF.h @@ -12,7 +12,6 @@ public: /** * Allows sending messages without actually "owing" a message queue. * Not sure whether this is actually a good idea. - * Must be implemented by a subclass. */ static ReturnValue_t sendMessage(MessageQueueId_t sendTo, MessageQueueMessageIF* message, size_t maxMessageSize, diff --git a/returnvalues/HasReturnvaluesIF.h b/returnvalues/HasReturnvaluesIF.h index 9bbaf601..b51dfc82 100644 --- a/returnvalues/HasReturnvaluesIF.h +++ b/returnvalues/HasReturnvaluesIF.h @@ -1,5 +1,5 @@ -#ifndef HASRETURNVALUESIF_H_ -#define HASRETURNVALUESIF_H_ +#ifndef FRAMEWORK_RETURNVALUES_HASRETURNVALUESIF_H_ +#define FRAMEWORK_RETURNVALUES_HASRETURNVALUESIF_H_ #include #include From e27310da40a420e78cc894746301d318da632bc9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 10:47:31 +0200 Subject: [PATCH 19/31] updates for tmtcbridge --- tmtcservices/TmTcBridge.cpp | 17 +++++------------ tmtcservices/TmTcBridge.h | 11 ++++++----- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/tmtcservices/TmTcBridge.cpp b/tmtcservices/TmTcBridge.cpp index c5eaf78b..f9a7d3bc 100644 --- a/tmtcservices/TmTcBridge.cpp +++ b/tmtcservices/TmTcBridge.cpp @@ -3,6 +3,7 @@ #include #include #include +#include TmTcBridge::TmTcBridge(object_id_t objectId_, object_id_t ccsdsPacketDistributor_): SystemObject(objectId_), @@ -85,7 +86,7 @@ ReturnValue_t TmTcBridge::handleTm() { return RETURN_FAILED; } - if(tmStored && communicationLinkUp) { + if(tmStored and communicationLinkUp) { result = handleStoredTm(); } return result; @@ -141,11 +142,11 @@ ReturnValue_t TmTcBridge::storeDownlinkData(TmTcMessage *message) { ReturnValue_t TmTcBridge::handleStoredTm() { uint8_t counter = 0; ReturnValue_t result = RETURN_OK; - while(not tmFifo.empty() && counter < sentPacketsPerCycle) { + while(not tmFifo.empty() and counter < sentPacketsPerCycle) { //info << "TMTC Bridge: Sending stored TM data. There are " // << (int) fifo.size() << " left to send\r\n" << std::flush; store_address_t storeId; - const uint8_t* data = NULL; + const uint8_t* data = nullptr; size_t size = 0; tmFifo.retrieve(&storeId); result = tmStore->getData(storeId, &data, &size); @@ -186,14 +187,6 @@ MessageQueueId_t TmTcBridge::getReportReceptionQueue(uint8_t virtualChannel) { } - void TmTcBridge::printData(uint8_t * data, size_t dataLen) { - sif::info << "TMTC Bridge: Printing data: ["; - for(uint32_t i = 0; i < dataLen; i++) { - sif::info << std::hex << (int)data[i]; - if(i < dataLen-1){ - sif::info << " , "; - } - } - sif::info << " ] " << std::endl; + arrayprinter::print(data, dataLen); } diff --git a/tmtcservices/TmTcBridge.h b/tmtcservices/TmTcBridge.h index 0a1fcfab..3e0432d8 100644 --- a/tmtcservices/TmTcBridge.h +++ b/tmtcservices/TmTcBridge.h @@ -43,26 +43,27 @@ public: */ ReturnValue_t setMaxNumberOfPacketsStored(uint8_t maxNumberOfPacketsStored); - void registerCommConnect(); - void registerCommDisconnect(); + virtual void registerCommConnect(); + virtual void registerCommDisconnect(); /** * Initializes necessary FSFW components for the TMTC Bridge * @return */ - ReturnValue_t initialize() override; + virtual ReturnValue_t initialize() override; /** * @brief Handles TMTC reception */ - ReturnValue_t performOperation(uint8_t operationCode = 0) override; + virtual ReturnValue_t performOperation(uint8_t operationCode = 0) override; /** * Return TMTC Reception Queue * @param virtualChannel * @return */ - MessageQueueId_t getReportReceptionQueue(uint8_t virtualChannel = 0) override; + MessageQueueId_t getReportReceptionQueue( + uint8_t virtualChannel = 0) override; protected: //! Used to send and receive TMTC messages. //! TmTcMessage is used to transport messages between tasks. From 45ffb7549aa0654f1423ef5cd8dd0f012ab0537a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 10:58:48 +0200 Subject: [PATCH 20/31] storage accessor const in own file now --- storagemanager/ConstStorageAccessor.cpp | 93 +++++++++++++++++++ storagemanager/ConstStorageAccessor.h | 116 ++++++++++++++++++++++++ storagemanager/StorageAccessor.cpp | 98 +------------------- storagemanager/StorageAccessor.h | 109 +--------------------- 4 files changed, 215 insertions(+), 201 deletions(-) create mode 100644 storagemanager/ConstStorageAccessor.cpp create mode 100644 storagemanager/ConstStorageAccessor.h diff --git a/storagemanager/ConstStorageAccessor.cpp b/storagemanager/ConstStorageAccessor.cpp new file mode 100644 index 00000000..be3188c1 --- /dev/null +++ b/storagemanager/ConstStorageAccessor.cpp @@ -0,0 +1,93 @@ +#include +#include +#include + +ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): + storeId(storeId) {} + +ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId, + StorageManagerIF* store): + storeId(storeId), store(store) { + internalState = AccessState::ASSIGNED; +} + +ConstStorageAccessor::~ConstStorageAccessor() { + if(deleteData and store != nullptr) { + sif::debug << "deleting store data" << std::endl; + store->deleteData(storeId); + } +} + +ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): + constDataPointer(other.constDataPointer), storeId(other.storeId), + size_(other.size_), store(other.store), deleteData(other.deleteData), + internalState(other.internalState) { + // This prevent premature deletion + other.store = nullptr; +} + +ConstStorageAccessor& ConstStorageAccessor::operator=( + ConstStorageAccessor&& other) { + constDataPointer = other.constDataPointer; + storeId = other.storeId; + store = other.store; + size_ = other.size_; + deleteData = other.deleteData; + this->store = other.store; + // This prevents premature deletion + other.store = nullptr; + return *this; +} + +const uint8_t* ConstStorageAccessor::data() const { + return constDataPointer; +} + +size_t ConstStorageAccessor::size() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + } + return size_; +} + +ReturnValue_t ConstStorageAccessor::getDataCopy(uint8_t *pointer, + size_t maxSize) { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + if(size_ > maxSize) { + sif::error << "StorageAccessor: Supplied buffer not large enough" << std::endl; + return HasReturnvaluesIF::RETURN_FAILED; + } + std::copy(constDataPointer, constDataPointer + size_, pointer); + return HasReturnvaluesIF::RETURN_OK; +} + +void ConstStorageAccessor::release() { + deleteData = false; +} + +store_address_t ConstStorageAccessor::getId() const { + return storeId; +} + +void ConstStorageAccessor::print() const { + if(internalState == AccessState::UNINIT) { + sif::warning << "StorageAccessor: Not initialized!" << std::endl; + return; + } + sif::info << "StorageAccessor: Printing data: ["; + for(uint16_t iPool = 0; iPool < size_; iPool++) { + sif::info << std::hex << (int)constDataPointer[iPool]; + if(iPool < size_ - 1){ + sif::info << " , "; + } + } + sif::info << " ] " << std::endl; +} + +void ConstStorageAccessor::assignStore(StorageManagerIF* store) { + internalState = AccessState::ASSIGNED; + this->store = store; +} diff --git a/storagemanager/ConstStorageAccessor.h b/storagemanager/ConstStorageAccessor.h new file mode 100644 index 00000000..cf8ca08f --- /dev/null +++ b/storagemanager/ConstStorageAccessor.h @@ -0,0 +1,116 @@ +#ifndef FRAMEWORK_STORAGEMANAGER_CONSTSTORAGEACCESSOR_H_ +#define FRAMEWORK_STORAGEMANAGER_CONSTSTORAGEACCESSOR_H_ + +#include +#include +#include + +class StorageManagerIF; + +/** + * @brief Helper classes to facilitate safe access to storages which is also + * conforming to RAII principles + * @details + * Accessor class which can be returned by pool manager or passed and set by + * pool managers to have safe access to the pool resources. + * + * These helper can be used together with the StorageManager classes to manage + * access to a storage. It can take care of thread-safety while also providing + * mechanisms to automatically clear storage data. + */ +class ConstStorageAccessor { + //! StorageManager classes have exclusive access to private variables. + template + friend class PoolManager; + template + friend class LocalPool; +public: + /** + * @brief Simple constructor which takes the store ID of the storage + * entry to access. + * @param storeId + */ + ConstStorageAccessor(store_address_t storeId); + ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store); + + /** + * @brief The destructor in default configuration takes care of + * deleting the accessed pool entry and unlocking the mutex + */ + virtual ~ConstStorageAccessor(); + + /** + * @brief Returns a pointer to the read-only data + * @return + */ + const uint8_t* data() const; + + /** + * @brief Copies the read-only data to the supplied pointer + * @param pointer + */ + virtual ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize); + + /** + * @brief Calling this will prevent the Accessor from deleting the data + * when the destructor is called. + */ + void release(); + + /** + * Get the size of the data + * @return + */ + size_t size() const; + + /** + * Get the storage ID. + * @return + */ + store_address_t getId() const; + + void print() const; + + /** + * @brief Move ctor and move assignment allow returning accessors as + * a returnvalue. They prevent resource being free prematurely. + * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ + * move-constructors-and-move-assignment-operators-cpp.md + * @param + * @return + */ + ConstStorageAccessor& operator= (ConstStorageAccessor&&); + ConstStorageAccessor (ConstStorageAccessor&&); + + //! The copy ctor and copy assignemnt should be deleted implicitely + //! according to https://foonathan.net/2019/02/special-member-functions/ + //! but I still deleted them to make it more explicit. (remember rule of 5). + ConstStorageAccessor& operator= (ConstStorageAccessor&) = delete; + ConstStorageAccessor (ConstStorageAccessor&) = delete; +protected: + const uint8_t* constDataPointer = nullptr; + store_address_t storeId; + size_t size_ = 0; + //! Managing pool, has to assign itself. + StorageManagerIF* store = nullptr; + bool deleteData = true; + + enum class AccessState { + UNINIT, + ASSIGNED + }; + //! Internal state for safety reasons. + AccessState internalState = AccessState::UNINIT; + /** + * Used by the pool manager instances to assign themselves to the + * accessor. This is necessary to delete the data when the acessor + * exits the scope ! The internal state will be considered read + * when this function is called, so take care all data is set properly as + * well. + * @param + */ + void assignStore(StorageManagerIF*); +}; + + +#endif /* FRAMEWORK_STORAGEMANAGER_CONSTSTORAGEACCESSOR_H_ */ diff --git a/storagemanager/StorageAccessor.cpp b/storagemanager/StorageAccessor.cpp index c0c8126b..b827b9c0 100644 --- a/storagemanager/StorageAccessor.cpp +++ b/storagemanager/StorageAccessor.cpp @@ -1,96 +1,6 @@ #include #include - -ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): - storeId(storeId) {} - -ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId, - StorageManagerIF* store): - storeId(storeId), store(store) { - internalState = AccessState::ASSIGNED; -} - -ConstStorageAccessor::~ConstStorageAccessor() { - if(deleteData and store != nullptr) { - sif::debug << "deleting store data" << std::endl; - store->deleteData(storeId); - } -} - -ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other): - constDataPointer(other.constDataPointer), storeId(other.storeId), - size_(other.size_), store(other.store), deleteData(other.deleteData), - internalState(other.internalState) { - // This prevent premature deletion - other.store = nullptr; -} - -ConstStorageAccessor& ConstStorageAccessor::operator=( - ConstStorageAccessor&& other) { - constDataPointer = other.constDataPointer; - storeId = other.storeId; - store = other.store; - size_ = other.size_; - deleteData = other.deleteData; - this->store = other.store; - // This prevents premature deletion - other.store = nullptr; - return *this; -} - -const uint8_t* ConstStorageAccessor::data() const { - return constDataPointer; -} - -size_t ConstStorageAccessor::size() const { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - } - return size_; -} - -ReturnValue_t ConstStorageAccessor::getDataCopy(uint8_t *pointer, - size_t maxSize) { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - if(size_ > maxSize) { - sif::error << "StorageAccessor: Supplied buffer not large enough" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - std::copy(constDataPointer, constDataPointer + size_, pointer); - return HasReturnvaluesIF::RETURN_OK; -} - -void ConstStorageAccessor::release() { - deleteData = false; -} - -store_address_t ConstStorageAccessor::getId() const { - return storeId; -} - -void ConstStorageAccessor::print() const { - if(internalState == AccessState::UNINIT) { - sif::warning << "StorageAccessor: Not initialized!" << std::endl; - return; - } - sif::info << "StorageAccessor: Printing data: ["; - for(uint16_t iPool = 0; iPool < size_; iPool++) { - sif::info << std::hex << (int)constDataPointer[iPool]; - if(iPool < size_ - 1){ - sif::info << " , "; - } - } - sif::info << " ] " << std::endl; -} - -void ConstStorageAccessor::assignStore(StorageManagerIF* store) { - internalState = AccessState::ASSIGNED; - this->store = store; -} - +#include StorageAccessor::StorageAccessor(store_address_t storeId): ConstStorageAccessor(storeId) { @@ -120,7 +30,8 @@ ReturnValue_t StorageAccessor::getDataCopy(uint8_t *pointer, size_t maxSize) { return HasReturnvaluesIF::RETURN_FAILED; } if(size_ > maxSize) { - sif::error << "StorageAccessor: Supplied buffer not large enough" << std::endl; + sif::error << "StorageAccessor: Supplied buffer not large " + "enough" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } std::copy(dataPointer, dataPointer + size_, pointer); @@ -141,7 +52,8 @@ ReturnValue_t StorageAccessor::write(uint8_t *data, size_t size, return HasReturnvaluesIF::RETURN_FAILED; } if(offset + size > size_) { - sif::error << "StorageAccessor: Data too large for pool entry!" << std::endl; + sif::error << "StorageAccessor: Data too large for pool " + "entry!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } std::copy(data, data + size, dataPointer + offset); diff --git a/storagemanager/StorageAccessor.h b/storagemanager/StorageAccessor.h index 06947974..c5e38306 100644 --- a/storagemanager/StorageAccessor.h +++ b/storagemanager/StorageAccessor.h @@ -1,117 +1,10 @@ #ifndef FRAMEWORK_STORAGEMANAGER_STORAGEACCESSOR_H_ #define FRAMEWORK_STORAGEMANAGER_STORAGEACCESSOR_H_ -#include -#include +#include class StorageManagerIF; -/** - * @brief Helper classes to facilitate safe access to storages which is also - * conforming to RAII principles - * @details - * Accessor class which can be returned by pool manager or passed and set by - * pool managers to have safe access to the pool resources. - * - * These helper can be used together with the StorageManager classes to manage - * access to a storage. It can take care of thread-safety while also providing - * mechanisms to automatically clear storage data. - */ -class ConstStorageAccessor { - //! StorageManager classes have exclusive access to private variables. - template - friend class PoolManager; - template - friend class LocalPool; -public: - /** - * @brief Simple constructor which takes the store ID of the storage - * entry to access. - * @param storeId - */ - ConstStorageAccessor(store_address_t storeId); - ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store); - - /** - * @brief The destructor in default configuration takes care of - * deleting the accessed pool entry and unlocking the mutex - */ - virtual ~ConstStorageAccessor(); - - /** - * @brief Returns a pointer to the read-only data - * @return - */ - const uint8_t* data() const; - - /** - * @brief Copies the read-only data to the supplied pointer - * @param pointer - */ - virtual ReturnValue_t getDataCopy(uint8_t *pointer, size_t maxSize); - - /** - * @brief Calling this will prevent the Accessor from deleting the data - * when the destructor is called. - */ - void release(); - - /** - * Get the size of the data - * @return - */ - size_t size() const; - - /** - * Get the storage ID. - * @return - */ - store_address_t getId() const; - - void print() const; - - /** - * @brief Move ctor and move assignment allow returning accessors as - * a returnvalue. They prevent resource being free prematurely. - * Refer to: https://github.com/MicrosoftDocs/cpp-docs/blob/master/docs/cpp/ - * move-constructors-and-move-assignment-operators-cpp.md - * @param - * @return - */ - ConstStorageAccessor& operator= (ConstStorageAccessor&&); - ConstStorageAccessor (ConstStorageAccessor&&); - - //! The copy ctor and copy assignemnt should be deleted implicitely - //! according to https://foonathan.net/2019/02/special-member-functions/ - //! but I still deleted them to make it more explicit. (remember rule of 5). - ConstStorageAccessor& operator= (const ConstStorageAccessor&) = delete; - ConstStorageAccessor (const ConstStorageAccessor&) = delete; -protected: - const uint8_t* constDataPointer = nullptr; - store_address_t storeId; - size_t size_ = 0; - //! Managing pool, has to assign itself. - StorageManagerIF* store = nullptr; - bool deleteData = true; - - enum class AccessState { - UNINIT, - ASSIGNED - }; - //! Internal state for safety reasons. - AccessState internalState = AccessState::UNINIT; - /** - * Used by the pool manager instances to assign themselves to the - * accessor. This is necessary to delete the data when the acessor - * exits the scope ! The internal state will be considered read - * when this function is called, so take care all data is set properly as - * well. - * @param - */ - void assignStore(StorageManagerIF*); -}; - - /** * @brief Child class for modifyable data. Also has a normal pointer member. */ From 446e7d2f82d46ce9690d3f2c4c84b166407a8c2d Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 11:00:40 +0200 Subject: [PATCH 21/31] const storage accessor improvement --- storagemanager/ConstStorageAccessor.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/storagemanager/ConstStorageAccessor.cpp b/storagemanager/ConstStorageAccessor.cpp index be3188c1..0bfde58c 100644 --- a/storagemanager/ConstStorageAccessor.cpp +++ b/storagemanager/ConstStorageAccessor.cpp @@ -1,6 +1,7 @@ #include #include #include +#include ConstStorageAccessor::ConstStorageAccessor(store_address_t storeId): storeId(storeId) {} @@ -73,18 +74,11 @@ store_address_t ConstStorageAccessor::getId() const { } void ConstStorageAccessor::print() const { - if(internalState == AccessState::UNINIT) { + if(internalState == AccessState::UNINIT or constDataPointer == nullptr) { sif::warning << "StorageAccessor: Not initialized!" << std::endl; return; } - sif::info << "StorageAccessor: Printing data: ["; - for(uint16_t iPool = 0; iPool < size_; iPool++) { - sif::info << std::hex << (int)constDataPointer[iPool]; - if(iPool < size_ - 1){ - sif::info << " , "; - } - } - sif::info << " ] " << std::endl; + arrayprinter::print(constDataPointer, size_); } void ConstStorageAccessor::assignStore(StorageManagerIF* store) { From 2ecd7c44937180ab0aad13a1185ed9cc54c9eaa4 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 11:05:40 +0200 Subject: [PATCH 22/31] some minor improvements --- storagemanager/LocalPool.h | 11 +++++++---- storagemanager/PoolManager.h | 3 +++ storagemanager/PoolManager.tpp | 6 +++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/storagemanager/LocalPool.h b/storagemanager/LocalPool.h index bb94a3be..7e349dcd 100644 --- a/storagemanager/LocalPool.h +++ b/storagemanager/LocalPool.h @@ -35,7 +35,7 @@ public: /** * @brief This is the default constructor for a pool manager instance. * @details By passing two arrays of size NUMBER_OF_POOLS, the constructor - * allocates memory (with \c new) for store and size_list. These + * allocates memory (with @c new) for store and size_list. These * regions are all set to zero on start up. * @param setObjectId The object identifier to be set. This allows for * multiple instances of LocalPool in the system. @@ -87,7 +87,7 @@ public: ReturnValue_t initialize() override; protected: /** - * With this helper method, a free element of \c size is reserved. + * With this helper method, a free element of @c size is reserved. * @param size The minimum packet size that shall be reserved. * @param[out] address Storage ID of the reserved data. * @return - #RETURN_OK on success, @@ -100,7 +100,8 @@ protected: private: /** * Indicates that this element is free. - * This value limits the maximum size of a pool. Change to larger data type if increase is required. + * This value limits the maximum size of a pool. Change to larger data type + * if increase is required. */ static const uint32_t STORAGE_FREE = 0xFFFFFFFF; /** @@ -126,7 +127,9 @@ private: * is also dynamically allocated there. */ uint32_t* size_list[NUMBER_OF_POOLS]; - bool spillsToHigherPools; //!< A variable to determine whether higher n pools are used if the store is full. + //! A variable to determine whether higher n pools are used if + //! the store is full. + bool spillsToHigherPools; /** * @brief This method safely stores the given data in the given packet_id. * @details It also sets the size in size_list. The method does not perform diff --git a/storagemanager/PoolManager.h b/storagemanager/PoolManager.h index 7c2e8a71..3c1c1217 100644 --- a/storagemanager/PoolManager.h +++ b/storagemanager/PoolManager.h @@ -29,6 +29,9 @@ public: store_address_t* storeId = nullptr) override; protected: + //! Default mutex timeout value to prevent permanent blocking. + static constexpr uint32_t mutexTimeout = 50; + ReturnValue_t reserveSpace(const uint32_t size, store_address_t* address, bool ignoreFault) override; diff --git a/storagemanager/PoolManager.tpp b/storagemanager/PoolManager.tpp index a35b810c..4ca22b85 100644 --- a/storagemanager/PoolManager.tpp +++ b/storagemanager/PoolManager.tpp @@ -21,7 +21,7 @@ inline PoolManager::~PoolManager(void) { template inline ReturnValue_t PoolManager::reserveSpace( const uint32_t size, store_address_t* address, bool ignoreFault) { - MutexHelper mutexHelper(mutex,MutexIF::BLOCKING); + MutexHelper mutexHelper(mutex, mutexTimeout); ReturnValue_t status = LocalPool::reserveSpace(size, address,ignoreFault); return status; @@ -33,7 +33,7 @@ inline ReturnValue_t PoolManager::deleteData( // debug << "PoolManager( " << translateObject(getObjectId()) << // " )::deleteData from store " << packet_id.pool_index << // ". id is "<< packet_id.packet_index << std::endl; - MutexHelper mutexHelper(mutex,MutexIF::BLOCKING); + MutexHelper mutexHelper(mutex, mutexTimeout); ReturnValue_t status = LocalPool::deleteData(packet_id); return status; } @@ -41,7 +41,7 @@ inline ReturnValue_t PoolManager::deleteData( template inline ReturnValue_t PoolManager::deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) { - MutexHelper mutexHelper(mutex,MutexIF::BLOCKING); + MutexHelper mutexHelper(mutex, mutexTimeout); ReturnValue_t status = LocalPool::deleteData(buffer, size, storeId); return status; From 6f4682e1c8848e8fb578af9329c6f274e84a6f60 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 12:53:30 +0200 Subject: [PATCH 23/31] hasactionsIF include guard and doc --- action/HasActionsIF.h | 47 ++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/action/HasActionsIF.h b/action/HasActionsIF.h index 12ecb89a..9c2a9c01 100644 --- a/action/HasActionsIF.h +++ b/action/HasActionsIF.h @@ -1,5 +1,5 @@ -#ifndef HASACTIONSIF_H_ -#define HASACTIONSIF_H_ +#ifndef FRAMEWORK_ACTION_HASACTIONSIF_H_ +#define FRAMEWORK_ACTION_HASACTIONSIF_H_ #include #include @@ -11,22 +11,26 @@ * Interface for component which uses actions * * @details - * This interface is used to execute actions in the component. Actions, in the sense of this interface, are activities with a well-defined beginning and - * end in time. They may adjust sub-states of components, but are not supposed to change - * the main mode of operation, which is handled with the HasModesIF described below. + * This interface is used to execute actions in the component. Actions, in the + * sense of this interface, are activities with a well-defined beginning and + * end in time. They may adjust sub-states of components, but are not supposed + * to change the main mode of operation, which is handled with the HasModesIF + * described below. * - * The HasActionsIF allows components to define such actions and make them available - * for other components to use. Implementing the interface is straightforward: There’s a - * single executeAction call, which provides an identifier for the action to execute, as well - * as arbitrary parameters for input. Aside from direct, software-based - * actions, it is used in device handler components as an interface to forward commands to - * devices. - * Implementing components of the interface are supposed to check identifier (ID) and - * parameters and immediately start execution of the action. It is, however, not required to - * immediately finish execution. Instead, this may be deferred to a later point in time, at - * which the component needs to inform the caller about finished or failed execution. + * The HasActionsIF allows components to define such actions and make them + * available for other components to use. Implementing the interface is + * straightforward: There’s a single executeAction call, which provides an + * identifier for the action to execute, as well as arbitrary parameters for + * input. + * Aside from direct, software-based actions, it is used in device handler + * components as an interface to forward commands to devices. + * Implementing components of the interface are supposed to check identifier + * (ID) and parameters and immediately start execution of the action. + * It is, however, not required to immediately finish execution. + * Instead, this may be deferred to a later point in time, at which the + * component needs to inform the caller about finished or failed execution. * - * \ingroup interfaces + * @ingroup interfaces */ class HasActionsIF { public: @@ -43,13 +47,14 @@ public: virtual MessageQueueId_t getCommandQueue() const = 0; /** * Execute or initialize the execution of a certain function. - * Returning #EXECUTION_FINISHED or a failure code, nothing else needs to be done. - * When needing more steps, return RETURN_OK and issue steps and completion manually. + * Returning #EXECUTION_FINISHED or a failure code, nothing else needs to + * be done. When needing more steps, return RETURN_OK and issue steps and + * completion manually. * One "step failed" or completion report must be issued! */ - virtual ReturnValue_t executeAction(ActionId_t actionId, MessageQueueId_t commandedBy, - const uint8_t* data, size_t size) = 0; + virtual ReturnValue_t executeAction(ActionId_t actionId, + MessageQueueId_t commandedBy, const uint8_t* data, size_t size) = 0; }; -#endif /* HASACTIONSIF_H_ */ +#endif /* FRAMEWORK_ACTION_HASACTIONSIF_H_ */ From 5f16d30d822a26fd42ddd2cdc4bc5211db695f41 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 13:47:38 +0200 Subject: [PATCH 24/31] all ones value for return failed now --- returnvalues/HasReturnvaluesIF.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/returnvalues/HasReturnvaluesIF.h b/returnvalues/HasReturnvaluesIF.h index b51dfc82..6ead6fbb 100644 --- a/returnvalues/HasReturnvaluesIF.h +++ b/returnvalues/HasReturnvaluesIF.h @@ -12,7 +12,8 @@ typedef uint16_t ReturnValue_t; class HasReturnvaluesIF { public: static const ReturnValue_t RETURN_OK = 0; - static const ReturnValue_t RETURN_FAILED = 0xFFFF; + //! This will be the alll-ones value irrespective of used unsigned datatype. + static const ReturnValue_t RETURN_FAILED = -1; virtual ~HasReturnvaluesIF() {} }; From 67366c25a0bd0b4e532cb8ee48d3d795215c773b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 13:49:25 +0200 Subject: [PATCH 25/31] typo fix --- returnvalues/HasReturnvaluesIF.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/returnvalues/HasReturnvaluesIF.h b/returnvalues/HasReturnvaluesIF.h index 6ead6fbb..7a861b50 100644 --- a/returnvalues/HasReturnvaluesIF.h +++ b/returnvalues/HasReturnvaluesIF.h @@ -12,7 +12,7 @@ typedef uint16_t ReturnValue_t; class HasReturnvaluesIF { public: static const ReturnValue_t RETURN_OK = 0; - //! This will be the alll-ones value irrespective of used unsigned datatype. + //! This will be the all-ones value irrespective of used unsigned datatype. static const ReturnValue_t RETURN_FAILED = -1; virtual ~HasReturnvaluesIF() {} }; From 56455a5fa290d69f62ad6a17a2ad66b311401c5e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 14:11:53 +0200 Subject: [PATCH 26/31] added static function as alternative to macro --- returnvalues/HasReturnvaluesIF.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/returnvalues/HasReturnvaluesIF.h b/returnvalues/HasReturnvaluesIF.h index 7a861b50..04acd66e 100644 --- a/returnvalues/HasReturnvaluesIF.h +++ b/returnvalues/HasReturnvaluesIF.h @@ -15,6 +15,10 @@ public: //! This will be the all-ones value irrespective of used unsigned datatype. static const ReturnValue_t RETURN_FAILED = -1; virtual ~HasReturnvaluesIF() {} + + static ReturnValue_t makeReturnCode(uint8_t interfaceId, uint8_t number) { + return (interfaceId << 8) + number; + } }; #endif /* HASRETURNVALUESIF_H_ */ From 905c1a92e38865558e25bebb10a8c7ca380d2ab6 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 23 Jun 2020 21:03:22 +0200 Subject: [PATCH 27/31] reverted some changes --- action/ActionHelper.cpp | 3 +-- datalinklayer/MapPacketExtraction.cpp | 3 +-- events/EventManager.cpp | 1 - events/EventManagerIF.h | 3 +-- health/HealthHelper.cpp | 2 -- housekeeping/HousekeepingMessage.cpp | 10 +++++----- housekeeping/HousekeepingMessage.h | 2 +- ipc/CommandMessage.cpp | 22 +++++++++++----------- ipc/CommandMessage.h | 2 +- ipc/CommandMessageBase.cpp | 16 ++++++---------- ipc/CommandMessageBase.h | 7 ------- ipc/MessageQueueMessage.cpp | 16 ++++++---------- ipc/MessageQueueMessage.h | 12 ------------ ipc/MessageQueueMessageIF.h | 14 +------------- ipc/MessageQueueSenderIF.h | 7 +++---- modes/ModeHelper.cpp | 5 +---- monitoring/LimitViolationReporter.cpp | 3 +-- osal/FreeRTOS/MessageQueue.cpp | 19 ++++++------------- osal/FreeRTOS/MessageQueue.h | 5 ++--- osal/FreeRTOS/QueueFactory.cpp | 11 +++-------- parameters/ParameterHelper.cpp | 6 ++---- tmtcpacket/pus/TmPacketStored.cpp | 2 +- tmtcservices/VerificationReporter.cpp | 8 ++++---- 23 files changed, 57 insertions(+), 122 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 026df4e1..0df5f2d4 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -101,8 +101,7 @@ ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, //TODO Service Implementation sucks at the moment if (hideSender){ - result = MessageQueueSenderIF::sendMessage(reportTo, &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE); + result = MessageQueueSenderIF::sendMessage(reportTo, &reply); } else { result = queueToUse->sendMessage(reportTo, &reply); } diff --git a/datalinklayer/MapPacketExtraction.cpp b/datalinklayer/MapPacketExtraction.cpp index 87b46556..4ea45e89 100644 --- a/datalinklayer/MapPacketExtraction.cpp +++ b/datalinklayer/MapPacketExtraction.cpp @@ -117,8 +117,7 @@ ReturnValue_t MapPacketExtraction::sendCompletePacket(uint8_t* data, ReturnValue_t status = this->packetStore->addData(&store_id, data, size); if (status == RETURN_OK) { TmTcMessage message(store_id); - status = MessageQueueSenderIF::sendMessage(tcQueueId,&message, - MessageQueueMessage::MAX_MESSAGE_SIZE); + status = MessageQueueSenderIF::sendMessage(tcQueueId,&message); } return status; } diff --git a/events/EventManager.cpp b/events/EventManager.cpp index a5d14569..04be6f68 100644 --- a/events/EventManager.cpp +++ b/events/EventManager.cpp @@ -52,7 +52,6 @@ void EventManager::notifyListeners(EventMessage* message) { for (auto iter = listenerList.begin(); iter != listenerList.end(); ++iter) { if (iter->second.match(message)) { MessageQueueSenderIF::sendMessage(iter->first, message, - MessageQueueMessage::MAX_MESSAGE_SIZE, message->getSender()); } } diff --git a/events/EventManagerIF.h b/events/EventManagerIF.h index 053e320b..bc0de432 100644 --- a/events/EventManagerIF.h +++ b/events/EventManagerIF.h @@ -44,8 +44,7 @@ public: eventmanagerQueue = eventmanager->getEventReportQueue(); } } - MessageQueueSenderIF::sendMessage(eventmanagerQueue, message, - MessageQueueMessage::MAX_MESSAGE_SIZE, sentFrom); + MessageQueueSenderIF::sendMessage(eventmanagerQueue, message, sentFrom); } }; diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index 935bf5e3..0b3bffcb 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -70,7 +70,6 @@ void HealthHelper::informParent(HasHealthIF::HealthState health, HealthMessage::setHealthMessage(&information, HealthMessage::HEALTH_INFO, health, oldHealth); if (MessageQueueSenderIF::sendMessage(parentQueue, &information, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::informParent: sending health reply failed." << std::endl; @@ -91,7 +90,6 @@ void HealthHelper::handleSetHealthCommand(CommandMessage* command) { reply.setReplyRejected(result, command->getCommand()); } if (MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::handleHealthCommand: sending health " "reply failed." << std::endl; diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index d7efd703..f425d16e 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -3,13 +3,13 @@ HousekeepingMessage::HousekeepingMessage(MessageQueueMessageIF* message): CommandMessageBase(message) { - if(message->getMaximumMessageSize() < HK_MESSAGE_SIZE) { + if(message->getMessageSize() < HK_MESSAGE_SIZE) { sif::error << "HousekeepingMessage::HousekeepingMessage: Passed " "message buffer can not hold minimum " << HK_MESSAGE_SIZE << " bytes!" << std::endl; return; } - message->setMessageSize(HK_MESSAGE_SIZE); + //message->setMessageSize(HK_MESSAGE_SIZE); } HousekeepingMessage::~HousekeepingMessage() {} @@ -38,9 +38,9 @@ void HousekeepingMessage::setHkDiagnosticsMessage(sid_t sid, setParameter(storeId.raw); } -size_t HousekeepingMessage::getMinimumMessageSize() const { - return HK_MESSAGE_SIZE; -} +//size_t HousekeepingMessage::getMinimumMessageSize() const { +// return HK_MESSAGE_SIZE; +//} sid_t HousekeepingMessage::getSid() const { sid_t sid; diff --git a/housekeeping/HousekeepingMessage.h b/housekeeping/HousekeepingMessage.h index c6a33c83..f568a418 100644 --- a/housekeeping/HousekeepingMessage.h +++ b/housekeeping/HousekeepingMessage.h @@ -104,7 +104,7 @@ public: //! to distinguish between diagnostics and regular HK packets sid_t getHkReportMessage(store_address_t * storeIdToSet) const; - virtual size_t getMinimumMessageSize() const override; + //virtual size_t getMinimumMessageSize() const override; virtual void clear() override; private: diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 9a8a82f2..11355e38 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -9,14 +9,14 @@ CommandMessage::CommandMessage(MessageQueueMessageIF* receiverMessage): " message!" << std::endl; return; } - if(receiverMessage->getMaximumMessageSize() < - getMinimumMessageSize()) { - sif::error << "CommandMessage::ComandMessage: Passed message buffer" - " can not hold minimum "<< getMinimumMessageSize() - << " bytes!" << std::endl; - return; - } - internalMessage->setMessageSize(getMinimumMessageSize()); +// if(receiverMessage->getMessageSize() < +// getMinimumMessageSize()) { +// sif::error << "CommandMessage::ComandMessage: Passed message buffer" +// " can not hold minimum "<< getMinimumMessageSize() +// << " bytes!" << std::endl; +// return; +// } +// internalMessage->setMessageSize(getMinimumMessageSize()); } CommandMessage::CommandMessage(MessageQueueMessageIF* messageToSet, @@ -49,9 +49,9 @@ void CommandMessage::setParameter2(uint32_t parameter2) { sizeof(parameter2)); } -size_t CommandMessage::getMinimumMessageSize() const { - return MINIMUM_COMMAND_MESSAGE_SIZE; -} +//size_t CommandMessage::getMinimumMessageSize() const { +// return MINIMUM_COMMAND_MESSAGE_SIZE; +//} bool CommandMessage::isClearedCommandMessage() { return getCommand() == CMD_NONE; diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index a09494c6..ae4016b0 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -54,7 +54,7 @@ public: virtual ~CommandMessage() {} /** MessageQueueMessageIF functions used for minimum size check. */ - size_t getMinimumMessageSize() const override; + //size_t getMinimumMessageSize() const override; /** * Get the first parameter of the message diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp index 80f9d378..ea50f4ea 100644 --- a/ipc/CommandMessageBase.cpp +++ b/ipc/CommandMessageBase.cpp @@ -47,25 +47,21 @@ const uint8_t* CommandMessageBase::getData() const { return internalMessage->getData() + sizeof(Command_t); } -void CommandMessageBase::setMessageSize(size_t messageSize) { - internalMessage->setMessageSize(messageSize); -} +//void CommandMessageBase::setMessageSize(size_t messageSize) { +// //internalMessage->setMessageSize(messageSize); +//} size_t CommandMessageBase::getMessageSize() const { return internalMessage->getMessageSize(); } -size_t CommandMessageBase::getMaximumMessageSize() const { - return internalMessage->getMaximumMessageSize(); -} - MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { return internalMessage; } -size_t CommandMessageBase::getMinimumMessageSize() const { - return MINIMUM_COMMAND_MESSAGE_BASE_SIZE; -} +//size_t CommandMessageBase::getMinimumMessageSize() const { +// return MINIMUM_COMMAND_MESSAGE_BASE_SIZE; +//} void CommandMessageBase::setReplyRejected(ReturnValue_t reason, Command_t initialCommand) { diff --git a/ipc/CommandMessageBase.h b/ipc/CommandMessageBase.h index b237b68f..1ee1b970 100644 --- a/ipc/CommandMessageBase.h +++ b/ipc/CommandMessageBase.h @@ -63,15 +63,8 @@ public: virtual MessageQueueId_t getSender() const override; virtual uint8_t * getData() override; virtual const uint8_t* getData() const override; - virtual void setMessageSize(size_t messageSize) override; virtual size_t getMessageSize() const override; - //! This depends on the maximum message size of the passed internal message. - virtual size_t getMaximumMessageSize() const override; - - //! Return the constant minimum message size. - virtual size_t getMinimumMessageSize() const override; - /** * A command message can be rejected and needs to offer a function * to set a rejected reply diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index b173f2c4..f3665074 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -51,9 +51,9 @@ void MessageQueueMessage::setSender(MessageQueueId_t setId) { memcpy(this->internalBuffer, &setId, sizeof(MessageQueueId_t)); } -size_t MessageQueueMessage::getMinimumMessageSize() const { - return this->HEADER_SIZE; -} +//size_t MessageQueueMessage::getMinimumMessageSize() const { +// return this->HEADER_SIZE; +//} void MessageQueueMessage::print() { sif::debug << "MessageQueueMessage has size: " << this->messageSize << @@ -69,13 +69,9 @@ void MessageQueueMessage::clear() { } size_t MessageQueueMessage::getMessageSize() const { - return this->messageSize; -} - -size_t MessageQueueMessage::getMaximumMessageSize() const { return this->MAX_MESSAGE_SIZE; } -void MessageQueueMessage::setMessageSize(size_t messageSize) { - this->messageSize = messageSize; -} +//void MessageQueueMessage::setMessageSize(size_t messageSize) { +// this->messageSize = messageSize; +//} diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 8716df54..a994c323 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -139,20 +139,8 @@ public: * The message queue id that identifies the sending message queue. */ void setSender(MessageQueueId_t setId) override; - /** - * @brief This helper function is used by the MessageQueue class to check - * the size of an incoming message. - * @details - * The method must be overwritten by child classes if size - * checks shall be more strict. - * @return The default implementation returns HEADER_SIZE. - */ - virtual size_t getMinimumMessageSize() const override; virtual size_t getMessageSize() const override; - virtual void setMessageSize(size_t messageSize) override; - - virtual size_t getMaximumMessageSize() const override; /** * @brief This is a debug method that prints the content diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index f34cab98..19c69bfb 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -68,23 +68,11 @@ public: virtual uint8_t* getData() = 0; /** - * @brief This helper function is used by the MessageQueue class to - * check the size of an incoming message. - */ - virtual size_t getMinimumMessageSize() const = 0; - - /** - * Get message size of current message implementation. + * Get constant message size of current message implementation. * @return */ virtual size_t getMessageSize() const = 0; - virtual void setMessageSize(size_t messageSize) = 0; - /** - * Get maximum allowed size of current message implementation. - * @return - */ - virtual size_t getMaximumMessageSize() const = 0; }; diff --git a/ipc/MessageQueueSenderIF.h b/ipc/MessageQueueSenderIF.h index 8bc14948..e4b9670e 100644 --- a/ipc/MessageQueueSenderIF.h +++ b/ipc/MessageQueueSenderIF.h @@ -10,17 +10,16 @@ public: virtual ~MessageQueueSenderIF() {} /** - * Allows sending messages without actually "owing" a message queue. + * Allows sending messages without actually "owning" a message queue. * Not sure whether this is actually a good idea. */ static ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxMessageSize, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = MessageQueueMessageIF::NO_QUEUE, - bool ignoreFault=false); + bool ignoreFault = false); private: MessageQueueSenderIF() {} }; - #endif /* FRAMEWORK_IPC_MESSAGEQUEUESENDERIF_H_ */ diff --git a/modes/ModeHelper.cpp b/modes/ModeHelper.cpp index f636c0bf..59a6d974 100644 --- a/modes/ModeHelper.cpp +++ b/modes/ModeHelper.cpp @@ -28,7 +28,6 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { if (result != HasReturnvaluesIF::RETURN_OK) { ModeMessage::cantReachMode(&reply, result); MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()); break; } @@ -51,7 +50,6 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_REPLY, mode, submode); MessageQueueSenderIF::sendMessage(command->getSender(), &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()); } break; @@ -97,7 +95,7 @@ void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, ownerMode, ownerSubmode); } MessageQueueSenderIF::sendMessage(theOneWhoCommandedAMode, &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()); + owner->getCommandQueue()); } } @@ -111,7 +109,6 @@ void ModeHelper::sendModeInfoMessage(Mode_t ownerMode, ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_INFO, ownerMode, ownerSubmode); MessageQueueSenderIF::sendMessage(parentQueueId, &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, owner->getCommandQueue()); } } diff --git a/monitoring/LimitViolationReporter.cpp b/monitoring/LimitViolationReporter.cpp index c6e2b5db..1378d754 100644 --- a/monitoring/LimitViolationReporter.cpp +++ b/monitoring/LimitViolationReporter.cpp @@ -34,8 +34,7 @@ ReturnValue_t LimitViolationReporter::sendLimitViolationReport(const SerializeIF MessageQueueMessage message; CommandMessage report(&message); MonitoringMessage::setLimitViolationReport(&report, storeId); - return MessageQueueSenderIF::sendMessage(reportQueue, &report, - MessageQueueMessage::MAX_MESSAGE_SIZE); + return MessageQueueSenderIF::sendMessage(reportQueue, &report); } ReturnValue_t LimitViolationReporter::checkClassLoaded() { diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index c9adf229..464a4f16 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -50,8 +50,8 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { - return sendMessageFromMessageQueue(sendTo, message, maxMessageSize, sentFrom, - ignoreFault, callContext); + return sendMessageFromMessageQueue(sendTo, message, sentFrom, ignoreFault, + callContext); } @@ -80,9 +80,9 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, } ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { - if(message->getMaximumMessageSize() < maxMessageSize) { + if(message->getMessageSize() < maxMessageSize) { sif::error << "MessageQueue::receiveMessage: Message size " - << message->getMaximumMessageSize() << + << message->getMessageSize() << " too small to receive data!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } @@ -127,17 +127,10 @@ bool MessageQueue::isDefaultDestinationSet() const { // static core function to send messages. ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxSize, - MessageQueueId_t sentFrom, bool ignoreFault, CallContext callContext) { + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, + bool ignoreFault, CallContext callContext) { message->setSender(sentFrom); BaseType_t result; - if(message->getMaximumMessageSize() > maxSize) { - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size " - << message->getMaximumMessageSize() << " too large for queue" - " with max. message size " << maxSize << "!" - << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } if(callContext == CallContext::TASK) { result = xQueueSendToBack(reinterpret_cast(sendTo), diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index e87e884f..b7e52bb3 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -194,9 +194,8 @@ protected: * @param context Specify whether call is made from task or from an ISR. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxSize, - MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault=false, - CallContext callContext = CallContext::TASK); + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, + bool ignoreFault=false, CallContext callContext = CallContext::TASK); static ReturnValue_t handleSendResult(BaseType_t result, bool ignoreFault); diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index d754b2b9..beb4969b 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -7,14 +7,9 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxSize, - MessageQueueId_t sentFrom, bool ignoreFault) { - if(maxSize == 0) { - sif::error << "MessageQueueSenderIF::sendMessage: Max Size is 0!" - << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - return MessageQueue::sendMessageFromMessageQueue(sendTo,message, maxSize, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, + bool ignoreFault) { + return MessageQueue::sendMessageFromMessageQueue(sendTo,message, sentFrom,ignoreFault); } diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 68ff5ab0..8b91020f 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -107,8 +107,7 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, ParameterMessage::setParameterDumpReply(&reply, id, address); - MessageQueueSenderIF::sendMessage(to, &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, ownerQueueId); + MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); return HasReturnvaluesIF::RETURN_OK; } @@ -130,6 +129,5 @@ void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, MessageQueueMessage message; CommandMessage reply(&message); reply.setReplyRejected(reason, initialCommand); - MessageQueueSenderIF::sendMessage(to, &reply, - MessageQueueMessage::MAX_MESSAGE_SIZE, ownerQueueId); + MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); } diff --git a/tmtcpacket/pus/TmPacketStored.cpp b/tmtcpacket/pus/TmPacketStored.cpp index d589db8d..c0ff874b 100644 --- a/tmtcpacket/pus/TmPacketStored.cpp +++ b/tmtcpacket/pus/TmPacketStored.cpp @@ -118,7 +118,7 @@ ReturnValue_t TmPacketStored::sendPacket(MessageQueueId_t destination, } TmTcMessage tmMessage(getStoreAddress()); ReturnValue_t result = MessageQueueSenderIF::sendMessage(destination, - &tmMessage, MessageQueueMessage::MAX_MESSAGE_SIZE, sentFrom); + &tmMessage, sentFrom); if (result != HasReturnvaluesIF::RETURN_OK) { deletePacket(); if (doErrorReporting) { diff --git a/tmtcservices/VerificationReporter.cpp b/tmtcservices/VerificationReporter.cpp index a54c1f8d..bcb4756c 100644 --- a/tmtcservices/VerificationReporter.cpp +++ b/tmtcservices/VerificationReporter.cpp @@ -23,7 +23,7 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, current_packet->getPacketId(), current_packet->getPacketSequenceControl(), 0, set_step); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, - &message, MessageQueueMessage::MAX_MESSAGE_SIZE); + &message); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendSuccessReport: Error writing " "to queue. Code: " << std::hex << (uint16_t) status << std::endl; @@ -39,7 +39,7 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, PusVerificationMessage message(set_report_id, ackFlags, tcPacketId, tcSequenceControl, 0, set_step); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, - &message, MessageQueueMessage::MAX_MESSAGE_SIZE); + &message); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendSuccessReport: Error writing " "to queue. Code: " << std::hex << (uint16_t) status << std::endl; @@ -58,7 +58,7 @@ void VerificationReporter::sendFailureReport(uint8_t report_id, current_packet->getPacketSequenceControl(), error_code, step, parameter1, parameter2); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, - &message, MessageQueueMessage::MAX_MESSAGE_SIZE); + &message); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendFailureReport Error writing to queue. Code: " @@ -76,7 +76,7 @@ void VerificationReporter::sendFailureReport(uint8_t report_id, PusVerificationMessage message(report_id, ackFlags, tcPacketId, tcSequenceControl, error_code, step, parameter1, parameter2); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, - &message, MessageQueueMessage::MAX_MESSAGE_SIZE); + &message); if (status != HasReturnvaluesIF::RETURN_OK) { sif::error << "VerificationReporter::sendFailureReport Error writing to queue. Code: " From 3bf29a7315d926dc1447e954494f393061ee2c8a Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 00:24:15 +0200 Subject: [PATCH 28/31] removed CommandMessageBase, changed interfaces --- action/ActionHelper.cpp | 15 +-- action/CommandActionHelper.cpp | 3 +- action/SimpleActionHelper.cpp | 3 +- controller/ControllerBase.cpp | 3 +- datapool/HkSwitchHelper.cpp | 3 +- datapoolglob/DataPoolAdmin.cpp | 9 +- datapoollocal/LocalDataPoolManager.cpp | 33 +++-- datapoollocal/LocalDataPoolManager.h | 2 +- devicehandlers/AssemblyBase.cpp | 3 +- devicehandlers/DeviceHandlerBase.cpp | 18 +-- devicehandlers/HealthDevice.cpp | 3 +- health/HealthHelper.cpp | 6 +- housekeeping/HousekeepingMessage.cpp | 22 ++-- housekeeping/HousekeepingMessage.h | 7 +- ipc/CommandMessage.cpp | 77 ++++++++---- ipc/CommandMessage.h | 73 ++++++++--- ipc/CommandMessageBase.cpp | 168 ++++++++++++------------- ipc/CommandMessageBase.h | 67 +++------- ipc/CommandMessageIF.h | 34 +++-- ipc/MessageQueueMessage.cpp | 41 +++--- ipc/MessageQueueMessage.h | 16 ++- ipc/MessageQueueMessageIF.h | 11 +- memory/MemoryHelper.cpp | 15 +-- modes/ModeHelper.cpp | 9 +- monitoring/LimitViolationReporter.cpp | 3 +- parameters/ParameterHelper.cpp | 6 +- power/Fuse.cpp | 3 +- power/PowerSensor.cpp | 3 +- subsystem/Subsystem.cpp | 14 +-- subsystem/SubsystemBase.cpp | 14 +-- thermal/AbstractTemperatureSensor.cpp | 5 +- thermal/Heater.cpp | 3 +- tmtcservices/CommandingServiceBase.cpp | 28 ++--- tmtcservices/CommandingServiceBase.h | 10 +- 34 files changed, 348 insertions(+), 382 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 0df5f2d4..57b64686 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -33,15 +33,13 @@ ReturnValue_t ActionHelper::initialize(MessageQueueIF* queueToUse_) { } void ActionHelper::step(uint8_t step, MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ActionMessage::setStepReply(&reply, commandId, step + STEP_OFFSET, result); queueToUse->sendMessage(reportTo, &reply); } void ActionHelper::finish(MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ActionMessage::setCompletionReply(&reply, commandId, result); queueToUse->sendMessage(reportTo, &reply); } @@ -56,8 +54,7 @@ void ActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t act size_t size = 0; ReturnValue_t result = ipcStore->getData(dataAddress, &dataPtr, &size); if (result != HasReturnvaluesIF::RETURN_OK) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ActionMessage::setStepReply(&reply, actionId, 0, result); queueToUse->sendMessage(commandedBy, &reply); return; @@ -65,8 +62,7 @@ void ActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t act result = owner->executeAction(actionId, commandedBy, dataPtr, size); ipcStore->deleteData(dataAddress); if (result != HasReturnvaluesIF::RETURN_OK) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ActionMessage::setStepReply(&reply, actionId, 0, result); queueToUse->sendMessage(commandedBy, &reply); return; @@ -75,8 +71,7 @@ void ActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t act ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, ActionId_t replyId, SerializeIF* data, bool hideSender) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; store_address_t storeAddress; uint8_t *dataPtr; size_t maxSize = data->getSerializedSize(); diff --git a/action/CommandActionHelper.cpp b/action/CommandActionHelper.cpp index c871a2d0..77295401 100644 --- a/action/CommandActionHelper.cpp +++ b/action/CommandActionHelper.cpp @@ -53,8 +53,7 @@ ReturnValue_t CommandActionHelper::commandAction(object_id_t commandTo, ReturnValue_t CommandActionHelper::sendCommand(MessageQueueId_t queueId, ActionId_t actionId, store_address_t storeId) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ActionMessage::setCommand(&command, actionId, storeId); ReturnValue_t result = queueToUse->sendMessage(queueId, &command); if (result != HasReturnvaluesIF::RETURN_OK) { diff --git a/action/SimpleActionHelper.cpp b/action/SimpleActionHelper.cpp index f97474b1..6861fc28 100644 --- a/action/SimpleActionHelper.cpp +++ b/action/SimpleActionHelper.cpp @@ -36,8 +36,7 @@ void SimpleActionHelper::resetHelper() { void SimpleActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t actionId, store_address_t dataAddress) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; if (isExecuting) { ipcStore->deleteData(dataAddress); ActionMessage::setStepReply(&reply, actionId, 0, diff --git a/controller/ControllerBase.cpp b/controller/ControllerBase.cpp index ad0a0b97..674d0fc8 100644 --- a/controller/ControllerBase.cpp +++ b/controller/ControllerBase.cpp @@ -56,8 +56,7 @@ MessageQueueId_t ControllerBase::getCommandQueue() const { } void ControllerBase::handleQueue() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result; for (result = commandQueue->receiveMessage(&command); result == RETURN_OK; result = commandQueue->receiveMessage(&command)) { diff --git a/datapool/HkSwitchHelper.cpp b/datapool/HkSwitchHelper.cpp index 6a923776..cf5114f7 100644 --- a/datapool/HkSwitchHelper.cpp +++ b/datapool/HkSwitchHelper.cpp @@ -21,8 +21,7 @@ ReturnValue_t HkSwitchHelper::initialize() { } ReturnValue_t HkSwitchHelper::performOperation(uint8_t operationCode) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; while (actionQueue->receiveMessage(&command) == HasReturnvaluesIF::RETURN_OK) { ReturnValue_t result = commandActionHelper.handleReply(&command); if (result == HasReturnvaluesIF::RETURN_OK) { diff --git a/datapoolglob/DataPoolAdmin.cpp b/datapoolglob/DataPoolAdmin.cpp index 6387263f..05de1eb2 100644 --- a/datapoolglob/DataPoolAdmin.cpp +++ b/datapoolglob/DataPoolAdmin.cpp @@ -66,8 +66,7 @@ ReturnValue_t DataPoolAdmin::getParameter(uint8_t domainId, } void DataPoolAdmin::handleCommand() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != RETURN_OK) { return; @@ -283,8 +282,7 @@ ReturnValue_t DataPoolAdmin::sendParameter(MessageQueueId_t to, uint32_t id, return result; } - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ParameterMessage::setParameterDumpReply(&reply, id, address); @@ -296,8 +294,7 @@ ReturnValue_t DataPoolAdmin::sendParameter(MessageQueueId_t to, uint32_t id, //identical to ParameterHelper::rejectCommand() void DataPoolAdmin::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, Command_t initialCommand) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; reply.setReplyRejected(reason, initialCommand); commandQueue->sendMessage(to, &reply); } diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index e0731a96..bc57468a 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -45,8 +45,8 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { } ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( - HousekeepingMessage& message) { - Command_t command = message.getCommand(); + CommandMessage* message) { + Command_t command = message->getCommand(); switch(command) { // I think those are the only commands which can be handled here.. case(HousekeepingMessage::ADD_HK_REPORT_STRUCT): @@ -55,10 +55,10 @@ ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( return HasReturnvaluesIF::RETURN_OK; case(HousekeepingMessage::REPORT_DIAGNOSTICS_REPORT_STRUCTURES): case(HousekeepingMessage::REPORT_HK_REPORT_STRUCTURES): - return generateSetStructurePacket(message.getSid()); + //return generateSetStructurePacket(message->getSid()); case(HousekeepingMessage::GENERATE_ONE_PARAMETER_REPORT): case(HousekeepingMessage::GENERATE_ONE_DIAGNOSTICS_REPORT): - return generateHousekeepingPacket(message.getSid()); + //return generateHousekeepingPacket(message->getSid()); default: return CommandMessageIF::UNKNOWN_COMMAND; } @@ -105,19 +105,18 @@ ReturnValue_t LocalDataPoolManager::generateHousekeepingPacket(sid_t sid) { } // and now we set a HK message and send it the HK packet destination. - MessageQueueMessage message; - HousekeepingMessage hkMessage(&message); - hkMessage.setHkReportMessage(sid, storeId); - if(hkQueue == nullptr) { - return QUEUE_NOT_SET; - } - - if(currentHkPacketDestination != MessageQueueIF::NO_QUEUE) { - result = hkQueue->sendMessage(currentHkPacketDestination, &hkMessage); - } - else { - result = hkQueue->sendToDefault(&hkMessage); - } + //HousekeepingMessage hkMessage; +// hkMessage.setHkReportMessage(sid, storeId); +// if(hkQueue == nullptr) { +// return QUEUE_NOT_SET; +// } +// +// if(currentHkPacketDestination != MessageQueueIF::NO_QUEUE) { +// result = hkQueue->sendMessage(currentHkPacketDestination, &hkMessage); +// } +// else { +// result = hkQueue->sendToDefault(&hkMessage); +// } return result; } diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index b6491724..9d072bf4 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -58,7 +58,7 @@ public: ReturnValue_t generateHousekeepingPacket(sid_t sid); ReturnValue_t generateSetStructurePacket(sid_t sid); - ReturnValue_t handleHousekeepingMessage(HousekeepingMessage& message); + ReturnValue_t handleHousekeepingMessage(CommandMessage* message); /** * This function is used to fill the local data pool map with pool diff --git a/devicehandlers/AssemblyBase.cpp b/devicehandlers/AssemblyBase.cpp index a5dcad21..bd85d1de 100644 --- a/devicehandlers/AssemblyBase.cpp +++ b/devicehandlers/AssemblyBase.cpp @@ -148,8 +148,7 @@ void AssemblyBase::handleModeTransitionFailed(ReturnValue_t result) { void AssemblyBase::sendHealthCommand(MessageQueueId_t sendTo, HealthState health) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; HealthMessage::setHealthMessage(&command, HealthMessage::HEALTH_SET, health); if (commandQueue->sendMessage(sendTo, &command) == RETURN_OK) { diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 1ae056a6..0442be89 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -214,12 +214,7 @@ void DeviceHandlerBase::readCommandQueue() { return; } - // This is not ideal. What if it is not a command message? (e.g. another - // message with 3 parameters). The full buffer is filled anyway - // and I could just copy the content into the other message but - // all I need are few additional functions the other message type offers. - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != RETURN_OK) { return; @@ -247,8 +242,7 @@ void DeviceHandlerBase::readCommandQueue() { return; } - HousekeepingMessage hkMessage(&message); - result = hkManager.handleHousekeepingMessage(hkMessage); + result = hkManager.handleHousekeepingMessage(&command); if (result == RETURN_OK) { return; } @@ -484,13 +478,12 @@ void DeviceHandlerBase::setMode(Mode_t newMode) { void DeviceHandlerBase::replyReturnvalueToCommand(ReturnValue_t status, uint32_t parameter) { - MessageQueueMessage message; //This is actually the reply protocol for raw and misc DH commands. if (status == RETURN_OK) { - CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, parameter); + CommandMessage reply(CommandMessage::REPLY_COMMAND_OK, 0, parameter); commandQueue->reply(&reply); } else { - CommandMessage reply(&message, CommandMessage::REPLY_REJECTED, status, parameter); + CommandMessage reply(CommandMessage::REPLY_REJECTED, status, parameter); commandQueue->reply(&reply); } } @@ -760,8 +753,7 @@ void DeviceHandlerBase::replyRawData(const uint8_t *data, size_t len, return; } - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; DeviceHandlerMessage::setDeviceHandlerRawReplyMessage(&command, getObjectId(), address, isCommand); diff --git a/devicehandlers/HealthDevice.cpp b/devicehandlers/HealthDevice.cpp index 7e3a59bd..0bce34ec 100644 --- a/devicehandlers/HealthDevice.cpp +++ b/devicehandlers/HealthDevice.cpp @@ -13,8 +13,7 @@ HealthDevice::~HealthDevice() { } ReturnValue_t HealthDevice::performOperation(uint8_t opCode) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { healthHelper.handleHealthCommand(&command); diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index 0b3bffcb..e01e6fbb 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -65,8 +65,7 @@ void HealthHelper::informParent(HasHealthIF::HealthState health, if (parentQueue == MessageQueueMessageIF::NO_QUEUE) { return; } - MessageQueueMessage message; - CommandMessage information(&message); + CommandMessage information; HealthMessage::setHealthMessage(&information, HealthMessage::HEALTH_INFO, health, oldHealth); if (MessageQueueSenderIF::sendMessage(parentQueue, &information, @@ -81,8 +80,7 @@ void HealthHelper::handleSetHealthCommand(CommandMessage* command) { if (command->getSender() == MessageQueueMessageIF::NO_QUEUE) { return; } - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; if (result == HasReturnvaluesIF::RETURN_OK) { HealthMessage::setHealthMessage(&reply, HealthMessage::REPLY_HEALTH_SET); diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index f425d16e..0d0f9860 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -1,15 +1,7 @@ #include #include -HousekeepingMessage::HousekeepingMessage(MessageQueueMessageIF* message): - CommandMessageBase(message) { - if(message->getMessageSize() < HK_MESSAGE_SIZE) { - sif::error << "HousekeepingMessage::HousekeepingMessage: Passed " - "message buffer can not hold minimum " << HK_MESSAGE_SIZE - << " bytes!" << std::endl; - return; - } - //message->setMessageSize(HK_MESSAGE_SIZE); +HousekeepingMessage::HousekeepingMessage(): CommandMessage() { } HousekeepingMessage::~HousekeepingMessage() {} @@ -26,14 +18,14 @@ uint32_t HousekeepingMessage::getParameter() const { void HousekeepingMessage::setHkReportMessage(sid_t sid, store_address_t storeId) { - CommandMessageBase::setCommand(HK_REPORT); + CommandMessage::setCommand(HK_REPORT); setSid(sid); setParameter(storeId.raw); } void HousekeepingMessage::setHkDiagnosticsMessage(sid_t sid, store_address_t storeId) { - CommandMessageBase::setCommand(DIAGNOSTICS_REPORT); + CommandMessage::setCommand(DIAGNOSTICS_REPORT); setSid(sid); setParameter(storeId.raw); } @@ -44,7 +36,7 @@ void HousekeepingMessage::setHkDiagnosticsMessage(sid_t sid, sid_t HousekeepingMessage::getSid() const { sid_t sid; - std::memcpy(&sid.raw, CommandMessageBase::getData(), sizeof(sid.raw)); + std::memcpy(&sid.raw, CommandMessage::getData(), sizeof(sid.raw)); return sid; } @@ -58,16 +50,16 @@ sid_t HousekeepingMessage::getHkReportMessage( } void HousekeepingMessage::setSid(sid_t sid) { - std::memcpy(CommandMessageBase::getData(), &sid.raw, sizeof(sid.raw)); + std::memcpy(CommandMessage::getData(), &sid.raw, sizeof(sid.raw)); } uint8_t* HousekeepingMessage::getData() { - return CommandMessageBase::getData() + sizeof(sid_t); + return CommandMessage::getData() + sizeof(sid_t); } const uint8_t* HousekeepingMessage::getData() const { - return CommandMessageBase::getData() + sizeof(sid_t); + return CommandMessage::getData() + sizeof(sid_t); } void HousekeepingMessage::clear() { diff --git a/housekeeping/HousekeepingMessage.h b/housekeeping/HousekeepingMessage.h index f568a418..8958201e 100644 --- a/housekeeping/HousekeepingMessage.h +++ b/housekeeping/HousekeepingMessage.h @@ -1,6 +1,7 @@ #ifndef FRAMEWORK_HK_HOUSEKEEPINGMESSAGE_H_ #define FRAMEWORK_HK_HOUSEKEEPINGMESSAGE_H_ -#include + +#include #include #include #include @@ -33,7 +34,7 @@ union sid_t { * This message is slightly larger than regular command messages to accomodate * the uint64_t structure ID (SID). */ -class HousekeepingMessage : public CommandMessageBase { +class HousekeepingMessage : public CommandMessage { public: static constexpr size_t HK_MESSAGE_SIZE = CommandMessageIF::HEADER_SIZE + @@ -44,7 +45,7 @@ public: * the message data, see CommandMessageIF and getInternalMessage(). * @param message */ - HousekeepingMessage(MessageQueueMessageIF* message); + HousekeepingMessage(); virtual ~HousekeepingMessage(); static constexpr uint8_t MESSAGE_ID = messagetypes::HOUSEKEEPING; diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 11355e38..6aa433df 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -1,57 +1,64 @@ #include +#include #include -CommandMessage::CommandMessage(MessageQueueMessageIF* receiverMessage): - CommandMessageBase(receiverMessage) { - if(receiverMessage == nullptr) { - sif::error << "CommandMessage::CommandMessage: Don't pass a nullptr" - " as the message queue message, pass the address of an actual" - " message!" << std::endl; - return; - } -// if(receiverMessage->getMessageSize() < -// getMinimumMessageSize()) { -// sif::error << "CommandMessage::ComandMessage: Passed message buffer" -// " can not hold minimum "<< getMinimumMessageSize() -// << " bytes!" << std::endl; -// return; -// } -// internalMessage->setMessageSize(getMinimumMessageSize()); +CommandMessage::CommandMessage() { + MessageQueueMessage::setMessageSize(DEFAULT_COMMAND_MESSAGE_SIZE); + setCommand(CMD_NONE); } -CommandMessage::CommandMessage(MessageQueueMessageIF* messageToSet, - Command_t command, uint32_t parameter1, uint32_t parameter2): - CommandMessage(messageToSet) { +CommandMessage::CommandMessage(Command_t command, uint32_t parameter1, + uint32_t parameter2) { + MessageQueueMessage::setMessageSize(DEFAULT_COMMAND_MESSAGE_SIZE); setCommand(command); setParameter(parameter1); setParameter2(parameter2); } +Command_t CommandMessage::getCommand() const { + Command_t command; + std::memcpy(&command, getData(), sizeof(Command_t)); + return command; +} + +void CommandMessage::setCommand(Command_t command) { + std::memcpy(getData(), &command, sizeof(Command_t)); +} + +uint8_t CommandMessage::getMessageType() const { + // first byte of command ID. + return getCommand() >> 8 & 0xff; +} + uint32_t CommandMessage::getParameter() const { uint32_t parameter1; - std::memcpy(¶meter1, CommandMessageBase::getData(), sizeof(parameter1)); + std::memcpy(¶meter1, MessageQueueMessage::getData(), sizeof(parameter1)); return parameter1; } void CommandMessage::setParameter(uint32_t parameter1) { - std::memcpy(CommandMessageBase::getData(), ¶meter1, sizeof(parameter1)); + std::memcpy(MessageQueueMessage::getData(), ¶meter1, sizeof(parameter1)); } uint32_t CommandMessage::getParameter2() const { uint32_t parameter2; - std::memcpy(¶meter2, CommandMessageBase::getData() + sizeof(uint32_t), + std::memcpy(¶meter2, MessageQueueMessage::getData() + sizeof(uint32_t), sizeof(parameter2)); return parameter2; } void CommandMessage::setParameter2(uint32_t parameter2) { - std::memcpy(CommandMessageBase::getData() + sizeof(uint32_t), ¶meter2, + std::memcpy(MessageQueueMessage::getData() + sizeof(uint32_t), ¶meter2, sizeof(parameter2)); } -//size_t CommandMessage::getMinimumMessageSize() const { -// return MINIMUM_COMMAND_MESSAGE_SIZE; -//} +size_t CommandMessage::getMinimumMessageSize() const { + return MINIMUM_COMMAND_MESSAGE_SIZE; +} + +void CommandMessage::clear() { + CommandMessageCleaner::clearCommandMessage(this); +} bool CommandMessage::isClearedCommandMessage() { return getCommand() == CMD_NONE; @@ -62,3 +69,21 @@ void CommandMessage::setToUnknownCommand() { this->clear(); setReplyRejected(UNKNOWN_COMMAND, initialCommand); } + +void CommandMessage::setReplyRejected(ReturnValue_t reason, + Command_t initialCommand) { + std::memcpy(getData(), &reason, sizeof(reason)); + std::memcpy(getData() + sizeof(reason), &initialCommand, + sizeof(initialCommand)); +} + +ReturnValue_t CommandMessage::getReplyRejectedReason( + Command_t *initialCommand) const { + ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; + std::memcpy(&reason, getData(), sizeof(reason)); + if(initialCommand != nullptr) { + std::memcpy(initialCommand, getData() + sizeof(reason), + sizeof(Command_t)); + } + return reason; +} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index ae4016b0..a319a0cf 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -1,7 +1,7 @@ #ifndef FRAMEWORK_IPC_COMMANDMESSAGE_H_ #define FRAMEWORK_IPC_COMMANDMESSAGE_H_ -#include +#include #include #include @@ -20,23 +20,21 @@ * currently has an internal message size of 28 bytes. * @author Bastian Baetz */ -class CommandMessage: public CommandMessageBase { +class CommandMessage: public MessageQueueMessage, public CommandMessageIF { public: - /** - * This is the size of a message as it is seen by the MessageQueue. - * It can hold the CommandMessage header plus 2 4-byte parameters. - * 14 of the 24 available MessageQueueMessage bytes are used. - */ - static const size_t MINIMUM_COMMAND_MESSAGE_SIZE = - CommandMessageIF::HEADER_SIZE + 2 * sizeof(uint32_t); + /** + * Default size can accomodate 2 4-byte parameters. + */ + static constexpr size_t DEFAULT_COMMAND_MESSAGE_SIZE = + CommandMessageIF::MINIMUM_COMMAND_MESSAGE_SIZE + sizeof(uint32_t); /** - * Default Constructor, does not initialize anything. - * + * @brief Default Constructor, does not initialize anything. + * @details * This constructor should be used when receiving a Message, as the * content is filled by the MessageQueue. */ - CommandMessage(MessageQueueMessageIF* receiverMessage); + CommandMessage(); /** * This constructor creates a new message with all message content * initialized @@ -45,17 +43,13 @@ public: * @param parameter1 The first parameter * @param parameter2 The second parameter */ - CommandMessage(MessageQueueMessageIF* messageToSet, Command_t command, - uint32_t parameter1, uint32_t parameter2); + CommandMessage(Command_t command, uint32_t parameter1, uint32_t parameter2); /** * @brief Default Destructor */ virtual ~CommandMessage() {} - /** MessageQueueMessageIF functions used for minimum size check. */ - //size_t getMinimumMessageSize() const override; - /** * Get the first parameter of the message * @return the first Parameter of the message @@ -91,8 +85,49 @@ public: * Sets the command to REPLY_REJECTED with parameter UNKNOWN_COMMAND. * Is needed quite often, so we better code it once only. */ - void setToUnknownCommand(); + void setToUnknownCommand() override; + + /** + * A command message can be rejected and needs to offer a function + * to set a rejected reply + * @param reason + * @param initialCommand + */ + void setReplyRejected(ReturnValue_t reason, + Command_t initialCommand) override; + /** + * Corrensonding getter function. + * @param initialCommand + * @return + */ + ReturnValue_t getReplyRejectedReason( + Command_t* initialCommand = nullptr) const override; + + virtual void clear() override; + + /** + * Read the DeviceHandlerCommand_t that is stored in the message, + * usually used after receiving. + * + * @return the Command stored in the Message + */ + virtual Command_t getCommand() const override; + /** + * Set the command type of the message. Default implementation also + * sets the message type, which will be the first byte of the command ID. + * @param the Command to be sent + */ + virtual void setCommand(Command_t command); + + /** + * Extract message ID, which is the first byte of the command ID for the + * default implementation. + * @return + */ + virtual uint8_t getMessageType() const override; + + /** MessageQueueMessageIF functions used for minimum size check. */ + size_t getMinimumMessageSize() const override; }; - #endif /* COMMANDMESSAGE_H_ */ diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp index ea50f4ea..745e6c5f 100644 --- a/ipc/CommandMessageBase.cpp +++ b/ipc/CommandMessageBase.cpp @@ -1,86 +1,86 @@ -#include -#include -#include - -CommandMessageBase::CommandMessageBase(MessageQueueMessageIF *message): - internalMessage(message) { -} - -Command_t CommandMessageBase::getCommand() const { - Command_t command; - std::memcpy(&command, internalMessage->getData(), sizeof(Command_t)); - return command; -} - -void CommandMessageBase::setCommand(Command_t command) { - std::memcpy(internalMessage->getData(), &command, sizeof(Command_t)); -} - -uint8_t CommandMessageBase::getMessageType() const { - // first byte of command ID. - return getCommand() >> 8 & 0xff; -} - -MessageQueueId_t CommandMessageBase::getSender() const { - return internalMessage->getSender(); -} - -uint8_t* CommandMessageBase::getBuffer() { - return internalMessage->getBuffer(); -} - -void CommandMessageBase::setSender(MessageQueueId_t setId) { - internalMessage->setSender(setId); -} - -const uint8_t* CommandMessageBase::getBuffer() const { - return internalMessage->getBuffer(); -} - -// Header includes command ID. -uint8_t* CommandMessageBase::getData() { - return internalMessage->getData() + sizeof(Command_t); -} - -// Header includes command ID. -const uint8_t* CommandMessageBase::getData() const { - return internalMessage->getData() + sizeof(Command_t); -} - -//void CommandMessageBase::setMessageSize(size_t messageSize) { -// //internalMessage->setMessageSize(messageSize); +//#include +//#include +//#include +// +//CommandMessageBase::CommandMessageBase(MessageQueueMessageIF *message): +// internalMessage(message) { //} - -size_t CommandMessageBase::getMessageSize() const { - return internalMessage->getMessageSize(); -} - -MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { - return internalMessage; -} - -//size_t CommandMessageBase::getMinimumMessageSize() const { -// return MINIMUM_COMMAND_MESSAGE_BASE_SIZE; +// +//Command_t CommandMessageBase::getCommand() const { +// Command_t command; +// std::memcpy(&command, internalMessage->getData(), sizeof(Command_t)); +// return command; +//} +// +//void CommandMessageBase::setCommand(Command_t command) { +// std::memcpy(internalMessage->getData(), &command, sizeof(Command_t)); +//} +// +//uint8_t CommandMessageBase::getMessageType() const { +// // first byte of command ID. +// return getCommand() >> 8 & 0xff; +//} +// +//MessageQueueId_t CommandMessageBase::getSender() const { +// return internalMessage->getSender(); +//} +// +//uint8_t* CommandMessageBase::getBuffer() { +// return internalMessage->getBuffer(); +//} +// +//void CommandMessageBase::setSender(MessageQueueId_t setId) { +// internalMessage->setSender(setId); +//} +// +//const uint8_t* CommandMessageBase::getBuffer() const { +// return internalMessage->getBuffer(); +//} +// +//// Header includes command ID. +//uint8_t* CommandMessageBase::getData() { +// return internalMessage->getData() + sizeof(Command_t); +//} +// +//// Header includes command ID. +//const uint8_t* CommandMessageBase::getData() const { +// return internalMessage->getData() + sizeof(Command_t); +//} +// +////void CommandMessageBase::setMessageSize(size_t messageSize) { +//// //internalMessage->setMessageSize(messageSize); +////} +// +//size_t CommandMessageBase::getMessageSize() const { +// return internalMessage->getMessageSize(); +//} +// +//MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { +// return internalMessage; +//} +// +////size_t CommandMessageBase::getMinimumMessageSize() const { +//// return MINIMUM_COMMAND_MESSAGE_BASE_SIZE; +////} +// +//void CommandMessageBase::setReplyRejected(ReturnValue_t reason, +// Command_t initialCommand) { +// std::memcpy(getData(), &reason, sizeof(reason)); +// std::memcpy(getData() + sizeof(reason), &initialCommand, +// sizeof(initialCommand)); +//} +// +//ReturnValue_t CommandMessageBase::getReplyRejectedReason( +// Command_t *initialCommand) const { +// ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; +// std::memcpy(&reason, getData(), sizeof(reason)); +// if(initialCommand != nullptr) { +// std::memcpy(initialCommand, getData() + sizeof(reason), +// sizeof(Command_t)); +// } +// return reason; +//} +// +//void CommandMessageBase::clear() { +// CommandMessageCleaner::clearCommandMessage(this); //} - -void CommandMessageBase::setReplyRejected(ReturnValue_t reason, - Command_t initialCommand) { - std::memcpy(getData(), &reason, sizeof(reason)); - std::memcpy(getData() + sizeof(reason), &initialCommand, - sizeof(initialCommand)); -} - -ReturnValue_t CommandMessageBase::getReplyRejectedReason( - Command_t *initialCommand) const { - ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; - std::memcpy(&reason, getData(), sizeof(reason)); - if(initialCommand != nullptr) { - std::memcpy(initialCommand, getData() + sizeof(reason), - sizeof(Command_t)); - } - return reason; -} - -void CommandMessageBase::clear() { - CommandMessageCleaner::clearCommandMessage(this); -} diff --git a/ipc/CommandMessageBase.h b/ipc/CommandMessageBase.h index 1ee1b970..59795114 100644 --- a/ipc/CommandMessageBase.h +++ b/ipc/CommandMessageBase.h @@ -23,67 +23,28 @@ */ class CommandMessageBase: public CommandMessageIF { public: - //! This minimum size is derived from the interface requirement to be able - //! to set a rejected reply, which contains a returnvalue and the initial - //! command. - static constexpr size_t MINIMUM_COMMAND_MESSAGE_BASE_SIZE = - CommandMessageIF::HEADER_SIZE + sizeof(ReturnValue_t) + - sizeof(Command_t); + CommandMessageBase(MessageQueueMessageIF* message); - /** - * Read the DeviceHandlerCommand_t that is stored in the message, - * usually used after receiving. - * - * @return the Command stored in the Message - */ - virtual Command_t getCommand() const override; - /** - * Set the command type of the message. Default implementation also - * sets the message type, which will be the first byte of the command ID. - * @param the Command to be sent - */ - virtual void setCommand(Command_t command); - /** - * Extract message ID, which is the first byte of the command ID for the - * default implementation. - * @return - */ - virtual uint8_t getMessageType() const override; - /* - * MessageQueueMessageIF functions, which generally just call the - * respective functions of the internal message queue message. - */ - virtual uint8_t * getBuffer() override; - virtual const uint8_t * getBuffer() const override; - virtual void setSender(MessageQueueId_t setId) override; - virtual MessageQueueId_t getSender() const override; - virtual uint8_t * getData() override; - virtual const uint8_t* getData() const override; - virtual size_t getMessageSize() const override; +// /* +// * MessageQueueMessageIF functions, which generally just call the +// * respective functions of the internal message queue message. +// */ +// virtual uint8_t * getBuffer() override; +// virtual const uint8_t * getBuffer() const override; +// virtual void setSender(MessageQueueId_t setId) override; +// virtual MessageQueueId_t getSender() const override; +// virtual uint8_t * getData() override; +// virtual const uint8_t* getData() const override; +// virtual size_t getMessageSize() const override; - /** - * A command message can be rejected and needs to offer a function - * to set a rejected reply - * @param reason - * @param initialCommand - */ - void setReplyRejected(ReturnValue_t reason, - Command_t initialCommand) override; - /** - * Corrensonding getter function. - * @param initialCommand - * @return - */ - ReturnValue_t getReplyRejectedReason( - Command_t* initialCommand = nullptr) const override; - virtual MessageQueueMessageIF* getInternalMessage() const override; + //virtual MessageQueueMessageIF* getInternalMessage() const override; + - virtual void clear() override; protected: /** * @brief Pointer to the message containing the data. diff --git a/ipc/CommandMessageIF.h b/ipc/CommandMessageIF.h index fbfbddbb..ca050c32 100644 --- a/ipc/CommandMessageIF.h +++ b/ipc/CommandMessageIF.h @@ -5,20 +5,24 @@ #include #include - #define MAKE_COMMAND_ID( number ) ((MESSAGE_ID << 8) + (number)) typedef uint16_t Command_t; -// TODO: actually, this interface propably does not have to implement -// MQM IF, because there is a getter function for the internal message.. -// But it is also convenient to have the full access to all MQM IF functions. -// That way, I can just pass CommandMessages to functions expecting a MQM IF. -// The command message implementations just forwards the calls. Maybe -// we should just leave it like that. -class CommandMessageIF: public MessageQueueMessageIF { +class CommandMessageIF: virtual public MessageQueueMessageIF { public: - static constexpr size_t HEADER_SIZE = sizeof(MessageQueueId_t) + + /** + * Header consists of sender ID and command ID. + */ + static constexpr size_t HEADER_SIZE = MessageQueueMessageIF::HEADER_SIZE + sizeof(Command_t); + /** + * This minimum size is derived from the interface requirement to be able + * to set a rejected reply, which contains a returnvalue and the initial + * command. + */ + static constexpr size_t MINIMUM_COMMAND_MESSAGE_SIZE = + CommandMessageIF::HEADER_SIZE + sizeof(ReturnValue_t) + + sizeof(Command_t); static const uint8_t INTERFACE_ID = CLASS_ID::COMMAND_MESSAGE; static const ReturnValue_t UNKNOWN_COMMAND = MAKE_RETURN_CODE(0x01); @@ -60,15 +64,9 @@ public: virtual ReturnValue_t getReplyRejectedReason( Command_t* initialCommand = nullptr) const = 0; - /** - * This function is used to get a pointer to the internal message, as - * the command message implementations always operate on the memory - * contained in the message queue message implementation. - * This pointer can be used to set the internal message of different - * command message implementations. - * @return - */ - virtual MessageQueueMessageIF* getInternalMessage() const = 0; + virtual void setToUnknownCommand() = 0; + + virtual void clear() = 0; }; diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index f3665074..0f4d861b 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -1,10 +1,10 @@ #include #include - -#include +#include +#include MessageQueueMessage::MessageQueueMessage() : - messageSize(this->HEADER_SIZE) { + messageSize(getMinimumMessageSize()) { memset(this->internalBuffer, 0, sizeof(this->internalBuffer)); } @@ -51,17 +51,15 @@ void MessageQueueMessage::setSender(MessageQueueId_t setId) { memcpy(this->internalBuffer, &setId, sizeof(MessageQueueId_t)); } -//size_t MessageQueueMessage::getMinimumMessageSize() const { -// return this->HEADER_SIZE; -//} - -void MessageQueueMessage::print() { - sif::debug << "MessageQueueMessage has size: " << this->messageSize << - std::hex << std::endl; - for (uint8_t count = 0; count < this->messageSize; count++) { - sif::debug << (uint32_t) this->internalBuffer[count] << ":"; +void MessageQueueMessage::print(bool printWholeMessage) { + sif::debug << "MessageQueueMessage content: " << std::endl; + if(printWholeMessage) { + arrayprinter::print(getData(), getMaximumMessageSize()); } - sif::debug << std::dec << std::endl; + else { + arrayprinter::print(getData(), getMessageSize()); + } + } void MessageQueueMessage::clear() { @@ -69,9 +67,18 @@ void MessageQueueMessage::clear() { } size_t MessageQueueMessage::getMessageSize() const { - return this->MAX_MESSAGE_SIZE; + return this->messageSize; +} + +void MessageQueueMessage::setMessageSize(size_t messageSize) { + this->messageSize = messageSize; +} + +size_t MessageQueueMessage::getMinimumMessageSize() const { + return this->MIN_MESSAGE_SIZE; +} + +size_t MessageQueueMessage::getMaximumMessageSize() const { + return this->MAX_MESSAGE_SIZE; } -//void MessageQueueMessage::setMessageSize(size_t messageSize) { -// this->messageSize = messageSize; -//} diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index a994c323..c4e82a94 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -23,7 +23,7 @@ * receive messages from other tasks. * @ingroup message_queue */ -class MessageQueueMessage: public MessageQueueMessageIF { +class MessageQueueMessage: virtual public MessageQueueMessageIF { public: /** * @brief The class is initialized empty with this constructor. @@ -74,11 +74,7 @@ public: * as small as possible. */ static const size_t MAX_DATA_SIZE = 24; - /** - * @brief This constants defines the size of the header, - * which is added to every message. - */ - static const size_t HEADER_SIZE = sizeof(MessageQueueId_t); + /** * @brief This constant defines the maximum total size in bytes * of a sent message. @@ -141,12 +137,14 @@ public: void setSender(MessageQueueId_t setId) override; virtual size_t getMessageSize() const override; + virtual void setMessageSize(size_t messageSize) override; + virtual size_t getMinimumMessageSize() const override; + virtual size_t getMaximumMessageSize() const override; /** - * @brief This is a debug method that prints the content - * (till messageSize) to the debug output. + * @brief This is a debug method that prints the content. */ - void print(); + void print(bool printWholeMessage); }; #endif /* MESSAGEQUEUEMESSAGE_H_ */ diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 19c69bfb..c6c677c1 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -18,7 +18,12 @@ typedef uint32_t MessageQueueId_t; class MessageQueueMessageIF { public: - static const MessageQueueId_t NO_QUEUE = 0xffffffff; + static const MessageQueueId_t NO_QUEUE = -1; + /** + * @brief This constants defines the size of the header, + * which is added to every message. + */ + static const size_t HEADER_SIZE = sizeof(MessageQueueId_t); virtual ~MessageQueueMessageIF() {}; @@ -73,6 +78,10 @@ public: */ virtual size_t getMessageSize() const = 0; + virtual void setMessageSize(size_t messageSize) = 0; + virtual size_t getMinimumMessageSize() const = 0; + virtual size_t getMaximumMessageSize() const = 0; + }; diff --git a/memory/MemoryHelper.cpp b/memory/MemoryHelper.cpp index fb4d9eb6..ea7ef9e1 100644 --- a/memory/MemoryHelper.cpp +++ b/memory/MemoryHelper.cpp @@ -51,16 +51,14 @@ void MemoryHelper::completeLoad(ReturnValue_t errorCode, break; default: ipcStore->deleteData(ipcAddress); - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; MemoryMessage::setMemoryReplyFailed(&reply, errorCode, MemoryMessage::CMD_MEMORY_LOAD); queueToUse->sendMessage(lastSender, &reply); return; } //Only reached on success - MessageQueueMessage message; - CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, 0); + CommandMessage reply( CommandMessage::REPLY_COMMAND_OK, 0, 0); queueToUse->sendMessage(lastSender, &reply); ipcStore->deleteData(ipcAddress); } @@ -68,8 +66,7 @@ void MemoryHelper::completeLoad(ReturnValue_t errorCode, void MemoryHelper::completeDump(ReturnValue_t errorCode, const uint8_t* dataToCopy, const uint32_t size) { busy = false; - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; MemoryMessage::setMemoryReplyFailed(&reply, errorCode, lastCommand); switch (errorCode) { case HasMemoryIF::DO_IT_MYSELF: @@ -154,8 +151,7 @@ void MemoryHelper::handleMemoryLoad(CommandMessage* message) { completeLoad(returnCode, p_data, size, dataPointer); } else { //At least inform sender. - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; MemoryMessage::setMemoryReplyFailed(&reply, returnCode, MemoryMessage::CMD_MEMORY_LOAD); queueToUse->sendMessage(lastSender, &reply); @@ -173,8 +169,7 @@ void MemoryHelper::handleMemoryCheckOrDump(CommandMessage* message) { reservedSpaceInIPC); completeDump(returnCode, dataPointer, size); } else { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; MemoryMessage::setMemoryReplyFailed(&reply, returnCode, lastCommand); queueToUse->sendMessage(lastSender, &reply); } diff --git a/modes/ModeHelper.cpp b/modes/ModeHelper.cpp index 59a6d974..030f1db3 100644 --- a/modes/ModeHelper.cpp +++ b/modes/ModeHelper.cpp @@ -12,8 +12,7 @@ ModeHelper::~ModeHelper() { } ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; Mode_t mode; Submode_t submode; switch (command->getCommand()) { @@ -79,8 +78,7 @@ void ModeHelper::modeChanged(Mode_t ownerMode, Submode_t ownerSubmode) { void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, Submode_t ownerSubmode) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; if (theOneWhoCommandedAMode != MessageQueueMessageIF::NO_QUEUE) { if (ownerMode != commandedMode or ownerSubmode != commandedSubmode) @@ -101,8 +99,7 @@ void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, void ModeHelper::sendModeInfoMessage(Mode_t ownerMode, Submode_t ownerSubmode) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; if (theOneWhoCommandedAMode != parentQueueId and parentQueueId != MessageQueueMessageIF::NO_QUEUE) { diff --git a/monitoring/LimitViolationReporter.cpp b/monitoring/LimitViolationReporter.cpp index 1378d754..234a0bff 100644 --- a/monitoring/LimitViolationReporter.cpp +++ b/monitoring/LimitViolationReporter.cpp @@ -31,8 +31,7 @@ ReturnValue_t LimitViolationReporter::sendLimitViolationReport(const SerializeIF if (result != HasReturnvaluesIF::RETURN_OK) { return result; } - MessageQueueMessage message; - CommandMessage report(&message); + CommandMessage report; MonitoringMessage::setLimitViolationReport(&report, storeId); return MessageQueueSenderIF::sendMessage(reportQueue, &report); } diff --git a/parameters/ParameterHelper.cpp b/parameters/ParameterHelper.cpp index 8b91020f..20e7e8ec 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -102,8 +102,7 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, return result; } - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ParameterMessage::setParameterDumpReply(&reply, id, address); @@ -126,8 +125,7 @@ ReturnValue_t ParameterHelper::initialize() { void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, Command_t initialCommand) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; reply.setReplyRejected(reason, initialCommand); MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); } diff --git a/power/Fuse.cpp b/power/Fuse.cpp index a80a111f..eee91984 100644 --- a/power/Fuse.cpp +++ b/power/Fuse.cpp @@ -163,8 +163,7 @@ void Fuse::setAllMonitorsToUnchecked() { } void Fuse::checkCommandQueue() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != HasReturnvaluesIF::RETURN_OK) { return; diff --git a/power/PowerSensor.cpp b/power/PowerSensor.cpp index b1aea270..5433acc9 100644 --- a/power/PowerSensor.cpp +++ b/power/PowerSensor.cpp @@ -74,8 +74,7 @@ void PowerSensor::setAllMonitorsToUnchecked() { } void PowerSensor::checkCommandQueue() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != HasReturnvaluesIF::RETURN_OK) { return; diff --git a/subsystem/Subsystem.cpp b/subsystem/Subsystem.cpp index d774b4d5..52f2c474 100644 --- a/subsystem/Subsystem.cpp +++ b/subsystem/Subsystem.cpp @@ -309,8 +309,7 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { break; case ModeSequenceMessage::READ_FREE_SEQUENCE_SLOTS: { uint32_t freeSlots = modeSequences.maxSize() - modeSequences.size(); - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ModeSequenceMessage::setModeSequenceMessage(&reply, ModeSequenceMessage::FREE_SEQUENCE_SLOTS, freeSlots); commandQueue->reply(&reply); @@ -318,8 +317,7 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { break; case ModeSequenceMessage::READ_FREE_TABLE_SLOTS: { uint32_t free = modeTables.maxSize() - modeTables.size(); - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ModeSequenceMessage::setModeSequenceMessage(&reply, ModeSequenceMessage::FREE_TABLE_SLOTS, free); commandQueue->reply(&reply); @@ -332,12 +330,11 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { } void Subsystem::replyToCommand(ReturnValue_t status, uint32_t parameter) { - MessageQueueMessage message; if (status == RETURN_OK) { - CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, 0); + CommandMessage reply(CommandMessage::REPLY_COMMAND_OK, 0, 0); commandQueue->reply(&reply); } else { - CommandMessage reply(&message, CommandMessage::REPLY_REJECTED, status, 0); + CommandMessage reply(CommandMessage::REPLY_REJECTED, status, 0); commandQueue->reply(&reply); } } @@ -620,8 +617,7 @@ void Subsystem::sendSerializablesAsCommandMessage(Command_t command, for (uint8_t i = 0; i < count; i++) { elements[i]->serialize(&storeBuffer, &size, maxSize, true); } - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ModeSequenceMessage::setModeSequenceMessage(&reply, command, address); if (commandQueue->reply(&reply) != RETURN_OK) { IPCStore->deleteData(address); diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index 5527f215..737accef 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -77,8 +77,7 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable( } void SubsystemBase::executeTable(HybridIterator tableIter, Submode_t targetSubmode) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; std::map::iterator iter; @@ -129,7 +128,7 @@ void SubsystemBase::executeTable(HybridIterator tableIter, Submod continue; //don't send redundant mode commands (produces event spam), but still command if mode is forced to reach lower levels } ReturnValue_t result = commandQueue->sendMessage( - iter->second.commandQueue, &message); + iter->second.commandQueue, &command); if (result == RETURN_OK) { ++commandsOutstanding; } @@ -297,8 +296,7 @@ void SubsystemBase::setToExternalControl() { void SubsystemBase::announceMode(bool recursive) { triggerEvent(MODE_INFO, mode, submode); if (recursive) { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_ANNOUNCE_RECURSIVELY, 0, 0); commandAllChildren(&command); @@ -307,8 +305,7 @@ void SubsystemBase::announceMode(bool recursive) { void SubsystemBase::checkCommandQueue() { ReturnValue_t result; - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; for (result = commandQueue->receiveMessage(&command); result == RETURN_OK; result = commandQueue->receiveMessage(&command)) { @@ -330,8 +327,7 @@ void SubsystemBase::checkCommandQueue() { result = handleCommandMessage(&command); if (result != RETURN_OK) { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; reply.setReplyRejected(CommandMessage::UNKNOWN_COMMAND, command.getCommand()); replyToCommand(&reply); diff --git a/thermal/AbstractTemperatureSensor.cpp b/thermal/AbstractTemperatureSensor.cpp index 759af87e..41230f59 100644 --- a/thermal/AbstractTemperatureSensor.cpp +++ b/thermal/AbstractTemperatureSensor.cpp @@ -44,8 +44,7 @@ ReturnValue_t AbstractTemperatureSensor::performHealthOp() { } void AbstractTemperatureSensor::handleCommandQueue() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { result = healthHelper.handleHealthCommand(&command); @@ -57,7 +56,7 @@ void AbstractTemperatureSensor::handleCommandQueue() { return; } command.setToUnknownCommand(); - commandQueue->reply(&message); + commandQueue->reply(&command); } } diff --git a/thermal/Heater.cpp b/thermal/Heater.cpp index 6e191fc1..b1cf38f2 100644 --- a/thermal/Heater.cpp +++ b/thermal/Heater.cpp @@ -279,8 +279,7 @@ ReturnValue_t Heater::initialize() { } void Heater::handleQueue() { - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { result = healthHelper.handleHealthCommand(&command); diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 4d29c10e..bd6fa5b5 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -76,28 +76,18 @@ ReturnValue_t CommandingServiceBase::initialize() { } void CommandingServiceBase::handleCommandQueue() { - MessageQueueMessage message; - CommandMessage reply(&message); + CommandMessage reply; ReturnValue_t result = RETURN_FAILED; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; result = commandQueue->receiveMessage(&reply)) { - if(reply.getInternalMessage() == nullptr) { - // This should never happen unless the passed message maximum size - // is too small! - sif::error << "CommandingServiceBase::handleCommandMessage: Reply" - "does not satisfy minimum requirements for a command " - "message!" << std::endl; - continue; - } handleCommandMessage(&reply); } } -void CommandingServiceBase::handleCommandMessage(CommandMessageIF* reply) { +void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { bool isStep = false; - MessageQueueMessage message; - CommandMessage nextCommand(&message); + CommandMessage nextCommand; CommandMapIter iter = commandMap.find(reply->getSender()); // handle unrequested reply first @@ -303,8 +293,7 @@ ReturnValue_t CommandingServiceBase::sendTmPacket(uint8_t subservice, void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, CommandMapIter iter) { ReturnValue_t result = RETURN_OK; - MessageQueueMessage message; - CommandMessage command(&message); + CommandMessage command; iter->subservice = storedPacket->getSubService(); result = prepareCommand(&command, iter->subservice, storedPacket->getApplicationData(), @@ -316,7 +305,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, case RETURN_OK: if (command.getCommand() != CommandMessage::CMD_NONE) { sendResult = commandQueue->sendMessage(iter.value->first, - &message); + &command); } if (sendResult == RETURN_OK) { Clock::getUptime(&iter->uptimeOfStart); @@ -338,7 +327,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, if (command.getCommand() != CommandMessage::CMD_NONE) { //Fire-and-forget command. sendResult = commandQueue->sendMessage(iter.value->first, - &message); + &command); } if (sendResult == RETURN_OK) { verificationReporter.sendSuccessReport(TC_VERIFY::START_SUCCESS, @@ -384,9 +373,8 @@ void CommandingServiceBase::checkAndExecuteFifo(CommandMapIter iter) { } -void CommandingServiceBase::handleUnrequestedReply(CommandMessageIF* reply) { - CommandMessage commandReply(reply->getInternalMessage()); - commandReply.clear(); +void CommandingServiceBase::handleUnrequestedReply(CommandMessage* reply) { + reply->clear(); } diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 05f50709..54a0dc7a 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -139,7 +139,7 @@ protected: * @param objectId Target object ID * @return */ - virtual ReturnValue_t prepareCommand(CommandMessageIF *message, + virtual ReturnValue_t prepareCommand(CommandMessage* message, uint8_t subservice, const uint8_t *tcData, size_t tcDataLen, uint32_t *state, object_id_t objectId) = 0; @@ -168,9 +168,9 @@ protected: * a failure verification message with the reason as the error parameter * and the initial command as failure parameter 1. */ - virtual ReturnValue_t handleReply(const CommandMessageIF *reply, + virtual ReturnValue_t handleReply(const CommandMessage* reply, Command_t previousCommand, uint32_t *state, - CommandMessageIF *optionalNextCommand, object_id_t objectId, + CommandMessage* optionalNextCommand, object_id_t objectId, bool *isStep) = 0; /** @@ -182,7 +182,7 @@ protected: * Reply which is non-const so the default implementation can clear the * message. */ - virtual void handleUnrequestedReply(CommandMessageIF *reply); + virtual void handleUnrequestedReply(CommandMessage* reply); virtual void doPeriodicOperation(); @@ -310,7 +310,7 @@ private: void startExecution(TcPacketStored *storedPacket, CommandMapIter iter); - void handleCommandMessage(CommandMessageIF* reply); + void handleCommandMessage(CommandMessage* reply); void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, CommandMessageIF* nextCommand,CommandMessageIF* reply, bool& isStep); From c7c49b4239fe42f4c4da47eba686c41ff687cf17 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 00:30:32 +0200 Subject: [PATCH 29/31] deleted command messge base --- ipc/CommandMessageBase.cpp | 86 -------------------------- ipc/CommandMessageBase.h | 60 ------------------ ipc/CommandMessageIF.h | 2 +- ipc/MessageQueueMessage.h | 2 +- tmtcservices/CommandingServiceBase.cpp | 4 +- tmtcservices/CommandingServiceBase.h | 2 +- 6 files changed, 5 insertions(+), 151 deletions(-) delete mode 100644 ipc/CommandMessageBase.cpp delete mode 100644 ipc/CommandMessageBase.h diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp deleted file mode 100644 index 745e6c5f..00000000 --- a/ipc/CommandMessageBase.cpp +++ /dev/null @@ -1,86 +0,0 @@ -//#include -//#include -//#include -// -//CommandMessageBase::CommandMessageBase(MessageQueueMessageIF *message): -// internalMessage(message) { -//} -// -//Command_t CommandMessageBase::getCommand() const { -// Command_t command; -// std::memcpy(&command, internalMessage->getData(), sizeof(Command_t)); -// return command; -//} -// -//void CommandMessageBase::setCommand(Command_t command) { -// std::memcpy(internalMessage->getData(), &command, sizeof(Command_t)); -//} -// -//uint8_t CommandMessageBase::getMessageType() const { -// // first byte of command ID. -// return getCommand() >> 8 & 0xff; -//} -// -//MessageQueueId_t CommandMessageBase::getSender() const { -// return internalMessage->getSender(); -//} -// -//uint8_t* CommandMessageBase::getBuffer() { -// return internalMessage->getBuffer(); -//} -// -//void CommandMessageBase::setSender(MessageQueueId_t setId) { -// internalMessage->setSender(setId); -//} -// -//const uint8_t* CommandMessageBase::getBuffer() const { -// return internalMessage->getBuffer(); -//} -// -//// Header includes command ID. -//uint8_t* CommandMessageBase::getData() { -// return internalMessage->getData() + sizeof(Command_t); -//} -// -//// Header includes command ID. -//const uint8_t* CommandMessageBase::getData() const { -// return internalMessage->getData() + sizeof(Command_t); -//} -// -////void CommandMessageBase::setMessageSize(size_t messageSize) { -//// //internalMessage->setMessageSize(messageSize); -////} -// -//size_t CommandMessageBase::getMessageSize() const { -// return internalMessage->getMessageSize(); -//} -// -//MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { -// return internalMessage; -//} -// -////size_t CommandMessageBase::getMinimumMessageSize() const { -//// return MINIMUM_COMMAND_MESSAGE_BASE_SIZE; -////} -// -//void CommandMessageBase::setReplyRejected(ReturnValue_t reason, -// Command_t initialCommand) { -// std::memcpy(getData(), &reason, sizeof(reason)); -// std::memcpy(getData() + sizeof(reason), &initialCommand, -// sizeof(initialCommand)); -//} -// -//ReturnValue_t CommandMessageBase::getReplyRejectedReason( -// Command_t *initialCommand) const { -// ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; -// std::memcpy(&reason, getData(), sizeof(reason)); -// if(initialCommand != nullptr) { -// std::memcpy(initialCommand, getData() + sizeof(reason), -// sizeof(Command_t)); -// } -// return reason; -//} -// -//void CommandMessageBase::clear() { -// CommandMessageCleaner::clearCommandMessage(this); -//} diff --git a/ipc/CommandMessageBase.h b/ipc/CommandMessageBase.h deleted file mode 100644 index 59795114..00000000 --- a/ipc/CommandMessageBase.h +++ /dev/null @@ -1,60 +0,0 @@ -#ifndef FRAMEWORK_IPC_COMMANDMESSAGEBASE_H_ -#define FRAMEWORK_IPC_COMMANDMESSAGEBASE_H_ -#include -#include - -/** - * @brief Base implementation of a generic command message, which has - * a Command_t ID and message type ID in the header in addition - * to the sender message queue ID. - * @details - * This is the base implementation serves as a base for other command messages - * and which implements most functions required for MessageQueueMessageIF. - * The only functions which have to be supplied by a specific command message - * impelementations are the size related functions which are used for - * size checks: - * - * 1. getMinimumMessageSize() - * - * The maximum message size generally depends on the buffer size of the passed - * internal message. - * Don't forget to set the message size of the passed message in the concrete - * commandmessage implementation! - */ -class CommandMessageBase: public CommandMessageIF { -public: - - - CommandMessageBase(MessageQueueMessageIF* message); - - - -// /* -// * MessageQueueMessageIF functions, which generally just call the -// * respective functions of the internal message queue message. -// */ -// virtual uint8_t * getBuffer() override; -// virtual const uint8_t * getBuffer() const override; -// virtual void setSender(MessageQueueId_t setId) override; -// virtual MessageQueueId_t getSender() const override; -// virtual uint8_t * getData() override; -// virtual const uint8_t* getData() const override; -// virtual size_t getMessageSize() const override; - - - //virtual MessageQueueMessageIF* getInternalMessage() const override; - - -protected: - /** - * @brief Pointer to the message containing the data. - * @details - * The command message does not actually own the memory containing a - * message, it just oprates on it via a pointer to a message queue message. - */ - MessageQueueMessageIF* internalMessage = nullptr; -}; - - - -#endif /* FRAMEWORK_IPC_COMMANDMESSAGEBASE_H_ */ diff --git a/ipc/CommandMessageIF.h b/ipc/CommandMessageIF.h index ca050c32..2cb6324e 100644 --- a/ipc/CommandMessageIF.h +++ b/ipc/CommandMessageIF.h @@ -8,7 +8,7 @@ #define MAKE_COMMAND_ID( number ) ((MESSAGE_ID << 8) + (number)) typedef uint16_t Command_t; -class CommandMessageIF: virtual public MessageQueueMessageIF { +class CommandMessageIF { public: /** * Header consists of sender ID and command ID. diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index c4e82a94..3d5d4283 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -23,7 +23,7 @@ * receive messages from other tasks. * @ingroup message_queue */ -class MessageQueueMessage: virtual public MessageQueueMessageIF { +class MessageQueueMessage: public MessageQueueMessageIF { public: /** * @brief The class is initialized empty with this constructor. diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index bd6fa5b5..acf66389 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -146,8 +146,8 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { } void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, - CommandMapIter iter, CommandMessageIF* nextCommand, - CommandMessageIF* reply, bool& isStep) { + CommandMapIter iter, CommandMessage* nextCommand, + CommandMessage* reply, bool& isStep) { iter->command = nextCommand->getCommand(); // In case a new command is to be sent immediately, this is performed here. diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 54a0dc7a..8806bd52 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -312,7 +312,7 @@ private: void handleCommandMessage(CommandMessage* reply); void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, - CommandMessageIF* nextCommand,CommandMessageIF* reply, bool& isStep); + CommandMessage* nextCommand, CommandMessage* reply, bool& isStep); void checkTimeout(); }; From af24cc7d047cccf09ff5a002a86aa222b18b19ed Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 00:49:13 +0200 Subject: [PATCH 30/31] some bugfixes --- ipc/CommandMessage.cpp | 82 +++++++++++++++++++--------------- ipc/CommandMessage.h | 30 +++++++------ ipc/CommandMessageCleaner.cpp | 20 ++++----- ipc/CommandMessageCleaner.h | 6 +-- osal/FreeRTOS/MessageQueue.cpp | 6 --- 5 files changed, 74 insertions(+), 70 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 6aa433df..2b0c34d5 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -3,57 +3,57 @@ #include CommandMessage::CommandMessage() { - MessageQueueMessage::setMessageSize(DEFAULT_COMMAND_MESSAGE_SIZE); + MessageQueueMessage::setMessageSize(DEFAULT_COMMAND_MESSAGE_SIZE); setCommand(CMD_NONE); } CommandMessage::CommandMessage(Command_t command, uint32_t parameter1, uint32_t parameter2) { MessageQueueMessage::setMessageSize(DEFAULT_COMMAND_MESSAGE_SIZE); - setCommand(command); - setParameter(parameter1); - setParameter2(parameter2); + setCommand(command); + setParameter(parameter1); + setParameter2(parameter2); } Command_t CommandMessage::getCommand() const { - Command_t command; - std::memcpy(&command, getData(), sizeof(Command_t)); - return command; + Command_t command; + std::memcpy(&command, MessageQueueMessage::getData(), sizeof(Command_t)); + return command; } void CommandMessage::setCommand(Command_t command) { - std::memcpy(getData(), &command, sizeof(Command_t)); + std::memcpy(MessageQueueMessage::getData(), &command, sizeof(Command_t)); } uint8_t CommandMessage::getMessageType() const { - // first byte of command ID. - return getCommand() >> 8 & 0xff; + // first byte of command ID. + return getCommand() >> 8 & 0xff; } uint32_t CommandMessage::getParameter() const { - uint32_t parameter1; - std::memcpy(¶meter1, MessageQueueMessage::getData(), sizeof(parameter1)); - return parameter1; + uint32_t parameter1; + std::memcpy(¶meter1, this->getData(), sizeof(parameter1)); + return parameter1; } void CommandMessage::setParameter(uint32_t parameter1) { - std::memcpy(MessageQueueMessage::getData(), ¶meter1, sizeof(parameter1)); + std::memcpy(this->getData(), ¶meter1, sizeof(parameter1)); } uint32_t CommandMessage::getParameter2() const { - uint32_t parameter2; - std::memcpy(¶meter2, MessageQueueMessage::getData() + sizeof(uint32_t), - sizeof(parameter2)); - return parameter2; + uint32_t parameter2; + std::memcpy(¶meter2, this->getData() + sizeof(uint32_t), + sizeof(parameter2)); + return parameter2; } void CommandMessage::setParameter2(uint32_t parameter2) { - std::memcpy(MessageQueueMessage::getData() + sizeof(uint32_t), ¶meter2, - sizeof(parameter2)); + std::memcpy(this->getData() + sizeof(uint32_t), ¶meter2, + sizeof(parameter2)); } size_t CommandMessage::getMinimumMessageSize() const { - return MINIMUM_COMMAND_MESSAGE_SIZE; + return MINIMUM_COMMAND_MESSAGE_SIZE; } void CommandMessage::clear() { @@ -61,29 +61,37 @@ void CommandMessage::clear() { } bool CommandMessage::isClearedCommandMessage() { - return getCommand() == CMD_NONE; + return getCommand() == CMD_NONE; } void CommandMessage::setToUnknownCommand() { - Command_t initialCommand = getCommand(); - this->clear(); - setReplyRejected(UNKNOWN_COMMAND, initialCommand); + Command_t initialCommand = getCommand(); + this->clear(); + setReplyRejected(UNKNOWN_COMMAND, initialCommand); } void CommandMessage::setReplyRejected(ReturnValue_t reason, - Command_t initialCommand) { - std::memcpy(getData(), &reason, sizeof(reason)); - std::memcpy(getData() + sizeof(reason), &initialCommand, - sizeof(initialCommand)); + Command_t initialCommand) { + std::memcpy(getData(), &reason, sizeof(reason)); + std::memcpy(getData() + sizeof(reason), &initialCommand, + sizeof(initialCommand)); } ReturnValue_t CommandMessage::getReplyRejectedReason( - Command_t *initialCommand) const { - ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; - std::memcpy(&reason, getData(), sizeof(reason)); - if(initialCommand != nullptr) { - std::memcpy(initialCommand, getData() + sizeof(reason), - sizeof(Command_t)); - } - return reason; + Command_t *initialCommand) const { + ReturnValue_t reason = HasReturnvaluesIF::RETURN_FAILED; + std::memcpy(&reason, getData(), sizeof(reason)); + if(initialCommand != nullptr) { + std::memcpy(initialCommand, getData() + sizeof(reason), + sizeof(Command_t)); + } + return reason; +} + +uint8_t* CommandMessage::getData() { + return MessageQueueMessage::getData() + sizeof(Command_t); +} + +const uint8_t* CommandMessage::getData() const { + return MessageQueueMessage::getData() + sizeof(Command_t); } diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index a319a0cf..53df1c08 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -50,6 +50,22 @@ public: */ virtual ~CommandMessage() {} + /** + * Read the DeviceHandlerCommand_t that is stored in the message, + * usually used after receiving. + * + * @return the Command stored in the Message + */ + virtual Command_t getCommand() const override; + /** + * Set the command type of the message. Default implementation also + * sets the message type, which will be the first byte of the command ID. + * @param the Command to be sent + */ + virtual void setCommand(Command_t command); + + virtual uint8_t* getData() override; + virtual const uint8_t* getData() const override; /** * Get the first parameter of the message * @return the first Parameter of the message @@ -105,20 +121,6 @@ public: virtual void clear() override; - /** - * Read the DeviceHandlerCommand_t that is stored in the message, - * usually used after receiving. - * - * @return the Command stored in the Message - */ - virtual Command_t getCommand() const override; - /** - * Set the command type of the message. Default implementation also - * sets the message type, which will be the first byte of the command ID. - * @param the Command to be sent - */ - virtual void setCommand(Command_t command); - /** * Extract message ID, which is the first byte of the command ID for the * default implementation. diff --git a/ipc/CommandMessageCleaner.cpp b/ipc/CommandMessageCleaner.cpp index 7153a8e7..23e6e168 100644 --- a/ipc/CommandMessageCleaner.cpp +++ b/ipc/CommandMessageCleaner.cpp @@ -9,34 +9,34 @@ #include #include -void CommandMessageCleaner::clearCommandMessage(CommandMessageIF* message) { +void CommandMessageCleaner::clearCommandMessage(CommandMessage* message) { switch(message->getMessageType()){ case messagetypes::MODE_COMMAND: - ModeMessage::clear(dynamic_cast(message)); + ModeMessage::clear(message); break; case messagetypes::HEALTH_COMMAND: - HealthMessage::clear(dynamic_cast(message)); + HealthMessage::clear(message); break; case messagetypes::MODE_SEQUENCE: - ModeSequenceMessage::clear(dynamic_cast(message)); + ModeSequenceMessage::clear(message); break; case messagetypes::ACTION: - ActionMessage::clear(dynamic_cast(message)); + ActionMessage::clear(message); break; case messagetypes::DEVICE_HANDLER_COMMAND: - DeviceHandlerMessage::clear(dynamic_cast(message)); + DeviceHandlerMessage::clear(message); break; case messagetypes::MEMORY: - MemoryMessage::clear(dynamic_cast(message)); + MemoryMessage::clear(message); break; case messagetypes::MONITORING: - MonitoringMessage::clear(dynamic_cast(message)); + MonitoringMessage::clear(message); break; case messagetypes::TM_STORE: - TmStoreMessage::clear(dynamic_cast(message)); + TmStoreMessage::clear(message); break; case messagetypes::PARAMETER: - ParameterMessage::clear(dynamic_cast(message)); + ParameterMessage::clear(message); break; default: messagetypes::clearMissionMessage(message); diff --git a/ipc/CommandMessageCleaner.h b/ipc/CommandMessageCleaner.h index 0bd369a2..48aaa478 100644 --- a/ipc/CommandMessageCleaner.h +++ b/ipc/CommandMessageCleaner.h @@ -1,15 +1,15 @@ #ifndef FRAMEWORK_IPC_COMMANDMESSAGECLEANER_H_ #define FRAMEWORK_IPC_COMMANDMESSAGECLEANER_H_ -#include +#include namespace messagetypes { // Implemented in config. -void clearMissionMessage(CommandMessageIF* message); +void clearMissionMessage(CommandMessage* message); } class CommandMessageCleaner { public: - static void clearCommandMessage(CommandMessageIF* message); + static void clearCommandMessage(CommandMessage* message); }; diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index 464a4f16..8eb669ed 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -80,12 +80,6 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, } ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { - if(message->getMessageSize() < maxMessageSize) { - sif::error << "MessageQueue::receiveMessage: Message size " - << message->getMessageSize() << - " too small to receive data!" << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } BaseType_t result = xQueueReceive(handle,reinterpret_cast( message->getBuffer()), 0); if (result == pdPASS){ From 16cbbb2693a18cc588661270c482325d4257e95f Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 24 Jun 2020 01:11:48 +0200 Subject: [PATCH 31/31] linux fixes --- osal/linux/MessageQueue.cpp | 15 +++------------ osal/linux/MessageQueue.h | 3 +-- osal/linux/QueueFactory.cpp | 6 +++--- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index c4b24a13..b11b9b6f 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -281,8 +281,7 @@ ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { - return sendMessageFromMessageQueue(sendTo,message, maxMessageSize, - sentFrom,ignoreFault); + return sendMessageFromMessageQueue(sendTo,message, sentFrom,ignoreFault); } @@ -297,22 +296,14 @@ bool MessageQueue::isDefaultDestinationSet() const { uint16_t MessageQueue::queueCounter = 0; ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF *message, size_t maxSize, - MessageQueueId_t sentFrom, bool ignoreFault) { + MessageQueueMessageIF *message, MessageQueueId_t sentFrom, + bool ignoreFault) { if(message == nullptr) { sif::error << "MessageQueue::sendMessageFromMessageQueue: Message is " "nullptr!" << std::endl; return HasReturnvaluesIF::RETURN_FAILED; } - if(message->getMaximumMessageSize() > maxSize) { - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message size " - << message->getMaximumMessageSize() << " too large for queue" - " with max. message size " << maxSize << "!" - << std::endl; - return HasReturnvaluesIF::RETURN_FAILED; - } - message->setSender(sentFrom); int result = mq_send(sendTo, reinterpret_cast(message->getBuffer()), diff --git a/osal/linux/MessageQueue.h b/osal/linux/MessageQueue.h index 22725d18..8e3a5191 100644 --- a/osal/linux/MessageQueue.h +++ b/osal/linux/MessageQueue.h @@ -149,8 +149,7 @@ protected: * \param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxSize, - MessageQueueId_t sentFrom = NO_QUEUE, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault=false); private: /** diff --git a/osal/linux/QueueFactory.cpp b/osal/linux/QueueFactory.cpp index 95264a42..8bd9d52c 100644 --- a/osal/linux/QueueFactory.cpp +++ b/osal/linux/QueueFactory.cpp @@ -9,10 +9,10 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, size_t maxSize, - MessageQueueId_t sentFrom, bool ignoreFault) { + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, + bool ignoreFault) { return MessageQueue::sendMessageFromMessageQueue(sendTo,message, - maxSize, sentFrom,ignoreFault); + sentFrom,ignoreFault); } QueueFactory* QueueFactory::instance() {