From 7c9295f4a6d3562d98fd9d463cb6c8ca1a1ac4ad Mon Sep 17 00:00:00 2001 From: Katelyn Schiesser Date: Sat, 12 Jun 2021 19:02:06 -0700 Subject: [PATCH 1/5] Change UBL gcode-input integers to uint8_t where negative numbers would not be expected; Update menu items for UBL Mesh repetition to not supply an 'R' value when trying to target all points. Changes in da4b6896f7e4f102d8c2164e7aecf22cf2922fe2 included changing the variable that stores incoming 'R' values as int8_t (-128..127), whereas before it was just an int. Menu items still using 'R999' would overflow and become '-25' which is not valid. Additionally, for UBL with MESH_SIZE > 11, a user would not be able to supply a valid repetition count over 127, even though UBL allows up to 225 points. --- Marlin/src/feature/bedlevel/ubl/ubl.cpp | 2 +- Marlin/src/feature/bedlevel/ubl/ubl.h | 8 ++++---- Marlin/src/lcd/menu/menu_ubl.cpp | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.cpp b/Marlin/src/feature/bedlevel/ubl/ubl.cpp index b7a2c380ce89..d10ef69cc3e5 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl.cpp @@ -263,7 +263,7 @@ bool unified_bed_leveling::sanity_check() { void GcodeSuite::M1004() { #define ALIGN_GCODE TERN(Z_STEPPER_AUTO_ALIGN, "G34", "") - #define PROBE_GCODE TERN(HAS_BED_PROBE, "G29P1\nG29P3", "G29P4R255") + #define PROBE_GCODE TERN(HAS_BED_PROBE, "G29P1\nG29P3", "G29P4R") #if HAS_HOTEND if (parser.seenval('H')) { // Handle H# parameter to set Hotend temp diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.h b/Marlin/src/feature/bedlevel/ubl/ubl.h index 562f15f74b9d..f67c378dd059 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.h +++ b/Marlin/src/feature/bedlevel/ubl/ubl.h @@ -47,11 +47,11 @@ struct mesh_index_pair; typedef struct { bool C_seen; - int8_t V_verbosity, - P_phase, - R_repetition, - KLS_storage_slot, + int8_t KLS_storage_slot, T_map_type; + uint8_t R_repetition, + V_verbosity, + P_phase; float B_shim_thickness, C_constant; xy_pos_t XY_pos; diff --git a/Marlin/src/lcd/menu/menu_ubl.cpp b/Marlin/src/lcd/menu/menu_ubl.cpp index 880b76ff7651..467bd81acfc7 100644 --- a/Marlin/src/lcd/menu/menu_ubl.cpp +++ b/Marlin/src/lcd/menu/menu_ubl.cpp @@ -176,7 +176,7 @@ void _menu_ubl_height_adjust() { void _lcd_ubl_edit_mesh() { START_MENU(); BACK_ITEM(MSG_UBL_TOOLS); - GCODES_ITEM(MSG_UBL_FINE_TUNE_ALL, PSTR("G29P4R999T")); + GCODES_ITEM(MSG_UBL_FINE_TUNE_ALL, PSTR("G29P4RT")); GCODES_ITEM(MSG_UBL_FINE_TUNE_CLOSEST, PSTR("G29P4T")); SUBMENU(MSG_UBL_MESH_HEIGHT_ADJUST, _menu_ubl_height_adjust); ACTION_ITEM(MSG_INFO_SCREEN, ui.return_to_status); @@ -594,9 +594,9 @@ void _menu_ubl_tools() { GCODES_ITEM(MSG_UBL_1_BUILD_COLD_MESH, PSTR("G29NP1")); GCODES_ITEM(MSG_UBL_2_SMART_FILLIN, PSTR("G29P3T0")); SUBMENU(MSG_UBL_3_VALIDATE_MESH_MENU, _lcd_ubl_validate_mesh); - GCODES_ITEM(MSG_UBL_4_FINE_TUNE_ALL, PSTR("G29P4R999T")); + GCODES_ITEM(MSG_UBL_4_FINE_TUNE_ALL, PSTR("G29P4RT")); SUBMENU(MSG_UBL_5_VALIDATE_MESH_MENU, _lcd_ubl_validate_mesh); - GCODES_ITEM(MSG_UBL_6_FINE_TUNE_ALL, PSTR("G29P4R999T")); + GCODES_ITEM(MSG_UBL_6_FINE_TUNE_ALL, PSTR("G29P4RT")); ACTION_ITEM(MSG_UBL_7_SAVE_MESH, _lcd_ubl_save_mesh_cmd); END_MENU(); } From 1e86276f6b860967039ef3184b2a636198e19459 Mon Sep 17 00:00:00 2001 From: Katelyn Schiesser Date: Sat, 12 Jun 2021 20:45:10 -0700 Subject: [PATCH 2/5] match 'int16_t' which is passed back by `parser.value_int()`, and take advantage of `parser.value_byte()` --- Marlin/src/feature/bedlevel/ubl/ubl.h | 2 +- Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.h b/Marlin/src/feature/bedlevel/ubl/ubl.h index f67c378dd059..f8c5a7a82843 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.h +++ b/Marlin/src/feature/bedlevel/ubl/ubl.h @@ -47,7 +47,7 @@ struct mesh_index_pair; typedef struct { bool C_seen; - int8_t KLS_storage_slot, + int16_t KLS_storage_slot, T_map_type; uint8_t R_repetition, V_verbosity, diff --git a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp index b5773b0d4604..7a9f11b100c5 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp @@ -305,7 +305,7 @@ void unified_bed_leveling::G29() { bool probe_deployed = false; if (G29_parse_parameters()) return; // Abort on parameter error - const int8_t p_val = parser.intval('P', -1); + const int8_t p_val = parser.byteval('P', 255); const bool may_move = p_val == 1 || p_val == 2 || p_val == 4 || parser.seen_test('J'); #if ENABLED(HAS_MULTI_HOTEND) const uint8_t old_tool_index = active_extruder; @@ -321,7 +321,7 @@ void unified_bed_leveling::G29() { // Invalidate one or more nearby mesh points, possibly all. if (parser.seen('I')) { - int16_t count = parser.has_value() ? parser.value_int() : 1; + uint8_t count = parser.has_value() ? parser.value_byte() : 1; bool invalidate_all = count >= GRID_MAX_POINTS; if (!invalidate_all) { while (count--) { @@ -345,7 +345,7 @@ void unified_bed_leveling::G29() { } if (parser.seen('Q')) { - const int test_pattern = parser.has_value() ? parser.value_int() : -99; + const int16_t test_pattern = parser.has_value() ? parser.value_int() : -99; if (!WITHIN(test_pattern, -1, 2)) { SERIAL_ECHOLNPGM("Invalid test_pattern value. (-1 to 2)\n"); return; @@ -1083,7 +1083,7 @@ bool unified_bed_leveling::G29_parse_parameters() { param.R_repetition = 0; if (parser.seen('R')) { - param.R_repetition = parser.has_value() ? parser.value_int() : GRID_MAX_POINTS; + param.R_repetition = parser.has_value() ? parser.value_byte() : GRID_MAX_POINTS; NOMORE(param.R_repetition, GRID_MAX_POINTS); if (param.R_repetition < 1) { SERIAL_ECHOLNPGM("?(R)epetition count invalid (1+).\n"); @@ -1091,14 +1091,14 @@ bool unified_bed_leveling::G29_parse_parameters() { } } - param.V_verbosity = parser.intval('V'); + param.V_verbosity = parser.byteval('V'); if (!WITHIN(param.V_verbosity, 0, 4)) { SERIAL_ECHOLNPGM("?(V)erbose level implausible (0-4).\n"); err_flag = true; } if (parser.seen('P')) { - const int pv = parser.value_int(); + const int pv = parser.value_byte(); #if !HAS_BED_PROBE if (pv == 1) { SERIAL_ECHOLNPGM("G29 P1 requires a probe.\n"); @@ -1181,7 +1181,7 @@ bool unified_bed_leveling::G29_parse_parameters() { } #endif - param.T_map_type = parser.intval('T'); + param.T_map_type = parser.byteval('T'); if (!WITHIN(param.T_map_type, 0, 2)) { SERIAL_ECHOLNPGM("Invalid map type.\n"); return UBL_ERR; From da9a81784d0fa2fca6cca46eee59988c5d053287 Mon Sep 17 00:00:00 2001 From: Katelyn Schiesser Date: Sat, 12 Jun 2021 21:04:50 -0700 Subject: [PATCH 3/5] since `unified_bed_leveling::storage_slot` is int8_t, make `KLS_storage_slot` match. make things that touch `GRID_MAX_POINTS` uint8_t where appropriate; fix params to `display_map(..)` --- Marlin/src/feature/bedlevel/ubl/ubl.cpp | 2 +- Marlin/src/feature/bedlevel/ubl/ubl.h | 6 +++--- Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.cpp b/Marlin/src/feature/bedlevel/ubl/ubl.cpp index d10ef69cc3e5..37c8be5bd8f5 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl.cpp @@ -164,7 +164,7 @@ static void serial_echo_column_labels(const uint8_t sp) { * 2: TODO: Display on Graphical LCD * 4: Compact Human-Readable */ -void unified_bed_leveling::display_map(const int map_type) { +void unified_bed_leveling::display_map(const uint8_t map_type) { const bool was = gcode.set_autoreport_paused(true); constexpr uint8_t eachsp = 1 + 6 + 1, // [-3.567] diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.h b/Marlin/src/feature/bedlevel/ubl/ubl.h index f8c5a7a82843..9fcf7c042cb8 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.h +++ b/Marlin/src/feature/bedlevel/ubl/ubl.h @@ -47,11 +47,11 @@ struct mesh_index_pair; typedef struct { bool C_seen; - int16_t KLS_storage_slot, - T_map_type; + int8_t KLS_storage_slot; uint8_t R_repetition, V_verbosity, - P_phase; + P_phase, + T_map_type; float B_shim_thickness, C_constant; xy_pos_t XY_pos; diff --git a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp index 7a9f11b100c5..25a6cfecfbc5 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp @@ -592,7 +592,7 @@ void unified_bed_leveling::G29() { // if (parser.seen('L')) { // Load Current Mesh Data - param.KLS_storage_slot = parser.has_value() ? parser.value_int() : storage_slot; + param.KLS_storage_slot = parser.has_value() ? (int8_t)parser.value_int() : storage_slot; int16_t a = settings.calc_num_meshes(); @@ -617,7 +617,7 @@ void unified_bed_leveling::G29() { // if (parser.seen('S')) { // Store (or Save) Current Mesh Data - param.KLS_storage_slot = parser.has_value() ? parser.value_int() : storage_slot; + param.KLS_storage_slot = parser.has_value() ? (int8_t)parser.value_int() : storage_slot; if (param.KLS_storage_slot == -1) // Special case, the user wants to 'Export' the mesh to the return report_current_mesh(); // host program to be saved on the user's computer @@ -673,7 +673,7 @@ void unified_bed_leveling::G29() { */ void unified_bed_leveling::adjust_mesh_to_mean(const bool cflag, const_float_t offset) { float sum = 0; - int n = 0; + uint8_t n = 0; GRID_LOOP(x, y) if (!isnan(z_values[x][y])) { sum += z_values[x][y]; @@ -734,7 +734,7 @@ void unified_bed_leveling::shift_mesh_height() { do { if (do_ubl_mesh_map) display_map(param.T_map_type); - const int point_num = (GRID_MAX_POINTS) - count + 1; + const uint8_t point_num = (GRID_MAX_POINTS - count) + 1; SERIAL_ECHOLNPAIR("Probing mesh point ", point_num, "/", GRID_MAX_POINTS, "."); TERN_(HAS_STATUS_MESSAGE, ui.status_printf_P(0, PSTR(S_FMT " %i/%i"), GET_TEXT(MSG_PROBING_MESH), point_num, int(GRID_MAX_POINTS))); @@ -1098,7 +1098,7 @@ bool unified_bed_leveling::G29_parse_parameters() { } if (parser.seen('P')) { - const int pv = parser.value_byte(); + const int16_t pv = parser.value_byte(); #if !HAS_BED_PROBE if (pv == 1) { SERIAL_ECHOLNPGM("G29 P1 requires a probe.\n"); @@ -1833,7 +1833,7 @@ void unified_bed_leveling::smart_fill_mesh() { return; } - param.KLS_storage_slot = parser.value_int(); + param.KLS_storage_slot = (int8_t)parser.value_int(); float tmp_z_values[GRID_MAX_POINTS_X][GRID_MAX_POINTS_Y]; settings.load_mesh(param.KLS_storage_slot, &tmp_z_values); From a5fd4732931fad95f25f5c806c901c23a2f11f2e Mon Sep 17 00:00:00 2001 From: Katelyn Schiesser Date: Sat, 12 Jun 2021 21:10:26 -0700 Subject: [PATCH 4/5] missed display_map in ubl.h --- Marlin/src/feature/bedlevel/ubl/ubl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/feature/bedlevel/ubl/ubl.h b/Marlin/src/feature/bedlevel/ubl/ubl.h index 9fcf7c042cb8..cf00a282cfdd 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl.h +++ b/Marlin/src/feature/bedlevel/ubl/ubl.h @@ -98,7 +98,7 @@ class unified_bed_leveling { static void report_state(); static void save_ubl_active_state_and_disable(); static void restore_ubl_active_state_and_leave(); - static void display_map(const int) _O0; + static void display_map(const uint8_t) _O0; static mesh_index_pair find_closest_mesh_point_of_type(const MeshPointType, const xy_pos_t&, const bool=false, MeshFlags *done_flags=nullptr) _O0; static mesh_index_pair find_furthest_invalid_mesh_point() _O0; static void reset(); From 5da0d10111da72eabe2d79b5b5ecbf3f26c64609 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 13 Jun 2021 20:41:11 -0500 Subject: [PATCH 5/5] ok to treat 'P' as unsigned --- Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp index 25a6cfecfbc5..11c05f6054e7 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp @@ -305,7 +305,7 @@ void unified_bed_leveling::G29() { bool probe_deployed = false; if (G29_parse_parameters()) return; // Abort on parameter error - const int8_t p_val = parser.byteval('P', 255); + const uint8_t p_val = parser.byteval('P'); const bool may_move = p_val == 1 || p_val == 2 || p_val == 4 || parser.seen_test('J'); #if ENABLED(HAS_MULTI_HOTEND) const uint8_t old_tool_index = active_extruder; @@ -619,8 +619,8 @@ void unified_bed_leveling::G29() { if (parser.seen('S')) { // Store (or Save) Current Mesh Data param.KLS_storage_slot = parser.has_value() ? (int8_t)parser.value_int() : storage_slot; - if (param.KLS_storage_slot == -1) // Special case, the user wants to 'Export' the mesh to the - return report_current_mesh(); // host program to be saved on the user's computer + if (param.KLS_storage_slot == -1) // Special case: 'Export' the mesh to the + return report_current_mesh(); // host so it can be saved in a file. int16_t a = settings.calc_num_meshes(); @@ -1098,7 +1098,7 @@ bool unified_bed_leveling::G29_parse_parameters() { } if (parser.seen('P')) { - const int16_t pv = parser.value_byte(); + const uint8_t pv = parser.value_byte(); #if !HAS_BED_PROBE if (pv == 1) { SERIAL_ECHOLNPGM("G29 P1 requires a probe.\n");