UBL cleanup, optimization

[thinkyhead]

Followup to #6150, aec85ad, 9a1949a, 9d3ac66, 171904b, d8724bb

Concise diff

[Roxy-3D]

One incremental clean up we might want to consider adding to this.... I wonder if we should delete all the

#if ENABLED(ULTRA_LCD)

code in UBL. The original thinking was that UBL would quickly be made to not need an LCD Panel. But that coding practice was not followed consistently and probably should just be removed until we are ready to do it for real. And in fact... If we were going to have conditional code, we don't want it conditional on ULTRA_LCD any way. It should be changed to be sensitive to any LCD Panel display.

Your thoughts???

[thinkyhead]

It's true that any code relating to button-clicks should be conditional on NEWPANEL and not ULTRA_LCD. However, the current supported method of waiting for the user is to use the wait_for_user flag because it is compatible with both NEWPANEL and EMERGENCY_PARSER.

If UBL is completely dependent on having an LCD, and that's not going to change, then it makes sense to just leave out these checks.

If the long-term plan is to make UBL work without needing an LCD then I suggest integrating it into the existing monolithic ABL version of gcode_G29 which at this current point is the most evolved framework we have to perform leveling either with or without an LCD controller.

We can do that gradually and develop a more generalized class for bed leveling that defines all the standard hooks. The mesh_bed_leveling class is pretty close to the model we should follow.

[Roxy-3D]

If the long-term plan is to make UBL work without needing an LCD then I suggest integrating it into the existing monolithic ABL version of gcode_G29 which at this current point is the most evolved framework we have to perform leveling either with or without an LCD controller.

We can do that gradually and develop a more generalized class for bed leveling that defines all the standard hooks. The mesh_bed_leveling class is pretty close to the model we should follow.

OK... I'll start by reviewing the existing code, both UBL and the various places where 'wait_for_user' is queried.

[thinkyhead]

For interfacing with the LCD, check out the gcode_M600 function, and how it does its magic. That's a good example of the LCD being "taken over" by an outside controller (M600) that depends on an LCD.

[Roxy-3D]

For interfacing with the LCD, check out the gcode_M600 function, and how it does its magic.

Will do... That was on my list for tomorrow morning....

[thinkyhead]

For the preferred method going forward, see the code in ultralcd.cpp pertaining to LCD_BED_LEVELING, PROBE_MANUALLY, and MESH_BED_LEVELING in my branch https://github.com/thinkyhead/Marlin/tree/rc_probe_manually (PR #6050). The code is based on the MBL method, and makes sense to adopt this for PROBE_MANUALLY since it makes all bed leveling work with or without a probe.

In short, any bed leveling implementation will need to:

• Provide hooks that both gcode_G29 and the LCD-based manual probe method use. For example, a method to get the Nth probe point XY (by zig-zag index), a method to set the Z, a (common) method to move to the next point, etc.
• Expose globals (or, in future, just define public data members or accessors) that the UI code might need to display. For example, the index of the point currently being measured.
• Provide a GCode-based interface to go to the next step (like MBL does with G29 S2), or break up the G29 procedure into smaller functions and call those.

Note in the PROBE_MANUALLY branch that gcode_G29 uses static variables for the G29 parameters when PROBE_MANUALLY is enabled (because G29 needs to keep track of its current state and be re-entrant). But otherwise it uses stack-based automatic variables, since these only need to live for the duration of gcode_G29. So, while it still costs memory to enable a probe, this is one place where having a probe actually saves a few dozen bytes. 💃

[Roxy-3D]

So... Tomorrow when I start looking at this... And M600... I should probably do a 'Sync' so I have all of these changes in my local copy, right?

[thinkyhead]

I should probably do a 'Sync' so I have all of these changes in my local copy, right?

These changes are not in RCBugFix, but only exist in my branch, so there's nothing to sync yet. So your easiest route is to wait till this PR is merged before adding on (unless you'd like to try adding a new remote, making a copy of the branch, adding onto it, pushing to your fork, and making a PR targeted at my branch…).

Before I merge this, you should test the code from the branch https://github.com/thinkyhead/Marlin/tree/rc_cleanup_6150_etc and make sure I haven't broken anything by mistake.

---

Git tip o' the day…

To make a copy of this branch in your fork, ready to edit:

git remote add thinkyhead [email protected]:thinkyhead/Marlin.git

(git remote add only needed once)

git fetch thinkyhead
git checkout thinkyhead/rc_cleanup_6150_etc -b copy_of_cleanup_6150_etc
git push --set-upstream origin copy_of_cleanup_6150_etc

---

…and to delete the branch, both local and in your upstream ('origin')…

git checkout RCBugFix
git branch -D copy_of_cleanup_6150_etc
git push --delete origin copy_of_cleanup_6150_etc

---

To see all your branches, including remotes, with details, you can use:

git branch -v --all

[teemuatlut]

The original thinking was that UBL would quickly be made to not need an LCD Panel

Yes please. I have small project that brings all the u8g2 supported screens to Marlin for those who just want an information screen and have no need for a menu system and have no encoder wheel in the first place. Which might just be me...

[thinkyhead]

Yes please.

An extension to ABL is in the works also. PROBE_MANUALLY makes G29 a step-by-step procedure. Start with G28, then G29. Nozzle moves to the first probe point and waits. Adjust Z with G1 commands or buttons in the host software, with paper under the nozzle. Send G29 again to go to the next probe point. Continue sending G29 until all points are measured.

If someone does happen to have an LCD (and no probe), then the "Level Bed" menu item can still be enabled with LCD_BED_LEVELING and PROBE_MANUALLY, and whichever type of AUTO_BED_LEVELING_* is enabled, it will be guided and controlled from the LCD. See #6050. 😁

[Roxy-3D]

What commands do I type into Git-Shell to get rid of beckdac/Marlin but not hurt the real repository up on GitHub? There does not seem to be any way to remove it from GitHub-Desktop. I've saved your Tip's of the Day to a Word file so I will have easy access to them in the future. I'll see if I can get your branch onto my local machine and bring it up with UBL enabled.

[Roxy-3D]

And... Your "Tip of the Day" directions probably need a 'mkdir' and 'cd' in them, right? I followed the directions and I think I messed up my fork I already had to track Marlin and do things:

[Roxy-3D]

I pulled down a .ZIP file... I dropped my Configuration.h and Configuration_adv.h files in place. Everything compiled and flashed without trauma. UBL appears to be alive and well. I can do an automatic probe with G29 P1. I can edit mesh points with G29 P4. And the G26 Mesh Validation Pattern prints. All of those previously mentioned functions can be aborted with a 'Press and Hold'.

The only thing I noticed that is messed up is this: You lost the \n on both of the header lines. After the (9,9) there should be a new line. And after the (202,179) there should be a new line. It is possible we should have an extra space in that 12 space function to get the columns to line up better.

If you can get the new lines back in there... It may be time to merge this Pull Request before it gets any bigger.

>>> G29 M
SENDING:G29 M
ubl_eeprom_start=544
Bed Topography Report:
(0,9)                                                                                                    (9,9)(1,1)                                                                                                (202,179)  0.420     0.440     0.390     0.285     0.300     0.005    -0.005    -0.095    -0.290    -0.400

  0.350     0.165     0.290     0.240     0.135     0.025     0.020    -0.140    -0.305    -0.410

  0.165     0.080     0.125     0.175     0.045     0.070    -0.025    -0.085    -0.255    -0.310

  0.080     0.095     0.190     0.135     0.090     0.390     0.010    -0.050    -0.175    -0.220

  0.065    -0.095     0.015   [ 0.000]   0.015     0.010     0.025    -0.070    -0.080    -0.180

 -0.140    -0.130    -0.115    -0.125    -0.080    -0.025    -0.020    -0.050    -0.055    -0.140

 -0.430    -0.395    -0.160    -0.155    -0.025    -0.105    -0.125    -0.050    -0.105    -0.055

 -0.695    -0.420    -0.355    -0.170    -0.235    -0.190    -0.165    -0.135    -0.125    -0.095

 -0.860    -0.740    -0.605    -0.395    -0.320    -0.260    -0.225    -0.160    -0.105    -0.085

 -0.935    -0.905    -0.710    -0.615    -0.400    -0.350    -0.310    -0.155    -0.050    -0.165
(1,1)                                                                                                    (202,1)
(0,0)                                                                                                       (9,0)

I've started a print just to make sure it still prints!!! :)

[thinkyhead]

What commands do I type into Git-Shell to get rid of beckdac/Marlin but not hurt the real repository up on GitHub?

git remote remove beckdac

[thinkyhead]

It is possible we should have an extra space in that 12 space function to get the columns to line up better.

The columns with the numbers in them are 10 characters long, so I've changed the function to print 10 spaces.