Remove with_probe from do_z_clearance()

[classicrocker883]

Description

The issue stems from when homing.

in motion.cpp void do_z_clearance():

It is better to omit the with_probe from here so that a normal Homing height (having a bed probe) can be achieved.

Otherwise it adds +/- the Z Offset - but in opposite logic fashion.
Because when the machine has already HOMED Z axis, this finds there the bed surface is. so it knows where 0.00 is. No need to +/- a Z Offset to get to Z_CLEARANCE_FOR_HOMING height.

Before change:

• Say the Nozzle rests 1.00mm above the bed surface when Z axis is 0.00. Now when Homed, and Z_CLEARANCE_FOR_HOMING is 10, the actual measured distance is 11.00 above the bed surface when Z Offset is 0.00. Introduce a negative Z Offset (-1.00) to get the Nozzle touching the bed surface when Z = 0.00, and Home Z Axis is 10 -(-1.00)... or 11.00.
• Going the other way, with Z Offset positive, Home Z axis is 10 -(+1.00)... or 9.00.

After change:

• Nozzle rests at 10.00 after Z Axis gets Homed. This distance is true no matter the Z Offset
  Because before it would +/- the Z Offset and it would not resting at the proposed 10.00.

Requirements

Benefits

• If we omit the lines with_probe, we get a normal Home Z height (if set @ 5 or 10)

Configurations

Related Issues

#27294

[thinkyhead]

As you can imagine, reviewing a set of changes like these involves going back over the history of all the decisions that led to the code being the way it is, then considering from memory and experience all the possible scenarios where these changes might have been needed and trying to imagine how each change might lead to some unwanted effect.

Also, this set of changes needs to be tested across a variety of hardware with different types of probe setups, Z homing directions and homing methods, and so on, before they can be fully trusted.

Every change we have made in this area has been in response to some issue that has arisen due to the way the code had been before the change. So we really need notes on each and every change that has been made here, along with a review of how each change was arrived at in past discussions and bug reports, and a summation of why each change is helpful or at least harmless in consideration of any issues the previous code was designed to address.

If you are responding to a specific bug report and attempting to address the bug, that needs to be done in a very careful way working with the creator of that bug report. Addressing the specific problem behavior they are reporting must not step on changes that were made for other good reasons along the way.

However, if you are simply looking at the code and not "getting" why a change was done or how the code got to be the way it is, and then you're trying to fix it to "make better sense," please don't do that. You really need to go through all the history, blame, and discussion before deciding that something "doesn't make sense." It very often makes perfectly good sense.

That doesn't mean there can't be a "better way." But every time we touch homing and probing code we need to do a thorough top-down point-by-point analysis of the entire homing / probing / leveling process and reacquaint ourselves with the whole thing, and only after discussing every scenario in detail can we properly review and trust a big set of changes like those suggested here.

[DerAndere1]

With this change, Z_AFTER_PROBING is a fixed, absolute Z hight of the nozzle after probing. I like to have these fixed endpoints for G-code commands. But we have to keep in mind that this is a change that breaks backwards-compatibillity. Currently, users can freely adjust nozzle-to-probe offset becausse the clearance after probing will be adjusted. With this change, they must keep probe.offset.z > (- Z_AFTER_PROBING).

[classicrocker883]

With this change, Z_AFTER_PROBING is a fixed, absolute Z hight of the nozzle after probing. I like to have these fixed endpoints for G-code commands. But we have to keep in mind that this is a change that breaks backwards-compatibillity. Currently, users can freely adjust nozzle-to-probe offset becausse the clearance after probing will be adjusted. With this change, they must keep probe.offset.z > (- Z_AFTER_PROBING).

This has nothing really to do with "backwards-compatibility", especially reverting back to older code. All this does is stop adding/subtracting the probe's Z offset from Homing / Z clearance - which in turn gives the intended true height.
Clearance before and after changes is not greatly affected. Z clearance height is true after changes, and before changes it is +/- probe Z offset.

I'm trying to see what you mean either way...:

Currently, users can freely adjust nozzle-to-probe offset becausse the clearance after probing will be adjusted.

Clearance after probing is not affected negatively, but it is beneficial.

• If +probe.offset.z ( > 0 ):

• • This brings the nozzle further UP

• If -probe.offset.z ( < 0 ):

• • This brings the nozzle further DOWN

---

See the diagram in my related issue #27294 :

The blue line represents where the Nozzle is when Z=0. As for the icon of the nozzle with the double white line and the word 'Home' represents where it is when Z is Home.

Let's take what you say if probe.offset.z > (- Z_AFTER_PROBING), according to the diagram above:

Homing will not bring the Z_AFTER_PROBING to the desired +10.00 above the bed. With the current logic, Z Offset +/positive gets subtracted, -/negative gets added (zdest -= probe.offset.z).

• This is represented in the Middle. Let's say you need a positive Z Offset of +10.00 in order for the Nozzle to meet the bed's surface, +10.00 gets subtracted from +10.00 of Z_AFTER_PROBING (assuming that is 10), so you're left 0.00 height at Home which is AT the bed surface, not above.

