klipper
klipper copied to clipboard
Per-tool Z probe.
Hey, Klipper devs. This mod got a bit involved and maybe there is a chance to get this upstreamed.
I have a Toolchanger setup where each toolhead has it's own bed probe and there is no independant bed probe. So this adds a new virtual probe Z endstop that can be reassigned between multiple probes.
The code here is very much an initial concept, happy to hear your feedback and make changes as needed.
thank you for submitting a PR, please refer to https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md paying particular attention to the signed-off-by line.
Thanks James
Sure, will do that. And documentation and tests, etc.
In the meantime if you can comment on:
- is this kind of function reasonable to include to Klipper proper. Or too esoteric and better to stay as external patch.
- is the overall technical approach reasonable. I primary focused on keeping the changes self contained. Refactoring the Probe file would probably help with removing remaining duplication.
On Mon, Apr 24, 2023, 09:51 JamesH1978 @.***> wrote:
thank you for submitting a PR, please refer to https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md paying particular attention to the signed-off-by line.
Thanks James
— Reply to this email directly, view it on GitHub https://github.com/Klipper3d/klipper/pull/6179#issuecomment-1519561711, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMQT47CEFW5TYUOAZYHMWTXCYWJTANCNFSM6AAAAAAXIWNFXM . You are receiving this because you authored the thread.Message ID: @.***>
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:
- 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.
- 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.
- 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.
Thanks. As high-level feedback, I'm not really sure what this code does. I don't have this hardware, so it's hard for me to review or give feedback. Perhaps you could explain what goes wrong with the current code, which users will make use of this new functionality, and how they would go about using it.
Separately, this code seems to be making changes to stepper.py which seems odd for probing code.
Also, note (as James indicated above) that new contributions would need a signed-off-by line.
Cheers, -Kevin
Hey, Updated the pull request with a better description. Including the meaning of stepper.py changes. I agree that this is pretty niche feature, hence I'm looking for early feedback if this makes sense in broader Klipper environment.
The long story: Toolchanger printers have multiple tools and a automated tool changing sequence to remove a tool and install another during print.
Generally some hotend equipend is shared and remains on the shuttle while the rest is docked and replaced with the other tool. In my case, the shuttle has no equipment at all, which removes the need to run any wiring to it. See this video
Which means there is no dedicated Z endstop on the toolhead anymore. But each hotend has it's own nozzle operated bed probe, which turns out has quite a few advantages:
- automated tool offset adjustments after nozzle changes by comparing Z probe height
- use the probe as docking switch, allowing to auto-detect which hotends are docked and which one is mounted.
But leaves with a Z homing problem, with few options:
- use a dedicated Z enstop - need to add an extra endstop that is not on the gantry, tricky for self bed leveling setups, like Voron Trident or 2.4
- designate one tool to be the Z endstop, which means you need to swap into that tool before you can home. Which might be impossible if Z homing is required to change tools.
- this patch dynamically assign a probe as the Z endstop
You configure it with
[tool_probe_endstop]
# registers virtual endstop pin
[tool_probe tool0]
tool: 0
z_offset: -1.68
# regular probe params...
[tool_probe tool1]
tool: 1
z_offset: -1.68
# regular probe params...
And control with
SET_ACTIVE_TOOL_PROBE T=0
or autodetect active tool with
DETECT_ACTIVE_TOOL_PROBE
and access active tool with
printer.tool_probe_endstop.active_tool_number
This is all very preliminary.
I can see 3 ways forward:
- incorporate this patch in full, Yay!
- incorporate only the stepper.py changes, allowing the rest to exist as an Klipper extension script
- keep it separate, which is quite cumbersome to maintain.
Cheers!
Okay, thanks for the description. As high-level feedback I don't think the approach taken in this PR is ideal.
I think we want to keep the stepper code related just to movement and I don't think it should be changed to facilitate different types of probes. The PrinterRail position_endstop
is intended to be a position on the physical rail that a carriage can not be moved past - that position on the rail does not change with different tools.
For what it is worth, I think enabling/disabling different rails (see the dual_carriage code for an example) or selecting between multiple probes may be something to look at as an alternative.
-Kevin
Thanks for the input, I will see what I can do. Can def play with non kinematic moves...
One nit - isn't the min/max positions meant to be the physical limits?
On Thu, May 25, 2023, 17:53 KevinOConnor @.***> wrote:
Okay, thanks for the description. As high-level feedback I don't think the approach taken in this PR is ideal.
I think we want to keep the stepper code related just to movement and I don't think it should be changed to facilitate different types of probes. The PrinterRail position_endstop is intended to be a position on the physical rail that a carriage can not be moved past - that position on the rail does not change with different tools.
For what it is worth, I think enabling/disabling different rails (see the dual_carriage code for an example) or selecting between multiple probes may be something to look at as an alternative.
-Kevin
— Reply to this email directly, view it on GitHub https://github.com/Klipper3d/klipper/pull/6179#issuecomment-1563140749, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMQT47PJTEFWEEZVZU2VKLXH556HANCNFSM6AAAAAAXIWNFXM . You are receiving this because you authored the thread.Message ID: @.***>
One nit - isn't the min/max positions meant to be the physical limits?
Sorry, yes you are correct - the position_min/max are the maximum travel for the rail. I guess I better understand the issue you are having (the tool position when hitting the endstop is different with different tools). However, I'm still not sure it is a good idea to modify stepper.py. Alternatives may be to swap rails for different tools (eg, dual_carriage) or to apply a gcode offset with each new tool.
-Kevin
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.