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

[BUG] (manual) Mesh Bed Leveling has high initial Z #21239

Closed
pinchies opened this issue Mar 2, 2021 · 10 comments
Closed

[BUG] (manual) Mesh Bed Leveling has high initial Z #21239

pinchies opened this issue Mar 2, 2021 · 10 comments

Comments

@pinchies
Copy link
Contributor

pinchies commented Mar 2, 2021

Bug Description

Regression due to #20160
This addition of MANUAL_PROBE_START_Z being defined in Conditionals_post.h messes up manual mesh bed levelling.

When you perform mesh bed levelling, after the home, the nozzle moves to the first point. The height of that point should be "zero" - at the last home position. After levelling, the nozzle should move up, to the safe travel height, then move to the next XY point 2, and then move down to the previous Z height, same as point 1.

Walkthrough of problem in code:

For mesh bed levelling:
if MESH_BED_LEVELING --> if true, sets PROBE_SELECTED = 1 (line 795 in conditionals_LCD.h) [**]
if PROBE_SELECTED, then if not defined, Z_CLEARANCE_BETWEEN_PROBES gets defined from Z_HOMING_HEIGHT (line 2613 in Conditionals_post.h)
If Z_CLEARANCE_BETWEEN_PROBES is defined, then MANUAL_PROBE_START_Z is defined (line 2629 on Conditionals_post.h)
Then if MANUAL_PROBE_START_Z is defined, then lines 224-230 in bedlevel.cpp will not run
and thus MANUAL_PROBE_START_Z will be used for all bed levelling points as the start Z height.

The result is, that after moving to each new XY point during manual mesh bed levelling, the nozzle starts from Z = MANUAL_PROBE_START_Z rather than Z = Zprevious. (lines 224-230 in bedlevel.cpp)

One solution (UGLY, and WRONG, but WORKS perfectly) as short term workaround

change line 216 in bedlevel.cpp:
#ifdef MANUAL_PROBE_START_Z
becomes
#if defined(MANUAL_PROBE_START_Z) && !defined(MESH_BED_LEVELING)

[**] perhaps this should not be true??

@borland1
Copy link
Contributor

borland1 commented Mar 2, 2021

The MANUAL_PROBE_START_Z is defined so that the nozzle won't crash into the bed during manual bed leveling in cases where the first probe point happens to be lower than any of subsequent probe points. By default it is undefined in Configuration.h..

//#define PROBE_MANUALLY
//#define MANUAL_PROBE_START_Z 0.2

Probably should be changed to something like this

//#define PROBE_MANUALLY
#if ENABLED(PROBE_MANUALLY)
  #define MANUAL_PROBE_START_Z 0.2
#endif

@pinchies
Copy link
Contributor Author

pinchies commented Mar 8, 2021

@borland1 -- sure, that makes sense. Are you saying that you agree that this is a bug though?

The bug I am referring to is that when MANUAL_PROBE_START_Z is set, the previous Z height is not used, not for any of the probe points. Even if MANUAL_PROBE_START_Z = 0, then the previous bed height will NEVER be called. From what I can see this is a bug in the logic.

The code is extremely convoluted and there are zero code comments... does that make sense?

@borland1
Copy link
Contributor

borland1 commented Mar 8, 2021

I think what you described is expected behavior when calibrating the bed mesh with the bed geometry. When #define MANUAL_PROBE_START_Z 0.2 is uncommented (enabled), then during the Manual Bed Leveling calibration/setup, Marlin will position the nozzle height at 0.2mm above the Z homing height position for each grid point. As you calibrate each mesh point, Marlin keeps track of each point's adjustment until you save the mesh grid point offsets to EEPROM.

Before running the Manual Bed Leveling, you should have already leveled the bed's four corners with copy paper as a feeler gauge between nozzle and bed. Depending on your bed's flatness, you can change the 0.2mm value of MANUAL_PROBE_START_Z, but that default value seems like a reasonable one for most large beds which are rarely perfectly flat. After you save the mesh to EEPROM, for each reboot of the printer, you must first send G-code G420 S to enable the previously saved grid mesh. Bed leveling won't be active until after you Home the printer or send G-code G28 (Home).

I agree that the code is not well laid out with Probe Options and Bed Leveling as two separate topics in the config. You can find more information about bed leveling setup in the Configuration Documentation on Marlin's Web site. There is also bed leveling information in the G-code help section.

@pinchies
Copy link
Contributor Author

pinchies commented Mar 8, 2021

I disagree that this is as expected -- because this behaviour has changed, and the users I support have all noticed and complained! It did not operate as you describe previously. The commit I refer to does not give any indication that they are aware of how this change affected the manual mesh bed levelling process -- they only refer to levelling with probes.

Can you acknowledge those two points? That (a) this behaviour has changed, and (b) that it does not appear the behaviour change was anticipated?

I understand the process how to manually level a bed, have used it for many years! 0.2mm is not a useful value for all cases, some beds are out by up to +0.4mm or more, especially for warped aluminium beds on large printers -- my point being that one value does not work in all cases, and having a large MANUAL_PROBE_START_Z makes the levelling process very slow (under the present mode of operation). I would suggest it should be perhaps previous levelling point Z value + some new set offset like 0.2mm. The way this used to work was quick and efficient, and I did not enconunter any issues with the previous z value simply being used as the start Z for the next leveling point.

I hope I have been able to be clear, thanks.

@pinchies
Copy link
Contributor Author

pinchies commented Mar 20, 2021

Requesting review for this one.

Looking for someone else to check my conclusions that:
(a) manual mesh bed levelling behaviour (i.e. NON-probe manual mesh) has changed due to #20160 , and
(b) that it does not appear the behaviour change was anticipated?

The lack of commenting in this section of particularly obtuse convoluted code is unforgivable. For bonus points, if someone can confirm the correct behaviour during bed levelling as designed (documented anywhere?) that would also be great. Or I can propose a new system.

@pinchies
Copy link
Contributor Author

pinchies commented Apr 13, 2021

Trying again: prior to 18 Nov 2020, the previous behaviour for manual mesh bed leveling was that the previous leveled point height was used as the start height for the next mesh point. (Of course, the nozzle was moving up while traveling between points, so as not to scrape the bed with the nozzle.) This was a very reasonable process of operation in my opinion.

In the current build, the MANUAL_PROBE_START_Z is used as the starting Z height for ALL points, meaning it takes a long time to manually lower the nozzle at every single point.

I think this code change was a bug due to oversight (and cryptic convoluted uncommented code... grrr), as this code change was done with relation to ABL levelling and was not a deliberate change to the manual mesh bed levelling process.

@thinkyhead thinkyhead changed the title [BUG] Z initial height is high for ALL points when performing manual mesh bed levelling Fix high Z initial height in manual Mesh Bed Leveling Apr 24, 2021
@thinkyhead thinkyhead changed the title Fix high Z initial height in manual Mesh Bed Leveling Fix (manual) Mesh Bed Leveling high initial Z Apr 24, 2021
@thinkyhead thinkyhead changed the title Fix (manual) Mesh Bed Leveling high initial Z [BUG] (manual) Mesh Bed Leveling has high initial Z Apr 24, 2021
@thinkyhead
Copy link
Member

Please check on the latest code, since the PR was merged. It attempts to achieve consistency and provide a proper fallback if MANUAL_PROBE_START_Z is not defined while one of the manual-only leveling systems is enabled. Hopefully the behavior is more sensible. The old behavior of MBL should now be achievable by simply leaving MANUAL_PROBE_START_Z undefined while using either MBL or ABL+PROBE_MANUALLY.

@pinchies
Copy link
Contributor Author

I’ll give it a look over. Cheers.

@thisiskeithb
Copy link
Member

Closing since #21692 was merged.

@github-actions
Copy link

github-actions bot commented Oct 1, 2021

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 Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants