diff --git a/action/ActionHelper.cpp b/action/ActionHelper.cpp index b43c676d0..70c3f0fca 100644 --- a/action/ActionHelper.cpp +++ b/action/ActionHelper.cpp @@ -1,3 +1,4 @@ +#include #include "ActionHelper.h" #include "HasActionsIF.h" #include "../objectmanager/ObjectManagerIF.h" diff --git a/action/SimpleActionHelper.cpp b/action/SimpleActionHelper.cpp index d79a3c977..51e3fa1c2 100644 --- a/action/SimpleActionHelper.cpp +++ b/action/SimpleActionHelper.cpp @@ -1,5 +1,6 @@ #include "HasActionsIF.h" #include "SimpleActionHelper.h" + SimpleActionHelper::SimpleActionHelper(HasActionsIF* setOwner, MessageQueueIF* useThisQueue) : ActionHelper(setOwner, useThisQueue), isExecuting(false), lastCommander( diff --git a/devicehandlers/CommunicationMessage.cpp b/devicehandlers/CommunicationMessage.cpp index abdfd3a86..577742709 100644 --- a/devicehandlers/CommunicationMessage.cpp +++ b/devicehandlers/CommunicationMessage.cpp @@ -122,7 +122,7 @@ uint32_t CommunicationMessage::getUint32Data() const{ } void CommunicationMessage::setDataByte(uint8_t byte, uint8_t position) { - if(0 <= position && position <= 3) { + if(position <= 3) { memcpy(getData() + 3 * sizeof(uint32_t) + position * sizeof(uint8_t), &byte, sizeof(byte)); } else { @@ -131,7 +131,7 @@ void CommunicationMessage::setDataByte(uint8_t byte, uint8_t position) { } uint8_t CommunicationMessage::getDataByte(uint8_t position) const { - if(0 <= position && position <= 3) { + if(position <= 3) { uint8_t byte; memcpy(&byte, getData() + 3 * sizeof(uint32_t) + position * sizeof(uint8_t), sizeof(byte)); return byte; diff --git a/health/HealthHelper.cpp b/health/HealthHelper.cpp index cced11d9b..d574634d8 100644 --- a/health/HealthHelper.cpp +++ b/health/HealthHelper.cpp @@ -71,7 +71,7 @@ void HealthHelper::setHealth(HasHealthIF::HealthState health) { void HealthHelper::informParent(HasHealthIF::HealthState health, HasHealthIF::HealthState oldHealth) { - if (parentQueue == MessageQueueMessageIF::NO_QUEUE) { + if (parentQueue == MessageQueueIF::NO_QUEUE) { return; } CommandMessage information; @@ -86,7 +86,7 @@ void HealthHelper::informParent(HasHealthIF::HealthState health, void HealthHelper::handleSetHealthCommand(CommandMessage* command) { ReturnValue_t result = owner->setHealth(HealthMessage::getHealth(command)); - if (command->getSender() == MessageQueueMessageIF::NO_QUEUE) { + if (command->getSender() == MessageQueueIF::NO_QUEUE) { return; } CommandMessage reply; diff --git a/ipc/MessageQueueIF.h b/ipc/MessageQueueIF.h index 96bd379f9..e9a4a00bf 100644 --- a/ipc/MessageQueueIF.h +++ b/ipc/MessageQueueIF.h @@ -1,21 +1,23 @@ #ifndef FSFW_IPC_MESSAGEQUEUEIF_H_ #define FSFW_IPC_MESSAGEQUEUEIF_H_ +#include "MessageQueueMessageIF.h" +#include "messageQueueDefintions.h" +#include "../returnvalues/HasReturnvaluesIF.h" + +#include + + // COULDDO: We could support blocking calls // semaphores are being implemented, which makes this idea even more iteresting. - /** * @defgroup message_queue Message Queue * @brief Message Queue related software components */ - -#include "../ipc/MessageQueueMessage.h" -#include "../ipc/MessageQueueSenderIF.h" -#include "../returnvalues/HasReturnvaluesIF.h" class MessageQueueIF { public: - static const MessageQueueId_t NO_QUEUE = MessageQueueMessageIF::NO_QUEUE; //!< Ugly hack. + static const MessageQueueId_t NO_QUEUE = 0; static const uint8_t INTERFACE_ID = CLASS_ID::MESSAGE_QUEUE_IF; //! No new messages on the queue diff --git a/ipc/MessageQueueMessage.h b/ipc/MessageQueueMessage.h index f68e9b9f9..5234f64ff 100644 --- a/ipc/MessageQueueMessage.h +++ b/ipc/MessageQueueMessage.h @@ -2,7 +2,6 @@ #define FSFW_IPC_MESSAGEQUEUEMESSAGE_H_ #include "../ipc/MessageQueueMessageIF.h" -#include "../ipc/MessageQueueSenderIF.h" #include /** diff --git a/ipc/MessageQueueMessageIF.h b/ipc/MessageQueueMessageIF.h index 91753ced9..daf7e8bb2 100644 --- a/ipc/MessageQueueMessageIF.h +++ b/ipc/MessageQueueMessageIF.h @@ -1,24 +1,13 @@ #ifndef FRAMEWORK_IPC_MESSAGEQUEUEMESSAGEIF_H_ #define FRAMEWORK_IPC_MESSAGEQUEUEMESSAGEIF_H_ + +#include "messageQueueDefintions.h" #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 = -1; + /** * @brief This constants defines the size of the header, * which is added to every message. diff --git a/ipc/MessageQueueSenderIF.h b/ipc/MessageQueueSenderIF.h index b967bbdf5..beb27f507 100644 --- a/ipc/MessageQueueSenderIF.h +++ b/ipc/MessageQueueSenderIF.h @@ -1,6 +1,7 @@ #ifndef FRAMEWORK_IPC_MESSAGEQUEUESENDERIF_H_ #define FRAMEWORK_IPC_MESSAGEQUEUESENDERIF_H_ +#include "../ipc/MessageQueueIF.h" #include "../ipc/MessageQueueMessageIF.h" #include "../objectmanager/ObjectManagerIF.h" @@ -15,7 +16,7 @@ public: */ static ReturnValue_t sendMessage(MessageQueueId_t sendTo, MessageQueueMessageIF* message, - MessageQueueId_t sentFrom = MessageQueueMessageIF::NO_QUEUE, + MessageQueueId_t sentFrom = MessageQueueIF::NO_QUEUE, bool ignoreFault = false); private: MessageQueueSenderIF() {} diff --git a/ipc/QueueFactory.h b/ipc/QueueFactory.h index be5544061..5fbfdf2d1 100644 --- a/ipc/QueueFactory.h +++ b/ipc/QueueFactory.h @@ -2,7 +2,9 @@ #define FRAMEWORK_IPC_QUEUEFACTORY_H_ #include "MessageQueueIF.h" +#include "MessageQueueMessage.h" #include + /** * Creates message queues. * This class is a "singleton" interface, i.e. it provides an diff --git a/ipc/messageQueueDefintions.h b/ipc/messageQueueDefintions.h new file mode 100644 index 000000000..3d625de14 --- /dev/null +++ b/ipc/messageQueueDefintions.h @@ -0,0 +1,18 @@ +#ifndef FSFW_IPC_MESSAGEQUEUEDEFINTIONS_H_ +#define FSFW_IPC_MESSAGEQUEUEDEFINTIONS_H_ + +#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.. + */ +using MessageQueueId_t = uint32_t; + +#endif /* FSFW_IPC_MESSAGEQUEUEDEFINTIONS_H_ */ diff --git a/modes/ModeHelper.cpp b/modes/ModeHelper.cpp index e7fc0bcb4..68f009e7b 100644 --- a/modes/ModeHelper.cpp +++ b/modes/ModeHelper.cpp @@ -1,3 +1,4 @@ +#include "../ipc/MessageQueueSenderIF.h" #include "../modes/HasModesIF.h" #include "../modes/ModeHelper.h" #include "../serviceinterface/ServiceInterfaceStream.h" @@ -35,7 +36,7 @@ ReturnValue_t ModeHelper::handleModeCommand(CommandMessage* command) { commandedMode = mode; commandedSubmode = submode; - if ((parentQueueId != MessageQueueMessageIF::NO_QUEUE) + if ((parentQueueId != MessageQueueIF::NO_QUEUE) && (theOneWhoCommandedAMode != parentQueueId)) { owner->setToExternalControl(); } @@ -73,13 +74,13 @@ void ModeHelper::modeChanged(Mode_t ownerMode, Submode_t ownerSubmode) { forced = false; sendModeReplyMessage(ownerMode, ownerSubmode); sendModeInfoMessage(ownerMode, ownerSubmode); - theOneWhoCommandedAMode = MessageQueueMessageIF::NO_QUEUE; + theOneWhoCommandedAMode = MessageQueueIF::NO_QUEUE; } void ModeHelper::sendModeReplyMessage(Mode_t ownerMode, Submode_t ownerSubmode) { CommandMessage reply; - if (theOneWhoCommandedAMode != MessageQueueMessageIF::NO_QUEUE) + if (theOneWhoCommandedAMode != MessageQueueIF::NO_QUEUE) { if (ownerMode != commandedMode or ownerSubmode != commandedSubmode) { @@ -101,7 +102,7 @@ void ModeHelper::sendModeInfoMessage(Mode_t ownerMode, Submode_t ownerSubmode) { CommandMessage reply; if (theOneWhoCommandedAMode != parentQueueId - and parentQueueId != MessageQueueMessageIF::NO_QUEUE) + and parentQueueId != MessageQueueIF::NO_QUEUE) { ModeMessage::setModeMessage(&reply, ModeMessage::REPLY_MODE_INFO, ownerMode, ownerSubmode); diff --git a/osal/FreeRTOS/MessageQueue.cpp b/osal/FreeRTOS/MessageQueue.cpp index e3e0ef1c1..2dfe5ab65 100644 --- a/osal/FreeRTOS/MessageQueue.cpp +++ b/osal/FreeRTOS/MessageQueue.cpp @@ -1,5 +1,5 @@ #include "MessageQueue.h" - +#include "../../objectmanager/ObjectManagerIF.h" #include "../../serviceinterface/ServiceInterfaceStream.h" // TODO I guess we should have a way of checking if we are in an ISR and then @@ -40,7 +40,7 @@ ReturnValue_t MessageQueue::sendToDefaultFrom(MessageQueueMessageIF* message, } ReturnValue_t MessageQueue::reply(MessageQueueMessageIF* message) { - if (this->lastPartner != MessageQueueMessageIF::NO_QUEUE) { + if (this->lastPartner != MessageQueueIF::NO_QUEUE) { return sendMessageFrom(this->lastPartner, message, this->getId()); } else { return NO_REPLY_PARTNER; @@ -58,8 +58,8 @@ ReturnValue_t MessageQueue::sendMessageFrom(MessageQueueId_t sendTo, ReturnValue_t MessageQueue::handleSendResult(BaseType_t result, bool ignoreFault) { if (result != pdPASS) { if (not ignoreFault) { - InternalErrorReporterIF* internalErrorReporter = - objectManager->get( + InternalErrorReporterIF* internalErrorReporter = objectManager-> + get( objects::INTERNAL_ERROR_REPORTER); if (internalErrorReporter != nullptr) { internalErrorReporter->queueMessageNotSent(); diff --git a/osal/FreeRTOS/MessageQueue.h b/osal/FreeRTOS/MessageQueue.h index e953ec34b..b99bf7c8d 100644 --- a/osal/FreeRTOS/MessageQueue.h +++ b/osal/FreeRTOS/MessageQueue.h @@ -8,6 +8,7 @@ #include #include +#include // TODO: this class assumes that MessageQueueId_t is the same size as void* // (the FreeRTOS handle type), compiler will catch this but it might be nice @@ -139,8 +140,8 @@ protected: private: bool defaultDestinationSet = false; QueueHandle_t handle; - MessageQueueId_t defaultDestination = 0; - MessageQueueId_t lastPartner = 0; + MessageQueueId_t defaultDestination = MessageQueueIF::NO_QUEUE; + MessageQueueId_t lastPartner = MessageQueueIF::NO_QUEUE; const size_t maxMessageSize; //! Stores the current system context CallContext callContext = CallContext::TASK; diff --git a/osal/FreeRTOS/QueueFactory.cpp b/osal/FreeRTOS/QueueFactory.cpp index cd99afd98..5b3c73dca 100644 --- a/osal/FreeRTOS/QueueFactory.cpp +++ b/osal/FreeRTOS/QueueFactory.cpp @@ -1,3 +1,4 @@ +#include "../../ipc/MessageQueueSenderIF.h" #include "../../ipc/QueueFactory.h" #include "../../osal/FreeRTOS/MessageQueue.h"