From 03ef63302b681651daefde2f17421dabda4c6945 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 20:45:15 +0200 Subject: [PATCH 1/2] Replaced Magic Numbers --- subsystem/SubsystemBase.cpp | 9 ++++----- subsystem/SubsystemBase.h | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index bcfa4b0e3..565e0712d 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -5,8 +5,7 @@ SubsystemBase::SubsystemBase(object_id_t setObjectId, object_id_t parent, Mode_t initialMode, uint16_t commandQueueDepth) : - SystemObject(setObjectId), mode(initialMode), submode(SUBMODE_NONE), - childrenChangedMode(false), + SystemObject(setObjectId), mode(initialMode), commandQueue(QueueFactory::instance()->createMessageQueue( commandQueueDepth, CommandMessage::MAX_MESSAGE_SIZE)), healthHelper(this, setObjectId), modeHelper(this), parentId(parent) { @@ -167,16 +166,16 @@ MessageQueueId_t SubsystemBase::getCommandQueue() const { } ReturnValue_t SubsystemBase::initialize() { - MessageQueueId_t parentQueue = 0; + MessageQueueId_t parentQueue = MessageQueueIF::NO_QUEUE; ReturnValue_t result = SystemObject::initialize(); if (result != RETURN_OK) { return result; } - if (parentId != 0) { + if (parentId != objects::NO_OBJECT) { SubsystemBase *parent = objectManager->get(parentId); - if (parent == NULL) { + if (parent == nullptr) { return RETURN_FAILED; } parentQueue = parent->getCommandQueue(); diff --git a/subsystem/SubsystemBase.h b/subsystem/SubsystemBase.h index b8e4f9029..5d107701d 100644 --- a/subsystem/SubsystemBase.h +++ b/subsystem/SubsystemBase.h @@ -56,9 +56,9 @@ protected: Mode_t mode; - Submode_t submode; + Submode_t submode = SUBMODE_NONE; - bool childrenChangedMode; + bool childrenChangedMode = false; /** * Always check this against <=0, so you are robust against too many replies From 06631d06a57c30946563d8726ea9ef9a7e6c1c84 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 22:01:56 +0200 Subject: [PATCH 2/2] Added comments in AssemblyBase and SubsystemBase --- devicehandlers/AssemblyBase.h | 22 +++++++++++++++++++--- subsystem/SubsystemBase.h | 11 +++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/devicehandlers/AssemblyBase.h b/devicehandlers/AssemblyBase.h index 353d5f899..6cac81b49 100644 --- a/devicehandlers/AssemblyBase.h +++ b/devicehandlers/AssemblyBase.h @@ -24,6 +24,13 @@ * 1. check logic when active-> checkChildrenStateOn * 2. transition logic to change the mode -> commandChildren * + * Important: + * + * The implementation must call registerChild(object_id_t child) + * for all commanded children during initialization. + * The implementation must call the initialization function of the base class. + * (This will call the function in SubsystemBase) + * */ class AssemblyBase: public SubsystemBase { public: @@ -41,9 +48,6 @@ public: virtual ~AssemblyBase(); protected: - - // SHOULDDO: Change that OVERWRITE_HEALTH may be returned - // (or return internalState directly?) /** * Command children to reach [mode,submode] combination * Can be done by setting #commandsOutstanding correctly, @@ -68,6 +72,18 @@ protected: virtual ReturnValue_t checkChildrenStateOn(Mode_t wantedMode, Submode_t wantedSubmode) = 0; + /** + * Check whether a combination of mode and submode is valid. + * + * Ground Controller like precise return values from HasModesIF. + * So, please return any of them. + * + * @param mode The targeted mode + * @param submode The targeted submmode + * @return Any information why this combination is invalid from HasModesIF + * like HasModesIF::INVALID_SUBMODE. + * On success return HasReturnvaluesIF::RETURN_OK + */ virtual ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) = 0; diff --git a/subsystem/SubsystemBase.h b/subsystem/SubsystemBase.h index 5d107701d..6b2e9b5fd 100644 --- a/subsystem/SubsystemBase.h +++ b/subsystem/SubsystemBase.h @@ -37,6 +37,17 @@ public: virtual MessageQueueId_t getCommandQueue() const override; + /** + * Function to register the child objects. + * Performs a checks if the child does implement HasHealthIF and/or HasModesIF + * + * Also adds them to the internal childrenMap. + * + * @param objectId + * @return RETURN_OK if successful + * CHILD_DOESNT_HAVE_MODES if Child is no HasHealthIF and no HasModesIF + * COULD_NOT_INSERT_CHILD If the Child could not be added to the ChildrenMap + */ ReturnValue_t registerChild(object_id_t objectId); virtual ReturnValue_t initialize() override;