MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.17k stars 19.21k forks source link

[BUG] Safe homing uses XY_PROBE_SPEED instead of HOMING_FEEDRATE_XY when moving from the origin to the safe homing position #17857

Closed acwest closed 4 years ago

acwest commented 4 years ago

Bug Description

When homing with G28 Marlin (at least in Bugfix-2.0.x, I didn't check any other versions) is using the wrong speed to move to the safe homing position. Currently the code is using XY_PROBE_SPEED, which is the speed to use when moving between points while doing ABL, which may or may not be configured by the user. This speed is often set quite high (the default is certainly quite high on my system), which tends to cause quite a lot of noise at the beginning of every print. This should probably be HOMING_FEEDRATE_XY, as this is part of every G28, not ABL at all.

My Configurations

configuration.zip

Steps to Reproduce

  1. G28 (with safe homing enabled)

Expected behavior: homes using the homing feedrate

Actual behavior: homes using the ABL probing feedrate

Additional Information

The code in Marlin/src/gcode/calibrate/G28.cpp uses do_blocking_move_to for both quick_home_xy() and home_z_safely(), while it should probably be using do_homing_move() The other possibility is that do_blocking_move_to() should use homing_feedrate() for xy, it already uses it for z, but it seems unlikely (although the fact that it uses it for Z might be a bug)

thinkyhead commented 4 years ago

When not defined HOMING_FEEDRATE_XY is assigned to it automatically. It is the preferred setting to use because it is intended to differentiate the speed that could harm the probe from the speed of approach to endstops.

acwest commented 4 years ago

There is another difference, however. The ABL is something that is frequently only done occasionally, while homing occurs on every print. It is often the case that you set a higher probe speed as ABL can take a long time, but is done rarely, so the higher noise levels are acceptable in that instance, while homing, which occurs for every print, should not make a disproportionate amount of noise. There is also the issue of naming. If you have a name HOMING_FEEDRATE, you expect that to be the feedrate to be used for homing. XY_PROBE_SPEED is the speed you expect it to use while probing, and, in fact, the comments in the file reflect that:

// Homing speeds (mm/m)
#define HOMING_FEEDRATE_XY (50*60)
#define HOMING_FEEDRATE_Z  (4*60)
// X and Y axis travel speed (mm/m) between probes
#define XY_PROBE_SPEED 8000

I've been programming longer than you would believe (1978), and the one thing I have learned over the years, names are IMPORTANT. In the case that you want to preserve the distinction between approaching endstops and harming the probe (which should, if it isa a problem, be mentioned in the comments), a better solution is to define a safe homing speed as a separate define. The current system is inaccurate, and very difficult to track down due to the naming issue

thinkyhead commented 4 years ago

Sure, we all know our names are important. I couldn't have written Amiga 680x0 assembler for all those years if I wasn't fairly organized. Anyway, as you must know from your long years of Sinclair ZX-80 programming, when you inherit a large codebase you don't get around to fixing all the names in the first week. It takes some time. And HOMING_FEEDRATE was here long before I arrived, and in fact long before the concept of homing with a probe arrived. Fortunately for you and I, our advanced years have taught us patience, and in time we'll get around to rejiggering all the names.

AnHardt commented 4 years ago

Seen strictly HOMING_FEEDRATE is only the first move towards the endstop. This is constrained by: fast enough to trigger STALLGUARD and show enough to not smash the switch/frame (Braking distance). XY_PROBE_SPEED is for the xy-moves to the next location of the probe point grid. If the z-probe is used as the z-homeswitch, the move to the center of the bed is exactly that - a xy-move to the next point to probe. (If the z-homeswitch is not a probe, SAVEHOMING does not make sense anyway.) For me this is as it should be.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.