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

Babystep insertion fixes #3006

Merged
merged 8 commits into from
Feb 26, 2021
Merged

Babystep insertion fixes #3006

merged 8 commits into from
Feb 26, 2021

Conversation

wavexx
Copy link
Collaborator

@wavexx wavexx commented Jan 29, 2021

This PR prevents the user to enter Live Z (through long-press or via the menu) while performing mesh bed leveling or issuing individual Z probes.

Obviously these interfere with each other. We do this by setting/checking for the mesh_bed_leveling/homing flag when relevant.

We also cleanup some of the code around babystep insertion (notably, the alternative to BABYSTEP_LOADZ_BY_PLANNER is broken and has been removed).

PFW-1210

@DRracer
Copy link
Collaborator

DRracer commented Feb 2, 2021

@wavexx what is the preferred procedure of testing this PR? It makes sense to get merged as it improves the safety of operation with the printer, however we must be sure it doesn't break anything else.

@wavexx
Copy link
Collaborator Author

wavexx commented Feb 18, 2021

So while coming up with testing scenarios I was able to find even more unhandled situations which make the printer fail in various way.. I'll address these as well.

The code around these calls _requires_ that the steps are immediately
processed and/or added to the subsequent planner moves.

The only part that doesn't care about immediate insertion is the
direct user-insertion though the lcd encoder.
Further restrict babystep insertion when the lcd_update is enabled by
toggling homing_flag when probing Z (where Z shouldn't be touched
anyway as it would disrupt the measurement)

Also reset the encoder value during mesh leveling.
This prevents to perform disruptive actions during homing or between MBL
probes, which would result in a failure.
Move axis queues movements, which disrupts a normal print, homing (when
XY is combined) or MBL.

Likewise, "Disable steppers" only makes sense when the printer is fully
idle.

Only allow such actions when the printer is not active and/or in the
paused state.
Instead of resetting the encoder status when homing or leveling, simply
exit the move/liveZ menu.

When transitioning from idle->printing, axis move shouldn't be allowed
as it would insert moves during a print. This is always wrong.
The menu must be always dismissed. Instead of checking all places where
the menu could be active, automatically dimiss the menu from within
_lcd_move when homing/MBL is happening. The long-push function and the
settings menu checks if "axis move" is possible, and thus
prevent the user to re-enter the menu already.

When doing the first layer calibration, the _lcd_babystep_z is
automatically brought back after MBL has completed.

Technically we should do the same when entering/exiting the paused state
in _lcd_move. However, it's better to dismiss _any_ menu in
stop_and_save_print_to_ram/restore_print_from_ram_and_continue instead.

To be done later...
@wavexx
Copy link
Collaborator Author

wavexx commented Feb 25, 2021

The fix turned out slightly different than anticipated, however it covers 3 related scenarios involving axis moves where there shouldn't be any.

To reproduce (and verify):

Scenario 1: Move Z can be triggered during homing/leveling/single Z probe, making it fail.
To reproduce:

  • Reset
  • Calibration -> First Layer Cal -> PLA
  • Long-press encoder to enter Move Z
  • Wait there until MBL starts
  • Turn the encoder to make MBL fail

Scenario 2: The same with Live Z. This needs an SD or USB print to reproduce, but similarly:

G28 W
G4 S10 ; long-press encoder here and wait
G80 ; turn encoder while probing

MBL will not fail due to the small shifts, however the resulting mesh is completely wrong.

We fix these two by kicking the user out of Move/Live Z menu when homing/MBL is performed. Since the long-press function (and the other code paths) already check for it, it's not possible to enter again until MBL is fully complete.

This is better than just waiting (as incorrectly done previously), since the problem is that it's not guaranteed that the long-press function will be the same when the homing/mbl is over. Example: long press while preheating is usually "Move Z", but changes to "Live Z" as soon as printing starts. Previously this would allow to insert huge Z jumps while printing.

Scenario 3: Insert random moves during a print (several possible entry points).
To reproduce (easy):

  • Calibration -> First Layer Cal -> PLA
  • Enter Settings -> Move Axis
  • Wait until MBL starts
  • Start moving the encoder
    Second:
  • While recovering a print, during homing:
  • Settings -> Disable steppers

We now disable these two entries using the PRINTER_ACTIVE check, which now also includes homing and MBL.
I removed one extra commit about the Tune/Preheat menu constantly flashing which was fixed in PR #3015

@DRracer DRracer merged commit 8f216ab into prusa3d:MK3 Feb 26, 2021
@wavexx wavexx deleted the babystep_fixes branch March 21, 2021 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants