jlas1 / Klicky-Probe

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

Lasted update to Klicky macros causes an edge case where the toolhead can crash #263

Open jmeier5261 opened 3 months ago

jmeier5261 commented 3 months ago

In the latest changes to klicky macros (https://github.com/jlas1/Klicky-Probe/commit/ac98aefb41dad7a62af7ded3610c21b70bca1efb#diff-1d362a82153582bae3cbefb9848fdaf09f1b0aa250e3998dc6ef1e65bf38f63fR653) There was code added here: https://github.com/jlas1/Klicky-Probe/blob/c3366bb83294e6d1ec908a14d25dc9b091fde428/Klipper_macros/klicky-macros.cfg#L654 which is assigning the current Z position to overwrite the configured safe_z if the machine is already in a homed state without any consideration for whether the current z position is safe or not. This causes the toolhead to potentially crash if the user attempts to home the machine again when it is already homed but is currently at a Z height below safe_z such as after a small print or a failed print.

I have opened a PR https://github.com/jlas1/Klicky-Probe/pull/262 to only do this if the current Z position is greater than the configured safe_z however I am unsure if this is the correct approach or not since I am unsure if this line is needed at all given that in every other place i could find references to safe_z it is only used if current Z is below it.

PigeonGiraffe commented 3 months ago

Just ran into the same issue here. When XYZ have not been homed it will do a safe z move and continue homing. However, after it has been homed, and you move the tool head below the safe_z height, it will not prevent a collision. I believe this collision only happens then your dock is in the homing path of the toolhead. I could move my dock out of the homing path, but instead I change line 654 to what @jmeier5261 is suggesting. Tested below safe_z height, moves bed down to prevent collision. Tested above safe_z height, bed did not move. Works great.