Probe offset wizard: STOW probe and loosen endstops when using PROBE_OFFSET_WIZARD_XY_POS

[FanDjango]

Description

When you enable the PROBE OFFSET WIZARD:

Defining PROBE_OFFSET_WIZARD_XY_POS, means the code will use a

probe.probe_at_point(...)

at that XY place, which, when complete, will need the software endstops to be "loosened" once again, otherwise manual movement of the nozzle towards the bed (using the wizard lcd menu) will be constrained at some point and in some cases.

The software endstops are already loosened at the beginning of the sequence, but this is then lost.

Therefore, "loosening" the software endstops should be performed later.

Also, it is cleaner to STOW the probe after this operation, although the code DOES alleviate the danger of leaving the probe deployed by raising far enough.

Benefits

Defining PROBE_OFFSET_WIZARD_XY_POS can cause the probe wizard to fail otherwise.

[qwewer0]

Marlin/Marlin/src/lcd/menu/menu_probe_offset.cpp

Lines 128 to 130 in f3bddc4

	#ifndef PROBE_OFFSET_WIZARD_XY_POS
	#define PROBE_OFFSET_WIZARD_XY_POS XY_CENTER
	#endif

It will be defined either way, so this is a fix for all. 👍
I misread it.

[swissnorp]

PullRequest #20344 was in draft when merged. So i think it was not tested.

The Idea of PROBE_PT_RAISE was, that there should be a z rise after probing. I didn't realize that PROBE_PT_RAISE excludes a STOW (but it's logic 🙃 ).

I think we should add the following after probing:

// Rise Nozzle after probing for probes, they don't rise when stowing.
#if EITHER(FIX_MOUNTED_PROBE, NOZZLE_AS_PROBE)
  do_z_raise(_MAX(Z_CLEARANCE_BETWEEN_PROBES, Z_CLEARANCE_DEPLOY_PROBE));
#endif

[FanDjango]

Too bad there is no PROBE_PT_RAISE_STOW as a combination.

I wonder what the tradeoff is in terms of code bytes to create that in probe.cpp (couple of bytes only I think). But that code increase would hit everyone that has a probe, and in here it only hits those that activate this functionality. Your thoughts?

Also I don't really want to mess around with probe.cpp any more in this lifetime.

[sjasonsmith]

Z_CLEARANCE_DEPLOY_PROBE will be used along with PROBE_PT_RAISE_STOW.

Other commands that probe multiple times/points only stow the probe at the end of the entire sequence. They only raise between points. They then provide an argument such as G29 E if your probe needs to stow after each probe. This is useful for probes such as solenoids which may overheat if deployed for too long.

Some probes require a slow or manual deploy/stow process, so you don't want to actually deploy/stow multiple times.

[sjasonsmith]

That is an ideal though. This PR doesn't have to wait for a larger rework.

[sjasonsmith]

Nevermind, I was confusing this with G35. Always looking at code out of context 😉

[sjasonsmith]

I think we should add the following after probing:

@swissnorp I think that capability could make sense. I don't know if it belongs as part of the probe code itself or simply as a move at the end of the wizard. I can imagine there will be someone annoyed and reporting bugs if it were made the behavior after every probe.

[swissnorp]

Too bad there is no PROBE_PT_RAISE_STOW as a combination.

I wonder what the tradeoff is in terms of code bytes to create that in probe.cpp (couple of bytes only I think). But that code increase would hit everyone that has a probe, and in here it only hits those that activate this functionality. Your thoughts?

Also I don't really want to mess around with probe.cpp any more in this lifetime.

@FanDjango I was thinking about the same even before you had posted it. 😄
But I finaly think its not needed.
I realy like the solution you provided in #20439

I think we should add the following after probing:

@swissnorp I think that capability could make sense. I don't know if it belongs as part of the probe code itself or simply as a move at the end of the wizard. I can imagine there will be someone annoyed and reporting bugs if it were made the behavior after every probe.

@sjasonsmith I didn't realize that it was a private function and do_z_clearance would be a better solution because do_z_raise does lift the nozzle even higher with a negative probe_offset.z. But I think #20439 is the way to go!

Marlin/Marlin/src/module/probe.cpp

Lines 259 to 267 in e817773

	/**
	* Raise Z to a minimum height to make room for a probe to move
	*/
	void Probe::do_z_raise(const float z_raise) {
	if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPAIR("Probe::do_z_raise(", z_raise, ")");
	float z_dest = z_raise;
	if (offset.z < 0) z_dest -= offset.z;
	do_z_clearance(z_dest);
	}

[FanDjango]

Whoaa there, I was just now working putting in @swissnorp s addition. You're very quick, @sjasonsmith . I just added it to this PR right now.

[sjasonsmith]

Whoaa there, I was just now working putting in @swissnorp s addition. You're very quick, @sjasonsmith . I just added it to this PR right now.

Lol, sorry about that. Once again I am confusing things, and based on the second comment thought the proposal was entirely targeting updating probing code, not simply a lift here.

[sjasonsmith]

My urgency to merge was motivating by wanting to fix the failure to stow ASAP.

[FanDjango]

I'm happy. @swissnorp is concerned that a STOW after probing done by probe_pt(...) might not clear a different kind of probe far enough from the bed - there is a move after probing to bring the nozzle into the position that was probed. A short move, true, but potential for disaster.

The discussion about probe_pt(...) was my fault. If a STOW alone is not good enough in some situations, might not a STOW+RAISE be needed in many other places?

I can live with leaving it like this and waiting for users to report further needed actions.

[sjasonsmith]

The discussion about probe_pt(...) was my fault. If a STOW alone is not good enough in some situations, might not a STOW+RAISE be needed in many other places?

Z_CLEARANCE_DEPLOY_PROBE is intended to provide mechanical clearance for the act of stowing the probe, such as swinging a servo arm. Hence the lack of anything for NOZZLE_AS_PROBE. FIX_MOUNTING_PROBE often do require stowing, because they are often used with PAUSE_BEFORE_DEPLOY_STOW and are manually attached to the printer.

Where a height is wanted after probing you would set Z_AFTER_PROBING to define the desired height. I'm not sure that value makes sense for this command.

I think all you would need is a move to Z_CLEARANCE_BETWEEN_PROBES prior to the X/Y move to reposition the nozzle. You don't need to worry about stow height, because that will have already happened in probe_pt.

[FanDjango]

In that case one would add solely:

  do_z_raise(_Z_CLEARANCE_BETWEEN_PROBES);

to be absolutely safe for all kinds of configurations.

I'll open a new one for this and test it first.

[FanDjango]

@swissnorp :

Everything works, but I am not seeing this message. I get a blank screen instead.

if (ui.should_draw()) MenuItem_static::draw(1, GET_TEXT(MSG_PROBE_WIZARD_PROBING));

Any ideas?

[swissnorp]

@swissnorp :

Everything works, but I am not seeing this message. I get a blank screen instead.

if (ui.should_draw()) MenuItem_static::draw(1, GET_TEXT(MSG_PROBE_WIZARD_PROBING));

Any ideas?

@FanDjango Could you solve it in the meantime?
I just can tell that it worked back then: menu_probe_offset.cpp.