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

Fix manual mesh bed levelling #5

Closed
wants to merge 11 commits into from

Conversation

pinchies
Copy link
Owner

Simple but crude fix for manual mesh bed leveling operation, for non-probe systems.

Requirements

  • Filling out this template is required. Pull Requests without a clear description may be closed at the maintainers' discretion.

Description

Bug Description

Regression due to MarlinFirmware#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) (perhaps this should not be true??)
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)

Until there is time to refactor the bed level code, this is the simple fix.

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

Benefits

Fixes the manual mesh bed levelling process. After each point, the nozzle moves to the z height of the previous point.

Related Issues

Resolves MarlinFirmware#21239

@thinkyhead
Copy link

This PR seems to be directed at your own fork.

@thinkyhead
Copy link

How can we explain to everyone that you are the world … you are the children … you are the ones who make a better day so let's start giving … any time and effort you can spare to help improve the comments in the code. I could spend all day on it but then I would never get on to anything else.

@pinchies
Copy link
Owner Author

pinchies commented Apr 25, 2021

Hi Scott, thanks so much for taking a look at the section of code in depth, and working to make it better.

I’m sorry if this frustration felt directed at you or the team. My criticism is perhaps related to the Marlin code style — the Marlin code style does not encourage the kind of more verbose comments that I personally would use:

https://marlinfw.org/docs/development/coding_standards.html

I’m not sure if this code was simply an oversight, but the complexity of the conditionals and compiler directives made this problem incredibly head-scratching to track through.

Again, thank you for helping to fix this issue. 🙌

@pinchies pinchies closed this Apr 25, 2021
@thinkyhead
Copy link

I can relate to frustrations everyone feels with Marlin code, its heavy use of macros and conditionals, and so on. It's never been the most fun codebase to try to wrangle, and there is a never-ending list of wrangles. This issue is at least relatively benign. We do want some way to use MANUAL_PROBE_START_Z when it's defined, but also allow it to be left undefined and then use the last Z position in that case. So, that's what I'm trying to solidify at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] (manual) Mesh Bed Leveling has high initial Z
2 participants