From f07e206398d40be31d04fe5f68d93a4fc80db9f1 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Mon, 13 Mar 2023 09:15:12 +0100 Subject: [PATCH] Hold/Resume Idler+Selector even after their homing errors MMU-222 --- src/logic/command_base.cpp | 7 +++++-- src/modules/idler.cpp | 11 ++++++----- src/modules/movable_base.cpp | 4 ++-- src/modules/movable_base.h | 19 +++++++++++-------- src/modules/pulley.cpp | 5 +++-- src/modules/selector.cpp | 6 +++--- tests/unit/logic/homing/test_homing.cpp | 14 +++++++------- tests/unit/logic/stubs/homing.cpp | 4 ++-- .../logic/tool_change/test_tool_change.cpp | 4 ++-- 9 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/logic/command_base.cpp b/src/logic/command_base.cpp index 6bbd06a..c5f05d8 100644 --- a/src/logic/command_base.cpp +++ b/src/logic/command_base.cpp @@ -56,8 +56,8 @@ static ErrorCode __attribute__((noinline)) AddErrorAxisBit(ErrorCode ec, uint8_t return ec; } -ErrorCode CheckMovable(mm::MovableBase &m) { - switch (m.State()) { +ErrorCode CheckMovable(const mm::MovableBase &m) { + switch (m.State() & (~mm::MovableBase::OnHold)) { // clear the on-hold bit from the state check case mm::MovableBase::TMCFailed: return AddErrorAxisBit(TMC2130ToErrorCode(m.TMCErrorFlags()), m.Axis()); case mm::MovableBase::HomingFailed: @@ -84,12 +84,14 @@ bool CommandBase::WaitForOneModuleErrorRecovery(ErrorCode ec, modules::motion::M errorBeforeModuleFailed = error; error = ec; state = ProgressCode::ERRWaitingForUser; // such a situation always requires user's attention -> let the printer display an error screen + HoldIdlerSelector(); } // are we already recovering an error - that would mean we got another one if (recoveringMovableErrorAxisMask) { error = ec; state = ProgressCode::ERRWaitingForUser; // such a situation always requires user's attention -> let the printer display an error screen + HoldIdlerSelector(); } switch (state) { @@ -99,6 +101,7 @@ bool CommandBase::WaitForOneModuleErrorRecovery(ErrorCode ec, modules::motion::M mui::Event ev = mui::userInput.ConsumeEvent(); if (ev == mui::Event::Middle) { recoveringMovableErrorAxisMask |= axisMask; + ResumeIdlerSelector(); m.PlanHome(); // force initiate a new homing attempt state = ProgressCode::Homing; error = ErrorCode::RUNNING; diff --git a/src/modules/idler.cpp b/src/modules/idler.cpp index 783ec9f..64c67fa 100644 --- a/src/modules/idler.cpp +++ b/src/modules/idler.cpp @@ -69,7 +69,7 @@ bool Idler::StallGuardAllowed(bool forward) const { } Idler::OperationResult Idler::Disengage() { - if (state == Moving || state == OnHold) { + if (state == Moving || IsOnHold()) { dbg_logic_P(PSTR("Moving --> Disengage refused")); return OperationResult::Refused; } @@ -101,7 +101,7 @@ Idler::OperationResult Idler::Engage(uint8_t slot) { } Idler::OperationResult Idler::PlanMoveInner(uint8_t slot, Operation plannedOp) { - if (state == Moving || state == OnHold) { + if (state == Moving || IsOnHold()) { dbg_logic_P(PSTR("Moving --> Engage refused")); return OperationResult::Refused; } @@ -132,6 +132,10 @@ Idler::OperationResult Idler::PlanMoveInner(uint8_t slot, Operation plannedOp) { } bool Idler::Step() { + if (IsOnHold()) { + return true; // just wait, do nothing! + } + if (state != TMCFailed) { CheckTMC(); } @@ -155,10 +159,7 @@ bool Idler::Step() { return false; } return true; - case OnHold: - return true; // just wait, do nothing! case TMCFailed: - dbg_logic_P(PSTR("Idler Failed")); default: return true; } diff --git a/src/modules/movable_base.cpp b/src/modules/movable_base.cpp index 134e2d2..c0392a8 100644 --- a/src/modules/movable_base.cpp +++ b/src/modules/movable_base.cpp @@ -8,7 +8,7 @@ namespace motion { MovableBase::OperationResult MovableBase::PlanHome() { InvalidateHoming(); - if (state == OnHold) + if (IsOnHold()) return OperationResult::Refused; // switch to normal mode on this axis @@ -32,7 +32,7 @@ void __attribute__((noinline)) MovableBase::SetCurrents(uint8_t iRun, uint8_t iH } void MovableBase::HoldOn() { - state = OnHold; + state |= OnHold; // set the on-hold bit mm::motion.AbortPlannedMoves(axis); // Force turn off motors - prevent overheating and allow servicing during an error state. // And don't worry about TMC2130 creep after axis enabled - we'll rehome both axes later when needed. diff --git a/src/modules/movable_base.h b/src/modules/movable_base.h index 74d1585..bca6cdd 100644 --- a/src/modules/movable_base.h +++ b/src/modules/movable_base.h @@ -13,13 +13,13 @@ public: /// Internal states of the state machine enum { Ready = 0, // intentionally set as zero in order to allow zeroing the Idler structure upon startup -> avoid explicit initialization code - Moving, - PlannedHome, - HomeForward, - HomeBack, - TMCFailed, - HomingFailed, - OnHold, + Moving = 1, + PlannedHome = 2, + HomeForward = 3, + HomeBack = 4, + TMCFailed = 5, + HomingFailed = 6, + OnHold = 0x80, ///< needs to be a separate bit due to homing recovery infrastructure }; /// Operation (Engage/Disengage/MoveToSlot) return values @@ -78,8 +78,11 @@ public: /// Also, disables the axis void HoldOn(); + /// @returns true if the movable is on-hold + bool IsOnHold() const { return state & OnHold; } + /// Allows the movable to move/home again after begin suspended by HoldOn - void Resume() { state = Ready; } + void Resume() { state &= ~OnHold; } #ifndef UNITTEST protected: diff --git a/src/modules/pulley.cpp b/src/modules/pulley.cpp index c8d3aa5..81d43a2 100644 --- a/src/modules/pulley.cpp +++ b/src/modules/pulley.cpp @@ -17,6 +17,9 @@ bool __attribute__((noinline)) Pulley::FinishHomingAndPlanMoveToParkPos() { } bool Pulley::Step() { + if (IsOnHold()) { + return true; // just wait, do nothing! + } if (state != TMCFailed) { CheckTMC(); } @@ -31,8 +34,6 @@ bool Pulley::Step() { return true; case Ready: return true; - case OnHold: - return true; case TMCFailed: default: return true; diff --git a/src/modules/selector.cpp b/src/modules/selector.cpp index 64410a0..2e1ebad 100644 --- a/src/modules/selector.cpp +++ b/src/modules/selector.cpp @@ -89,6 +89,9 @@ Selector::OperationResult Selector::MoveToSlot(uint8_t slot) { } bool Selector::Step() { + if (IsOnHold()) { + return true; // just wait, do nothing! + } if (state != TMCFailed) { CheckTMC(); } @@ -123,10 +126,7 @@ bool Selector::Step() { return false; } return true; - case OnHold: - return true; // just wait, do nothing! case TMCFailed: - dbg_logic_P(PSTR("Selector Failed")); default: return true; } diff --git a/tests/unit/logic/homing/test_homing.cpp b/tests/unit/logic/homing/test_homing.cpp index 6dacc0e..ea5a79b 100644 --- a/tests/unit/logic/homing/test_homing.cpp +++ b/tests/unit/logic/homing/test_homing.cpp @@ -168,25 +168,25 @@ bool OnHold(uint8_t slot) { // now put movables on hold logic::CommandBase::HoldIdlerSelector(); - REQUIRE(mi::idler.state == mi::Idler::OnHold); - REQUIRE(ms::selector.state == ms::Selector::OnHold); + REQUIRE(mi::idler.IsOnHold()); + REQUIRE(ms::selector.IsOnHold()); // both movables should ignore all attempts to perform moves REQUIRE(mi::idler.PlanHome() == mi::Idler::OperationResult::Refused); - REQUIRE(mi::idler.state == mi::Idler::OnHold); + REQUIRE(mi::idler.IsOnHold()); REQUIRE(mi::idler.Disengaged()); REQUIRE(ms::selector.PlanHome() == ms::Selector::OperationResult::Refused); - REQUIRE(ms::selector.state == ms::Selector::OnHold); + REQUIRE(ms::selector.IsOnHold()); REQUIRE(mi::idler.Disengage() == mi::Idler::OperationResult::Refused); - REQUIRE(mi::idler.state == mi::Idler::OnHold); + REQUIRE(mi::idler.IsOnHold()); REQUIRE(mi::idler.Engage(slot) == mi::Idler::OperationResult::Refused); - REQUIRE(mi::idler.state == mi::Idler::OnHold); + REQUIRE(mi::idler.IsOnHold()); REQUIRE(mi::idler.Disengaged()); REQUIRE(ms::selector.MoveToSlot((slot + 1) % config::toolCount) == ms::Selector::OperationResult::Refused); - REQUIRE(ms::selector.state == ms::Selector::OnHold); + REQUIRE(ms::selector.IsOnHold()); return true; } diff --git a/tests/unit/logic/stubs/homing.cpp b/tests/unit/logic/stubs/homing.cpp index a37f90a..1057af0 100644 --- a/tests/unit/logic/stubs/homing.cpp +++ b/tests/unit/logic/stubs/homing.cpp @@ -209,7 +209,7 @@ bool SimulateFailedHomeFirstTime(logic::CommandBase &cb) { } } - while (ms::selector.State() != mm::MovableBase::HomingFailed) { + while (!(ms::selector.State() & mm::MovableBase::OnHold)) { main_loop(); cb.Step(); } @@ -250,7 +250,7 @@ bool SimulateFailedHomeSelectorRepeated(logic::CommandBase &cb) { } } - while (ms::selector.State() != mm::MovableBase::HomingFailed) { + while (!(ms::selector.State() & mm::MovableBase::OnHold)) { main_loop(); cb.Step(); } diff --git a/tests/unit/logic/tool_change/test_tool_change.cpp b/tests/unit/logic/tool_change/test_tool_change.cpp index 7cdd933..c2ef04b 100644 --- a/tests/unit/logic/tool_change/test_tool_change.cpp +++ b/tests/unit/logic/tool_change/test_tool_change.cpp @@ -434,8 +434,8 @@ void ToolChangeWithFlickeringFINDA(logic::ToolChange &tc, uint8_t fromSlot, uint tc.Step(); // now both Idler and Selector are on hold again - REQUIRE(mi::idler.state == mi::Idler::OnHold); - REQUIRE(ms::selector.state == ms::Selector::OnHold); + REQUIRE(mi::idler.IsOnHold()); + REQUIRE(ms::selector.IsOnHold()); REQUIRE(VerifyState2(tc, mg::FilamentLoadState::AtPulley, mi::idler.IdleSlotIndex(), fromSlot, true, false, toSlot, ml::off, ml::blink0, ErrorCode::FINDA_FLICKERS, ProgressCode::ERRWaitingForUser));