G28: Safe homing. Fix mistaken assignment of Z to Y

[Baekalfen]

Description

During safe homing, I noticed the value of Z_HOMING_HEIGHT was mistakenly assigned to the Y coordinate. I traced it down to line 140, where an incorrect assignment of the destination is made.

Requirements

This was found on a RAMPS 1.4 board for an MPCNC. But any setup with Z_SAFE_HOMING enabled should do.

Benefits

It fixes the assignment of the Y and Z coordinate to not crash the probe anymore.

Configurations

My configurations are very specific to my use-case. But enabling Z_SAFE_HOMING and setting Z_HOMING_HEIGHT to something unique, like 11, will reproduce the bug.

Related Issues

[thinkyhead]

Can you test whether this is broken generally? For example, place this code in setup()…

xy_pos_t my_pos { 111, 222 };
xyze_pos_t my_dst { 11, 22, 33, 44 };
SERIAL_ECHOLNPGM("my_pos is { ", my_pos.x, ", ", my_pos.y, " }");
SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.e, " }");
my_dst.set(my_pos, 3333.0);
SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.e, " }");

I get the correct output, which is…

my_pos is { 111, 222 }
my_dst is { 11, 22, 33, 44 }
my_dst is { 111, 222, 3333, 44 }

[Baekalfen]

I cloned the repo in a new place and checked out bugfix-2.0.x. And I get the following:

my_pos is { 111.00, 222.00 }
my_dst is { 11.00, 22.00, 33.00, 44.00 }
my_dst is { 111.00, 222.00, 33.00, 3333.00 }

I should add, that my own setup is for a CNC, therefore I have no extruders. That might be why it change Y/Z pos, while this changes the E pos. But it's still different from your's.

I'm using VS code and PlatformIO. This the first few lines when compiling. Do we use the same?

Processing mega2560 (board: megaatmega2560; platform: atmelavr@~3.4; framework: arduino)
-------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/atmelavr/megaatmega2560.html
PLATFORM: Atmel AVR (3.4.0) > Arduino Mega or Mega 2560 ATmega2560 (Mega 2560)
HARDWARE: ATMEGA2560 16MHz, 8KB RAM, 248KB Flash
DEBUG: Current (avr-stub) On-board (avr-stub, simavr)
PACKAGES: 
 - framework-arduino-avr 5.1.0 
 - tool-avrdude 1.60300.200527 (6.3.0) 
 - toolchain-atmelavr 1.70300.191015 (7.3.0)
Converting Marlin.ino
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 5 compatible libraries
Scanning dependencies...
Dependency Graph
|-- <SPI> 1.0
|-- <Wire> 1.0
|-- <EEPROM> 2.0
|-- <SoftwareSerial> 1.0
Building in release mode
Compiling .pio/build/mega2560/src/src/HAL/AVR/HAL.cpp.o
Compiling .pio/build/mega2560/src/src/HAL/AVR/HAL_SPI.cpp.o
...

[DerAndere1]

With LINEAR_AXES 4 and EXTRUDERS 2 or EXTRUDERS 1and this test code

  xy_pos_t my_pos { 111, 222 };
  xyze_pos_t my_dst { 11, 22, 33, 44, 55 };
  SERIAL_ECHOLNPGM("my_pos is { ", my_pos.x, ", ", my_pos.y, " }");
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.i, ", ", my_dst.e, " }");
  my_dst.set(my_pos, 3333.0);
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.i, ", ", my_dst.e, " }");

I get this output:
my_pos is { 111.00, 222.00 }
my_dst is { 11.00, 22.00, 33.00, 44.00, 55.00 }
my_dst is { 111.00, 222.00, 33.00, 44.00, 3333.00 }

[DerAndere1]

LINEAR_AXES 4, EXTRUDERS 0 gives an even worse result with this test code:

  xy_pos_t my_pos { 111, 222 };
  xyze_pos_t my_dst { 11, 22, 33, 44 };
  SERIAL_ECHOLNPGM("my_pos is { ", my_pos.x, ", ", my_pos.y, " }");
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.i, " }");
  my_dst.set(my_pos, 3333.0);
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, ", ", my_dst.i, " }");

output:

my_pos is { 111.00, 222.00 }
my_dst is { 11.00, 22.00, 33.00, 44.00 }
my_dst is { 1.00, 3333.00, 33.00, 44.00 }

---

With LINEAR_AXES 3 and EXTRUDERS 0 and this test code

  xy_pos_t my_pos { 111, 222 };
  xyze_pos_t my_dst { 11, 22, 33 };
  SERIAL_ECHOLNPGM("my_pos is { ", my_pos.x, ", ", my_pos.y, " }");
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, " }");
  my_dst.set(my_pos, 3333.0);
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, " }");

I get this output:

my_pos is { 111.00, 222.00 }
my_dst is { 11.00, 22.00, 33.00 }
my_dst is { 1.00, 3333.00, 33.00 }

[DerAndere1]

Good news: I could not find another call of set(const XYval pxy, const T pz) in Marlin so far.

[Baekalfen]

With LINEAR_AXES 3 and EXTRUDERS 0 and this test code

  xy_pos_t my_pos { 111, 222 };
  xyze_pos_t my_dst { 11, 22, 33 };
  SERIAL_ECHOLNPGM("my_pos is { ", my_pos.x, ", ", my_pos.y, " }");
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, " }");
  my_dst.set(my_pos, 3333.0);
  SERIAL_ECHOLNPGM("my_dst is { ", my_dst.x, ", ", my_dst.y, ", ", my_dst.z, " }");

I get this output:

my_pos is { 111.00, 222.00 }
my_dst is { 11.00, 22.00, 33.00 }
my_dst is { 1.00, 3333.00, 33.00 }

That's actually "good" news, because that means you're also able to reproduce the exact issue I had ― assignment to y instead of z when there's 3 axis and no extruder.

[thinkyhead]

Thanks for the thorough notes! The shorthand method taking xy, z is certainly not required, but it can be useful in some cases. I'll see if there is some way to solve the unexpected behavior of that method. It's possible that some other set method is being called, rather than the intended one, possibly due to automatic typecasting. Should be an interesting puzzle to solve, and if it cannot be solved then we'll just throw out this form of set.

[thinkyhead]

The bug is pretty obvious (now) in types.h…

- FI void set(const XYval<T> pxy, const T pe)    { set(pxy); e = pe; }
+ FI void set(const XYval<T> pxy, const T pz)    { set(pxy); z = pz; }
+ FI void set(const XYval<T> pxy, const T pz, const T pe) { set(pxy, pz); e = pe; }

Please test with this patch, and if all is well then this can be merged, and hot-patched to 2.0.x.