Skip to content

Remove BLTouch initialization if not needed#21124

Closed
ldursw wants to merge 9 commits intoMarlinFirmware:bugfix-2.0.xfrom
ldursw:bugfix-2.0.x
Closed

Remove BLTouch initialization if not needed#21124
ldursw wants to merge 9 commits intoMarlinFirmware:bugfix-2.0.xfrom
ldursw:bugfix-2.0.x

Conversation

@ldursw
Copy link
Contributor

@ldursw ldursw commented Feb 18, 2021

Description

When BLTOUCH is enabled and Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is disabled the homing sequence will home XY, have a small pause, and home Z using the endstop switch. The delay can be removed if the BLTouch is initialized only if it is going to be used as a Z probe.

Requirements

BLTouch and Z endstop switch.

Benefits

This patch will make homing a little bit faster by removing a delay between homing XY and homing Z.

@thinkyhead
Copy link
Member

Please review #14381 by @bigironguru which there is an explanation of the problem. Even though the probe is not being used for homing, there is some risk that the probe may still be in an undefined or deployed state. Moving at homing speeds in the XY plane while a BLTouch probe is deployed could potentially damage the extended pin or its internals. The simplest way to get the probe into a defined state is to call bltouch.init() so this is done before the XY move even when the probe is not necessarily going to be used.

A more complete solution might try to read the state of the probe or otherwise determine whether the probe could possibly be deployed before doing the extra initialization. In any case I don't want to run ahead and undo #14381 without a strong replacement.

@thinkyhead
Copy link
Member

As an alternative, see if bltouch.stow() is any quicker or more reliable. It needs to be able to work even if you physically pull the pin down so that the firmware doesn't know it is 'deployed.'

@ghost
Copy link

ghost commented Feb 21, 2021

Gosh, all that stuff was so long ago...

At that time, when BTLOUCH code was in progressive change due to the V3.0 fiasco, many changes were made and also discussed in great detail - the decision was always to "be safe" when moving.

At printer initialisation even, the init of the BLTOUCH annoyed some people. Before a move to a probe location... and so on.

At that time Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN didn't work right either, so there were a lot of other issues influencing BLTOUCH probing and the associated code.

If someone really wants to get the highest possible speeds out of the BLTOUCH, then maybe this PR should be coupled to the BTLOUCH High-Speed Mode, where movement with a lowered pin is part of the featureset. So that selecting High-Speed Mode would also replace the init sequence with a STOW, mabye? That wouldn't change the above PR very much and is worth discussing.

Now to the suggestions by thinkyhead:

A simple STOW would be faster than the init(), but does not always succeed (if the BLTOUCH is in an error state, for example).

A query of the probe state will work for most probes, (some clones will fail) but the query command sequence (which involves switching into SW mode and back) takes longer than the init sequence.

@ldursw
Copy link
Contributor Author

ldursw commented Feb 22, 2021

I did some tests and the current code homes XY, stows the probe, then homes Z. It can still possibly damage the probe.

Maybe the homing sequence could be changed to raise Z, stow the probe, and home XYZ?

As an alternative, see if bltouch.stow() is any quicker or more reliable.

If it is stowed it is as fast as init. If it is deployed it is slower.

@FanDjango
Copy link
Contributor

If it is stowed it is as fast as init. If it is deployed it is slower.

I don't see that reflected in the code. Can you indicate the sequence of events (BLTOUCH commands sent) that show different behaviour based on probe being stowed or deployed? I see STOWING a STOWED probe is equally time consuming as STOWING a DEPLOYED probe, in both cases a STOW command. There is no prior (time consuming checking) of the probes status.

init in the code sequences a reset (500ms) command followed by a stow (750ms) command.

stow in the code sequences only a stow (750ms) command.

All this assumes no errors from the probe and no 5V/OD mode changes on init, which makes things take a lot longer.

@FanDjango
Copy link
Contributor

FanDjango commented Feb 22, 2021

Maybe the homing sequence could be changed to raise Z

Depending on a lot things in the configuration, I always thought that there actually should be a raise before the XY home? But also dependant on Z axis unknown/untrusted etc. Lots of things to test...

