Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove with_probe from do_z_clearance() #27305

Open
wants to merge 8 commits into
base: bugfix-2.1.x
Choose a base branch
from
4 changes: 1 addition & 3 deletions Marlin/src/gcode/calibrate/G28.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,18 +375,16 @@ void GcodeSuite::G28() {
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("R0 = No Z raise");
}
else {
bool with_probe = ENABLED(HOMING_Z_WITH_PROBE);
// Raise above the current Z (which should be synced in the planner)
// The "height" for Z is a coordinate. But if Z is not trusted/homed make it relative.
if (seenR || !(z_min_trusted || axis_should_home(Z_AXIS))) {
z_homing_height += current_position.z;
with_probe = false;
}

if (may_skate) {
// Apply Z clearance before doing any lateral motion
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("Raise Z before homing:");
do_z_clearance(z_homing_height, with_probe);
do_z_clearance(z_homing_height);
}
}

Expand Down
8 changes: 3 additions & 5 deletions Marlin/src/module/motion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1289,18 +1289,16 @@ void do_blocking_move_to(const xyze_pos_t &raw, const_feedRate_t fr_mm_s/*=0.0f*
* - If lowering is not allowed then skip a downward move
* - Execute the move at the probing (or homing) feedrate
*/
void do_z_clearance(const_float_t zclear, const bool with_probe/*=true*/, const bool lower_allowed/*=false*/) {
UNUSED(with_probe);
void do_z_clearance(const_float_t zclear, const bool lower_allowed/*=false*/) {
float zdest = zclear;
TERN_(HAS_BED_PROBE, if (with_probe && probe.offset.z < 0) zdest -= probe.offset.z);
NOMORE(zdest, Z_MAX_POS);
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("do_z_clearance(", zclear, " [", current_position.z, " to ", zdest, "], ", lower_allowed, ")");
if ((!lower_allowed && zdest < current_position.z) || zdest == current_position.z) return;
do_blocking_move_to_z(zdest, TERN(HAS_BED_PROBE, z_probe_fast_mm_s, homing_feedrate(Z_AXIS)));
}
void do_z_clearance_by(const_float_t zclear) {
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("do_z_clearance_by(", zclear, ")");
do_z_clearance(current_position.z + zclear, false);
do_z_clearance(current_position.z + zclear);
}
/**
* Move Z to Z_POST_CLEARANCE,
Expand All @@ -1309,7 +1307,7 @@ void do_blocking_move_to(const xyze_pos_t &raw, const_feedRate_t fr_mm_s/*=0.0f*
void do_move_after_z_homing() {
DEBUG_SECTION(mzah, "do_move_after_z_homing", DEBUGGING(LEVELING));
#ifdef Z_POST_CLEARANCE
do_z_clearance(Z_POST_CLEARANCE, true, true);
do_z_clearance(Z_POST_CLEARANCE, true);
#elif ENABLED(USE_PROBE_FOR_Z_HOMING)
probe.move_z_after_probing();
#endif
Expand Down
4 changes: 2 additions & 2 deletions Marlin/src/module/motion.h
Original file line number Diff line number Diff line change
Expand Up @@ -423,12 +423,12 @@ void restore_feedrate_and_scaling();
#define Z_POST_CLEARANCE Z_CLEARANCE_FOR_HOMING
#endif
#endif
void do_z_clearance(const_float_t zclear, const bool with_probe=true, const bool lower_allowed=false);
void do_z_clearance(const_float_t zclear, const bool lower_allowed=false);
void do_z_clearance_by(const_float_t zclear);
void do_move_after_z_homing();
inline void do_z_post_clearance() { do_z_clearance(Z_POST_CLEARANCE); }
#else
inline void do_z_clearance(float, bool=true, bool=false) {}
inline void do_z_clearance(float, bool=false) {}
inline void do_z_clearance_by(float) {}
#endif

Expand Down
4 changes: 2 additions & 2 deletions Marlin/src/module/probe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ float Probe::run_z_probe(const bool sanity_check/*=true*/, const_float_t z_min_p
if (DEBUGGING(LEVELING)) DEBUG_ECHOLNPGM("1st Probe Z:", z1);

// Raise to give the probe clearance
do_z_clearance(z1 + (Z_CLEARANCE_MULTI_PROBE), false);
do_z_clearance(z1 + (Z_CLEARANCE_MULTI_PROBE));

#elif Z_PROBE_FEEDRATE_FAST != Z_PROBE_FEEDRATE_SLOW

Expand Down Expand Up @@ -857,7 +857,7 @@ float Probe::run_z_probe(const bool sanity_check/*=true*/, const_float_t z_min_p
#if EXTRA_PROBING > 0
< TOTAL_PROBING - 1
#endif
) do_z_clearance(z + (Z_CLEARANCE_MULTI_PROBE), false);
) do_z_clearance(z + (Z_CLEARANCE_MULTI_PROBE));
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/module/probe.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class Probe {
static void move_z_after_probing() {
DEBUG_SECTION(mzah, "move_z_after_probing", DEBUGGING(LEVELING));
#ifdef Z_AFTER_PROBING
do_z_clearance(Z_AFTER_PROBING, true, true); // Move down still permitted
do_z_clearance(Z_AFTER_PROBING, true); // Move down still permitted
Copy link
Contributor

@DerAndere1 DerAndere1 Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

@classicrocker883 classicrocker883 Aug 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :

zoffsetHoming

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).

#endif
}

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/module/temperature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ volatile bool Temperature::raw_temps_ready = false;
planner.sync_fan_speeds(fan_speed);
#endif

do_z_clearance(MPC_TUNING_END_Z, false);
do_z_clearance(MPC_TUNING_END_Z);

TERN_(TEMP_TUNING_MAINTAIN_FAN, adaptive_fan_slowing = true);
}
Expand Down