Ignore Idler's SG signal from TMC when rolling over filaments

src/hal/tmc2130.h

@@ -25,7 +25,7 @@ struct MotorParams {
     gpio::GPIO_pin stepPin; ///< step pin
     gpio::GPIO_pin sgPin; ///< StallGuard pin
     config::MRes mRes; ///< microstep resolution
-    int16_t sg_thrs;
+    int8_t sg_thrs;
     uint8_t axis;
 };
 

src/modules/idler.cpp

@@ -20,9 +20,6 @@ void Idler::PrepareMoveToPlannedSlot() {
 }
 
 void Idler::PlanHomingMoveForward() {
-    //    ml::leds.SetMode(0, ml::red, ml::off);
-    fwdHomeTrigger = true;
-    SetSGTHRS(32767); // @@TODO debug screw the homing sensitivity - to be removed for final
     mm::motion.SetPosition(mm::Idler, mm::unitToSteps<mm::I_pos_t>(config::IdlerOffsetFromHome));
     axisStart = mm::axisUnitToTruncatedUnit<config::U_deg>(mm::motion.CurPosition<mm::Idler>());
     mm::motion.PlanMove<mm::Idler>(mm::unitToAxisUnit<mm::I_pos_t>(config::idlerLimits.lenght * 2),
@@ -30,15 +27,7 @@ void Idler::PlanHomingMoveForward() {
     dbg_logic_P(PSTR("Plan Homing Idler Forward"));
 }
 
-void modules::idler::Idler::SetSGTHRS(int16_t sgthrs) {
-    mm::motion.PlanStallGuardThreshold(mm::Idler, sgthrs);
-    mm::motion.DriverForAxis(mm::Idler).SetSGTHRS(mm::axisParams[mm::Idler].params);
-}
-
 void Idler::PlanHomingMoveBack() {
-    fwdHomeTrigger = true;
-    SetSGTHRS(32767); //@@TODO debug screw
-    //    ml::leds.SetMode(0, ml::red, ml::off);
     // we expect that we are at the front end of the axis, set the expected axis' position
     mm::motion.SetPosition(mm::Idler, mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght));
     axisStart = mm::axisUnitToTruncatedUnit<config::U_deg>(mm::motion.CurPosition<mm::Idler>());
@@ -74,25 +63,11 @@ void Idler::FinishMove() {
     }
 }
 
-bool Idler::SGAllowed(bool forward) const {
+bool Idler::StallGuardAllowed(bool forward) const {
     const uint8_t checkDistance = forward ? 220 : 200;
     return AxisDistance(mm::axisUnitToTruncatedUnit<config::U_deg>(mm::motion.CurPosition<mm::Idler>())) > checkDistance;
 }
 
-void Idler::UpdateAdaptiveSGTHRS(bool forward) {
-    //    return;
-    const uint8_t checkDistance = forward ? 220 : 200;
-
-    if (AxisDistance(mm::axisUnitToTruncatedUnit<config::U_deg>(mm::motion.CurPosition<mm::Idler>()))
-            > checkDistance
-        && fwdHomeTrigger) {
-        // set higher sensitivity - i.e. lower threshold
-        //        ml::leds.SetMode(0, ml::red, ml::on);
-        SetSGTHRS(mg::globals.StallGuardThreshold(mm::Idler) - 1);
-        fwdHomeTrigger = false;
-    }
-}
-
 Idler::OperationResult Idler::Disengage() {
     if (state == Moving || state == OnHold) {
         dbg_logic_P(PSTR("Moving --> Disengage refused"));

src/modules/idler.h

@@ -17,8 +17,7 @@ public:
     inline constexpr Idler()
         : MovableBase(mm::Idler)
         , plannedMove(Operation::disengage)
-        , currentlyEngaged(Operation::disengage)
+        , currentlyEngaged(Operation::disengage) {}
-        , fwdHomeTrigger(false) {}
 
     /// Plan engaging of the idler to a specific filament slot
     /// @param slot index to be activated
@@ -71,8 +70,7 @@ protected:
     virtual void PlanHomingMoveBack() override;
     virtual bool FinishHomingAndPlanMoveToParkPos() override;
     virtual void FinishMove() override;
-    virtual void UpdateAdaptiveSGTHRS(bool forward) override;
+    virtual bool StallGuardAllowed(bool forward) const override;
-    virtual bool SGAllowed(bool forward) const override;
 
 private:
     enum class Operation : uint8_t {
@@ -88,9 +86,6 @@ private:
 
     /// current state
     Operation currentlyEngaged;
-    void SetSGTHRS(int16_t sgthrs);
-
-    bool fwdHomeTrigger;
 };
 
 /// The one and only instance of Idler in the FW

src/modules/motion.cpp

@@ -91,7 +91,7 @@ void Motion::StallGuardReset(Axis axis) {
     axisData[axis].drv.ClearStallguard();
 }
 
-void Motion::PlanStallGuardThreshold(config::Axis axis, int16_t sg_thrs) {
+void Motion::PlanStallGuardThreshold(config::Axis axis, int8_t sg_thrs) {
     mm::axisParams[axis].params.sg_thrs = sg_thrs;
 }
 

src/modules/motion.h

@@ -105,7 +105,7 @@ public:
 
     /// Sets (plans) StallGuard threshold for an axis (basically the higher number the lower sensitivity)
     /// The new SGTHRS value gets applied in Init(), it is _NOT_ written into the TMC immediately in this method.
-    void PlanStallGuardThreshold(Axis axis, int16_t sg_thrs);
+    void PlanStallGuardThreshold(Axis axis, int8_t sg_thrs);
 
     /// Enqueue a single axis move in steps starting and ending at zero speed with maximum
     /// feedrate. Moves can only be enqueued if the axis is not Full().

src/modules/movable_base.cpp

@@ -69,15 +69,13 @@ void MovableBase::PerformHomeForward() {
     if (mm::motion.StallGuard(axis)) {
         // we have reached the front end of the axis - first part homed probably ok
         mm::motion.StallGuardReset(axis);
-        if (SGAllowed(true)) {
+        if (StallGuardAllowed(true)) {
             mm::motion.AbortPlannedMoves(axis, true);
             PlanHomingMoveBack();
             state = HomeBack;
         }
     } else if (mm::motion.QueueEmpty(axis)) {
         HomeFailed();
-    } else {
-        UpdateAdaptiveSGTHRS(true);
     }
 }
 
@@ -85,7 +83,7 @@ void MovableBase::PerformHomeBack() {
     if (mm::motion.StallGuard(axis)) {
         // we have reached the back end of the axis - second part homed probably ok
         mm::motion.StallGuardReset(axis);
-        if (SGAllowed(false)) {
+        if (StallGuardAllowed(false)) {
             mm::motion.AbortPlannedMoves(axis, true);
             mm::motion.SetMode(axis, mg::globals.MotorsStealth() ? mm::Stealth : mm::Normal);
             if (!FinishHomingAndPlanMoveToParkPos()) {
@@ -98,8 +96,6 @@ void MovableBase::PerformHomeBack() {
         }
     } else if (mm::motion.QueueEmpty(axis)) {
         HomeFailed();
-    } else {
-        UpdateAdaptiveSGTHRS(false);
     }
 }
 

src/modules/movable_base.h

@@ -109,10 +109,12 @@ protected:
     /// @returns true if the measured axis length is within the expected range, false otherwise
     virtual bool FinishHomingAndPlanMoveToParkPos() = 0;
     virtual void FinishMove() = 0;
-
+    /// @returns true if the StallGuard signal is to be considered while homing.
-    /// default implementation is empty
+    /// It may sound counterintuitive, but due to SG/homing issues on the Idler,
-    virtual void UpdateAdaptiveSGTHRS(bool /*forward*/) {}
+    /// it needs to avoid processing the SG while rotating over the filament.
-    virtual bool SGAllowed(bool forward) const { return true; }
+    /// The Idler must consider SG signal only when close to its real end stops.
+    /// Selector considers the SG signal all the time while homing, therefore the default implementation is empty
+    virtual bool StallGuardAllowed(bool forward) const { return true; }
 
     /// Initializes movement of a movable module.
     /// Beware: this operation reinitializes the axis/TMC driver as well (may introduce axis creep as we have seen on the Idler)

tests/unit/logic/homing/test_homing.cpp

@@ -207,7 +207,7 @@ void AdaptiveIdlerHoming() {
     main_loop();
     CHECK(mm::motion.CurPosition<mm::Idler>().v == mm::unitToSteps<mm::I_pos_t>(config::IdlerOffsetFromHome) + 1); // magic constant just to tune the motor steps
     CHECK(mi::idler.axisStart == config::IdlerOffsetFromHome.v + 2);
-    CHECK(mm::axes[mm::Idler].sg_thrs == 32767);
+    CHECK(mm::axes[mm::Idler].sg_thrs == 32767); // @@TODO sg_thrs is int8_t by default
     // do exact number of steps before triggering SG
     uint32_t idlerSteps = mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght);
     uint32_t sgChange = mm::unitToAxisUnit<mm::I_pos_t>(config::idlerLimits.lenght - 15.0_deg).v;
@@ -243,6 +243,8 @@ void AdaptiveIdlerHoming() {
     }
 }
 
-TEST_CASE("homing::adaptive", "[homing]") {
+// currently disabled, adaptive stallguard threshold turned out to be highly unreliable when homing the Idler
+// but the code will be kept around for some time as a proof of concept
+TEST_CASE("homing::adaptive", "[homing][.]") {
     AdaptiveIdlerHoming();
 }

tests/unit/logic/stubs/homing.cpp

@@ -57,8 +57,8 @@ void SimulateIdlerAndSelectorHoming(logic::CommandBase &cb) {
 }
 
 void SimulateIdlerHoming(logic::CommandBase &cb) {
-    // do 5 steps until we trigger the simulated StallGuard
+    uint32_t idlerStepsFwd = mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght - 5.0_deg);
-    for (uint8_t i = 0; i < 5; ++i) {
+    for (uint32_t i = 0; i < idlerStepsFwd; ++i) {
         main_loop();
         cb.Step();
     }
@@ -163,9 +163,12 @@ bool SimulateFailedHomeFirstTime(logic::CommandBase &cb) {
     if (ms::selector.HomingValid())
         return false;
 
+    constexpr uint32_t selectorSteps = mm::unitToSteps<mm::S_pos_t>(config::selectorLimits.lenght) + 1;
     {
         // do 5 steps until we trigger the simulated StallGuard
-        for (uint8_t i = 0; i < 5; ++i) {
+        constexpr uint32_t idlerStepsFwd = mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght - 5.0_deg);
+        static_assert(idlerStepsFwd < selectorSteps); // beware, we expect that the Idler homes faster than Selector (less steps)
+        for (uint32_t i = 0; i < idlerStepsFwd; ++i) {
             main_loop();
             cb.Step();
         }
@@ -178,11 +181,11 @@ bool SimulateFailedHomeFirstTime(logic::CommandBase &cb) {
         mm::motion.StallGuardReset(mm::Idler);
     }
     // now do a correct amount of steps of each axis towards the other end
-    uint32_t idlerSteps = mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght);
+    constexpr uint32_t idlerSteps = mm::unitToSteps<mm::I_pos_t>(config::idlerLimits.lenght);
     // now do LESS steps than expected to simulate something is blocking the selector
-    uint32_t selectorSteps = mm::unitToSteps<mm::S_pos_t>(config::selectorLimits.lenght) + 1;
+
-    uint32_t selectorTriggerShort = std::min(idlerSteps, selectorSteps) / 2;
+    constexpr uint32_t selectorTriggerShort = std::min(idlerSteps, selectorSteps) / 2;
-    uint32_t maxSteps = selectorTriggerShort + 1;
+    constexpr uint32_t maxSteps = selectorTriggerShort + 1;
     {
         for (uint32_t i = 0; i < maxSteps; ++i) {
             main_loop();

tests/unit/logic/stubs/stub_motion.cpp

@@ -41,7 +41,7 @@ void TriggerStallGuard(Axis axis) {
     axes[axis].stallGuard = true;
 }
 
-void Motion::PlanStallGuardThreshold(Axis axis, int16_t sg_thrs) {
+void Motion::PlanStallGuardThreshold(Axis axis, int8_t sg_thrs) {
     // do nothing for now
     axes[axis].sg_thrs = sg_thrs;
 }

tests/unit/logic/stubs/stub_motion.h

@@ -10,7 +10,7 @@ struct AxisSim {
     bool enabled;
     bool homed;
     bool stallGuard;
-    int16_t sg_thrs;
+    int8_t sg_thrs;
     std::deque<pos_t> plannedMoves;
 };