Looks like I will do some testing on my machine when it is free to see if I can confirm your statements.

Be nice to see your config, maybe.

@FanDjango
Copy link
Contributor

I did some tests and the current code homes XY, stows the probe, then homes Z. It can still possibly damage the probe.

If that is so, it needs to be investigated and the STOW or INIT is being done in that wrong place/at the wrong moment.

@ldursw
Copy link
Contributor Author

ldursw commented Feb 22, 2021

Can you indicate the sequence of events (BLTOUCH commands sent) that show different behaviour based on probe being stowed or deployed?

I pulled the pin manually and the probe entered the error state (blinking red). With init the probe is reset and stowed. With stow the probe tries to stow but doesn't work because of the error, then it resets and stows again which this time works. If the probe is in an error state it is slower than just resetting it.

if (_stow_query_alarm()) {
// The stow might have failed
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("BLTouch ALARM or TRIGGER after STOW, recovering");
_reset(); // This RESET will then also pull up the pin. If it doesn't
// work and the pin is still down, there will no longer be
// an ALARM condition though.
// But one more STOW will catch that

I always thought that there actually should be a raise before the XY home?

Sorry I wasn't clear in my wording. It currently raises Z before homing XY but the probe is only stowed after homing XY. Could that cause a collision? My suggestion is to raise Z, stow the probe, and continue the homing sequence.

Be nice to see your config, maybe.

Configuration.zip

@FanDjango
Copy link
Contributor

I pulled the pin manually and the probe entered the error state (blinking red). With init the probe is reset and stowed. With stow the probe tries to stow but doesn't work because of the error, then it resets and stows again which this time works. If the probe is in an error state it is slower than just resetting it.

Ok, that was the missing info: Probe in error state. I agree with the above statement.

Earlier versions of the BLTOUCH code queried the error state PRIOR to actions and were quicker to recognize this special case: Namely a manual intervention causing an Error State.

It currently raises Z before homing XY but the probe is only stowed after homing XY.

Ok. Needs to be looked at.

@ldursw ldursw marked this pull request as draft February 22, 2021 20:19
@FanDjango
Copy link
Contributor

In motion.cpp your change (and subsequent edits by thinkyhead) introduce an INIT, directly in front of the DEPLOY. What is that supposed to achieve?

@FanDjango
Copy link
Contributor

FanDjango commented Feb 23, 2021

@ldursw I had a quick opportunity to test the following two things and the second one confirms the behaviour you are addressing:

No. 1: Pull out pin, probe goes into ERROR state and push reset button of printer. The initialisation of the printer will INIT (= RESET, STOW) the probe, prior to anything bad that might happen. So that part is good. Has nothing to do with your PR, I know, but I wanted to make sure.

No. 2: Freshly reset unhomed printer, then pull out pin, probe goes into ERROR state and then choose Auto Home (X, Y & Z). XY Homing movement is done with the pin down and the probe blinking. Prior to the subsequent Z home, the probe will reset and stow.

Same behaviour if the printer is homed (all axes trusted) before the test.

So in my opinion this PR is primarily not about a (tolerable) delay for an init, but rather a PR for moving the init to a "better" place in the sequence of events.

It will be a few days before I could test the above commits and collaborate on this, but I certainly agree that this is important.

@ldursw
Copy link
Contributor Author

ldursw commented Feb 23, 2021

The initial patch in the PR was to remove the initialization between homing XY and homing Z if not using the probe for homing. After thinkyhead's comment other issue showed up related to homing with the probe deployed.

I've set the PR as a draft. I'll redo the changes to stow the probe after raising Z and change the PR's title. The changes in this PR don't currently reflect what we are discussing now.

@ldursw ldursw closed this Feb 25, 2021
@ldursw ldursw deleted the bugfix-2.0.x branch February 25, 2021 19:19
@ldursw
Copy link
Contributor Author

ldursw commented Feb 25, 2021

@FanDjango Created a new PR #21192

@thinkyhead
Copy link
Member

Thanks! It looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants