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

Corner Leveling Marlin Fix #2825

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kisslorand
Copy link
Contributor

@kisslorand kisslorand commented Aug 20, 2023

Requirements

BTT or MKS TFT

Description

Some Marlin updates broke the corner leveling.
PR MarlinFirmware/Marlin#24390 makes the TFT to freeze when corner levels are probed.
PR MarlinFirmware/Marlin#25996 breaks the positioning of the probe to the right positions if home offset is used.

This PR fixes these issues making it backwards compatible. There's a FW size reduction by removing unnecessary code and optimizing the corners probe flow.
Also there's some cosmetic addition too:

  • if corners are not probed than "---" is shown in the value section of each corner represented on the TFT
  • "M48" is replaced with a button for "Home Z" function
  • in manual leveling menu the button for disarming the X and Y steppers is removed (it had no practical usage there), the need for a submenu became redundant so "Distance" and "Shim" buttons are visible now right in the menu itself

Benefits

Corner leveling usable with newest Marlin bugfix (as of 2023.VIII.21).

Fixes #2815
Fixes #2849
Fixes #2855

Related Issues

#2815, #2849, #2855

PR Status

Ready to merge but I keep my hopes low, posted here mostly for a reference in my repository.

Notes

Leechers are welcome to include it in their own private build and share it through Thingiverse or whatever else.

@radek8
Copy link
Contributor

radek8 commented Sep 30, 2023

@kisslorand
I tested your fix.
Unfortunately, it only works partially.
The front left and front right corners are not tested.
Only error information appears (Z out of range) and then only the right and left rear corners are tested.

@kisslorand
Copy link
Contributor Author

kisslorand commented Sep 30, 2023

Do you use "home offset" in Marlin?
Did you test it with higher edge value?

@radek8
Copy link
Contributor

radek8 commented Sep 30, 2023

I do not use "home offset"

@radek8
Copy link
Contributor

radek8 commented Sep 30, 2023

@radek8
Copy link
Contributor

radek8 commented Sep 30, 2023

The problem is that in the configuration I have set the minimum value of the Y axis to a negative value.
min Y = - 35

If I set the corner offset to 45mm, the probing is done in the front, but in the wrong place (10mm from the edge of the bed).
If I set the corner offset to 44mm probing does not take place.

This is also wrong. Negative values are the problem.

@kisslorand
Copy link
Contributor Author

kisslorand commented Oct 1, 2023

Can you please provide the response of the printer to the "M115" command?
I am interested in the response that looks like:
"area:{full:{min:{x:0.000,y:0.000,z:0.000},max:{x:305.000,y:305.000,z:390.000}},work:{min:{x:0.000,y:0.000,z:0.000},max:{x:305.000,y:305.000,z:390.000}}}"

The main problem is that Marlin reports only the motion limits that are not necessarily reflecting the bed size. To be able to know where are the edges, the bed size must be known, The FW of the TFT assumes that the data coming from M115 report somehow reflects the bed size (max - min). It was almost true for majority of the printers until recent Marlin updates.
I am checking right now the code and try to solve this once and for all.

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

20231001_100639

@kisslorand
Copy link
Contributor Author

Thank you. Until I finish working on fixing this please try the following:
Replace else if (ack_seen("full:")) in "parseACK.c" with else if (ack_seen("work:")).
It should work on your case although it's not a universal fix suitable for all the possible printer configurations.

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

The edit to the parseACK.c file you suggested solved my problem.
Thank you

The question is whether this modification does not break some other display menu.
In the past, Marlin stopped displaying the "full:" value correctly and displayed the same value for "full:" and "work:".
I had to ask the Marln repository for a fix because some TFT display users had a problem with it.
But I don't remember what the problem was, I would have to find a specific discussion.

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

Now I see that the original value was "work:"
You changed the value to pomto PR and that broke the feature for people with a configuration like mine.
Why did you change it when it worked?
Revert this change to undo and the problem will be gone :-)

@kisslorand
Copy link
Contributor Author

kisslorand commented Oct 1, 2023

I changed it because the value of "work" is inconsistent.
Make this test:

  1. boot your printer
  2. set a home offset with M206' (for ex M206 X13 Y14`)
  3. send `M115' and remember the values for "work" area
  4. home printer (G28)
  5. send M115 again and check the values for "work" area

This is just one test to show you what I am talking about, there are other scenarios when "work" area is inconsistent. It is always dependent if you homed or not the printer since the boot of the motherboard. Before homing "work" area is always 0 for minimum and the maximum is the bed size. After homing the minimum and maximum area corresponds to the soft endstops if they are enabled, otherwise remains the same.
There is no command to check if the printer was homed or not before so the TFT doesn't know what those "work" values represent.. There are other problems too, I am working on make it suitable for all printer setups.

You changed the value to pomto PR and that broke the feature for people with a configuration like mine.

Latest Marlin broke everyone's setup if using "work" and home offset.

BTW, I saw you use

#define X_MIN_POS -12
#define Y_MIN_POS -35

According to Marlin it represents how much further the axis can travel after triggering the endstop sensor. Does your X and Y axis can travel further after triggering the endstop sensor?

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

BTW, I saw you use

#define X_MIN_POS -12
#define Y_MIN_POS -35

According to Marlin it represents how much further the axis can travel after triggering the endstop sensor. Does your X and Y axis can travel further after triggering the endstop sensor?

No, activation of the limit switch is also the physical end of travel.
After activating the switch, it must return to the beginning of the bed to the zero point.

@kisslorand
Copy link
Contributor Author

kisslorand commented Oct 1, 2023

After activating the switch, it must return to the beginning of the bed to the zero point.

I believe that's what the "home offset" is for.

If you issue a G0 X117.5 Y117.5 command does your nozzle go exactly above the center of your bed?

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

I don't need a home offset. There are problems with him.

Yes. My bed is defined as 230x230.
If I give the command G0 X115 Y115, the active nozzle will be set to the center of the bed.

@radek8
Copy link
Contributor

radek8 commented Oct 1, 2023

A negative value of X will allow the second nozzle to reach the entire bed, i.e. the left edge of the bed. (the second nozle one is placed approx. 12mm to the right of the first one)

A negative Y value will allow me to probe the entire bed. (the probe is cce 34mm behind the nozzles)

@kisslorand
Copy link
Contributor Author

OK, understood. Thanks for the explanations.

@kisslorand
Copy link
Contributor Author

@radek
I finished redoing this PR to accommodate more printer config scenarios while playing along with latest Marlin too. Please test it with your configuration.
Also please note that "config.ini" is changed, new data is required for this PR to work.
Thank you in advance.

@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch from bd62536 to 5aad55b Compare October 2, 2023 08:08
@radek8
Copy link
Contributor

radek8 commented Oct 2, 2023

Thank you..
When I'm at the printer, I'll try your repair.

@radek8
Copy link
Contributor

radek8 commented Oct 3, 2023

Latest edits tested.
It works as expected.
Thank you

@radek8
Copy link
Contributor

radek8 commented Oct 4, 2023

Link also #2815
It's the same problem

@kisslorand
Copy link
Contributor Author

Thank you, I was just searching for it to be included.

@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch 3 times, most recently from cd640eb to c55da09 Compare October 4, 2023 04:56
@radek8
Copy link
Contributor

radek8 commented Oct 5, 2023

I have a question.
I'm missing the Home button in the corner leveling menu.
If I move the screws, the center of the bed also moves, and I would like to execute the G28 command again.
If the M48 button were moved elsewhere, there would be a free position.
The M48 button is not important when leveling corners. I think the M48 button is already in the BL-Touch menu.
Would you consider this change?

@kisslorand
Copy link
Contributor Author

kisslorand commented Oct 5, 2023

I am contemplating for a long time about the need for that button there. I still have to figure out how to make it with double function, sometimes only G28 Z is enough but sometimes all axis need to be homed if the bed moves while user adjusts the bed's screws (some might have very low IDLE time for steppers disable).
Please make a feature request so I can refer to it. I'm glad I am not the only one who sees homing button better suited there and sees M48 pointless in that menu.

I am still not sure about G28 Z or rather G28 would be a better choice there.

Would you consider this change?

Absolutely yes!

@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch 2 times, most recently from 206934b to 6de5e66 Compare October 6, 2023 10:37
@kisslorand
Copy link
Contributor Author

PR updated with the implementation of the FR #2855.

@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch 9 times, most recently from 62c3cf4 to a2bae33 Compare October 8, 2023 14:59
@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch 2 times, most recently from 2dd0946 to a9f2e2d Compare November 7, 2023 05:06
@kisslorand kisslorand force-pushed the Corner-Leveling-fix-after-Marlin-changes branch from a9f2e2d to 09b3560 Compare November 7, 2023 19:18
@radek8
Copy link
Contributor

radek8 commented Dec 9, 2023

@bigtreetech
Please merge. This is a bug fix.
Thank you

Copy link

This PR has been automatically marked as stale because it has had no activity for the last 60 days. It will be closed in 7 days if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the Stale label Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants