From 0ade2ae0ee3d5ef4bd53736fa117d2e4d2834560 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 5 Apr 2023 15:05:32 +0200 Subject: [PATCH 1/5] str assembly mode checks --- mission/acs/str/StarTrackerHandler.h | 3 --- mission/acs/str/strHelpers.h | 3 +++ mission/system/acs/StrAssembly.cpp | 9 +++++++++ tmtc | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/mission/acs/str/StarTrackerHandler.h b/mission/acs/str/StarTrackerHandler.h index 69b3e121..f143a660 100644 --- a/mission/acs/str/StarTrackerHandler.h +++ b/mission/acs/str/StarTrackerHandler.h @@ -54,9 +54,6 @@ class StarTrackerHandler : public DeviceHandlerBase { Submode_t getInitialSubmode() override; - static const Submode_t SUBMODE_BOOTLOADER = 1; - static const Submode_t SUBMODE_FIRMWARE = 2; - protected: void doStartUp() override; void doShutDown() override; diff --git a/mission/acs/str/strHelpers.h b/mission/acs/str/strHelpers.h index 002d9934..4eea2635 100644 --- a/mission/acs/str/strHelpers.h +++ b/mission/acs/str/strHelpers.h @@ -11,6 +11,9 @@ namespace startracker { +static const Submode_t SUBMODE_BOOTLOADER = 1; +static const Submode_t SUBMODE_FIRMWARE = 2; + /** * @brief Returns the frame type field of a decoded frame. */ diff --git a/mission/system/acs/StrAssembly.cpp b/mission/system/acs/StrAssembly.cpp index abd7a4ce..574ad634 100644 --- a/mission/system/acs/StrAssembly.cpp +++ b/mission/system/acs/StrAssembly.cpp @@ -2,6 +2,8 @@ #include +#include "mission/acs/str/strHelpers.h" + StrAssembly::StrAssembly(object_id_t objectId) : AssemblyBase(objectId) { ModeListEntry entry; entry.setObject(objects::STAR_TRACKER); @@ -31,5 +33,12 @@ ReturnValue_t StrAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_t wan } ReturnValue_t StrAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) { + if (mode == DeviceHandlerIF::MODE_NORMAL and submode != SUBMODE_NONE) { + return HasModesIF::INVALID_SUBMODE; + } + if (mode == MODE_ON and + (submode != startracker::SUBMODE_BOOTLOADER and submode != startracker::SUBMODE_FIRMWARE)) { + return HasModesIF::INVALID_SUBMODE; + } return returnvalue::OK; } diff --git a/tmtc b/tmtc index 50668ca7..dcf7d0af 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 50668ca7a74edd4219456e393cd10f7858591130 +Subproject commit dcf7d0af71f6ba9d569f9f56604e9245a0233427 -- 2.43.0 From 600921e3a00b4b26e4c88ab238fc54777d89d3a9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 5 Apr 2023 15:11:37 +0200 Subject: [PATCH 2/5] some code fixes --- mission/acs/str/StarTrackerHandler.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mission/acs/str/StarTrackerHandler.cpp b/mission/acs/str/StarTrackerHandler.cpp index 52751f24..3837d9e5 100644 --- a/mission/acs/str/StarTrackerHandler.cpp +++ b/mission/acs/str/StarTrackerHandler.cpp @@ -277,7 +277,7 @@ void StarTrackerHandler::performOperationHook() { } } -Submode_t StarTrackerHandler::getInitialSubmode() { return SUBMODE_BOOTLOADER; } +Submode_t StarTrackerHandler::getInitialSubmode() { return startracker::SUBMODE_BOOTLOADER; } ReturnValue_t StarTrackerHandler::buildNormalDeviceCommand(DeviceCommandId_t* id) { if (strHelperHandlingSpecialRequest) { @@ -705,7 +705,7 @@ ReturnValue_t StarTrackerHandler::isModeCombinationValid(Mode_t mode, Submode_t return INVALID_SUBMODE; } case MODE_ON: - if (submode == SUBMODE_BOOTLOADER || submode == SUBMODE_FIRMWARE) { + if (submode == startracker::SUBMODE_BOOTLOADER || submode == startracker::SUBMODE_FIRMWARE) { return returnvalue::OK; } else { return INVALID_SUBMODE; @@ -736,6 +736,7 @@ void StarTrackerHandler::doTransition(Mode_t modeFrom, Submode_t subModeFrom) { } void StarTrackerHandler::doOnTransition(Submode_t subModeFrom) { + using namespace startracker; uint8_t dhbSubmode = getSubmode(); // We hide that the transition to submode firmware actually goes through the submode bootloader. // This is because the startracker always starts in bootloader mode but we want to allow direct @@ -762,6 +763,7 @@ void StarTrackerHandler::doOnTransition(Submode_t subModeFrom) { } void StarTrackerHandler::doNormalTransition(Mode_t modeFrom, Submode_t subModeFrom) { + using namespace startracker; if (subModeFrom == SUBMODE_FIRMWARE) { setMode(MODE_NORMAL); } else if (subModeFrom == SUBMODE_BOOTLOADER) { @@ -787,7 +789,7 @@ void StarTrackerHandler::bootFirmware(Mode_t toMode) { if (toMode == MODE_NORMAL) { setMode(toMode, 0); } else { - setMode(toMode, SUBMODE_FIRMWARE); + setMode(toMode, startracker::SUBMODE_FIRMWARE); } sif::info << "STR: Firmware boot success" << std::endl; internalState = InternalState::IDLE; -- 2.43.0 From f5e47c6114655e7c94119ed3e75877605270baeb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 5 Apr 2023 15:29:03 +0200 Subject: [PATCH 3/5] some more mode checks --- mission/system/acs/DualLaneAssemblyBase.cpp | 7 +++++-- mission/system/acs/DualLaneAssemblyBase.h | 3 ++- mission/system/acs/ImtqAssembly.cpp | 5 ++++- mission/system/com/SyrlinksAssembly.cpp | 11 +++++++++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/mission/system/acs/DualLaneAssemblyBase.cpp b/mission/system/acs/DualLaneAssemblyBase.cpp index ecb91689..b83b85b0 100644 --- a/mission/system/acs/DualLaneAssemblyBase.cpp +++ b/mission/system/acs/DualLaneAssemblyBase.cpp @@ -112,8 +112,11 @@ ReturnValue_t DualLaneAssemblyBase::pwrStateMachineWrapper() { ReturnValue_t DualLaneAssemblyBase::isModeCombinationValid(Mode_t mode, Submode_t submode) { using namespace duallane; - if (submode != A_SIDE and submode != B_SIDE and submode != DUAL_MODE) { - return returnvalue::FAILED; + if (mode != MODE_OFF and (submode != A_SIDE and submode != B_SIDE and submode != DUAL_MODE)) { + return HasModesIF::INVALID_SUBMODE; + } + if(mode == MODE_OFF and submode != SUBMODE_NONE) { + return HasModesIF::INVALID_SUBMODE; } return returnvalue::OK; } diff --git a/mission/system/acs/DualLaneAssemblyBase.h b/mission/system/acs/DualLaneAssemblyBase.h index d15a2f30..40d9aefc 100644 --- a/mission/system/acs/DualLaneAssemblyBase.h +++ b/mission/system/acs/DualLaneAssemblyBase.h @@ -70,7 +70,8 @@ class DualLaneAssemblyBase : public AssemblyBase, public ConfirmsFailuresIF { * @param submode */ virtual ReturnValue_t pwrStateMachineWrapper(); - virtual ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) override; + ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) override; + /** * Custom recovery implementation to ensure that the power lines are commanded off for a * recovery. diff --git a/mission/system/acs/ImtqAssembly.cpp b/mission/system/acs/ImtqAssembly.cpp index 14d90b75..37d7fcc1 100644 --- a/mission/system/acs/ImtqAssembly.cpp +++ b/mission/system/acs/ImtqAssembly.cpp @@ -35,9 +35,12 @@ ReturnValue_t ImtqAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_t wa ReturnValue_t ImtqAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) { if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) { + if(submode != SUBMODE_NONE) { + return HasModesIF::INVALID_SUBMODE; + } return returnvalue::OK; } - return returnvalue::FAILED; + return HasModesIF::INVALID_MODE; } ReturnValue_t ImtqAssembly::checkAndHandleHealthState(Mode_t deviceMode, Submode_t deviceSubmode) { diff --git a/mission/system/com/SyrlinksAssembly.cpp b/mission/system/com/SyrlinksAssembly.cpp index b5e50924..aef54d9a 100644 --- a/mission/system/com/SyrlinksAssembly.cpp +++ b/mission/system/com/SyrlinksAssembly.cpp @@ -1,6 +1,7 @@ #include "SyrlinksAssembly.h" #include +#include using namespace returnvalue; @@ -34,10 +35,16 @@ ReturnValue_t SyrlinksAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_ } ReturnValue_t SyrlinksAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) { - if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) { + if (mode == MODE_ON or mode == DeviceHandlerIF::MODE_NORMAL) { + if (submode >= com::Submode::NUM_SUBMODES or submode < com::Submode::RX_ONLY) { + return HasModesIF::INVALID_SUBMODE; + } return returnvalue::OK; } - return returnvalue::FAILED; + if(mode == MODE_OFF and submode != SUBMODE_NONE) { + return HasModesIF::INVALID_SUBMODE; + } + return returnvalue::OK; } ReturnValue_t SyrlinksAssembly::checkAndHandleHealthState(Mode_t deviceMode, -- 2.43.0 From c65b402361c182b9d3594d0dc03f1afad3e1db69 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 5 Apr 2023 15:29:44 +0200 Subject: [PATCH 4/5] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e678fff..5754910c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ will consitute of a breaking change warranting a new major release: - Adapted HK data rates to new table for LEOP SAFE mode. - GPS controller HK is now generated periodically as well. +- Better mode combination checks for assembly components. This includes: + - IMTQ assembly + - Syrlinks assembly + - Dual Lane Assembly # [v1.43.1] 2023-04-04 -- 2.43.0 From 56c5838d1579cee18b6d91a32404d3596039abdc Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 5 Apr 2023 16:06:59 +0200 Subject: [PATCH 5/5] str assembly include mode off --- mission/system/acs/StrAssembly.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mission/system/acs/StrAssembly.cpp b/mission/system/acs/StrAssembly.cpp index 574ad634..a6ae6000 100644 --- a/mission/system/acs/StrAssembly.cpp +++ b/mission/system/acs/StrAssembly.cpp @@ -33,7 +33,7 @@ ReturnValue_t StrAssembly::checkChildrenStateOn(Mode_t wantedMode, Submode_t wan } ReturnValue_t StrAssembly::isModeCombinationValid(Mode_t mode, Submode_t submode) { - if (mode == DeviceHandlerIF::MODE_NORMAL and submode != SUBMODE_NONE) { + if ((mode == DeviceHandlerIF::MODE_NORMAL or mode == MODE_OFF) and submode != SUBMODE_NONE) { return HasModesIF::INVALID_SUBMODE; } if (mode == MODE_ON and -- 2.43.0