Extruder should ignore M114 subcodes > 0
[clinton/Smoothieware.git] / src / modules / robot / Robot.cpp
index 11d91ee..3c88f3d 100644 (file)
@@ -110,13 +110,14 @@ Robot::Robot()
     this->motion_mode =  MOTION_MODE_SEEK;
     this->select_plane(X_AXIS, Y_AXIS, Z_AXIS);
     clear_vector(this->last_milestone);
-    clear_vector(this->transformed_last_milestone);
+    clear_vector(this->last_machine_position);
     this->arm_solution = NULL;
     seconds_per_minute = 60.0F;
     this->clearToolOffset();
     this->compensationTransform = nullptr;
     this->wcs_offsets.fill(wcs_t(0.0F, 0.0F, 0.0F));
     this->g92_offset = wcs_t(0.0F, 0.0F, 0.0F);
+    this->next_command_is_MCS = false;
 }
 
 //Called when the module has just been loaded
@@ -223,7 +224,7 @@ void  Robot::push_state()
 {
     bool am = this->absolute_mode;
     bool im = this->inch_mode;
-    saved_state_t s(this->feed_rate, this->seek_rate, am, im);
+    saved_state_t s(this->feed_rate, this->seek_rate, am, im, current_wcs);
     state_stack.push(s);
 }
 
@@ -236,9 +237,78 @@ void Robot::pop_state()
         this->seek_rate = std::get<1>(s);
         this->absolute_mode = std::get<2>(s);
         this->inch_mode = std::get<3>(s);
+        this->current_wcs = std::get<4>(s);
     }
 }
 
+std::vector<Robot::wcs_t> Robot::get_wcs_state() const
+{
+    std::vector<wcs_t> v;
+    v.push_back(wcs_t(current_wcs, k_max_wcs, 0));
+    for(auto& i : wcs_offsets) {
+        v.push_back(i);
+    }
+    v.push_back(g92_offset);
+    v.push_back(tool_offset);
+    return v;
+}
+
+int Robot::print_position(uint8_t subcode, char *buf, size_t bufsize) const
+{
+    // M114.1 is a new way to do this (similar to how GRBL does it).
+    // it returns the realtime position based on the current step position of the actuators.
+    // this does require a FK to get a machine position from the actuator position
+    // and then invert all the transforms to get a workspace position from machine position
+    // M114 just does it the old way uses last_milestone and does inversse tranfroms to get the requested position
+    int n = 0;
+    if(subcode == 0) { // M114 print WCS
+        wcs_t pos= mcs2wcs(last_milestone);
+        n = snprintf(buf, bufsize, "C: X:%1.3f Y:%1.3f Z:%1.3f", from_millimeters(std::get<X_AXIS>(pos)), from_millimeters(std::get<Y_AXIS>(pos)), from_millimeters(std::get<Z_AXIS>(pos)));
+
+    } else if(subcode == 4) { // M114.3 print last milestone (which should be the same as machine position if axis are not moving and no level compensation)
+        n = snprintf(buf, bufsize, "LMS: X:%1.3f Y:%1.3f Z:%1.3f", last_milestone[X_AXIS], last_milestone[Y_AXIS], last_milestone[Z_AXIS]);
+
+    } else if(subcode == 5) { // M114.4 print last machine position (which should be the same as M114.1 if axis are not moving and no level compensation)
+        n = snprintf(buf, bufsize, "LMCS: X:%1.3f Y:%1.3f Z:%1.3f", last_machine_position[X_AXIS], last_machine_position[Y_AXIS], last_machine_position[Z_AXIS]);
+
+    } else {
+        // get real time positions
+        // current actuator position in mm
+        ActuatorCoordinates current_position{
+            actuators[X_AXIS]->get_current_position(),
+            actuators[Y_AXIS]->get_current_position(),
+            actuators[Z_AXIS]->get_current_position()
+        };
+
+        // get machine position from the actuator position using FK
+        float mpos[3];
+        arm_solution->actuator_to_cartesian(current_position, mpos);
+
+        if(subcode == 1) { // M114.1 print realtime WCS
+            // FIXME this currently includes the compensation transform which is incorrect so will be slightly off if it is in effect (but by very little)
+            wcs_t pos= mcs2wcs(mpos);
+            n = snprintf(buf, bufsize, "C: X:%1.3f Y:%1.3f Z:%1.3f", from_millimeters(std::get<X_AXIS>(pos)), from_millimeters(std::get<Y_AXIS>(pos)), from_millimeters(std::get<Z_AXIS>(pos)));
+
+        } else if(subcode == 2) { // M114.1 print realtime Machine coordinate system
+            n = snprintf(buf, bufsize, "MPOS: X:%1.3f Y:%1.3f Z:%1.3f", mpos[X_AXIS], mpos[Y_AXIS], mpos[Z_AXIS]);
+
+        } else if(subcode == 3) { // M114.2 print realtime actuator position
+            n = snprintf(buf, bufsize, "APOS: A:%1.3f B:%1.3f C:%1.3f", current_position[X_AXIS], current_position[Y_AXIS], current_position[Z_AXIS]);
+        }
+    }
+    return n;
+}
+
+// converts current last milestone (machine position without compensation transform) to work coordinate system (inverse transform)
+Robot::wcs_t Robot::mcs2wcs(const float *pos) const
+{
+    return std::make_tuple(
+        pos[X_AXIS] - std::get<X_AXIS>(wcs_offsets[current_wcs]) + std::get<X_AXIS>(g92_offset) - std::get<X_AXIS>(tool_offset),
+        pos[Y_AXIS] - std::get<Y_AXIS>(wcs_offsets[current_wcs]) + std::get<Y_AXIS>(g92_offset) - std::get<Y_AXIS>(tool_offset),
+        pos[Z_AXIS] - std::get<Z_AXIS>(wcs_offsets[current_wcs]) + std::get<Z_AXIS>(g92_offset) - std::get<Z_AXIS>(tool_offset)
+    );
+}
+
 // this does a sanity check that actuator speeds do not exceed steps rate capability
 // we will override the actuator max_rate if the combination of max_rate and steps/sec exceeds base_stepping_frequency
 void Robot::check_max_actuator_speeds()
@@ -260,14 +330,13 @@ void Robot::on_gcode_received(void *argument)
 
     this->motion_mode = -1;
 
-    //G-letter Gcodes are mostly what the Robot module is interrested in, other modules also catch the gcode event and do stuff accordingly
     if( gcode->has_g) {
         switch( gcode->g ) {
-            case 0:  this->motion_mode = MOTION_MODE_SEEK;  break;
-            case 1:  this->motion_mode = MOTION_MODE_LINEAR;   break;
-            case 2:  this->motion_mode = MOTION_MODE_CW_ARC;   break;
-            case 3:  this->motion_mode = MOTION_MODE_CCW_ARC;   break;
-            case 4: {
+            case 0:  this->motion_mode = MOTION_MODE_SEEK;    break;
+            case 1:  this->motion_mode = MOTION_MODE_LINEAR;  break;
+            case 2:  this->motion_mode = MOTION_MODE_CW_ARC;  break;
+            case 3:  this->motion_mode = MOTION_MODE_CCW_ARC; break;
+            case 4: { // G4 pause
                 uint32_t delay_ms = 0;
                 if (gcode->has_letter('P')) {
                     delay_ms = gcode->get_int('P');
@@ -282,23 +351,42 @@ void Robot::on_gcode_received(void *argument)
                     uint32_t start = us_ticker_read(); // mbed call
                     while ((us_ticker_read() - start) < delay_ms * 1000) {
                         THEKERNEL->call_event(ON_IDLE, this);
+                        if(THEKERNEL->is_halted()) return;
                     }
                 }
             }
             break;
 
-            case 10: // G10 L2 Pn Xn Yn Zn set WCS
-                // TODO implement G10 L20
-                if(gcode->has_letter('L') && gcode->get_int('L') == 2 && gcode->has_letter('P')) {
+            case 10: // G10 L2 [L20] Pn Xn Yn Zn set WCS
+                if(gcode->has_letter('L') && (gcode->get_int('L') == 2 || gcode->get_int('L') == 20) && gcode->has_letter('P')) {
                     size_t n = gcode->get_uint('P');
                     if(n == 0) n = current_wcs; // set current coordinate system
                     else --n;
                     if(n < k_max_wcs) {
                         float x, y, z;
                         std::tie(x, y, z) = wcs_offsets[n];
-                        if(gcode->has_letter('X')) x = this->to_millimeters(gcode->get_value('X'));
-                        if(gcode->has_letter('Y')) y = this->to_millimeters(gcode->get_value('Y'));
-                        if(gcode->has_letter('Z')) z = this->to_millimeters(gcode->get_value('Z'));
+                        if(gcode->get_int('L') == 20) {
+                            // this makes the current machine position (less compensation transform) the offset
+                            // get current position in WCS
+                            wcs_t pos= mcs2wcs(last_milestone);
+
+                            if(gcode->has_letter('X')){
+                                x -= to_millimeters(gcode->get_value('X')) - std::get<X_AXIS>(pos);
+                            }
+
+                            if(gcode->has_letter('Y')){
+                                y -= to_millimeters(gcode->get_value('Y')) - std::get<Y_AXIS>(pos);
+                            }
+                            if(gcode->has_letter('Z')) {
+                                z -= to_millimeters(gcode->get_value('Z')) - std::get<Z_AXIS>(pos);
+                            }
+
+                        } else {
+                            // the value is the offset from machine zero
+                            if(gcode->has_letter('X')) x = to_millimeters(gcode->get_value('X'));
+                            if(gcode->has_letter('Y')) y = to_millimeters(gcode->get_value('Y'));
+                            if(gcode->has_letter('Z')) z = to_millimeters(gcode->get_value('Z'));
+                        }
                         wcs_offsets[n] = wcs_t(x, y, z);
                     }
                 }
@@ -323,45 +411,30 @@ void Robot::on_gcode_received(void *argument)
             case 91: this->absolute_mode = false;   break;
 
             case 92: {
-                if(gcode->subcode == 1 || gcode->subcode == 2 || gcode->get_num_args() == 0) {
+                 if(gcode->subcode == 1 || gcode->subcode == 2 || gcode->get_num_args() == 0) {
                     // reset G92 offsets to 0
                     g92_offset = wcs_t(0, 0, 0);
 
-                } else if(gcode->subcode == 3) {
-                    // non-standard - set machine coordinates (like old G92 would do) basically a manual home
-                    if(gcode->get_num_args() == 0) {
-                        // sets current position to 0,0,0 in machine coordinates
-                        for (int i = X_AXIS; i <= Z_AXIS; ++i) reset_axis_position(0, i);
-
-                        // now we need to clear the g92 offsets, and set last_milestone
-                        // TODO is this correct? What about the compensationTransform? do we need the dreaded inverseCompensation Transform?
-                        g92_offset = wcs_t(0, 0, 0);
-                        this->last_milestone[0] -= std::get<0>(wcs_offsets[current_wcs]);
-                        this->last_milestone[1] -= std::get<1>(wcs_offsets[current_wcs]);
-                        this->last_milestone[2] -= std::get<2>(wcs_offsets[current_wcs]);
-
-                    } else {
-                        // sets the given axis to the provided value
-                        float x, y, z;
-                        std::tie(x, y, z) = g92_offset;
-                        if(gcode->has_letter('X')) { reset_axis_position(this->to_millimeters(gcode->get_value('X')), X_AXIS); this->last_milestone[0] -= std::get<0>(wcs_offsets[current_wcs]); x= 0; }
-                        if(gcode->has_letter('Y')) { reset_axis_position(this->to_millimeters(gcode->get_value('Y')), Y_AXIS); this->last_milestone[1] -= std::get<1>(wcs_offsets[current_wcs]); y= 0; }
-                        if(gcode->has_letter('Z')) { reset_axis_position(this->to_millimeters(gcode->get_value('Z')), Z_AXIS); this->last_milestone[2] -= std::get<2>(wcs_offsets[current_wcs]); z= 0; }
-
-                        // reset the g92 offset
-                        g92_offset = wcs_t(x, y, z);
-                    }
-
-
                 } else {
-                    // standard setting of the g92 offsets, making current position whatever the coordinate arguments are
+                    // standard setting of the g92 offsets, making current WCS position whatever the coordinate arguments are
                     float x, y, z;
                     std::tie(x, y, z) = g92_offset;
-                    if(gcode->has_letter('X')) x = last_milestone[0] - to_millimeters(gcode->get_value('X'));
-                    if(gcode->has_letter('Y')) y = last_milestone[1] - to_millimeters(gcode->get_value('Y'));
-                    if(gcode->has_letter('Z')) z = last_milestone[2] - to_millimeters(gcode->get_value('Z'));
+                    // get current position in WCS
+                    wcs_t pos= mcs2wcs(last_milestone);
+
+                    // adjust g92 offset to make the current wpos == the value requested
+                    if(gcode->has_letter('X')){
+                        x += to_millimeters(gcode->get_value('X')) - std::get<X_AXIS>(pos);
+                    }
+                    if(gcode->has_letter('Y')){
+                        y += to_millimeters(gcode->get_value('Y')) - std::get<Y_AXIS>(pos);
+                    }
+                    if(gcode->has_letter('Z')) {
+                        z += to_millimeters(gcode->get_value('Z')) - std::get<Z_AXIS>(pos);
+                    }
                     g92_offset = wcs_t(x, y, z);
                 }
+
                 return;
             }
         }
@@ -371,7 +444,6 @@ void Robot::on_gcode_received(void *argument)
             case 2: // M2 end of program
                 current_wcs = 0;
                 absolute_mode = true;
-                motion_mode = MOTION_MODE_LINEAR; // feed
                 break;
 
             case 92: // M92 - set steps per mm
@@ -389,32 +461,12 @@ void Robot::on_gcode_received(void *argument)
                 check_max_actuator_speeds();
                 return;
 
-            case 114: {
+            case 114:{
                 char buf[64];
-                int n = 0;
-                if(gcode->subcode == 0) { // M114 print WCS
-                    n = snprintf(buf, sizeof(buf), "C: X:%1.3f Y:%1.3f Z:%1.3f",
-                                 from_millimeters(this->last_milestone[0]),
-                                 from_millimeters(this->last_milestone[1]),
-                                 from_millimeters(this->last_milestone[2]));
-
-                } else if(gcode->subcode == 1) { // M114.1 print Machine coordinate system
-                    // TODO figure this out
-                    n = snprintf(buf, sizeof(buf), "X:%1.3f Y:%1.3f Z:%1.3f",
-                                 from_millimeters(this->last_milestone[0]) + (std::get<0>(wcs_offsets[current_wcs]) + std::get<0>(g92_offset)),
-                                 from_millimeters(this->last_milestone[1]) + (std::get<1>(wcs_offsets[current_wcs]) + std::get<1>(g92_offset)),
-                                 from_millimeters(this->last_milestone[2]) + (std::get<2>(wcs_offsets[current_wcs]) + std::get<2>(g92_offset)) );
-
-                } else if(gcode->subcode == 2) { // M114.2 print realtime actuator position
-                    n = snprintf(buf, sizeof(buf), "A:%1.3f B:%1.3f C:%1.3f",
-                                 actuators[X_AXIS]->get_current_position(),
-                                 actuators[Y_AXIS]->get_current_position(),
-                                 actuators[Z_AXIS]->get_current_position() );
-                }
-                if(n > 0)
-                    gcode->txt_after_ok.append(buf, n);
+                int n= print_position(gcode->subcode, buf, sizeof buf);
+                if(n > 0) gcode->txt_after_ok.append(buf, n);
+                return;
             }
-            return;
 
             case 120: // push state
                 push_state();
@@ -536,9 +588,9 @@ void Robot::on_gcode_received(void *argument)
                 // save wcs_offsets and current_wcs
                 // TODO this may need to be done whenever they change to be compliant
                 gcode->stream->printf(";WCS settings\n");
-                gcode->stream->printf("G5%c", std::min(current_wcs, (uint8_t)(5 + '4')));
+                gcode->stream->printf("G5%c", std::min(current_wcs, (uint8_t)5) + '4');
                 if(current_wcs >= 6) {
-                    gcode->stream->printf(".%c\n",  '1' + (current_wcs - 5));
+                    gcode->stream->printf(".%c\n",  '1' + (current_wcs - 6));
                 } else {
                     gcode->stream->printf("\n");
                 }
@@ -555,6 +607,7 @@ void Robot::on_gcode_received(void *argument)
 
             if(gcode->m == 503) {
                 // just print the G92 setting as it is not saved
+                // TODO linuxcnc does seem to save G92, so maybe we should here too
                 if(g92_offset != wcs_t(0, 0, 0)) {
                     float x, y, z;
                     std::tie(x, y, z) = g92_offset;
@@ -597,23 +650,60 @@ void Robot::on_gcode_received(void *argument)
         }
     }
 
-    if( this->motion_mode < 0)
-        return;
+    if( this->motion_mode >= 0) {
+        process_move(gcode);
+    }
 
-//Get parameters
-    float target[3], offset[3];
-    clear_vector(offset);
+    next_command_is_MCS = false; // must be on same line as G0 or G1
+}
 
-    memcpy(target, this->last_milestone, sizeof(target));    //default to last target
+// process a G0/G1/G2/G3
+void Robot::process_move(Gcode *gcode)
+{
+    // we have a G0/G1/G2/G3 so extract parameters and apply offsets to get machine coordinate target
+    float param[3]{NAN, NAN, NAN};
+    for(int i= X_AXIS; i <= Z_AXIS; ++i) {
+        char letter= 'X'+i;
+        if( gcode->has_letter(letter) ) {
+            param[i] = this->to_millimeters(gcode->get_value(letter));
+        }
+    }
 
+    float offset[3]{0,0,0};
     for(char letter = 'I'; letter <= 'K'; letter++) {
         if( gcode->has_letter(letter) ) {
             offset[letter - 'I'] = this->to_millimeters(gcode->get_value(letter));
         }
     }
-    for(char letter = 'X'; letter <= 'Z'; letter++) {
-        if( gcode->has_letter(letter) ) {
-            target[letter - 'X'] = this->to_millimeters(gcode->get_value(letter)) + (this->absolute_mode ? this->toolOffset[letter - 'X'] : target[letter - 'X']);
+
+    // calculate target in machine coordinates (less compensation transform which needs to be done after segmentation)
+    float target[3]{last_milestone[X_AXIS], last_milestone[Y_AXIS], last_milestone[Z_AXIS]};
+    if(!next_command_is_MCS) {
+        if(this->absolute_mode) {
+            // apply wcs offsets and g92 offset and tool offset
+            if(!isnan(param[X_AXIS])) {
+                target[X_AXIS]= param[X_AXIS] + std::get<X_AXIS>(wcs_offsets[current_wcs]) - std::get<X_AXIS>(g92_offset) + std::get<X_AXIS>(tool_offset);
+            }
+
+            if(!isnan(param[Y_AXIS])) {
+                target[Y_AXIS]= param[Y_AXIS] + std::get<Y_AXIS>(wcs_offsets[current_wcs]) - std::get<Y_AXIS>(g92_offset) + std::get<Y_AXIS>(tool_offset);
+            }
+
+            if(!isnan(param[Z_AXIS])) {
+                target[Z_AXIS]= param[Z_AXIS] + std::get<Z_AXIS>(wcs_offsets[current_wcs]) - std::get<Z_AXIS>(g92_offset) + std::get<Z_AXIS>(tool_offset);
+            }
+
+        }else{
+            // they are deltas from the last_milestone if specified
+            for(int i= X_AXIS; i <= Z_AXIS; ++i) {
+                if(!isnan(param[i])) target[i] = param[i] + last_milestone[i];
+            }
+        }
+
+    }else{
+        // already in machine coordinates, we do not add tool offset for that
+        for(int i= X_AXIS; i <= Z_AXIS; ++i) {
+            if(!isnan(param[i])) target[i] = param[i];
         }
     }
 
@@ -624,17 +714,27 @@ void Robot::on_gcode_received(void *argument)
             this->feed_rate = this->to_millimeters( gcode->get_value('F') );
     }
 
-//Perform any physical actions
+    bool moved= false;
+    //Perform any physical actions
     switch(this->motion_mode) {
-        case MOTION_MODE_CANCEL: break;
-        case MOTION_MODE_SEEK  : this->append_line(gcode, target, this->seek_rate / seconds_per_minute ); break;
-        case MOTION_MODE_LINEAR: this->append_line(gcode, target, this->feed_rate / seconds_per_minute ); break;
+        case MOTION_MODE_CANCEL:
+            break;
+        case MOTION_MODE_SEEK:
+            moved= this->append_line(gcode, target, this->seek_rate / seconds_per_minute );
+            break;
+        case MOTION_MODE_LINEAR:
+            moved= this->append_line(gcode, target, this->feed_rate / seconds_per_minute );
+            break;
         case MOTION_MODE_CW_ARC:
-        case MOTION_MODE_CCW_ARC: this->compute_arc(gcode, offset, target ); break;
+        case MOTION_MODE_CCW_ARC:
+            moved= this->compute_arc(gcode, offset, target );
+            break;
     }
 
-// last_milestone was set to target in append_milestone, no need to do it again
-
+    if(moved) {
+        // set last_milestone to the calculated target
+        memcpy(this->last_milestone, target, sizeof(this->last_milestone));
+    }
 }
 
 // We received a new gcode, and one of the functions
@@ -646,65 +746,53 @@ void Robot::distance_in_gcode_is_known(Gcode * gcode)
     THEKERNEL->conveyor->append_gcode(gcode);
 }
 
-// reset the position for all axis (used in homing for delta as last_milestone may be bogus)
+// reset the machine position for all axis. Used for homing.
+// During homing compensation is turned off (actually not used as it drives steppers directly)
+// once homed and reset_axis called compensation is used for the move to origin and back off home if enabled,
+// so in those cases the final position is compensated.
 void Robot::reset_axis_position(float x, float y, float z)
 {
-    this->last_milestone[X_AXIS] = x;
-    this->last_milestone[Y_AXIS] = y;
-    this->last_milestone[Z_AXIS] = z;
-    this->transformed_last_milestone[X_AXIS] = x;
-    this->transformed_last_milestone[Y_AXIS] = y;
-    this->transformed_last_milestone[Z_AXIS] = z;
+    // these are set to the same as compensation was not used to get to the current position
+    last_machine_position[X_AXIS]= last_milestone[X_AXIS] = x;
+    last_machine_position[Y_AXIS]= last_milestone[Y_AXIS] = y;
+    last_machine_position[Z_AXIS]= last_milestone[Z_AXIS] = z;
 
+    // now set the actuator positions to match
     ActuatorCoordinates actuator_pos;
-    arm_solution->cartesian_to_actuator(this->last_milestone, actuator_pos);
+    arm_solution->cartesian_to_actuator(this->last_machine_position, actuator_pos);
     for (size_t i = 0; i < actuators.size(); i++)
         actuators[i]->change_last_milestone(actuator_pos[i]);
-
-    // we also reset G92 offsets
-    g92_offset = wcs_t(0, 0, 0);
 }
 
 // Reset the position for an axis (used in homing)
 void Robot::reset_axis_position(float position, int axis)
 {
-    this->last_milestone[axis] = position;
-    this->transformed_last_milestone[axis] = position;
-
-    ActuatorCoordinates actuator_pos;
-    arm_solution->cartesian_to_actuator(this->last_milestone, actuator_pos);
-
-    for (size_t i = 0; i < actuators.size(); i++){
-        actuators[i]->change_last_milestone(actuator_pos[i]);
-    }
-
-    // we also reset G92 offsets
-    switch(axis) {
-        case X_AXIS: std::get<0>(g92_offset)= 0; break;
-        case Y_AXIS: std::get<1>(g92_offset)= 0; break;
-        case Z_AXIS: std::get<2>(g92_offset)= 0; break;
-    }
-
+    last_milestone[axis] = position;
+    reset_axis_position(last_milestone[X_AXIS], last_milestone[Y_AXIS], last_milestone[Z_AXIS]);
 }
 
-// Use FK to find out where actuator is and reset lastmilestone to match
+// Use FK to find out where actuator is and reset to match
 void Robot::reset_position_from_current_actuator_position()
 {
     ActuatorCoordinates actuator_pos;
     for (size_t i = 0; i < actuators.size(); i++) {
         actuator_pos[i] = actuators[i]->get_current_position();
     }
-    arm_solution->actuator_to_cartesian(actuator_pos, this->last_milestone);
-    memcpy(this->transformed_last_milestone, this->last_milestone, sizeof(this->transformed_last_milestone));
+    arm_solution->actuator_to_cartesian(actuator_pos, last_machine_position);
+    // FIXME problem is this includes any compensation transform, and without an inverse compensation we cannot get a correct last_milestone
+    memcpy(last_milestone, last_machine_position, sizeof last_milestone);
 
     // now reset actuator correctly, NOTE this may lose a little precision
-    arm_solution->cartesian_to_actuator(this->last_milestone, actuator_pos);
-    for (size_t i = 0; i < actuators.size(); i++)
-        actuators[i]->change_last_milestone(actuator_pos[i]);
+    // NOTE I do not recall why this was necessary, maybe to correct for small errors in FK
+    // arm_solution->cartesian_to_actuator(last_machine_position, actuator_pos);
+    // for (size_t i = 0; i < actuators.size(); i++)
+    //     actuators[i]->change_last_milestone(actuator_pos[i]);
 }
 
-// Convert target from millimeters to steps, and append this to the planner
-void Robot::append_milestone(Gcode * gcode, float target[], float rate_mm_s)
+// Convert target (in machine coordinates) from millimeters to steps, and append this to the planner
+// target is in machine coordinates without the compensation transform, however we save a last_machine_position that includes
+// all transforms and is what we actually convert to actuator positions
+bool Robot::append_milestone(Gcode * gcode, const float target[], float rate_mm_s)
 {
     float deltas[3];
     float unit_vec[3];
@@ -721,21 +809,21 @@ void Robot::append_milestone(Gcode * gcode, float target[], float rate_mm_s)
         compensationTransform(transformed_target);
     }
 
-    // apply wcs offsets and g92 offset
-    transformed_target[0] += (std::get<0>(wcs_offsets[current_wcs]) + std::get<0>(g92_offset));
-    transformed_target[1] += (std::get<1>(wcs_offsets[current_wcs]) + std::get<1>(g92_offset));
-    transformed_target[2] += (std::get<2>(wcs_offsets[current_wcs]) + std::get<2>(g92_offset));
-
-    // find distance moved by each axis, use transformed target from last_transformed_target
+    // find distance moved by each axis, use transformed target from the current machine position
     for (int axis = X_AXIS; axis <= Z_AXIS; axis++) {
-        deltas[axis] = transformed_target[axis] - transformed_last_milestone[axis];
+        deltas[axis] = transformed_target[axis] - last_machine_position[axis];
     }
-    // store last transformed
-    memcpy(this->transformed_last_milestone, transformed_target, sizeof(this->transformed_last_milestone));
 
     // Compute how long this move moves, so we can attach it to the block for later use
     millimeters_of_travel = sqrtf( powf( deltas[X_AXIS], 2 ) +  powf( deltas[Y_AXIS], 2 ) +  powf( deltas[Z_AXIS], 2 ) );
 
+    // it is unlikely but we need to protect against divide by zero, so ignore insanely small moves here
+    // as the last milestone won't be updated we do not actually lose any moves as they will be accounted for in the next move
+    if(millimeters_of_travel < 0.00001F) return false;
+
+    // this is the machine position
+    memcpy(this->last_machine_position, transformed_target, sizeof(this->last_machine_position));
+
     // find distance unit vector
     for (int i = 0; i < 3; i++)
         unit_vec[i] = deltas[i] / millimeters_of_travel;
@@ -750,8 +838,8 @@ void Robot::append_milestone(Gcode * gcode, float target[], float rate_mm_s)
         }
     }
 
-    // find actuator position given cartesian position, use actual adjusted target
-    arm_solution->cartesian_to_actuator( transformed_target, actuator_pos );
+    // find actuator position given the machine position, use actual adjusted target
+    arm_solution->cartesian_to_actuator( this->last_machine_position, actuator_pos );
 
     float isecs = rate_mm_s / millimeters_of_travel;
     // check per-actuator speed limits
@@ -766,22 +854,18 @@ void Robot::append_milestone(Gcode * gcode, float target[], float rate_mm_s)
     // Append the block to the planner
     THEKERNEL->planner->append_block( actuator_pos, rate_mm_s, millimeters_of_travel, unit_vec );
 
-    // Update the last_milestone to the current target for the next time we use last_milestone, use the requested target not the adjusted one
-    memcpy(this->last_milestone, target, sizeof(this->last_milestone)); // this->last_milestone[] = target[];
-
+    return true;
 }
 
 // Append a move to the queue ( cutting it into segments if needed )
-void Robot::append_line(Gcode * gcode, float target[], float rate_mm_s )
+bool Robot::append_line(Gcode *gcode, const float target[], float rate_mm_s )
 {
-    // Find out the distance for this gcode
+    // Find out the distance for this move in MCS
     // NOTE we need to do sqrt here as this setting of millimeters_of_travel is used by extruder and other modules even if there is no XYZ move
-    gcode->millimeters_of_travel = sqrtf(powf( target[X_AXIS] - this->last_milestone[X_AXIS], 2 ) +  powf( target[Y_AXIS] - this->last_milestone[Y_AXIS], 2 ) +  powf( target[Z_AXIS] - this->last_milestone[Z_AXIS], 2 ));
+    gcode->millimeters_of_travel = sqrtf(powf( target[X_AXIS] - last_milestone[X_AXIS], 2 ) +  powf( target[Y_AXIS] - last_milestone[Y_AXIS], 2 ) +  powf( target[Z_AXIS] - last_milestone[Z_AXIS], 2 ));
 
     // We ignore non- XYZ moves ( for example, extruder moves are not XYZ moves )
-    if( gcode->millimeters_of_travel < 0.00001F ) {
-        return;
-    }
+    if( gcode->millimeters_of_travel < 0.00001F ) return false;
 
     // Mark the gcode as having a known distance
     this->distance_in_gcode_is_known( gcode );
@@ -802,8 +886,7 @@ void Robot::append_line(Gcode * gcode, float target[], float rate_mm_s )
         }
     }
 
