Add unit tests + refactor the original proposal

src/config/config.h

@@ -69,7 +69,15 @@ static constexpr uint8_t feedToBondtechMaxRetries = 2;
 
 /// Max attempts of ToolChange before throwing out an error - obviously, this has to be >= 1
 static constexpr uint8_t toolChangeAttempts = 3;
-static_assert(toolChangeAttempts >= 1);
+static_assert(toolChangeAttempts >= 1, "ToolChange's attempts must be > 0");
+
+/// Max attempts of UnloadFilament before throwing out an error - obviously, this has to be >= 1
+static constexpr uint8_t unloadAttempts = 3;
+static_assert(unloadAttempts >= 1, "UnloadFilament's attempts must be > 0");
+
+/// Max attempts of LoadFilament before throwing out an error - obviously, this has to be >= 1
+static constexpr uint8_t loadAttempts = 1;
+static_assert(loadAttempts >= 1, "LoadFilament's attempts must be > 0");
 
 /// Distances
 static constexpr U_mm pulleyToCuttingEdge = 33.0_mm; /// 33.0_mm /// Pulley to cutting edge.
@@ -183,7 +191,7 @@ static constexpr AxisConfig idler = {
     .iHold = 23, /// 398mA
     .stealth = false,
     .stepsPerUnit = (200 * 16 / 360.),
-    .sg_thrs = 7,
+    .sg_thrs = 6,
 };
 
 /// Idler motion limits

src/logic/command_base.cpp

@@ -186,6 +186,14 @@ bool CommandBase::CheckToolIndex(uint8_t index) {
     }
 }
 
