From 2b92a76f2f01333ee6bc0ff365da8c8d9896c630 Mon Sep 17 00:00:00 2001 From: Oleksandr Shyshatskyi Date: Mon, 31 Aug 2026 21:14:44 +0300 Subject: [PATCH 1/3] The PA sweep never moves the carriage, so park it over the chute first SET_PIN PIN=enable_pin_tmc_x VALUE=1.00 disables the X driver outright. It is not Klipper's stepper enable -- that is !PJ6 and !PJ2 -- but a second latch on PH2/PH3 that Klipper knows nothing about. So for the whole sweep every G1 X in the ladder is planned, step-generated and clocked out against dead drivers, and the gram of filament falls straight down from wherever the toolhead was standing. clearNozzleEddy settles it. Just before its jal to paTestMgr it builds the string resetTmcPa replays: X = 275 + tool_dx, Y = 254 + tool_dy, Z = probeZ + 2.8, from the constants at 0xdb6b2c/0xdb6b28/0xdb6b24 -- the purge chute, the same pair ff-filament.cfg carries as purge_x and purge_y, with a -8.0 that is literally our tool_dy[3]. The app parks over the chute, kills X and Y, dumps into the chute, and then tells Klipper the carriage is still at the chute, which is only sane because it never left. We inherited the latch and the height without the park, so the ladder piled onto the plate wherever the nozzle happened to be. Park in _FF_FILAMENT_PREP's own three moves before the latch goes on, replay SET_KINEMATIC_POSITION after, and shout when the pins will not unlatch rather than leaving a printer that cannot move. The replay lands after RESTORE_GCODE_STATE, not before: set_position fires toolhead:set_position and gcode_move recomputes last_position from base_position/homing_position, so it has to see the restored offsets. --- docs/notes/51-pa-calibration-recovered.md | 81 +++++- pkgs/klipper-config/payload/config/ff-pa.cfg | 36 ++- .../payload/klipper/klippy/extras/ff_pa.py | 231 +++++++++++++++--- 3 files changed, 298 insertions(+), 50 deletions(-) diff --git a/docs/notes/51-pa-calibration-recovered.md b/docs/notes/51-pa-calibration-recovered.md index 7db01d4..f323a5a 100644 --- a/docs/notes/51-pa-calibration-recovered.md +++ b/docs/notes/51-pa-calibration-recovered.md @@ -143,8 +143,10 @@ Things worth noticing: slow-fast-slow-slow-fast-slow across X40..X200, so each line contains two accel/decel pairs at a 10x flow step -- the only place pressure advance can show up. - **The candidate order is scrambled** (0.010, 0.020, 0.015, 0.035, 0.025, 0.030, 0.040) while - the Y positions are sequential. Adjacent lines on the bed are therefore never adjacent in PA, - which keeps a slow drift (nozzle temperature, bed tilt) from biasing a contiguous band. + the Y positions are sequential. This *looks* like a guard against a slow drift biasing a + contiguous band of the bed, and an earlier revision of this note said so. It cannot be: as + [section 3a](#3a-the-carriage-does-not-move) shows, the carriage never moves and there is no + band. The scramble is kept because it is the app's, not because it buys anything here. - **`good[0]`, not the median.** Within one pass, several candidates can score 9; the app takes the *smallest* passing value. It then repeats the whole 7-line sweep until three passes have produced a winner, and averages exactly those three. Worst case 5 sweeps = 35 lines; fewer @@ -152,8 +154,54 @@ Things worth noticing: - **Nothing restores pressure advance.** After the sweep the active extruder is still at the last candidate tried (0.0400). Only the caller's `SET_PA_ADVANCE` is meant to install the result -- see the caveat in [section 6](#6-set_pa_advance-is-not-in-any-klipper-we-have). -- No heating, no homing, no Z move inside `paTestMgr`. The caller has already parked the tool at - the wipe spot and set the temperature. +- No heating, no homing, no Z move inside `paTestMgr`. The caller has already parked the tool and + set the temperature -- see below, because *where* it parked turns out to be the whole story. + +## 3a. The carriage does not move + +`SET_PIN PIN=enable_pin_tmc_x VALUE=1.00` **disables the X driver**, and `..._y` the Y driver. +They are not Klipper's stepper enables -- `[stepper_x] enable_pin: !PJ6`, `[stepper_y] +enable_pin: !PJ2` -- but a second, independent latch on `PH2`/`PH3` that Klipper knows nothing +about. So for the whole sweep every `G1 X40 ... X200` is planned, step-generated and clocked out +against dead drivers. **The toolhead does not move, no lines are drawn, and the whole ~1 g of +filament falls straight down from wherever the caller parked it.** + +The proof is the argument the caller hands `resetTmcPa`. Disassembling `clearNozzleEddy` just +before its `jal 0x791640`: + +``` +0078f030 lui $v0, 0xdb +0078f034 lwc1 $f0, 0x6b24($v0) ; -8.0 <- tool_dy[3] +0078f04c lwc1 $f1, 0x30($fp) +0078f054 lwc1 $f0, 0x6b28($v0) ; 254.0 <- purge_y +0078f058 add.s $f0, $f1, $f0 ; Y = 254 + tool_dy +0078f068 lwc1 $f0, 0x6b2c($v0) ; 275.0 <- purge_x +0078f06c add.s $f0, $f1, $f0 ; X = 275 + tool_dx +0078f080 ldc1 $f0, 0x6a90($v0) ; 2.8 +0078f084 add.d $f0, $f1, $f0 ; Z = + 2.8 +0078f0b8 addiu $a1, $v0, 0x35b0 ; "SET_KINEMATIC_POSITION X=" +0078f0d4 addiu $a2, $v0, 0x3240 ; " Y=" +0078f13c addiu $a2, $v0, 0x3b28 ; " Z=" +0078f244 jal 0x791640 ; paTestMgr(pa, ok, restore_gcode) +``` + +275.0 @0xdb6b2c and 254.0 @0xdb6b28 are the **purge chute**, the same pair +[`ff-filament.cfg`](../../pkgs/klipper-config/payload/config/ff-filament.cfg) carries as +`purge_x`/`purge_y`; the -8.0 @0xdb6b24 is literally our `tool_dy[3]`. The app parks over the +chute, kills X and Y, dumps the gram down the chute, and then tells Klipper the carriage is +**still at the chute** -- which is only a sane thing to say because it never left. Had the +ladder been real motion, forcing the position back to X275 Y246 would drive the next move into +the far right limit. + +`resetTmcPa` @0x79350c, disassembled in full, is exactly: + +``` +SET_PIN PIN=enable_pin_tmc_x VALUE=0.00 +SET_PIN PIN=enable_pin_tmc_y VALUE=0.00 +M400 +SET_KINEMATIC_POSITION X=<275+dx> Y=<254+dy> Z= +M400 +``` ## 4. Timing: the app does not synchronise, and neither should we @@ -256,8 +304,9 @@ have not pinned what each means. ### Why this matters for the port -The lines on the bed are a **byproduct**, not the measurement. Nothing -- no camera, no probe, no -later pass -- ever looks at them. They exist because you cannot produce a real pressure transient +The extrudate is a **byproduct**, not the measurement -- and since the carriage is latched still +([section 3a](#3a-the-carriage-does-not-move)) it is not even lines, just a gram of filament in +the chute. Nothing -- no camera, no probe, no later pass -- ever looks at it. They exist because you cannot produce a real pressure transient without really extruding. The scoring happens live, during the move, from a signal the toolhead is already streaming. That is why the port can reproduce this at all: we keep the eBoard firmware and the sensor, so we get the same verdicts for free. @@ -331,9 +380,19 @@ state, and the last run's table. procedure can structurally never report below its smallest candidate. - **Cold extruder, empty tool, out-of-range geometry are refused before anything moves**, rather than failing mid-line with the eBoard armed and the drivers latched. -- **`SET_KINEMATIC_POSITION` is not replayed.** The app needs it because its caller force-drove - the axes; our sweep is plain `G1` under Klipper's own kinematics, so replaying it would only - lie to Klipper about where the carriage is. +- **The park is ours, the latch is the app's.** `[ff_pa] park` (default on) drives the nozzle to + the chute in `_FF_FILAMENT_PREP`'s own three moves *before* the drivers are latched -- the app + relies on its caller having already gone there, and `FF_PA_CALIBRATE` has no such caller. With + `park: False` the gram lands wherever the nozzle happens to be, and the command says so before + it starts. +- **`SET_KINEMATIC_POSITION` *is* replayed**, to the parked coordinates. It has to be: after the + sweep Klipper believes the carriage is at the end of the ladder and it is over the chute, and + the axes are still flagged homed. An earlier revision of this port deliberately skipped it on + the reasoning that "our sweep is plain G1 under Klipper's own kinematics" -- that reasoning + was wrong, because the latch means our G1s move nothing either. +- **Failing to unlatch is reported, not swallowed.** Both `SET_PIN`s go back to their snapshot + value, and if that fails (a shutdown mid-run makes `run_script_from_command` raise) the command + prints the recovery line rather than leaving a printer that silently cannot move. - **`SAVE_GCODE_STATE`/`RESTORE_GCODE_STATE`** wrap the run; the app leaks its modal state. - **`min()` on floats**, not the app's lexicographic sort of fixed-width strings. Identical for its own seven values, correct for an override with mixed decimal widths. @@ -379,6 +438,10 @@ and the replica's include-wiring check are the whole automated story. | 0x9f50b0 | `paTestMgr` call in `BuildPage::clearNozzlePrint` | | 0x9f60fc | `SET_PA_ADVANCE T0=..` there | | 0x7a30c4 | `SET_PA_ADVANCE .. ENABLE=0` in `CommMgr::serialPrint` | +| 0x78f030 | the chute coordinates and `SET_KINEMATIC_POSITION` restore string, in `clearNozzleEddy` | +| 0xdb6b2c / 0xdb6b28 | `275.0` / `254.0` -- the purge chute, `purge_x` / `purge_y` | +| 0xdb6b24 | `-8.0` -- the tool dy applied to it, our `tool_dy[3]` | +| 0xdb6a90 | `2.8` (double) -- the Z added to the probe Z for the restore | | 0x791de8 | outer bound, `slti 5` | | 0x792878 | the verdict test, `bne $v1, 9` | | 0xdb6a80 | `3.0f`, the divisor | diff --git a/pkgs/klipper-config/payload/config/ff-pa.cfg b/pkgs/klipper-config/payload/config/ff-pa.cfg index ef27cdc..ef59ddd 100644 --- a/pkgs/klipper-config/payload/config/ff-pa.cfg +++ b/pkgs/klipper-config/payload/config/ff-pa.cfg @@ -19,10 +19,16 @@ # it gives the stock app. # # Four facts that shape this file: -# * The lines are extruded ~8 mm above the plate, IN FREE AIR. Nothing -# prints them and nothing ever looks at them -- they exist only to make a -# pressure transient at the two 10x flow steps. Expect about a gram of -# loose filament on the plate, and clear it before the next print. +# * NOTHING IS DRAWN AND THE CARRIAGE DOES NOT MOVE. The sweep latches +# enable_pin_tmc_x/y, which disables the X and Y drivers outright, so +# every G1 X in the ladder is planned by Klipper and executed by nobody. +# The ~1 g of filament falls straight down from wherever the toolhead was +# parked. We park it over the PURGE CHUTE first, which is what the app +# does (clearNozzleEddy builds X=275+tool_dx, Y=254+tool_dy from the same +# constants ff-filament.cfg carries as purge_x/purge_y), so the extrudate +# goes down the chute instead of onto the plate. The X/Y ladder is a +# timing generator: it stretches each candidate across the two 10x flow +# steps, and that is its whole job. # * Only the verdict 9 passes. Within a sweep the SMALLEST passing candidate # wins; the result is the mean of three sweep winners. So the answer can # never come out below the smallest candidate (0.0100 by default). @@ -42,14 +48,16 @@ # dressed up as a measurement. See require_discrimination. # * cold extruder, empty tool and out-of-range geometry are refused before # anything moves, instead of failing mid-line with the eBoard armed. -# * the app's SET_KINEMATIC_POSITION replay is NOT ported: our sweep is -# plain G1 under Klipper's own kinematics, so there is nothing to correct -# and replaying it would only lie to Klipper about where the carriage is. +# * a failure to unlatch the driver pins on exit is reported loudly instead +# of swallowed. That latch is the one piece of state that can leave the +# machine unable to move at all, and Klipper's own stepper enable is a +# different pin that knows nothing about it. # * the timing is NOT "improved". pa_action is an immediate MCU command and # there is deliberately no M400 between the last G1 and ACTION=0. The # closed scorer was tuned against that. Do not add barriers. # -# FF_PA_CALIBRATE [TOOL=] [TEMP=] [SWEEPS=] [CANDIDATES=] [DRY_RUN=1] +# FF_PA_CALIBRATE [TOOL=] [TEMP=] [SWEEPS=] [CANDIDATES=] [PARK=0] +# [DRY_RUN=1] # the full sweep on the MOUNTED tool. Roughly 4-5 minutes and ~1 g of # filament for the full five sweeps. DRY_RUN=1 prints the whole script # and the computed Z without moving. SWEEPS=1 is one sweep, ~1 min. @@ -105,6 +113,18 @@ # The X/Y driver latch the app holds for the duration, restored to whatever # it was. Empty disables the latch entirely. #tmc_pins: enable_pin_tmc_x, enable_pin_tmc_y +# Park over the purge chute before the drivers are latched, and replay the +# parked coordinates through SET_KINEMATIC_POSITION afterwards -- Klipper +# spends the sweep believing the carriage walked the ladder, and it never +# moved. park: False latches wherever the nozzle already stands and dumps the +# gram THERE, which is only ever right if you have put something under it +# yourself. The chute follows [gcode_macro _FF_FILAMENT]'s purge_x/purge_y +# and the tool's own dx/dy; park_x/park_y override that outright. +#park: True +#park_x: +#park_y: +# The staging X the app approaches the chute from (its "G1 X250"). +#park_approach_dx: 25 # pa_action's two arguments. The firmware tests == 11 for start and treats # everything else as stop; pc is stored and never read back on this eBoard # firmware, so 666 is cargo we keep for fidelity. diff --git a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py index d6c6e8c..01a0130 100644 --- a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py +++ b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py @@ -15,10 +15,28 @@ # firmware unchanged, so we get the same verdicts the touchscreen does. # [pa_adjust] is the 40-line forwarder that carries the two commands. # -# * THE LINES ARE EXTRUDED IN FREE AIR, ~8 mm above the bed, and nothing -# ever looks at them. They exist only to make a pressure transient at the -# two 10x flow steps. There is no first layer here and no adhesion to -# get right -- just about a gram of loose filament on the plate. +# * THE CARRIAGE DOES NOT MOVE, AND THAT IS THE WHOLE DESIGN. The prologue +# latches enable_pin_tmc_x/y, which hard-disables the X and Y drivers, so +# every `G1 X` in the sweep is planned by Klipper and executed by nobody. +# The toolhead stays exactly where the prologue parked it and the ~1 g of +# filament falls straight down from there. The app parks over the PURGE +# CHUTE first -- clearNozzleEddy @0x78f030 builds the restore string from +# X = 275 + tool_dx, Y = 254 + tool_dy, the same two constants +# ff-filament.cfg carries as purge_x/purge_y -- so the extrudate goes down +# the chute and never touches the plate. We now do the same. +# +# The X/Y ladder is therefore a TIMING GENERATOR and nothing else. It +# exists only to stretch each candidate over the two 10x flow steps where +# pressure advance can show up. Nothing draws lines, nothing looks at +# them, and the scrambled candidate order buys no spatial separation +# because there is no space -- it is kept because it is the app's. +# +# * SO SET_KINEMATIC_POSITION IS MANDATORY ON EXIT. After the sweep Klipper +# believes the carriage walked to the end of the ladder while it never +# left the chute. The app replays the parked coordinates (resetTmcPa +# @0x79350c takes them as its argument) and so must we -- without it the +# next move is planned from a position that is a lie, and it is homed +# state that makes it dangerous rather than merely wrong. # # * THE MISSING M400 IS DELIBERATE. pa_action is an immediate MCU command, # so ACTION=0 fires while motion is still queued; what bounds it is @@ -69,6 +87,15 @@ ACTION_STOP = 0 # ... and treats everything else as stop ACTION_PC = 666 # the app's "material" code; inert on this eBoard LINE_Z = 8.0 # RAW eddy frame, == _FF_FILAMENT's purge_z LINE_Z_FEED = 3000 +# The purge chute, and the staging X the app approaches it from. Defaults +# only: when [gcode_macro _FF_FILAMENT] is present its purge_x/purge_y and +# per-tool dx/dy win, so the chute has ONE definition in the tree. +PARK_X = 275.0 # @0xdb6b2c, == _FF_FILAMENT's purge_x +PARK_Y = 254.0 # @0xdb6b28, == _FF_FILAMENT's purge_y +PARK_APPROACH_DX = 25.0 # the app's "G1 X250" staging move +PARK_X_FEED = 6000 +PARK_Y_FEED = 24000 +PARK_APPROACH_FEED = 6000 TMC_PINS = ('enable_pin_tmc_x', 'enable_pin_tmc_y') FILAMENT_AREA_175 = 2.405 # mm^2, for the cross-section warning @@ -140,6 +167,14 @@ class FFPA: # definition of "8 mm above the plate" in the tree. self.line_z = config.getfloat('line_z', None) self.line_z_feed = config.getint('line_z_feed', LINE_Z_FEED, minval=1) + # Park over the purge chute before the drivers are latched. The whole + # ~1 g lands wherever this leaves the nozzle, so 'False' means "I have + # put something under the nozzle myself" -- not "park somewhere else". + self.park = config.getboolean('park', True) + self.park_x = config.getfloat('park_x', None) + self.park_y = config.getfloat('park_y', None) + self.park_approach_dx = config.getfloat('park_approach_dx', + PARK_APPROACH_DX) self.tmc_pin_names = [n.strip() for n in config.getlist('tmc_pins', list(TMC_PINS)) if n.strip()] @@ -159,6 +194,9 @@ class FFPA: self.dry_run = False self.running = False self.last = None + # Where the prologue left the nozzle, in G-code coordinates. What + # _restore feeds SET_KINEMATIC_POSITION; None until a run parks. + self.parked_at = None self.gcode.register_command('FF_PA_CALIBRATE', self.cmd_FF_PA_CALIBRATE, @@ -316,7 +354,7 @@ class FFPA: " require_filament_sensor: False if the switch is wrong." % (self.name, tool)) - def _check_bounds(self, gcmd): + def _check_bounds(self, gcmd, tool): """The ladder must fit the machine. The defaults sit well inside a Creator 5's envelope; an overridden ladder need not.""" _toolhead, status = self._toolhead_status() @@ -326,6 +364,13 @@ class FFPA: xs = [self.x_start] + [seg[0] for seg in self.segments] ys = [self.y_start + self.y_step * i for i in range(len(self.candidates))] + # The ladder is phantom motion, but Klipper still plans it and still + # range-checks it. The park is the one move that is real, so it has + # to fit for a different and more literal reason. + if self.park: + px, py, _xf, _yf, _af = self._chute(tool) + xs += [px, px - self.park_approach_dx] + ys.append(py) for axis, values, lo, hi in (('X', xs, low.x, high.x), ('Y', ys, low.y, high.y)): if min(values) < lo or max(values) > hi: @@ -335,6 +380,20 @@ class FFPA: " y_start/y_step." % (self.name, axis, min(values), max(values), lo, hi)) + def _where_note(self, tool, raw_z, z_target): + """Say plainly where the gram of filament is going to end up.""" + if not self.park: + return (" ! PARK=0: the drivers are latched where the nozzle" + " already stands, so the whole ~1 g lands THERE, at Z" + " %.3f raw / %.3f gcode. Nothing will move it aside." + % (raw_z, z_target)) + x, y, _xf, _yf, _af = self._chute(tool) + return (" the carriage parks over the purge chute at X%.1f Y%.1f," + " Z %.3f raw / %.3f gcode, and STAYS THERE: the X/Y drivers" + " are latched off for the sweep, so the ladder is timing" + " only and the ~1 g goes down the chute." + % (x, y, raw_z, z_target)) + def _cross_section_note(self, extruder): """Klipper caps extrude cross-section at 4*nozzle^2. The sweep's 0.137 mm^2 is comfortable on a 0.4 nozzle and tight on a 0.2.""" @@ -385,9 +444,60 @@ class FFPA: # ---------------- the sweep ---------------- - def _prologue(self, z_target): + def _chute(self, tool): + """The purge chute in G-code coordinates for TOOL. + + Follows [gcode_macro _FF_FILAMENT] when it is loaded so the chute has + one definition in the tree, and falls back to the app's own constants + when it is not. park_x/park_y in [ff_pa] override both.""" + ff = self.printer.lookup_object('gcode_macro _FF_FILAMENT', None) + x, y = PARK_X, PARK_Y + x_feed, y_feed = PARK_X_FEED, PARK_Y_FEED + approach_feed = PARK_APPROACH_FEED + if ff is not None: + status = ff.get_status(self.reactor.monotonic()) + x = status.get('purge_x', x) + y = status.get('purge_y', y) + x_feed = status.get('travel_x_feed', x_feed) + y_feed = status.get('travel_y_feed', y_feed) + approach_feed = status.get('approach_feed', approach_feed) + dx = status.get('tool_dx') or [] + dy = status.get('tool_dy') or [] + if tool < len(dx): + x += dx[tool] + if tool < len(dy): + y += dy[tool] + if self.park_x is not None: + x = self.park_x + if self.park_y is not None: + y = self.park_y + return x, y, int(x_feed), int(y_feed), int(approach_feed) + + def _park(self, tool): + """Put the nozzle over the chute, in the app's own three moves. + + This is the ONLY real XY motion in the whole procedure, and it has to + happen while the drivers still answer -- the latch that follows is + what makes the sweep itself go nowhere. Returns the parked position + for SET_KINEMATIC_POSITION to replay, or None when parking is off.""" + if not self.park: + return None + x, y, x_feed, y_feed, approach_feed = self._chute(tool) + # Same order as _FF_FILAMENT_PREP: stage clear of the chute on X, + # square up on Y, then come in on X at the slow approach feed. + self._run('G1 X%.3f F%d' % (x - self.park_approach_dx, x_feed)) + self._run('G1 Y%.3f F%d' % (y, y_feed)) + self._run('G1 X%.3f F%d' % (x, approach_feed)) + self._wait_moves() + return x, y + + def _prologue(self, tool, z_target): """The app's prologue, plus SAVE_GCODE_STATE and an explicit G90 -- - it leaks its modal state and we do not.""" + it leaks its modal state and we do not. + + ORDER MATTERS. Every real move -- the Z raise and the travel to the + chute -- happens BEFORE the driver latch. Once the latch is on, + nothing the toolhead is told to do reaches the machine.""" self._run('SAVE_GCODE_STATE NAME=_ff_pa') self._run('G90') self._run('G92 E0') @@ -395,15 +505,19 @@ class FFPA: self._run('SET_VELOCITY_LIMIT ACCEL=%.0f' % self.sweep_accel) self._run('SET_VELOCITY_LIMIT SQUARE_CORNER_VELOCITY=%.3f' % self.sweep_scv) - for pin_name, _pin in self.tmc_pins: - self._run('SET_PIN PIN=%s VALUE=1.00' % pin_name) - self._wait_moves() # Raise before travelling, never lower -- same rule as the purge in - # ff-filament.cfg. The XY travel is the first line's own G1. + # ff-filament.cfg. _toolhead, status = self._toolhead_status() if self.dry_run or status['position'].z < z_target: self._run('G1 Z%.3f F%d' % (z_target, self.line_z_feed)) self._wait_moves() + parked = self._park(tool) + if parked is not None: + self.parked_at = (parked[0], parked[1], z_target) + # The latch goes on last, and comes off first in _restore. + for pin_name, _pin in self.tmc_pins: + self._run('SET_PIN PIN=%s VALUE=1.00' % pin_name) + self._wait_moves() def _one_line(self, index, pa_text, extruder_name, verbose): """One candidate: arm, set PA, draw the line, disarm, read a verdict. @@ -530,11 +644,29 @@ class FFPA: except (FFPAError, self.printer.command_error): pass # 2. Driver pins back to what they were, not to the app's hardcoded - # 0.00. Left latched, whatever they gate stays energised. + # 0.00. THIS is the step that gives X and Y back: while the latch + # is on the carriage is dead, and nothing later in a print will + # lift it -- Klipper's own stepper enable is a different pin and + # knows nothing about this one. So a failure here is shouted + # about, not swallowed: it is the difference between a run that + # went wrong and a printer that cannot move until klippy restarts. + stuck = [] for pin_name, value in snapshot['pins']: try: self._run('SET_PIN PIN=%s VALUE=%.2f' % (pin_name, value)) except self.printer.command_error: + stuck.append(pin_name) + if stuck: + try: + self.gcode.respond_info( + "%s: !! COULD NOT UNLATCH %s. The X/Y drivers are still" + " disabled and will stay that way -- the carriage will" + " not move and homing will not work. Recover with" + " %s, or restart klippy." + % (self.name, ", ".join(stuck), + " ".join("SET_PIN PIN=%s VALUE=0.00" % n + for n in stuck))) + except Exception: pass # 3. Motion limits. The shipped values are 30000/9, so the sweep's # 5000 is a real reduction that must not leak into the next print. @@ -559,11 +691,26 @@ class FFPA: self._run('RESTORE_GCODE_STATE NAME=_ff_pa') except self.printer.command_error: pass - # The app's resetTmcPa also replays SET_KINEMATIC_POSITION. That is - # NOT ported and must not be: it needs it because its caller - # force-drove the axes, while our sweep is plain G1 under Klipper's - # own kinematics. Replaying it here would only lie to Klipper about - # where the carriage is. + # 5. LAST, and only now: tell Klipper where the carriage really is. + # It planned the whole ladder against dead drivers, so its idea of + # X and Y is the end of the ladder while the machine never left the + # chute -- and the axes are still flagged homed, which is what + # makes that dangerous rather than merely untidy. The app passes + # exactly this string into resetTmcPa (@0x79350c). + # + # After RESTORE_GCODE_STATE, not before. SET_KINEMATIC_POSITION + # fires toolhead:set_position, which has gcode_move recompute its + # last_position from base_position/homing_position -- so it has to + # see the RESTORED offsets, not the sweep's. + if self.parked_at is not None and not self.dry_run: + try: + self._wait_moves() + self._run('SET_KINEMATIC_POSITION X=%.3f Y=%.3f Z=%.3f' + % self.parked_at) + self._wait_moves() + except self.printer.command_error: + pass + self.parked_at = None def _heat(self, gcmd, tool, extruder): temp = gcmd.get_float('TEMP', None) @@ -578,17 +725,18 @@ class FFPA: cmd_FF_PA_CALIBRATE_help = ( "Measure pressure advance on the MOUNTED tool by sweeping candidates" - " through the eBoard's scorer. Extrudes ~1 g in free air onto the" - " plate and REPORTS a number -- it saves nothing and applies nothing" - " ([TOOL=] [TEMP=] [SWEEPS=] [WINNERS=] [CANDIDATES=] [PASS_VALUE=]" - " [Y_START=] [Y_STEP=] [Z=] [VERBOSE=1] [DRY_RUN=1])") + " through the eBoard's scorer. Parks over the purge chute, extrudes" + " ~1 g into it and REPORTS a number -- it saves nothing and applies" + " nothing ([TOOL=] [TEMP=] [SWEEPS=] [WINNERS=] [CANDIDATES=]" + " [PASS_VALUE=] [Y_START=] [Y_STEP=] [Z=] [PARK=0] [VERBOSE=1]" + " [DRY_RUN=1])") def cmd_FF_PA_CALIBRATE(self, gcmd): if self.running: raise gcmd.error("%s: a calibration is already running" % self.name) saved = (self.candidates, self.sweeps, self.winners, - self.pass_value, self.y_start, self.y_step) + self.pass_value, self.y_start, self.y_step, self.park) self.dry_run = bool(gcmd.get_int('DRY_RUN', 0, minval=0, maxval=1)) verbose = bool(gcmd.get_int('VERBOSE', 0, minval=0, maxval=1)) try: @@ -599,7 +747,7 @@ class FFPA: self.running = False self.dry_run = False (self.candidates, self.sweeps, self.winners, self.pass_value, - self.y_start, self.y_step) = saved + self.y_start, self.y_step, self.park) = saved def _apply_overrides(self, gcmd): raw = gcmd.get('CANDIDATES', None) @@ -621,6 +769,8 @@ class FFPA: self.pass_value = gcmd.get_int('PASS_VALUE', self.pass_value) self.y_start = gcmd.get_float('Y_START', self.y_start) self.y_step = gcmd.get_float('Y_STEP', self.y_step) + self.park = bool(gcmd.get_int('PARK', int(self.park), + minval=0, maxval=1)) if self.dry_run: # A dry run never collects a winner, so it would otherwise print # every sweep. One is what anyone wants to read. @@ -630,7 +780,7 @@ class FFPA: tool = self._resolve_tool(gcmd) self._check_homed(gcmd) self._check_filament(gcmd, tool) - self._check_bounds(gcmd) + self._check_bounds(gcmd, tool) self._ensure_extruder(tool) extruder = self._extruder() self._heat(gcmd, tool, extruder) @@ -642,10 +792,7 @@ class FFPA: lines = ["%s: T%d (%s), %d candidates x up to %d sweeps" % (self.name, tool, extruder.get_name(), len(self.candidates), self.sweeps), - " lines are extruded in FREE AIR at Z %.3f raw / Z %.3f" - " gcode. Nothing prints them and nothing inspects them --" - " expect ~1 g of loose filament on the plate." - % (raw_z, z_target)] + self._where_note(tool, raw_z, z_target)] note = self._cross_section_note(extruder) if note: lines.append(note) @@ -654,7 +801,7 @@ class FFPA: extruder_name = extruder.get_name() with self._guarded(extruder): try: - self._prologue(z_target) + self._prologue(tool, z_target) best, census, rows = self._sweep(gcmd, extruder_name, verbose) except FFPAError as err: raise gcmd.error("%s: %s" % (self.name, err)) @@ -721,7 +868,7 @@ class FFPA: "Draw ONE calibration line and print its eBoard verdict. The" " bring-up tool: run it across the candidates by hand and confirm" " you get more than one distinct answer before trusting an averaged" - " number ([PA=] [Y=] [TOOL=] [TEMP=] [Z=] [DRY_RUN=1])") + " number ([PA=] [Y=] [TOOL=] [TEMP=] [Z=] [PARK=0] [DRY_RUN=1])") def cmd_FF_PA_PROBE(self, gcmd): if self.running: @@ -743,30 +890,35 @@ class FFPA: raise gcmd.error("%s: PA='%s' is not a number" % (self.name, pa_text)) y = gcmd.get_float('Y', self.y_start) + saved_park = self.park + self.park = bool(gcmd.get_int('PARK', int(self.park), + minval=0, maxval=1)) tool = self._resolve_tool(gcmd) self._check_homed(gcmd) self._check_filament(gcmd, tool) - self._check_bounds(gcmd) + self._check_bounds(gcmd, tool) self._ensure_extruder(tool) extruder = self._extruder() self._heat(gcmd, tool, extruder) extruder = self._extruder() if not self.dry_run: self._check_can_extrude(gcmd, extruder) - _raw_z, z_target = self._line_z(gcmd, tool) + raw_z, z_target = self._line_z(gcmd, tool) extruder_name = extruder.get_name() + gcmd.respond_info(self._where_note(tool, raw_z, z_target)) # One line at the requested Y: index 0 with the ladder rebased. saved_y = self.y_start with self._guarded(extruder): try: self.y_start = y - self._prologue(z_target) + self._prologue(tool, z_target) verdict, unflushed = self._one_line(0, pa_text, extruder_name, True) except FFPAError as err: raise gcmd.error("%s: %s" % (self.name, err)) finally: self.y_start = saved_y + self.park = saved_park if self.dry_run: return extra = '' @@ -781,6 +933,18 @@ class FFPA: "Show the PA calibration config in force, whether the eBoard" " commands are bound, and the last run's table") + def _park_note(self): + """One line for FF_PA_STATUS. Deliberately not _where_note: that one + needs a tool and a Z, and STATUS must answer with neither.""" + if not self.park: + return "OFF -- the gram lands wherever the nozzle stands" + ff = self.printer.lookup_object('gcode_macro _FF_FILAMENT', None) + source = ("_FF_FILAMENT purge_x/purge_y + tool dx/dy" + if ff is not None else "built-in defaults") + if self.park_x is not None or self.park_y is not None: + source = "park_x/park_y override" + return "the purge chute, from %s" % source + def cmd_FF_PA_STATUS(self, gcmd): action = query = None bound = "no ([pa_adjust] not loaded)" @@ -799,6 +963,7 @@ class FFPA: " ladder: X%.1f..%.1f, Y%.1f step %.1f" % (self.x_start, self.segments[-1][0], self.y_start, self.y_step), + " park: %s" % self._park_note(), " eBoard commands bound: %s" % bound, " toolchanger: %s" % ("loaded" if self.toolchange is not None else "MISSING"), From 8cd284d3eeba89dc6ace9d6881fb49380aee25c0 Mon Sep 17 00:00:00 2001 From: Oleksandr Shyshatskyi Date: Mon, 31 Aug 2026 21:22:12 +0300 Subject: [PATCH 2/3] A driver latch found already on is debris, not state to preserve _guarded snapshots the pins on entry and _restore puts back what it found, which is right for every value except one. If a run ever ends with the latch still on, the next run snapshots 1.00, faithfully restores 1.00, and the drivers stay dead -- one aborted run turning into a printer that relatches itself on every subsequent run. Release the latch instead of reinstating it; only a non-latched value is worth restoring truthfully. Also correct an overstatement in the previous commit. A klippy or MCU shutdown does NOT leave the drivers latched: [output_pin] hands shutdown_value, which defaults to 0., to setup_start_value, so the MCU drives PH2/PH3 low by itself. The guard earns its keep on the ordinary failures -- a command_error, a cancel, an exception out of the sweep -- which unwind through _restore with the machine still running. The latched and released values are named rather than spelled 1.00 and 0.00 at four call sites, since the comparison in the snapshot now depends on them agreeing. --- docs/notes/51-pa-calibration-recovered.md | 14 ++++- pkgs/klipper-config/payload/config/ff-pa.cfg | 16 +++-- .../payload/klipper/klippy/extras/ff_pa.py | 61 ++++++++++++++----- 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/docs/notes/51-pa-calibration-recovered.md b/docs/notes/51-pa-calibration-recovered.md index f323a5a..202be26 100644 --- a/docs/notes/51-pa-calibration-recovered.md +++ b/docs/notes/51-pa-calibration-recovered.md @@ -390,9 +390,17 @@ state, and the last run's table. the axes are still flagged homed. An earlier revision of this port deliberately skipped it on the reasoning that "our sweep is plain G1 under Klipper's own kinematics" -- that reasoning was wrong, because the latch means our G1s move nothing either. -- **Failing to unlatch is reported, not swallowed.** Both `SET_PIN`s go back to their snapshot - value, and if that fails (a shutdown mid-run makes `run_script_from_command` raise) the command - prints the recovery line rather than leaving a printer that silently cannot move. +- **The latch is always released, and never faithfully reinstated.** Both `SET_PIN`s go back to + the value read on entry, with one exception: a pin found *already latched* is released instead. + Restoring that state truthfully is what would turn one aborted run into a printer that relatches + its own drivers on every subsequent run. A failure to release is printed with the recovery + command rather than left silent. + + Worth being exact about the shutdown case, because it is easy to overstate: `[output_pin]` + passes `shutdown_value` (which defaults to `0.`) to `setup_start_value`, so on a klippy or MCU + shutdown the MCU drives `PH2`/`PH3` low itself and the drivers come back without any help from + us. The guard matters for the ordinary failures -- a `command_error`, a cancel, an exception out + of the sweep -- which unwind through `_restore` with the machine still running. - **`SAVE_GCODE_STATE`/`RESTORE_GCODE_STATE`** wrap the run; the app leaks its modal state. - **`min()` on floats**, not the app's lexicographic sort of fixed-width strings. Identical for its own seven values, correct for an override with mixed decimal widths. diff --git a/pkgs/klipper-config/payload/config/ff-pa.cfg b/pkgs/klipper-config/payload/config/ff-pa.cfg index ef59ddd..f03b8d6 100644 --- a/pkgs/klipper-config/payload/config/ff-pa.cfg +++ b/pkgs/klipper-config/payload/config/ff-pa.cfg @@ -48,10 +48,13 @@ # dressed up as a measurement. See require_discrimination. # * cold extruder, empty tool and out-of-range geometry are refused before # anything moves, instead of failing mid-line with the eBoard armed. -# * a failure to unlatch the driver pins on exit is reported loudly instead -# of swallowed. That latch is the one piece of state that can leave the -# machine unable to move at all, and Klipper's own stepper enable is a -# different pin that knows nothing about it. +# * the driver latch is always released. It is put back to what it read on +# entry, EXCEPT when that was itself the latched value -- a latch left on +# by an earlier run is cleared rather than faithfully reinstated, which is +# what would otherwise make the condition permanent. A failure to release +# it is reported loudly instead of swallowed. (A klippy or MCU shutdown +# needs none of this: [output_pin]'s shutdown_value defaults to 0, so the +# MCU drives these pins low on its own.) # * the timing is NOT "improved". pa_action is an immediate MCU command and # there is deliberately no M400 between the last G1 and ACTION=0. The # closed scorer was tuned against that. Do not add barriers. @@ -113,6 +116,11 @@ # The X/Y driver latch the app holds for the duration, restored to whatever # it was. Empty disables the latch entirely. #tmc_pins: enable_pin_tmc_x, enable_pin_tmc_y +# What "latched" and "released" mean on those pins. The app writes 1.00 to +# arm and hardcodes 0.00 to release; a pin found sitting at tmc_latch_value +# when a run starts is treated as debris and released on exit, not preserved. +#tmc_latch_value: 1.0 +#tmc_released_value: 0.0 # Park over the purge chute before the drivers are latched, and replay the # parked coordinates through SET_KINEMATIC_POSITION afterwards -- Klipper # spends the sweep believing the carriage walked the ladder, and it never diff --git a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py index 01a0130..c533f35 100644 --- a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py +++ b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py @@ -97,6 +97,10 @@ PARK_X_FEED = 6000 PARK_Y_FEED = 24000 PARK_APPROACH_FEED = 6000 TMC_PINS = ('enable_pin_tmc_x', 'enable_pin_tmc_y') +# The latch, and the value that is NOT the latch. The app writes 1.00 to arm +# and hardcodes 0.00 to release; printer.base.cfg ships these pins at 0. +TMC_LATCH_VALUE = 1.0 +TMC_RELEASED_VALUE = 0.0 FILAMENT_AREA_175 = 2.405 # mm^2, for the cross-section warning @@ -178,6 +182,10 @@ class FFPA: self.tmc_pin_names = [n.strip() for n in config.getlist('tmc_pins', list(TMC_PINS)) if n.strip()] + self.tmc_latch_value = config.getfloat('tmc_latch_value', + TMC_LATCH_VALUE) + self.tmc_released_value = config.getfloat('tmc_released_value', + TMC_RELEASED_VALUE) self.action_start = config.getint('action_start', ACTION_START) self.action_stop = config.getint('action_stop', ACTION_STOP) self.action_pc = config.getint('action_pc', ACTION_PC) @@ -516,7 +524,8 @@ class FFPA: self.parked_at = (parked[0], parked[1], z_target) # The latch goes on last, and comes off first in _restore. for pin_name, _pin in self.tmc_pins: - self._run('SET_PIN PIN=%s VALUE=1.00' % pin_name) + self._run('SET_PIN PIN=%s VALUE=%.2f' + % (pin_name, self.tmc_latch_value)) self._wait_moves() def _one_line(self, index, pa_text, extruder_name, verbose): @@ -610,6 +619,22 @@ class FFPA: # ---------------- state ---------------- + def _pin_restore_value(self, pin, now): + """What this driver pin should read when the run is over. + + Normally whatever it read on entry -- we put back what we found. The + exception is finding it ALREADY LATCHED, which is not a state worth + preserving: it means an earlier run died without releasing it, and + faithfully restoring it would relatch the drivers on every run and + make the condition permanent. A latch we did not set is still a latch + we can clear, so clear it.""" + if pin is None: + return self.tmc_released_value + value = pin.get_status(now)['value'] + if abs(value - self.tmc_latch_value) < 1e-6: + return self.tmc_released_value + return value + @contextlib.contextmanager def _guarded(self, extruder): """Snapshot on entry, restore unconditionally on exit, every step @@ -625,7 +650,7 @@ class FFPA: 'pa': ext.get('pressure_advance', 0.), 'smooth': ext.get('smooth_time', 0.), 'name': extruder.get_name(), - 'pins': [(n, (p.get_status(now)['value'] if p is not None else 0.)) + 'pins': [(n, self._pin_restore_value(p, now)) for n, p in self.tmc_pins], } try: @@ -643,13 +668,18 @@ class FFPA: action_cmd.send([self.action_stop, self.action_pc]) except (FFPAError, self.printer.command_error): pass - # 2. Driver pins back to what they were, not to the app's hardcoded - # 0.00. THIS is the step that gives X and Y back: while the latch - # is on the carriage is dead, and nothing later in a print will - # lift it -- Klipper's own stepper enable is a different pin and - # knows nothing about this one. So a failure here is shouted - # about, not swallowed: it is the difference between a run that - # went wrong and a printer that cannot move until klippy restarts. + # 2. Driver pins back to what they were (see _pin_restore_value), + # not to the app's hardcoded 0.00. THIS is the step that gives X + # and Y back: while the latch is on the carriage is dead, and + # nothing later in a print will lift it -- Klipper's own stepper + # enable is a different pin and knows nothing about this one. + # + # A klippy or MCU shutdown is the one case that heals itself: + # [output_pin] hands shutdown_value (default 0) to + # setup_start_value, so the MCU drives these pins low on its own + # without needing us. Everything else -- a command_error, a + # cancel, an exception out of the sweep -- comes back through + # here, which is why a failure is reported rather than swallowed. stuck = [] for pin_name, value in snapshot['pins']: try: @@ -659,12 +689,15 @@ class FFPA: if stuck: try: self.gcode.respond_info( - "%s: !! COULD NOT UNLATCH %s. The X/Y drivers are still" - " disabled and will stay that way -- the carriage will" - " not move and homing will not work. Recover with" - " %s, or restart klippy." + "%s: !! COULD NOT UNLATCH %s. The X/Y drivers are" + " disabled and nothing else will lift them -- the" + " carriage will not move and homing will not work." + " Recover with %s. (A klippy or MCU shutdown would" + " clear them by itself via shutdown_value; this was" + " not one.)" % (self.name, ", ".join(stuck), - " ".join("SET_PIN PIN=%s VALUE=0.00" % n + " ".join("SET_PIN PIN=%s VALUE=%.2f" + % (n, self.tmc_released_value) for n in stuck))) except Exception: pass From 4e041a0897c3398438c7d51843ea02df02dd4e36 Mon Sep 17 00:00:00 2001 From: Oleksandr Shyshatskyi Date: Mon, 31 Aug 2026 23:25:28 +0300 Subject: [PATCH 3/3] A failed pre-flight check must not leak the PARK= override _probe wrote the PARK= argument into self.park before running the pre-flight checks, but the restore sat in the finally inside _guarded -- so a probe refused for a cold extruder or unhomed axes kept the override. FF_PA_PROBE PARK=0 followed by any refusal would silently turn parking off for every later FF_PA_CALIBRATE, landing the gram on the plate instead of down the chute. The override now goes on inside its own try/finally that covers the checks too, with the rest of the probe split into _probe_inner. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UBzgZQiUiARJcG1NupWgY2 --- .../payload/klipper/klippy/extras/ff_pa.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py index c533f35..7b42a9b 100644 --- a/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py +++ b/pkgs/klipper/payload/klipper/klippy/extras/ff_pa.py @@ -923,9 +923,19 @@ class FFPA: raise gcmd.error("%s: PA='%s' is not a number" % (self.name, pa_text)) y = gcmd.get_float('Y', self.y_start) + # PARK= is a per-invocation override of a persistent default, so it + # must be undone on EVERY exit -- including a refusal from the checks + # below, which run before _guarded gets a finally in place. Leaking it + # would flip what the next FF_PA_CALIBRATE does with its gram. saved_park = self.park - self.park = bool(gcmd.get_int('PARK', int(self.park), - minval=0, maxval=1)) + try: + self.park = bool(gcmd.get_int('PARK', int(self.park), + minval=0, maxval=1)) + self._probe_inner(gcmd, y, pa_text) + finally: + self.park = saved_park + + def _probe_inner(self, gcmd, y, pa_text): tool = self._resolve_tool(gcmd) self._check_homed(gcmd) self._check_filament(gcmd, tool) @@ -951,7 +961,6 @@ class FFPA: raise gcmd.error("%s: %s" % (self.name, err)) finally: self.y_start = saved_y - self.park = saved_park if self.dry_run: return extra = ''