From 8ff6506ad90965f6a42d2b03a1fa2b2638f436c9 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Jun 2020 02:18:39 +0200 Subject: [PATCH 01/12] MessageQueue refactoring complete --- action/ActionHelper.cpp | 17 ++++--- action/CommandActionHelper.cpp | 3 +- action/SimpleActionHelper.cpp | 3 +- controller/ControllerBase.cpp | 17 +++---- datalinklayer/MapPacketExtraction.cpp | 6 +-- datapool/HkSwitchHelper.cpp | 11 ++--- datapoolglob/DataPoolAdmin.cpp | 9 ++-- devicehandlers/AssemblyBase.cpp | 3 +- devicehandlers/DeviceHandlerBase.cpp | 29 ++++++------ devicehandlers/HealthDevice.cpp | 7 +-- health/HealthHelper.cpp | 20 +++++---- ipc/CommandMessage.cpp | 62 ++++++++++++++++++++------ ipc/CommandMessage.h | 28 +++++++++--- ipc/MessageQueueIF.h | 16 +++---- ipc/MessageQueueMessage.cpp | 2 +- ipc/MessageQueueMessage.h | 2 +- ipc/MessageQueueMessageIF.h | 29 +++++++++--- ipc/MessageQueueSenderIF.h | 16 ++----- memory/MemoryHelper.cpp | 15 ++++--- modes/ModeHelper.cpp | 31 +++++++------ monitoring/LimitViolationReporter.cpp | 3 +- osal/FreeRTOS/MessageQueue.cpp | 18 ++++---- osal/FreeRTOS/MessageQueue.h | 19 ++++---- osal/FreeRTOS/QueueFactory.cpp | 2 +- parameters/ParameterHelper.cpp | 9 ++-- power/Fuse.cpp | 3 +- power/PowerSensor.cpp | 3 +- subsystem/Subsystem.cpp | 14 +++--- subsystem/SubsystemBase.cpp | 43 +++++++++--------- thermal/AbstractTemperatureSensor.cpp | 11 ++--- thermal/Heater.cpp | 9 ++-- tmstorage/TmStoreMessage.cpp | 2 +- tmstorage/TmStoreMessage.h | 2 +- tmtcservices/CommandingServiceBase.cpp | 20 +++++---- 34 files changed, 293 insertions(+), 191 deletions(-) diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index 19a84d82..0df5f2d4 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -16,7 +16,7 @@ ReturnValue_t ActionHelper::handleActionMessage(CommandMessage* command) { ActionMessage::getStoreId(command)); return HasReturnvaluesIF::RETURN_OK; } else { - return CommandMessage::UNKNOW_COMMAND; + return CommandMessage::UNKNOWN_COMMAND; } } @@ -33,13 +33,15 @@ ReturnValue_t ActionHelper::initialize(MessageQueueIF* queueToUse_) { } void ActionHelper::step(uint8_t step, MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ActionMessage::setStepReply(&reply, commandId, step + STEP_OFFSET, result); queueToUse->sendMessage(reportTo, &reply); } void ActionHelper::finish(MessageQueueId_t reportTo, ActionId_t commandId, ReturnValue_t result) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ActionMessage::setCompletionReply(&reply, commandId, result); queueToUse->sendMessage(reportTo, &reply); } @@ -54,7 +56,8 @@ 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) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ActionMessage::setStepReply(&reply, actionId, 0, result); queueToUse->sendMessage(commandedBy, &reply); return; @@ -62,7 +65,8 @@ void ActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t act result = owner->executeAction(actionId, commandedBy, dataPtr, size); ipcStore->deleteData(dataAddress); if (result != HasReturnvaluesIF::RETURN_OK) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ActionMessage::setStepReply(&reply, actionId, 0, result); queueToUse->sendMessage(commandedBy, &reply); return; @@ -71,7 +75,8 @@ void ActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t act ReturnValue_t ActionHelper::reportData(MessageQueueId_t reportTo, ActionId_t replyId, SerializeIF* data, bool hideSender) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); store_address_t storeAddress; uint8_t *dataPtr; size_t maxSize = data->getSerializedSize(); diff --git a/action/CommandActionHelper.cpp b/action/CommandActionHelper.cpp index 77295401..c871a2d0 100644 --- a/action/CommandActionHelper.cpp +++ b/action/CommandActionHelper.cpp @@ -53,7 +53,8 @@ ReturnValue_t CommandActionHelper::commandAction(object_id_t commandTo, ReturnValue_t CommandActionHelper::sendCommand(MessageQueueId_t queueId, ActionId_t actionId, store_address_t storeId) { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); 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 6861fc28..f97474b1 100644 --- a/action/SimpleActionHelper.cpp +++ b/action/SimpleActionHelper.cpp @@ -36,7 +36,8 @@ void SimpleActionHelper::resetHelper() { void SimpleActionHelper::prepareExecution(MessageQueueId_t commandedBy, ActionId_t actionId, store_address_t dataAddress) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); if (isExecuting) { ipcStore->deleteData(dataAddress); ActionMessage::setStepReply(&reply, actionId, 0, diff --git a/controller/ControllerBase.cpp b/controller/ControllerBase.cpp index 7b11bdfa..ad0a0b97 100644 --- a/controller/ControllerBase.cpp +++ b/controller/ControllerBase.cpp @@ -56,26 +56,27 @@ MessageQueueId_t ControllerBase::getCommandQueue() const { } void ControllerBase::handleQueue() { - CommandMessage message; + MessageQueueMessage message; + CommandMessage command(&message); ReturnValue_t result; - for (result = commandQueue->receiveMessage(&message); result == RETURN_OK; - result = commandQueue->receiveMessage(&message)) { + for (result = commandQueue->receiveMessage(&command); result == RETURN_OK; + result = commandQueue->receiveMessage(&command)) { - result = modeHelper.handleModeCommand(&message); + result = modeHelper.handleModeCommand(&command); if (result == RETURN_OK) { continue; } - result = healthHelper.handleHealthCommand(&message); + result = healthHelper.handleHealthCommand(&command); if (result == RETURN_OK) { continue; } - result = handleCommandMessage(&message); + result = handleCommandMessage(&command); if (result == RETURN_OK) { continue; } - message.setToUnknownCommand(); - commandQueue->reply(&message); + command.setToUnknownCommand(); + commandQueue->reply(&command); } } diff --git a/datalinklayer/MapPacketExtraction.cpp b/datalinklayer/MapPacketExtraction.cpp index 11b0792a..4ea45e89 100644 --- a/datalinklayer/MapPacketExtraction.cpp +++ b/datalinklayer/MapPacketExtraction.cpp @@ -16,9 +16,9 @@ MapPacketExtraction::MapPacketExtraction(uint8_t setMapId, object_id_t setPacketDestination) : - lastSegmentationFlag(NO_SEGMENTATION), mapId(setMapId), packetLength(0), bufferPosition( - packetBuffer), packetDestination(setPacketDestination), packetStore( - NULL), tcQueueId(MessageQueueSenderIF::NO_QUEUE) { + lastSegmentationFlag(NO_SEGMENTATION), mapId(setMapId), packetLength(0), + bufferPosition(packetBuffer), packetDestination(setPacketDestination), + packetStore(nullptr), tcQueueId(MessageQueueMessageIF::NO_QUEUE) { memset(packetBuffer, 0, sizeof(packetBuffer)); } diff --git a/datapool/HkSwitchHelper.cpp b/datapool/HkSwitchHelper.cpp index 844fcd90..6a923776 100644 --- a/datapool/HkSwitchHelper.cpp +++ b/datapool/HkSwitchHelper.cpp @@ -21,14 +21,15 @@ ReturnValue_t HkSwitchHelper::initialize() { } ReturnValue_t HkSwitchHelper::performOperation(uint8_t operationCode) { - CommandMessage message; - while (actionQueue->receiveMessage(&message) == HasReturnvaluesIF::RETURN_OK) { - ReturnValue_t result = commandActionHelper.handleReply(&message); + MessageQueueMessage message; + CommandMessage command(&message); + while (actionQueue->receiveMessage(&command) == HasReturnvaluesIF::RETURN_OK) { + ReturnValue_t result = commandActionHelper.handleReply(&command); if (result == HasReturnvaluesIF::RETURN_OK) { continue; } - message.setToUnknownCommand(); - actionQueue->reply(&message); + command.setToUnknownCommand(); + actionQueue->reply(&command); } return HasReturnvaluesIF::RETURN_OK; diff --git a/datapoolglob/DataPoolAdmin.cpp b/datapoolglob/DataPoolAdmin.cpp index 05de1eb2..6387263f 100644 --- a/datapoolglob/DataPoolAdmin.cpp +++ b/datapoolglob/DataPoolAdmin.cpp @@ -66,7 +66,8 @@ ReturnValue_t DataPoolAdmin::getParameter(uint8_t domainId, } void DataPoolAdmin::handleCommand() { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != RETURN_OK) { return; @@ -282,7 +283,8 @@ ReturnValue_t DataPoolAdmin::sendParameter(MessageQueueId_t to, uint32_t id, return result; } - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ParameterMessage::setParameterDumpReply(&reply, id, address); @@ -294,7 +296,8 @@ 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) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); reply.setReplyRejected(reason, initialCommand); commandQueue->sendMessage(to, &reply); } diff --git a/devicehandlers/AssemblyBase.cpp b/devicehandlers/AssemblyBase.cpp index bd85d1de..a5dcad21 100644 --- a/devicehandlers/AssemblyBase.cpp +++ b/devicehandlers/AssemblyBase.cpp @@ -148,7 +148,8 @@ void AssemblyBase::handleModeTransitionFailed(ReturnValue_t result) { void AssemblyBase::sendHealthCommand(MessageQueueId_t sendTo, HealthState health) { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); 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 94c9dc63..11cffc74 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -218,8 +218,9 @@ void DeviceHandlerBase::readCommandQueue() { // 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. - CommandMessage cmdMessage; - ReturnValue_t result = commandQueue->receiveMessage(&cmdMessage); + MessageQueueMessage message; + CommandMessage command(&message); + ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != RETURN_OK) { return; } @@ -234,23 +235,23 @@ void DeviceHandlerBase::readCommandQueue() { // } if(healthHelperActive) { - result = healthHelper.handleHealthCommand(&cmdMessage); + result = healthHelper.handleHealthCommand(&command); if (result == RETURN_OK) { return; } } - result = modeHelper.handleModeCommand(&cmdMessage); + result = modeHelper.handleModeCommand(&command); if (result == RETURN_OK) { return; } - result = actionHelper.handleActionMessage(&cmdMessage); + result = actionHelper.handleActionMessage(&command); if (result == RETURN_OK) { return; } - result = parameterHelper.handleParameterMessage(&cmdMessage); + result = parameterHelper.handleParameterMessage(&command); if (result == RETURN_OK) { return; } @@ -261,17 +262,17 @@ void DeviceHandlerBase::readCommandQueue() { // return; // } - result = handleDeviceHandlerMessage(&cmdMessage); + result = handleDeviceHandlerMessage(&command); if (result == RETURN_OK) { return; } - result = letChildHandleMessage(&cmdMessage); + result = letChildHandleMessage(&command); if (result == RETURN_OK) { return; } - replyReturnvalueToCommand(CommandMessage::UNKNOW_COMMAND); + replyReturnvalueToCommand(CommandMessage::UNKNOWN_COMMAND); } @@ -492,12 +493,13 @@ 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(CommandMessage::REPLY_COMMAND_OK, 0, parameter); + CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, parameter); commandQueue->reply(&reply); } else { - CommandMessage reply(CommandMessage::REPLY_REJECTED, status, parameter); + CommandMessage reply(&message, CommandMessage::REPLY_REJECTED, status, parameter); commandQueue->reply(&reply); } } @@ -767,9 +769,10 @@ void DeviceHandlerBase::replyRawData(const uint8_t *data, size_t len, return; } - CommandMessage message; + MessageQueueMessage message; + CommandMessage command(&message); - DeviceHandlerMessage::setDeviceHandlerRawReplyMessage(&message, + DeviceHandlerMessage::setDeviceHandlerRawReplyMessage(&command, getObjectId(), address, isCommand); // this->DeviceHandlerCommand = CommandMessage::CMD_NONE; diff --git a/devicehandlers/HealthDevice.cpp b/devicehandlers/HealthDevice.cpp index ea0b99ff..1c9d18f6 100644 --- a/devicehandlers/HealthDevice.cpp +++ b/devicehandlers/HealthDevice.cpp @@ -13,10 +13,11 @@ HealthDevice::~HealthDevice() { } ReturnValue_t HealthDevice::performOperation(uint8_t opCode) { - CommandMessage message; - ReturnValue_t result = commandQueue->receiveMessage(&message); + MessageQueueMessage message; + CommandMessage command(&message); + ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { - healthHelper.handleHealthCommand(&message); + healthHelper.handleHealthCommand(&command); } return HasReturnvaluesIF::RETURN_OK; } diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index fdfc0967..b23a8bd2 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -65,29 +65,31 @@ void HealthHelper::informParent(HasHealthIF::HealthState health, if (parentQueue == 0) { return; } - CommandMessage message; - HealthMessage::setHealthMessage(&message, HealthMessage::HEALTH_INFO, + MessageQueueMessage message; + CommandMessage information(&message); + HealthMessage::setHealthMessage(&information, HealthMessage::HEALTH_INFO, health, oldHealth); - if (MessageQueueSenderIF::sendMessage(parentQueue, &message, + if (MessageQueueSenderIF::sendMessage(parentQueue, &information, owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::informParent: sending health reply failed." << std::endl; } } -void HealthHelper::handleSetHealthCommand(CommandMessage* message) { - ReturnValue_t result = owner->setHealth(HealthMessage::getHealth(message)); - if (message->getSender() == 0) { +void HealthHelper::handleSetHealthCommand(CommandMessage* command) { + ReturnValue_t result = owner->setHealth(HealthMessage::getHealth(command)); + if (command->getSender() == MessageQueueMessageIF::NO_QUEUE) { return; } - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); if (result == HasReturnvaluesIF::RETURN_OK) { HealthMessage::setHealthMessage(&reply, HealthMessage::REPLY_HEALTH_SET); } else { - reply.setReplyRejected(result, message->getCommand()); + reply.setReplyRejected(result, command->getCommand()); } - if (MessageQueueSenderIF::sendMessage(message->getSender(), &reply, + if (MessageQueueSenderIF::sendMessage(command->getSender(), &reply, owner->getCommandQueue()) != HasReturnvaluesIF::RETURN_OK) { sif::debug << "HealthHelper::handleHealthCommand: sending health " "reply failed." << std::endl; diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index f9e2e010..cd75aee4 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -14,14 +14,26 @@ void clearMissionMessage(CommandMessage* message); } -CommandMessage::CommandMessage() { - this->messageSize = COMMAND_MESSAGE_SIZE; +CommandMessage::CommandMessage(MessageQueueMessage* receiverMessage): + internalMessage(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; setCommand(CMD_NONE); } -CommandMessage::CommandMessage(Command_t command, uint32_t parameter1, - uint32_t parameter2) { - this->messageSize = COMMAND_MESSAGE_SIZE; +CommandMessage::CommandMessage(MessageQueueMessage* messageToSet, + Command_t command, uint32_t parameter1, uint32_t parameter2): + internalMessage(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; setCommand(command); setParameter(parameter1); setParameter2(parameter2); @@ -29,7 +41,7 @@ CommandMessage::CommandMessage(Command_t command, uint32_t parameter1, Command_t CommandMessage::getCommand() const { Command_t command; - memcpy(&command, getData(), sizeof(Command_t)); + memcpy(&command, internalMessage->getData(), sizeof(Command_t)); return command; } @@ -38,29 +50,35 @@ uint8_t CommandMessage::getMessageType() const { } void CommandMessage::setCommand(Command_t command) { - memcpy(getData(), &command, sizeof(command)); + memcpy(internalMessage->getData(), &command, sizeof(command)); } uint32_t CommandMessage::getParameter() const { uint32_t parameter1; - memcpy(¶meter1, getData() + sizeof(Command_t), sizeof(parameter1)); + memcpy(¶meter1, internalMessage->getData() + sizeof(Command_t), + sizeof(parameter1)); return parameter1; } void CommandMessage::setParameter(uint32_t parameter1) { - memcpy(getData() + sizeof(Command_t), ¶meter1, sizeof(parameter1)); + memcpy(internalMessage->getData() + sizeof(Command_t), + ¶meter1, sizeof(parameter1)); } uint32_t CommandMessage::getParameter2() const { uint32_t parameter2; - memcpy(¶meter2, getData() + sizeof(Command_t) + sizeof(uint32_t), - sizeof(parameter2)); + memcpy(¶meter2, internalMessage->getData() + sizeof(Command_t) + + sizeof(uint32_t), sizeof(parameter2)); return parameter2; } void CommandMessage::setParameter2(uint32_t parameter2) { - memcpy(getData() + sizeof(Command_t) + sizeof(uint32_t), ¶meter2, - sizeof(parameter2)); + memcpy(internalMessage-> getData() + sizeof(Command_t) + sizeof(uint32_t), + ¶meter2, sizeof(parameter2)); +} + +void CommandMessage::clear() { + clearCommandMessage(); } void CommandMessage::clearCommandMessage() { @@ -109,7 +127,7 @@ size_t CommandMessage::getMinimumMessageSize() const { void CommandMessage::setToUnknownCommand() { Command_t initialCommand = getCommand(); clearCommandMessage(); - setReplyRejected(UNKNOW_COMMAND, initialCommand); + setReplyRejected(UNKNOWN_COMMAND, initialCommand); } void CommandMessage::setReplyRejected(ReturnValue_t reason, @@ -118,3 +136,19 @@ void CommandMessage::setReplyRejected(ReturnValue_t reason, 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(); +} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index a5c187f5..f6fd86cb 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -13,10 +13,10 @@ typedef uint16_t Command_t; * @brief Used to pass command messages between tasks * @author Bastian Baetz */ -class CommandMessage : public MessageQueueMessage { +class CommandMessage: public MessageQueueMessageIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::COMMAND_MESSAGE; - static const ReturnValue_t UNKNOW_COMMAND = MAKE_RETURN_CODE(0x01); + static const ReturnValue_t UNKNOWN_COMMAND = MAKE_RETURN_CODE(0x01); static const uint8_t MESSAGE_ID = messagetypes::COMMAND; @@ -31,7 +31,7 @@ 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 = HEADER_SIZE + static const size_t COMMAND_MESSAGE_SIZE = MessageQueueMessage::HEADER_SIZE + sizeof(Command_t) + 2 * sizeof(uint32_t); /** @@ -40,7 +40,7 @@ public: * This constructor should be used when receiving a Message, as the * content is filled by the MessageQueue. */ - CommandMessage(); + CommandMessage(MessageQueueMessage* receiverMessage); /** * This constructor creates a new message with all message content * initialized @@ -49,7 +49,7 @@ public: * @param parameter1 The first parameter * @param parameter2 The second parameter */ - CommandMessage(Command_t command, + CommandMessage(MessageQueueMessage* messageToSet, Command_t command, uint32_t parameter1, uint32_t parameter2); /** @@ -58,6 +58,8 @@ public: virtual ~CommandMessage() { } + uint8_t * getBuffer() override; + const uint8_t * getBuffer() const override; /** * Read the DeviceHandlerCommand_t that is stored in the message, * usually used after receiving. @@ -66,6 +68,16 @@ public: */ Command_t getCommand() const; + /** + * @brief This method is used to set the sender's message queue id + * information prior to sending the message. + * @param setId + * The message queue id that identifies the sending message queue. + */ + void setSender(MessageQueueId_t setId) override; + + MessageQueueId_t getSender() const override; + uint8_t getMessageType() const; /** * Set the DeviceHandlerCOmmand_t of the message @@ -102,6 +114,7 @@ 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 @@ -129,7 +142,10 @@ public: void setToUnknownCommand(); void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); - size_t getMinimumMessageSize() const; + size_t getMinimumMessageSize() const override; + +private: + MessageQueueMessage* internalMessage; }; diff --git a/ipc/MessageQueueIF.h b/ipc/MessageQueueIF.h index 480019eb..500f71e2 100644 --- a/ipc/MessageQueueIF.h +++ b/ipc/MessageQueueIF.h @@ -15,7 +15,7 @@ #include class MessageQueueIF { public: - static const MessageQueueId_t NO_QUEUE = MessageQueueSenderIF::NO_QUEUE; //!< Ugly hack. + static const MessageQueueId_t NO_QUEUE = MessageQueueMessageIF::NO_QUEUE; //!< Ugly hack. static const uint8_t INTERFACE_ID = CLASS_ID::MESSAGE_QUEUE_IF; /** @@ -41,7 +41,7 @@ public: * @return RETURN_OK if ok * @return NO_REPLY_PARTNER Should return NO_REPLY_PARTNER if partner was found */ - virtual ReturnValue_t reply( MessageQueueMessage* message ) = 0; + virtual ReturnValue_t reply( MessageQueueMessageIF* message ) = 0; /** * @brief This function reads available messages from the message queue and returns the sender. @@ -50,7 +50,7 @@ public: * @param message A pointer to a message in which the received data is stored. * @param receivedFrom A pointer to a queue id in which the sender's id is stored. */ - virtual ReturnValue_t receiveMessage(MessageQueueMessage* message, + virtual ReturnValue_t receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t *receivedFrom) = 0; /** @@ -65,7 +65,7 @@ public: * @return -@c RETURN_OK on success * -@c MessageQueueIF::EMPTY if queue is empty */ - virtual ReturnValue_t receiveMessage(MessageQueueMessage* message) = 0; + virtual ReturnValue_t receiveMessage(MessageQueueMessageIF* message) = 0; /** * Deletes all pending messages in the queue. * @param count The number of flushed messages. @@ -95,7 +95,7 @@ public: * -@c MessageQueueIF::FULL if queue is full */ virtual ReturnValue_t sendMessageFrom( MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault = false ) = 0; /** @@ -107,7 +107,7 @@ public: * @param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ virtual ReturnValue_t sendMessage( MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault = false ) = 0; + MessageQueueMessageIF* message, bool ignoreFault = false ) = 0; /** * @brief The sendToDefaultFrom method sends a queue message to the default destination. @@ -118,7 +118,7 @@ public: * @return -@c RETURN_OK on success * -@c MessageQueueIF::FULL if queue is full */ - virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessage* message, + virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault = false ) = 0; /** * @brief This operation sends a message to the default destination. @@ -128,7 +128,7 @@ public: * @return -@c RETURN_OK on success * -@c MessageQueueIF::FULL if queue is full */ - virtual ReturnValue_t sendToDefault( MessageQueueMessage* message ) = 0; + virtual ReturnValue_t sendToDefault( MessageQueueMessageIF* message ) = 0; /** * \brief This method is a simple setter for the default destination. */ diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index 544d18e8..c3e1cb7a 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -51,7 +51,7 @@ void MessageQueueMessage::setSender(MessageQueueId_t setId) { memcpy(this->internalBuffer, &setId, sizeof(MessageQueueId_t)); } -size_t MessageQueueMessage::getMinimumMessageSize() { +size_t MessageQueueMessage::getMinimumMessageSize() const { return this->HEADER_SIZE; } diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 93671321..29d1a869 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -132,7 +132,7 @@ public: * @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(); + virtual size_t getMinimumMessageSize() const; }; #endif /* MESSAGEQUEUEMESSAGE_H_ */ diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 70c87d3c..0a6288b0 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -1,10 +1,23 @@ #ifndef FRAMEWORK_IPC_MESSAGEQUEUEMESSAGEIF_H_ #define FRAMEWORK_IPC_MESSAGEQUEUEMESSAGEIF_H_ -#include #include +#include + +/* + * TODO: Actually, the definition of this ID to be a uint32_t is not ideal and + * breaks layering. However, it is difficult to keep layering, as the ID is + * stored in many places and sent around in MessageQueueMessage. + * Ideally, one would use the (current) object_id_t only, however, doing a + * lookup of queueIDs for every call does not sound ideal. + * In a first step, I'll circumvent the issue by not touching it, + * maybe in a second step. This also influences Interface design + * (getCommandQueue) and some other issues.. */ +typedef uint32_t MessageQueueId_t; class MessageQueueMessageIF { public: + static const MessageQueueId_t NO_QUEUE = 0xffffffff; + virtual ~MessageQueueMessageIF() {}; /** @@ -12,11 +25,11 @@ public: * size is set to zero. */ virtual void clear() = 0; - /** - * @brief This is a debug method that prints the content - * (till messageSize) to the debug output. - */ - virtual void print() = 0; +// /** +// * @brief This is a debug method that prints the content +// * (till messageSize) to the debug output. +// */ +// virtual void print() = 0; /** * @brief Get read-only pointer to the raw buffer. @@ -37,11 +50,13 @@ public: */ virtual void setSender(MessageQueueId_t setId) = 0; + virtual MessageQueueId_t getSender() const = 0; + /** * @brief This helper function is used by the MessageQueue class to * check the size of an incoming message. */ - virtual size_t getMinimumMessageSize() = 0; + virtual size_t getMinimumMessageSize() const = 0; }; diff --git a/ipc/MessageQueueSenderIF.h b/ipc/MessageQueueSenderIF.h index ae66cd12..3172454f 100644 --- a/ipc/MessageQueueSenderIF.h +++ b/ipc/MessageQueueSenderIF.h @@ -1,22 +1,11 @@ #ifndef FRAMEWORK_IPC_MESSAGEQUEUESENDERIF_H_ #define FRAMEWORK_IPC_MESSAGEQUEUESENDERIF_H_ +#include #include -class MessageQueueMessage; - - -//TODO: Actually, the definition of this ID to be a uint32_t is not ideal and breaks layering. -//However, it is difficult to keep layering, as the ID is stored in many places and sent around in -//MessageQueueMessage. -//Ideally, one would use the (current) object_id_t only, however, doing a lookup of queueIDs for every -//call does not sound ideal. -//In a first step, I'll circumvent the issue by not touching it, maybe in a second step. -//This also influences Interface design (getCommandQueue) and some other issues.. -typedef uint32_t MessageQueueId_t; class MessageQueueSenderIF { public: - static const MessageQueueId_t NO_QUEUE = 0xffffffff; virtual ~MessageQueueSenderIF() {} @@ -26,7 +15,8 @@ public: * Must be implemented by a subclass. */ static ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom = MessageQueueSenderIF::NO_QUEUE, + MessageQueueMessageIF* message, + MessageQueueId_t sentFrom = MessageQueueMessageIF::NO_QUEUE, bool ignoreFault=false); private: MessageQueueSenderIF() {} diff --git a/memory/MemoryHelper.cpp b/memory/MemoryHelper.cpp index de609252..2c384651 100644 --- a/memory/MemoryHelper.cpp +++ b/memory/MemoryHelper.cpp @@ -51,14 +51,16 @@ void MemoryHelper::completeLoad(ReturnValue_t errorCode, break; default: ipcStore->deleteData(ipcAddress); - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); MemoryMessage::setMemoryReplyFailed(&reply, errorCode, MemoryMessage::CMD_MEMORY_LOAD); queueToUse->sendMessage(lastSender, &reply); return; } //Only reached on success - CommandMessage reply(CommandMessage::REPLY_COMMAND_OK, 0, 0); + MessageQueueMessage message; + CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, 0); queueToUse->sendMessage(lastSender, &reply); ipcStore->deleteData(ipcAddress); } @@ -66,7 +68,8 @@ void MemoryHelper::completeLoad(ReturnValue_t errorCode, void MemoryHelper::completeDump(ReturnValue_t errorCode, const uint8_t* dataToCopy, const uint32_t size) { busy = false; - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); MemoryMessage::setMemoryReplyFailed(&reply, errorCode, lastCommand); switch (errorCode) { case HasMemoryIF::DO_IT_MYSELF: @@ -151,7 +154,8 @@ void MemoryHelper::handleMemoryLoad(CommandMessage* message) { completeLoad(returnCode, p_data, size, dataPointer); } else { //At least inform sender. - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); MemoryMessage::setMemoryReplyFailed(&reply, returnCode, MemoryMessage::CMD_MEMORY_LOAD); queueToUse->sendMessage(lastSender, &reply); @@ -169,7 +173,8 @@ void MemoryHelper::handleMemoryCheckOrDump(CommandMessage* message) { reservedSpaceInIPC); completeDump(returnCode, dataPointer, size); } else { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); MemoryMessage::setMemoryReplyFailed(&reply, returnCode, lastCommand); queueToUse->sendMessage(lastSender, &reply); } diff --git a/modes/ModeHelper.cpp b/modes/ModeHelper.cpp index 66726772..b79082af 100644 --- a/modes/ModeHelper.cpp +++ b/modes/ModeHelper.cpp @@ -11,31 +11,32 @@ ModeHelper::~ModeHelper() { } -ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* message) { - CommandMessage reply; +ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { + MessageQueueMessage message; + CommandMessage reply(&message); Mode_t mode; Submode_t submode; - switch (message->getCommand()) { + switch (command->getCommand()) { case ModeMessage::CMD_MODE_COMMAND_FORCED: forced = true; /* NO BREAK falls through*/ case ModeMessage::CMD_MODE_COMMAND: { - mode = ModeMessage::getMode(message); - submode = ModeMessage::getSubmode(message); + mode = ModeMessage::getMode(command); + submode = ModeMessage::getSubmode(command); uint32_t timeout; ReturnValue_t result = owner->checkModeCommand(mode, submode, &timeout); if (result != HasReturnvaluesIF::RETURN_OK) { ModeMessage::cantReachMode(&reply, result); - MessageQueueSenderIF::sendMessage(message->getSender(), &reply, + MessageQueueSenderIF::sendMessage(command->getSender(), &reply, owner->getCommandQueue()); break; } //Free to start transition - theOneWhoCommandedAMode = message->getSender(); + theOneWhoCommandedAMode = command->getSender(); commandedMode = mode; commandedSubmode = submode; - if ((parentQueueId != MessageQueueSenderIF::NO_QUEUE) + if ((parentQueueId != MessageQueueMessageIF::NO_QUEUE) && (theOneWhoCommandedAMode != parentQueueId)) { owner->setToExternalControl(); } @@ -48,7 +49,7 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* message) { owner->getMode(&mode, &submode); ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_REPLY, mode, submode); - MessageQueueSenderIF::sendMessage(message->getSender(), &reply, + MessageQueueSenderIF::sendMessage(command->getSender(), &reply, owner->getCommandQueue()); } break; @@ -73,13 +74,14 @@ void ModeHelper::modeChanged(Mode_t ownerMode, Submode_t ownerSubmode) { forced = false; sendModeReplyMessage(ownerMode, ownerSubmode); sendModeInfoMessage(ownerMode, ownerSubmode); - theOneWhoCommandedAMode = MessageQueueSenderIF::NO_QUEUE; + theOneWhoCommandedAMode = MessageQueueMessageIF::NO_QUEUE; } void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, Submode_t ownerSubmode) { - CommandMessage reply; - if (theOneWhoCommandedAMode != MessageQueueSenderIF::NO_QUEUE) + MessageQueueMessage message; + CommandMessage reply(&message); + if (theOneWhoCommandedAMode != MessageQueueMessageIF::NO_QUEUE) { if (ownerMode != commandedMode or ownerSubmode != commandedSubmode) { @@ -99,9 +101,10 @@ void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, void ModeHelper::sendModeInfoMessage(Mode_t ownerMode, Submode_t ownerSubmode) { - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); if (theOneWhoCommandedAMode != parentQueueId - and parentQueueId != MessageQueueSenderIF::NO_QUEUE) + and parentQueueId != MessageQueueMessageIF::NO_QUEUE) { ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_INFO, ownerMode, ownerSubmode); diff --git a/monitoring/LimitViolationReporter.cpp b/monitoring/LimitViolationReporter.cpp index 234a0bff..1378d754 100644 --- a/monitoring/LimitViolationReporter.cpp +++ b/monitoring/LimitViolationReporter.cpp @@ -31,7 +31,8 @@ ReturnValue_t LimitViolationReporter::sendLimitViolationReport(const SerializeIF if (result != HasReturnvaluesIF::RETURN_OK) { return result; } - CommandMessage report; + MessageQueueMessage message; + CommandMessage report(&message); MonitoringMessage::setLimitViolationReport(&report, storeId); return MessageQueueSenderIF::sendMessage(reportQueue, &report); } diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index 0a579296..8c6ec80b 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -25,21 +25,21 @@ void MessageQueue::switchSystemContext(CallContext callContext) { } ReturnValue_t MessageQueue::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault) { + MessageQueueMessageIF* message, bool ignoreFault) { return sendMessageFrom(sendTo, message, this->getId(), ignoreFault); } -ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessageIF* message) { return sendToDefaultFrom(message, this->getId()); } -ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessage* message, +ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFrom(defaultDestination,message,sentFrom,ignoreFault); } -ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { - if (this->lastPartner != 0) { +ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { + if (this->lastPartner != MessageQueueMessageIF::NO_QUEUE) { return sendMessageFrom(this->lastPartner, message, this->getId()); } else { return NO_REPLY_PARTNER; @@ -47,7 +47,7 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { } ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFromMessageQueue(sendTo, message, sentFrom, ignoreFault, callContext); @@ -69,7 +69,7 @@ ReturnValue_t MessageQueue::handleSendResult(BaseType_t result, bool ignoreFault return HasReturnvaluesIF::RETURN_OK; } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t* receivedFrom) { ReturnValue_t status = this->receiveMessage(message); if(status == HasReturnvaluesIF::RETURN_OK) { @@ -78,7 +78,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, return status; } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { BaseType_t result = xQueueReceive(handle,reinterpret_cast( message->getBuffer()), 0); if (result == pdPASS){ @@ -120,7 +120,7 @@ bool MessageQueue::isDefaultDestinationSet() const { // static core function to send messages. ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessage *message, MessageQueueId_t sentFrom, + MessageQueueMessageIF *message, MessageQueueId_t sentFrom, bool ignoreFault, CallContext callContext) { message->setSender(sentFrom); BaseType_t result; diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index 81b4c186..6165c1cf 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -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, - MessageQueueMessage* message, bool ignoreFault = false ); + MessageQueueMessageIF* message, bool ignoreFault = false ); /** * @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( MessageQueueMessage* message ); + ReturnValue_t sendToDefault( MessageQueueMessageIF* message ); /** * @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( MessageQueueMessage* message ); + ReturnValue_t reply( MessageQueueMessageIF* message ); /** * @brief With the sendMessage call, a queue message is sent to a receiving queue. @@ -113,8 +113,9 @@ 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, MessageQueueMessage* 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 ); /** * @brief The sendToDefault method sends a queue message to the default destination. @@ -123,7 +124,7 @@ public: * @param sentFrom The sentFrom information can be set to inject the sender's queue id into the message. * This variable is set to zero by default. */ - virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessage* message, + virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false ); /** @@ -133,7 +134,7 @@ public: * @param message A pointer to a message in which the received data is stored. * @param receivedFrom A pointer to a queue id in which the sender's id is stored. */ - ReturnValue_t receiveMessage(MessageQueueMessage* message, + ReturnValue_t receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t *receivedFrom); /** @@ -144,7 +145,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(MessageQueueMessage* message); + ReturnValue_t receiveMessage(MessageQueueMessageIF* message); /** * Deletes all pending messages in the queue. * @param count The number of flushed messages. @@ -191,7 +192,7 @@ protected: * @param context Specify whether call is made from task or from an ISR. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom = NO_QUEUE, + 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 372d0d41..beb4969b 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -7,7 +7,7 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + 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 e0756882..8b91020f 100644 --- a/parameters/ParameterHelper.cpp +++ b/parameters/ParameterHelper.cpp @@ -102,7 +102,8 @@ ReturnValue_t ParameterHelper::sendParameter(MessageQueueId_t to, uint32_t id, return result; } - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ParameterMessage::setParameterDumpReply(&reply, id, address); @@ -123,8 +124,10 @@ ReturnValue_t ParameterHelper::initialize() { } } -void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, Command_t initialCommand) { - CommandMessage reply; +void ParameterHelper::rejectCommand(MessageQueueId_t to, ReturnValue_t reason, + Command_t initialCommand) { + MessageQueueMessage message; + CommandMessage reply(&message); reply.setReplyRejected(reason, initialCommand); MessageQueueSenderIF::sendMessage(to, &reply, ownerQueueId); } diff --git a/power/Fuse.cpp b/power/Fuse.cpp index eee91984..a80a111f 100644 --- a/power/Fuse.cpp +++ b/power/Fuse.cpp @@ -163,7 +163,8 @@ void Fuse::setAllMonitorsToUnchecked() { } void Fuse::checkCommandQueue() { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != HasReturnvaluesIF::RETURN_OK) { return; diff --git a/power/PowerSensor.cpp b/power/PowerSensor.cpp index 5433acc9..b1aea270 100644 --- a/power/PowerSensor.cpp +++ b/power/PowerSensor.cpp @@ -74,7 +74,8 @@ void PowerSensor::setAllMonitorsToUnchecked() { } void PowerSensor::checkCommandQueue() { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); ReturnValue_t result = commandQueue->receiveMessage(&command); if (result != HasReturnvaluesIF::RETURN_OK) { return; diff --git a/subsystem/Subsystem.cpp b/subsystem/Subsystem.cpp index 52f2c474..d774b4d5 100644 --- a/subsystem/Subsystem.cpp +++ b/subsystem/Subsystem.cpp @@ -309,7 +309,8 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { break; case ModeSequenceMessage::READ_FREE_SEQUENCE_SLOTS: { uint32_t freeSlots = modeSequences.maxSize() - modeSequences.size(); - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ModeSequenceMessage::setModeSequenceMessage(&reply, ModeSequenceMessage::FREE_SEQUENCE_SLOTS, freeSlots); commandQueue->reply(&reply); @@ -317,7 +318,8 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { break; case ModeSequenceMessage::READ_FREE_TABLE_SLOTS: { uint32_t free = modeTables.maxSize() - modeTables.size(); - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); ModeSequenceMessage::setModeSequenceMessage(&reply, ModeSequenceMessage::FREE_TABLE_SLOTS, free); commandQueue->reply(&reply); @@ -330,11 +332,12 @@ ReturnValue_t Subsystem::handleCommandMessage(CommandMessage* message) { } void Subsystem::replyToCommand(ReturnValue_t status, uint32_t parameter) { + MessageQueueMessage message; if (status == RETURN_OK) { - CommandMessage reply(CommandMessage::REPLY_COMMAND_OK, 0, 0); + CommandMessage reply(&message, CommandMessage::REPLY_COMMAND_OK, 0, 0); commandQueue->reply(&reply); } else { - CommandMessage reply(CommandMessage::REPLY_REJECTED, status, 0); + CommandMessage reply(&message, CommandMessage::REPLY_REJECTED, status, 0); commandQueue->reply(&reply); } } @@ -617,7 +620,8 @@ void Subsystem::sendSerializablesAsCommandMessage(Command_t command, for (uint8_t i = 0; i < count; i++) { elements[i]->serialize(&storeBuffer, &size, maxSize, true); } - CommandMessage reply; + MessageQueueMessage message; + CommandMessage reply(&message); 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 6df0b64f..5527f215 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -9,7 +9,7 @@ SubsystemBase::SubsystemBase(object_id_t setObjectId, object_id_t parent, false), commandsOutstanding(0), commandQueue(NULL), healthHelper(this, setObjectId), modeHelper(this), parentId(parent) { commandQueue = QueueFactory::instance()->createMessageQueue(commandQueueDepth, - CommandMessage::MAX_MESSAGE_SIZE); + MessageQueueMessage::MAX_MESSAGE_SIZE); } SubsystemBase::~SubsystemBase() { @@ -77,8 +77,8 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable( } void SubsystemBase::executeTable(HybridIterator tableIter, Submode_t targetSubmode) { - - CommandMessage message; + MessageQueueMessage message; + CommandMessage command(&message); std::map::iterator iter; @@ -100,17 +100,17 @@ void SubsystemBase::executeTable(HybridIterator tableIter, Submod if (healthHelper.healthTable->hasHealth(object)) { if (healthHelper.healthTable->isFaulty(object)) { - ModeMessage::setModeMessage(&message, + ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND, HasModesIF::MODE_OFF, SUBMODE_NONE); } else { if (modeHelper.isForced()) { - ModeMessage::setModeMessage(&message, + ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND_FORCED, tableIter.value->getMode(), submodeToCommand); } else { if (healthHelper.healthTable->isCommandable(object)) { - ModeMessage::setModeMessage(&message, + ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND, tableIter.value->getMode(), submodeToCommand); } else { @@ -119,12 +119,12 @@ void SubsystemBase::executeTable(HybridIterator tableIter, Submod } } } else { - ModeMessage::setModeMessage(&message, ModeMessage::CMD_MODE_COMMAND, + ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_COMMAND, tableIter.value->getMode(), submodeToCommand); } - if ((iter->second.mode == ModeMessage::getMode(&message)) - && (iter->second.submode == ModeMessage::getSubmode(&message)) + if ((iter->second.mode == ModeMessage::getMode(&command)) + && (iter->second.submode == ModeMessage::getSubmode(&command)) && !modeHelper.isForced()) { continue; //don't send redundant mode commands (produces event spam), but still command if mode is forced to reach lower levels } @@ -297,7 +297,8 @@ void SubsystemBase::setToExternalControl() { void SubsystemBase::announceMode(bool recursive) { triggerEvent(MODE_INFO, mode, submode); if (recursive) { - CommandMessage command; + MessageQueueMessage message; + CommandMessage command(&message); ModeMessage::setModeMessage(&command, ModeMessage::CMD_MODE_ANNOUNCE_RECURSIVELY, 0, 0); commandAllChildren(&command); @@ -306,31 +307,33 @@ void SubsystemBase::announceMode(bool recursive) { void SubsystemBase::checkCommandQueue() { ReturnValue_t result; - CommandMessage message; + MessageQueueMessage message; + CommandMessage command(&message); - for (result = commandQueue->receiveMessage(&message); result == RETURN_OK; - result = commandQueue->receiveMessage(&message)) { + for (result = commandQueue->receiveMessage(&command); result == RETURN_OK; + result = commandQueue->receiveMessage(&command)) { - result = healthHelper.handleHealthCommand(&message); + result = healthHelper.handleHealthCommand(&command); if (result == RETURN_OK) { continue; } - result = modeHelper.handleModeCommand(&message); + result = modeHelper.handleModeCommand(&command); if (result == RETURN_OK) { continue; } - result = handleModeReply(&message); + result = handleModeReply(&command); if (result == RETURN_OK) { continue; } - result = handleCommandMessage(&message); + result = handleCommandMessage(&command); if (result != RETURN_OK) { - CommandMessage reply; - reply.setReplyRejected(CommandMessage::UNKNOW_COMMAND, - message.getCommand()); + MessageQueueMessage message; + CommandMessage reply(&message); + reply.setReplyRejected(CommandMessage::UNKNOWN_COMMAND, + command.getCommand()); replyToCommand(&reply); } } diff --git a/thermal/AbstractTemperatureSensor.cpp b/thermal/AbstractTemperatureSensor.cpp index be143410..759af87e 100644 --- a/thermal/AbstractTemperatureSensor.cpp +++ b/thermal/AbstractTemperatureSensor.cpp @@ -44,18 +44,19 @@ ReturnValue_t AbstractTemperatureSensor::performHealthOp() { } void AbstractTemperatureSensor::handleCommandQueue() { - CommandMessage message; - ReturnValue_t result = commandQueue->receiveMessage(&message); + MessageQueueMessage message; + CommandMessage command(&message); + ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { - result = healthHelper.handleHealthCommand(&message); + result = healthHelper.handleHealthCommand(&command); if (result == HasReturnvaluesIF::RETURN_OK) { return; } - result = parameterHelper.handleParameterMessage(&message); + result = parameterHelper.handleParameterMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { return; } - message.setToUnknownCommand(); + command.setToUnknownCommand(); commandQueue->reply(&message); } } diff --git a/thermal/Heater.cpp b/thermal/Heater.cpp index 29daa15d..6e191fc1 100644 --- a/thermal/Heater.cpp +++ b/thermal/Heater.cpp @@ -279,14 +279,15 @@ ReturnValue_t Heater::initialize() { } void Heater::handleQueue() { - CommandMessage message; - ReturnValue_t result = commandQueue->receiveMessage(&message); + MessageQueueMessage message; + CommandMessage command(&message); + ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { - result = healthHelper.handleHealthCommand(&message); + result = healthHelper.handleHealthCommand(&command); if (result == HasReturnvaluesIF::RETURN_OK) { return; } - parameterHelper.handleParameterMessage(&message); + parameterHelper.handleParameterMessage(&command); } } diff --git a/tmstorage/TmStoreMessage.cpp b/tmstorage/TmStoreMessage.cpp index 4509ba91..669ecc0e 100644 --- a/tmstorage/TmStoreMessage.cpp +++ b/tmstorage/TmStoreMessage.cpp @@ -74,7 +74,7 @@ void TmStoreMessage::clear(CommandMessage* cmd) { case DELETE_STORE_CONTENT_BLOCKS: case DOWNLINK_STORE_CONTENT_BLOCKS: case REPORT_INDEX_REQUEST: - cmd->setCommand(UNKNOW_COMMAND); + cmd->setCommand(CommandMessage::UNKNOWN_COMMAND); cmd->setParameter(0); cmd->setParameter2(0); break; diff --git a/tmstorage/TmStoreMessage.h b/tmstorage/TmStoreMessage.h index bd6b2def..93c23e40 100644 --- a/tmstorage/TmStoreMessage.h +++ b/tmstorage/TmStoreMessage.h @@ -5,7 +5,7 @@ #include #include #include -class TmStoreMessage: public CommandMessage { +class TmStoreMessage { public: static ReturnValue_t setEnableStoringMessage(CommandMessage* cmd, bool setEnabled); diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index adc9400f..a4fc1110 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -77,7 +77,10 @@ ReturnValue_t CommandingServiceBase::initialize() { } void CommandingServiceBase::handleCommandQueue() { - CommandMessage reply, nextCommand; + MessageQueueMessage replyMessage; + CommandMessage reply(&replyMessage); + MessageQueueMessage nextCommandMessage; + CommandMessage nextCommand(&nextCommandMessage); ReturnValue_t result, sendResult = RETURN_OK; bool isStep = false; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; @@ -268,16 +271,17 @@ void CommandingServiceBase::startExecution( typename FixedMap::Iterator *iter) { ReturnValue_t result, sendResult = RETURN_OK; - CommandMessage message; + MessageQueueMessage message; + CommandMessage command(&message); (*iter)->subservice = storedPacket->getSubService(); - result = prepareCommand(&message, (*iter)->subservice, + result = prepareCommand(&command, (*iter)->subservice, storedPacket->getApplicationData(), storedPacket->getApplicationDataSize(), &(*iter)->state, (*iter)->objectId); switch (result) { case RETURN_OK: - if (message.getCommand() != CommandMessage::CMD_NONE) { + if (command.getCommand() != CommandMessage::CMD_NONE) { sendResult = commandQueue->sendMessage((*iter).value->first, &message); } @@ -286,20 +290,20 @@ void CommandingServiceBase::startExecution( (*iter)->step = 0; // (*iter)->state = 0; (*iter)->subservice = storedPacket->getSubService(); - (*iter)->command = message.getCommand(); + (*iter)->command = command.getCommand(); (*iter)->tcInfo.ackFlags = storedPacket->getAcknowledgeFlags(); (*iter)->tcInfo.tcPacketId = storedPacket->getPacketId(); (*iter)->tcInfo.tcSequenceControl = storedPacket->getPacketSequenceControl(); acceptPacket(TC_VERIFY::START_SUCCESS, storedPacket); } else { - message.clearCommandMessage(); + command.clearCommandMessage(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } break; case EXECUTION_COMPLETE: - if (message.getCommand() != CommandMessage::CMD_NONE) { + if (command.getCommand() != CommandMessage::CMD_NONE) { //Fire-and-forget command. sendResult = commandQueue->sendMessage((*iter).value->first, &message); @@ -310,7 +314,7 @@ void CommandingServiceBase::startExecution( acceptPacket(TC_VERIFY::COMPLETION_SUCCESS, storedPacket); checkAndExecuteFifo(iter); } else { - message.clearCommandMessage(); + command.clearCommandMessage(); rejectPacket(TC_VERIFY::START_FAILURE, storedPacket, sendResult); checkAndExecuteFifo(iter); } From 47aa9ddcc382393279c0a769e51e7d732cf25a35 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 9 Jun 2020 02:46:19 +0200 Subject: [PATCH 02/12] doc for mqm improved --- ipc/CommandMessage.cpp | 8 +++ ipc/CommandMessage.h | 56 +++++++++++-------- ipc/MessageQueueMessage.h | 107 +++++++++++++++++++++--------------- ipc/MessageQueueMessageIF.h | 28 +++++++--- 4 files changed, 127 insertions(+), 72 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index cd75aee4..44424e02 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -152,3 +152,11 @@ void CommandMessage::setSender(MessageQueueId_t 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(); +} diff --git a/ipc/CommandMessage.h b/ipc/CommandMessage.h index f6fd86cb..772bd774 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -1,16 +1,25 @@ #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; /** - * @brief Used to pass command messages between tasks + * @brief Used to pass command messages between tasks. Primary message type + * for IPC. Contains sender, 2-byte command field, and 2 4-byte + * parameters. + * @details + * It operates on an external memory which is contained inside a + * MessageQueueMessage 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. * @author Bastian Baetz */ class CommandMessage: public MessageQueueMessageIF { @@ -53,13 +62,10 @@ public: uint32_t parameter1, uint32_t parameter2); /** - * Default Destructor + * @brief Default Destructor */ - virtual ~CommandMessage() { - } + virtual ~CommandMessage() {} - uint8_t * getBuffer() override; - const uint8_t * getBuffer() const override; /** * Read the DeviceHandlerCommand_t that is stored in the message, * usually used after receiving. @@ -68,53 +74,54 @@ public: */ Command_t getCommand() const; - /** - * @brief This method is used to set the sender's message queue id - * information prior to sending the message. - * @param setId - * The message queue id that identifies the sending message queue. + /* + * 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; + size_t getMinimumMessageSize() const override; + /** + * Extract message ID, which is the first byte of the command ID. + * @return + */ uint8_t getMessageType() const; /** - * Set the DeviceHandlerCOmmand_t of the message - * + * 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 */ uint32_t getParameter() const; /** * Set the first parameter of the message - * * @param the first parameter of the message */ void setParameter(uint32_t parameter1); /** * Get the second parameter of the message - * * @return the second Parameter of the message */ uint32_t getParameter2() const; /** * Set the second parameter of the message - * * @param the second parameter of the message */ 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 @@ -126,6 +133,7 @@ public: * */ void clearCommandMessage(); + void clear() override; /** * check if a message was cleared @@ -134,7 +142,6 @@ public: */ bool isClearedCommandMessage(); - /** * Sets the command to REPLY_REJECTED with parameter UNKNOWN_COMMAND. * Is needed quite often, so we better code it once only. @@ -142,9 +149,14 @@ public: void setToUnknownCommand(); void setReplyRejected(ReturnValue_t reason, Command_t initialCommand = CMD_NONE); - size_t getMinimumMessageSize() const override; 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/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index 29d1a869..bee39ae6 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -1,5 +1,5 @@ -#ifndef MESSAGEQUEUEMESSAGE_H_ -#define MESSAGEQUEUEMESSAGE_H_ +#ifndef FRAMEWORK_IPC_MESSAGEQUEUEMESSAGE_H_ +#define FRAMEWORK_IPC_MESSAGEQUEUEMESSAGE_H_ #include #include @@ -8,7 +8,6 @@ /** * @brief This class is the representation and data organizer * for interprocess messages. - * * @details * To facilitate and standardize interprocess communication, this class was * created to handle a lightweight "interprocess message protocol". @@ -28,12 +27,14 @@ class MessageQueueMessage: public MessageQueueMessageIF { public: /** * @brief The class is initialized empty with this constructor. - * @details The messageSize attribute is set to the header's size and the - * whole content is set to zero. + * @details + * The messageSize attribute is set to the header's size and the whole + * content is set to zero. */ MessageQueueMessage(); /** - * @brief With this constructor the class is initialized with the given content. + * @brief With this constructor the class is initialized with + * the given content. * @details * If the passed message size fits into the buffer, the passed data is * copied to the internal buffer and the messageSize information is set. @@ -46,7 +47,14 @@ public: MessageQueueMessage(uint8_t* data, size_t size); /** - * @brief The size information of each message is stored in this attribute. + * @brief As no memory is allocated in this class, + * the destructor is empty. + */ + virtual ~MessageQueueMessage(); + + /** + * @brief The size information of each message is stored in + * this attribute. * @details * It is public to simplify usage and to allow for passing the size * address as a pointer. Care must be taken when inheriting from this class, @@ -59,19 +67,26 @@ public: */ size_t messageSize; /** - * @brief This constant defines the maximum size of the data content, excluding the header. - * @details It may be changed if necessary, but in general should be kept as small as possible. + * @brief This constant defines the maximum size of the data content, + * excluding the header. + * @details + * It may be changed if necessary, but in general should be kept + * 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. + * @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. - * @details It is the sum of the maximum data and the header size. Be aware that this constant - * is used to define the buffer sizes for every message queue in the system. So, a change - * here may have significant impact on the required resources. + * @brief This constant defines the maximum total size in bytes + * of a sent message. + * @details + * It is the sum of the maximum data and the header size. Be aware that + * this constant is used to define the buffer sizes for every message + * 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; /** @@ -81,58 +96,64 @@ public: static const size_t MIN_MESSAGE_SIZE = HEADER_SIZE; private: /** - * @brief This is the internal buffer that contains the actual message data. + * @brief This is the internal buffer that contains the + * actual message data. */ uint8_t internalBuffer[MAX_MESSAGE_SIZE]; public: /** - * @brief As no memory is allocated in this class, the destructor is empty. + * @brief This method is used to get the complete data of the message. */ - virtual ~MessageQueueMessage(); + const uint8_t* getBuffer() const override; /** * @brief This method is used to get the complete data of the message. */ - const uint8_t* getBuffer() const; - /** - * @brief This method is used to get the complete data of the message. - */ - uint8_t* getBuffer(); + uint8_t* getBuffer() override; /** * @brief This method is used to fetch the data content of the message. - * @details It shall be used by child classes to add data at the right position. + * @details + * It shall be used by child classes to add data at the right position. */ - const uint8_t* getData() const; + const uint8_t* getData() const override; /** * @brief This method is used to fetch the data content of the message. - * @details It shall be used by child classes to add data at the right position. + * @details + * It shall be used by child classes to add data at the right position. */ - uint8_t* getData(); + uint8_t* getData() override; /** - * @brief This method is used to extract the sender's message queue id information from a - * received message. + * @brief This method is used to extract the sender's message + * queue id information from a received message. */ - MessageQueueId_t getSender() const; + MessageQueueId_t getSender() const override; /** - * @brief With this method, the whole content and the message size is set to zero. + * @brief With this method, the whole content + * and the message size is set to zero. */ - void clear(); + void clear() override; + /** - * @brief This is a debug method that prints the content (till messageSize) to the debug output. + * @brief This method is used to set the sender's message queue id + * information prior to ing the message. + * @param setId + * The message queue id that identifies the sending message queue. */ - void print(); + void setSender(MessageQueueId_t setId) override; /** - * @brief This method is used to set the sender's message queue id information prior to - * sending the message. - * @param setId The message queue id that identifies the sending message queue. - */ - void setSender(MessageQueueId_t setId); - /** - * @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. + * @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; + virtual size_t getMinimumMessageSize() const override; + + /** + * @brief This is a debug method that prints the content + * (till messageSize) to the debug output. + */ + void print(); }; #endif /* MESSAGEQUEUEMESSAGE_H_ */ diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 0a6288b0..197a26fd 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -11,7 +11,9 @@ * lookup of queueIDs for every call does not sound ideal. * In a first step, I'll circumvent the issue by not touching it, * maybe in a second step. This also influences Interface design - * (getCommandQueue) and some other issues.. */ + * (getCommandQueue) and some other issues.. + */ + typedef uint32_t MessageQueueId_t; class MessageQueueMessageIF { @@ -25,14 +27,9 @@ public: * size is set to zero. */ virtual void clear() = 0; -// /** -// * @brief This is a debug method that prints the content -// * (till messageSize) to the debug output. -// */ -// virtual void print() = 0; /** - * @brief Get read-only pointer to the raw buffer. + * @brief Get read-only pointer to the complete data of the message. * @return */ virtual const uint8_t* getBuffer() const = 0; @@ -50,8 +47,25 @@ public: */ virtual void setSender(MessageQueueId_t setId) = 0; + /** + * @brief This method is used to extract the sender's message queue id + * information from a received message. + */ virtual MessageQueueId_t getSender() const = 0; + /** + * @brief This method is used to fetch the data content of the message. + * @details + * It shall be used by child classes to add data at the right position. + */ + virtual const uint8_t* getData() const = 0; + /** + * @brief This method is used to fetch the data content of the message. + * @details + * It shall be used by child classes to add data at the right position. + */ + virtual uint8_t* getData() = 0; + /** * @brief This helper function is used by the MessageQueue class to * check the size of an incoming message. From b6f6a61656ac423b2411e4c4ab187d54d0b18b1b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 20:51:59 +0200 Subject: [PATCH 03/12] iomanip include missing --- objectmanager/ObjectManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/objectmanager/ObjectManager.cpp b/objectmanager/ObjectManager.cpp index cfdf6562..1c54355a 100644 --- a/objectmanager/ObjectManager.cpp +++ b/objectmanager/ObjectManager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include ObjectManager::ObjectManager( void (*setProducer)() ): From 52c887925fe68f0729db72925c24e3b703ca600b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 20:53:28 +0200 Subject: [PATCH 04/12] iomanip include --- objectmanager/ObjectManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/objectmanager/ObjectManager.cpp b/objectmanager/ObjectManager.cpp index cfdf6562..1c54355a 100644 --- a/objectmanager/ObjectManager.cpp +++ b/objectmanager/ObjectManager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include ObjectManager::ObjectManager( void (*setProducer)() ): From 606957dc243f31d18d80f51885f4684fc134186b Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 21:26:45 +0200 Subject: [PATCH 05/12] PSB update --- tmtcservices/PusServiceBase.cpp | 74 +++++++++++++++++++-------------- tmtcservices/PusServiceBase.h | 8 ++-- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/tmtcservices/PusServiceBase.cpp b/tmtcservices/PusServiceBase.cpp index c27defe6..f5d1e30b 100644 --- a/tmtcservices/PusServiceBase.cpp +++ b/tmtcservices/PusServiceBase.cpp @@ -9,10 +9,11 @@ object_id_t PusServiceBase::packetSource = 0; object_id_t PusServiceBase::packetDestination = 0; -PusServiceBase::PusServiceBase(object_id_t setObjectId, uint16_t setApid, uint8_t setServiceId) : - SystemObject(setObjectId), apid(setApid), serviceId(setServiceId), errorParameter1( - 0), errorParameter2(0), requestQueue(NULL) { - requestQueue = QueueFactory::instance()->createMessageQueue(PUS_SERVICE_MAX_RECEPTION); +PusServiceBase::PusServiceBase(object_id_t setObjectId, uint16_t setApid, + uint8_t setServiceId) : + SystemObject(setObjectId), apid(setApid), serviceId(setServiceId) { + requestQueue = QueueFactory::instance()-> + createMessageQueue(PUS_SERVICE_MAX_RECEPTION); } PusServiceBase::~PusServiceBase() { @@ -20,51 +21,60 @@ PusServiceBase::~PusServiceBase() { } ReturnValue_t PusServiceBase::performOperation(uint8_t opCode) { + handleRequestQueue(); + ReturnValue_t result = this->performService(); + if (result != RETURN_OK) { + sif::error << "PusService " << (uint16_t) this->serviceId + << ": performService returned with " << (int16_t) result + << std::endl; + return RETURN_FAILED; + } + return RETURN_OK; +} + +void PusServiceBase::handleRequestQueue() { TmTcMessage message; + ReturnValue_t result = RETURN_FAILED; for (uint8_t count = 0; count < PUS_SERVICE_MAX_RECEPTION; count++) { ReturnValue_t status = this->requestQueue->receiveMessage(&message); - // debug << "PusServiceBase::performOperation: Receiving from MQ ID: " << std::hex << this->requestQueue.getId() - // << std::dec << " returned: " << status << std::endl; + // debug << "PusServiceBase::performOperation: Receiving from MQ ID: " + // << std::hex << this->requestQueue.getId() + // << std::dec << " returned: " << status << std::endl; if (status == RETURN_OK) { this->currentPacket.setStoreAddress(message.getStorageId()); - // info << "Service " << (uint16_t) this->serviceId << ": new packet!" << std::endl; + //info << "Service " << (uint16_t) this->serviceId << + // ": new packet!" << std::endl; - ReturnValue_t return_code = this->handleRequest(currentPacket.getSubService()); + result = this->handleRequest(currentPacket.getSubService()); - // debug << "Service " << (uint16_t)this->serviceId << ": handleRequest returned: " << (int)return_code << std::endl; - if (return_code == RETURN_OK) { + // debug << "Service " << (uint16_t)this->serviceId << + // ": handleRequest returned: " << (int)return_code << std::endl; + if (result == RETURN_OK) { this->verifyReporter.sendSuccessReport( TC_VERIFY::COMPLETION_SUCCESS, &this->currentPacket); - } else { + } + else { this->verifyReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, &this->currentPacket, - return_code, 0, errorParameter1, errorParameter2); + result, 0, errorParameter1, errorParameter2); } this->currentPacket.deletePacket(); errorParameter1 = 0; errorParameter2 = 0; - } else if (status == MessageQueueIF::EMPTY) { + } + else if (status == MessageQueueIF::EMPTY) { status = RETURN_OK; - // debug << "PusService " << (uint16_t)this->serviceId << ": no new packet." << std::endl; + // debug << "PusService " << (uint16_t)this->serviceId << + // ": no new packet." << std::endl; break; - } else { - + } + else { sif::error << "PusServiceBase::performOperation: Service " << (uint16_t) this->serviceId << ": Error receiving packet. Code: " << std::hex << status << std::dec << std::endl; } } - ReturnValue_t return_code = this->performService(); - if (return_code == RETURN_OK) { - return RETURN_OK; - } else { - - sif::error << "PusService " << (uint16_t) this->serviceId - << ": performService returned with " << (int16_t) return_code - << std::endl; - return RETURN_FAILED; - } } uint16_t PusServiceBase::getIdentifier() { @@ -80,19 +90,21 @@ ReturnValue_t PusServiceBase::initialize() { if (result != RETURN_OK) { return result; } - AcceptsTelemetryIF* dest_service = objectManager->get( + AcceptsTelemetryIF* destService = objectManager->get( packetDestination); PUSDistributorIF* distributor = objectManager->get( packetSource); - if ((dest_service != NULL) && (distributor != NULL)) { + if ((destService != nullptr) && (distributor != nullptr)) { this->requestQueue->setDefaultDestination( - dest_service->getReportReceptionQueue()); + destService->getReportReceptionQueue()); distributor->registerService(this); return RETURN_OK; - } else { + } + else { sif::error << "PusServiceBase::PusServiceBase: Service " << (uint32_t) this->serviceId << ": Configuration error." - << " Make sure packetSource and packetDestination are defined correctly" << std::endl; + << " Make sure packetSource and packetDestination are defined " + "correctly" << std::endl; return RETURN_FAILED; } } diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index dbb1754c..5f741410 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -97,16 +97,16 @@ protected: /** * One of two error parameters for additional error information. */ - uint32_t errorParameter1; + uint32_t errorParameter1 = 0; /** * One of two error parameters for additional error information. */ - uint32_t errorParameter2; + uint32_t errorParameter2 = 0; /** * This is a complete instance of the Telecommand reception queue of the class. * It is initialized on construction of the class. */ - MessageQueueIF* requestQueue; + MessageQueueIF* requestQueue = nullptr; /** * An instance of the VerificationReporter class, that simplifies sending any kind of * Verification Message to the TC Verification Service. @@ -127,6 +127,8 @@ private: * Remember that one packet must be completely handled in one #handleRequest call. */ static const uint8_t PUS_SERVICE_MAX_RECEPTION = 10; + + void handleRequestQueue(); }; #endif /* PUSSERVICEBASE_H_ */ From 659594bac71505492c01feff182a2577f79fb1fe Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 21:35:47 +0200 Subject: [PATCH 06/12] better include guard and doc form improvement --- tmtcservices/PusServiceBase.h | 80 +++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/tmtcservices/PusServiceBase.h b/tmtcservices/PusServiceBase.h index 5f741410..39600c41 100644 --- a/tmtcservices/PusServiceBase.h +++ b/tmtcservices/PusServiceBase.h @@ -1,5 +1,5 @@ -#ifndef PUSSERVICEBASE_H_ -#define PUSSERVICEBASE_H_ +#ifndef FRAMEWORK_TMTCSERVICES_PUSSERVICEBASE_H_ +#define FRAMEWORK_TMTCSERVICES_PUSSERVICEBASE_H_ #include #include @@ -9,7 +9,6 @@ #include #include #include -#include #include namespace Factory{ @@ -17,18 +16,22 @@ void setStaticFrameworkObjectIds(); } /** - * \defgroup pus_services PUS Service Framework + * @defgroup pus_services PUS Service Framework * These group contains all implementations of PUS Services in the OBSW. * Most of the Services are directly taken from the ECSS PUS Standard. */ /** - * \brief This class is the basis for all PUS Services, which can immediately process Telecommand Packets. - * It manages Telecommand reception and the generation of Verification Reports. Every class that inherits - * from this abstract class has to implement handleRequest and performService. Services that are created with this + * @brief This class is the basis for all PUS Services, + * which can immediately process Telecommand Packets. + * @details + * It manages Telecommand reception and the generation of Verification Reports. + * Every class that inherits from this abstract class has to implement + * handleRequest and performService. Services that are created with this * Base class have to handle any kind of request immediately on reception. - * All PUS Services are System Objects, so an Object ID needs to be specified on construction. - * \ingroup pus_services + * All PUS Services are System Objects, so an Object ID needs to be specified + * on construction. + * @ingroup pus_services */ class PusServiceBase : public ExecutableObjectIF, public AcceptsTelecommandsIF, @@ -37,49 +40,61 @@ class PusServiceBase : public ExecutableObjectIF, friend void (Factory::setStaticFrameworkObjectIds)(); public: /** - * The constructor for the class. - * The passed values are set, but inter-object initialization is done in the initialize method. - * @param setObjectId The system object identifier of this Service instance. - * @param set_apid The APID the Service is instantiated for. - * @param set_service_id The Service Identifier as specified in ECSS PUS. + * @brief The passed values are set, but inter-object initialization is + * done in the initialize method. + * @param setObjectId + * The system object identifier of this Service instance. + * @param setApid + * The APID the Service is instantiated for. + * @param setServiceId + * The Service Identifier as specified in ECSS PUS. */ - PusServiceBase( object_id_t setObjectId, uint16_t setApid, uint8_t setServiceId); + PusServiceBase( object_id_t setObjectId, uint16_t setApid, + uint8_t setServiceId); /** * The destructor is empty. */ virtual ~PusServiceBase(); /** - * @brief The handleRequest method shall handle any kind of Telecommand Request immediately. + * @brief The handleRequest method shall handle any kind of Telecommand + * Request immediately. * @details - * Implemetations can take the Telecommand in currentPacket and perform any kind of operation. - * They may send additional "Start Success (1,3)" messages with the verifyReporter, but Completion - * Success or Failure Reports are generated automatically after execution of this method. + * Implemetations can take the Telecommand in currentPacket and perform + * any kind of operation. + * They may send additional "Start Success (1,3)" messages with the + * verifyReporter, but Completion Success or Failure Reports are generated + * automatically after execution of this method. * * If a Telecommand can not be executed within one call cycle, * this Base class is not the right parent. * - * The child class may add additional error information by setting #errorParameters which are - * attached to the generated verification message. + * The child class may add additional error information by setting + * #errorParameters which aren attached to the generated verification + * message. * * Subservice checking should be implemented in this method. * - * @return The returned status_code is directly taken as main error code in the Verification Report. + * @return The returned status_code is directly taken as main error code + * in the Verification Report. * On success, RETURN_OK shall be returned. */ virtual ReturnValue_t handleRequest(uint8_t subservice) = 0; /** - * In performService, implementations can handle periodic, non-TC-triggered activities. + * In performService, implementations can handle periodic, + * non-TC-triggered activities. * The performService method is always called. - * @return A success or failure code that does not trigger any kind of verification message. + * @return Currently, everything other that RETURN_OK only triggers + * diagnostic output. */ virtual ReturnValue_t performService() = 0; /** * This method implements the typical activity of a simple PUS Service. - * It checks for new requests, and, if found, calls handleRequest, sends completion verification messages and deletes + * It checks for new requests, and, if found, calls handleRequest, sends + * completion verification messages and deletes * the TC requests afterwards. * performService is always executed afterwards. - * @return \c RETURN_OK if the periodic performService was successful. - * \c RETURN_FAILED else. + * @return @c RETURN_OK if the periodic performService was successful. + * @c RETURN_FAILED else. */ ReturnValue_t performOperation(uint8_t opCode); virtual uint16_t getIdentifier(); @@ -103,13 +118,13 @@ protected: */ uint32_t errorParameter2 = 0; /** - * This is a complete instance of the Telecommand reception queue of the class. - * It is initialized on construction of the class. + * This is a complete instance of the telecommand reception queue + * of the class. It is initialized on construction of the class. */ MessageQueueIF* requestQueue = nullptr; /** - * An instance of the VerificationReporter class, that simplifies sending any kind of - * Verification Message to the TC Verification Service. + * An instance of the VerificationReporter class, that simplifies + * sending any kind of verification message to the TC Verification Service. */ VerificationReporter verifyReporter; /** @@ -124,7 +139,8 @@ protected: private: /** * This constant sets the maximum number of packets accepted per call. - * Remember that one packet must be completely handled in one #handleRequest call. + * Remember that one packet must be completely handled in one + * #handleRequest call. */ static const uint8_t PUS_SERVICE_MAX_RECEPTION = 10; From 5007041bc87e39a44284de14f465fa0478cbc2a0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:12:29 +0200 Subject: [PATCH 07/12] cleaned up includes and improved doc a bit --- tmtcservices/CommandingServiceBase.cpp | 27 +++--- tmtcservices/CommandingServiceBase.h | 114 ++++++++++++++----------- 2 files changed, 76 insertions(+), 65 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index adc9400f..361baf3b 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -1,22 +1,21 @@ -/* - * CommandingServiceBase.cpp - * - * Created on: 28.08.2019 - * Author: gaisser - */ +#include +#include +#include #include +#include +#include +#include +#include CommandingServiceBase::CommandingServiceBase(object_id_t setObjectId, uint16_t apid, uint8_t service, uint8_t numberOfParallelCommands, - uint16_t commandTimeout_seconds, object_id_t setPacketSource, + uint16_t commandTimeoutSeconds, object_id_t setPacketSource, object_id_t setPacketDestination, size_t queueDepth) : - SystemObject(setObjectId), apid(apid), service(service), timeout_seconds( - commandTimeout_seconds), tmPacketCounter(0), IPCStore(NULL), TCStore( - NULL), commandQueue(NULL), requestQueue(NULL), commandMap( - numberOfParallelCommands), failureParameter1(0), failureParameter2( - 0), packetSource(setPacketSource), packetDestination( - setPacketDestination),executingTask(NULL) { + SystemObject(setObjectId), apid(apid), service(service), + timeoutSeconds(commandTimeoutSeconds), + commandMap(numberOfParallelCommands), packetSource(setPacketSource), + packetDestination(setPacketDestination) { commandQueue = QueueFactory::instance()->createMessageQueue(queueDepth); requestQueue = QueueFactory::instance()->createMessageQueue(queueDepth); } @@ -369,7 +368,7 @@ void CommandingServiceBase::checkTimeout() { typename FixedMap::Iterator iter; for (iter = commandMap.begin(); iter != commandMap.end(); ++iter) { - if ((iter->uptimeOfStart + (timeout_seconds * 1000)) < uptime) { + if ((iter->uptimeOfStart + (timeoutSeconds * 1000)) < uptime) { verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index e79e806d..37f6891c 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -1,35 +1,33 @@ -#ifndef COMMANDINGSERVICEBASE_H_ -#define COMMANDINGSERVICEBASE_H_ +#ifndef FRAMEWORK_TMTCSERVICES_COMMANDINGSERVICEBASE_H_ +#define FRAMEWORK_TMTCSERVICES_COMMANDINGSERVICEBASE_H_ -#include -#include -#include -#include #include -#include #include #include -#include -#include -#include +#include #include -#include -#include + #include -#include -#include -#include +#include +#include +#include +#include + +class TcPacketStored; /** - * \brief This class is the basis for all PUS Services, which have to relay Telecommands to software bus. + * @brief This class is the basis for all PUS Services, which have to + * relay Telecommands to software bus. * - * It manages Telecommand reception and the generation of Verification Reports like PUSServiceBase. - * Every class that inherits from this abstract class has to implement four adaption points: + * It manages Telecommand reception and the generation of Verification Reports + * similar to PusServiceBase. This class is used if a telecommand can't be + * handled immediately and must be relayed to the internal software bus. * - isValidSubservice * - getMessageQueueAndObject * - prepareCommand * - handleReply - * \ingroup pus_services + * @author gaisser + * @ingroup pus_services */ class CommandingServiceBase: public SystemObject, public AcceptsTelecommandsIF, @@ -59,7 +57,7 @@ public: */ CommandingServiceBase(object_id_t setObjectId, uint16_t apid, uint8_t service, uint8_t numberOfParallelCommands, - uint16_t commandTimeout_seconds, object_id_t setPacketSource, + uint16_t commandTimeoutSeconds, object_id_t setPacketSource, object_id_t setPacketDestination, size_t queueDepth = 20); virtual ~CommandingServiceBase(); @@ -113,8 +111,9 @@ protected: virtual ReturnValue_t isValidSubservice(uint8_t subservice) = 0; /** - * Once a TC Request is valid, the existence of the destination and its target interface is checked and retrieved. - * The target message queue ID can then be acquired by using the target interface. + * Once a TC Request is valid, the existence of the destination and its + * target interface is checked and retrieved. The target message queue ID + * can then be acquired by using the target interface. * @param subservice * @param tcData Application Data of TC Packet * @param tcDataLen @@ -129,10 +128,10 @@ protected: object_id_t *objectId) = 0; /** - * After the Message Queue and Object ID are determined, - * the command is prepared by using an implementation specific CommandMessage type - * which is sent to the target object. - * It contains all necessary information for the device to execute telecommands. + * After the Message Queue and Object ID are determined, the command is + * prepared by using an implementation specific CommandMessage type + * which is sent to the target object. It contains all necessary information + * for the device to execute telecommands. * @param message * @param subservice * @param tcData @@ -146,25 +145,40 @@ protected: uint32_t *state, object_id_t objectId) = 0; /** - * This function is responsible for the communication between the Command Service Base - * and the respective PUS Commanding Service once the execution has started. - * The PUS Commanding Service receives replies from the target device and forwards them by calling this function. - * There are different translations of these replies to specify how the Command Service proceeds. - * @param reply[out] Command Message which contains information about the command - * @param previousCommand [out] - * @param state - * @param optionalNextCommand + * This function is implemented by child services to specify how replies + * to a command from another software component are handled + * @param reply + * This is the reply in form of a command message. + * @param previousCommand + * Command_t of related command + * @param state [out/in] + * Additional parameter which can be used to pass state information. + * State of the communication + * @param optionalNextCommand [out] + * An optional next command which can be set in this function * @param objectId Source object ID * @param isStep Flag value to mark steps of command execution - * @return - @c RETURN_OK, @c EXECUTION_COMPLETE or @c NO_STEP_MESSAGE to generate TC verification success - * - @c INVALID_REPLY can handle unrequested replies - * - Anything else triggers a TC verification failure + * @return + * - @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 */ virtual ReturnValue_t handleReply(const CommandMessage *reply, Command_t previousCommand, uint32_t *state, CommandMessage *optionalNextCommand, object_id_t objectId, bool *isStep) = 0; + /** + * This function can be overidden to handle unrequested reply, + * when the reply sender ID is unknown or is not found is the command map. + * @param reply + */ + virtual void handleUnrequestedReply(CommandMessage *reply); + + virtual void doPeriodicOperation(); + + struct CommandInfo { struct tcInfo { uint8_t ackFlags; @@ -184,33 +198,35 @@ protected: const uint8_t service; - const uint16_t timeout_seconds; + const uint16_t timeoutSeconds; - uint8_t tmPacketCounter; + uint8_t tmPacketCounter = 0; - StorageManagerIF *IPCStore; + StorageManagerIF *IPCStore = nullptr; - StorageManagerIF *TCStore; + StorageManagerIF *TCStore = nullptr; - MessageQueueIF* commandQueue; + MessageQueueIF* commandQueue = nullptr; - MessageQueueIF* requestQueue; + MessageQueueIF* requestQueue = nullptr; VerificationReporter verificationReporter; FixedMap commandMap; - uint32_t failureParameter1; //!< May be set be children to return a more precise failure condition. - uint32_t failureParameter2; //!< May be set be children to return a more precise failure condition. + /* May be set be children to return a more precise failure condition. */ + uint32_t failureParameter1 = 0; + uint32_t failureParameter2 = 0; object_id_t packetSource; object_id_t packetDestination; /** - * Pointer to the task which executes this component, is invalid before setTaskIF was called. + * Pointer to the task which executes this component, + * is invalid before setTaskIF was called. */ - PeriodicTaskIF* executingTask; + PeriodicTaskIF* executingTask = nullptr; /** * @brief Send TM data from pointer to data. @@ -246,10 +262,6 @@ protected: ReturnValue_t sendTmPacket(uint8_t subservice, SerializeIF* content, SerializeIF* header = nullptr); - virtual void handleUnrequestedReply(CommandMessage *reply); - - virtual void doPeriodicOperation(); - void checkAndExecuteFifo( typename FixedMap::Iterator *iter); From bb9a299e33ce7429314acb58606f964613a31589 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:19:40 +0200 Subject: [PATCH 08/12] added back removed comments --- tmtcservices/CommandingServiceBase.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 37f6891c..f97eabb0 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -132,11 +132,12 @@ protected: * prepared by using an implementation specific CommandMessage type * which is sent to the target object. It contains all necessary information * for the device to execute telecommands. - * @param message - * @param subservice - * @param tcData - * @param tcDataLen - * @param state + * @param message [out] message which can be set and is sent to the object + * @param subservice Subservice of the current communication + * @param tcData Application data of command + * @param tcDataLen Application data length + * @param state [out/in] Setable state of the communication. + * communication * @param objectId Target object ID * @return */ From 3268806f75a3ea8ceaf7ce933276133d26b05719 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Wed, 10 Jun 2020 22:51:54 +0200 Subject: [PATCH 09/12] split up huge function to be more readable --- tmtcservices/CommandingServiceBase.cpp | 193 ++++++++++++++----------- tmtcservices/CommandingServiceBase.h | 7 + 2 files changed, 113 insertions(+), 87 deletions(-) diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 361baf3b..4c557f82 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -56,7 +56,7 @@ ReturnValue_t CommandingServiceBase::initialize() { objectManager->get(packetDestination); PUSDistributorIF* distributor = objectManager->get( packetSource); - if ((packetForwarding == NULL) && (distributor == NULL)) { + if (packetForwarding == nullptr or distributor == nullptr) { return RETURN_FAILED; } @@ -67,7 +67,7 @@ ReturnValue_t CommandingServiceBase::initialize() { IPCStore = objectManager->get(objects::IPC_STORE); TCStore = objectManager->get(objects::TC_STORE); - if ((IPCStore == NULL) || (TCStore == NULL)) { + if (IPCStore == nullptr or TCStore == nullptr) { return RETURN_FAILED; } @@ -76,97 +76,116 @@ ReturnValue_t CommandingServiceBase::initialize() { } void CommandingServiceBase::handleCommandQueue() { - CommandMessage reply, nextCommand; - ReturnValue_t result, sendResult = RETURN_OK; - bool isStep = false; + CommandMessage reply; + ReturnValue_t result = RETURN_FAILED; for (result = commandQueue->receiveMessage(&reply); result == RETURN_OK; result = commandQueue->receiveMessage(&reply)) { - isStep = false; - typename FixedMap::Iterator iter; - if (reply.getSender() == MessageQueueIF::NO_QUEUE) { - handleUnrequestedReply(&reply); - continue; - } - if ((iter = commandMap.find(reply.getSender())) == commandMap.end()) { - handleUnrequestedReply(&reply); - continue; - } - nextCommand.setCommand(CommandMessage::CMD_NONE); - result = handleReply(&reply, iter->command, &iter->state, &nextCommand, - iter->objectId, &isStep); - switch (result) { - case EXECUTION_COMPLETE: - case RETURN_OK: - case NO_STEP_MESSAGE: - iter->command = nextCommand.getCommand(); - if (nextCommand.getCommand() != CommandMessage::CMD_NONE) { - sendResult = commandQueue->sendMessage(reply.getSender(), - &nextCommand); - } - if (sendResult == RETURN_OK) { - if (isStep) { - if (result != NO_STEP_MESSAGE) { - verificationReporter.sendSuccessReport( - TC_VERIFY::PROGRESS_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, ++iter->step); - } - } else { - verificationReporter.sendSuccessReport( - TC_VERIFY::COMPLETION_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, 0); - checkAndExecuteFifo(&iter); - } - } else { - if (isStep) { - 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(); - verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, sendResult, 0, - failureParameter1, failureParameter2); - } - failureParameter1 = 0; - failureParameter2 = 0; - checkAndExecuteFifo(&iter); - } - break; - case INVALID_REPLY: - //might be just an unrequested reply at a bad moment - handleUnrequestedReply(&reply); - break; - default: - if (isStep) { - verificationReporter.sendFailureReport( - TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, - result, ++iter->step, failureParameter1, - failureParameter2); - } else { - verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, - result, 0, failureParameter1, failureParameter2); - } - failureParameter1 = 0; - failureParameter2 = 0; - checkAndExecuteFifo(&iter); - break; - } - + handleCommandMessage(reply); } } +void CommandingServiceBase::handleCommandMessage(CommandMessage& reply) { + bool isStep = false; + CommandMessage nextCommand; + CommandMapIter iter; + if (reply.getSender() == MessageQueueIF::NO_QUEUE) { + handleUnrequestedReply(&reply); + return; + } + if ((iter = commandMap.find(reply.getSender())) == 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, + &nextCommand, iter->objectId, &isStep); + + 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); + break; + case INVALID_REPLY: + //might be just an unrequested reply at a bad moment + handleUnrequestedReply(&reply); + break; + default: + if (isStep) { + verificationReporter.sendFailureReport( + TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, + iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + result, ++iter->step, failureParameter1, + failureParameter2); + } else { + verificationReporter.sendFailureReport( + TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, + iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + result, 0, failureParameter1, failureParameter2); + } + failureParameter1 = 0; + failureParameter2 = 0; + checkAndExecuteFifo(&iter); + break; + } + +} + +void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, + 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. + // 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 (sendResult == RETURN_OK) { + if (isStep and result != NO_STEP_MESSAGE) { + verificationReporter.sendSuccessReport( + TC_VERIFY::PROGRESS_SUCCESS, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, ++iter->step); + } + else { + verificationReporter.sendSuccessReport( + TC_VERIFY::COMPLETION_SUCCESS, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, 0); + checkAndExecuteFifo(&iter); + } + } + else { + if (isStep) { + 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(); + verificationReporter.sendFailureReport( + TC_VERIFY::COMPLETION_FAILURE, + iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, + iter->tcInfo.tcSequenceControl, sendResult, 0, + failureParameter1, failureParameter2); + } + failureParameter1 = 0; + failureParameter2 = 0; + checkAndExecuteFifo(&iter); + } +} + void CommandingServiceBase::handleRequestQueue() { TmTcMessage message; ReturnValue_t result; diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index f97eabb0..9a68a2d3 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -267,6 +267,9 @@ protected: typename FixedMap::Iterator *iter); private: + using CommandMapIter = FixedMap::Iterator; + /** * This method handles internal execution of a command, * once it has been started by @sa{startExecution()} in the Request Queue handler. @@ -298,6 +301,10 @@ private: void startExecution(TcPacketStored *storedPacket, typename FixedMap::Iterator *iter); + void handleCommandMessage(CommandMessage& reply); + void handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, + CommandMessage& nextCommand,CommandMessage& reply, bool& isStep); + void checkTimeout(); }; From a9c7ad84c8b93c39021194e99306e023432b4ee0 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Jun 2020 02:03:18 +0200 Subject: [PATCH 10/12] added new interface to host and linux osal --- ipc/CommandMessage.cpp | 8 ++++++++ ipc/CommandMessage.h | 2 ++ ipc/MessageQueueMessage.cpp | 8 ++++++++ ipc/MessageQueueMessage.h | 4 ++++ ipc/MessageQueueMessageIF.h | 12 ++++++++++++ osal/host/MessageQueue.cpp | 30 ++++++++++++++++++++---------- osal/host/MessageQueue.h | 16 ++++++++-------- osal/host/QueueFactory.cpp | 2 +- osal/linux/MessageQueue.cpp | 25 +++++++++++++------------ osal/linux/MessageQueue.h | 21 +++++++++++++-------- osal/linux/QueueFactory.cpp | 2 +- 11 files changed, 90 insertions(+), 40 deletions(-) diff --git a/ipc/CommandMessage.cpp b/ipc/CommandMessage.cpp index 44424e02..7d2ecc9b 100644 --- a/ipc/CommandMessage.cpp +++ b/ipc/CommandMessage.cpp @@ -160,3 +160,11 @@ uint8_t* CommandMessage::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 772bd774..25d608f0 100644 --- a/ipc/CommandMessage.h +++ b/ipc/CommandMessage.h @@ -85,6 +85,8 @@ public: uint8_t * getData() override; const uint8_t* getData() const override; size_t getMinimumMessageSize() const override; + size_t getMessageSize() const override; + size_t getMaximumMessageSize() const override; /** * Extract message ID, which is the first byte of the command ID. diff --git a/ipc/MessageQueueMessage.cpp b/ipc/MessageQueueMessage.cpp index c3e1cb7a..8430a8ea 100644 --- a/ipc/MessageQueueMessage.cpp +++ b/ipc/MessageQueueMessage.cpp @@ -67,3 +67,11 @@ void MessageQueueMessage::print() { void MessageQueueMessage::clear() { memset(this->getBuffer(), 0, this->MAX_MESSAGE_SIZE); } + +size_t MessageQueueMessage::getMessageSize() const { + return this->messageSize; +} + +size_t MessageQueueMessage::getMaximumMessageSize() const { + return this->MAX_MESSAGE_SIZE; +} diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index bee39ae6..31a43f66 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -149,6 +149,10 @@ public: */ virtual size_t getMinimumMessageSize() const override; + virtual size_t getMessageSize() const override; + + virtual size_t getMaximumMessageSize() const override; + /** * @brief This is a debug method that prints the content * (till messageSize) to the debug output. diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 197a26fd..05330ce9 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -71,6 +71,18 @@ public: * check the size of an incoming message. */ virtual size_t getMinimumMessageSize() const = 0; + + /** + * Get message size of current message implementation. + * @return + */ + virtual size_t getMessageSize() const = 0; + + /** + * Get maximum allowed size of current message implementation. + * @return + */ + virtual size_t getMaximumMessageSize() const = 0; }; diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 3e30c8ed..7af5fbd9 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -18,20 +18,20 @@ MessageQueue::~MessageQueue() { } ReturnValue_t MessageQueue::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault) { + MessageQueueMessageIF* message, bool ignoreFault) { return sendMessageFrom(sendTo, message, this->getId(), ignoreFault); } -ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessageIF* message) { return sendToDefaultFrom(message, this->getId()); } -ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessage* message, +ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFrom(defaultDestination,message,sentFrom,ignoreFault); } -ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { if (this->lastPartner != 0) { return sendMessageFrom(this->lastPartner, message, this->getId()); } else { @@ -40,13 +40,13 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { } ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFromMessageQueue(sendTo, message, sentFrom, ignoreFault); } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t* receivedFrom) { ReturnValue_t status = this->receiveMessage(message); if(status == HasReturnvaluesIF::RETURN_OK) { @@ -55,7 +55,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, return status; } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { if(messageQueue.empty()) { return MessageQueueIF::EMPTY; } @@ -102,9 +102,9 @@ bool MessageQueue::isDefaultDestinationSet() const { // static core function to send messages. ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessage *message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { - if(message->messageSize > MessageQueueMessage::MAX_DATA_SIZE) { + if(message->getMessageSize() > message->getMaximumMessageSize()) { // Actually, this should never happen or an error will be emitted // in MessageQueueMessage. // But I will still return a failure here. @@ -127,7 +127,17 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { MutexHelper mutexLock(targetQueue->queueLock, 20); - targetQueue->messageQueue.push(*message); + // not ideal, works for now though. + MessageQueueMessage* mqmMessage = + dynamic_cast(message); + if(message != nullptr) { + targetQueue->messageQueue.push(*mqmMessage); + } + else { + sif::error << "MessageQueue::sendMessageFromMessageQueue: Message" + "is not MessageQueueMessage!" << std::endl; + } + } else { return MessageQueueIF::FULL; diff --git a/osal/host/MessageQueue.h b/osal/host/MessageQueue.h index 619b5e91..7fc77f7a 100644 --- a/osal/host/MessageQueue.h +++ b/osal/host/MessageQueue.h @@ -77,7 +77,7 @@ public: * is not incremented if queue is full. */ ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault = false) override; + 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 @@ -85,7 +85,7 @@ public: * queue id as "sentFrom" information. * @param message A pointer to a previously created message, which is sent. */ - ReturnValue_t sendToDefault(MessageQueueMessage* message) override; + 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 @@ -93,7 +93,7 @@ public: * message received yet (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(MessageQueueMessage* message) override; + ReturnValue_t reply(MessageQueueMessageIF* message) override; /** * @brief With the sendMessage call, a queue message is sent to a @@ -113,7 +113,7 @@ public: * is not incremented if queue is full. */ virtual ReturnValue_t sendMessageFrom( MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom = NO_QUEUE, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false) override; /** @@ -127,7 +127,7 @@ public: * sender's queue id into the message. This variable is set to zero by * default. */ - virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessage* message, + virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false) override; @@ -140,7 +140,7 @@ public: * @param message A pointer to a message in which the received data is stored. * @param receivedFrom A pointer to a queue id in which the sender's id is stored. */ - ReturnValue_t receiveMessage(MessageQueueMessage* message, + ReturnValue_t receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t *receivedFrom) override; /** @@ -153,7 +153,7 @@ public: * function returns immediately. * @param message A pointer to a message in which the received data is stored. */ - ReturnValue_t receiveMessage(MessageQueueMessage* message) override; + ReturnValue_t receiveMessage(MessageQueueMessageIF* message) override; /** * Deletes all pending messages in the queue. * @param count The number of flushed messages. @@ -205,7 +205,7 @@ protected: * @param context Specify whether call is made from task or from an ISR. */ static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom = NO_QUEUE, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault=false); //static ReturnValue_t handleSendResult(BaseType_t result, bool ignoreFault); diff --git a/osal/host/QueueFactory.cpp b/osal/host/QueueFactory.cpp index 60274c84..225bb8c7 100644 --- a/osal/host/QueueFactory.cpp +++ b/osal/host/QueueFactory.cpp @@ -7,7 +7,7 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return MessageQueue::sendMessageFromMessageQueue(sendTo,message, sentFrom,ignoreFault); diff --git a/osal/linux/MessageQueue.cpp b/osal/linux/MessageQueue.cpp index a0749ded..605dea2a 100644 --- a/osal/linux/MessageQueue.cpp +++ b/osal/linux/MessageQueue.cpp @@ -118,15 +118,15 @@ ReturnValue_t MessageQueue::handleError(mq_attr* attributes, } ReturnValue_t MessageQueue::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault) { + MessageQueueMessageIF* message, bool ignoreFault) { return sendMessageFrom(sendTo, message, this->getId(), false); } -ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::sendToDefault(MessageQueueMessageIF* message) { return sendToDefaultFrom(message, this->getId()); } -ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { if (this->lastPartner != 0) { return sendMessageFrom(this->lastPartner, message, this->getId()); } else { @@ -134,21 +134,21 @@ ReturnValue_t MessageQueue::reply(MessageQueueMessage* message) { } } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message, +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t* receivedFrom) { ReturnValue_t status = this->receiveMessage(message); *receivedFrom = this->lastPartner; return status; } -ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { +ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { unsigned int messagePriority = 0; int status = mq_receive(id,reinterpret_cast(message->getBuffer()), - message->MAX_MESSAGE_SIZE,&messagePriority); + message->getMaximumMessageSize(),&messagePriority); if (status > 0) { this->lastPartner = message->getSender(); //Check size of incoming message. - if (message->messageSize < message->getMinimumMessageSize()) { + if (message->getMessageSize() < message->getMinimumMessageSize()) { return HasReturnvaluesIF::RETURN_FAILED; } return HasReturnvaluesIF::RETURN_OK; @@ -158,7 +158,7 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessage* message) { } else { //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); + memset(message->getData(), 0, message->getMaximumMessageSize()); switch(errno){ case EAGAIN: //O_NONBLOCK or MQ_NONBLOCK was set and there are no messages @@ -259,13 +259,13 @@ void MessageQueue::setDefaultDestination(MessageQueueId_t defaultDestination) { } ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFromMessageQueue(sendTo,message,sentFrom,ignoreFault); } -ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessage* message, +ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return sendMessageFrom(defaultDestination, message, sentFrom, ignoreFault); } @@ -281,11 +281,12 @@ bool MessageQueue::isDefaultDestinationSet() const { uint16_t MessageQueue::queueCounter = 0; ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, - MessageQueueMessage *message, MessageQueueId_t sentFrom, + MessageQueueMessageIF *message, MessageQueueId_t sentFrom, bool ignoreFault) { message->setSender(sentFrom); int result = mq_send(sendTo, - reinterpret_cast(message->getBuffer()), message->messageSize,0); + reinterpret_cast(message->getBuffer()), + message->getMessageSize(),0); //TODO: Check if we're in ISR. if (result != 0) { diff --git a/osal/linux/MessageQueue.h b/osal/linux/MessageQueue.h index 2808d36e..91066bf7 100644 --- a/osal/linux/MessageQueue.h +++ b/osal/linux/MessageQueue.h @@ -56,14 +56,14 @@ public: * @param ignoreFault If set to true, the internal software fault counter is not incremented if queue is full. */ virtual ReturnValue_t sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, bool ignoreFault = false ); + MessageQueueMessageIF* message, bool ignoreFault = false ); /** * @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. */ - virtual ReturnValue_t sendToDefault( MessageQueueMessage* message ); + virtual ReturnValue_t sendToDefault( MessageQueueMessageIF* message ); /** * @brief This operation sends a message to the last communication partner. * @details This operation simplifies answering an incoming message by using the stored @@ -71,7 +71,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( MessageQueueMessage* message ); + ReturnValue_t reply( MessageQueueMessageIF* message ); /** * @brief This function reads available messages from the message queue and returns the sender. @@ -80,7 +80,7 @@ public: * @param message A pointer to a message in which the received data is stored. * @param receivedFrom A pointer to a queue id in which the sender's id is stored. */ - ReturnValue_t receiveMessage(MessageQueueMessage* message, + ReturnValue_t receiveMessage(MessageQueueMessageIF* message, MessageQueueId_t *receivedFrom); /** @@ -91,7 +91,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(MessageQueueMessage* message); + ReturnValue_t receiveMessage(MessageQueueMessageIF* message); /** * Deletes all pending messages in the queue. * @param count The number of flushed messages. @@ -114,7 +114,9 @@ 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, MessageQueueMessage* message, MessageQueueId_t sentFrom, bool ignoreFault = false ); + virtual ReturnValue_t sendMessageFrom( MessageQueueId_t sendTo, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, + bool ignoreFault = false ); /** * \brief The sendToDefault method sends a queue message to the default destination. * \details In all other aspects, it works identical to the sendMessage method. @@ -122,7 +124,8 @@ public: * \param sentFrom The sentFrom information can be set to inject the sender's queue id into the message. * This variable is set to zero by default. */ - virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessage* message, MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false ); + virtual ReturnValue_t sendToDefaultFrom( MessageQueueMessageIF* message, + MessageQueueId_t sentFrom = NO_QUEUE, bool ignoreFault = false ); /** * \brief This method is a simple setter for the default destination. */ @@ -145,7 +148,9 @@ protected: * 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. */ - static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo,MessageQueueMessage* message, MessageQueueId_t sentFrom = NO_QUEUE,bool ignoreFault=false); + static ReturnValue_t sendMessageFromMessageQueue(MessageQueueId_t sendTo, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom = NO_QUEUE, + bool ignoreFault=false); private: /** * @brief The class stores the queue id it got assigned from the operating system in this attribute. diff --git a/osal/linux/QueueFactory.cpp b/osal/linux/QueueFactory.cpp index 268d0b99..8bd9d52c 100644 --- a/osal/linux/QueueFactory.cpp +++ b/osal/linux/QueueFactory.cpp @@ -9,7 +9,7 @@ QueueFactory* QueueFactory::factoryInstance = nullptr; ReturnValue_t MessageQueueSenderIF::sendMessage(MessageQueueId_t sendTo, - MessageQueueMessage* message, MessageQueueId_t sentFrom, + MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { return MessageQueue::sendMessageFromMessageQueue(sendTo,message, sentFrom,ignoreFault); From 206235ed47e0b086acc06015aeb36484bfeeab76 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Jun 2020 16:03:09 +0200 Subject: [PATCH 11/12] dataset base bugfixes --- datapool/DataSetBase.cpp | 10 +++++++--- health/HealthHelper.cpp | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/datapool/DataSetBase.cpp b/datapool/DataSetBase.cpp index 9104c8e7..60b20f29 100644 --- a/datapool/DataSetBase.cpp +++ b/datapool/DataSetBase.cpp @@ -36,7 +36,7 @@ ReturnValue_t DataSetBase::read(uint32_t lockTimeout) { if (state == States::DATA_SET_UNINITIALISED) { lockDataPool(lockTimeout); for (uint16_t count = 0; count < fillCount; count++) { - ReturnValue_t result = readVariable(count); + result = readVariable(count); if(result != RETURN_OK) { break; } @@ -46,14 +46,15 @@ ReturnValue_t DataSetBase::read(uint32_t lockTimeout) { } else { sif::error << "DataSet::read(): " - "Call made in wrong position." << std::endl; + "Call made in wrong position. Don't forget to commit" + " member datasets!" << std::endl; result = SET_WAS_ALREADY_READ; } return result; } ReturnValue_t DataSetBase::readVariable(uint16_t count) { - ReturnValue_t result = DataSetIF::INVALID_PARAMETER_DEFINITION; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; // These checks are often performed by the respective // variable implementation too, but I guess a double check does not hurt. if (registeredVariables[count]->getReadWriteMode() != @@ -62,6 +63,9 @@ ReturnValue_t DataSetBase::readVariable(uint16_t count) { != PoolVariableIF::NO_PARAMETER) { result = registeredVariables[count]->readWithoutLock(); + if(result != HasReturnvaluesIF::RETURN_OK) { + result = INVALID_PARAMETER_DEFINITION; + } } return result; } diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index b23a8bd2..df259b52 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -62,7 +62,7 @@ void HealthHelper::setHealth(HasHealthIF::HealthState health) { void HealthHelper::informParent(HasHealthIF::HealthState health, HasHealthIF::HealthState oldHealth) { - if (parentQueue == 0) { + if (parentQueue == MessageQueueMessageIF::NO_QUEUE) { return; } MessageQueueMessage message; From 6838a9e7687a7232f903b5b39c11fe026bf476ff Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Thu, 11 Jun 2020 16:19:22 +0200 Subject: [PATCH 12/12] serial buffer adapter bugfix --- serialize/SerialBufferAdapter.cpp | 16 ++++++++-------- serialize/SerialBufferAdapter.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/serialize/SerialBufferAdapter.cpp b/serialize/SerialBufferAdapter.cpp index e409686f..d417ba45 100644 --- a/serialize/SerialBufferAdapter.cpp +++ b/serialize/SerialBufferAdapter.cpp @@ -21,29 +21,29 @@ SerialBufferAdapter::~SerialBufferAdapter() { } template -ReturnValue_t SerialBufferAdapter::serialize(uint8_t** buffer, - size_t* size, const size_t max_size, bool bigEndian) const { +ReturnValue_t SerialBufferAdapter::serialize(uint8_t** buffer_, + size_t* size_, const size_t max_size, bool bigEndian) const { uint32_t serializedLength = bufferLength; if (serializeLength) { serializedLength += AutoSerializeAdapter::getSerializedSize( &bufferLength); } - if (*size + serializedLength > max_size) { + if (*size_ + serializedLength > max_size) { return BUFFER_TOO_SHORT; } else { if (serializeLength) { - AutoSerializeAdapter::serialize(&bufferLength, buffer, size, + AutoSerializeAdapter::serialize(&bufferLength, buffer_, size_, max_size, bigEndian); } if (constBuffer != nullptr) { - memcpy(*buffer, constBuffer, bufferLength); + memcpy(*buffer_, this->constBuffer, bufferLength); } else if (buffer != nullptr) { - memcpy(*buffer, buffer, bufferLength); + memcpy(*buffer_, this->buffer, bufferLength); } else { return HasReturnvaluesIF::RETURN_FAILED; } - *size += bufferLength; - (*buffer) += bufferLength; + *size_ += bufferLength; + (*buffer_) += bufferLength; return HasReturnvaluesIF::RETURN_OK; } } diff --git a/serialize/SerialBufferAdapter.h b/serialize/SerialBufferAdapter.h index cb799be5..02690021 100644 --- a/serialize/SerialBufferAdapter.h +++ b/serialize/SerialBufferAdapter.h @@ -45,7 +45,7 @@ public: virtual ~SerialBufferAdapter(); - virtual ReturnValue_t serialize(uint8_t** buffer, size_t* size, + virtual ReturnValue_t serialize(uint8_t** buffer_, size_t* size, const size_t max_size, bool bigEndian) const override; virtual size_t getSerializedSize() const;