+void CommandBase::GoToRetryIfPossible(uint8_t slot, ErrorCode ec) {
+    if (--attempts) {
+        Reset(slot, attempts);
+    } else {
+        GoToErrDisengagingIdler(ec);
+    }
+}
+
 void CommandBase::ErrDisengagingIdler() {
     if (!mi::idler.Engaged()) {
         state = ProgressCode::ERRWaitingForUser;

src/logic/command_base.h

@@ -23,12 +23,13 @@ namespace logic {
 /// These tasks report their progress and only one of these tasks is allowed to run at once.
 class CommandBase {
 public:
-    inline CommandBase()
+    constexpr CommandBase(uint8_t attempts)
         : state(ProgressCode::OK)
         , error(ErrorCode::OK)
         , stateBeforeModuleFailed(ProgressCode::Empty)
         , errorBeforeModuleFailed(ErrorCode::OK)
-        , recoveringMovableErrorAxisMask(0) {}
+        , recoveringMovableErrorAxisMask(0)
+        , attempts(attempts) {}
 
     // Normally, a base class should (must) have a virtual destructor to enable correct deallocation of superstructures.
     // However, in our case we don't want ANY destruction of these objects and moreover - adding a destructor like this
@@ -87,7 +88,16 @@ public:
     /// filament presence according to known sensors (FINDA+FSensor)
     static void InvalidateHomingAndFilamentState();
 
+#ifdef UNITTEST
+    inline void SetAttempts(uint8_t att) { attempts = att; }
+    inline uint8_t Attempts() const { return attempts; }
+#endif
+
 protected:
+    /// Inner part of Reset - sets the number of attempts for each Command
+    /// Contains the default implementation for commands, which do not cause any retry-errors (for optimization purposes)
+    virtual bool Reset(uint8_t param, uint8_t att) { return true; }
+
     /// @returns true if the slot/tool index is within specified range (0 - config::toolCount)
     /// If not, it returns false and sets the error to ErrorCode::INVALID_TOOL
     bool CheckToolIndex(uint8_t index);
@@ -104,6 +114,10 @@ protected:
     /// Perform disengaging idler in ErrDisengagingIdler state
     void ErrDisengagingIdler();
 
+    /// Try to repeat the Command if attempts > 0
+    /// Go to ErrDisengageIdler if no more attempts are available
+    void GoToRetryIfPossible(uint8_t slot, ErrorCode ec);
+
     /// Transit the state machine into ErrDisengagingIdler
     void GoToErrDisengagingIdler(ErrorCode ec);
 
@@ -118,6 +132,7 @@ protected:
     ProgressCode stateBeforeModuleFailed; ///< saved state of the state machine before a common error happened
     ErrorCode errorBeforeModuleFailed; ///< saved error of the state machine before a common error happened
     uint8_t recoveringMovableErrorAxisMask;
+    uint8_t attempts; ///< how many attempts shall the state machine try before throwing out an error - obviously this has to be >= 1
 };
 
 } // namespace logic

src/logic/cut_filament.cpp

@@ -19,8 +19,13 @@ bool CutFilament::Reset(uint8_t param) {
         return false;
     }
 
+    return Reset(param, config::toolChangeAttempts);
+}
+
+bool CutFilament::Reset(uint8_t param, uint8_t att) {
     error = ErrorCode::RUNNING;
     cutSlot = param;
+    attempts = att;
 
     if (mg::globals.FilamentLoaded() >= mg::FilamentLoadState::InSelector) {
         state = ProgressCode::UnloadingFilament;

src/logic/cut_filament.h

@@ -11,7 +11,7 @@ namespace logic {
 class CutFilament : public CommandBase {
 public:
     inline CutFilament()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     /// @param param index of filament slot to perform cut onto
@@ -23,6 +23,10 @@ public:
     ProgressCode State() const override;
 
     ErrorCode Error() const override;
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override;
 
 private:
     constexpr static const uint16_t cutStepsPre = 700;

src/logic/eject_filament.cpp

@@ -20,8 +20,13 @@ bool EjectFilament::Reset(uint8_t param) {
         return false;
     }
 
+    return Reset(param, config::toolChangeAttempts);
+}
+
+bool EjectFilament::Reset(uint8_t param, uint8_t att) {
     error = ErrorCode::RUNNING;
     slot = param;
+    attempts = att;
 
     if (mg::globals.FilamentLoaded() == mg::FilamentLoadState::NotLoaded) {
         FinishedOK();

src/logic/eject_filament.h

@@ -23,7 +23,7 @@ static constexpr modules::motion::P_speed_t ejectSpeed = 1000.0_P_mm_s; //@@TODO
 class EjectFilament : public CommandBase {
 public:
     inline EjectFilament()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     /// @param param index of filament slot to eject
@@ -35,6 +35,10 @@ public:
     ProgressCode State() const override;
 
     ErrorCode Error() const override;
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override;
 
 private:
     UnloadFilament unl; ///< a high-level command/operation may be used as a building block of other operations as well

src/logic/feed_to_bondtech.h

@@ -24,7 +24,7 @@ struct FeedToBondtech {
         //        PulleyStalled
     };
 
-    inline FeedToBondtech()
+    constexpr FeedToBondtech()
         : state(OK)
         , maxRetries(1) {}
 

src/logic/feed_to_finda.h

@@ -23,9 +23,10 @@ struct FeedToFinda {
         Stopped
     };
 
-    inline FeedToFinda()
+    constexpr FeedToFinda()
         : state(OK)
-        , feedPhaseLimited(true) {}
+        , feedPhaseLimited(true)
+        , haltAtEnd(true) {}
 
     /// Restart the automaton
     /// @param feedPhaseLimited

src/logic/home.h

@@ -20,8 +20,8 @@ namespace logic {
 /// This high-level command is just a way to invoke re-homing from the printer while all safety measures are kept.
 class Home : public CommandBase {
 public:
-    inline Home()
+    constexpr Home()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     /// @param param unused
@@ -29,6 +29,11 @@ public:
 
     /// @returns true if the state machine finished its job, false otherwise
     bool StepInner() override;
+
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override { return true; };
 };
 
 /// The one and only instance of Home state machine in the FW

src/logic/load_filament.cpp

@@ -15,6 +15,10 @@ namespace logic {
 
 LoadFilament loadFilament;
 
+LoadFilament::LoadFilament()
+    : CommandBase(config::loadAttempts)
+    , verifyLoadedFilament(0) {}
+
 bool LoadFilament::Reset(uint8_t param) {
     if (!CheckToolIndex(param)) {
         return false;
@@ -26,9 +30,15 @@ bool LoadFilament::Reset(uint8_t param) {
     if (mg::globals.FilamentLoaded() > mg::FilamentLoadState::AtPulley && mg::globals.ActiveSlot() != param) {
         return false;
     }
+
+    return Reset(param, config::toolChangeAttempts);
+}
+
+bool LoadFilament::Reset(uint8_t param, uint8_t att) {
     dbg_logic_P(PSTR("Load Filament"));
     mg::globals.SetFilamentLoaded(param, mg::FilamentLoadState::AtPulley); // still at pulley, haven't moved yet
     verifyLoadedFilament = 1;
+    attempts = att;
     Reset2(false);
     return true;
 }
@@ -68,7 +78,7 @@ bool LoadFilament::StepInner() {
         if (feed.Step()) {
             switch (feed.State()) {
             case FeedToFinda::Failed: // @@TODO - try to repeat 6x - push/pull sequence - probably something to put into feed_to_finda as an option
-                GoToErrDisengagingIdler(ErrorCode::FINDA_DIDNT_SWITCH_ON); // signal loading error
+                GoToRetryIfPossible(mg::globals.ActiveSlot(), ErrorCode::FINDA_DIDNT_SWITCH_ON); // signal loading error
                 break;
             case FeedToFinda::Stopped:
                 // as requested in MMU-116 - stopping an unsuccessful feed should retract as well but not check the filament
@@ -83,7 +93,7 @@ bool LoadFilament::StepInner() {
     case ProgressCode::RetractingFromFinda:
         if (retract.Step()) {
             if (retract.State() == RetractFromFinda::Failed) {
-                GoToErrDisengagingIdler(ErrorCode::FINDA_DIDNT_SWITCH_OFF); // signal loading error
+                GoToRetryIfPossible(mg::globals.ActiveSlot(), ErrorCode::FINDA_DIDNT_SWITCH_OFF); // signal loading error
             } else {
                 if (verifyLoadedFilament) {
                     --verifyLoadedFilament;

src/logic/load_filament.h

@@ -10,9 +10,7 @@ namespace logic {
 /// @brief A high-level command state machine - handles the complex logic of loading filament into a filament slot.
 class LoadFilament : public CommandBase {
 public:
-    inline LoadFilament()
+    LoadFilament();
-        : CommandBase()
-        , verifyLoadedFilament(0) {}
 
     /// Restart the automaton - performs unlimited rotation of the Pulley
     /// @param param index of filament slot to load
@@ -25,6 +23,11 @@ public:
     /// @returns true if the state machine finished its job, false otherwise
     bool StepInner() override;
 
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override;
+
 private:
     void GoToRetractingFromFinda();
     void Reset2(bool feedPhaseLimited);

src/logic/move_selector.h

@@ -15,8 +15,8 @@ namespace logic {
 /// and/or from the MMU's buttons while all safety measures are kept.
 class MoveSelector : public CommandBase {
 public:
-    inline MoveSelector()
+    constexpr MoveSelector()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     /// @param param target selector slot

src/logic/no_command.h

@@ -8,8 +8,8 @@ namespace logic {
 /// @brief A dummy No-command operation just to make the init of the firmware consistent (and cleaner code during processing).
 class NoCommand : public CommandBase {
 public:
-    inline NoCommand()
+    constexpr NoCommand()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     bool Reset(uint8_t /*param*/) override { return true; }

src/logic/retract_from_finda.h

@@ -20,7 +20,7 @@ struct RetractFromFinda {
         Failed
     };
 
-    inline RetractFromFinda()
+    constexpr RetractFromFinda()
         : state(OK) {}
 
     /// Restart the automaton

src/logic/set_mode.h

@@ -14,7 +14,7 @@ namespace logic {
 class SetMode : public CommandBase {
 public:
     inline SetMode()
-        : CommandBase() {}
+        : CommandBase(1) {}
 
     /// Restart the automaton
     bool Reset(uint8_t param) override;
@@ -22,6 +22,11 @@ public:
     /// @returns true if the state machine finished its job, false otherwise
     /// Since we perform the TMC2130 mode change in the Reset directly, the return is always true here (command finished ok)
     bool StepInner() override { return true; }
+
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override { return true; };
 };
 
 /// The one and only instance of NoCommand state machine in the FW

src/logic/tool_change.cpp

@@ -18,8 +18,7 @@ namespace logic {
 ToolChange toolChange;
 
 ToolChange::ToolChange()
-    : CommandBase()
+    : CommandBase(config::toolChangeAttempts) {}
-    , attempts(config::toolChangeAttempts) {}
 
 bool ToolChange::Reset(uint8_t param) {
     if (!CheckToolIndex(param)) {
@@ -32,41 +31,33 @@ bool ToolChange::Reset(uint8_t param) {
         return true;
     }
 
-    return Reset(param, config::toolChangeAttempts);
-}
-
-bool ToolChange::Reset(uint8_t param, uint8_t att) {
     // @@TODO establish printer in charge of UI processing for the ToolChange command only.
     // We'll see how that works and then probably we'll introduce some kind of protocol settings to switch UI handling.
     mui::userInput.SetPrinterInCharge(true);
 
+    return Reset(param, config::toolChangeAttempts);
+}
+
+bool ToolChange::Reset(uint8_t param, uint8_t att) {
     // we are either already at the correct slot, just the filament is not loaded - load the filament directly
     // or we are standing at another slot ...
     plannedSlot = param;
     attempts = att;
 
-    if (mg::globals.FilamentLoaded() >= mg::FilamentLoadState::InSelector) {
+    // Not sure if we can reach InSelector and NOT finda.Pressed() or vice versa, may be in case of an error.
+    // In any case - if the FINDA is pressed we must do an unload
+    if (mg::globals.FilamentLoaded() >= mg::FilamentLoadState::InSelector && mf::finda.Pressed()) {
         dbg_logic_P(PSTR("Filament is loaded --> unload"));
         state = ProgressCode::UnloadingFilament;
         unl.Reset(mg::globals.ActiveSlot());
     } else {
-        state = ProgressCode::FeedingToFinda;
+        GoToFeedingToFinda();
-        error = ErrorCode::RUNNING;
+        // @@TODO originally we set: mg::globals.SetFilamentLoaded(plannedSlot, mg::FilamentLoadState::InSelector);
-        dbg_logic_P(PSTR("Filament is not loaded --> load"));
+        // GoToFeedingToFinda sets it to AtPulley (may cause autohome)
-        mg::globals.SetFilamentLoaded(plannedSlot, mg::FilamentLoadState::InSelector);
-        feed.Reset(true, false);
     }
     return true;
 }
 
-void ToolChange::GoToRetryIfPossible(ErrorCode ec) {
-    if (--attempts) {
-        Reset(mg::globals.ActiveSlot(), attempts);
-    } else {
-        GoToErrDisengagingIdler(ec);
-    }
-}
-
 void logic::ToolChange::GoToFeedingToBondtech() {
     ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::blink0, ml::off);
     james.Reset(3);
@@ -100,7 +91,7 @@ bool ToolChange::StepInner() {
     case ProgressCode::FeedingToFinda:
         if (feed.Step()) {
             if (feed.State() == FeedToFinda::Failed) {
-                GoToRetryIfPossible(ErrorCode::FINDA_DIDNT_SWITCH_ON); // signal loading error
+                GoToRetryIfPossible(mg::globals.ActiveSlot(), ErrorCode::FINDA_DIDNT_SWITCH_ON); // signal loading error
             } else {
                 GoToFeedingToBondtech();
             }
@@ -110,10 +101,10 @@ bool ToolChange::StepInner() {
         if (james.Step()) {
             switch (james.State()) {
             case FeedToBondtech::Failed:
-                GoToRetryIfPossible(ErrorCode::FSENSOR_DIDNT_SWITCH_ON); // signal loading error
+                GoToRetryIfPossible(mg::globals.ActiveSlot(), ErrorCode::FSENSOR_DIDNT_SWITCH_ON); // signal loading error
                 break;
             case FeedToBondtech::FSensorTooEarly:
-                GoToRetryIfPossible(ErrorCode::FSENSOR_TOO_EARLY); // signal loading error
+                GoToRetryIfPossible(mg::globals.ActiveSlot(), ErrorCode::FSENSOR_TOO_EARLY); // signal loading error
                 break;
             default:
                 ToolChangeFinishedCorrectly();
@@ -143,11 +134,8 @@ bool ToolChange::StepInner() {
             // However - if we run into "FSensor didn't trigger", the situation is exactly opposite - it is beneficial
             // to unload the filament and try the whole sequence again
             // Therefore we only switch to FeedingToFinda if FINDA is not pressed (we suppose the filament is unloaded completely)
-            if (mf::finda.Pressed()) {
+            // Moved the reset logic into the Reset method
-                Reset(mg::globals.ActiveSlot());
+            Reset(mg::globals.ActiveSlot(), attempts);
-            } else {
-                GoToFeedingToFinda();
-            }
             break;
         case mui::Event::Right: // problem resolved - the user pushed the fillament by hand?
             // we should check the state of all the sensors and either report another error or confirm the correct state

src/logic/tool_change.h

@@ -23,16 +23,16 @@ public:
     ProgressCode State() const override;
 
     ErrorCode Error() const override;
+#ifndef UNITTEST
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override;
 
 #ifndef UNITTEST
 private:
-#else
-    inline void SetAttempts(uint8_t att) { attempts = att; }
 #endif
     void GoToFeedingToBondtech();
     void GoToFeedingToFinda();
-    bool Reset(uint8_t param, uint8_t att);
-    void GoToRetryIfPossible(ErrorCode ec);
 
     /// Common code for a correct completion of UnloadFilament
     void ToolChangeFinishedCorrectly();
@@ -41,7 +41,6 @@ private:
     FeedToFinda feed;
     FeedToBondtech james; // bond ;)
     uint8_t plannedSlot;
-    uint8_t attempts; ///< how many attempts shall the state machine try before throwing out an error - obviously this has to be >= 1
 };
 
 /// The one and only instance of ToolChange state machine in the FW

src/logic/unload_filament.cpp

@@ -16,7 +16,10 @@ namespace logic {
 
 UnloadFilament unloadFilament;
 
-bool UnloadFilament::Reset(uint8_t /*param*/) {
+UnloadFilament::UnloadFilament()
+    : CommandBase(config::unloadAttempts) {}
+
+bool UnloadFilament::Reset(uint8_t param) {
 
     if (!mf::finda.Pressed() && mg::globals.FilamentLoaded() < mg::FilamentLoadState::InSelector) {
         // it looks like we have nothing in the PTFE tube, at least FINDA doesn't sense anything
@@ -24,11 +27,16 @@ bool UnloadFilament::Reset(uint8_t /*param*/) {
         return true;
     }
 
+    return Reset(param, config::toolChangeAttempts);
+}
+
+bool UnloadFilament::Reset(uint8_t param, uint8_t att) {
     // unloads filament from extruder - filament is above Bondtech gears
     mpu::pulley.InitAxis();
     state = ProgressCode::UnloadingToFinda;
     error = ErrorCode::RUNNING;
-    unl.Reset(maxRetries);
+    attempts = att;
+    unl.Reset(1);
     ml::leds.SetPairButOffOthers(mg::globals.ActiveSlot(), ml::off, ml::off);
     return true;
 }
@@ -58,11 +66,11 @@ bool UnloadFilament::StepInner() {
         if (unl.Step()) {
             if (unl.State() == UnloadToFinda::FailedFINDA) {
                 // couldn't unload to FINDA, report error and wait for user to resolve it
-                GoToErrDisengagingIdler(ErrorCode::FINDA_DIDNT_SWITCH_OFF);
+                GoToRetryIfPossible(0, ErrorCode::FINDA_DIDNT_SWITCH_OFF);
             } else if (unl.State() == UnloadToFinda::FailedFSensor) {
                 // fsensor still pressed - that smells bad - a piece of filament may still be present in the heatsink
                 // and that would cause serious problems while loading another filament
-                GoToErrDisengagingIdler(ErrorCode::FSENSOR_DIDNT_SWITCH_OFF);
+                GoToRetryIfPossible(0, ErrorCode::FSENSOR_DIDNT_SWITCH_OFF);
             } else {
                 GoToRetractingFromFinda();
             }
@@ -71,7 +79,7 @@ bool UnloadFilament::StepInner() {
     case ProgressCode::RetractingFromFinda:
         if (retract.Step()) {
             if (retract.State() == RetractFromFinda::Failed) {
-                GoToErrDisengagingIdler(ErrorCode::FINDA_DIDNT_SWITCH_OFF); // signal unloading error
+                GoToRetryIfPossible(0, ErrorCode::FINDA_DIDNT_SWITCH_OFF); // signal unloading error
             } else {
                 state = ProgressCode::DisengagingIdler;
                 mi::idler.Disengage();
@@ -150,7 +158,7 @@ bool UnloadFilament::StepInner() {
         // recovery mode - we assume the filament is somewhere between the idle position and FINDA - thus blocking the selector
         if (feed.Step()) {
             if (feed.State() == FeedToFinda::Failed) {
-                GoToErrDisengagingIdler(ErrorCode::FINDA_DIDNT_SWITCH_ON);
+                GoToRetryIfPossible(0, ErrorCode::FINDA_DIDNT_SWITCH_ON);
             } else {
                 state = ProgressCode::RetractingFromFinda;
                 retract.Reset();

src/logic/unload_filament.h

@@ -11,8 +11,7 @@ namespace logic {
 /// @brief A high-level command state machine - handles the complex logic of unloading filament
 class UnloadFilament : public CommandBase {
 public:
-    inline UnloadFilament()
+    UnloadFilament();
-        : CommandBase() {}
 
     /// Restart the automaton
     /// @param param is not used, always unloads from the active slot
@@ -21,9 +20,12 @@ public:
     /// @returns true if the state machine finished its job, false otherwise
     bool StepInner() override;
 
-private:
+#ifndef UNITTEST
-    constexpr static const uint8_t maxRetries = 1;
+protected:
+#endif
+    virtual bool Reset(uint8_t param, uint8_t att) override;
 
+private:
     /// Common code for a correct completion of UnloadFilament
     void UnloadFinishedCorrectly();
     void GoToRetractingFromFinda();

src/logic/unload_to_finda.h

@@ -21,8 +21,10 @@ struct UnloadToFinda {
         FailedFINDA,
         FailedFSensor
     };
-    inline UnloadToFinda()
+    constexpr UnloadToFinda()
-        : maxTries(3) {}
+        : state(OK)
+        , maxTries(3)
+        , unloadStart_mm(0) {}
 
     /// Restart the automaton
     /// @param maxTries maximum number of retried attempts before reporting a fail

tests/unit/logic/tool_change/test_tool_change.cpp

@@ -73,6 +73,7 @@ void ToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     // restart the automaton
     tc.Reset(toSlot);
     tc.SetAttempts(1);
+    tc.unl.SetAttempts(1);
 
     REQUIRE(WhileCondition(
         tc,
@@ -108,6 +109,7 @@ void NoToolChange(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
     // restart the automaton
     tc.Reset(toSlot);
     tc.SetAttempts(1);
+    tc.unl.SetAttempts(1);
 
     // should not do anything
     REQUIRE(tc.TopLevelState() == ProgressCode::OK);
@@ -125,6 +127,7 @@ void JustLoadFilament(logic::ToolChange &tc, uint8_t slot) {
     // restart the automaton
     tc.Reset(slot);
     tc.SetAttempts(1);
+    tc.unl.SetAttempts(1);
 
     FeedingToFinda(tc, slot);
 
@@ -176,7 +179,7 @@ TEST_CASE("tool_change::same_slot_just_unloaded_filament", "[tool_change]") {
     }
 }
 
-void ToolChangeFailLoadToFinda(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
+void ToolChangeFailLoadToFinda(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot, uint8_t attempts) {
     ForceReinitAllAutomata();
 
     REQUIRE(EnsureActiveSlotIndex(fromSlot, mg::FilamentLoadState::InNozzle));
@@ -185,15 +188,23 @@ void ToolChangeFailLoadToFinda(logic::ToolChange &tc, uint8_t fromSlot, uint8_t
 
     // restart the automaton
     tc.Reset(toSlot);
-    tc.SetAttempts(1);
+    tc.SetAttempts(attempts);
+    tc.unl.SetAttempts(1); // do not complicate the test with unload attempts
 
     REQUIRE(WhileCondition(tc, std::bind(SimulateUnloadToFINDA, _1, 100, 2'000), 200'000));
     REQUIRE(WhileTopState(tc, ProgressCode::UnloadingFilament, 5000));
 
     REQUIRE(mg::globals.FilamentLoaded() == mg::FilamentLoadState::AtPulley);
 
-    // feeding to finda, but fails - do not trigger FINDA
+    // Feeding to finda, but fails - do not trigger FINDA
-    REQUIRE(WhileTopState(tc, ProgressCode::FeedingToFinda, 50000UL));
+    // In case there are multiple attempts, we have to check that only one is processed at a time
+    // A repeated attempt should try FeedToFinda again
+    for (int a = attempts; a > 0; --a) {
+        REQUIRE(WhileCondition(
+            tc,
+            [&](uint32_t) { return tc.TopLevelState() == ProgressCode::FeedingToFinda && tc.Attempts() == a; },
+            50000UL));
+    }
 
     // should end up in error disengage idler
     REQUIRE(VerifyState(tc, mg::FilamentLoadState::InSelector, toSlot, toSlot, false, true, ml::off, ml::blink0, ErrorCode::FINDA_DIDNT_SWITCH_ON, ProgressCode::ERRDisengagingIdler));
@@ -294,11 +305,13 @@ void ToolChangeFailLoadToFindaRightBtn(logic::ToolChange &tc, uint8_t toSlot) {
 
 TEST_CASE("tool_change::load_fail_FINDA_resolve_btnL", "[tool_change]") {
     logic::ToolChange tc;
-    for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
+    for (uint8_t attempts = 1; attempts < 4; ++attempts) {
-        for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
+        for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
-            if (fromSlot != toSlot) {
+            for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
-                ToolChangeFailLoadToFinda(tc, fromSlot, toSlot);
+                if (fromSlot != toSlot) {
-                ToolChangeFailLoadToFindaLeftBtn(tc, toSlot);
+                    ToolChangeFailLoadToFinda(tc, fromSlot, toSlot, attempts);
+                    ToolChangeFailLoadToFindaLeftBtn(tc, toSlot);
+                }
             }
         }
     }
@@ -306,11 +319,13 @@ TEST_CASE("tool_change::load_fail_FINDA_resolve_btnL", "[tool_change]") {
 
 TEST_CASE("tool_change::load_fail_FINDA_resolve_btnM", "[tool_change]") {
     logic::ToolChange tc;
-    for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
+    for (uint8_t attempts = 1; attempts < 4; ++attempts) {
-        for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
+        for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
-            if (fromSlot != toSlot) {
+            for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
-                ToolChangeFailLoadToFinda(tc, fromSlot, toSlot);
+                if (fromSlot != toSlot) {
-                ToolChangeFailLoadToFindaMiddleBtn(tc, toSlot);
+                    ToolChangeFailLoadToFinda(tc, fromSlot, toSlot, attempts);
+                    ToolChangeFailLoadToFindaMiddleBtn(tc, toSlot);
+                }
             }
         }
     }
@@ -318,11 +333,13 @@ TEST_CASE("tool_change::load_fail_FINDA_resolve_btnM", "[tool_change]") {
 
 TEST_CASE("tool_change::load_fail_FINDA_resolve_btnR_FINDA_FSensor", "[tool_change]") {
     logic::ToolChange tc;
-    for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
+    for (uint8_t attempts = 1; attempts < 4; ++attempts) {
-        for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
+        for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
-            if (fromSlot != toSlot) {
+            for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
-                ToolChangeFailLoadToFinda(tc, fromSlot, toSlot);
+                if (fromSlot != toSlot) {
-                ToolChangeFailLoadToFindaRightBtnFINDA_FSensor(tc, toSlot);
+                    ToolChangeFailLoadToFinda(tc, fromSlot, toSlot, attempts);
+                    ToolChangeFailLoadToFindaRightBtnFINDA_FSensor(tc, toSlot);
+                }
             }
         }
     }
@@ -330,17 +347,19 @@ TEST_CASE("tool_change::load_fail_FINDA_resolve_btnR_FINDA_FSensor", "[tool_chan
 
 TEST_CASE("tool_change::load_fail_FINDA_resolve_btnR_FINDA", "[tool_change]") {
     logic::ToolChange tc;
-    for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
+    for (uint8_t attempts = 1; attempts < 4; ++attempts) {
-        for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
+        for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
-            if (fromSlot != toSlot) {
+            for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
-                ToolChangeFailLoadToFinda(tc, fromSlot, toSlot);
+                if (fromSlot != toSlot) {
-                ToolChangeFailLoadToFindaRightBtnFINDA(tc, toSlot);
+                    ToolChangeFailLoadToFinda(tc, fromSlot, toSlot, attempts);
+                    ToolChangeFailLoadToFindaRightBtnFINDA(tc, toSlot);
+                }
             }
         }
     }
 }
 
-void ToolChangeFailFSensor(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot) {
+void ToolChangeFailFSensor(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSlot, uint8_t attempts) {
     using namespace std::placeholders;
     ForceReinitAllAutomata();
 
@@ -350,11 +369,12 @@ void ToolChangeFailFSensor(logic::ToolChange &tc, uint8_t fromSlot, uint8_t toSl
 
     // restart the automaton
     tc.Reset(toSlot);
-    tc.SetAttempts(1);
+    tc.SetAttempts(attempts);
+    tc.unl.SetAttempts(attempts); // unload will do its attempts on its own
 
     REQUIRE(VerifyState(tc, mg::FilamentLoadState::InNozzle, mi::idler.IdleSlotIndex(), fromSlot, true, true, ml::off, ml::off, ErrorCode::RUNNING, ProgressCode::UnloadingFilament));
     // simulate unload to finda but fail the fsensor test
-    REQUIRE(WhileCondition(tc, std::bind(SimulateUnloadToFINDA, _1, 500'000, 10'000), 200'000));
+    REQUIRE(WhileCondition(tc, std::bind(SimulateUnloadToFINDA, _1, 1500'000, 10'000), 1200'000));
     REQUIRE(VerifyState(tc, mg::FilamentLoadState::InSelector, mi::idler.IdleSlotIndex(), fromSlot, false, false, ml::off, ml::blink0, ErrorCode::FSENSOR_DIDNT_SWITCH_OFF, ProgressCode::UnloadingFilament));
     REQUIRE(tc.unl.State() == ProgressCode::ERRWaitingForUser);
 }
@@ -408,9 +428,25 @@ TEST_CASE("tool_change::load_fail_FSensor_resolve_btnM", "[tool_change]") {
     for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
         for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
             if (fromSlot != toSlot) {
-                ToolChangeFailFSensor(tc, fromSlot, toSlot);
+                ToolChangeFailFSensor(tc, fromSlot, toSlot, 1);
                 ToolChangeFailFSensorMiddleBtn(tc, fromSlot, toSlot);
             }
         }
     }
 }
+
+// @@TODO test case temporarily disabled, because it is unknown if we want to resolve the errors just on the MMU side
+// or if the printer shall be the "brain" of the operation ... it looks more like the printer...
+TEST_CASE("tool_change::load_fail_FSensor_retry_resolve_btnM", "[tool_change][.]") {
+    logic::ToolChange tc;
+    for (uint8_t attempts = 1; attempts < 4; ++attempts) {
+        for (uint8_t fromSlot = 0; fromSlot < config::toolCount; ++fromSlot) {
+            for (uint8_t toSlot = 0; toSlot < config::toolCount; ++toSlot) {
+                if (fromSlot != toSlot) {
+                    ToolChangeFailFSensor(tc, fromSlot, toSlot, attempts);
+                    ToolChangeFailFSensorMiddleBtn(tc, fromSlot, toSlot);
+                }
+            }
+        }
+    }
+}

tests/unit/logic/unload_filament/test_unload_filament.cpp

@@ -39,6 +39,7 @@ void RegularUnloadFromSlot04Init(uint8_t slot, logic::UnloadFilament &uf) {
 
     // restart the automaton
     uf.Reset(slot);
+    uf.SetAttempts(1);
 }
 
 void RegularUnloadFromSlot04(uint8_t slot, logic::UnloadFilament &uf, uint8_t entryIdlerSlotIndex,
@@ -121,6 +122,7 @@ void FindaDidntTriggerCommonSetup(uint8_t slot, logic::UnloadFilament &uf) {
 
     // restart the automaton
     uf.Reset(slot);
+    uf.SetAttempts(1);
 
     // Stage 0 - verify state just after Reset()
     // we still think we have filament loaded at this stage
@@ -293,6 +295,7 @@ TEST_CASE("unload_filament::not_loaded", "[unload_filament]") {
 
     // restart the automaton
     uf.Reset(0);
+    uf.SetAttempts(1);
 
     // Stage 0 - unload filament should finish immediately as there is no filament loaded
     REQUIRE(VerifyState(uf, mg::FilamentLoadState::AtPulley, mi::Idler::IdleSlotIndex(), 0, false, false, ml::off, ml::off, ErrorCode::OK, ProgressCode::OK));