-    // We cut the line into smaller segments. This is not usefull in a cartesian robot, but necessary for robots with rotational axes.
-    // In cartesian robot, a high "mm_per_line_segment" setting will prevent waste.
+    // We cut the line into smaller segments. This is only needed on a cartesian robot for zgrid, but always necessary for robots with rotational axes like Deltas.
     // In delta robots either mm_per_line_segment can be used OR delta_segments_per_second
     // The latter is more efficient and avoids splitting fast long lines into very small segments, like initial z move to 0, it is what Johanns Marlin delta port does
     uint16_t segments;
@@ -825,10 +908,11 @@ void Robot::append_line(Gcode * gcode, float target[], float rate_mm_s )
         }
     }
 
+    bool moved= false;
     if (segments > 1) {
         // A vector to keep track of the endpoint of each segment
         float segment_delta[3];
-        float segment_end[3];
+        float segment_end[3]{last_milestone[X_AXIS], last_milestone[Y_AXIS], last_milestone[Z_AXIS]};
 
         // How far do we move each segment?
         for (int i = X_AXIS; i <= Z_AXIS; i++)
@@ -837,25 +921,32 @@ void Robot::append_line(Gcode * gcode, float target[], float rate_mm_s )
         // segment 0 is already done - it's the end point of the previous move so we start at segment 1
         // We always add another point after this loop so we stop at segments-1, ie i < segments
         for (int i = 1; i < segments; i++) {
-            if(THEKERNEL->is_halted()) return; // don't queue any more segments
+            if(THEKERNEL->is_halted()) return false; // don't queue any more segments
             for(int axis = X_AXIS; axis <= Z_AXIS; axis++ )
-                segment_end[axis] = last_milestone[axis] + segment_delta[axis];
+                segment_end[axis] += segment_delta[axis];
 
             // Append the end of this segment to the queue
-            this->append_milestone(gcode, segment_end, rate_mm_s);
+            bool b= this->append_milestone(gcode, segment_end, rate_mm_s);
+            moved= moved || b;
         }
     }
 
     // Append the end of this full move to the queue
