Fix problem with "run from processor" (#2266)

Fixed problem with the "run from processor" where it did not use the original line number to skip commands. It now stores the gcode state for skipped lines and applies it once the selected line number is being processed.

Also fixed a problem with the "arc expander processor" applying the wrong feed rate.

ugs-core/src/com/willwinder/universalgcodesender/gcode/processors/ArcExpander.java

@@ -73,7 +73,7 @@ public List<String> processCommand(String command, GcodeState state) throws Gcod
 
         List<String> results = new ArrayList<>();
 
-        List<GcodeMeta> commands = GcodeParserUtils.processCommand(command, 0, state);
+        List<GcodeMeta> commands = GcodeParserUtils.processCommand(command, 0, state, true);
 
         // If this is not an arc, there is nothing to do.
         Code c = hasArcCommand(commands);
@@ -100,13 +100,9 @@ public List<String> processCommand(String command, GcodeState state) throws Gcod
         points.remove(0);
 
         if (convertToLines) {
-            // Tack the speed onto the first line segment in case the arc also
-            // changed the feed value.
-            String feed = "F" + arcMeta.point.getFeedRate();
             for (Position point : points) {
-                results.add(GcodePreprocessorUtils.generateLineFromPoints(G1, start, point, state.inAbsoluteMode, df) + feed);
+                results.add(GcodePreprocessorUtils.generateLineFromPoints(G1, start, point, state.inAbsoluteMode, df));
                 start = point;
-                feed = "";
             }
         } else {
             // TODO: Generate arc segments.

ugs-core/src/com/willwinder/universalgcodesender/gcode/processors/CommandProcessorList.java

@@ -48,7 +48,7 @@ public List<String> processCommand(String command, final GcodeState initialState
                 List<String> intermediate = p.processCommand(ret.remove(0), tempState);
 
                 // process results to update the state and collect PointSegments
-                for(String c : intermediate) {
+                for (String c : intermediate) {
                     tempState = testState(c, tempState);
                 }
 
@@ -71,7 +71,7 @@ static private GcodeState testState(String command, GcodeState state) throws Gco
         GcodeState ret = state;
 
         // Add command get meta doesn't update the state, so we need to do that manually.
-        Collection<GcodeParser.GcodeMeta> metaObjects = GcodeParserUtils.processCommand(command, 0, state);
+        Collection<GcodeParser.GcodeMeta> metaObjects = GcodeParserUtils.processCommand(command, state.commandNumber, state);
         if (metaObjects != null) {
             for (GcodeParser.GcodeMeta c : metaObjects) {
                 if (c.state != null) {

ugs-core/src/com/willwinder/universalgcodesender/gcode/processors/RunFromProcessor.java

@@ -31,8 +31,9 @@ This file is part of Universal Gcode Sender (UGS).
 
 public class RunFromProcessor implements CommandProcessor {
     private int lineNumber;
-    private final GcodeParser parser = new GcodeParser();
+    private GcodeParser parser;
     private Double clearanceHeight = 0.0;
+
     /**
      * Truncates gcode to the specified line, and rewrites the preamble with the GcodeState.
      *
@@ -57,47 +58,61 @@ public List<String> processCommand(String command, GcodeState state) throws Gcod
             return Collections.singletonList(command);
         }
 
-        // Don't trust the input's machine state, this processor is discarding lines which would have updated it.
-        Position pos = parser.getCurrentState().currentPoint;
-
         if (state.commandNumber < lineNumber) {
-            parser.addCommand(command);
-            clearanceHeight = Math.max(clearanceHeight, pos.z);
-            return ImmutableList.of();
+            return skipLine(command);
+        } else if (state.commandNumber == lineNumber && parser != null) {
+            return getSkippedLinesState(command);
+        }
+
+        return ImmutableList.of(command);
+    }
+
+    private ImmutableList<String> getSkippedLinesState(String command) {
+        Position pos = parser.getCurrentState().currentPoint;
+        String moveToClearanceHeight = "G0Z" + clearanceHeight;
+        String moveToXY = "G0X" + pos.x + "Y" + pos.y;
+        String plunge = "G1Z" + pos.z;
+
+        GcodeState s = parser.getCurrentState();
+        String normalized = command;
+        try {
+            normalized = normalizeCommand(command, s);
+        } catch (GcodeParserException e) {
+            // If command couldn't be normalized, send as is
         }
 
-        if (state.commandNumber == lineNumber) {
+        // Reset the parser to prevent the state to be re-added
+        parser = null;
 
-            String moveToClearanceHeight = "G0Z" + clearanceHeight;
-            String moveToXY = "G0X" + pos.x + "Y" + pos.y;
-            String plunge = "G1Z" + pos.z;
+        return ImmutableList.of(
+                // Initialize state
+                s.machineStateCode(),
 
-            GcodeState s = parser.getCurrentState();
-            String normalized = command;
-            try {
-                normalized = normalizeCommand(command, s);
-            } catch (GcodeParserException e) {
-                // If command couldn't be normalized, send as is
-            }
+                // Move to start location
+                moveToClearanceHeight,
+                moveToXY,
 
-            return ImmutableList.of(
-                    // Initialize state
-                    s.machineStateCode(),
+                // Start spindle and set feed/speed before plunging into the work.
+                s.toAccessoriesCode(),
+                plunge,
 
-                    // Move to start location
-                    moveToClearanceHeight,
-                    moveToXY,
+                // Append normalized command
+                normalized
+        );
+    }
 
-                    // Start spindle and set feed/speed before plunging into the work.
-                    s.toAccessoriesCode(),
-                    plunge,
+    private ImmutableList<String> skipLine(String command) throws GcodeParserException {
+        createParser();
+        parser.addCommand(command);
+        Position pos = parser.getCurrentState().currentPoint;
+        clearanceHeight = Math.max(clearanceHeight, pos.z);
+        return ImmutableList.of();
+    }
 
-                    // Append normalized command
-                    normalized
-            );
+    private void createParser() {
+        if (parser == null) {
+            parser = new GcodeParser();
         }
-
-        return ImmutableList.of(command);
     }
 
     @Override

ugs-core/test/com/willwinder/universalgcodesender/gcode/fixtures/FixturesTest.java

@@ -128,7 +128,7 @@ public void runFixture(String basePath, String fixtureName, String parserName, G
         String inputFixture = basePath + fixtureName + ".input.nc";
         String streamOutputFixture = basePath + "out" + parserName + "/" + fixtureName + ".stream_output.nc";
         String outputxFixture = basePath + "out" + parserName + "/" + fixtureName  + ".parsed_output.nc";
-        System.out.printf("Running fixture %s...%n", inputFixture);
+        System.out.printf("Running fixture %s -> %s...%n", inputFixture, outputxFixture);
 
         // write parsed output to temp file
         Path output = Files.createTempFile(fixtureName, "output");

ugs-core/test/com/willwinder/universalgcodesender/gcode/processors/ArcExpanderTest.java

@@ -26,12 +26,16 @@ This file is part of Universal Gcode Sender (UGS).
 import com.willwinder.universalgcodesender.i18n.Localization;
 import com.willwinder.universalgcodesender.model.Position;
 import static com.willwinder.universalgcodesender.model.UnitUtils.Units.MM;
+
 import java.util.Collection;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.junit.Test;
 import static org.assertj.core.api.Assertions.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import org.assertj.core.data.Offset;
 import org.junit.Assert;
 
@@ -43,8 +47,6 @@ public class ArcExpanderTest {
 
     @Test
     public void arcExpandadState() throws Exception {
-        System.out.println("arcExpandBadState");
-
         GcodeState state = new GcodeState();
         state.currentPoint = null;
         ArcExpander instance = new ArcExpander(true, 1);
@@ -56,7 +58,6 @@ public void arcExpandadState() throws Exception {
 
     @Test
     public void modalsReturnedFirst() throws Exception {
-        System.out.println("arcExpandWithModals");
         GcodeState state = new GcodeState();
         state.currentPoint = new Position(0,0,0,MM);
         state.plane = XY;
@@ -69,12 +70,10 @@ public void modalsReturnedFirst() throws Exception {
 
     @Test
     public void arcExpandIgnoreNonArc() throws Exception {
-        System.out.println("arcExpandIgnoreNonArc");
         GcodeState state = new GcodeState();
         state.currentPoint = new Position(0,0,0,MM);
         state.plane = XY;
         ArcExpander instance = new ArcExpander(true, 1);
-        boolean threwException = false;
         String command = "G17 G0 X12";
         List<String> result = instance.processCommand(command, state);
         assertThat(result.size()).isEqualTo(1);
@@ -83,7 +82,6 @@ public void arcExpandIgnoreNonArc() throws Exception {
 
     @Test
     public void expandArcG17() throws Exception {
-        System.out.println("expandArcG17");
         GcodeState state = new GcodeState();
         state.currentPoint = new Position(-1,0,0,MM);
         state.plane = XY;
@@ -110,7 +108,6 @@ public void expandArcG17() throws Exception {
 
     @Test
     public void expandArcG18() throws Exception {
-        System.out.println("expandArcG18");
         GcodeState state = new GcodeState();
         state.currentPoint = new Position(0,0,-1,MM);
         state.plane = ZX;
@@ -137,7 +134,6 @@ public void expandArcG18() throws Exception {
 
     @Test
     public void expandArcG19() throws Exception {
-        System.out.println("expandArcG19");
         GcodeState state = new GcodeState();
         state.currentPoint = new Position(0,-1,0,MM);
         state.plane = YZ;
@@ -162,6 +158,27 @@ public void expandArcG19() throws Exception {
         }
     }
 
+    @Test
+    public void verifyFeedRateNotAddedFromPreviousState() throws GcodeParserException {
+        GcodeState gcodeState = new GcodeState();
+        gcodeState.feedRate = 100;
+
+        ArcExpander arcExpander = new ArcExpander(true, 1);
+        List<String> expandedGcode = arcExpander.processCommand("G2 X0. Y-5 I5 J0", gcodeState);
+
+        assertFalse("Did not expect the feed rate to be added from previous state but was \"" + expandedGcode.get(0) + "\"", expandedGcode.get(0).contains("F"));
+    }
+
+    @Test
+    public void verifyFeedRateAddedFromCurrentState() throws GcodeParserException {
+        GcodeState gcodeState = new GcodeState();
+        gcodeState.feedRate = 100;
+
+        ArcExpander arcExpander = new ArcExpander(true, 1);
+        List<String> expandedGcode = arcExpander.processCommand("G2 X0. Y-5 I5 J0 F1000", gcodeState);
+        assertTrue("Expected the feed rate to be added to the first expanded command but was \"" + expandedGcode.get(0) + "\"", expandedGcode.get(0).endsWith("F1000"));
+        assertTrue("Expected motion on next command but was \"" + expandedGcode.get(1) + "\"", expandedGcode.get(1).startsWith("G1"));
+    }
 
     /**
      * Verify that the points around given center point have a known radius and
@@ -174,9 +191,9 @@ static void verifyLines(Position center, Collection<String> lines, double radius
     }
 
     static Pattern LINE_COORDS = Pattern.compile("G1"
-                + "X([\\-\\+]?[0-9]+(?:\\.[0-9]+)?)"
-                + "Y([\\-\\+]?[0-9]+(?:\\.[0-9]+)?)"
-                + "Z([\\-\\+]?[0-9]+(?:\\.[0-9]+)?)");
+                + "X([\\-+]?[0-9]+(?:\\.[0-9]+)?)"
+                + "Y([\\-+]?[0-9]+(?:\\.[0-9]+)?)"
+                + "Z([\\-+]?[0-9]+(?:\\.[0-9]+)?)");
                 //+ "(F\\d+)?");
     static void verifyLine(Position center, String line, double radius, Position min, Position max, Plane p) {
         Matcher m = LINE_COORDS.matcher(line);

ugs-core/test/com/willwinder/universalgcodesender/gcode/processors/RunFromProcessorTest.java

@@ -4,6 +4,8 @@
 import com.willwinder.universalgcodesender.gcode.util.GcodeParserException;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import static org.junit.Assert.*;
@@ -31,11 +33,11 @@ public void processCommandShouldAppendStateUpToSelectedLine() throws GcodeParser
         RunFromProcessor processor = new RunFromProcessor(2);
         GcodeState gcodeState = new GcodeState();
         gcodeState.commandNumber = 0;
-        List<String> result = processor.processCommand("G0X10", gcodeState);
+        List<String> result = processor.processCommand("G0X10F100", gcodeState);
         assertEquals(0, result.size());
 
         gcodeState.commandNumber = 1;
-        result = processor.processCommand("G0Y10", gcodeState);
+        result = processor.processCommand("G0Y10S1000", gcodeState);
         assertEquals(0, result.size());
 
         gcodeState.commandNumber = 2;
@@ -45,8 +47,66 @@ public void processCommandShouldAppendStateUpToSelectedLine() throws GcodeParser
         assertEquals("G21G90G91.1G94G54G17", result.get(0));
         assertEquals("G0Z0.0", result.get(1));
         assertEquals("G0X10.0Y10.0", result.get(2));
-        assertEquals("S0.0F0.0", result.get(3));
+        assertEquals("S1000.0F100.0", result.get(3));
         assertEquals("G1Z0.0", result.get(4));
-        assertEquals("F0.0S0.0G0Y11", result.get(5));
+        assertEquals("F100.0S1000.0G0Y11", result.get(5));
+    }
+
+    @Test
+    public void processArcs() throws GcodeParserException {
+        List<String> gcode = Arrays.asList("G17 G21 G90 G94 G54 M0 M5 M9",
+                "G0 Z1 F100 S100",
+                "X-5 Y0",
+                "M3",
+                "G1 Z0",
+                "G2 X0. Y-5 I5 J0",
+                "G2 X-5 Y0 I0 J5");
+
+        RunFromProcessor processor = new RunFromProcessor(6);
+        GcodeState gcodeState = new GcodeState();
+
+        List<String> result = new ArrayList<>();
+        for (int i = 0; i < gcode.size(); i++) {
+            gcodeState.commandNumber = i;
+            result.addAll(processor.processCommand(gcode.get(i), gcodeState));
+        }
+
+        assertEquals("G21G90G91.1G94G54G17", result.get(0));
+        assertEquals("G0Z1.0", result.get(1));
+        assertEquals("G0X0.0Y-5.0", result.get(2)); // Should start where the first arc segment ended
+        assertEquals("M3S100.0F100.0", result.get(3));
+        assertEquals("G1Z0.0", result.get(4));
+        assertEquals("F100.0S100.0G2X-5Y0I0J5", result.get(5)); // Should contain the last arc segment only
+    }
+
+    @Test
+    public void processArcsAfterExpand() throws GcodeParserException {
+        List<String> gcode = Arrays.asList("G17 G21 G90 G94 G54 M0 M5 M9",
+                "G0 Z1 F100 S100",
+                "X-5 Y0",
+                "M3",
+                "G1 Z0",
+                "G2 X0. Y-5 I5 J0",
+                "G2 X-5 Y0 I0 J5");
+
+        CommandProcessorList processorList = new CommandProcessorList();
+        processorList.add(new ArcExpander(true, 1));
+        processorList.add(new RunFromProcessor(6));
+
+        GcodeState gcodeState = new GcodeState();
+        List<String> result = new ArrayList<>();
+        for (int i = 0; i < gcode.size(); i++) {
+            gcodeState.commandNumber = i;
+            result.addAll(processorList.processCommand(gcode.get(i), gcodeState));
+        }
+
+        assertEquals("G21G90G91.1G94G54G17", result.get(0));
+        assertEquals("G0Z1.0", result.get(1));
+        assertEquals("G0X0.0Y-5.0", result.get(2)); // Should start where the first arc segment ended
+        assertEquals("M3S100.0F100.0", result.get(3));
+        assertEquals("G1Z0.0", result.get(4));
+
+        String regex = "F100\\.0S100\\.0G1X-0\\.9\\d+Y0\\.0\\d+Z0";
+        assertTrue("Expected a movement command with the the active feed and speed matching \"" +  regex + "\" but was " + result.get(5), result.get(5).matches(regex));
     }
 }

ugs-core/test/resources/gcode/fixtures/arc/arc_feed.input.nc

@@ -0,0 +1,5 @@
+G17
+G00 X0 Y0
+G01 X0 Y0 F100
+G02 X0 Y0 Z5 I1.5 J0 F200
+G03 X0 Y0 Z10 I1.5 J0 F300

ugs-core/test/resources/gcode/fixtures/arc/outArc/G17_G2_one_arc.parsed_output.nc

@@ -1,7 +1,7 @@
 G17
 G0X-0.75Y0Z0
 G0X-1.5Y0Z0
-G1X-1.496788385Y0.098104694Z0F0.0
+G1X-1.496788385Y0.098104694Z0
 G1X-1.487167292Y0.195789288Z0
 G1X-1.471177921Y0.292635483Z0
 G1X-1.448888739Y0.388228568Z0

ugs-core/test/resources/gcode/fixtures/arc/outArc/G17_G2_one_arc.stream_output.nc

@@ -2,7 +2,7 @@ gsw_meta:51
 G17¶¶G17¶¶1¶¶
 G00 X-1.5 Y0.¶¶G0X-0.75Y0Z0¶¶2¶¶
 G00 X-1.5 Y0.¶¶G0X-1.5Y0Z0¶¶2¶¶
-G02 X1.5 Y0 I1.5 J0¶¶G1X-1.496788385Y0.098104694Z0F0.0¶¶3¶¶
+G02 X1.5 Y0 I1.5 J0¶¶G1X-1.496788385Y0.098104694Z0¶¶3¶¶
 G02 X1.5 Y0 I1.5 J0¶¶G1X-1.487167292Y0.195789288Z0¶¶3¶¶
 G02 X1.5 Y0 I1.5 J0¶¶G1X-1.471177921Y0.292635483Z0¶¶3¶¶
 G02 X1.5 Y0 I1.5 J0¶¶G1X-1.448888739Y0.388228568Z0¶¶3¶¶

ugs-core/test/resources/gcode/fixtures/arc/outArc/G17_G3_one_arc.parsed_output.nc

@@ -1,7 +1,7 @@
 G17
 G0X-0.75Y0Z0
 G0X-1.5Y0Z0
-G1X-1.496788385Y-0.098104694Z0F0.0
+G1X-1.496788385Y-0.098104694Z0
 G1X-1.487167292Y-0.195789288Z0
 G1X-1.471177921Y-0.292635483Z0
 G1X-1.448888739Y-0.388228568Z0