Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

z_endstop_set_calibration: Calibrate endstop based on current position #4670

Closed

Conversation

mhier
Copy link
Contributor

@mhier mhier commented Sep 7, 2021

Add new module which allows to calibrate the z-axis endstop position
based on the current position. This can be used with load cell-based
or accelerometer-based probes, as discussed in #3496.

Add new module which allows to calibrate the z-axis endstop position
based on the current position. This can be used with load cell-based
or accelerometer-based probes, as discussed in Klipper3d#3496.

Signed-off-by: Martin Hierholzer <hier@beta-centauri.de>
@KevinOConnor
Copy link
Collaborator

Thanks. What functionality does this provide over the new Z_OFFSET_APPLY_ENDSTOP command? That is, could a user interested in this functionality implement it via a macro and Z_OFFSET_APPLY_ENDSTOP?

-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Sep 15, 2021
@mhier
Copy link
Contributor Author

mhier commented Sep 16, 2021

Honestly, I wasn't aware of the new Z_OFFSET_APPLY_ENDSTOP (was likely introduced after I wrote this code - it just took me too long to create this PR, sorry... :-)). It looks like both are doing pretty much the same. What worries me a bit is that Z_OFFSET_APPLY_ENDSTOP is part of the manual probe module - while I need to use it with the load cell based probe.

Can I use this without the manual_probe? Or do I need to move it into a separate module for this?

@mhier mhier marked this pull request as draft September 16, 2021 07:19
@KevinOConnor
Copy link
Collaborator

The manual_probe module is automatically loaded - its commands are always available.

-Kevin

@mhier
Copy link
Contributor Author

mhier commented Sep 16, 2021

The manual_probe module is automatically loaded - its commands are always available.

-Kevin

I see now one difference: Z_OFFSET_APPLY_ENDSTOP uses the "gcode homing", while Z_ENDSTOP_SET_CALIBRATION uses the current position. I do not see how I can set the gcode homing based on the current position. G92 does not change the gcode homing, but the gcode base. SET_GCODE_OFFSET sets the gcode homing, but not based on the current position.

I tried setting the homing_origin from a gcode macro, like this:

[gcode_macro SET_GCODE_Z_ORIGIN_FROM_CURRENT_POSITION]
gcode:
    {% set printer.gcode_move.homing_origin.z = printer.toolhead.position.z %}

but this results in a syntax error... Is there any other way to achieve this without code change?

If not, I see two choices:

  • extend SET_GCODE_OFFSET command such that it can optionally work relative to the current position, e.g. by introducing X_RELATIVE, Y_RELATIVE, Z_RELATIVE parameters, or
  • keep the pull request as is.

My feeling is, the first approach (extend SET_GCODE_OFFSET) seems more elegant. What do you think?

@KevinOConnor
Copy link
Collaborator

Well, you don't want to use the current position though, because with the latest code it is possible to overshoot slightly during a probe attempt. Best is to extract the probe position as reported by {printer.probe.last_z_result}.

Setting the Z offset should be possible with something like: SET_GCODE_OFFSET Z={printer.probe.last_z_result} This is totally untested though - it might need adjustment based on the current probe Z offset.

-Kevin

@mhier
Copy link
Contributor Author

mhier commented Sep 17, 2021

Ok I understand. My probe algorithm currently moves to the determined Z=0 position to avoid this kind of overshoot. This is useful, as in some situations one can then use G92 or SET_KINEMATIC_POSITION without referring to any variable.

Now it should also set last_z_result, then this approach can work. I will test this this evening, and then likely close this PR.

(The load-cell probe does not have any offsets, so this shall work without issues.)

@mhier
Copy link
Contributor Author

mhier commented Sep 17, 2021

Ok, it works, but not as easy as I thought:

  • I do not know how to alter {printer.probe.last_z_result}, so I introduced my own variable which I can access via {printer["load_cell_probe"].last_z_result}.
  • Since the macro is parsed completely before executing, I have to introduce a helper macro "z_offset_from_probe_result" to set the z offset after running the probe in the same macro.

So my macro looks like this now:

[gcode_macro z_offset_from_probe_result]
gcode:
    SET_GCODE_OFFSET Z={printer["load_cell_probe"].last_z_result}

[gcode_macro bed_calibrate]
gcode:
    G0 Z10
    G0 X100 Y100
    G0 Z2
    PROBE
    Z_OFFSET_FROM_PROBE_RESULT
    Z_OFFSET_APPLY_ENDSTOP
    GET_POSITION
    SET_KINEMATIC_POSITION Z=0
    GET_POSITION
    G0 Z2
    BED_MESH_CALIBRATE
    SAVE_CONFIG

I think this is acceptable, hence I close this PR (and prepare the next one ;-)).

@mhier mhier closed this Sep 17, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending feedback Topic is pending feedback from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants