Add prototype of unified handling of HW errors in the logic layer

src/config/config.h

@@ -106,6 +106,10 @@ static constexpr IdlerLimits idlerLimits = {
     .accel = 1000.0_deg_s2,
 };
 
+
+/// Max retries of FeedToBondtech used in LoadFilament
+static constexpr uint8_t feedToBondtechMaxRetries = 2;
+
 // TMC2130 setup
 
 static constexpr int8_t tmc2130_sg_thrs = 3; // @todo 7bit two's complement for the sg_thrs

src/hal/tmc2130.h

@@ -80,7 +80,9 @@ public:
         const MotorCurrents &currents,
         MotorMode mode);
 
-    /// (re)initialization of the chip
+    /// (re)initialization of the chip - please note this is necessary due to some HW flaws in the original MMU boards.
+    /// And yes, the TMC may not get correctly initialized.
+    /// @returns true if the TMC2130 was inited correctly
     bool Init(const MotorParams &params);
 
     /// Get the current motor mode

src/logic/command_base.cpp

@@ -1,5 +1,25 @@
 #include "command_base.h"
+#include "../modules/idler.h"
+
+namespace mi = modules::idler;
 
 namespace logic {
 
+bool CommandBase::Step() {
+    // check the global HW errors - may be we should avoid the modules layer and check for the HAL layer errors directly
+    // @@TODO discuss...
+    bool any_error = mi::idler.State() == mi::Idler::Failed;
+
+    // @@TODO check all other HW issues here to be able to respond with the appropriate error code into the printer
+
+    if (any_error) {
+        state = ProgressCode::ERR1TMCInitFailed;
+        error = ErrorCode::TMC_INIT_ERROR;
+        return true; // the HW error prevents us from continuing with the with the state machine
+        // the MMU must be restarted/fixed before continuing
+    } else {
+        return StepInner();
+    }
+}
+
 } // namespace logic

src/logic/command_base.h

@@ -30,9 +30,16 @@ public:
     /// @param param numerical parameter that comes with some commands (e.g. T1 for tool change 1)
     virtual void Reset(uint8_t param) = 0;
 
-    /// steps the state machine
+    /// Steps the state machine. This is the preferred way of stepping the machine
+    /// as it handles the global HW error states uniformly (so that the derived classes do not have to deal
+    /// with these error states on their own).
+    /// Each derived class then only implements its own logic via the virtual #StepInner method.
     /// @returns true if the automaton finished its work
-    virtual bool Step() = 0;
+    bool Step();
+
+    /// Each derived class shall implement its own state machine logic in this method
+    /// It is being called from #Step after the HW error states have been checked
+    virtual bool StepInner() = 0;
 
     /// @returns progress of operation - each automaton consists of several internal states
     /// which should be reported to the user via the printer's LCD

src/logic/cut_filament.cpp

@@ -38,10 +38,10 @@ void CutFilament::SelectFilamentSlot() {
     ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off);
 }
 
