Hold/Resume Idler+Selector even after their homing errors

MMU-222

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) {
+ErrorCode CheckMovable(const mm::MovableBase &m) {
-    switch (m.State()) {
+    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;

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;
     }

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.

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,
+        Moving = 1,
-        PlannedHome,
+        PlannedHome = 2,
-        HomeForward,
+        HomeForward = 3,
-        HomeBack,
+        HomeBack = 4,
-        TMCFailed,
+        TMCFailed = 5,
-        HomingFailed,
+        HomingFailed = 6,
-        OnHold,
+        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:

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;

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;
     }

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(mi::idler.IsOnHold());
-    REQUIRE(ms::selector.state == ms::Selector::OnHold);
+    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;
 }

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();
         }

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(mi::idler.IsOnHold());
-        REQUIRE(ms::selector.state == ms::Selector::OnHold);
+        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));