Fix G34 probing range/error bug

[jufimu12]

Requirements

-Multiple Z axis
-Z_STEPPER_AUTO_ALIGN

Description

Fixed a bug where G34 (Z stepper auto alignment) fails when the build plate is tilted more than a few millimetres.

The bug was caused by one of two things:

• If the corner of the build plate is too high (near the probe), the probing sanity check could trigger, not expecting the probe to be triggered that quickly
• If the corner of the build plate is too low (away from the probe), the probing crash check could trigger, thinking the probe might be disconnected and the printbed is being rammed.

Benefits

These bugs are fixed:

• G34_M422.cpp, l. 156: sign was wrong (- instead of +)
• G34_M422.cpp, l. 157: needed to update the position in the planner
• G34_M422.cpp, l. 201: disable probe-trigger-too-early sanity check. This would always trigger because of l. 156 (which was there before, but not working, hence this sanity check didn't kick in either).
• G34_M422.cpp, l. 178: Reverted back to an explict for loop, rather than the LOOP_L_N macro to be able to use the counting variable further down the code.

Related Issues

Follow-up to PR #17013

TODO

@sjasonsmith
I don't fully understand the computation of z_probe in line 142..150 yet. It seems a bit odd to me that in a calculation looking like Pythagoras theorem, the x coordinate is subtracted from the y coordinate, than squared, i.e. sqrt((x0-y1)² + (x1-y1)²). Shouldn't it be something like sqrt((x0-x1)² + (y0-y1)²)?

Also, I think there is a "SQRT(" missing in line 148.

[sjasonsmith]

I noticed a warning about the for loop iteration variable yesterday. I'm glad you're fixing it.

I don't fully understand the computation of z_probe in line 142..150 yet.

I didn't write that code, and never tried to verify its correctness. My name probably shows up on plenty of Blame history on there due to minor cleanup from my other PRs, and even those were likely thinkyhead pushing style changes along with my edits.

My contribution to all of this was adding the "known stepper positions" feature. I tried to leave everything around it as untouched as possible, to avoid breaking existing users. There is very likely other bugs in this space that could use improvements.

It all comes down to having more users willing to contribute, so I'm really happy you're doing this. That printer is off-site for me and a bit inconvenient to get to, so my ability to work with this code is currently somewhat limited.

[thinkyhead]

One thing I noticed in G34 is that it's marking Z as "not homed" so it will get corrected later. But Z is known as long as there was a G28 Z and the motor hasn't been turned off. So one improvement would be to get Z rectified before exiting and then it doesn't need to be re-homed.

And, I believe it's possible to keep the safety check in the probing. Simply increase the amount of margin being added to Z by whatever amount is the largest reasonable discrepancy you would expect. In this way the probe will at least stop before going to the point of damage.

[jufimu12]

There is very likely other bugs in this space that could use improvements.

I probably will think about that z_probe formula, but I'll tackle my other 3D printing issues first.

One thing I noticed in G34 is that it's marking Z as "not homed" so it will get corrected later. But Z is known as long as there was a G28 Z and the motor hasn't been turned off. So one improvement would be to get Z rectified before exiting and then it doesn't need to be re-homed.

Yes, the printer could know what position it is at, given that it just probed it. However, I don't have enough knowledge of Marlin to implement that while being sure I don't mess up some internal movement planning systems.

Also, at least for my printer, the homing is done by probing in the middle of the bed, while G34 is not. If the bed is uneven, this could potentially decrease accuracy.

And, I believe it's possible to keep the safety check in the probing. Simply increase the amount of margin being added to Z by whatever amount is the largest reasonable discrepancy you would expect. In this way the probe will at least stop before going to the point of damage.

There are two different kinds of safety check here:
a) probe.cpp, l. 574:
if (probe_down_to_z(z_probe_low_point, MMM_TO_MMS(Z_PROBE_SPEED_FAST)) // No probe trigger?
This prevents the probe from going down insanely far, in case it does not trigger at all. That is still kept on, but as the Z origin is moved by messing with current_position.z, it triggers later than it usually would, allowing the probe to reach the bed, before an error is thrown, even if it is tilted by a reasonable amount. I think, this is what you meant.

b) probe.cpp, l. 575:
|| (sanity_check && current_position.z > -offset.z + _MAX(Z_CLEARANCE_BETWEEN_PROBES, 4) / 2) // Probe triggered too high?
This throws an error, if the probe is triggered unexpectedly early. The definition of unexpectedly early is pretty much hardcoded, unfortunately.

Let's assume that z_probe (G34_M422.cpp, l.142) is 40mm. Then, the Z coordinate is shifted by 40/2 == 20mm, so that when the nozzle is touching the printbed, Z would read 20mm. That leads to the fact that the probe triggers at Z = ~22mm, which is far higher than what the probe routine expects. Hence, it throws the error, because it is triggered early, not late. That is why I disabled it.

[thinkyhead]

while being sure I don't mess up some internal movement planning systems

You'd have to go out of your way and hit the planner directly to get it wrong. The do_blocking_move and other high level motion functions manage the details of keeping all the coordinates straight.

[thinkyhead]

because it is triggered early, not late. That is why I disabled it.

Yes, well the only real issue is current_position.z += z_probe * 0.5f; as a way to prevent run_z_probe from throwing an error. This workaround just introduces a potential "too early" error.

A different "spoof" is just CBI(axis_known_position, Z_AXIS) before the probe and then restore axis_known_position afterward. That's because…

const float z_probe_low_point = TEST(axis_known_position, Z_AXIS)
                              ? -offset.z + Z_PROBE_LOW_POINT : -10.0;

With that trick the probe gets a full centimeter of freedom below Z=0.