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

Prevent accidental homing using minimal stepper speed when MBL is not enabled #5525

Closed
fiveangle opened this issue Dec 16, 2016 · 5 comments
Closed

Comments

@fiveangle
Copy link
Contributor

fiveangle commented Dec 16, 2016

Current code will accept G29 S0 command to give mesh report if MBL is enabled, but if ABL is enabled, the command is interpreted as "perform auto bed level with speed 0" but instead of error, actually executes:

Marlin_main.cpp:3953: xy_probe_feedrate_mm_s = MMM_TO_MMS(code_seen('S') ? code_value_linear_units() : XY_PROBE_SPEED);

Once passed down to the main movement engine, xy_probe_feedrate_mm_s presumably gets adjusted to the minimum stepper movement speed available, resulting in the appearance of the bot not moving, with the first active stepper making a grinding sound. This can lead to the user believing there is a catastrophic issue and yanking power (ask me how I know ? :)

Possible ways to address:

  1. Provide current ABL report info stored (if any) just as S0 with MBL enabled does
  2. Provide error response if S0 parameter is received with G29 command when ABL enabled (something like "Probe speed must be > 0 mm/s")
  3. If ABL has not been executed, 2, if it has, 1

My thinking behind this is:

  1. User requested 0 mm/s and we did not/cannot give it them anyway, so why continue ?
  2. Users switching from MBL to ABL may have heart attack when executing S0, thinking they will get the report for the ABL and getting a grinding bot instead.

Given enough patience though, it does work :-D

Long log ``` Send: G29 S0 Recv: G29 Auto Bed Leveling Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: echo:busy: processing Recv: Eqn coefficients: a: -0.0001837 b: -0.0000821 d: 1.2480180 Recv: Recv: Recv: Bed Level Correction Matrix: Recv: +1.000000 +0.000000 -0.000184 Recv: -0.000000 +1.000000 -0.000082 Recv: +0.000184 +0.000082 +1.000000 Recv: X:102.00 Y:127.00 Z:4.19 E:0.00 Count X: 8160 Y:10160 Z:8518 Recv: ok ```
@fiveangle
Copy link
Contributor Author

fiveangle commented Dec 16, 2016

I hadn't noticed until now, but it seems many don't understand the ABL S parameter, #5508. Perhaps we need more than my above suggestion in order to clue people in to what's happening; maybe my suggestions for addressing "S0" as above, but also echo "G29 Auto Bed Level started at speed n mm/s" in response to "G29 Sn" ?

@fiveangle
Copy link
Contributor Author

fiveangle commented Dec 16, 2016

aaaaaaand, I just realized XY_PROBE_SPEED is interpreted as mm/min, which explains the painfully slow speed, even when using "S60".

@thinkyhead
Copy link
Member

How about this solution?

      xy_probe_feedrate_mm_s = MMM_TO_MMS(code_seen('S') ? code_value_linear_units() : XY_PROBE_SPEED);
+     NOLESS(xy_probe_feedrate_mm_s, 600);

@fiveangle
Copy link
Contributor Author

fiveangle commented Feb 20, 2017

Thanks for reviewing.

It will prevent users from beleiving the printer has froze or worse. If that is the only goal then yes, the above would do it.

I would much prefer to see it report the same info as MBL does (the current ABL matrix, assuming it is available) or throw an error, because logically, nobody who beleives they are using ABL would ever knowingly issue "S0", and to have Marlin execute bed leveling at some arbitrary minimum speed when when receiving "S0" would logically never be the expected result.

That said, you're the master of the deisgn goals and the final judge on what error checking is worth it (based on resulting code size, complexity, and your time) so it really comes down to if you're okay with it.

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants