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

Add Extra ProUI features #26761

Open
wants to merge 64 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

classicrocker883
Copy link
Contributor

@classicrocker883 classicrocker883 commented Feb 1, 2024

Description

This adds the following features to ProUI:

  1. Show Mesh creation / Cancel auto bed leveling, ABL/UBL
    • While having the option to cancel and break mid operation probing, it specifically allows the probe to show the bed map being created for UBL and ABL, G29

  1. Ability to change MULTIPLE_PROBING/TOTAL_PROBING through the UI (how many probes when tramming/bed leveling/creating mesh
    • You can variate how many "probes" when probing, such as for bed tramming and creating leveling mesh

  1. Variable Mesh_Inset. Able to change the mesh's inset: 4 sides of Cartesian bed [X/Y] | [Min/Max]
    • You can variate the area of the mesh. Change the inset from each edge.

  1. Ability to change Probe Z feedrate (subsequent slow probes) now added

  1. Change the grid array - 3x3, 5x5, 9x9... (removed for future feature)
    • the only limitation I foresee for ProUI is because of the specific MeshViewer, it can only view up to 9x9, but I have another PR which allows switching between the two, once that is merged only then we can be able to allow this to go up to 10x10 even 15x15 if need. (goes with changing grid array)

  1. Rearrange menu items
    • Add levelMenu + swap w/advancedSettingsMenu
    • Keep printer settings in Advanced Settings menu and move access to Control menu
    • Designate the now Level menu for Bed Leveling and mesh stuff
    • Preheat menu altered
    • Add MSG_MESH_SETTINGS "Mesh Settings"

Other Changes

Changes to Gcode Thumbnail preview.
Move struct hmiData from dwin.h to dwin_defines.h
Remove redundant headers
Changed Probe::run_z_probe to accommodate variable MULTIPLE_PROBING - for ProUI


Note

if there is one more thing I'd like to add is being able to change the Z feedrate.
MSG_Z_FEED_RATE goes unused -- not anymore
Having the grid type be 10x10 or more is something we can work on next

one more thing is a couple other changes, like the toggleCheckboxLine() now uses ^= true

  • moved to other PR

Requirements

Benefits

Less clumped.
Better readability.
This goes for UI navigation.

Configurations

Related Issues

for void do_z_clearance(),
you have UNUSED(with_probe), yet it does get used...
so I change it to
IF_DISABLED(HAS_BED_PROBE, UNUSED(with_probe));
I just wanted to point this out, because it just didn't make sense, and hope this change is correct.

trying to get the bugs out of MULTIPLE_PROBING. its difficult to say which line of code to use. any advice would be helpful.

  • Multiple probing should be fine, there may be hiccups with HS (High Speed) Mode enabled, but that may depend on bed probe type (3D touch vs BL touch)

Code Below is not relevant to this PR anymore since being able to change grid array/grid mesh points was removed.

I get this weird warning only through github actions

Marlin/src/feature/bedlevel/ubl/ubl.cpp: In static member function 'static void unified_bed_leveling::set_all_mesh_points_to_value(const_float_t)':
Marlin/src/feature/bedlevel/ubl/ubl.cpp:107:20: warning: iteration 9 invokes undefined behavior [-Waggressive-loop-optimizations]
  107 |     z_values[x][y] = value;
      |     ~~~~~~~~~~~~~~~^~~~~~~
In file included from Marlin/src/feature/bedlevel/ubl/../../../inc/MarlinConfigPre.h:49,
                 from Marlin/src/feature/bedlevel/ubl/../../../inc/MarlinConfig.h:28,
                 from Marlin/src/feature/bedlevel/ubl/ubl.cpp:23:
Marlin/src/feature/bedlevel/ubl/../../../inc/Conditionals_LCD.h:1568:96: note: within this loop
 1568 |   #define GRID_LOOP(A,B) for (uint8_t A = 0; A < GRID_MAX_POINTS_X; ++A) for (uint8_t B = 0; B < GRID_MAX_POINTS_Y; ++B)
Marlin/src/feature/bedlevel/ubl/ubl.cpp:106:3: note: in expansion of macro 'GRID_LOOP'
  106 |   GRID_LOOP(x, y) {

and also for static void unified_bed_leveling::report_current_mesh()

@thisiskeithb
Copy link
Member

It would be nice if these kinds of features were implemented at a higher level so everyone could use them instead of a subset of LCDs/UIs.

Copy link
Contributor Author

@classicrocker883 classicrocker883 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if these kinds of features were implemented at a higher level so everyone could use them instead of a subset of LCDs/UIs.

I'm not so familiar with how that would be; could you point to me which file/line of those, maybe I could figure something out. I'd like to add some more to this.

@classicrocker883 classicrocker883 changed the title Add Cancel ABL feature Add Extra ProUI features Feb 3, 2024
@classicrocker883 classicrocker883 marked this pull request as draft February 3, 2024 15:27
@classicrocker883 classicrocker883 marked this pull request as ready for review February 8, 2024 09:06
@thinkyhead
Copy link
Member

I think it would be best to remove the variable mesh grid points part of this PR because we are trying to solve that problem more generally in another PR. The variable points would then apply to all displays and also extend G29 M420 M421 G-codes as needed. This ad hoc solution for ProUI is too "hacky" and invasive to the rest of the code and certainly won't be retained once the general solution is completed.

Copy link
Contributor Author

@classicrocker883 classicrocker883 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be best to remove the variable mesh grid points part of this PR because we are trying to solve that problem more generally in another PR. The variable points would then apply to all displays and also extend G29 M420 M421 G-codes as needed. This ad hoc solution for ProUI is too "hacky" and invasive to the rest of the code and certainly won't be retained once the general solution is completed.

ok yeah I see. can you point to me which PR you're talking about with variable mesh grid points

@classicrocker883
Copy link
Contributor Author

I may need some help figuring out getting Bilinear auto bed leveling to work with the variable mesh inset.
so far UBL works, I haven't yet tested Manual Mesh, but I think a change is needed since it uses the same get_mesh_x/y

I might be able to just adjust like so:
like for Manual Mesh bed leveling, add
#define MESH_X_DIST (float((MESH_MAX_X) - (MESH_MIN_X)) / (GRID_MAX_CELLS_X))

and in feature/bedlevel/abl/bbl.h

- static float get_mesh_x(const uint8_t i) { return grid_start.x + i * grid_spacing.x; }
- static float get_mesh_y(const uint8_t j) { return grid_start.y + j * grid_spacing.y; }
+ static float get_mesh_x(const uint8_t i) { return MESH_MIN_X + i * (MESH_X_DIST); }
+ static float get_mesh_y(const uint8_t j) { return MESH_MIN_Y + j * (MESH_Y_DIST); }

but to do it right, I think grid_start and grid_spacing has to be set correctly
fortunately I think I found the code in question for Bilinear:
Marlin\src\gcode\bedlevel\abl\G29.cpp

      const float x_min = probe.min_x(), x_max = probe.max_x(),
                  y_min = probe.min_y(), y_max = probe.max_y();

      if (parser.seen('H')) {
        const int16_t size = (int16_t)parser.value_linear_units();
        abl.probe_position_lf.set(_MAX((X_CENTER) - size / 2, x_min), _MAX((Y_CENTER) - size / 2, y_min));
        abl.probe_position_rb.set(_MIN(abl.probe_position_lf.x + size, x_max), _MIN(abl.probe_position_lf.y + size, y_max));
      }
      else {
        abl.probe_position_lf.set(parser.linearval('L', x_min), parser.linearval('F', y_min));
        abl.probe_position_rb.set(parser.linearval('R', x_max), parser.linearval('B', y_max));
      }

...
---
      // Probe at the points of a lattice grid
      abl.gridSpacing.set((abl.probe_position_rb.x - abl.probe_position_lf.x) / (abl.grid_points.x - 1),
                          (abl.probe_position_rb.y - abl.probe_position_lf.y) / (abl.grid_points.y - 1));

as you can see abl.gridSpacing.set uses the same format (MESH_MAX_X) - (MESH_MIN_X)) / (GRID_MAX_CELLS_X)
and abl.grid_points.x - 1 = GRID_MAX_CELLS_X

for context, bedleve.set_grid() sets the values for grid_spacing/grid_start,
and is used in get_mesh_x/y()

so where I am stuck is how do I set probe_position_rb/probe_position_lf to be MESH_MAX_X/MESH_MIN_Y without it being like a hack job?

@classicrocker883 classicrocker883 marked this pull request as ready for review May 22, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants