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

safe_z_home: Add hooks to safe_z_home #5947

Closed
wants to merge 1 commit into from

Conversation

ibash
Copy link

@ibash ibash commented Dec 19, 2022

This adds hooks to safe_z_home which allow running custom gcode before/after each axis is homed.

I need to move the toolhead a little after homing x to prevent a crash. I also have a euclid probe, which means I need to deploy/retract the probe. Rather than recreating the z-hop logic in gcode (which isn't totally possible anyway, there's no way to mark z as not homed in gcode). I found it to be simpler to add hooks in safe_z_home.

This allows me a nice short config and is safer than homing override. e.g.

[safe_z_home]
home_xy_position: 150,150
speed: 25
z_hop: 15
z_hop_speed: 5
after_x:
  # move z a little to prevent crashing into a magnum pulley
  G1 X20 F3600
before_z:
  M401
after_z:
  M402

Signed-off-by: Islam Sharabash islam.sharabash@gmail.com

This adds hooks to safe_z_home which allow running custom gcode
before/after each axis is homed.

I need to move the toolhead a little after homing x to prevent a crash.
I also have a euclid probe, which means I need to deploy/retract the
probe. Rather than recreating the z-hop logic in gcode (which isn't
totally possible anyway, there's no way to mark z as not homed in
gcode). I found it to be simpler to add hooks in safe_z_home.

This allows me a nice short config and is safer than homing override.
e.g.

```
[safe_z_home]
home_xy_position: 150,150
speed: 25
z_hop: 15
z_hop_speed: 5
after_x:
  # move z a little to prevent crashing into a magnum pulley
  G1 X20 F3600
before_z:
  M401
after_z:
  M402
```

Signed-off-by: Islam Sharabash <islam.sharabash@gmail.com>
@KevinOConnor
Copy link
Collaborator

Thanks. However, safe_z_home was intended as a simplified version of homing_override. If you need the more complicated version, then I think it would make sense to use homing_override instead. I fear adding more complexity to safe_z_home would remove it's main advantage - it's easier to configure.

-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Dec 31, 2022
@ibash
Copy link
Author

ibash commented Jan 2, 2023

I can understand that, however I think it's becoming more common do things like deploy a probe before/after z.

Reproducing all the functionality safe_z_home does in homing_override would require some code changes to klipper to let gcode call note_z_not_homed. Even then, it's a huge PIA to implement since you need to parse out the requested axis and handle them separately. I know this because I tried implementing this in homing_override first, then made a code change to expose additional functionality to homing_override.

Adding hooks into safe_z_home was the most elegant and easy version.

@KevinOConnor KevinOConnor removed the pending feedback Topic is pending feedback from submitter label Jan 4, 2023
@KevinOConnor
Copy link
Collaborator

I understand. However, I don't currently think this is a good candidate for the Klipper master branch. There are already several ways to customize the homing process (eg, homing_override, overriding the G28 macro, using macros to home and avoiding G28 entirely). I'm concerned that adding yet another way to customize homing increases complexity.

-Kevin

@ibash
Copy link
Author

ibash commented Jan 4, 2023

Definitely valid concern, and I really do like that klipper keeps things simple. I'm happy to maintain my fork since I find it useful.

That said I think this change would allow the voron switchwire, legacy, v1, and v2 to all use safe_z_home instead of homing_override. Would need input from that community since I don't have a voron.

Potentially can simplify this monster of a homing_override: https://github.com/jlas1/Klicky-Probe/blob/2bfb9bff0f7248ce583f53de4798b05bfb2be10c/Klipper_macros/klicky-macros.cfg#L566

Maybe cc @jlas1 to get an opinion?

@github-actions
Copy link

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

  1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review
    If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
  2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
  3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Unfortunately a reviewer has not assigned themselves to this GitHub Pull Request and it is therefore being closed. It is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@github-actions github-actions bot closed this Feb 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants