diff --git a/src/hal/tmc2130.h b/src/hal/tmc2130.h index a4a5715..5a95f7a 100644 --- a/src/hal/tmc2130.h +++ b/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; }; diff --git a/src/modules/idler.cpp b/src/modules/idler.cpp index 63ae10c..783ec9f 100644 --- a/src/modules/idler.cpp +++ b/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(config::IdlerOffsetFromHome)); axisStart = mm::axisUnitToTruncatedUnit(mm::motion.CurPosition()); mm::motion.PlanMove(mm::unitToAxisUnit(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(config::idlerLimits.lenght)); axisStart = mm::axisUnitToTruncatedUnit(mm::motion.CurPosition()); @@ -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(mm::motion.CurPosition())) > checkDistance; } -void Idler::UpdateAdaptiveSGTHRS(bool forward) { - // return; - const uint8_t checkDistance = forward ? 220 : 200; - - if (AxisDistance(mm::axisUnitToTruncatedUnit(mm::motion.CurPosition())) - > 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")); diff --git a/src/modules/idler.h b/src/modules/idler.h index 3021774..0aeef0f 100644 --- a/src/modules/idler.h +++ b/src/modules/idler.h @@ -17,8 +17,7 @@ public: inline constexpr Idler() : MovableBase(mm::Idler) , plannedMove(Operation::disengage) - , currentlyEngaged(Operation::disengage) - , fwdHomeTrigger(false) {} + , currentlyEngaged(Operation::disengage) {} /// 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 SGAllowed(bool forward) const override; + virtual bool StallGuardAllowed(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 diff --git a/src/modules/motion.cpp b/src/modules/motion.cpp index 7c3df1b..0be7a63 100644 --- a/src/modules/motion.cpp +++ b/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; } diff --git a/src/modules/motion.h b/src/modules/motion.h index 491722b..0651185 100644 --- a/src/modules/motion.h +++ b/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(). diff --git a/src/modules/movable_base.cpp b/src/modules/movable_base.cpp index 164d0a1..134e2d2 100644 --- a/src/modules/movable_base.cpp +++ b/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); } } diff --git a/src/modules/movable_base.h b/src/modules/movable_base.h index fdb39e0..74d1585 100644 --- a/src/modules/movable_base.h +++ b/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; - - /// default implementation is empty - virtual void UpdateAdaptiveSGTHRS(bool /*forward*/) {} - virtual bool SGAllowed(bool forward) const { return true; } + /// @returns true if the StallGuard signal is to be considered while homing. + /// It may sound counterintuitive, but due to SG/homing issues on the Idler, + /// it needs to avoid processing the SG while rotating over the filament. + /// 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) diff --git a/tests/unit/logic/homing/test_homing.cpp b/tests/unit/logic/homing/test_homing.cpp index ecce700..747caf2 100644 --- a/tests/unit/logic/homing/test_homing.cpp +++ b/tests/unit/logic/homing/test_homing.cpp @@ -207,7 +207,7 @@ void AdaptiveIdlerHoming() { main_loop(); CHECK(mm::motion.CurPosition().v == mm::unitToSteps(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(config::idlerLimits.lenght); uint32_t sgChange = mm::unitToAxisUnit(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(); } diff --git a/tests/unit/logic/stubs/homing.cpp b/tests/unit/logic/stubs/homing.cpp index dad3f75..a37f90a 100644 --- a/tests/unit/logic/stubs/homing.cpp +++ b/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 - for (uint8_t i = 0; i < 5; ++i) { + uint32_t idlerStepsFwd = mm::unitToSteps(config::idlerLimits.lenght - 5.0_deg); + 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(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(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(config::idlerLimits.lenght); + constexpr uint32_t idlerSteps = mm::unitToSteps(config::idlerLimits.lenght); // now do LESS steps than expected to simulate something is blocking the selector - uint32_t selectorSteps = mm::unitToSteps(config::selectorLimits.lenght) + 1; - uint32_t selectorTriggerShort = std::min(idlerSteps, selectorSteps) / 2; - uint32_t maxSteps = selectorTriggerShort + 1; + + constexpr uint32_t selectorTriggerShort = std::min(idlerSteps, selectorSteps) / 2; + constexpr uint32_t maxSteps = selectorTriggerShort + 1; { for (uint32_t i = 0; i < maxSteps; ++i) { main_loop(); diff --git a/tests/unit/logic/stubs/stub_motion.cpp b/tests/unit/logic/stubs/stub_motion.cpp index d458157..8d0af15 100644 --- a/tests/unit/logic/stubs/stub_motion.cpp +++ b/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; } diff --git a/tests/unit/logic/stubs/stub_motion.h b/tests/unit/logic/stubs/stub_motion.h index 2b7c302..5f3d3e3 100644 --- a/tests/unit/logic/stubs/stub_motion.h +++ b/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 plannedMoves; };