-bool CutFilament::Step() {
+bool CutFilament::StepInner() {
     switch (state) {
     case ProgressCode::UnloadingFilament:
-        if (unl.Step()) {
+        if (unl.StepInner()) {
             // unloading sequence finished - basically, no errors can occurr here
             // as UnloadFilament should handle all the possible error states on its own
             // There is no way the UnloadFilament to finish in an error state

src/logic/cut_filament.h

@@ -17,7 +17,7 @@ public:
     void Reset(uint8_t param) override;
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override;
+    bool StepInner() override;
 
     ProgressCode State() const override;
 

src/logic/eject_filament.cpp

@@ -36,10 +36,10 @@ void EjectFilament::MoveSelectorAside() {
     ms::selector.MoveToSlot(selectorParkedPos);
 }
 
-bool EjectFilament::Step() {
+bool EjectFilament::StepInner() {
     switch (state) {
     case ProgressCode::UnloadingFilament:
-        if (unl.Step()) {
+        if (unl.StepInner()) {
             // unloading sequence finished - basically, no errors can occurr here
             // as UnloadFilament should handle all the possible error states on its own
             // There is no way the UnloadFilament to finish in an error state

src/logic/eject_filament.h

@@ -29,7 +29,7 @@ public:
     void Reset(uint8_t param) override;
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override;
+    bool StepInner() override;
 
     ProgressCode State() const override;
 

src/logic/load_filament.cpp

@@ -29,7 +29,7 @@ void LoadFilament::Reset(uint8_t param) {
     ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off);
 }
 
-bool LoadFilament::Step() {
+bool LoadFilament::StepInner() {
     switch (state) {
     case ProgressCode::EngagingIdler:
         if (mi::idler.Engaged()) {
@@ -49,7 +49,7 @@ bool LoadFilament::Step() {
                 ml::leds.SetMode(mg::globals.ActiveSlot(), ml::Color::red, ml::Mode::blink0); // signal loading error
             } else {
                 state = ProgressCode::FeedingToBondtech;
-                james.Reset(2);
+                james.Reset(config::feedToBondtechMaxRetries);
             }
         }
         break;

src/logic/load_filament.h

@@ -17,7 +17,7 @@ public:
     void Reset(uint8_t param) override;
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override;
+    bool StepInner() override;
 
 private:
     FeedToFinda feed;

src/logic/no_command.h

@@ -15,7 +15,7 @@ public:
     void Reset(uint8_t /*param*/) override {}
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override { return true; }
+    bool StepInner() override { return true; }
 };
 
 /// The one and only instance of NoCommand state machine in the FW

src/logic/progress_codes.h

@@ -21,6 +21,7 @@ enum class ProgressCode : uint_fast8_t {
     ERR1WaitingForUser,
     ERRInternal,
     ERR1HelpingFilament,
+    ERR1TMCInitFailed,
 
     UnloadingFilament,
     LoadingFilament,

src/logic/tool_change.cpp

@@ -29,10 +29,10 @@ void ToolChange::Reset(uint8_t param) {
     }
 }
 
-bool ToolChange::Step() {
+bool ToolChange::StepInner() {
     switch (state) {
     case ProgressCode::UnloadingFilament:
-        if (unl.Step()) {
+        if (unl.StepInner()) {
             // unloading sequence finished - basically, no errors can occurr here
             // as UnloadFilament should handle all the possible error states on its own
             // There is no way the UnloadFilament to finish in an error state
@@ -41,7 +41,7 @@ bool ToolChange::Step() {
         }
         break;
     case ProgressCode::LoadingFilament:
-        if (load.Step()) {
+        if (load.StepInner()) {
             // loading sequence finished - basically, no errors can occurr here
             // as LoadFilament should handle all the possible error states on its own
             // There is no way the LoadFilament to finish in an error state

src/logic/tool_change.h

@@ -17,7 +17,7 @@ public:
     void Reset(uint8_t param) override;
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override;
+    bool StepInner() override;
 
     ProgressCode State() const override;
 

src/logic/unload_filament.cpp

@@ -28,7 +28,7 @@ void UnloadFilament::Reset(uint8_t /*param*/) {
     ml::leds.SetMode(mg::globals.ActiveSlot(), ml::red, ml::off);
 }
 
-bool UnloadFilament::Step() {
+bool UnloadFilament::StepInner() {
     switch (state) {
     // state 1 engage idler - will be done by the Unload to FINDA state machine
     case ProgressCode::UnloadingToFinda: // state 2 rotate pulley as long as the FINDA is on

src/logic/unload_filament.h

@@ -16,7 +16,7 @@ public:
     void Reset(uint8_t param) override;
 
     /// @returns true if the state machine finished its job, false otherwise
-    bool Step() override;
+    bool StepInner() override;
 
 private:
     constexpr static const uint8_t maxRetries = 3;

src/modules/idler.cpp

@@ -11,9 +11,9 @@ Idler idler;
 
 namespace mm = modules::motion;
 
-bool Idler::Disengage() {
+Idler::EngageDisengage Idler::Disengage() {
     if (state == Moving)
-        return false;
+        return EngageDisengage::Refused;
 
     plannedEngage = false;
 
@@ -25,11 +25,22 @@ bool Idler::Disengage() {
     mm::motion.PlanMoveTo<mm::Idler>(SlotPosition(IdleSlotIndex()), 1000._I_deg_s); // @@TODO
     state = Moving;
     return true;
+//        return EngageDisengage::Accepted;
+//
+//    if (!mm::motion.InitAxis(mm::Idler)) {
+//        state = Failed;
+//        return EngageDisengage::Failed;
+//    } else {
+//        // plan move to idle position
+//        mm::motion.PlanMove(mm::Idler, config::idlerSlotPositions[IdleSlotIndex()] - mm::motion.Position(mm::Idler), 1000); // @@TODO
+//        state = Moving;
+//        return EngageDisengage::Accepted;
+//    }
 }
 
-bool Idler::Engage(uint8_t slot) {
+Idler::EngageDisengage Idler::Engage(uint8_t slot) {
     if (state == Moving)
-        return false;
+        return EngageDisengage::Refused;
 
     plannedSlot = slot;
     plannedEngage = true;
@@ -41,6 +52,16 @@ bool Idler::Engage(uint8_t slot) {
     mm::motion.PlanMoveTo<mm::Idler>(SlotPosition(slot), 1000._I_deg_s); // @@TODO
     state = Moving;
     return true;
+//        return EngageDisengage::Accepted;
+//
+//    if (!mm::motion.InitAxis(mm::Idler)) {
+//        state = Failed;
+//        return EngageDisengage::Failed;
+//    } else {
+//        mm::motion.PlanMove(mm::Idler, config::idlerSlotPositions[slot] - mm::motion.Position(mm::Idler), 1000); // @@TODO
+//        state = Moving;
+//        return EngageDisengage::Accepted;
+//    }
 }
 
 bool Idler::Home() {

src/modules/idler.h

@@ -26,14 +26,21 @@ public:
         , currentSlot(0)
         , currentlyEngaged(false) {}
 
+    /// Engage/Disengage return values
+    enum class EngageDisengage : uint8_t {
+        Accepted, ///< the operation has been successfully started
+        Refused, ///< another operation is currently underway, cannot start a new one
+        Failed ///< the operation could not been started due to HW issues
+    };
+
     /// Plan engaging of the idler to a specific filament slot
     /// @param slot index to be activated
-    /// @returns false in case an operation is already underway
+    /// @returns #EngageDisengage
-    bool Engage(uint8_t slot);
+    EngageDisengage Engage(uint8_t slot);
 
     /// Plan disengaging of the idler, i.e. parking the idler
-    /// @returns false in case an operation is already underway
+    /// @returns #EngageDisengage
-    bool Disengage();
+    EngageDisengage Disengage();
 
     /// Plan homing of the idler axis
     /// @returns false in case an operation is already underway
@@ -59,6 +66,9 @@ public:
     /// @returns the index of idle position of the idler, usually 5 in case of 0-4 valid indices of filament slots
     inline static constexpr uint8_t IdleSlotIndex() { return config::toolCount; }
 
+    /// @returns internal state of the Idler
+    inline uint8_t State() const { return state; }
+
 private:
     /// internal state of the automaton
     uint8_t state;

src/modules/motion.cpp

@@ -5,13 +5,11 @@ namespace motion {
 
 Motion motion;
 
-void Motion::InitAxis(Axis axis) {
+bool Motion::InitAxis(Axis axis) {
-    for (uint8_t i = 0; i != NUM_AXIS; ++i) {
+    // disable the axis and re-init the driver: this will clear the internal
-        // disable the axis and re-init the driver: this will clear the internal
+    // StallGuard data as a result without special handling
-        // StallGuard data as a result without special handling
+    Disable(axis);
-        Disable(axis);
+    return axisData[axis].drv.Init(axisParams[axis].params);
-        axisData[axis].drv.Init(axisParams[axis].params);
-    }
 }
 
 void Motion::SetEnabled(Axis axis, bool enabled) {

src/modules/motion.h

@@ -75,7 +75,8 @@ public:
     /// Init axis driver - @@TODO this should be probably hidden
     /// somewhere deeper ... something should manage the axes and their
     /// state especially when the TMC may get randomly reset (deinited)
-    void InitAxis(Axis axis);
+    /// @returns true if the init was successful (TMC2130 responded ok)
+    bool InitAxis(Axis axis);
 
     /// Set axis power status. One must manually ensure no moves are currently being
     /// performed by calling QueueEmpty().

tests/unit/logic/cut_filament/CMakeLists.txt

@@ -1,6 +1,7 @@
 # define the test executable
 add_executable(
   cut_filament_tests
+  ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/cut_filament.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp

tests/unit/logic/eject_filament/CMakeLists.txt

@@ -1,6 +1,7 @@
 # define the test executable
 add_executable(
   eject_filament_tests
+  ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/eject_filament.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp

tests/unit/logic/load_filament/CMakeLists.txt

@@ -1,6 +1,7 @@
 # define the test executable
 add_executable(
   load_filament_tests
+  ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_bondtech.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/load_filament.cpp

tests/unit/logic/stubs/stub_motion.cpp

@@ -14,8 +14,9 @@ AxisSim axes[3] = {
     { -32767, -32767, false, false, false }, // idler
 };
 
-void Motion::InitAxis(Axis axis) {
+bool Motion::InitAxis(Axis axis) {
     axes[axis].enabled = true;
+    return true;
 }
 
 void Motion::SetEnabled(Axis axis, bool enabled) {

tests/unit/logic/tool_change/CMakeLists.txt

@@ -1,6 +1,7 @@
 # define the test executable
 add_executable(
   tool_change_tests
+  ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_bondtech.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/load_filament.cpp

tests/unit/logic/unload_filament/CMakeLists.txt

@@ -1,6 +1,7 @@
 # define the test executable
 add_executable(
   unload_filament_tests
+  ${CMAKE_SOURCE_DIR}/src/logic/command_base.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/feed_to_finda.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/unload_filament.cpp
   ${CMAKE_SOURCE_DIR}/src/logic/unload_to_finda.cpp

tests/unit/logic/unload_filament/test_unload_filament.cpp

@@ -186,7 +186,7 @@ void FindaDidntTriggerResolveHelp(uint8_t slot, logic::UnloadFilament &uf) {
     hal::adc::SetADC(config::buttonsADCIndex, config::buttonADCLimits[0][0] + 1);
     while (!mb::buttons.ButtonPressed(0)) {
         main_loop();
-        uf.Step();
+        uf.StepInner();
     }
 
     // we still think we have filament loaded at this stage
@@ -270,7 +270,7 @@ void FindaDidntTriggerResolveTryAgain(uint8_t slot, logic::UnloadFilament &uf) {
     hal::adc::SetADC(config::buttonsADCIndex, config::buttonADCLimits[1][0] + 1);
     while (!mb::buttons.ButtonPressed(1)) {
         main_loop();
-        uf.Step();
+        uf.StepInner();
     }
 
     // we still think we have filament loaded at this stage