Fix probe clearance calculation (#17242)

[jpflouret]

Description

This pull request fixes the clearance calculation for z probing with zero probe offset which often causes the probe to fail. The calculation of required clearance of half the configured distance requres a probe to be mounted at least twice as far as the clearance between probe distance. Additionally, the calculation is done in integer math which also requires the probe to be mounted even slightly higher than twice the clearance.

Benefits

Increases the range of allowable z probe mounting distances for the z probe. Also allows probing with zero offset to find the initial settings for z offset.

Related Issues

Fixes #17242

[fiveangle]

Finally fixes #7902

[thinkyhead]

As I recall, the purpose of the check (added by @InsanityAutomation after much discussion) was to catch instances where the probe is stuck in a triggered state. When this occurs, the carriage simply rises between probe points and never moves down.

I'm not sure if it is merely the size of the clearance value, or whether the test is built around false assumptions. I would like to ensure that the sanity check does what it is intended to do, while still allowing for you to do probing with a completely incorrect Z probe offset when you need to.

So, please test this change by triggering the probe with your finger before it has a chance to do each point, and see if it catches the problem and throws the correct error.

[InsanityAutomation]

My original change was set for the maximum z offset, so even this PR is slightly more restrictive than my original check.

[thinkyhead]

was set for the maximum z offset

I assume you mean Z_PROBE_OFFSET_RANGE_MAX.
Of course that could be set to anything, and it is a very loose correlation, if it is one at all.

[InsanityAutomation]

Just the typically largest of all possibly involved values, started at the widest possible limit from none to limit false issues and let it get more constricted from there till the right balance is found.

[thinkyhead]

The issue it is meant to mitigate is the false triggering at a strange height. This indirect way of sensing the situation is insufficient to deal with the main issue. I suppose we could just use a fixed value like 50 and allow probing to raise and raise until it hits that threshold.

Or, remove the sanity check altogether, and simply look for a probe that is triggered before the probe moves towards the bed.

[jpflouret]

I can confirm that if you trigger the sensor before reaching the requested clearance height then the probing fails.

Log Output

    Send: M111 S32
    Recv: echo:DEBUG:LEVELING
    Recv: ok
    Send: G29
    Recv:   current_position= X-5.00 Y-5.00 Z0.00 : >>> G29
    Recv: Machine Type: Cartesian
    Recv: Probe: BLTOUCH
    Recv: Probe Offset X-25.00 Y-30.00 Z0.00 (Left-Front & Same Z as Nozzle)
    Recv: Auto Bed Leveling: BILINEAR (disabled)
    Recv: 
    Recv:   current_position= X-5.00 Y-5.00 Z0.00 : Probe::set_deployed
    Recv: deploy: 1
    Recv: Probe::move_z(10.00)
    Recv: >>> do_blocking_move_to X-5.00 Y-5.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X-5.00 Y-5.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: reset_bed_level
    Recv: >>> Probe::probe_at_point(10.00, 10.00, raise, 0, probe_relative)
    Recv:   current_position= X-5.00 Y-5.00 Z10.00 :
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv:   current_position= X35.00 Y40.00 Z10.00 : Probe::set_deployed
    Recv: deploy: 1
    Recv:   current_position= X35.00 Y40.00 Z10.00 : >>> Probe::run_z_probe
    Recv:   current_position= X35.00 Y40.00 Z10.00 : >>> Probe::probe_down_to_z
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z-2.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: BLTouch STOW requested
    Recv: BLTouch Command :90
    Recv: bltouch.stow_proc() end
    Recv:   current_position= X35.00 Y40.00 Z7.14 : sync_plan_position
    Recv:   current_position= X35.00 Y40.00 Z7.14 : <<< Probe::probe_down_to_z
    Recv: SLOW Probe fail! - Triggered early.
    Recv:   current_position= X35.00 Y40.00 Z7.14 : <<< run_z_probe
    Recv:   current_position= X35.00 Y40.00 Z7.14 : Probe::set_deployed
    Recv: deploy: 0
    Recv: Probe::move_z(10.00)
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: BLTouch STOW requested
    Recv: BLTouch Command :90
    Recv: bltouch.stow_proc() end
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: Error:Probing Failed

In the avove log, you can see that I triggered the probe at ~7mm by pushing it with my finger. 7mm is above the travel height between probes so it fails. This seems reasonable. If you trigger it within the travel height it succeeds as expected:

Log Output

    Send: G28
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: X:-5.00 Y:-5.00 Z:0.00 E:0.00 Count X:-400 Y:-400 Z:0
    Recv: ok
    Send: M111 S32
    Recv: echo:DEBUG:LEVELING
    Recv: ok
    Send: M851 Z0
    Recv: ok
    Send: G29
    Recv:   current_position= X-5.00 Y-5.00 Z0.00 : >>> G29
    Recv: Machine Type: Cartesian
    Recv: Probe: BLTOUCH
    Recv: Probe Offset X-25.00 Y-30.00 Z0.00 (Left-Front & Same Z as Nozzle)
    Recv: Auto Bed Leveling: BILINEAR (disabled)
    Recv: 
    Recv:   current_position= X-5.00 Y-5.00 Z0.00 : Probe::set_deployed
    Recv: deploy: 1
    Recv: Probe::move_z(10.00)
    Recv: >>> do_blocking_move_to X-5.00 Y-5.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X-5.00 Y-5.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: reset_bed_level
    Recv: >>> Probe::probe_at_point(10.00, 10.00, raise, 0, probe_relative)
    Recv:   current_position= X-5.00 Y-5.00 Z10.00 :
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z10.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv:   current_position= X35.00 Y40.00 Z10.00 : Probe::set_deployed
    Recv: deploy: 1
    Recv:   current_position= X35.00 Y40.00 Z10.00 : >>> Probe::run_z_probe
    Recv:   current_position= X35.00 Y40.00 Z10.00 : >>> Probe::probe_down_to_z
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z-2.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: BLTouch STOW requested
    Recv: BLTouch Command :90
    Recv: bltouch.stow_proc() end
    Recv:   current_position= X35.00 Y40.00 Z1.98 : sync_plan_position
    Recv:   current_position= X35.00 Y40.00 Z1.98 : <<< Probe::probe_down_to_z
    Recv:   current_position= X35.00 Y40.00 Z1.98 : <<< run_z_probe
    Recv: >>> do_blocking_move_to X35.00 Y40.00 Z6.98
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: <<< Probe::probe_at_point
    Recv: >>> Probe::probe_at_point(53.75, 10.00, raise, 0, probe_relative)
    Recv:   current_position= X35.00 Y40.00 Z6.98 :
    Recv: >>> do_blocking_move_to X78.75 Y40.00 Z6.98
    Recv: <<< do_blocking_move_to
    Recv:   current_position= X78.75 Y40.00 Z6.98 : Probe::set_deployed
    Recv: deploy: 1
    Recv:   current_position= X78.75 Y40.00 Z6.98 : >>> Probe::run_z_probe
    Recv:   current_position= X78.75 Y40.00 Z6.98 : >>> Probe::probe_down_to_z
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X78.75 Y40.00 Z-2.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: BLTouch STOW requested
    Recv: BLTouch Command :90
    Recv: bltouch.stow_proc() end
    Recv:   current_position= X78.75 Y40.00 Z3.90 : sync_plan_position
    Recv:   current_position= X78.75 Y40.00 Z3.90 : <<< Probe::probe_down_to_z
    Recv:   current_position= X78.75 Y40.00 Z3.90 : <<< run_z_probe
    Recv: >>> do_blocking_move_to X78.75 Y40.00 Z8.90
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: <<< Probe::probe_at_point
    Recv: >>> Probe::probe_at_point(97.50, 10.00, raise, 0, probe_relative)
    Recv:   current_position= X78.75 Y40.00 Z8.90 :
    Recv: >>> do_blocking_move_to X122.50 Y40.00 Z8.90
    Recv: <<< do_blocking_move_to
    Recv:   current_position= X122.50 Y40.00 Z8.90 : Probe::set_deployed
    Recv: deploy: 1
    Recv:   current_position= X122.50 Y40.00 Z8.90 : >>> Probe::run_z_probe
    Recv:   current_position= X122.50 Y40.00 Z8.90 : >>> Probe::probe_down_to_z
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X122.50 Y40.00 Z-2.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: echo:busy: processing
    Recv: BLTouch STOW requested
    Recv: BLTouch Command :90
    Recv: bltouch.stow_proc() end
    Recv:   current_position= X122.50 Y40.00 Z2.17 : sync_plan_position
    Recv:   current_position= X122.50 Y40.00 Z2.17 : <<< Probe::probe_down_to_z
    Recv:   current_position= X122.50 Y40.00 Z2.17 : <<< run_z_probe
    Recv: >>> do_blocking_move_to X122.50 Y40.00 Z7.17
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing
    Recv: <<< Probe::probe_at_point
    Recv: >>> Probe::probe_at_point(141.25, 10.00, raise, 0, probe_relative)
    Recv:   current_position= X122.50 Y40.00 Z7.17 :
    Recv: >>> do_blocking_move_to X166.25 Y40.00 Z7.17
    Recv: <<< do_blocking_move_to
    Recv:   current_position= X166.25 Y40.00 Z7.17 : Probe::set_deployed
    Recv: deploy: 1
    Recv:   current_position= X166.25 Y40.00 Z7.17 : >>> Probe::run_z_probe
    Recv:   current_position= X166.25 Y40.00 Z7.17 : >>> Probe::probe_down_to_z
    Recv: BLTouch DEPLOY requested
    Recv: BLTouch Command :10
    Recv: bltouch.deploy_proc() end
    Recv: >>> do_blocking_move_to X166.25 Y40.00 Z-2.00
    Recv: <<< do_blocking_move_to
    Recv: echo:busy: processing

You can see above that I triggered my probe at 3.9mm by inserting a 2mm spacer underneath the probe and the probe suceeded. This probe would have completely failed before.

[InsanityAutomation]

The issue with making it too tight came up on Delta's where the distances we're much greater. Unfortunately I don't currently have one to work with.

[jpflouret]

I can remove the sanity check if that is what you prefer. I don't see the harm in having it if it detects a potential error but I do think there is a problem if we detect a false positive which is what I'm trying to fix. The false positive here is triggering a failed probe at less than half the desired travel height between probes. Obviously, if it triggers before the desired travel height there will probably be an obstruction so this sanity check makes sure we don't hit anything in the way. Or, at least, that is what I understand the intent of the sanity check to be.

[InsanityAutomation]

Personally I think this is a sane check. For probes like the bltouch that only emit a pulse unless faulted this is likely to still be the best spot for the catch. A limit of 2mm can be a bit tight at times and removing the hard limit and allowing it to base off a configurable parameter makes for sense to me.

[thinkyhead]

It might be sane under sane conditions. The thing is, if people are using wonky Z probe offsets, as the OP, then that throws a wrench into the works. It must be able to deal with that kind of unknown to accommodate experimentation.

[jpflouret]

Well, I wouldn't say zero is a wonky offset, I guess I am biased. Zero initial Z offset seems to be oftern recommended for finding the actual offset of a probe after installation.

The way I see it, the sane clearance sanity check for the probe should be Z_CLEARANCE_BETWEEN_PROBES for the fast probe and Z_CLEARANCE_MULTI_PROBE for the slow probe. If the probe triggers between [Z_PROBE_LOW_POINT, Z_CLEARANCE_MULTI_PROBE) that should be OK. Unless I completely misunderstand how this works, this range should be the wroking range of Z probe. Triggering above that would indicate that the move between probes would be obstructed, but I don't see how triggering in between those thresholds would cause an issue.

This range allows for sufficient clearance for successive probes while still checking that the probe is not triggering early. Like I said originally, the current implementation of the sanity check requires that the probe trigger below _MAX(Z_CLEARANCE_MULTI_PROBE, 4) / 2 which rounds down to 2 in integer math with the default settings. That seems excesively restrictive and I don't see why triggering above that but below the Z_CLEARANCE_MULTI_PROBE would be an issue.

[thinkyhead]

Zero initial Z offset…

If both homing and probing are done with the probe having the same Z probe offset, there should be no instances where the probe triggers at some unexpected height, unless there is a problem.

But if someone is using an endstop to home, and then the probe is triggering according to an incorrect offset, that can throw things off and we can't really trust the test.

[InsanityAutomation]

Basically this would only trigger a false positive if not following the documented procedure of home, set z offset, then run g29 to build the mesh. Advanced users may know that you can shift the mesh by the new offset but it's not really documented standard procedure. So while I understand where @thinkyhead is coming from I also don't see the harm in loosening it up a bit more to allow the alternative process. The intent here was to prevent gross errors (in testing the report I let it ram the top of a 700mm gantry to see how far it would go!) not necessarily minor ones.

[jpflouret]

So, what did we conclude from this? Do I need to do something else?

[thinkyhead]

It's all good. Looser is better since it reduces the likelihood of false positives, and the clearance value is a good choice. I'll do more testing while I'm debugging leveling in-general and see how different parameters affect behavior.