Merge pull request #8747 from thinkyhead/bf2_sync_M420_M852

[2.0.x] M852 changes position. Position change reporting.

Marlin/src/feature/bedlevel/bedlevel.cpp

@@ -128,13 +128,17 @@ void set_bed_leveling_enabled(const bool enable/*=true*/) {
         // so compensation will give the right stepper counts.
         planner.unapply_leveling(current_position);
 
+      SYNC_PLAN_POSITION_KINEMATIC();
+
     #endif // OLDSCHOOL_ABL
   }
 }
 
 #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
 
-  void set_z_fade_height(const float zfh) {
+  void set_z_fade_height(const float zfh, const bool do_report/*=true*/) {
+
+    if (planner.z_fade_height == zfh) return; // do nothing if no change
 
     const bool level_active = planner.leveling_active;
 
@@ -145,6 +149,10 @@ void set_bed_leveling_enabled(const bool enable/*=true*/) {
     planner.set_z_fade_height(zfh);
 
     if (level_active) {
+      const float oldpos[XYZE] = {
+        current_position[X_AXIS], current_position[Y_AXIS],
+        current_position[Z_AXIS], current_position[E_AXIS]
+      };
       #if ENABLED(AUTO_BED_LEVELING_UBL)
         set_bed_leveling_enabled(true);  // turn back on after changing fade height
       #else
@@ -155,7 +163,10 @@ void set_bed_leveling_enabled(const bool enable/*=true*/) {
             Z_AXIS
           #endif
         );
+        SYNC_PLAN_POSITION_KINEMATIC();
       #endif
+      if (do_report && memcmp(oldpos, current_position, sizeof(oldpos)))
+        report_current_position();
     }
   }
 

Marlin/src/feature/bedlevel/bedlevel.h

@@ -47,7 +47,7 @@ void set_bed_leveling_enabled(const bool enable=true);
 void reset_bed_level();
 
 #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
-  void set_z_fade_height(const float zfh);
+  void set_z_fade_height(const float zfh, const bool do_report=true);
 #endif
 
 #if ENABLED(AUTO_BED_LEVELING_BILINEAR) || ENABLED(MESH_BED_LEVELING)

Marlin/src/feature/bedlevel/ubl/ubl.cpp

@@ -71,17 +71,19 @@
   volatile int unified_bed_leveling::encoder_diff;
 
   unified_bed_leveling::unified_bed_leveling() {
-    ubl_cnt++;  // Debug counter to insure we only have one UBL object present in memory.  We can eliminate this (and all references to ubl_cnt) very soon.
+    ubl_cnt++;  // Debug counter to ensure we only have one UBL object present in memory.  We can eliminate this (and all references to ubl_cnt) very soon.
     reset();
   }
 
   void unified_bed_leveling::reset() {
+    const bool was_enabled = planner.leveling_active;
     set_bed_leveling_enabled(false);
     storage_slot = -1;
     #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
       planner.set_z_fade_height(10.0);
     #endif
     ZERO(z_values);
+    if (was_enabled) report_current_position();
   }
 
   void unified_bed_leveling::invalidate() {

Marlin/src/gcode/bedlevel/M420.cpp

@@ -45,6 +45,11 @@
  */
 void GcodeSuite::M420() {
 
+  const float oldpos[XYZE] = {
+    current_position[X_AXIS], current_position[Y_AXIS],
+    current_position[Z_AXIS], current_position[E_AXIS]
+  };
+
   #if ENABLED(AUTO_BED_LEVELING_UBL)
 
     // L to load a mesh from the EEPROM
@@ -104,13 +109,16 @@ void GcodeSuite::M420() {
     #endif
   }
 
-  const bool to_enable = parser.boolval('S');
-  if (parser.seen('S')) set_bed_leveling_enabled(to_enable);
-
   #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
-    if (parser.seen('Z')) set_z_fade_height(parser.value_linear_units());
+    if (parser.seen('Z')) set_z_fade_height(parser.value_linear_units(), false);
   #endif
 
+  bool to_enable = false;
+  if (parser.seen('S')) {
+    to_enable = parser.value_bool();
+    set_bed_leveling_enabled(to_enable);
+  }
+
   const bool new_status = planner.leveling_active;
 
   if (to_enable && !new_status) {
@@ -129,6 +137,10 @@ void GcodeSuite::M420() {
     else
       SERIAL_ECHOLNPGM(MSG_OFF);
   #endif
+
+  // Report change in position
+  if (memcmp(oldpos, current_position, sizeof(oldpos)))
+    report_current_position();
 }
 
 #endif // HAS_LEVELING

Marlin/src/gcode/bedlevel/abl/G29.cpp

@@ -285,6 +285,7 @@ void GcodeSuite::G29() {
             bed_level_virt_interpolate();
           #endif
           set_bed_leveling_enabled(abl_should_enable);
+          if (abl_should_enable) report_current_position();
         }
         return;
       } // parser.seen('W')

Marlin/src/gcode/calibrate/M852.cpp

@@ -36,37 +36,47 @@
  *  K[yz_factor] - New YZ skew factor
  */
 void GcodeSuite::M852() {
-  const bool ijk = parser.seen('I') || parser.seen('S')
-    #if ENABLED(SKEW_CORRECTION_FOR_Z)
-      || parser.seen('J') || parser.seen('K')
-    #endif
-  ;
-  bool badval = false;
+  uint8_t ijk = 0, badval = 0, setval = 0;
 
   if (parser.seen('I') || parser.seen('S')) {
+    ++ijk;
     const float value = parser.value_linear_units();
-    if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX))
-      planner.xy_skew_factor = value;
+    if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX)) {
+      if (planner.xy_skew_factor != value) {
+        planner.xy_skew_factor = value;
+        ++setval;
+      }
+    }
     else
-      badval = true;
+      ++badval;
   }
 
   #if ENABLED(SKEW_CORRECTION_FOR_Z)
 
     if (parser.seen('J')) {
+      ++ijk;
       const float value = parser.value_linear_units();
-      if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX))
-        planner.xz_skew_factor = value;
+      if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX)) {
+        if (planner.xz_skew_factor != value) {
+          planner.xz_skew_factor = value;
+          ++setval;
+        }
+      }
       else