• The Left side represents a Z Offset of 0.00 (None/default). So homing may leave it +10 above the bed, but when Z=0 this will cause the Nozzle to go below the bed surface. That is why a positive Z Offset is needed, since it raises the Nozzle to the correct bed height.

• The Right side represents the opposite, a negative Z Offset will bring the Home position +20 above the bed, since probe.offset.z = -10.00; zdest = zdest - (-10.00);.
  And since a negative Z Offset lowers the nozzle when Z=0 it goes an additional 10 below the already 10 below, so 20 below the bed (or -20).

[classicrocker883]

However, if you are simply looking at the code and not "getting" why a change was done or how the code got to be the way it is, and then you're trying to fix it to "make better sense," please don't do that. You really need to go through all the history, blame, and discussion before deciding that something "doesn't make sense." It very often makes perfectly good sense.

@thinkyhead that isn't what is going on here. I understand perfectly "why a change was done". Have you ever heard of "the road to hell is paved with good intentions"? or what about "'Kindly let me help you or you will drown,' said the monkey putting the fish safely up a tree."?
i'm not saying this is like bad, I'm sure your intentions were for good, however, as with everything, it can be improved upon. To say things cannot absolutely be reverted because they were put there in the first place is ludicrous. you're right, it does often make good sense, however in practice, the proof is in the pudding, and I find in this case... it doesn't fully work as intended. I hate to say it but its true. we all wish things to work the first time, even worse, we all think what we do is always right and requires no change.

Since this code shouldn't affect different printers differently, what matters is testing all the scenarios, which doesn't require certain hardware, because as I said, the code is the same for all types.
basically, extensive testing isn't exactly necessary for this and I think you're over thinking it. it may be for other instances and code, but as far as I can tell, not for something as simple as this.

I laid out in the diagrams the different situations a user may have. I'm not saying every machine is the same, but the code they use is. So I disagree with this for the most part:

Also, this set of changes needs to be tested across a variety of hardware with different types of probe setups, Z homing directions and homing methods, and so on, before they can be fully trusted.

since this commit was added just about a year ago, no doubt the firmware has worked without it. so basically saying, at the very least the logic for adding the Z offset is backwards. It would be better just to omit the whole adding the probe Z offset to get the true Z homing height.

adding/subtracting Z offset AFTER homing does not affect it BEFORE homing. So I dont get what the issue is.

Z offset is meant to get the correct nozzle height when Z=0. it has nothing to do with Homing / do_z_clearance.

[classicrocker883]

this post on reddit also confirmed how this code is not right.

let me say it again because maybe I wasn't clear... so AFTER Homing, this sets the height to Z
Homing height (typically 5 or 10) minus the Z offset. according to that post, the Z offset being (-1.30) so the final Z home height is 6.30.

this should be 5, not anything else. the with_probe part here doesn't help at all.

maybe originally it was intended to include the Probe Z Offset BEFORE Homing such as to prevent a collision, but this is do_z_clearance which moves to a zdest after Homing. so it knows where the bed is, because this code is used only after it has Homed. Probe Z Offset is not necessary.

I stand by this PR as beneficial.

[thinkyhead]

Elements of this may very well appear beneficial by your own testing on all your varied hardware, but we did arrive at the concept of a raise that includes the probe offset after a lot of discussion and insistence from others just as certain as yourself that this was needed.

So, I will need to review all that past discussion. I will report back to you the important points and why some of the things were needed. We can bring in the original participants to go over it all again and get this sorted out.

[ThomasToka]

The concept of adding the z_offset is out of my sight with a simple ender 3 s1 pro where the z_offset is negative not the expected result. If i set Z_AFTER_PROBING to 5 it should be 5 for the nozzle position. And not 5 -(-z_offset) which adds up the z_offset.
If there are other cases where something like this is needed it should have a logic to catch those cases.
My2cents after i had to fight with this not logical behavior.

[classicrocker883]

The concept of adding the z_offset is out of my sight with a simple ender 3 s1 pro where the z_offset is negative not the expected result. If i set Z_AFTER_PROBING to 5 it should be 5 for the nozzle position. And not 5 -(-z_offset) which adds up the z_offset. If there are other cases where something like this is needed it should have a logic to catch those cases. My2cents after i had to fight with this not logical behavior.

so if I understand you correctly, then this change does make sense. adding z_offset isn't necessary when performing a Z clearance move. because once it begins this move, it has already homed, so the end result is where the position ENDS, not BEGINS.

as I described in the diagrams, this just complicated things.

including z_offset may be logical in some cases, however I just cannot find a need for it in this function at any rate.

[ThomasToka]

including z_offset may be logical in some cases, however I just cannot find a need for it in this function at any rate

if there are cases where this is absolutely needed it should be activated by those functions/boards/whatever. otherwise for me its not logical to have it. and marlin does not do what i want it todo and what the variables are set to. eg. Z_AFTER_PROBING 5 ends in Z_AFTER_PROBING 5 -(-z.offset).

thats like driving 100 shown on the speed meter. but in real driving 150. thats not cool.