-    this->append_milestone(gcode, target, rate_mm_s);
+    if(this->append_milestone(gcode, target, rate_mm_s)) moved= true;
 
-    // if adding these blocks didn't start executing, do that now
-    THEKERNEL->conveyor->ensure_running();
+    this->next_command_is_MCS = false; // always reset this
+
+    if(moved) {
+        // if adding these blocks didn't start executing, do that now
+        THEKERNEL->conveyor->ensure_running();
+    }
+
+    return moved;
 }
 
 
 // Append an arc to the queue ( cutting it into segments as needed )
-void Robot::append_arc(Gcode * gcode, float target[], float offset[], float radius, bool is_clockwise )
+bool Robot::append_arc(Gcode * gcode, const float target[], const float offset[], float radius, bool is_clockwise )
 {
 
     // Scary math
@@ -881,7 +972,7 @@ void Robot::append_arc(Gcode * gcode, float target[], float offset[], float radi
 
     // We don't care about non-XYZ moves ( for example the extruder produces some of those )
     if( gcode->millimeters_of_travel < 0.00001F ) {
-        return;
+        return false;
     }
 
     // Mark the gcode as having a known distance
@@ -930,8 +1021,9 @@ void Robot::append_arc(Gcode * gcode, float target[], float offset[], float radi
     // Initialize the linear axis
     arc_target[this->plane_axis_2] = this->last_milestone[this->plane_axis_2];
 
+    bool moved= false;
     for (i = 1; i < segments; i++) { // Increment (segments-1)
-        if(THEKERNEL->is_halted()) return; // don't queue any more segments
+        if(THEKERNEL->is_halted()) return false; // don't queue any more segments
 
         if (count < this->arc_correction ) {
             // Apply vector rotation matrix
@@ -955,16 +1047,18 @@ void Robot::append_arc(Gcode * gcode, float target[], float offset[], float radi
         arc_target[this->plane_axis_2] += linear_per_segment;
 
         // Append this segment to the queue
-        this->append_milestone(gcode, arc_target, this->feed_rate / seconds_per_minute);
-
+        bool b= this->append_milestone(gcode, arc_target, this->feed_rate / seconds_per_minute);
+        moved= moved || b;
     }
 
     // Ensure last segment arrives at target location.
-    this->append_milestone(gcode, target, this->feed_rate / seconds_per_minute);
+    if(this->append_milestone(gcode, target, this->feed_rate / seconds_per_minute)) moved= true;
+
+    return moved;
 }
 
 // Do the math for an arc and add it to the queue
-void Robot::compute_arc(Gcode * gcode, float offset[], float target[])
+bool Robot::compute_arc(Gcode * gcode, const float offset[], const float target[])
 {
 
     // Find the radius
@@ -977,8 +1071,7 @@ void Robot::compute_arc(Gcode * gcode, float offset[], float target[])
     }
 
     // Append arc
-    this->append_arc(gcode, target, offset,  radius, is_clockwise );
-
+    return this->append_arc(gcode, target, offset,  radius, is_clockwise );
 }
 
 
@@ -1005,11 +1098,11 @@ void Robot::select_plane(uint8_t axis_0, uint8_t axis_1, uint8_t axis_2)
 
 void Robot::clearToolOffset()
 {
-    memset(this->toolOffset, 0, sizeof(this->toolOffset));
+    this->tool_offset= wcs_t(0,0,0);
 }
 
 void Robot::setToolOffset(const float offset[3])
 {
-    memcpy(this->toolOffset, offset, sizeof(this->toolOffset));
+    this->tool_offset= wcs_t(offset[0], offset[1], offset[2]);
 }