DELTA do_blocking_move_to() more like the Chartesian one

[AnHardt]

Make DELTA do_blocking_move_to() more like the Cartesian one.
To get the straight lines and to warrant straight down, not subdivided, probing moves.

[ghost]

I've tested this.
Behavior of G29 is chaotic.

Video clip:

[Blue-Marlin]

It does not look soo bad. We can see the new double bumps. We can see different probed points. :-)

Sadly we can also see a messed up current_position - for what cause ever. :-(
The printer assumes to be too high and therefore dos not rise enough.

[thinkyhead]

How about this?

• Is Z raising up? Do that first (stay in safe zone if XY has moves).
• Do XY move, if any.
• Move Z to final destination, if not already there.

feedrate_mm_m = (fr_mm_m != 0.0) ? fr_mm_m : XY_PROBE_FEEDRATE_MM_M;

bool has_xy_move = !(x == current_position[X_AXIS] && y == current_position[Y_AXIS]);
float z_first = max(z, current_position[Z_AXIS]);          // move up only
if (has_xy_move) NOMORE(z_first, delta_clip_start_height); // stay in the safe zone for XY move

set_destination_to_current();

// Move up or down as needed
if (z_first != current_position[Z_AXIS]) {
  destination[Z_AXIS] = z_first;
  prepare_move_to_destination_raw(); // ...set_current_to_destination
}

// Move laterally to XY (with interpolation)
if (has_xy_move) {
  destination[X_AXIS] = x;
  destination[Y_AXIS] = y;
  prepare_move_to_destination();     // ...set_current_to_destination
}

// Final Z raise if needed
if (z > current_position[Z_AXIS]) {
  destination[Z_AXIS] = z;
  prepare_move_to_destination_raw(); // ...set_current_to_destination
}

[thinkyhead]

Note that if your XYZ final destination is in the "unreachable" zone, the order of moves doesn't matter. The final position will still be off. However, if the final position is reachable, as long as you move Z to the destination first (even at an "unsafe" height), the move will always be doable.

So a perfectly viable option is the much simpler:

feedrate_mm_m = (fr_mm_m != 0.0) ? fr_mm_m : XY_PROBE_FEEDRATE_MM_M;

set_destination_to_current();

// Move up or down as needed
if (z != current_position[Z_AXIS]) {
  destination[Z_AXIS] = z;
  prepare_move_to_destination_raw(); // ...set_current_to_destination
}

// Move laterally to XY (with interpolation)
if (x != current_position[X_AXIS] || y != current_position[Y_AXIS]) {
  destination[X_AXIS] = x;
  destination[Y_AXIS] = y;
  prepare_move_to_destination();     // ...set_current_to_destination
}

[thinkyhead]

a messed up current_position - for what cause ever

A definite cause can always be determined in a deterministic system.

[thinkyhead]

Did you know? Cartesian is named after René Descartes?
"I print, therefore I am."

[AnHardt]

Rebased und pimped up according to #4363 (comment)

[ghost]

Today's report:
G28 and G29 of DELTA are still broken.

Command:

m502
m500
g28
g29

Result (Video clip):

A branch that it's used for testing
https://github.com/esenapaj/Marlin/tree/My-Micromake-Kossel-MEGA2560

[thinkyhead]

G28 and G29 of DELTA are still broken.

I'm not surprised. Recent changes have been too rushed and thoughtless. We need the magic combination of someone who knows how to code who also owns a Delta. Otherwise we are flailing in the dark.

I would rather start from a known working point than from where we are, but I suppose I'm stuck trying to untangle this mess by running the code in my head. I really, really don't want to have to spend so much time on this. I have other things I would much rather be doing.

Anyway, I will scrub the code now, and when I figure out how it got cocked-up I will surely let you know.

[thinkyhead]

Let's see… This is the most obvious error for a start:

- #define SET_CURRENT_FROM_STEPPERS() current_position[Z_AXIS] = z_before - stepper.get_axis_position_mm(Z_AXIS) + z_mm
+ #define SET_CURRENT_FROM_STEPPERS() current_position[Z_AXIS] = z_before + stepper.get_axis_position_mm(Z_AXIS) - z_mm

In the case of starting from "10mm" height and probing down until it hits the bed at "5mm", the bad code will give you (for example) 10 - 5 + 10 == 15 while the correct code will give you 10 + (5 - 10) == 5.

Then there are these confidence-killing redundancies. No, we don't need to set current_position ahead of calling the do_blocking_move_to functions. (EDIT: And this breaks the code.)

- current_position[Z_AXIS] += home_bump_mm(Z_AXIS);
- do_blocking_move_to_z(current_position[Z_AXIS], Z_PROBE_SPEED_FAST);
+ do_blocking_move_to_z(current_position[Z_AXIS] + home_bump_mm(Z_AXIS), Z_PROBE_SPEED_FAST);

[thinkyhead]

@esenapaj I will merge #4373 shortly. It fixes the bug I pointed out above and removes the unnecessary redundancies setting destination from current_position in do_blocking_move_to (because the prepare_move_to_destination* functions always return with destination identical to current_position).

[Blue-Marlin]

Yesterdays extension to do_blocking move_to() does not work because:

When calling

current_position[Z_AXIS] = 123;
do_blocking move_to_z(current_position[Z_AXIS]);

And in do_blocking move_to() compare

if (current_position[Z_AXIS] > delta_clip_start_height) 

and try to see current_position[Z_AXIS] as the point where we are - this will fail.

[thinkyhead]

@Blue-Marlin Precisely. It was never meant to have current_position set ahead of calling. It must be called with current_position set to the current bloody position. One of those little "nuances" that we need to pay attention to.