From e365e03c2a0fc47be3a234990aa5380312736d11 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 19:00:37 +0200 Subject: [PATCH 1/6] possible bugfix --- mission/system/acs/AcsBoardAssembly.cpp | 1 + tmtc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mission/system/acs/AcsBoardAssembly.cpp b/mission/system/acs/AcsBoardAssembly.cpp index 6c4023f8..59d136a5 100644 --- a/mission/system/acs/AcsBoardAssembly.cpp +++ b/mission/system/acs/AcsBoardAssembly.cpp @@ -123,6 +123,7 @@ ReturnValue_t AcsBoardAssembly::handleNormalOrOnModeCmd(Mode_t mode, Submode_t s // TODO: Ugly hack. The base class should support arbitrary number of steps.. needsSecondStep = true; } else if (sideSwitchState == SideSwitchState::DISABLE_OTHER_SIDE) { + dualToSingleSideTransition = true; submode = targetSubmodeForSideSwitch; } auto cmdSeq = [&](object_id_t objectId, Mode_t devMode, ModeTableIdx tableIdx) { diff --git a/tmtc b/tmtc index aab50dce..5b613f98 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit aab50dce5ace6878432377fff5e6b2cd1c485213 +Subproject commit 5b613f98eea415242c152ed3498ed3c0434f139f From 65989261b0529d09821e0dfbf85a88efd6645f21 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 19:14:42 +0200 Subject: [PATCH 2/6] that should fix the issue --- mission/system/acs/AcsBoardAssembly.cpp | 15 +------------ mission/system/acs/DualLaneAssemblyBase.cpp | 24 ++++++++++++++++++++- mission/system/acs/DualLaneAssemblyBase.h | 1 + mission/system/acs/SusAssembly.cpp | 1 + 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/mission/system/acs/AcsBoardAssembly.cpp b/mission/system/acs/AcsBoardAssembly.cpp index 59d136a5..1868185e 100644 --- a/mission/system/acs/AcsBoardAssembly.cpp +++ b/mission/system/acs/AcsBoardAssembly.cpp @@ -112,20 +112,7 @@ ReturnValue_t AcsBoardAssembly::handleNormalOrOnModeCmd(Mode_t mode, Submode_t s using namespace duallane; ReturnValue_t result = returnvalue::OK; bool needsSecondStep = false; - if (sideSwitchState == SideSwitchState::REQUESTED) { - sideSwitchState = SideSwitchState::TO_DUAL; - } - // Switch to dual side first, and later switch back to the otherside - if (sideSwitchState == SideSwitchState::TO_DUAL) { - targetSubmodeForSideSwitch = static_cast(submode); - submode = Submodes::DUAL_MODE; - sideSwitchState = SideSwitchState::DISABLE_OTHER_SIDE; - // TODO: Ugly hack. The base class should support arbitrary number of steps.. - needsSecondStep = true; - } else if (sideSwitchState == SideSwitchState::DISABLE_OTHER_SIDE) { - dualToSingleSideTransition = true; - submode = targetSubmodeForSideSwitch; - } + handleSideSwitchStates(submode, needsSecondStep); auto cmdSeq = [&](object_id_t objectId, Mode_t devMode, ModeTableIdx tableIdx) { if (mode == devMode) { modeTable[tableIdx].setMode(mode); diff --git a/mission/system/acs/DualLaneAssemblyBase.cpp b/mission/system/acs/DualLaneAssemblyBase.cpp index e32717de..6c5fab89 100644 --- a/mission/system/acs/DualLaneAssemblyBase.cpp +++ b/mission/system/acs/DualLaneAssemblyBase.cpp @@ -130,7 +130,11 @@ void DualLaneAssemblyBase::handleModeReached() { // For dual to single side transition, devices should be logically off, but the switch // handling still needs to be done. if (dualToSingleSideTransition) { - pwrStateMachine.start(targetMode, targetSubmode); + if (sideSwitchState == SideSwitchState::DISABLE_OTHER_SIDE) { + pwrStateMachine.start(targetMode, targetSubmodeForSideSwitch); + } else { + pwrStateMachine.start(targetMode, targetSubmode); + } pwrStateMachineWrapper(); return; } @@ -238,6 +242,24 @@ bool DualLaneAssemblyBase::sideSwitchTransition(Mode_t mode, Submode_t submode) return false; } +void DualLaneAssemblyBase::handleSideSwitchStates(uint8_t& submode, bool& needsSecondStep) { + if (sideSwitchState == SideSwitchState::REQUESTED) { + sideSwitchState = SideSwitchState::TO_DUAL; + } + // Switch to dual side first, and later switch back to the otherside + if (sideSwitchState == SideSwitchState::TO_DUAL) { + targetSubmodeForSideSwitch = static_cast(submode); + submode = duallane::Submodes::DUAL_MODE; + sideSwitchState = SideSwitchState::DISABLE_OTHER_SIDE; + // TODO: Ugly hack. The base class should support arbitrary number of steps.. + needsSecondStep = true; + } else if (sideSwitchState == SideSwitchState::DISABLE_OTHER_SIDE) { + // Set this flag because the power needs to be switched off. + dualToSingleSideTransition = true; + submode = targetSubmodeForSideSwitch; + } +} + void DualLaneAssemblyBase::finishModeOp() { using namespace duallane; AssemblyBase::handleModeReached(); diff --git a/mission/system/acs/DualLaneAssemblyBase.h b/mission/system/acs/DualLaneAssemblyBase.h index fb17365c..4ece59df 100644 --- a/mission/system/acs/DualLaneAssemblyBase.h +++ b/mission/system/acs/DualLaneAssemblyBase.h @@ -47,6 +47,7 @@ class DualLaneAssemblyBase : public AssemblyBase, public ConfirmsFailuresIF { } customRecoveryStates = RecoveryCustomStates::IDLE; MessageQueueIF* eventQueue = nullptr; + void handleSideSwitchStates(uint8_t& submode, bool& needsSecondStep); /** * Check whether it makes sense to send mode commands to the device. diff --git a/mission/system/acs/SusAssembly.cpp b/mission/system/acs/SusAssembly.cpp index 45123ce4..13946c13 100644 --- a/mission/system/acs/SusAssembly.cpp +++ b/mission/system/acs/SusAssembly.cpp @@ -45,6 +45,7 @@ ReturnValue_t SusAssembly::handleNormalOrOnModeCmd(Mode_t mode, Submode_t submod using namespace duallane; ReturnValue_t result = returnvalue::OK; bool needsSecondStep = false; + handleSideSwitchStates(submode, needsSecondStep); auto cmdSeq = [&](object_id_t objectId, Mode_t devMode, uint8_t tableIdx) { if (mode == devMode) { modeTable[tableIdx].setMode(mode); From 95d220c3045c80d68ec60c131487d714cae5e6c1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 19:15:54 +0200 Subject: [PATCH 3/6] docs --- mission/system/acs/DualLaneAssemblyBase.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mission/system/acs/DualLaneAssemblyBase.h b/mission/system/acs/DualLaneAssemblyBase.h index 4ece59df..2269fd2a 100644 --- a/mission/system/acs/DualLaneAssemblyBase.h +++ b/mission/system/acs/DualLaneAssemblyBase.h @@ -47,6 +47,12 @@ class DualLaneAssemblyBase : public AssemblyBase, public ConfirmsFailuresIF { } customRecoveryStates = RecoveryCustomStates::IDLE; MessageQueueIF* eventQueue = nullptr; + + /** + * To be called in mode command packer function of the child class. + * @param submode + * @param needsSecondStep + */ void handleSideSwitchStates(uint8_t& submode, bool& needsSecondStep); /** From ff6ad60eed7e7794cc3bcabfadd5852a25af6245 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 19:16:57 +0200 Subject: [PATCH 4/6] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b69ce243..5e4bb196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ will consitute of a breaking change warranting a new major release: ## Fixed - Bugfix for side lane transitions of the dual lane assemblies, which only worked when the - assembly was directly commanded + assembly was directly commanded. - Syrlinks Handler: Bugfix so transition command is only sent once. ## Added From 23a4b08709880a9a5d9ceeae44bda74836415959 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 23:41:30 +0200 Subject: [PATCH 5/6] seems to work, improve dummy PCDU handler --- dummies/PcduHandlerDummy.cpp | 36 +++++++++++++++++++++++++++++++++++- dummies/PcduHandlerDummy.h | 4 ++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/dummies/PcduHandlerDummy.cpp b/dummies/PcduHandlerDummy.cpp index e3331c8d..0b1dec69 100644 --- a/dummies/PcduHandlerDummy.cpp +++ b/dummies/PcduHandlerDummy.cpp @@ -2,8 +2,12 @@ #include +#include "mission/power/defs.h" + PcduHandlerDummy::PcduHandlerDummy(object_id_t objectId, object_id_t comif, CookieIF *comCookie) - : DeviceHandlerBase(objectId, comif, comCookie), dummySwitcher(objectId, 18, 18, false) {} + : DeviceHandlerBase(objectId, comif, comCookie), dummySwitcher(objectId, 18, 18, false) { + switcherLock = MutexFactory::instance()->createMutex(); +} PcduHandlerDummy::~PcduHandlerDummy() {} @@ -44,6 +48,17 @@ ReturnValue_t PcduHandlerDummy::initializeLocalDataPool(localpool::DataPool &loc } ReturnValue_t PcduHandlerDummy::sendSwitchCommand(power::Switch_t switchNr, ReturnValue_t onOff) { + if (onOff == SWITCH_ON) { + triggerEvent(power::SWITCH_CMD_SENT, true, switchNr); + } else { + triggerEvent(power::SWITCH_CMD_SENT, false, switchNr); + } + { + MutexGuard mg(switcherLock); + // To simulate a real PCDU, remember the switch change to trigger a SWITCH_HAS_CHANGED event + // at a later stage. + switchChangeArray[switchNr] = true; + } return dummySwitcher.sendSwitchCommand(switchNr, onOff); } @@ -60,3 +75,22 @@ ReturnValue_t PcduHandlerDummy::getFuseState(uint8_t fuseNr) const { } uint32_t PcduHandlerDummy::getSwitchDelayMs(void) const { return dummySwitcher.getSwitchDelayMs(); } + +void PcduHandlerDummy::performOperationHook() { + SwitcherBoolArray switcherChangeCopy{}; + { + MutexGuard mg(switcherLock); + std::memcpy(switcherChangeCopy.data(), switchChangeArray.data(), switchChangeArray.size()); + } + for (uint8_t idx = 0; idx < switcherChangeCopy.size(); idx++) { + if (switcherChangeCopy[idx]) { + if (dummySwitcher.getSwitchState(idx) == PowerSwitchIF::SWITCH_ON) { + triggerEvent(power::SWITCH_HAS_CHANGED, true, idx); + } else { + triggerEvent(power::SWITCH_HAS_CHANGED, false, idx); + } + MutexGuard mg(switcherLock); + switchChangeArray[idx] = false; + } + } +} diff --git a/dummies/PcduHandlerDummy.h b/dummies/PcduHandlerDummy.h index 2f4c167a..7980629b 100644 --- a/dummies/PcduHandlerDummy.h +++ b/dummies/PcduHandlerDummy.h @@ -15,8 +15,12 @@ class PcduHandlerDummy : public DeviceHandlerBase, public PowerSwitchIF { virtual ~PcduHandlerDummy(); protected: + MutexIF *switcherLock; DummyPowerSwitcher dummySwitcher; + using SwitcherBoolArray = std::array; + SwitcherBoolArray switchChangeArray{}; + void performOperationHook() override; void doStartUp() override; void doShutDown() override; ReturnValue_t buildNormalDeviceCommand(DeviceCommandId_t *id) override; From 9b5fc8995c21e2d8d535e46acd9ae19d58db449c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 1 Apr 2023 13:43:24 +0200 Subject: [PATCH 6/6] changelog --- CHANGELOG.md | 2 ++ tmtc | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e4bb196..359c3718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ will consitute of a breaking change warranting a new major release: - Added `PTME_LOCKED` boolean lock which is used to lock the PTME so it is not used by the VC tasks anymore. This lock will be controlled by the CCSDS IP core handler and is locked when the PTME needs to be reset. Examples for this are datarate changes. +- Simulate real PCDU in PCDU dummy by remembering commandes switch change and triggering appropriate + events. Switch feedback is still immediate. ## Fixed diff --git a/tmtc b/tmtc index 5b613f98..bc85ccd8 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 5b613f98eea415242c152ed3498ed3c0434f139f +Subproject commit bc85ccd8eff0eaf2a168eee907d3436433e9b219