Variable number of G29 grid points (UBL, Bilinear)

[lukasradek]

Description

Allows for variable number of grid probing points up to the number (per axis) specified in config file.

The logic to determine the number of grid points can be based on many things (menu config, GCODE,...), but for now the aim is to automatically decrease the number of points when probing small area and point spacing is smaller than specified in Configuration.h by GRID_MIN_SPACING, which seems to be the issue that most people have with number of probing points.

Requirements

Probe, G29 Bilinear Leveling

Benefits

It addresses a lot of user requests and also decreases the probing time on larger machines when printing smaller things.
It goes together well with G29 LRFB syntax.

Configurations

The proposed changes in Configuration.h are submited in the PR.

Related Issues

• [FR] - Variable number of G29 mesh points #10547
• [FR] Variable point amount of probing grid #25660
• [FR] Adaptive Mesh Levelling #25689
• [FR] (Change # of Mesh Grid Points on TFT screen) #22861
• [FR] Change GRID_MAX_POINTS_X from configuration bed leveling menu #19209
• [FR] add mesh/probing variables to menu for changing without recompile #20974

Work In Progress

Todo Minimums:

• Implement for G29 H syntax
• Extend EEPROM storage logic
• Modify Bilinear Subdivision - subdivide_mesh
• Modify Extrapolation for non-rectangular beds - extrapolate_unprobed_bed_level

ToDo Possibilities:

• Accept arguments from G29 to set number of points directly
• Let user specify spacing (instead of grid points) in config and let the firmware determine number of points from probing area size.
• Allow 2x2 grids in Configuration.h (is there any reason why there is a sanity check that requires min 3x3?
• VARIABLE_GRID_POINTS flag should be automatically enabled when AUTO_BED_LEVELING_LINEAR is enabled. At this point the code always checks for #if ANY(AUTO_BED_LEVELING_LINEAR , VARIABLE_GRID_POINTS), but linear leveling has variable grid points actually implemented by default. Is there anything that such change would break?

In its current implementation Variable Grid Points work (math algos need to be checked) unless you want to use features from unchecked files mentioned below.

Extending Support and ToDo Files

Files that needs to be adjusted in order to fully support Variable Grid Points.
Without those adjustments, they should just fail gracefully and display NaN when showing value of grid point beyond the variable grid point limit (most of those changes concern UI printing leveling grid).

MESH_EDIT_MENU Support

• lcd\menu\menu_bed_leveling.cpp

G-code support

• gcode\bedlevel\G26.cpp
• gcode\bedlevel\G42.cpp
• gcode\bedlevel\M420.cpp
• gcode\bedlevel\abl\M421.cpp

Utils

• inc\Conditionals_LCD.h

e3v2 Support

• lcd\e3v2\jyersui\dwin.cpp
• lcd\e3v2\proui\bedlevel_tools.cpp
• lcd\e3v2\proui\dwin.cpp
• lcd\e3v2\proui\meshviewer.cpp

ExtUI Support

• lcd\extui\ui_api.cpp
• lcd\extui\anycubic_chiron\chiron_tft.cpp
• lcd\extui\anycubic_vyper\dgus_tft.cpp
• lcd\extui\dgus\mks\DGUSScreenHandler.cpp
• lcd\extui\dgus_e3s1pro\DGUSTxHandler.cpp
• lcd\extui\dgus_reloaded\DGUSScreenHandler.cpp
• lcd\extui\dgus_reloaded\DGUSTxHandler.cpp
• lcd\extui\ftdi_eve_touch_ui\generic\bed_mesh_base.cpp
• lcd\extui\ftdi_eve_touch_ui\generic\bed_mesh_edit_screen.cpp
• lcd\extui\ftdi_eve_touch_ui\generic\bed_mesh_view_screen.cpp

It looks like IA_CREALITY implements its own leveling and thus doesn't bother with VARIBALE_GRID_POINTS, but just to be sure...

• lcd\extui\ia_creality\ia_creality_extui.cpp
• lcd\extui\ia_creality\ia_creality_rts.cpp

Asking for HELP

Verify that mathematical algorithms in feature/bedlevel/alb/bbl.cpp class LevelingBilinear are correct.

• Extrapolation - extrapolate_unprobed_bed_level and extrapolate_one_point
• Cartesian moves - line_to_destination.
• Bilinear Subdivision - virt_2cmr, virt_cmr, virt_coord

Verify that EEPROM data storing and retrieval is implemented correctly

• EEPROM read a write of bedlevel.nr_grid_points is correct

Implement e3v2 and ExtUI support since I don't have the hardware to test it with.

• e3v2
• ExtUI

[lukasradek]

I have added G29 parameters N (will have to rename), M and G to let user override the GRID_MAX_POINTS_X (and Y) and GRID_MIN_SPACING configuration settings respectively.

G29 probing points determination logic:
Uses Configuration.h values GRID_MAX_POINTS_X, GRID_MAX_POINTS_Y and GRID_MIN_SPACING

By default, it will keep the spacing between points at (actually around) GRID_MIN_SPACING, but once the probing area gets big enough, that it would exceed the GRID_MAX_POINTS_?, the spacing will get bigger.

If user specifies N or M, it acts as a "hard" override and it will probe that number of points in given dimension (up to the limit of GRID_MAX_POINTS_?).

If user specifies G, it acts as a "soft" override. It overrides GRID_MIN_SPACING but respects N or M parameters (if set) instead of GRID_MAX_POINTS.

[thinkyhead]

The parameters G and M are verboten. The N parameter is also best avoided, though we sometimes begrudgingly allow it. If XY count parameters are needed, the usual choice is I and J.

[lukasradek]

I have noticed that N is used "internally" and now I am aware of the other two.
I will rename those.

[thinkyhead]

Anyway, we already use X and Y parameters with LINEAR leveling to set the number of points in X and Y so that will simply be reused here with BILINEAR.

I went ahead and added a commit to…

• Make this behavior optional with a VARIABLE_GRID_POINTS option.
• Adjust the code that applies GRID_MIN_SPACING.
• Remove the comments about abl.abl_points — Yes, that will also be recalculated, as it currently is with LINEAR, once the X/Y parameters are also applied to BILINEAR.
• Get a start on the changes in bbl.h and bbl.cpp which will need to apply to get_z_correction, extrapolate_unprobed_bed_level, etc., and the ABL_BILINEAR_SUBDIVISION methods.

[thinkyhead]

Other areas where this change will be taking effect include:

• EEPROM code in settings.cpp will need to save/load the number of points in X and Y.
• The ABL_BILINEAR_SUBDIVISION calculation of Catmull-Rom points will need to be updated.
• All LCD implementations that edit the mesh will need to be modified to account for variable size.
  • /lcd/dogm/marlinui_DOGM.cpp
  • /lcd/e3v2/jyersui/dwin.cpp
  • /lcd/e3v2/marlinui/ui_common.cpp
  • /lcd/e3v2/proui/bedlevel_tools.cpp
  • /lcd/e3v2/proui/dwin.cpp
  • /lcd/e3v2/proui/meshviewer.cpp
  • /lcd/extui/ui_api.cpp
  • /lcd/extui/ui_api.h
  • /lcd/extui/anycubic_vyper/dgus_tft.cpp
  • /lcd/extui/dgus/mks/DGUSScreenHandler.cpp
  • /lcd/extui/dgus_e3s1pro/DGUSTxHandler.cpp
  • /lcd/extui/dgus_reloaded/DGUSScreenHandler.cpp
  • /lcd/extui/dgus_reloaded/DGUSTxHandler.cpp
  • /lcd/extui/dgus_reloaded/config/DGUS_Constants.h
  • /lcd/extui/ftdi_eve_touch_ui/generic/bed_mesh_base.cpp
  • /lcd/extui/ftdi_eve_touch_ui/generic/bed_mesh_view_screen.cpp
  • /lcd/extui/ia_creality/ia_creality_extui.cpp
  • /lcd/extui/ia_creality/ia_creality_rts.cpp
  • /lcd/HD44780/marlinui_HD44780.cpp
  • /lcd/menu/menu_bed_leveling.cpp
  • /lcd/tft/ui_1024x600.cpp
  • /lcd/tft/ui_320x240.cpp
  • /lcd/tft/ui_480x320.cpp
  • /lcd/TFTGLCD/marlinui_TFTGLCD.cpp

[lukasradek]

Just to address a few things that I noticed at first glance.

1. You removed the whole G29 parameter logic... what should happen with that?
2. You rewrote the variable grid points logic to be based on spacing calculations instead of mine that was based on nr of probe points calculation. I actually wrote it your way the first time, but then rewrote because it was a bit less calculations... is the way you wrote it better in some way?

[thinkyhead]

You removed the whole G29 parameter logic... what should happen with that?

You should reuse the logic already implemented for LINEAR but extend it to BILINEAR.

You rewrote the variable grid points logic to be based on spacing calculations instead of mine that was based on nr of probe points calculation. I actually wrote it your way the first time, but then rewrote because it was a bit less calculations... is the way you wrote it better in some way?

I simply moved some lines around but tried to leave the logic the same. When calculations are made with constexpr values the compiler produces constants so there will be no extra calculation without GRID_MIN_SPACING. The only difference with and without GRID_MIN_SPACING is that there is additional calculation for GRID_MIN_SPACING, which applies with VARIABLE_GRID_POINTS or AUTO_BED_LEVELING_LINEAR.

Anyway, I'm done messing around and I'll get out of your way for a while as you work out the details. I assume you should be able to reuse the XY parameters that currently apply to LINEAR, but maybe you'll need to do it in a separate block. We'll see!

[thinkyhead]

Ok, just fixed my typos, and now I'll be scarce for a while! Hit me up on Discord if you run into any snags in any of the code affected by this option. You can go ahead and add VARIABLE_GRID_POINTS to one of the tests' (e.g., chitu_f103) opt_set section so that CI testing will flag any code issues as you go along.

[lukasradek]

@ThomasToka Can you please briefly explain what do your changes acomplish for this PR? As far as I can see most of the files you are modifying were not identified (albeit 2 years ago) as needing changes.

[ThomasToka]

Thanks for merging. Sadly i still cant run all tests locally at once. So i am dependant on the online checks.
But i am also working on a fix for the run_tests scripts so they are more reliable
I put another fix for the ui_api.cpp check. lukasradek#5 Sorry again.

What it does:

• add your missing todos
• make it fully TERNary so it can be merged without interruption for other implementations
• simplyfy some code
• make UBL the same like BILINEAR (i will have to add your spacing as last, thats the only difference atm)

Thank you so far for the merges!

[lukasradek]

@ThomasToka

Ok...

1. Create your own fork, submit commits there, GitHub will run thoses tests on your branch and after you are done, please submit a PR in full to my branch.

2. Included in that PR. Specify exactly what changes have you made, why have you made them and how they manifest in the Marlin's feature set. Also specify which specific todos from my OP are completed by your changes.

[ThomasToka]

Thanks for merging. We are now fine with the checks. Good idea with the fork. But i think it is a bit more of work needed to use the tests on github. But i will have a look into this.

The tests are now fine. I will write a little more tomorrow to your questions as i yesterday felt down the stairs and my right foot is demaged. I will go now to the surgery to make "a pic" of my leg ^^

[thinkyhead]

I've merged the latest codebase into this again to keep the concept rolling, and if you like I would be happy to rebase and squash it, with categories of changes divided into individual commits. Thank you for your patience while we've been focused on fixing bugs and getting Fixed-Time Motion into good condition. If we had known it would take us years to prepare the next release we might have allowed some more disruptive PRs like this one, but we wanted to avoid introducing sweeping changes that would require more followup patches and further delay the release.

I intend to release 2.1.3 today, if possible, and create a new branch for development of 2.2 and beyond, and that's where we can merge this and allow things to be a little bit broken while we find all the things that were missed or broken.

[thinkyhead]

The grid code for some displays is hard-coded to the manufacturer's specifications. Since not all displays will accomodate all forms of ABL, either, we may need to add sanity checks about which displays can be used with this new feature.

[thinkyhead]

Overall this is looking pretty good. I did my best in my initial review to make it a little neater, trying not to break too many things in the process. I will give it a test in the morning and if it doesn't explode then it just might make it into this release just under the wire…. No promises, though! There's still a small pile of things to review.

[lukasradek]

@thinkyhead that would be a great satisfaction for me.

If possible, try to merge Tramming Wizard as well. That one is completely ready for almost 2 years. The branch only needs merging with current state of bugfix. 🙏

#25339