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

remove usage of homing_positive_dir #290

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Conversation

Gethe
Copy link
Contributor

@Gethe Gethe commented Sep 9, 2023

As stated in Klipper docs: "It is better to use the default than to specify this parameter. The default is true if position_endstop is near position_max and false if near position_min."

Including this parameter seems to be at best redundant and at worst potentially damaging for printers that home at axis minimum.

As stated in Klipper docs: "It is better to use the default than to
specify this parameter. The default is true if `position_endstop` is
near `position_max` and false if near `position_min`."
Copy link
Owner

@Frix-x Frix-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR.

I agree that hardcoding the homing dir is not necessary anymore today. If I remember correctly, it was a mandatory parameter a couple of year ago but now that there is an automatic direction extrapolation from the endstop position, I agree that it could lead to dangerous behaviors. Let's remove this :)

However, as stated in the next comment, using the computed value is still necessary for the Zhop after homing the Z axis. I'm using the "settings" value and not the "config" value. So this should work even if there is nothing specified in the config.

macros/base/homing/homing_override.cfg Outdated Show resolved Hide resolved
@Frix-x Frix-x added the enhancement New feature or request label Sep 10, 2023
@Frix-x
Copy link
Owner

Frix-x commented Sep 12, 2023

Yeah the check is not necessary as this parameter is populated automatically by Klipper and it's this value that we retrieve (not the one from the config).

Thanks, I'll merge for the next version :)

@Frix-x Frix-x merged commit 2133a2d into Frix-x:main Sep 12, 2023
@Gethe Gethe deleted the homing_positive_dir branch October 22, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants