From 252bfa5c3935681ebd4717027eb67ce9dc5fcb0f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 14 Dec 2020 11:35:45 +0100 Subject: [PATCH] subsystem convergence --- subsystem/Subsystem.cpp | 39 ++++++++++------------ subsystem/Subsystem.h | 59 ++++++++++++++++++---------------- subsystem/SubsystemBase.cpp | 64 ++++++++++++++++++------------------- subsystem/SubsystemBase.h | 27 +++++++++------- 4 files changed, 96 insertions(+), 93 deletions(-) diff --git a/subsystem/Subsystem.cpp b/subsystem/Subsystem.cpp index 50fc4d05d..9712d7d42 100644 --- a/subsystem/Subsystem.cpp +++ b/subsystem/Subsystem.cpp @@ -1,5 +1,4 @@ #include "Subsystem.h" - #include "../health/HealthMessage.h" #include "../objectmanager/ObjectManagerIF.h" #include "../serialize/SerialArrayListAdapter.h" @@ -11,21 +10,12 @@ Subsystem::Subsystem(object_id_t setObjectId, object_id_t parent, uint32_t maxNumberOfSequences, uint32_t maxNumberOfTables) : - SubsystemBase(setObjectId, parent, 0), isInTransition(false), childrenChangedHealth( - false), uptimeStartTable(0), currentTargetTable(), targetMode( - 0), targetSubmode(SUBMODE_NONE), initialMode(0), currentSequenceIterator(), modeTables( - maxNumberOfTables), modeSequences(maxNumberOfSequences), IPCStore( - NULL) -#ifdef USE_MODESTORE -,modeStore(NULL) -#endif -{ + SubsystemBase(setObjectId, parent, 0), isInTransition(false), + childrenChangedHealth(false), currentTargetTable(), + targetSubmode(SUBMODE_NONE), currentSequenceIterator(), + modeTables(maxNumberOfTables), modeSequences(maxNumberOfSequences) {} -} - -Subsystem::~Subsystem() { - //Auto-generated destructor stub -} +Subsystem::~Subsystem() {} ReturnValue_t Subsystem::checkSequence(HybridIterator iter, Mode_t fallbackSequence) { @@ -351,7 +341,8 @@ ReturnValue_t Subsystem::addSequence(ArrayList *sequence, ReturnValue_t result; - //Before initialize() is called, tables must not be checked as the children are not added yet. + //Before initialize() is called, tables must not be checked as the + //children are not added yet. //Sequences added before are checked by initialize() if (!preInit) { result = checkSequence( @@ -397,8 +388,8 @@ ReturnValue_t Subsystem::addTable(ArrayList *table, Mode_t id, ReturnValue_t result; - //Before initialize() is called, tables must not be checked as the children are not added yet. - //Tables added before are checked by initialize() + //Before initialize() is called, tables must not be checked as the children + //are not added yet. Tables added before are checked by initialize() if (!preInit) { result = checkTable( HybridIterator(table->front(), table->back())); @@ -589,12 +580,14 @@ void Subsystem::transitionFailed(ReturnValue_t failureCode, triggerEvent(MODE_TRANSITION_FAILED, failureCode, parameter); if (mode == targetMode) { //already tried going back to the current mode - //go into fallback mode, also set current mode to fallback mode, so we come here at the next fail + //go into fallback mode, also set current mode to fallback mode, + //so we come here at the next fail modeHelper.setForced(true); ReturnValue_t result; if ((result = checkSequence(getFallbackSequence(mode))) != RETURN_OK) { triggerEvent(FALLBACK_FAILED, result, getFallbackSequence(mode)); - isInTransition = false; //keep still and allow arbitrary mode commands to recover + //keep still and allow arbitrary mode commands to recover + isInTransition = false; return; } mode = getFallbackSequence(mode); @@ -658,8 +651,10 @@ void Subsystem::cantKeepMode() { modeHelper.setForced(true); - //already set the mode, so that we do not try to go back in our old mode when the transition fails + //already set the mode, so that we do not try to go back in our old mode + //when the transition fails mode = getFallbackSequence(mode); - //SHOULDDO: We should store submodes for fallback sequence as well, otherwise we should get rid of submodes completely. + //SHOULDDO: We should store submodes for fallback sequence as well, + //otherwise we should get rid of submodes completely. startTransition(mode, SUBMODE_NONE); } diff --git a/subsystem/Subsystem.h b/subsystem/Subsystem.h index a40b8028d..09d7850ab 100644 --- a/subsystem/Subsystem.h +++ b/subsystem/Subsystem.h @@ -1,14 +1,19 @@ -#ifndef SUBSYSTEM_H_ -#define SUBSYSTEM_H_ +#ifndef FSFW_SUBSYSTEM_SUBSYSTEM_H_ +#define FSFW_SUBSYSTEM_SUBSYSTEM_H_ + +#include "SubsystemBase.h" +#include "modes/ModeDefinitions.h" #include "../container/FixedArrayList.h" #include "../container/FixedMap.h" #include "../container/HybridIterator.h" #include "../container/SinglyLinkedList.h" #include "../serialize/SerialArrayListAdapter.h" -#include "modes/ModeDefinitions.h" -#include "SubsystemBase.h" +/** + * @brief TODO: documentation missing + * @details + */ class Subsystem: public SubsystemBase, public HasModeSequenceIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::SUBSYSTEM; @@ -30,8 +35,13 @@ public: static const ReturnValue_t TARGET_TABLE_NOT_REACHED = MAKE_RETURN_CODE(0xA1); static const ReturnValue_t TABLE_CHECK_FAILED = MAKE_RETURN_CODE(0xA2); - - + /** + * TODO: Doc for constructor + * @param setObjectId + * @param parent + * @param maxNumberOfSequences + * @param maxNumberOfTables + */ Subsystem(object_id_t setObjectId, object_id_t parent, uint32_t maxNumberOfSequences, uint32_t maxNumberOfTables); virtual ~Subsystem(); @@ -44,31 +54,27 @@ public: void setInitialMode(Mode_t mode); - virtual ReturnValue_t initialize(); + virtual ReturnValue_t initialize() override; - virtual ReturnValue_t checkObjectConnections(); + virtual ReturnValue_t checkObjectConnections() override; - virtual MessageQueueId_t getSequenceCommandQueue() const; + virtual MessageQueueId_t getSequenceCommandQueue() const override; /** - * - * - * IMPORTANT: Do not call on non existing sequence! Use existsSequence() first - * + * @brief Checks whether a sequence, identified by a mode. * @param sequence * @return */ ReturnValue_t checkSequence(Mode_t sequence); /** - * - * - * IMPORTANT: Do not call on non existing sequence! Use existsSequence() first - * + * @brief Checks whether a sequence, identified by a mode list iterator + * and a fallback sequence. * @param iter * @return */ - ReturnValue_t checkSequence(HybridIterator iter, Mode_t fallbackSequence); + ReturnValue_t checkSequence(HybridIterator iter, + Mode_t fallbackSequence); protected: struct EntryPointer { @@ -92,15 +98,15 @@ protected: bool childrenChangedHealth; - uint32_t uptimeStartTable; + uint32_t uptimeStartTable = 0; HybridIterator currentTargetTable; - Mode_t targetMode; + Mode_t targetMode = 0; Submode_t targetSubmode; - Mode_t initialMode; + Mode_t initialMode = 0; HybridIterator currentSequenceIterator; @@ -108,10 +114,10 @@ protected: FixedMap modeSequences; - StorageManagerIF *IPCStore; + StorageManagerIF *IPCStore = nullptr; #ifdef USE_MODESTORE - ModeStoreIF *modeStore; + ModeStoreIF *modeStore = nullptr; #endif bool existsModeSequence(Mode_t id); @@ -124,8 +130,6 @@ protected: HybridIterator getCurrentTable(); -// void startSequence(Mode_t sequence); - /** * DO NOT USE ON NON EXISTING SEQUENCE * @@ -153,7 +157,8 @@ protected: virtual void startTransition(Mode_t mode, Submode_t submode); - void sendSerializablesAsCommandMessage(Command_t command, SerializeIF **elements, uint8_t count); + void sendSerializablesAsCommandMessage(Command_t command, + SerializeIF **elements, uint8_t count); void transitionFailed(ReturnValue_t failureCode, uint32_t parameter); @@ -161,4 +166,4 @@ protected: }; -#endif /* SUBSYSTEM_H_ */ +#endif /* FSFW_SUBSYSTEM_SUBSYSTEM_H_ */ diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index 787d6be12..f60c88478 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -1,15 +1,15 @@ -#include "SubsystemBase.h" - #include "../serviceinterface/ServiceInterfaceStream.h" +#include "../serviceinterface/ServiceInterfaceStream.h" +#include "../subsystem/SubsystemBase.h" #include "../ipc/QueueFactory.h" 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), commandsOutstanding(0), commandQueue(NULL), + childrenChangedMode(false), + commandQueue(QueueFactory::instance()->createMessageQueue( + commandQueueDepth, CommandMessage::MAX_MESSAGE_SIZE)), healthHelper(this, setObjectId), modeHelper(this), parentId(parent) { - commandQueue = QueueFactory::instance()->createMessageQueue(commandQueueDepth, - CommandMessage::MAX_MESSAGE_SIZE); } SubsystemBase::~SubsystemBase() { @@ -21,10 +21,11 @@ ReturnValue_t SubsystemBase::registerChild(object_id_t objectId) { ChildInfo info; HasModesIF *child = objectManager->get(objectId); - //This is a rather ugly hack to have the changedHealth info for all children available. (needed for FOGs). + // This is a rather ugly hack to have the changedHealth info for all + // children available. HasHealthIF* healthChild = objectManager->get(objectId); - if (child == NULL) { - if (healthChild == NULL) { + if (child == nullptr) { + if (healthChild == nullptr) { return CHILD_DOESNT_HAVE_MODES; } else { info.commandQueue = healthChild->getCommandQueue(); @@ -38,14 +39,11 @@ ReturnValue_t SubsystemBase::registerChild(object_id_t objectId) { info.submode = SUBMODE_NONE; info.healthChanged = false; - std::pair::iterator, bool> returnValue = - childrenMap.insert( - std::pair(objectId, info)); - if (!(returnValue.second)) { + auto resultPair = childrenMap.emplace(objectId, info); + if (not resultPair.second) { return COULD_NOT_INSERT_CHILD; - } else { - return RETURN_OK; } + return RETURN_OK; } ReturnValue_t SubsystemBase::checkStateAgainstTable( @@ -76,15 +74,15 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable( return RETURN_OK; } -void SubsystemBase::executeTable(HybridIterator tableIter, Submode_t targetSubmode) { - - CommandMessage message; +void SubsystemBase::executeTable(HybridIterator tableIter, + Submode_t targetSubmode) { + CommandMessage command; std::map::iterator iter; commandsOutstanding = 0; - for (; tableIter.value != NULL; ++tableIter) { + for (; tableIter.value != nullptr; ++tableIter) { object_id_t object = tableIter.value->getObject(); if ((iter = childrenMap.find(object)) == childrenMap.end()) { //illegal table entry, should only happen due to misconfigured mode table @@ -100,17 +98,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,17 +117,17 @@ 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 } ReturnValue_t result = commandQueue->sendMessage( - iter->second.commandQueue, &message); + iter->second.commandQueue, &command); if (result == RETURN_OK) { ++commandsOutstanding; } @@ -306,31 +304,31 @@ void SubsystemBase::announceMode(bool recursive) { void SubsystemBase::checkCommandQueue() { ReturnValue_t result; - CommandMessage message; + CommandMessage command; - 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::UNKNOWN_COMMAND, - message.getCommand()); + command.getCommand()); replyToCommand(&reply); } } diff --git a/subsystem/SubsystemBase.h b/subsystem/SubsystemBase.h index 61a7eaef1..b8e4f9029 100644 --- a/subsystem/SubsystemBase.h +++ b/subsystem/SubsystemBase.h @@ -1,5 +1,7 @@ -#ifndef SUBSYSTEMBASE_H_ -#define SUBSYSTEMBASE_H_ +#ifndef FSFW_SUBSYSTEM_SUBSYSTEMBASE_H_ +#define FSFW_SUBSYSTEM_SUBSYSTEMBASE_H_ + +#include "modes/HasModeSequenceIF.h" #include "../container/HybridIterator.h" #include "../health/HasHealthIF.h" @@ -7,11 +9,14 @@ #include "../modes/HasModesIF.h" #include "../objectmanager/SystemObject.h" #include "../returnvalues/HasReturnvaluesIF.h" -#include "modes/HasModeSequenceIF.h" #include "../tasks/ExecutableObjectIF.h" #include "../ipc/MessageQueueIF.h" #include +/** + * @defgroup subsystems Subsystem Objects + * Contains all Subsystem and Assemblies + */ class SubsystemBase: public SystemObject, public HasModesIF, public HasHealthIF, @@ -30,17 +35,17 @@ public: Mode_t initialMode = 0, uint16_t commandQueueDepth = 8); virtual ~SubsystemBase(); - virtual MessageQueueId_t getCommandQueue() const; + virtual MessageQueueId_t getCommandQueue() const override; ReturnValue_t registerChild(object_id_t objectId); - virtual ReturnValue_t initialize(); + virtual ReturnValue_t initialize() override; - virtual ReturnValue_t performOperation(uint8_t opCode); + virtual ReturnValue_t performOperation(uint8_t opCode) override; - virtual ReturnValue_t setHealth(HealthState health); + virtual ReturnValue_t setHealth(HealthState health) override; - virtual HasHealthIF::HealthState getHealth(); + virtual HasHealthIF::HealthState getHealth() override; protected: struct ChildInfo { @@ -58,9 +63,9 @@ protected: /** * Always check this against <=0, so you are robust against too many replies */ - int32_t commandsOutstanding; + int32_t commandsOutstanding = 0; - MessageQueueIF* commandQueue; + MessageQueueIF* commandQueue = nullptr; HealthHelper healthHelper; @@ -122,4 +127,4 @@ protected: virtual void modeChanged(); }; -#endif /* SUBSYSTEMBASE_H_ */ +#endif /* FSFW_SUBSYSTEM_SUBSYSTEMBASE_H_ */