endstop_phase: Obtain phases directly from stepper microsteps config

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>

6 files changed, 62 insertions, 54 deletions

diff --git a/config/printer-makergear-m2-2012.cfg b/config/printer-makergear-m2-2012.cfg
index 7993fba8..03ae2bd0 100644
--- a/config/printer-makergear-m2-2012.cfg
+++ b/config/printer-makergear-m2-2012.cfg
@@ -7,35 +7,36 @@
 step_pin: PC0
 dir_pin: !PL1
 enable_pin: !PA7
-step_distance: .0225
+microsteps: 8
+rotation_distance: 36
 endstop_pin: ^!PB6
 position_endstop: 0.0
 position_max: 200
 homing_speed: 50
 
 [endstop_phase stepper_x]
-phases: 32
 endstop_accuracy: .200
 
 [stepper_y]
 step_pin: PC1
 dir_pin: PL0
 enable_pin: !PA6
-step_distance: .0225
+microsteps: 8
+rotation_distance: 36
 endstop_pin: ^!PB5
 position_endstop: 0.0
 position_max: 250
 homing_speed: 50
 
 [endstop_phase stepper_y]
-phases: 32
 endstop_accuracy: .200
 
 [stepper_z]
 step_pin: PC2
 dir_pin: !PL2
 enable_pin: !PA5
-step_distance: .005
+microsteps: 8
+rotation_distance: 8
 endstop_pin: ^!PB4
 position_min: 0.1
 position_endstop: 0.7
@@ -43,14 +44,15 @@ position_max: 200
 homing_retract_dist: 2.0
 
 [endstop_phase stepper_z]
-phases: 32
 endstop_accuracy: .070
 
 [extruder]
 step_pin: PC3
 dir_pin: PL6
 enable_pin: !PA4
-step_distance: .004242
+microsteps: 8
+gear_ratio: 57:11
+rotation_distance: 35.170
 nozzle_diameter: 0.350
 filament_diameter: 1.750
 heater_pin: PH6
diff --git a/docs/Config_Changes.md b/docs/Config_Changes.md
index 6d4c03da..4218034f 100644
--- a/docs/Config_Changes.md
+++ b/docs/Config_Changes.md
@@ -6,6 +6,11 @@ All dates in this document are approximate.
 
 # Changes
 
+20201218: The `endstop_phase` setting in the endstop_phase module has
+been replaced with `trigger_phase`. If using the endstop phases module
+then it will be necessary to recalibrate any endstop phases by running
+the ENDSTOP_PHASE_CALIBRATE command.
+
 20201218: Rotary delta and polar printers must now specify a
 `gear_ratio` for their rotary steppers, and they may no longer specify
 a `step_distance` parameter.  See the
