UBL-related cleanup, spacing, standards

Reference: #6804

Marlin/Marlin.h

@@ -438,63 +438,56 @@ void do_blocking_move_to_xy(const float &x, const float &y, const float &fr_mm_s
 
 #if IS_KINEMATIC // (DELTA or SCARA)
 
-  #if ENABLED(DELTA)
-    #define DELTA_PRINTABLE_RADIUS_SQUARED ((float)DELTA_PRINTABLE_RADIUS * (float)DELTA_PRINTABLE_RADIUS )
-  #endif
-
   #if IS_SCARA
     extern const float L1, L2;
   #endif
 
-  inline bool position_is_reachable_raw_xy( float raw_x, float raw_y ) {
+  inline bool position_is_reachable_raw_xy(const float &rx, const float &ry) {
     #if ENABLED(DELTA)
-      return ( HYPOT2( raw_x, raw_y ) <= DELTA_PRINTABLE_RADIUS_SQUARED );
+      return HYPOT2(rx, ry) <= sq(DELTA_PRINTABLE_RADIUS);
     #elif IS_SCARA
       #if MIDDLE_DEAD_ZONE_R > 0
-        const float R2 = HYPOT2(raw_x - SCARA_OFFSET_X, raw_y - SCARA_OFFSET_Y);
+        const float R2 = HYPOT2(rx - SCARA_OFFSET_X, ry - SCARA_OFFSET_Y);
         return R2 >= sq(float(MIDDLE_DEAD_ZONE_R)) && R2 <= sq(L1 + L2);
       #else
-        return HYPOT2(raw_x - SCARA_OFFSET_X, raw_y - SCARA_OFFSET_Y) <= sq(L1 + L2);
+        return HYPOT2(rx - SCARA_OFFSET_X, ry - SCARA_OFFSET_Y) <= sq(L1 + L2);
       #endif
     #else // CARTESIAN
-      #error
+      // To be migrated from MakerArm branch in future
     #endif
   }
 
-  inline bool position_is_reachable_by_probe_raw_xy( float raw_x, float raw_y ) {
+  inline bool position_is_reachable_by_probe_raw_xy(const float &rx, const float &ry) {
 
-    // both the nozzle and the probe must be able to reach the point
+    // Both the nozzle and the probe must be able to reach the point.
+    // This won't work on SCARA since the probe offset rotates with the arm.
 
-    return ( position_is_reachable_raw_xy( raw_x, raw_y ) &&
+    return position_is_reachable_raw_xy(rx, ry)
-             position_is_reachable_raw_xy(
+        && position_is_reachable_raw_xy(rx - X_PROBE_OFFSET_FROM_EXTRUDER, ry - Y_PROBE_OFFSET_FROM_EXTRUDER);
-                raw_x - X_PROBE_OFFSET_FROM_EXTRUDER,
-                raw_y - Y_PROBE_OFFSET_FROM_EXTRUDER ));
   }
 
 #else // CARTESIAN
 
-  inline bool position_is_reachable_raw_xy( float raw_x, float raw_y ) {
+  inline bool position_is_reachable_raw_xy(const float &rx, const float &ry) {
-      // note to reviewer: this +/-0.0001 logic is copied from original postion_is_reachable
+      // Add 0.001 margin to deal with float imprecision
-      return WITHIN(raw_x, X_MIN_POS - 0.0001, X_MAX_POS + 0.0001)
+      return WITHIN(rx, X_MIN_POS - 0.001, X_MAX_POS + 0.001)
-          && WITHIN(raw_y, Y_MIN_POS - 0.0001, Y_MAX_POS + 0.0001);
+          && WITHIN(ry, Y_MIN_POS - 0.001, Y_MAX_POS + 0.001);
   }
 
-  inline bool position_is_reachable_by_probe_raw_xy( float raw_x, float raw_y ) {
+  inline bool position_is_reachable_by_probe_raw_xy(const float &rx, const float &ry) {
-      // note to reviewer: this logic is copied from UBL_G29.cpp and does not contain the +/-0.0001 above
+      // Add 0.001 margin to deal with float imprecision
-      return WITHIN(raw_x, MIN_PROBE_X, MAX_PROBE_X)
+      return WITHIN(rx, MIN_PROBE_X - 0.001, MAX_PROBE_X + 0.001)
-          && WITHIN(raw_y, MIN_PROBE_Y, MAX_PROBE_Y);
+          && WITHIN(ry, MIN_PROBE_Y - 0.001, MAX_PROBE_Y + 0.001);
   }
 
 #endif // CARTESIAN
 
-inline bool position_is_reachable_by_probe_xy( float target_x, float target_y ) {
+FORCE_INLINE bool position_is_reachable_by_probe_xy(const float &lx, const float &ly) {
-  return position_is_reachable_by_probe_raw_xy(
+  return position_is_reachable_by_probe_raw_xy(RAW_X_POSITION(lx), RAW_Y_POSITION(ly));
-            RAW_X_POSITION( target_x ),
-            RAW_Y_POSITION( target_y ));
 }
 
-inline bool position_is_reachable_xy( float target_x, float target_y ) {
+FORCE_INLINE bool position_is_reachable_xy(const float &lx, const float &ly) {
-  return position_is_reachable_raw_xy( RAW_X_POSITION( target_x ), RAW_Y_POSITION( target_y ));
+  return position_is_reachable_raw_xy(RAW_X_POSITION(lx), RAW_Y_POSITION(ly));
 }
 
 #endif //MARLIN_H

Marlin/Marlin_main.cpp

@@ -1684,7 +1684,7 @@ void do_blocking_move_to(const float &x, const float &y, const float &z, const f
 
   #if ENABLED(DELTA)
 
-    if ( ! position_is_reachable_xy( x, y )) return;
+    if (!position_is_reachable_xy(x, y)) return;
 
     feedrate_mm_s = fr_mm_s ? fr_mm_s : XY_PROBE_FEEDRATE_MM_S;
 
@@ -1740,7 +1740,7 @@ void do_blocking_move_to(const float &x, const float &y, const float &z, const f
 
   #elif IS_SCARA
 
-    if ( ! position_is_reachable_xy( x, y )) return;
+    if (!position_is_reachable_xy(x, y)) return;
 
     set_destination_to_current();
 
@@ -2366,7 +2366,7 @@ static void clean_up_after_endstop_or_probe_move() {
       }
     #endif
 
-    if ( ! position_is_reachable_by_probe_xy( x, y )) return NAN;
+    if (!position_is_reachable_by_probe_xy(x, y)) return NAN;
 
     const float old_feedrate_mm_s = feedrate_mm_s;
 
@@ -3713,7 +3713,7 @@ inline void gcode_G7(
       destination[Y_AXIS] -= Y_PROBE_OFFSET_FROM_EXTRUDER;
     #endif
 
-    if ( position_is_reachable_xy( destination[X_AXIS], destination[Y_AXIS] )) {
+    if (position_is_reachable_xy(destination[X_AXIS], destination[Y_AXIS])) {
 
       #if ENABLED(DEBUG_LEVELING_FEATURE)
         if (DEBUGGING(LEVELING)) DEBUG_POS("Z_SAFE_HOMING", destination);
@@ -4639,7 +4639,8 @@ void home_all_axes() { gcode_G28(true); }
             indexIntoAB[xCount][yCount] = abl_probe_index;
           #endif
 
-          if (position_is_reachable_xy( xProbe, yProbe )) break;
+          // Keep looping till a reachable point is found
+          if (position_is_reachable_xy(xProbe, yProbe)) break;
           ++abl_probe_index;
         }
 
@@ -4750,7 +4751,7 @@ void home_all_axes() { gcode_G28(true); }
 
             #if IS_KINEMATIC
               // Avoid probing outside the round or hexagonal area
-              if (!position_is_reachable_by_probe_xy( xProbe, yProbe )) continue;
+              if (!position_is_reachable_by_probe_xy(xProbe, yProbe)) continue;
             #endif
 
             measured_z = faux ? 0.001 * random(-100, 101) : probe_pt(xProbe, yProbe, stow_probe_after_each, verbose_level);
@@ -5055,7 +5056,7 @@ void home_all_axes() { gcode_G28(true); }
     const float xpos = code_seen('X') ? code_value_linear_units() : current_position[X_AXIS] + X_PROBE_OFFSET_FROM_EXTRUDER,
                 ypos = code_seen('Y') ? code_value_linear_units() : current_position[Y_AXIS] + Y_PROBE_OFFSET_FROM_EXTRUDER;
 
-    if (!position_is_reachable_by_probe_xy( xpos, ypos )) return;
+    if (!position_is_reachable_by_probe_xy(xpos, ypos)) return;
 
     // Disable leveling so the planner won't mess with us
     #if HAS_LEVELING
@@ -6513,7 +6514,7 @@ inline void gcode_M42() {
           #else
             // If we have gone out too far, we can do a simple fix and scale the numbers
             // back in closer to the origin.
-            while ( ! position_is_reachable_by_probe_xy( X_current, Y_current )) {
+            while (!position_is_reachable_by_probe_xy(X_current, Y_current)) {
               X_current *= 0.8;
               Y_current *= 0.8;
               if (verbose_level > 3) {

Marlin/ubl_G29.cpp

@@ -135,9 +135,9 @@
    *                    a subsequent G or T leveling operation for backward compatibility.
    *
    *   P1    Phase 1    Invalidate entire Mesh and continue with automatic generation of the Mesh data using
-   *                    the Z-Probe. Usually the probe can not reach all areas that the nozzle can reach.
+   *                    the Z-Probe. Usually the probe can't reach all areas that the nozzle can reach. On
-   *                    In Cartesian printers, mesh points within the X_OFFSET_FROM_EXTRUDER and Y_OFFSET_FROM_EXTRUDER
+   *                    Cartesian printers, points within the X_PROBE_OFFSET_FROM_EXTRUDER and Y_PROBE_OFFSET_FROM_EXTRUDER
-   *                    area can not be automatically probed.  For Delta printers the area in which DELTA_PROBEABLE_RADIUS
+   *                    area cannot be automatically probed. For Delta printers the area in which DELTA_PROBEABLE_RADIUS
    *                    and DELTA_PRINTABLE_RADIUS do not overlap will not be automatically probed.
    *
    *                    These points will be handled in Phase 2 and Phase 3. If the Phase 1 command is given the
@@ -497,7 +497,7 @@
 
           if (code_seen('H') && code_has_value()) height = code_value_float();
 
-          if ( !position_is_reachable_xy( x_pos, y_pos )) {
+          if (!position_is_reachable_xy(x_pos, y_pos)) {
             SERIAL_PROTOCOLLNPGM("(X,Y) outside printable radius.");
             return;
           }
@@ -660,9 +660,9 @@
           do_blocking_move_to_z(measured_z);
         } while (!ubl_lcd_clicked());
 
-        ubl.has_control_of_lcd_panel = true;   // There is a race condition for the Encoder Wheel getting clicked.
+        ubl.has_control_of_lcd_panel = true;   // There is a race condition for the encoder click.
                                                // It could get detected in lcd_mesh_edit (actually _lcd_mesh_fine_tune)
-                                               // or here. So, until we are done looking for a long Encoder Wheel Press,
+                                               // or here. So, until we are done looking for a long encoder press,
                                                // we need to take control of the panel
 
         KEEPALIVE_STATE(IN_HANDLER);
@@ -1346,10 +1346,10 @@
                       my = pgm_read_float(&ubl.mesh_index_to_ypos[j]);
 
           // If using the probe as the reference there are some unreachable locations.
-          // Also for round beds, there are grid points outside the bed that nozzle can't reach.
+          // Also for round beds, there are grid points outside the bed the nozzle can't reach.
           // Prune them from the list and ignore them till the next Phase (manual nozzle probing).
 
-          if ( ! (probe_as_reference ? position_is_reachable_by_probe_raw_xy(mx, my) : position_is_reachable_raw_xy(mx, my)) )
+          if (probe_as_reference ? !position_is_reachable_by_probe_raw_xy(mx, my) : !position_is_reachable_raw_xy(mx, my))
             continue;
 
           // Reachable. Check if it's the closest location to the nozzle.
@@ -1390,14 +1390,14 @@
   }
 
   void fine_tune_mesh(const float &lx, const float &ly, const bool do_ubl_mesh_map) {
-    if (!code_seen('R'))    // fine_tune_mesh() is special.  If no repetion count flag is specified
+    if (!code_seen('R'))    // fine_tune_mesh() is special. If no repetition count flag is specified
-      repetition_cnt = 1;   // we know to do exactly one mesh location. Otherwise we use what the parser decided.
+      repetition_cnt = 1;   // do exactly one mesh location. Otherwise use what the parser decided.
 
     mesh_index_pair location;
     uint16_t not_done[16];
     int32_t round_off;
 
-    if ( ! position_is_reachable_xy( lx, ly )) {
+    if (!position_is_reachable_xy(lx, ly)) {
       SERIAL_PROTOCOLLNPGM("(X,Y) outside printable radius.");
       return;
     }
@@ -1413,7 +1413,7 @@
     do {
       location = find_closest_mesh_point_of_type(SET_IN_BITMAP, lx, ly, USE_NOZZLE_AS_REFERENCE, not_done, false);
 
-      if (location.x_index < 0 ) break; // stop when we can't find any more reachable points.
+      if (location.x_index < 0) break; // stop when we can't find any more reachable points.
 
       bit_clear(not_done, location.x_index, location.y_index);  // Mark this location as 'adjusted' so we will find a
                                                                 // different location the next time through the loop
@@ -1421,9 +1421,8 @@
       const float rawx = pgm_read_float(&ubl.mesh_index_to_xpos[location.x_index]),
                   rawy = pgm_read_float(&ubl.mesh_index_to_ypos[location.y_index]);
 
-      if ( ! position_is_reachable_raw_xy( rawx, rawy )) { // SHOULD NOT OCCUR because find_closest_mesh_point_of_type will only return reachable
+      if (!position_is_reachable_raw_xy(rawx, rawy)) // SHOULD NOT OCCUR because find_closest_mesh_point_of_type will only return reachable
         break;
-      }
 
       float new_z = ubl.z_values[location.x_index][location.y_index];
 
@@ -1432,8 +1431,7 @@
         do_blocking_move_to_z(Z_CLEARANCE_DEPLOY_PROBE);    // Move the nozzle to where we are going to edit
         do_blocking_move_to_xy(LOGICAL_X_POSITION(rawx), LOGICAL_Y_POSITION(rawy));
 
-        round_off = (int32_t)(new_z * 1000.0);    // we chop off the last digits just to be clean. We are rounding to the
+        new_z = floor(new_z * 1000.0) * 0.001; // Chop off digits after the 1000ths place
-        new_z = float(round_off) / 1000.0;
 
         KEEPALIVE_STATE(PAUSED_FOR_USER);
         ubl.has_control_of_lcd_panel = true;
@@ -1451,9 +1449,9 @@
 
         lcd_return_to_status();
 
-        // There is a race condition for the Encoder Wheel getting clicked.
+        // The technique used here generates a race condition for the encoder click.
-        // It could get detected in lcd_mesh_edit (actually _lcd_mesh_fine_tune)
+        // It could get detected in lcd_mesh_edit (actually _lcd_mesh_fine_tune) or here.
-        // or here.
+        // Let's work on specifying a proper API for the LCD ASAP, OK?
         ubl.has_control_of_lcd_panel = true;
       }
 
@@ -1478,7 +1476,7 @@
 
       lcd_implementation_clear();
 
-    } while (( location.x_index >= 0 ) && (--repetition_cnt>0));
+    } while (location.x_index >= 0 && --repetition_cnt > 0);
 
     FINE_TUNE_EXIT: