diff --git a/src/fsfw/devicehandlers/AssemblyBase.cpp b/src/fsfw/devicehandlers/AssemblyBase.cpp index c29022e5..45f62ba0 100644 --- a/src/fsfw/devicehandlers/AssemblyBase.cpp +++ b/src/fsfw/devicehandlers/AssemblyBase.cpp @@ -196,7 +196,7 @@ ReturnValue_t AssemblyBase::checkModeCommand(Mode_t mode, Submode_t submode, } if ((mode != MODE_ON) && (mode != DeviceHandlerIF::MODE_NORMAL)) { - return INVALID_MODE; + return INVALID_MODE_RETVAL; } if (internalState != STATE_NONE) { diff --git a/src/fsfw/devicehandlers/AssemblyBase.h b/src/fsfw/devicehandlers/AssemblyBase.h index 3e235928..f6192533 100644 --- a/src/fsfw/devicehandlers/AssemblyBase.h +++ b/src/fsfw/devicehandlers/AssemblyBase.h @@ -53,7 +53,7 @@ class AssemblyBase : public SubsystemBase { * @param mode * @param submode * @return - * - @c RETURN_OK if ok + * - @c RETURN_OK if OK * - @c NEED_SECOND_STEP if children need to be commanded again */ virtual ReturnValue_t commandChildren(Mode_t mode, Submode_t submode) = 0; @@ -120,8 +120,19 @@ class AssemblyBase : public SubsystemBase { virtual ReturnValue_t handleHealthReply(CommandMessage *message); - virtual void performChildOperation(); + /** + * @brief Default periodic handler + * @details + * This is the default periodic handler which will be called by the SubsystemBase + * performOperation. It performs the child transitions or reacts to changed health/mode states + * of children objects + */ + virtual void performChildOperation() override; + /** + * This function handles changed mode or health states of children + * @return + */ bool handleChildrenChanged(); /** @@ -134,12 +145,31 @@ class AssemblyBase : public SubsystemBase { bool handleChildrenChangedHealth(); + /** + * Core transition handler. The default implementation will only do something if + * #commandsOutstanding is smaller or equal to zero, which means that all mode commands + * from the #doPerformTransition call were executed successfully. + * + * Unless a second step was requested, the function will then use #checkChildrenState to + * determine whether the target mode was reached. + * + * There is some special handling for certain (internal) modes: + * - A second step is necessary. #commandChildren will be performed again + * - The device health was overwritten. #commandChildren will be called + * - A recovery is ongoing. #checkAndHandleRecovery will be called. + */ virtual void handleChildrenTransition(); ReturnValue_t checkModeCommand(Mode_t mode, Submode_t submode, uint32_t *msToReachTheMode); virtual void startTransition(Mode_t mode, Submode_t submode); + /** + * This function starts the transition by setting the internal #targetSubmode and #targetMode + * variables and then calling the #commandChildren function. + * @param mode + * @param submode + */ virtual void doStartTransition(Mode_t mode, Submode_t submode); virtual bool isInTransition(); diff --git a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp index 03f8da54..6213ea86 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerBase.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerBase.cpp @@ -403,7 +403,7 @@ ReturnValue_t DeviceHandlerBase::isModeCombinationValid(Mode_t mode, Submode_t s return INVALID_SUBMODE; } default: - return HasModesIF::INVALID_MODE; + return HasModesIF::INVALID_MODE_RETVAL; } } diff --git a/src/fsfw/modes/HasModesIF.h b/src/fsfw/modes/HasModesIF.h index 850d4349..d59e0294 100644 --- a/src/fsfw/modes/HasModesIF.h +++ b/src/fsfw/modes/HasModesIF.h @@ -13,7 +13,7 @@ class HasModesIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::HAS_MODES_IF; - static const ReturnValue_t INVALID_MODE = MAKE_RETURN_CODE(0x01); + static const ReturnValue_t INVALID_MODE_RETVAL = MAKE_RETURN_CODE(0x01); static const ReturnValue_t TRANS_NOT_ALLOWED = MAKE_RETURN_CODE(0x02); static const ReturnValue_t IN_TRANSITION = MAKE_RETURN_CODE(0x03); static const ReturnValue_t INVALID_SUBMODE = MAKE_RETURN_CODE(0x04); @@ -37,9 +37,13 @@ class HasModesIF { //! The device is powered and ready to perform operations. In this mode, no commands are //! sent by the device handler itself, but direct commands van be commanded and will be //! interpreted - static const Mode_t MODE_ON = 1; + static constexpr Mode_t MODE_ON = 1; //! The device is powered off. The only command accepted in this mode is a mode change to on. - static const Mode_t MODE_OFF = 0; + static constexpr Mode_t MODE_OFF = 0; + + static constexpr Mode_t INVALID_MODE = -1; + static constexpr Mode_t UNDEFINED_MODE = -2; + //! To avoid checks against magic number "0". static const Submode_t SUBMODE_NONE = 0; diff --git a/src/fsfw/parameters/HasParametersIF.h b/src/fsfw/parameters/HasParametersIF.h index 48557b4a..e95b69ab 100644 --- a/src/fsfw/parameters/HasParametersIF.h +++ b/src/fsfw/parameters/HasParametersIF.h @@ -66,7 +66,7 @@ class HasParametersIF { * @param newValues * @param startAtIndex Linear index, runs left to right, top to bottom for * matrix indexes. - * @return + * @return RETURN_OK if parameter is valid and a set function of the parameter wrapper was called. */ virtual ReturnValue_t getParameter(uint8_t domainId, uint8_t uniqueIdentifier, ParameterWrapper *parameterWrapper, diff --git a/src/fsfw/parameters/ParameterWrapper.cpp b/src/fsfw/parameters/ParameterWrapper.cpp index 27552290..e772f1b6 100644 --- a/src/fsfw/parameters/ParameterWrapper.cpp +++ b/src/fsfw/parameters/ParameterWrapper.cpp @@ -211,9 +211,13 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from, if (data == nullptr) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "ParameterWrapper::copyFrom: Called on read-only variable!" << std::endl; + sif::warning << "ParameterWrapper::copyFrom: Called on read-only variable or " + "data pointer not set" + << std::endl; #else - sif::printWarning("ParameterWrapper::copyFrom: Called on read-only variable!\n"); + sif::printWarning( + "ParameterWrapper::copyFrom: Called on read-only variable " + "or data pointer not set\n"); #endif #endif /* FSFW_VERBOSE_LEVEL >= 1 */ return READONLY; @@ -222,9 +226,9 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from, if (from->readonlyData == nullptr) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "ParameterWrapper::copyFrom: Source not set!" << std::endl; + sif::warning << "ParameterWrapper::copyFrom: Source not set" << std::endl; #else - sif::printWarning("ParameterWrapper::copyFrom: Source not set!\n"); + sif::printWarning("ParameterWrapper::copyFrom: Source not set\n"); #endif #endif /* FSFW_VERBOSE_LEVEL >= 1 */ return SOURCE_NOT_SET; @@ -233,9 +237,9 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from, if (type != from->type) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "ParameterWrapper::copyFrom: Datatype missmatch!" << std::endl; + sif::warning << "ParameterWrapper::copyFrom: Datatype missmatch" << std::endl; #else - sif::printWarning("ParameterWrapper::copyFrom: Datatype missmatch!\n"); + sif::printWarning("ParameterWrapper::copyFrom: Datatype missmatch\n"); #endif #endif /* FSFW_VERBOSE_LEVEL >= 1 */ return DATATYPE_MISSMATCH; @@ -245,9 +249,9 @@ ReturnValue_t ParameterWrapper::copyFrom(const ParameterWrapper *from, if (rows == 0 or columns == 0) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "ParameterWrapper::copyFrom: Columns or rows zero!" << std::endl; + sif::warning << "ParameterWrapper::copyFrom: Columns or rows zero" << std::endl; #else - sif::printWarning("ParameterWrapper::copyFrom: Columns or rows zero!\n"); + sif::printWarning("ParameterWrapper::copyFrom: Columns or rows zero\n"); #endif #endif /* FSFW_VERBOSE_LEVEL >= 1 */ return COLUMN_OR_ROWS_ZERO; diff --git a/src/fsfw/subsystem/SubsystemBase.cpp b/src/fsfw/subsystem/SubsystemBase.cpp index f177ecd4..71c47c5f 100644 --- a/src/fsfw/subsystem/SubsystemBase.cpp +++ b/src/fsfw/subsystem/SubsystemBase.cpp @@ -33,8 +33,9 @@ ReturnValue_t SubsystemBase::registerChild(object_id_t objectId) { info.mode = MODE_OFF; } } else { + // intentional to force an initial command during system startup info.commandQueue = child->getCommandQueue(); - info.mode = -1; // intentional to force an initial command during system startup + info.mode = HasModesIF::UNDEFINED_MODE; } info.submode = SUBMODE_NONE; diff --git a/tests/src/fsfw_tests/integration/assemblies/TestAssembly.cpp b/tests/src/fsfw_tests/integration/assemblies/TestAssembly.cpp index a0313f96..8e79bbc6 100644 --- a/tests/src/fsfw_tests/integration/assemblies/TestAssembly.cpp +++ b/tests/src/fsfw_tests/integration/assemblies/TestAssembly.cpp @@ -125,7 +125,7 @@ ReturnValue_t TestAssembly::isModeCombinationValid(Mode_t mode, Submode_t submod return INVALID_SUBMODE; } } - return INVALID_MODE; + return INVALID_MODE_RETVAL; } ReturnValue_t TestAssembly::initialize() { diff --git a/tests/src/fsfw_tests/integration/devices/TestDeviceHandler.cpp b/tests/src/fsfw_tests/integration/devices/TestDeviceHandler.cpp index 41098723..cd15d6e0 100644 --- a/tests/src/fsfw_tests/integration/devices/TestDeviceHandler.cpp +++ b/tests/src/fsfw_tests/integration/devices/TestDeviceHandler.cpp @@ -208,7 +208,7 @@ ReturnValue_t TestDevice::buildNormalModeCommand(DeviceCommandId_t deviceCommand const uint8_t* commandData, size_t commandDataLen) { if (fullInfoPrintout) { -#if OBSW_VERBOSE_LEVEL >= 3 +#if FSFW_VERBOSE_LEVEL >= 3 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "TestDevice::buildTestCommand1: Building normal command" << std::endl; #else @@ -351,7 +351,7 @@ ReturnValue_t TestDevice::scanForReply(const uint8_t* start, size_t len, DeviceC switch (pendingCmd) { case (TEST_NORMAL_MODE_CMD): { if (fullInfoPrintout) { -#if OBSW_VERBOSE_LEVEL >= 3 +#if FSFW_VERBOSE_LEVEL >= 3 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "TestDevice::scanForReply: Reply for normal commnand (ID " << TEST_NORMAL_MODE_CMD << ") received!" << std::endl; @@ -678,7 +678,6 @@ ReturnValue_t TestDevice::getParameter(uint8_t domainId, uint8_t uniqueId, int32_t newValue = 0; ReturnValue_t result = newValues->getElement(&newValue, 0, 0); if (result == HasReturnvaluesIF::RETURN_OK) { -#if OBSW_DEVICE_HANDLER_PRINTOUT == 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "TestDevice" << deviceIdx << "::getParameter: Setting parameter 1 to " @@ -688,7 +687,6 @@ ReturnValue_t TestDevice::getParameter(uint8_t domainId, uint8_t uniqueId, sif::printInfo("TestDevice%d::getParameter: Setting parameter 1 to new value %lu\n", deviceIdx, static_cast(newValue)); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ -#endif /* OBSW_DEVICE_HANDLER_PRINTOUT == 1 */ } } parameterWrapper->set(testParameter1); @@ -702,7 +700,6 @@ ReturnValue_t TestDevice::getParameter(uint8_t domainId, uint8_t uniqueId, newValues->getElement(newVector + 2, 0, 2) != RETURN_OK) { return HasReturnvaluesIF::RETURN_FAILED; } -#if OBSW_DEVICE_HANDLER_PRINTOUT == 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "TestDevice" << deviceIdx << "::getParameter: Setting parameter 3 to " @@ -715,7 +712,6 @@ ReturnValue_t TestDevice::getParameter(uint8_t domainId, uint8_t uniqueId, "[%f, %f, %f]\n", deviceIdx, newVector[0], newVector[1], newVector[2]); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ -#endif /* OBSW_DEVICE_HANDLER_PRINTOUT == 1 */ } parameterWrapper->setVector(vectorFloatParams2); break;