diff --git a/docs/Config_Reference.md b/docs/Config_Reference.md
index 92a147fb..7d58c636 100644
--- a/docs/Config_Reference.md
+++ b/docs/Config_Reference.md
@@ -1112,21 +1112,17 @@ for additional information.
 
 ```
 [endstop_phase stepper_z]
-#phases:
-#   This specifies the number of phases of the given stepper motor
-#   driver (which is the number of micro-steps multiplied by four).
-#   This setting is automatically determined if one uses a TMC driver
-#   with run-time configuration. Otherwise, this parameter must be
-#   provided.
 #endstop_accuracy:
 #   Sets the expected accuracy (in mm) of the endstop. This represents
 #   the maximum error distance the endstop may trigger (eg, if an
 #   endstop may occasionally trigger 100um early or up to 100um late
 #   then set this to 0.200 for 200um). The default is
-#   phases*step_distance.
-#endstop_phase:
+#   4*rotation_distance/full_steps_per_rotation.
+#trigger_phase:
 #   This specifies the phase of the stepper motor driver to expect
-#   when hitting the endstop. Only set this value if one is sure the
+#   when hitting the endstop. It is composed of two numbers separated
+#   by a forward slash character - the phase and the total number of
+#   phases (eg, "7/64"). Only set this value if one is sure the
 #   stepper motor driver is reset every time the mcu is reset. If this
 #   is not set, then the stepper phase will be detected on the first
 #   home and that phase will be used on all subsequent homes.
diff --git a/docs/Endstop_Phase.md b/docs/Endstop_Phase.md
index ec9ee68e..fe0138d3 100644
--- a/docs/Endstop_Phase.md
+++ b/docs/Endstop_Phase.md
@@ -121,6 +121,6 @@ Additional notes
   will arrange for the micro-controller to always be reset via a USB
   power reset, which would arrange for both the micro-controller and
   stepper motor drivers to be reset together. If using this mechanism,
-  one would then need to manually configure the "endstop_phase" config
+  one would then need to manually configure the "trigger_phase" config
   sections (see [config reference](Config_Reference.md#endstop_phase)
   for the details).
diff --git a/klippy/extras/endstop_phase.py b/klippy/extras/endstop_phase.py
index ae22720d..66e9fc38 100644
--- a/klippy/extras/endstop_phase.py
+++ b/klippy/extras/endstop_phase.py
@@ -4,9 +4,13 @@
 #
 # This file may be distributed under the terms of the GNU GPLv3 license.
 import math, logging
+import stepper
 
 TRINAMIC_DRIVERS = ["tmc2130", "tmc2208", "tmc2209", "tmc2660", "tmc5160"]
 
+def convert_phase(driver_phase, driver_phases, phases):
+    return (int(float(driver_phase) / driver_phases * phases + .5) % phases)
+
 class EndstopPhase:
     def __init__(self, config):
         self.printer = config.get_printer()
@@ -17,37 +21,26 @@ class EndstopPhase:
         self.printer.register_event_handler("homing:home_rails_end",
                                             self.handle_home_rails_end)
         self.printer.load_object(config, "endstop_phase")
-        self.printer.load_object(config, "force_move")
+        # Obtain step_distance and microsteps from stepper config section
+        sconfig = config.getsection(self.name)
+        self.step_dist = stepper.parse_step_distance(sconfig)
+        self.phases = sconfig.getint("microsteps", note_valid=False) * 4
         # Read config
-        self.phases = config.getint('phases', None, minval=1)
-        self.endstop_phase = config.getint('endstop_phase', None, minval=0)
+        self.endstop_phase = None
+        trigger_phase = config.get('trigger_phase', None)
+        if trigger_phase is not None:
+            try:
+                p, ps = [int(v.strip()) for v in trigger_phase.split('/')]
+            except:
+                raise config.error("Unable to parse trigger_phase '%s'"
+                                   % (trigger_phase,))
+            if p >= ps:
+                raise config.error("Invalid trigger_phase '%s'"
+                                   % (trigger_phase,))
+            self.endstop_phase = convert_phase(p, ps, self.phases)
         self.endstop_align_zero = config.getboolean('endstop_align_zero', False)
         self.endstop_accuracy = config.getfloat('endstop_accuracy', None,
                                                 above=0.)
-        self.step_dist = self.endstop_phase_accuracy = None
-    def handle_connect(self):
-        # Determine number of stepper phases
-        for driver in TRINAMIC_DRIVERS:
-            driver_name = "%s %s" % (driver, self.name)
-            module = self.printer.lookup_object(driver_name, None)
-            if module is not None:
-                self.get_phase = module.get_phase
-                if self.phases is not None:
-                    raise self.printer.config_error(
-                        "endstop_phase phases set with tmc driver")
-                self.phases = module.get_microsteps() * 4
-                break
-        else:
-            self.get_phase = None
-            if self.phases is None:
-                raise self.printer.config_error(
-                    "endstop_phase phases must be specified")
-        if self.endstop_phase is not None and self.endstop_phase >= self.phases:
-            raise self.printer.config_error(
-                "endstop_phase endstop_phase parameter not valid")
-        # Lookup stepper step_dist
-        force_move = self.printer.lookup_object("force_move")
-        self.step_dist = force_move.lookup_stepper(self.name).get_step_dist()
         # Determine endstop accuracy
         if self.endstop_accuracy is None:
             self.endstop_phase_accuracy = self.phases//2 - 1
@@ -63,6 +56,15 @@ class EndstopPhase:
         if self.printer.get_start_args().get('debugoutput') is not None:
             self.endstop_phase_accuracy = self.phases
         self.phase_history = [0] * self.phases
+        self.get_phase = None
+    def handle_connect(self):
+        # Check for trinamic driver with get_phase() method
+        for driver in TRINAMIC_DRIVERS:
+            driver_name = "%s %s" % (driver, self.name)
+            module = self.printer.lookup_object(driver_name, None)
+            if module is not None:
+                self.get_phase = module.get_phase
+                break
     def align_endstop(self, pos):
         if not self.endstop_align_zero or self.endstop_phase is None:
             return pos
@@ -76,13 +78,14 @@ class EndstopPhase:
     def get_homed_offset(self, stepper):
         if self.get_phase is not None:
             try:
-                phase = self.get_phase()
+                driver_phase, driver_phases = self.get_phase()
             except Exception as e:
                 msg = "Unable to get stepper %s phase: %s" % (self.name, str(e))
                 logging.exception(msg)
                 raise self.printer.command_error(msg)
             if stepper.is_dir_inverted():
-                phase = (self.phases - 1) - phase
+                driver_phase = (driver_phases - 1) - driver_phase
+            phase = convert_phase(driver_phase, driver_phases, self.phases)
         else:
             phase = stepper.get_mcu_position() % self.phases
         self.phase_history[phase] += 1
@@ -140,10 +143,11 @@ class EndstopPhases:
         if get_phase is None:
             return
         try:
-            phase = get_phase()
+            driver_phase, driver_phases = get_phase()
         except:
             logging.exception("Error in EndstopPhases get_phase")
             return
+        phase = convert_phase(driver_phase, driver_phases, len(phase_history))
         phase_history[phase] += 1
     def handle_home_rails_end(self, rails):
         for rail in rails:
@@ -163,11 +167,12 @@ class EndstopPhases:
         if info is None:
             raise gcmd.error("Stats not available for stepper %s"
                              % (stepper_name,))
-        endstop_phase = self.generate_stats(stepper_name, info)
+        endstop_phase, phases = self.generate_stats(stepper_name, info)
         configfile = self.printer.lookup_object('configfile')
-        section = 'endstop_phase %s' % stepper_name
+        section = 'endstop_phase %s' % (stepper_name,)
         configfile.remove_section(section)
-        configfile.set(section, "endstop_phase", endstop_phase)
+        configfile.set(section, "trigger_phase",
+                       "%s/%s" % (endstop_phase, phases))
         gcmd.respond_info(
             "The SAVE_CONFIG command will update the printer config\n"
             "file with these parameters and restart the printer.")
@@ -188,9 +193,9 @@ class EndstopPhases:
                  if wph[j]]
         best_phase = best % phases
         lo, hi = found[0] % phases, found[-1] % phases
-        self.gcode.respond_info("%s: endstop_phase=%d (range %d to %d)"
-                                % (stepper_name, best_phase, lo, hi))
-        return best_phase
+        self.gcode.respond_info("%s: trigger_phase=%d/%d (range %d to %d)"
+                                % (stepper_name, best_phase, phases, lo, hi))
+        return best_phase, phases
     def report_stats(self):
         if not self.tracking:
             self.gcode.respond_info(
diff --git a/klippy/extras/tmc.py b/klippy/extras/tmc.py
index c14a5f0f..4d3296aa 100644
--- a/klippy/extras/tmc.py
+++ b/klippy/extras/tmc.py
@@ -283,7 +283,7 @@ class TMCMicrostepHelper:
             field_name = "MSTEP"
         reg = self.mcu_tmc.get_register(self.fields.lookup_register(field_name))
         mscnt = self.fields.get_field(field_name, reg)
-        return (1023 - mscnt) >> self.fields.get_field("MRES")
+        return 1023 - mscnt, 1024
 
 # Helper to configure "stealthchop" mode
 def TMCStealthchopHelper(config, mcu_tmc, tmc_freq):