-        badval = true;
+        ++badval;
     }
 
     if (parser.seen('K')) {
+      ++ijk;
       const float value = parser.value_linear_units();
-      if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX))
-        planner.yz_skew_factor = value;
+      if (WITHIN(value, SKEW_FACTOR_MIN, SKEW_FACTOR_MAX)) {
+        if (planner.yz_skew_factor != value) {
+          planner.yz_skew_factor = value;
+          ++setval;
+        }
+      }
       else
-        badval = true;
+        ++badval;
     }
 
   #endif
@@ -74,6 +84,13 @@ void GcodeSuite::M852() {
   if (badval)
     SERIAL_ECHOLNPGM(MSG_SKEW_MIN " " STRINGIFY(SKEW_FACTOR_MIN) " " MSG_SKEW_MAX " " STRINGIFY(SKEW_FACTOR_MAX));
 
+  // When skew is changed the current position changes
+  if (setval) {
+    set_current_from_steppers_for_axis(ALL_AXES);
+    SYNC_PLAN_POSITION_KINEMATIC();
+    report_current_position();
+  }
+
   if (!ijk) {
     SERIAL_ECHO_START();
     SERIAL_ECHOPAIR(MSG_SKEW_FACTOR " XY: ", planner.xy_skew_factor);

Marlin/src/module/configuration_store.cpp

@@ -216,14 +216,15 @@ MarlinSettings settings;
   float new_z_fade_height;
 #endif
 
-#if ENABLED(CNC_COORDINATE_SYSTEMS)
-  bool position_changed;
-#endif
-
 /**
  * Post-process after Retrieve or Reset
  */
 void MarlinSettings::postprocess() {
+  const float oldpos[XYZE] = {
+    current_position[X_AXIS], current_position[Y_AXIS],
+    current_position[Z_AXIS], current_position[E_AXIS]
+  };
+
   // steps per s2 needs to be updated to agree with units per s2
   planner.reset_acceleration_rates();
 
@@ -233,10 +234,6 @@ void MarlinSettings::postprocess() {
     recalc_delta_settings();
   #endif
 
-  // Refresh steps_to_mm with the reciprocal of axis_steps_per_mm
-  // and init stepper.count[], planner.position[] with current_position
-  planner.refresh_positioning();
-
   #if ENABLED(PIDTEMP)
     thermalManager.updatePID();
   #endif
@@ -249,7 +246,7 @@ void MarlinSettings::postprocess() {
   #endif
 
   #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
-    set_z_fade_height(new_z_fade_height);
+    set_z_fade_height(new_z_fade_height, false); // false = no report
   #endif
 
   #if ENABLED(AUTO_BED_LEVELING_BILINEAR)
@@ -264,13 +261,14 @@ void MarlinSettings::postprocess() {
   #if ENABLED(FWRETRACT)
     fwretract.refresh_autoretract();
   #endif
+ 
+  // Refresh steps_to_mm with the reciprocal of axis_steps_per_mm
+  // and init stepper.count[], planner.position[] with current_position
+  planner.refresh_positioning();
 
-  #if ENABLED(CNC_COORDINATE_SYSTEMS)
-    if (position_changed) {
-      report_current_position();
-      position_changed = false;
-    }
-  #endif
+  // Various factors can change the current position
+  if (memcmp(oldpos, current_position, sizeof(oldpos)))
+    report_current_position();
 }
 
 #if ENABLED(EEPROM_SETTINGS)
@@ -308,7 +306,7 @@ void MarlinSettings::postprocess() {
     EEPROM_WRITE(ver);     // invalidate data first
     EEPROM_SKIP(working_crc); // Skip the checksum slot
 
-    working_crc = 0;  // Init to 0. Accumulated by EEPROM_READ
+    working_crc = 0; // clear before first "real data"
 
     const uint8_t esteppers = COUNT(planner.axis_steps_per_mm) - XYZ;
     EEPROM_WRITE(esteppers);
@@ -342,7 +340,7 @@ void MarlinSettings::postprocess() {
     #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
       const float zfh = planner.z_fade_height;
     #else
-      const float zfh = 0.0;
+      const float zfh = 10.0;
     #endif
     EEPROM_WRITE(zfh);
 
@@ -725,7 +723,7 @@ void MarlinSettings::postprocess() {
       float dummy = 0;
       bool dummyb;
 
-      working_crc = 0; //clear before reading first "real data"
+      working_crc = 0;  // Init to 0. Accumulated by EEPROM_READ
 
       // Number of esteppers may change
       uint8_t esteppers;
@@ -913,7 +911,6 @@ void MarlinSettings::postprocess() {
       #if DISABLED(ULTIPANEL)
         int lcd_preheat_hotend_temp[2], lcd_preheat_bed_temp[2], lcd_preheat_fan_speed[2];
       #endif
-
       EEPROM_READ(lcd_preheat_hotend_temp); // 2 floats
       EEPROM_READ(lcd_preheat_bed_temp);    // 2 floats
       EEPROM_READ(lcd_preheat_fan_speed);   // 2 floats
@@ -1094,7 +1091,7 @@ void MarlinSettings::postprocess() {
       //
 
       #if ENABLED(CNC_COORDINATE_SYSTEMS)
-        position_changed = gcode.select_coordinate_system(-1); // Go back to machine space
+        (void)gcode.select_coordinate_system(-1); // Go back to machine space
         EEPROM_READ(gcode.coordinate_system);                  // 27 floats
       #else
         for (uint8_t q = 27; q--;) EEPROM_READ(dummy);

Marlin/src/module/motion.cpp

@@ -211,6 +211,12 @@ void get_cartesian_from_steppers() {
  * Set the current_position for an axis based on
  * the stepper positions, removing any leveling that
  * may have been applied.
+ *
+ * To prevent small shifts in axis position always call
+ * SYNC_PLAN_POSITION_KINEMATIC after updating axes with this.
+ *
+ * To keep hosts in sync, always call report_current_position
+ * after updating the current_position.
  */
 void set_current_from_steppers_for_axis(const AxisEnum axis) {
   get_cartesian_from_steppers();