search

jlas1 / Klicky-Probe

Microswitch probe with magnetic attachement, primarily aimed at CoreXY 3d printers
GNU General Public License v3.0
1.27k stars 282 forks source link

Fix an issue where toolhead can crash on repeat homing because safe_z is being blindly overwritten with current z position #262

Open jmeier5261 opened 5 months ago

jmeier5261 commented 5 months ago

Address an issue with blindly assuming that if Z is already homed then it is above safe_z which was overwriting the value of safe_z to the current Z position when re-homing if Z was already homed, regardless of whether the current position was actually above the safe_z. Additionally included small typo fixes for logs.

Use case here is that because the machine is in a homed state, if my print fails or ends or any other action leaves my toolhead below specified safe_z height, the machine will not be returned to the actual safe_z when re homing because the code added in ac98aefb41dad7a62af7ded3610c21b70bca1efb will overwrite the user variable safe_z with whatever the current Z position is. Ideally we would want to do this only if the current Z is actually above configured safe_z

jmeier5261 commented 5 months ago

I want to mention that I am unsure if the change which was added that caused this is actually necessary...it seems that at every point after this, safe_z is only considered if current z position is below it so I'm not sure what the point of setting safe_z to the value of current Z. It may be better just to remove the block itself? I didn't test that approach sufficiently to say. I did notice in the recent fix to that statement https://github.com/jlas1/Klicky-Probe/pull/244, the comment on the PR doesn't match the contents of the line itself: so not sure if this was intentional at all.

jlas1 commented 3 months ago

Thanks for this, let me check it out

islasa1 commented 3 months ago

Thank you so much @jmeier5261 !

I thought I was going crazy, searching online for what might be causing it. Looking at the logic it makes no sense, but as this was my first time setting up Klipper / Klicky I wasn't sure if I just had a setting wrong. It seems like the original intent was to avoid having to move the Z axis if it can be safely assumed to be below the safe_z height.

The if statement added here would do the above. Just tested my setup with the changes as well, and no more crashing into the Z endstop! 😬