From f578c3ea29731f29153741b1ee99ae95c02782c6 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Jun 2020 16:46:18 +0200 Subject: [PATCH 1/4] set buffer: const buffer is set too --- serialize/SerialBufferAdapter.cpp | 15 ++++++++++----- serialize/SerialBufferAdapter.h | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/serialize/SerialBufferAdapter.cpp b/serialize/SerialBufferAdapter.cpp index d417ba456..5e834fe26 100644 --- a/serialize/SerialBufferAdapter.cpp +++ b/serialize/SerialBufferAdapter.cpp @@ -37,9 +37,13 @@ ReturnValue_t SerialBufferAdapter::serialize(uint8_t** buffer_, } if (constBuffer != nullptr) { memcpy(*buffer_, this->constBuffer, bufferLength); - } else if (buffer != nullptr) { + } + else if (buffer != nullptr) { + // This will propably be never reached, constBuffer should always be + // set if non-const buffer is set. memcpy(*buffer_, this->buffer, bufferLength); - } else { + } + else { return HasReturnvaluesIF::RETURN_FAILED; } *size_ += bufferLength; @@ -93,7 +97,7 @@ template uint8_t * SerialBufferAdapter::getBuffer() { if(buffer == nullptr) { sif::error << "Wrong access function for stored type !" - " Use getConstBuffer()" << std::endl; + " Use getConstBuffer()." << std::endl; return nullptr; } return buffer; @@ -110,9 +114,10 @@ const uint8_t * SerialBufferAdapter::getConstBuffer() { template void SerialBufferAdapter::setBuffer(uint8_t* buffer, - count_t buffer_length) { + count_t bufferLength) { this->buffer = buffer; - bufferLength = buffer_length; + this->constBuffer = buffer; + this->bufferLength = bufferLength; } diff --git a/serialize/SerialBufferAdapter.h b/serialize/SerialBufferAdapter.h index 026900210..a30451a1a 100644 --- a/serialize/SerialBufferAdapter.h +++ b/serialize/SerialBufferAdapter.h @@ -67,7 +67,7 @@ public: uint8_t * getBuffer(); const uint8_t * getConstBuffer(); - void setBuffer(uint8_t* buffer_, count_t bufferLength_); + void setBuffer(uint8_t* buffer, count_t bufferLength); private: bool serializeLength = false; const uint8_t *constBuffer = nullptr; From 7b538e9750001a7b91a89fbde5a47f9b619674eb Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Fri, 12 Jun 2020 20:23:39 +0200 Subject: [PATCH 2/4] introduced command message base and command message IF --- housekeeping/HousekeepingMessage.cpp | 17 +--- housekeeping/HousekeepingMessage.h | 13 ++- ipc/CommandMessage.cpp | 128 +++++++++------------------ ipc/CommandMessage.h | 79 +++++------------ ipc/CommandMessageBase.cpp | 55 ++++++++++++ ipc/CommandMessageBase.h | 73 +++++++++++++++ ipc/CommandMessageIF.h | 25 ++++++ ipc/MessageQueueMessage.cpp | 4 + ipc/MessageQueueMessage.h | 3 +- ipc/MessageQueueMessageIF.h | 4 +- 10 files changed, 232 insertions(+), 169 deletions(-) create mode 100644 ipc/CommandMessageBase.cpp create mode 100644 ipc/CommandMessageBase.h create mode 100644 ipc/CommandMessageIF.h diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index c94a69d74..d9daf9745 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -1,18 +1,7 @@ #include -HousekeepingMessage::HousekeepingMessage() { +HousekeepingMessage::HousekeepingMessage(MessageQueueMessage *message): + CommandMessageBase (message) {} -} +HousekeepingMessage::~HousekeepingMessage() {} -void HousekeepingMessage::setHkReportMessage() { -} - - -//void HousekeepingMessage::setAddHkReportStructMessage(CommandMessage *message, -// set_t setId, store_address_t packet) { -// message->setCommand(ADD_HK_REPORT_STRUCT); -// message->setParameter(setId); -// message->setParameter2(packet.raw); -//} - -//void Housekeeping diff --git a/housekeeping/HousekeepingMessage.h b/housekeeping/HousekeepingMessage.h index 3ba6f94d5..d0cbd0eb5 100644 --- a/housekeeping/HousekeepingMessage.h +++ b/housekeeping/HousekeepingMessage.h @@ -1,6 +1,8 @@ #ifndef FRAMEWORK_HK_HOUSEKEEPINGMESSAGE_H_ #define FRAMEWORK_HK_HOUSEKEEPINGMESSAGE_H_ -#include +#include +#include +#include #include #include @@ -23,15 +25,14 @@ union sid_t { }; -class HousekeepingMessage: public MessageQueueMessage { +class HousekeepingMessage : public CommandMessageBase { public: /** * No instances of a message shall be created, instead * a CommandMessage instance is manipulated. */ - HousekeepingMessage(); -// HousekeepingMessage(const HousekeepingMessage&) = delete; -// HousekeepingMessage operator=(const HousekeepingMessage &) = delete; + HousekeepingMessage(MessageQueueMessage* message); + virtual ~HousekeepingMessage(); static constexpr uint8_t MESSAGE_ID = messagetypes::HOUSEKEEPING; static constexpr Command_t ADD_HK_REPORT_STRUCT = @@ -78,8 +79,6 @@ public: static constexpr Command_t MODIFY_DIAGNOSTICS_REPORT_COLLECTION_INTERVAL = MAKE_COMMAND_ID(32); -// static void setAddHkReportStructMessage(CommandMessage* message, -// DevisetId, store_address_t packet); static void setHkReportMessage(); }; diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 7d2ecc9bd..8f53b77ca 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -1,6 +1,7 @@ +#include + #include #include -#include #include #include #include @@ -8,82 +9,86 @@ #include #include -namespace messagetypes { -// Implemented in config. -void clearMissionMessage(CommandMessage* message); -} - - CommandMessage::CommandMessage(MessageQueueMessage* receiverMessage): - internalMessage(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; } - internalMessage->messageSize = COMMAND_MESSAGE_SIZE; + internalMessage->setMessageSize(COMMAND_MESSAGE_SIZE); setCommand(CMD_NONE); } CommandMessage::CommandMessage(MessageQueueMessage* messageToSet, Command_t command, uint32_t parameter1, uint32_t parameter2): - internalMessage(messageToSet) { + CommandMessageBase(messageToSet) { if(messageToSet == nullptr) { sif::error << "CommandMessage::CommandMessage: Don't pass a nullptr" " as the message queue message, pass the address of an actual" " message!" << std::endl; } - internalMessage->messageSize = COMMAND_MESSAGE_SIZE; + internalMessage->setMessageSize(COMMAND_MESSAGE_SIZE); setCommand(command); setParameter(parameter1); setParameter2(parameter2); } -Command_t CommandMessage::getCommand() const { - Command_t command; - memcpy(&command, internalMessage->getData(), sizeof(Command_t)); - return command; -} - -uint8_t CommandMessage::getMessageType() const { - return getCommand() >> 8 & 0xff; -} - -void CommandMessage::setCommand(Command_t command) { - memcpy(internalMessage->getData(), &command, sizeof(command)); -} - uint32_t CommandMessage::getParameter() const { uint32_t parameter1; - memcpy(¶meter1, internalMessage->getData() + sizeof(Command_t), - sizeof(parameter1)); + memcpy(¶meter1, CommandMessageBase::getData(), sizeof(parameter1)); return parameter1; } void CommandMessage::setParameter(uint32_t parameter1) { - memcpy(internalMessage->getData() + sizeof(Command_t), - ¶meter1, sizeof(parameter1)); + memcpy(CommandMessageBase::getData(), ¶meter1, sizeof(parameter1)); } uint32_t CommandMessage::getParameter2() const { uint32_t parameter2; - memcpy(¶meter2, internalMessage->getData() + sizeof(Command_t) - + sizeof(uint32_t), sizeof(parameter2)); + memcpy(¶meter2, CommandMessageBase::getData() + sizeof(uint32_t), + sizeof(parameter2)); return parameter2; } void CommandMessage::setParameter2(uint32_t parameter2) { - memcpy(internalMessage-> getData() + sizeof(Command_t) + sizeof(uint32_t), - ¶meter2, sizeof(parameter2)); + memcpy(CommandMessageBase::getData() + sizeof(uint32_t), ¶meter2, + sizeof(parameter2)); } +size_t CommandMessage::getMinimumMessageSize() const { + return COMMAND_MESSAGE_SIZE; +} + +size_t CommandMessage::getMaximumMessageSize() const { + return MessageQueueMessage::MAX_MESSAGE_SIZE; +} + +bool CommandMessage::isClearedCommandMessage() { + return getCommand() == CMD_NONE; +} + +void CommandMessage::setToUnknownCommand() { + Command_t initialCommand = getCommand(); + clearCommandMessage(); + setReplyRejected(UNKNOWN_COMMAND, initialCommand); +} + +void CommandMessage::setReplyRejected(ReturnValue_t reason, + Command_t initialCommand) { + setCommand(REPLY_REJECTED); + setParameter(reason); + setParameter2(initialCommand); +} + + void CommandMessage::clear() { clearCommandMessage(); } void CommandMessage::clearCommandMessage() { - switch(getMessageType()){ - case messagetypes::MODE_COMMAND: + switch(this->getMessageType()){ + case messagetypes::MODE_COMMAND: ModeMessage::clear(this); break; case messagetypes::HEALTH_COMMAND: @@ -115,56 +120,3 @@ void CommandMessage::clearCommandMessage() { break; } } - -bool CommandMessage::isClearedCommandMessage() { - return getCommand() == CMD_NONE; -} - -size_t CommandMessage::getMinimumMessageSize() const { - return COMMAND_MESSAGE_SIZE; -} - -void CommandMessage::setToUnknownCommand() { - Command_t initialCommand = getCommand(); - clearCommandMessage(); - setReplyRejected(UNKNOWN_COMMAND, initialCommand); -} - -void CommandMessage::setReplyRejected(ReturnValue_t reason, - Command_t initialCommand) { - setCommand(REPLY_REJECTED); - setParameter(reason); - setParameter2(initialCommand); -} - -MessageQueueId_t CommandMessage::getSender() const { - return internalMessage->getSender(); -} - -uint8_t* CommandMessage::getBuffer() { - return internalMessage->getBuffer(); -} - -void CommandMessage::setSender(MessageQueueId_t setId) { - internalMessage->setSender(setId); -} - -const uint8_t* CommandMessage::getBuffer() const { - return internalMessage->getBuffer(); -} - -uint8_t* CommandMessage::getData() { - return internalMessage->getData(); -} - -const uint8_t* CommandMessage::getData() const { - return internalMessage->getData(); -} - -size_t CommandMessage::getMessageSize() const { - return COMMAND_MESSAGE_SIZE; -} - -size_t CommandMessage::getMaximumMessageSize() const { - return MessageQueueMessage::MAX_MESSAGE_SIZE; -} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index 25d608f04..434f8c8ce 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -1,33 +1,35 @@ #ifndef FRAMEWORK_IPC_COMMANDMESSAGE_H_ #define FRAMEWORK_IPC_COMMANDMESSAGE_H_ +#include #include #include -#include - - -#define MAKE_COMMAND_ID( number ) ((MESSAGE_ID << 8) + (number)) -typedef uint16_t Command_t; +namespace messagetypes { +// Implemented in config. +void clearMissionMessage(CommandMessageIF* message); +} /** - * @brief Used to pass command messages between tasks. Primary message type - * for IPC. Contains sender, 2-byte command field, and 2 4-byte - * parameters. + * @brief Default command message used to pass command messages between tasks. + * Primary message type for IPC. Contains sender, 2-byte command ID + * field, and 2 4-byte parameters. * @details * It operates on an external memory which is contained inside a - * MessageQueueMessage by taking its address. + * class implementing MessageQueueMessageIF by taking its address. * This allows for a more flexible designs of message implementations. * The pointer can be passed to different message implementations without * the need of unnecessary copying. + * + * The command message is based of the generic MessageQueueMessage which + * currently has an internal message size of 28 bytes. * @author Bastian Baetz */ -class CommandMessage: public MessageQueueMessageIF { +class CommandMessage: public CommandMessageBase { public: static const uint8_t INTERFACE_ID = CLASS_ID::COMMAND_MESSAGE; static const ReturnValue_t UNKNOWN_COMMAND = MAKE_RETURN_CODE(0x01); - static const uint8_t MESSAGE_ID = messagetypes::COMMAND; //! Used internally, will be ignored static const Command_t CMD_NONE = MAKE_COMMAND_ID( 0 ); @@ -66,39 +68,11 @@ public: */ virtual ~CommandMessage() {} - /** - * Read the DeviceHandlerCommand_t that is stored in the message, - * usually used after receiving. - * - * @return the Command stored in the Message - */ - Command_t getCommand() const; - - /* - * MessageQueueMessageIF functions, which generally just call the - * respective functions of the internal message - */ - uint8_t * getBuffer() override; - const uint8_t * getBuffer() const override; - void setSender(MessageQueueId_t setId) override; - MessageQueueId_t getSender() const override; - uint8_t * getData() override; - const uint8_t* getData() const override; + /** MessageQueueMessageIF functions used for minimum size check. */ size_t getMinimumMessageSize() const override; - size_t getMessageSize() const override; + /** MessageQueueMessageIF functions used for maximum size check. */ size_t getMaximumMessageSize() const override; - /** - * Extract message ID, which is the first byte of the command ID. - * @return - */ - uint8_t getMessageType() const; - /** - * Set the command type of the message - * @param the Command to be sent - */ - void setCommand(Command_t command); - /** * Get the first parameter of the message * @return the first Parameter of the message @@ -123,19 +97,17 @@ public: */ void setParameter2(uint32_t parameter2); + void clear() override; /** - * Set the command to CMD_NONE and try to find - * the correct class to handle a more detailed - * clear. - * Also, calls a mission-specific clearMissionMessage - * function to separate between framework and mission - * messages. Not optimal, may be replaced by totally - * different auto-delete solution (e.g. smart pointers). + * Set the command to CMD_NONE and try to find the correct class to handle + * a more detailed clear. + * Also, calls a mission-specific clearMissionMessage function to separate + * between framework and mission messages. Not optimal, may be replaced by + * totally different auto-delete solution (e.g. smart pointers). * */ void clearCommandMessage(); - void clear() override; /** * check if a message was cleared @@ -151,15 +123,6 @@ public: void setToUnknownCommand(); void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); - -private: - /** - * @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. - */ - MessageQueueMessage* internalMessage; }; diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp new file mode 100644 index 000000000..aa84bd1ef --- /dev/null +++ b/ipc/CommandMessageBase.cpp @@ -0,0 +1,55 @@ +#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)); +} + +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(); +} diff --git a/ipc/CommandMessageBase.h b/ipc/CommandMessageBase.h new file mode 100644 index 000000000..fa3fd477a --- /dev/null +++ b/ipc/CommandMessageBase.h @@ -0,0 +1,73 @@ +#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() + * 2. getMaximumMessageSize() + * + * 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); + + /** + * 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 void setMessageSize(size_t messageSize) override; + virtual size_t getMessageSize() 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; +}; + + + +#endif /* FRAMEWORK_IPC_COMMANDMESSAGEBASE_H_ */ diff --git a/ipc/CommandMessageIF.h b/ipc/CommandMessageIF.h new file mode 100644 index 000000000..fbc40f88e --- /dev/null +++ b/ipc/CommandMessageIF.h @@ -0,0 +1,25 @@ +#ifndef FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ +#define FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ + +#include + +#define MAKE_COMMAND_ID( number ) ((MESSAGE_ID << 8) + (number)) +typedef uint16_t Command_t; + +class CommandMessageIF: public MessageQueueMessageIF { +public: + virtual ~CommandMessageIF() {}; + + /** + * A command message shall have a uint16_t command ID field. + * @return + */ + virtual Command_t getCommand() const = 0; + /** + * A command message shall have a uint8_t message type ID field. + * @return + */ + virtual uint8_t getMessageType() const = 0; +}; + +#endif /* FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ */ diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index 8430a8ea3..b173f2c41 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -75,3 +75,7 @@ size_t MessageQueueMessage::getMessageSize() const { 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 31a43f663..2eb1f4f56 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -3,7 +3,7 @@ #include #include -#include +#include /** * @brief This class is the representation and data organizer @@ -150,6 +150,7 @@ public: 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; diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 05330ce9e..f34cab98a 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -24,7 +24,8 @@ public: /** * @brief With this method, the whole content and the message - * size is set to zero. + * size is set to zero. Implementations should also take care + * to clear data which is stored indirectly (e.g. storage data). */ virtual void clear() = 0; @@ -77,6 +78,7 @@ public: * @return */ virtual size_t getMessageSize() const = 0; + virtual void setMessageSize(size_t messageSize) = 0; /** * Get maximum allowed size of current message implementation. From 6b67f46c802aa1b421689097d3faf96668073c71 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 13 Jun 2020 17:37:48 +0200 Subject: [PATCH 3/4] evil hidden bug found. CSB uses CommandMessageIF now --- datapoollocal/LocalDataPoolManager.cpp | 4 +- datapoollocal/LocalDataPoolManager.h | 2 +- devicehandlers/DeviceHandlerBase.cpp | 28 ++++-------- devicehandlers/HealthDevice.cpp | 2 +- housekeeping/HousekeepingMessage.cpp | 45 +++++++++++++++++-- housekeeping/HousekeepingMessage.h | 29 +++++++++--- ipc/CommandMessage.cpp | 33 +++++++++++--- ipc/CommandMessage.h | 12 +++-- ipc/CommandMessageBase.cpp | 6 ++- ipc/CommandMessageBase.h | 6 ++- ipc/CommandMessageIF.h | 16 +++++++ ipc/MessageQueueMessage.h | 4 +- osal/FreeRTOS/MessageQueue.cpp | 2 +- osal/FreeRTOS/MessageQueue.h | 22 ++++----- tmtcservices/CommandingServiceBase.cpp | 62 +++++++++++++++++--------- tmtcservices/CommandingServiceBase.h | 23 +++++++--- tmtcservices/VerificationReporter.cpp | 10 ++--- 17 files changed, 214 insertions(+), 92 deletions(-) diff --git a/datapoollocal/LocalDataPoolManager.cpp b/datapoollocal/LocalDataPoolManager.cpp index 37dbed368..58694508f 100644 --- a/datapoollocal/LocalDataPoolManager.cpp +++ b/datapoollocal/LocalDataPoolManager.cpp @@ -41,8 +41,8 @@ ReturnValue_t LocalDataPoolManager::initializeHousekeepingPoolEntriesOnce() { } ReturnValue_t LocalDataPoolManager::handleHousekeepingMessage( - MessageQueueMessage *message) { - return HasReturnvaluesIF::RETURN_OK; + HousekeepingMessage& message) { + return HasReturnvaluesIF::RETURN_FAILED; } ReturnValue_t LocalDataPoolManager::printPoolEntry( diff --git a/datapoollocal/LocalDataPoolManager.h b/datapoollocal/LocalDataPoolManager.h index 1d25fa48c..d1a2d410c 100644 --- a/datapoollocal/LocalDataPoolManager.h +++ b/datapoollocal/LocalDataPoolManager.h @@ -46,7 +46,7 @@ public: LocalDataPoolManager operator=(const LocalDataPoolManager&) = delete; ReturnValue_t generateHousekeepingPacket(sid_t sid); - ReturnValue_t handleHousekeepingMessage(MessageQueueMessage* message); + ReturnValue_t handleHousekeepingMessage(HousekeepingMessage& message); /** * This function is used to fill the local data pool map with pool diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 11cffc74c..bf6998485 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -225,15 +225,6 @@ void DeviceHandlerBase::readCommandQueue() { return; } - // This is really annoying. I can't cast a parent object to a child. - // But I want to use another message format.. -// CommandMessage* cmdMessage = dynamic_cast(msgPtr); -// if(cmdMessage == nullptr) { -// sif::error << "DeviceHandlerBase::readCommandQueue: Could not cast" -// " message to CommandMessage!" << std::endl; -// return; -// } - if(healthHelperActive) { result = healthHelper.handleHealthCommand(&command); if (result == RETURN_OK) { @@ -256,11 +247,11 @@ void DeviceHandlerBase::readCommandQueue() { return; } -// HousekeepingMessage* hkMessage = dynamic_cast(msgPtr); -// result = hkManager.handleHousekeepingMessage(hkMessage); -// if (result == RETURN_OK) { -// return; -// } + HousekeepingMessage hkMessage(&message); + result = hkManager.handleHousekeepingMessage(hkMessage); + if (result == RETURN_OK) { + return; + } result = handleDeviceHandlerMessage(&command); if (result == RETURN_OK) { @@ -758,7 +749,7 @@ ReturnValue_t DeviceHandlerBase::getStorageData(store_address_t storageAddress, void DeviceHandlerBase::replyRawData(const uint8_t *data, size_t len, MessageQueueId_t sendTo, bool isCommand) { - if (IPCStore == NULL || len == 0 || sendTo == MessageQueueIF::NO_QUEUE) { + if (IPCStore == nullptr or len == 0 or sendTo == MessageQueueIF::NO_QUEUE) { return; } store_address_t address; @@ -775,13 +766,12 @@ void DeviceHandlerBase::replyRawData(const uint8_t *data, size_t len, DeviceHandlerMessage::setDeviceHandlerRawReplyMessage(&command, getObjectId(), address, isCommand); -// this->DeviceHandlerCommand = CommandMessage::CMD_NONE; - - result = commandQueue->sendMessage(sendTo, &message); + result = commandQueue->sendMessage(sendTo, &command); if (result != RETURN_OK) { IPCStore->deleteData(address); - //Silently discard data, this indicates heavy TM traffic which should not be increased by additional events. + // Silently discard data, this indicates heavy TM traffic which + // should not be increased by additional events. } } diff --git a/devicehandlers/HealthDevice.cpp b/devicehandlers/HealthDevice.cpp index 1c9d18f64..7e3a59bd0 100644 --- a/devicehandlers/HealthDevice.cpp +++ b/devicehandlers/HealthDevice.cpp @@ -5,7 +5,7 @@ HealthDevice::HealthDevice(object_id_t setObjectId, MessageQueueId_t parentQueue) : SystemObject(setObjectId), lastHealth(HEALTHY), parentQueue( parentQueue), commandQueue(), healthHelper(this, setObjectId) { - commandQueue = QueueFactory::instance()->createMessageQueue(3, CommandMessage::COMMAND_MESSAGE_SIZE); + commandQueue = QueueFactory::instance()->createMessageQueue(3, CommandMessage::MINIMUM_COMMAND_MESSAGE_SIZE); } HealthDevice::~HealthDevice() { diff --git a/housekeeping/HousekeepingMessage.cpp b/housekeeping/HousekeepingMessage.cpp index d9daf9745..6dd1a7116 100644 --- a/housekeeping/HousekeepingMessage.cpp +++ b/housekeeping/HousekeepingMessage.cpp @@ -1,7 +1,46 @@ #include +#include -HousekeepingMessage::HousekeepingMessage(MessageQueueMessage *message): - CommandMessageBase (message) {} +HousekeepingMessage::HousekeepingMessage(MessageQueueMessageIF* message): + CommandMessageBase(message) { +} -HousekeepingMessage::~HousekeepingMessage() {} +HousekeepingMessage::~HousekeepingMessage() { +} +void HousekeepingMessage::setHkReportMessage(sid_t sid, + store_address_t storeId) { + CommandMessageBase::setCommand(HK_REPORT); + setSid(sid); + setParameter(storeId.raw); +} + +size_t HousekeepingMessage::getMinimumMessageSize() const { + return HK_MESSAGE_SIZE; +} + +size_t HousekeepingMessage::getMaximumMessageSize() const { + return MessageQueueMessage::MAX_MESSAGE_SIZE; +} + +void HousekeepingMessage::clear() { + // clear IPC store where it is needed. +} + +sid_t HousekeepingMessage::getSid() const { + sid_t sid; + std::memcpy(&sid.raw, CommandMessageBase::getData(), sizeof(sid.raw)); + return sid; +} + +uint8_t* HousekeepingMessage::getData() { + return internalMessage->getBuffer() + sizeof(sid_t); +} + +void HousekeepingMessage::setParameter(uint32_t parameter) { + memcpy(getData(), ¶meter, sizeof(parameter)); +} + +void HousekeepingMessage::setSid(sid_t sid) { + std::memcpy(CommandMessageBase::getData(), &sid.raw, sizeof(sid.raw)); +} diff --git a/housekeeping/HousekeepingMessage.h b/housekeeping/HousekeepingMessage.h index d0cbd0eb5..4e9d45531 100644 --- a/housekeeping/HousekeepingMessage.h +++ b/housekeeping/HousekeepingMessage.h @@ -13,13 +13,15 @@ union sid_t { struct { object_id_t objectId ; - // A generic 32 bit ID to identify unique HK packets for a single - // object. - // For example, the DeviceCommandId_t is used for DeviceHandlers + /** + * A generic 32 bit ID to identify unique HK packets for a single + * object. For example, the DeviceCommandId_t is used for + * DeviceHandlers + */ uint32_t ownerSetId; }; /** - * Alternative access to the raw value. + * Alternative access to the raw value. This is also the size of the type. */ uint64_t raw; }; @@ -27,14 +29,18 @@ union sid_t { class HousekeepingMessage : public CommandMessageBase { public: + + static constexpr size_t HK_MESSAGE_SIZE = sizeof(MessageQueueId_t) + + sizeof(Command_t) + sizeof(sid_t) * sizeof(uint32_t); /** * No instances of a message shall be created, instead * a CommandMessage instance is manipulated. */ - HousekeepingMessage(MessageQueueMessage* message); + HousekeepingMessage(MessageQueueMessageIF* message); virtual ~HousekeepingMessage(); static constexpr uint8_t MESSAGE_ID = messagetypes::HOUSEKEEPING; + static constexpr Command_t ADD_HK_REPORT_STRUCT = MAKE_COMMAND_ID(1); static constexpr Command_t ADD_DIAGNOSTICS_REPORT_STRUCT = @@ -79,7 +85,18 @@ public: static constexpr Command_t MODIFY_DIAGNOSTICS_REPORT_COLLECTION_INTERVAL = MAKE_COMMAND_ID(32); - static void setHkReportMessage(); + void setHkReportMessage(sid_t sid, store_address_t storeId); + + void setParameter(uint32_t parameter); + + virtual void clear() override; + virtual size_t getMinimumMessageSize() const override; + virtual size_t getMaximumMessageSize() const override; + + virtual uint8_t* getData() override; +private: + sid_t getSid() const; + void setSid(sid_t sid); }; diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 8f53b77ca..9d448aed6 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -9,18 +9,25 @@ #include #include -CommandMessage::CommandMessage(MessageQueueMessage* receiverMessage): +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; } - internalMessage->setMessageSize(COMMAND_MESSAGE_SIZE); - setCommand(CMD_NONE); + if(receiverMessage->getMaximumMessageSize() < + MINIMUM_COMMAND_MESSAGE_SIZE) { + sif::error << "CommandMessage::ComandMessage: Passed message buffer" + " can not hold minimum "<< MINIMUM_COMMAND_MESSAGE_SIZE + << " bytes!" << std::endl; + return; + } + internalMessage->setMessageSize(MINIMUM_COMMAND_MESSAGE_SIZE); } -CommandMessage::CommandMessage(MessageQueueMessage* messageToSet, +CommandMessage::CommandMessage(MessageQueueMessageIF* messageToSet, Command_t command, uint32_t parameter1, uint32_t parameter2): CommandMessageBase(messageToSet) { if(messageToSet == nullptr) { @@ -28,7 +35,14 @@ CommandMessage::CommandMessage(MessageQueueMessage* messageToSet, " as the message queue message, pass the address of an actual" " message!" << std::endl; } - internalMessage->setMessageSize(COMMAND_MESSAGE_SIZE); + if(messageToSet->getMaximumMessageSize() < + MINIMUM_COMMAND_MESSAGE_SIZE) { + sif::error << "CommandMessage::ComandMessage: Passed message buffer" + " can not hold minimum "<< MINIMUM_COMMAND_MESSAGE_SIZE + << " bytes!" << std::endl; + return; + } + internalMessage->setMessageSize(MINIMUM_COMMAND_MESSAGE_SIZE); setCommand(command); setParameter(parameter1); setParameter2(parameter2); @@ -57,7 +71,7 @@ void CommandMessage::setParameter2(uint32_t parameter2) { } size_t CommandMessage::getMinimumMessageSize() const { - return COMMAND_MESSAGE_SIZE; + return MINIMUM_COMMAND_MESSAGE_SIZE; } size_t CommandMessage::getMaximumMessageSize() const { @@ -81,6 +95,13 @@ void CommandMessage::setReplyRejected(ReturnValue_t reason, setParameter2(initialCommand); } +ReturnValue_t CommandMessage::getRejectedReplyReason( + Command_t* initialCommand) const { + if(initialCommand != nullptr) { + *initialCommand = getParameter2(); + } + return getParameter(); +} void CommandMessage::clear() { clearCommandMessage(); diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index 434f8c8ce..8e1e9c8cd 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -42,8 +42,9 @@ public: * This is the size of a message as it is seen by the MessageQueue. * 14 of the 24 available MessageQueueMessage bytes are used. */ - static const size_t COMMAND_MESSAGE_SIZE = MessageQueueMessage::HEADER_SIZE - + sizeof(Command_t) + 2 * sizeof(uint32_t); + static const size_t MINIMUM_COMMAND_MESSAGE_SIZE = + MessageQueueMessage::HEADER_SIZE + sizeof(Command_t) + + 2 * sizeof(uint32_t); /** * Default Constructor, does not initialize anything. @@ -51,7 +52,7 @@ public: * This constructor should be used when receiving a Message, as the * content is filled by the MessageQueue. */ - CommandMessage(MessageQueueMessage* receiverMessage); + CommandMessage(MessageQueueMessageIF* receiverMessage); /** * This constructor creates a new message with all message content * initialized @@ -60,7 +61,7 @@ public: * @param parameter1 The first parameter * @param parameter2 The second parameter */ - CommandMessage(MessageQueueMessage* messageToSet, Command_t command, + CommandMessage(MessageQueueMessageIF* messageToSet, Command_t command, uint32_t parameter1, uint32_t parameter2); /** @@ -121,8 +122,11 @@ public: * Is needed quite often, so we better code it once only. */ void setToUnknownCommand(); + void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); + ReturnValue_t getRejectedReplyReason( + Command_t* initialCommand = nullptr) const; }; diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp index aa84bd1ef..3ffe4daa2 100644 --- a/ipc/CommandMessageBase.cpp +++ b/ipc/CommandMessageBase.cpp @@ -12,7 +12,7 @@ Command_t CommandMessageBase::getCommand() const { } void CommandMessageBase::setCommand(Command_t command) { - std::memcpy(internalMessage->getData(), &command, sizeof(command)); + std::memcpy(internalMessage->getData(), &command, sizeof(Command_t)); } uint8_t CommandMessageBase::getMessageType() const { @@ -53,3 +53,7 @@ void CommandMessageBase::setMessageSize(size_t messageSize) { size_t CommandMessageBase::getMessageSize() const { return internalMessage->getMessageSize(); } + +MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { + return internalMessage; +} diff --git a/ipc/CommandMessageBase.h b/ipc/CommandMessageBase.h index fa3fd477a..b2b7d5f28 100644 --- a/ipc/CommandMessageBase.h +++ b/ipc/CommandMessageBase.h @@ -22,6 +22,9 @@ */ class CommandMessageBase: public CommandMessageIF { public: + static constexpr size_t HEADER_SIZE = sizeof(MessageQueueId_t) + + sizeof(Command_t); + CommandMessageBase(MessageQueueMessageIF* message); /** @@ -58,6 +61,7 @@ public: virtual void setMessageSize(size_t messageSize) override; virtual size_t getMessageSize() const override; + virtual MessageQueueMessageIF* getInternalMessage() const override; protected: /** * @brief Pointer to the message containing the data. @@ -65,7 +69,7 @@ protected: * 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; + MessageQueueMessageIF* internalMessage = nullptr; }; diff --git a/ipc/CommandMessageIF.h b/ipc/CommandMessageIF.h index fbc40f88e..44853c38b 100644 --- a/ipc/CommandMessageIF.h +++ b/ipc/CommandMessageIF.h @@ -6,6 +6,12 @@ #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 { public: virtual ~CommandMessageIF() {}; @@ -20,6 +26,16 @@ public: * @return */ virtual uint8_t getMessageType() 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; }; #endif /* FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ */ diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 2eb1f4f56..8716df54d 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -88,12 +88,12 @@ public: * queue in the system. So, a change here may have significant impact on * the required resources. */ - static const size_t MAX_MESSAGE_SIZE = MAX_DATA_SIZE + HEADER_SIZE; + static constexpr size_t MAX_MESSAGE_SIZE = MAX_DATA_SIZE + HEADER_SIZE; /** * @brief Defines the minimum size of a message where only the * header is included */ - static const size_t MIN_MESSAGE_SIZE = HEADER_SIZE; + static constexpr size_t MIN_MESSAGE_SIZE = HEADER_SIZE; private: /** * @brief This is the internal buffer that contains the diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index 8c6ec80b0..d44b0bc0f 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -101,7 +101,7 @@ ReturnValue_t MessageQueue::flush(uint32_t* count) { } MessageQueueId_t MessageQueue::getId() const { - return (MessageQueueId_t) handle; + return reinterpret_cast(handle); } void MessageQueue::setDefaultDestination(MessageQueueId_t defaultDestination) { diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index 6165c1cf5..c13a8a200 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include @@ -85,14 +85,14 @@ public: * @param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessageIF* message, bool ignoreFault = false ); + MessageQueueMessageIF* message, bool ignoreFault = false) override; /** * @brief This operation sends a message to the default destination. * @details As in the sendMessage method, this function uses the sendToDefault call of the * MessageQueueSender parent class and adds its queue id as "sentFrom" information. * @param message A pointer to a previously created message, which is sent. */ - ReturnValue_t sendToDefault( MessageQueueMessageIF* message ); + ReturnValue_t sendToDefault(MessageQueueMessageIF* message) override; /** * @brief This operation sends a message to the last communication partner. * @details This operation simplifies answering an incoming message by using the stored @@ -100,7 +100,7 @@ public: * (i.e. lastPartner is zero), an error code is returned. * @param message A pointer to a previously created message, which is sent. */ - ReturnValue_t reply( MessageQueueMessageIF* message ); + ReturnValue_t reply(MessageQueueMessageIF* message) override; /** * @brief With the sendMessage call, a queue message is sent to a receiving queue. @@ -113,9 +113,10 @@ public: * This variable is set to zero by default. * @param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ - virtual ReturnValue_t sendMessageFrom( MessageQueueId_t sendTo, - MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, - bool ignoreFault = false ); + virtual ReturnValue_t sendMessageFrom(MessageQueueId_t sendTo, + MessageQueueMessageIF* message, + MessageQueueId_t sentFrom = NO_QUEUE, + bool ignoreFault = false) override; /** * @brief The sendToDefault method sends a queue message to the default destination. @@ -125,7 +126,8 @@ public: * This variable is set to zero by default. */ virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, - MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false ); + MessageQueueId_t sentFrom = NO_QUEUE, + bool ignoreFault = false) override; /** * @brief This function reads available messages from the message queue and returns the sender. @@ -135,7 +137,7 @@ public: * @param receivedFrom A pointer to a queue id in which the sender's id is stored. */ ReturnValue_t receiveMessage(MessageQueueMessageIF* message, - MessageQueueId_t *receivedFrom); + MessageQueueId_t *receivedFrom) override; /** * @brief This function reads available messages from the message queue. @@ -145,7 +147,7 @@ public: * message's content is cleared and the function returns immediately. * @param message A pointer to a message in which the received data is stored. */ - ReturnValue_t receiveMessage(MessageQueueMessageIF* message); + ReturnValue_t receiveMessage(MessageQueueMessageIF* message) override; /** * Deletes all pending messages in the queue. * @param count The number of flushed messages. diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 72351995d..65053cc88 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -81,40 +81,58 @@ void CommandingServiceBase::handleCommandQueue() { ReturnValue_t result = RETURN_FAILED; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; result = commandQueue->receiveMessage(&reply)) { - handleCommandMessage(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(CommandMessage& reply) { +void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { bool isStep = false; MessageQueueMessage message; CommandMessage nextCommand(&message); - CommandMapIter iter; - if (reply.getSender() == MessageQueueIF::NO_QUEUE) { - handleUnrequestedReply(&reply); - return; - } - if ((iter = commandMap.find(reply.getSender())) == commandMap.end()) { - handleUnrequestedReply(&reply); + CommandMapIter iter = commandMap.find(reply->getSender()); + + // handle unrequested reply first + if (reply->getSender() == MessageQueueIF::NO_QUEUE or + iter == commandMap.end()) { + handleUnrequestedReply(reply); return; } nextCommand.setCommand(CommandMessage::CMD_NONE); + // Implemented by child class, specifies what to do with reply. - ReturnValue_t result = handleReply(&reply, iter->command, &iter->state, + ReturnValue_t result = handleReply(reply, iter->command, &iter->state, &nextCommand, iter->objectId, &isStep); + /* If the child implementation does not implement special handling for + * rejected replies (RETURN_FAILED is returned), a failure verification + * will be generated with the reason as the return code and the initial + * command as failure parameter 1 */ + if(reply->getCommand() == CommandMessage::REPLY_REJECTED and + result == RETURN_FAILED) { + result = reply->getRejectedReplyReason( + reinterpret_cast(&failureParameter1)); + } + switch (result) { case EXECUTION_COMPLETE: case RETURN_OK: case NO_STEP_MESSAGE: // handle result of reply handler implemented by developer. - handleReplyHandlerResult(result, iter, nextCommand, reply, isStep); + handleReplyHandlerResult(result, iter, &nextCommand, reply, isStep); break; case INVALID_REPLY: //might be just an unrequested reply at a bad moment - handleUnrequestedReply(&reply); + handleUnrequestedReply(reply); break; default: if (isStep) { @@ -138,17 +156,17 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage& reply) { } void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, - CommandMapIter iter, CommandMessage& nextCommand, CommandMessage& reply, + CommandMapIter iter, CommandMessage* nextCommand, CommandMessage* reply, bool& isStep) { - iter->command = nextCommand.getCommand(); + iter->command = nextCommand->getCommand(); // In case a new command is to be sent immediately, this is performed here. // If no new command is sent, only analyse reply result by initializing // sendResult as RETURN_OK ReturnValue_t sendResult = RETURN_OK; - if (nextCommand.getCommand() != CommandMessage::CMD_NONE) { - sendResult = commandQueue->sendMessage(reply.getSender(), - &nextCommand); + if (nextCommand->getCommand() != CommandMessage::CMD_NONE) { + sendResult = commandQueue->sendMessage(reply->getSender(), + nextCommand); } if (sendResult == RETURN_OK) { @@ -168,14 +186,14 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, } else { if (isStep) { - nextCommand.clearCommandMessage(); + nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, sendResult, ++iter->step, failureParameter1, failureParameter2); } else { - nextCommand.clearCommandMessage(); + nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, @@ -366,9 +384,9 @@ void CommandingServiceBase::checkAndExecuteFifo(CommandMapIter iter) { } -void CommandingServiceBase::handleUnrequestedReply( - CommandMessage* reply) { - reply->clearCommandMessage(); +void CommandingServiceBase::handleUnrequestedReply(CommandMessageIF* reply) { + CommandMessage commandReply(reply->getInternalMessage()); + commandReply.clear(); } diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 1dcafff82..5adfe4a69 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -147,9 +147,11 @@ protected: /** * This function is implemented by child services to specify how replies - * to a command from another software component are handled + * to a command from another software component are handled. * @param reply - * This is the reply in form of a command message. + * This is the reply which can be accessed via the command message + * interface. The internal message pointer can be passed to different + * command message implementations (see CommandMessageIF) * @param previousCommand * Command_t of related command * @param state [out/in] @@ -163,9 +165,12 @@ protected: * - @c RETURN_OK, @c EXECUTION_COMPLETE or @c NO_STEP_MESSAGE to * generate TC verification success * - @c INVALID_REPLY calls handleUnrequestedReply - * - Anything else triggers a TC verification failure + * - Anything else triggers a TC verification failure. If RETURN_FAILED + * is returned and the command ID is CommandMessage::REPLY_REJECTED, + * a failure verification message with the reason as the error parameter + * and the initial command as failure parameter 1. */ - virtual ReturnValue_t handleReply(const CommandMessage *reply, + virtual ReturnValue_t handleReply(const CommandMessageIF *reply, Command_t previousCommand, uint32_t *state, CommandMessage *optionalNextCommand, object_id_t objectId, bool *isStep) = 0; @@ -173,9 +178,13 @@ protected: /** * This function can be overidden to handle unrequested reply, * when the reply sender ID is unknown or is not found is the command map. + * The default implementation will clear the command message and all + * its contents. * @param reply + * Reply which is non-const so the default implementation can clear the + * message. */ - virtual void handleUnrequestedReply(CommandMessage *reply); + virtual void handleUnrequestedReply(CommandMessageIF *reply); virtual void doPeriodicOperation(); @@ -303,9 +312,9 @@ private: void startExecution(TcPacketStored *storedPacket, CommandMapIter iter); - void handleCommandMessage(CommandMessage& reply); + void handleCommandMessage(CommandMessage* reply); void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, - CommandMessage& nextCommand,CommandMessage& reply, bool& isStep); + CommandMessage* nextCommand,CommandMessage* reply, bool& isStep); void checkTimeout(); }; diff --git a/tmtcservices/VerificationReporter.cpp b/tmtcservices/VerificationReporter.cpp index 4484fb9b2..b02479449 100644 --- a/tmtcservices/VerificationReporter.cpp +++ b/tmtcservices/VerificationReporter.cpp @@ -24,9 +24,8 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, current_packet->getPacketSequenceControl(), 0, set_step); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); if (status != HasReturnvaluesIF::RETURN_OK) { - sif::error - << "VerificationReporter::sendSuccessReport: Error writing to queue. Code: " - << (uint16_t) status << std::endl; + sif::error << "VerificationReporter::sendSuccessReport: Error writing " + "to queue. Code: " << std::hex << (uint16_t) status << std::endl; } } @@ -40,9 +39,8 @@ void VerificationReporter::sendSuccessReport(uint8_t set_report_id, tcSequenceControl, 0, set_step); ReturnValue_t status = MessageQueueSenderIF::sendMessage(acknowledgeQueue, &message); if (status != HasReturnvaluesIF::RETURN_OK) { - sif::error - << "VerificationReporter::sendSuccessReport: Error writing to queue. Code: " - << (uint16_t) status << std::endl; + sif::error << "VerificationReporter::sendSuccessReport: Error writing " + "to queue. Code: " << std::hex << (uint16_t) status << std::endl; } } From 8c03f6a823afca715f51d01cbecdbaa5e6ca185e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 13 Jun 2020 21:01:01 +0200 Subject: [PATCH 4/4] command message only passed IF now --- ipc/CommandMessage.cpp | 74 +++----------------------- ipc/CommandMessage.h | 33 ------------ ipc/CommandMessageBase.cpp | 23 ++++++++ ipc/CommandMessageBase.h | 21 ++++++-- ipc/CommandMessageCleaner.cpp | 45 ++++++++++++++++ ipc/CommandMessageCleaner.h | 16 ++++++ ipc/CommandMessageIF.h | 34 ++++++++++++ memory/MemoryHelper.cpp | 2 +- tmtcservices/CommandingServiceBase.cpp | 16 +++--- tmtcservices/CommandingServiceBase.h | 8 +-- 10 files changed, 155 insertions(+), 117 deletions(-) create mode 100644 ipc/CommandMessageCleaner.cpp create mode 100644 ipc/CommandMessageCleaner.h diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 9d448aed6..392906880 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -1,13 +1,5 @@ #include - -#include -#include -#include -#include -#include -#include -#include -#include +#include CommandMessage::CommandMessage(MessageQueueMessageIF* receiverMessage): CommandMessageBase(receiverMessage) { @@ -50,23 +42,23 @@ CommandMessage::CommandMessage(MessageQueueMessageIF* messageToSet, uint32_t CommandMessage::getParameter() const { uint32_t parameter1; - memcpy(¶meter1, CommandMessageBase::getData(), sizeof(parameter1)); + std::memcpy(¶meter1, CommandMessageBase::getData(), sizeof(parameter1)); return parameter1; } void CommandMessage::setParameter(uint32_t parameter1) { - memcpy(CommandMessageBase::getData(), ¶meter1, sizeof(parameter1)); + std::memcpy(CommandMessageBase::getData(), ¶meter1, sizeof(parameter1)); } uint32_t CommandMessage::getParameter2() const { uint32_t parameter2; - memcpy(¶meter2, CommandMessageBase::getData() + sizeof(uint32_t), + std::memcpy(¶meter2, CommandMessageBase::getData() + sizeof(uint32_t), sizeof(parameter2)); return parameter2; } void CommandMessage::setParameter2(uint32_t parameter2) { - memcpy(CommandMessageBase::getData() + sizeof(uint32_t), ¶meter2, + std::memcpy(CommandMessageBase::getData() + sizeof(uint32_t), ¶meter2, sizeof(parameter2)); } @@ -84,60 +76,6 @@ bool CommandMessage::isClearedCommandMessage() { void CommandMessage::setToUnknownCommand() { Command_t initialCommand = getCommand(); - clearCommandMessage(); + this->clear(); setReplyRejected(UNKNOWN_COMMAND, initialCommand); } - -void CommandMessage::setReplyRejected(ReturnValue_t reason, - Command_t initialCommand) { - setCommand(REPLY_REJECTED); - setParameter(reason); - setParameter2(initialCommand); -} - -ReturnValue_t CommandMessage::getRejectedReplyReason( - Command_t* initialCommand) const { - if(initialCommand != nullptr) { - *initialCommand = getParameter2(); - } - return getParameter(); -} - -void CommandMessage::clear() { - clearCommandMessage(); -} - -void CommandMessage::clearCommandMessage() { - switch(this->getMessageType()){ - case messagetypes::MODE_COMMAND: - ModeMessage::clear(this); - break; - case messagetypes::HEALTH_COMMAND: - HealthMessage::clear(this); - break; - case messagetypes::MODE_SEQUENCE: - ModeSequenceMessage::clear(this); - break; - case messagetypes::ACTION: - ActionMessage::clear(this); - break; - case messagetypes::DEVICE_HANDLER_COMMAND: - DeviceHandlerMessage::clear(this); - break; - case messagetypes::MEMORY: - MemoryMessage::clear(this); - break; - case messagetypes::MONITORING: - MonitoringMessage::clear(this); - break; - case messagetypes::TM_STORE: - TmStoreMessage::clear(this); - break; - case messagetypes::PARAMETER: - ParameterMessage::clear(this); - break; - default: - messagetypes::clearMissionMessage(this); - break; - } -} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index 8e1e9c8cd..72f393573 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -5,11 +5,6 @@ #include #include -namespace messagetypes { -// Implemented in config. -void clearMissionMessage(CommandMessageIF* message); -} - /** * @brief Default command message used to pass command messages between tasks. * Primary message type for IPC. Contains sender, 2-byte command ID @@ -27,17 +22,6 @@ void clearMissionMessage(CommandMessageIF* message); */ class CommandMessage: public CommandMessageBase { public: - static const uint8_t INTERFACE_ID = CLASS_ID::COMMAND_MESSAGE; - static const ReturnValue_t UNKNOWN_COMMAND = MAKE_RETURN_CODE(0x01); - - static const uint8_t MESSAGE_ID = messagetypes::COMMAND; - //! Used internally, will be ignored - static const Command_t CMD_NONE = MAKE_COMMAND_ID( 0 ); - static const Command_t REPLY_COMMAND_OK = MAKE_COMMAND_ID( 3 ); - //! Reply indicating that the current command was rejected, - //! par1 should contain the error code - static const Command_t REPLY_REJECTED = MAKE_COMMAND_ID( 0xD1 ); - /** * This is the size of a message as it is seen by the MessageQueue. * 14 of the 24 available MessageQueueMessage bytes are used. @@ -98,18 +82,6 @@ public: */ void setParameter2(uint32_t parameter2); - void clear() override; - - /** - * Set the command to CMD_NONE and try to find the correct class to handle - * a more detailed clear. - * Also, calls a mission-specific clearMissionMessage function to separate - * between framework and mission messages. Not optimal, may be replaced by - * totally different auto-delete solution (e.g. smart pointers). - * - */ - void clearCommandMessage(); - /** * check if a message was cleared * @@ -122,11 +94,6 @@ public: * Is needed quite often, so we better code it once only. */ void setToUnknownCommand(); - - void setReplyRejected(ReturnValue_t reason, - Command_t initialCommand = CMD_NONE); - ReturnValue_t getRejectedReplyReason( - Command_t* initialCommand = nullptr) const; }; diff --git a/ipc/CommandMessageBase.cpp b/ipc/CommandMessageBase.cpp index 3ffe4daa2..7085ab88d 100644 --- a/ipc/CommandMessageBase.cpp +++ b/ipc/CommandMessageBase.cpp @@ -1,4 +1,5 @@ #include +#include #include CommandMessageBase::CommandMessageBase(MessageQueueMessageIF *message): @@ -57,3 +58,25 @@ size_t CommandMessageBase::getMessageSize() const { MessageQueueMessageIF* CommandMessageBase::getInternalMessage() const { return internalMessage; } + +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 b2b7d5f28..d2f24c7e6 100644 --- a/ipc/CommandMessageBase.h +++ b/ipc/CommandMessageBase.h @@ -22,9 +22,6 @@ */ class CommandMessageBase: public CommandMessageIF { public: - static constexpr size_t HEADER_SIZE = sizeof(MessageQueueId_t) + - sizeof(Command_t); - CommandMessageBase(MessageQueueMessageIF* message); /** @@ -61,7 +58,25 @@ public: virtual void setMessageSize(size_t messageSize) 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 void clear() override; protected: /** * @brief Pointer to the message containing the data. diff --git a/ipc/CommandMessageCleaner.cpp b/ipc/CommandMessageCleaner.cpp new file mode 100644 index 000000000..7153a8e71 --- /dev/null +++ b/ipc/CommandMessageCleaner.cpp @@ -0,0 +1,45 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +void CommandMessageCleaner::clearCommandMessage(CommandMessageIF* message) { + switch(message->getMessageType()){ + case messagetypes::MODE_COMMAND: + ModeMessage::clear(dynamic_cast(message)); + break; + case messagetypes::HEALTH_COMMAND: + HealthMessage::clear(dynamic_cast(message)); + break; + case messagetypes::MODE_SEQUENCE: + ModeSequenceMessage::clear(dynamic_cast(message)); + break; + case messagetypes::ACTION: + ActionMessage::clear(dynamic_cast(message)); + break; + case messagetypes::DEVICE_HANDLER_COMMAND: + DeviceHandlerMessage::clear(dynamic_cast(message)); + break; + case messagetypes::MEMORY: + MemoryMessage::clear(dynamic_cast(message)); + break; + case messagetypes::MONITORING: + MonitoringMessage::clear(dynamic_cast(message)); + break; + case messagetypes::TM_STORE: + TmStoreMessage::clear(dynamic_cast(message)); + break; + case messagetypes::PARAMETER: + ParameterMessage::clear(dynamic_cast(message)); + break; + default: + messagetypes::clearMissionMessage(message); + break; + } +} diff --git a/ipc/CommandMessageCleaner.h b/ipc/CommandMessageCleaner.h new file mode 100644 index 000000000..0bd369a2c --- /dev/null +++ b/ipc/CommandMessageCleaner.h @@ -0,0 +1,16 @@ +#ifndef FRAMEWORK_IPC_COMMANDMESSAGECLEANER_H_ +#define FRAMEWORK_IPC_COMMANDMESSAGECLEANER_H_ +#include + +namespace messagetypes { +// Implemented in config. +void clearMissionMessage(CommandMessageIF* message); +} + +class CommandMessageCleaner { +public: + static void clearCommandMessage(CommandMessageIF* message); +}; + + +#endif /* FRAMEWORK_IPC_COMMANDMESSAGECLEANER_H_ */ diff --git a/ipc/CommandMessageIF.h b/ipc/CommandMessageIF.h index 44853c38b..fbfbddbb9 100644 --- a/ipc/CommandMessageIF.h +++ b/ipc/CommandMessageIF.h @@ -2,6 +2,9 @@ #define FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ #include +#include +#include + #define MAKE_COMMAND_ID( number ) ((MESSAGE_ID << 8) + (number)) typedef uint16_t Command_t; @@ -14,6 +17,20 @@ typedef uint16_t Command_t; // we should just leave it like that. class CommandMessageIF: public MessageQueueMessageIF { public: + static constexpr size_t HEADER_SIZE = sizeof(MessageQueueId_t) + + sizeof(Command_t); + + static const uint8_t INTERFACE_ID = CLASS_ID::COMMAND_MESSAGE; + static const ReturnValue_t UNKNOWN_COMMAND = MAKE_RETURN_CODE(0x01); + + static const uint8_t MESSAGE_ID = messagetypes::COMMAND; + //! Used internally, shall be ignored + static const Command_t CMD_NONE = MAKE_COMMAND_ID( 0 ); + static const Command_t REPLY_COMMAND_OK = MAKE_COMMAND_ID( 1 ); + //! Reply indicating that the current command was rejected, + //! par1 should contain the error code + static const Command_t REPLY_REJECTED = MAKE_COMMAND_ID( 2 ); + virtual ~CommandMessageIF() {}; /** @@ -27,6 +44,22 @@ public: */ virtual uint8_t getMessageType() const = 0; + /** + * A command message can be rejected and needs to offer a function + * to set a rejected reply + * @param reason + * @param initialCommand + */ + virtual void setReplyRejected(ReturnValue_t reason, + Command_t initialCommand) = 0; + /** + * Corrensonding getter function. + * @param initialCommand + * @return + */ + 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 @@ -36,6 +69,7 @@ public: * @return */ virtual MessageQueueMessageIF* getInternalMessage() const = 0; + }; #endif /* FRAMEWORK_IPC_COMMANDMESSAGEIF_H_ */ diff --git a/memory/MemoryHelper.cpp b/memory/MemoryHelper.cpp index 2c3846512..fb4d9eb67 100644 --- a/memory/MemoryHelper.cpp +++ b/memory/MemoryHelper.cpp @@ -119,7 +119,7 @@ void MemoryHelper::completeDump(ReturnValue_t errorCode, break; } if (queueToUse->sendMessage(lastSender, &reply) != RETURN_OK) { - reply.clearCommandMessage(); + reply.clear(); } } diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 65053cc88..97856b40a 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -94,7 +94,7 @@ void CommandingServiceBase::handleCommandQueue() { } -void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { +void CommandingServiceBase::handleCommandMessage(CommandMessageIF* reply) { bool isStep = false; MessageQueueMessage message; CommandMessage nextCommand(&message); @@ -119,7 +119,7 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { * command as failure parameter 1 */ if(reply->getCommand() == CommandMessage::REPLY_REJECTED and result == RETURN_FAILED) { - result = reply->getRejectedReplyReason( + result = reply->getReplyRejectedReason( reinterpret_cast(&failureParameter1)); } @@ -156,8 +156,8 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { } void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, - CommandMapIter iter, CommandMessage* nextCommand, CommandMessage* reply, - bool& isStep) { + CommandMapIter iter, CommandMessageIF* nextCommand, + CommandMessageIF* reply, bool& isStep) { iter->command = nextCommand->getCommand(); // In case a new command is to be sent immediately, this is performed here. @@ -186,14 +186,14 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, } else { if (isStep) { - nextCommand->clearCommandMessage(); + nextCommand->clear(); verificationReporter.sendFailureReport( TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, sendResult, ++iter->step, failureParameter1, failureParameter2); } else { - nextCommand->clearCommandMessage(); + nextCommand->clear(); verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, @@ -329,7 +329,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, storedPacket->getPacketSequenceControl(); acceptPacket(TC_VERIFY::START_SUCCESS, storedPacket); } else { - command.clearCommandMessage(); + command.clear(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } @@ -346,7 +346,7 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, acceptPacket(TC_VERIFY::COMPLETION_SUCCESS, storedPacket); checkAndExecuteFifo(iter); } else { - command.clearCommandMessage(); + command.clear(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 5adfe4a69..5e2c31713 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -141,7 +141,7 @@ protected: * @param objectId Target object ID * @return */ - virtual ReturnValue_t prepareCommand(CommandMessage *message, + virtual ReturnValue_t prepareCommand(CommandMessageIF *message, uint8_t subservice, const uint8_t *tcData, size_t tcDataLen, uint32_t *state, object_id_t objectId) = 0; @@ -172,7 +172,7 @@ protected: */ virtual ReturnValue_t handleReply(const CommandMessageIF *reply, Command_t previousCommand, uint32_t *state, - CommandMessage *optionalNextCommand, object_id_t objectId, + CommandMessageIF *optionalNextCommand, object_id_t objectId, bool *isStep) = 0; /** @@ -312,9 +312,9 @@ private: void startExecution(TcPacketStored *storedPacket, CommandMapIter iter); - void handleCommandMessage(CommandMessage* reply); + void handleCommandMessage(CommandMessageIF* reply); void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, - CommandMessage* nextCommand,CommandMessage* reply, bool& isStep); + CommandMessageIF* nextCommand,CommandMessageIF* reply, bool& isStep); void checkTimeout(); };