From 1f14e950e7e0c08dcc5553dd4c6c1071ff14eef7 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Mon, 1 Sep 2025 13:34:59 -0400 Subject: [PATCH] stepper_enable: Unify explicit stepper enable/disable code There were several slightly different implementations of explicit stepper motor enabling/disabling in the force_move, stepper_enable, and manual_stepper modules. Introduce a new set_motors_enable() method and use this in all implementations. This simplifies the code and reduces the chance of obscure timing issues. This fixes a manual_stepper error introduced in commit 9399e738. That commit changed the manual_stepper class to no longer explicitly flush and clear all steps after each move, which broke the expectations of manual_stepper's custom enable code. Using the more robust implementation in stepper_enable fixes that issue. Signed-off-by: Kevin O'Connor --- klippy/extras/force_move.py | 33 +++++++++---------------- klippy/extras/manual_stepper.py | 10 ++------ klippy/extras/stepper_enable.py | 44 ++++++++++++++++++--------------- klippy/extras/z_tilt.py | 2 +- 4 files changed, 39 insertions(+), 50 deletions(-) diff --git a/klippy/extras/force_move.py b/klippy/extras/force_move.py index 00f835f5..3947292b 100644 --- a/klippy/extras/force_move.py +++ b/klippy/extras/force_move.py @@ -1,6 +1,6 @@ # Utility for manually moving a stepper for diagnostic purposes # -# Copyright (C) 2018-2019 Kevin O'Connor +# Copyright (C) 2018-2025 Kevin O'Connor # # This file may be distributed under the terms of the GNU GPLv3 license. import math, logging @@ -10,7 +10,6 @@ BUZZ_DISTANCE = 1. BUZZ_VELOCITY = BUZZ_DISTANCE / .250 BUZZ_RADIANS_DISTANCE = math.radians(1.) BUZZ_RADIANS_VELOCITY = BUZZ_RADIANS_DISTANCE / .250 -STALL_TIME = 0.100 # Calculate a move's accel_t, cruise_t, and cruise_v def calc_move_time(dist, speed, accel): @@ -56,24 +55,16 @@ class ForceMove: raise self.printer.config_error("Unknown stepper %s" % (name,)) return self.steppers[name] def _force_enable(self, stepper): - toolhead = self.printer.lookup_object('toolhead') - print_time = toolhead.get_last_move_time() + stepper_name = stepper.get_name() stepper_enable = self.printer.lookup_object('stepper_enable') - enable = stepper_enable.lookup_enable(stepper.get_name()) - was_enable = enable.is_motor_enabled() - if not was_enable: - enable.motor_enable(print_time) - toolhead.dwell(STALL_TIME) - return was_enable - def _restore_enable(self, stepper, was_enable): - if not was_enable: - toolhead = self.printer.lookup_object('toolhead') - toolhead.dwell(STALL_TIME) - print_time = toolhead.get_last_move_time() - stepper_enable = self.printer.lookup_object('stepper_enable') - enable = stepper_enable.lookup_enable(stepper.get_name()) - enable.motor_disable(print_time) - toolhead.dwell(STALL_TIME) + did_enable = stepper_enable.set_motors_enable([stepper_name], True) + return did_enable + def _restore_enable(self, stepper, did_enable): + if not did_enable: + return + stepper_name = stepper.get_name() + stepper_enable = self.printer.lookup_object('stepper_enable') + stepper_enable.set_motors_enable([stepper_name], False) def manual_move(self, stepper, dist, speed, accel=0.): toolhead = self.printer.lookup_object('toolhead') toolhead.flush_step_generation() @@ -100,7 +91,7 @@ class ForceMove: def cmd_STEPPER_BUZZ(self, gcmd): stepper = self._lookup_stepper(gcmd) logging.info("Stepper buzz %s", stepper.get_name()) - was_enable = self._force_enable(stepper) + did_enable = self._force_enable(stepper) toolhead = self.printer.lookup_object('toolhead') dist, speed = BUZZ_DISTANCE, BUZZ_VELOCITY if stepper.units_in_radians(): @@ -110,7 +101,7 @@ class ForceMove: toolhead.dwell(.050) self.manual_move(stepper, -dist, speed) toolhead.dwell(.450) - self._restore_enable(stepper, was_enable) + self._restore_enable(stepper, did_enable) cmd_FORCE_MOVE_help = "Manually move a stepper; invalidates kinematics" def cmd_FORCE_MOVE(self, gcmd): stepper = self._lookup_stepper(gcmd) diff --git a/klippy/extras/manual_stepper.py b/klippy/extras/manual_stepper.py index 3d3e614a..a2ce57da 100644 --- a/klippy/extras/manual_stepper.py +++ b/klippy/extras/manual_stepper.py @@ -50,15 +50,9 @@ class ManualStepper: self.next_cmd_time = print_time def do_enable(self, enable): self.sync_print_time() + stepper_names = [s.get_name() for s in self.steppers] stepper_enable = self.printer.lookup_object('stepper_enable') - if enable: - for s in self.steppers: - se = stepper_enable.lookup_enable(s.get_name()) - se.motor_enable(self.next_cmd_time) - else: - for s in self.steppers: - se = stepper_enable.lookup_enable(s.get_name()) - se.motor_disable(self.next_cmd_time) + stepper_enable.set_motors_enable(stepper_names, enable) self.sync_print_time() def do_set_position(self, setpos): toolhead = self.printer.lookup_object('toolhead') diff --git a/klippy/extras/stepper_enable.py b/klippy/extras/stepper_enable.py index 2bad7555..cd3f4f73 100644 --- a/klippy/extras/stepper_enable.py +++ b/klippy/extras/stepper_enable.py @@ -88,30 +88,30 @@ class PrinterStepperEnable: name = mcu_stepper.get_name() enable = setup_enable_pin(self.printer, config.get('enable_pin', None)) self.enable_lines[name] = EnableTracking(mcu_stepper, enable) + def set_motors_enable(self, stepper_names, enable): + toolhead = self.printer.lookup_object('toolhead') + toolhead.dwell(DISABLE_STALL_TIME) + print_time = toolhead.get_last_move_time() + did_change = False + for stepper_name in stepper_names: + el = self.enable_lines[stepper_name] + was_enabled = el.is_motor_enabled() + if enable: + el.motor_enable(print_time) + else: + el.motor_disable(print_time) + if was_enabled != el.is_motor_enabled(): + did_change = True + toolhead.dwell(DISABLE_STALL_TIME) + return did_change def motor_off(self): + self.set_motors_enable(self.get_steppers(), False) toolhead = self.printer.lookup_object('toolhead') - toolhead.dwell(DISABLE_STALL_TIME) - print_time = toolhead.get_last_move_time() - for el in self.enable_lines.values(): - el.motor_disable(print_time) toolhead.get_kinematics().clear_homing_state("xyz") - self.printer.send_event("stepper_enable:motor_off", print_time) - toolhead.dwell(DISABLE_STALL_TIME) - def motor_debug_enable(self, stepper, enable): - toolhead = self.printer.lookup_object('toolhead') - toolhead.dwell(DISABLE_STALL_TIME) - print_time = toolhead.get_last_move_time() - el = self.enable_lines[stepper] - if enable: - el.motor_enable(print_time) - logging.info("%s has been manually enabled", stepper) - else: - el.motor_disable(print_time) - logging.info("%s has been manually disabled", stepper) - toolhead.dwell(DISABLE_STALL_TIME) + self.printer.send_event("stepper_enable:motor_off") def get_status(self, eventtime): steppers = { name: et.is_motor_enabled() - for (name, et) in self.enable_lines.items() } + for (name, et) in self.enable_lines.items() } return {'steppers': steppers} def _handle_request_restart(self, print_time): self.motor_off() @@ -126,7 +126,11 @@ class PrinterStepperEnable: % (stepper_name,)) return stepper_enable = gcmd.get_int('ENABLE', 1) - self.motor_debug_enable(stepper_name, stepper_enable) + self.set_motors_enable([stepper_name], stepper_enable) + if stepper_enable: + logging.info("%s has been manually enabled", stepper_name) + else: + logging.info("%s has been manually disabled", stepper_name) def lookup_enable(self, name): if name not in self.enable_lines: raise self.printer.config_error("Unknown stepper '%s'" % (name,)) diff --git a/klippy/extras/z_tilt.py b/klippy/extras/z_tilt.py index 9f5ea0b9..0316ee72 100644 --- a/klippy/extras/z_tilt.py +++ b/klippy/extras/z_tilt.py @@ -79,7 +79,7 @@ class ZAdjustStatus: self.applied = False def get_status(self, eventtime): return {'applied': self.applied} - def _motor_off(self, print_time): + def _motor_off(self): self.reset() class RetryHelper: