PROBE_OFFSET_WIZARD follow up to #20414

[FanDjango]

#20414 urgently fixed a problem with the PROBE_OFFSET_WIZARD. Works fine now.

But there was a discussion in #20414 about PROBE_PT_RAISE vs. PROBE_PT_STOW - and I delved back into history to compare what could be done.

As is originally done by me in G34 long ago, and later copied into G35, let's do it the same way in PROBE_OFFSET_WIZARD: A separate final STOW after the probe operation(s).

The probe is not stowed unless a PROBE_PT_STOW is used, which ist isn't, because a 'PROBE_PT_RAISE` is desired.

Some day, one might want to add this capability to the probe_pt(...) routine, telling it "this is a final probe", and it would STOW the probe. Or a way to request a PROBE_PT_STOW_AND_RAISE.

I have tested this and it combines @swissnorp s recommendation to do a z_raise(...), which is private in probe.cpp and the need to do a STOW after probe operation.

[thinkyhead]

There should be a raise done in set_deployed when the probe is stowed. Maybe there is some missing condition preventing the raise from taking place. I'd take a closer look at set_deployed and try to figure out why the raise is not being done when probe_at_point calls stow, which calls set_deployed.

Perhaps the Z axis is being marked as "un-trusted" prematurely, while the UNKNOWN_Z_NO_RAISE is also enabled. That would be something to look into….

[FanDjango]

I understand.

I will investigate some more - I will attach a servo probe and test this change before going any further. Pehaps the STOW is needed always. For all probes.

[FanDjango]

Ok, keeping things safe and simple. Using the same tried and tested techniques that can be found in G34, G35, etc.

[swissnorp]

There should be a raise done in set_deployed when the probe is stowed. Maybe there is some missing condition preventing the raise from taking place. I'd take a closer look at set_deployed and try to figure out why the raise is not being done when probe_at_point calls stow, which calls set_deployed.

Perhaps the Z axis is being marked as "un-trusted" prematurely, while the UNKNOWN_Z_NO_RAISE is also enabled. That would be something to look into….

@thinkyhead As I understand the code, there is no rise when the probe is stowed with defined as FIX_MOUNTED_PROBE or NOZZLE_AS_PROBE.

Marlin/Marlin/src/module/probe.cpp

Lines 342 to 359 in 8a4f8e7

	// Make room for probe to deploy (or stow)
	// Fix-mounted probe should only raise for deploy
	// unless PAUSE_BEFORE_DEPLOY_STOW is enabled
	#if EITHER(FIX_MOUNTED_PROBE, NOZZLE_AS_PROBE) && DISABLED(PAUSE_BEFORE_DEPLOY_STOW)
	const bool deploy_stow_condition = deploy;
	#else
	constexpr bool deploy_stow_condition = true;
	#endif
	// For beds that fall when Z is powered off only raise for trusted Z
	#if ENABLED(UNKNOWN_Z_NO_RAISE)
	const bool unknown_condition = axis_is_trusted(Z_AXIS);
	#else
	constexpr float unknown_condition = true;
	#endif
	if (deploy_stow_condition && unknown_condition)
	do_z_raise(_MAX(Z_CLEARANCE_BETWEEN_PROBES, Z_CLEARANCE_DEPLOY_PROBE));

@FanDjango I like your final solution! Keep consistency 👍

[sjasonsmith]

@thinkyhead, I agree with @swissnorp's assessment of the behavior. As currently written, PROBE_PT_STOW only guarantees clearance for the probe to stow. If the probe does not need any clearance, it does not raise above the print surface at all.

I think the current solution on this PR is good. Do you want to do anything else with it prior to merging?

[FanDjango]

@swissnorp I found out why the "PROBING Z REFERENCE" message was not appearing. Took me a while..., sorry. Perhaps one would like to review this again for safety, as @sjasonsmith s approval needs to be refreshed due to these last minute commits (I was playing with PROBE_PT_STOW_AND_RAISE as well and by mistake that also came in here, but I removed it again): commit1 commit2

[FanDjango]

Just for information:

On my test Marlin, I am using

z_offset_ref = probe.probe_at_point(wizard_pos, PROBE_PT_STOW_AND_RAISE, 0, true);

with no subsequent separate STOW.

To do this, I made the following modification to probe.cpp, at the end of probe_at_point(...), which was surprisingly painless:

    if (raise_after == PROBE_PT_STOW || raise_after == PROBE_PT_STOW_AND_RAISE)
      if (stow()) measured_z = NAN;   // Error on stow?
    const bool big_raise = raise_after == PROBE_PT_BIG_RAISE;
    if (big_raise || raise_after == PROBE_PT_RAISE || raise_after == PROBE_PT_STOW_AND_RAISE)
      do_blocking_move_to_z(current_position.z + (big_raise ? 25 : Z_CLEARANCE_BETWEEN_PROBES), MMM_TO_MMS(Z_PROBE_SPEED_FAST));

Of course, PROBE_PT_STOW_AND_RAISE needed to be defined in the enumin probe.h

This would obviate all separate calls to probe.stow in many places where probe_at_point(...) is being used and where it is possible to programmatically decide that this probe operation is a "final" one.

[swissnorp]

@FanDjango
I realy would like to see a PR to have PROBE_PT_STOW_AND_RAISE implemented into probe_at_point() and replace all the probe.stow().

👍

[FanDjango]

@swissnorp Ok, but step by step

@ all: I am going to try to rebase squish this so a final review is easier

[FanDjango]

this single squished commit is what we are looking at finally. Please review this. Makes things clearer, sorry for all the back and forth previously.

[thinkyhead]

Thanks for the suggestion @swissnorp — I am actually following up on that, so should have something soon.