Fix problem with arcs with negative radius (#2711)

* Prevent spamming error dialogs when loading gcode with multiple errors. Removed unused parameter for spindle speed.

* Added unit tests and fix for handling arcs with negative radius

Author: breiler

ugs-core/src/com/willwinder/universalgcodesender/gcode/GcodePreprocessorUtils.java

@@ -32,6 +32,7 @@ This file is part of Universal Gcode Sender (UGS).
 import com.willwinder.universalgcodesender.model.PartialPosition;
 import com.willwinder.universalgcodesender.model.Position;
 import com.willwinder.universalgcodesender.model.UnitUtils;
+import static com.willwinder.universalgcodesender.utils.MathUtils.normalizeAngle;
 
 import java.text.DecimalFormat;
 import java.util.ArrayList;
@@ -96,14 +97,14 @@ public static String overridePosition(String originalCommand, PartialPosition up
     }
 
     /**
-     * Searches the command string for an 'f' and replaces the speed value 
+     * Searches the command string for an 'f' and replaces the speed value
      * between the 'f' and the next space with a percentage of that speed.
-     * In that way all speed values become a ratio of the provided speed 
+     * In that way all speed values become a ratio of the provided speed
      * and don't get overridden with just a fixed speed.
      */
     static public String overrideSpeed(String command, double speed) {
         String returnString = command;
-        
+
         // Check if command sets feed speed.
         Pattern pattern = Pattern.compile("F([0-9.]+)", Pattern.CASE_INSENSITIVE);
         Matcher matcher = pattern.matcher(command);
@@ -124,7 +125,7 @@ static public String overrideSpeed(String command, double speed) {
     static public String removeComment(String command) {
         return COMMENT.matcher(command).replaceAll(EMPTY);
     }
-    
+
     /**
      * Searches for a comment in the input string and returns the first match.
      */
@@ -135,13 +136,13 @@ static public String parseComment(String command) {
         //              "(?<=\()[^\(\)]*|(?<=\;)[^;]*"
         //              "(?<=\\()[^\\(\\)]*|(?<=\\;)[^;]*"
         Matcher matcher = COMMENTPARSE.matcher(command);
-        if (matcher.find()){
+        if (matcher.find()) {
             comment = matcher.group(0);
         }
 
         return comment;
     }
-    
+
     static public String truncateDecimals(int length, String command) {
         if (length != decimalLength) {
             //Only build the decimal formatter if the truncation length has changed.
@@ -158,7 +159,7 @@ static public String truncateDecimals(int length, String command) {
             matcher.appendReplacement(sb, decimalFormatter.format(d));
         }
         matcher.appendTail(sb);
-        
+
         // Return new command.
         return sb.toString();
     }
@@ -196,7 +197,7 @@ private static void updateDecimalFormatter(int length) {
     static public List<String> parseCodes(List<String> args, char code) {
         List<String> l = new ArrayList<>();
         char address = Character.toUpperCase(code);
-        
+
         for (String s : args) {
             if (s.length() > 0 && Character.toUpperCase(s.charAt(0)) == address) {
                 l.add(s.substring(1));
@@ -219,7 +220,7 @@ static public Position updatePointWithCommand(List<String> commandArgs, Position
         double c = parseCoord(commandArgs, 'C');
 
         if (Double.isNaN(x) && Double.isNaN(y) && Double.isNaN(z) &&
-            Double.isNaN(a) && Double.isNaN(b) && Double.isNaN(c)) {
+                Double.isNaN(a) && Double.isNaN(b) && Double.isNaN(c)) {
             return null;
         }
 
@@ -505,6 +506,11 @@ static public List<Position> generatePointsAlongArcBDring(
         double endAngle = GcodePreprocessorUtils.getAngle(center, end, plane);
         double sweep = GcodePreprocessorUtils.calculateSweep(startAngle, endAngle, clockwise);
 
+        // If the radius is negative we need invert the angle
+        if (radius < 0) {
+            startAngle = normalizeAngle(startAngle - Math.PI);
+        }
+
         int numPoints = calculateNumberOfPointsToExpand(r, start.getUnits(), minArcLengthMM, arcSegmentLengthMM, sweep);
         if (numPoints == 0) {
             return Collections.emptyList();
@@ -523,9 +529,9 @@ static public List<Position> generatePointsAlongArcBDring(
      * @param sweep              the angle of the arc
      * @return the number of segments to split the arc into to achieve the given arc segment length
      */
-    private static int calculateNumberOfPointsToExpand(double radius, UnitUtils.Units radiusUnits, double minArcLengthMM, double arcSegmentLengthMM, double sweep) {
+    public static int calculateNumberOfPointsToExpand(double radius, UnitUtils.Units radiusUnits, double minArcLengthMM, double arcSegmentLengthMM, double sweep) {
         // Convert units.
-        double arcLengthMM = sweep * radius * UnitUtils.scaleUnits(radiusUnits, UnitUtils.Units.MM);
+        double arcLengthMM = Math.abs(sweep * radius * UnitUtils.scaleUnits(radiusUnits, UnitUtils.Units.MM));
 
         // If this arc doesn't meet the minimum threshold, don't expand.
         if (minArcLengthMM > 0 && arcLengthMM < minArcLengthMM) {
@@ -620,7 +626,7 @@ public static List<Position> generatePointsAlongArcBDring(
      *
      * @return the center of rotation between two points with IJK codes.
      */
-    static private Position convertRToCenter(
+    public static Position convertRToCenter(
             Position start,
             Position end,
             double radius,
@@ -758,7 +764,7 @@ public static boolean isMotionWord(char character) {
      * <p>
      * If the code is implicit, like the command "X0Y0", we'll still extract "X0Y0".
      * If the code is G0 or G1 and G53 is found, it will also be extracted:
-     * http://linuxcnc.org/docs/html/gcode/g-code.html#gcode:g53
+     * <a href="http://linuxcnc.org/docs/html/gcode/g-code.html#gcode:g53">gcode:g53</a>
      */
     public static SplitCommand extractMotion(Code code, String command) {
         List<String> args = splitCommand(command);

ugs-core/src/com/willwinder/universalgcodesender/gcode/util/GcodeParserException.java

@@ -7,7 +7,11 @@
  * @author wwinder
  */
 public class GcodeParserException extends Exception {
-    public GcodeParserException(String s) {
-        super(s);
+    public GcodeParserException(String message) {
+        super(message);
+    }
+
+    public GcodeParserException(String message, Throwable throwable) {
+        super(message, throwable);
     }
 }

ugs-core/src/com/willwinder/universalgcodesender/utils/MathUtils.java

@@ -39,7 +39,7 @@ public class MathUtils {
 
     /**
      * To find orientation of ordered triplet (p1, p2, p3).
-     * https://www.geeksforgeeks.org/orientation-3-ordered-points/
+     * <a href="https://www.geeksforgeeks.org/orientation-3-ordered-points/">orientation-3-ordered-points</a>
      *
      * @param p1 a point in a polygon
      * @param p2 a point in a polygon
@@ -144,6 +144,21 @@ public static boolean isEqual(double d1, double d2, double delta) {
         return Math.abs(d1 - d2) <= delta;
     }
 
+    /**
+     * Normalizes an angle given in radians to make sure it is in the interval [0,2π]
+     *
+     * @param angle an angle in radians
+     * @return the normalized angle
+     */
+    public static double normalizeAngle(double angle) {
+        double twoPi = 2 * Math.PI;
+        angle = angle % twoPi;
+        if (angle < 0) {
+            angle += twoPi;
+        }
+        return angle;
+    }
+
     /**
      * Gets the center position given a list of line segments.
      *

ugs-core/src/com/willwinder/universalgcodesender/visualizer/GcodeViewParse.java

@@ -132,7 +132,6 @@ public List<LineSegment> toObjFromReader(IGcodeStreamReader reader,
 
         // Save the state
         Position start = new Position(Double.NaN, Double.NaN, Double.NaN, Double.NaN, Double.NaN, Double.NaN, gp.getCurrentState().getUnits());
-        double spindleSpeed = 0;
 
         while (reader.getNumRowsRemaining() > 0) {
             GcodeCommand commandObject = reader.getNextCommand();
@@ -141,9 +140,8 @@ public List<LineSegment> toObjFromReader(IGcodeStreamReader reader,
                 List<GcodeMeta> points = gp.addCommand(command, commandObject.getCommandNumber());
                 for (GcodeMeta meta : points) {
                     if (meta.point != null) {
-                        VisualizerUtils.addLinesFromPointSegment(start, meta.point, arcSegmentLength, lines, spindleSpeed);
+                        VisualizerUtils.addLinesFromPointSegment(start, meta.point, arcSegmentLength, lines);
                         start = meta.point.point();
-                        spindleSpeed = meta.point.getSpindleSpeed();
                     }
                 }
             }
@@ -176,16 +174,14 @@ public List<LineSegment> toObjRedux(List<String> gcode, double arcSegmentLength)
 
         // Save the state
         Position start = new Position(Double.NaN, Double.NaN, Double.NaN, gp.getCurrentState().getUnits());
-        double spindleSpeed = 0;
 
         for (String s : gcode) {
             List<String> commands = gp.preprocessCommand(s, gp.getCurrentState());
             for (String command : commands) {
                 List<GcodeMeta> points = gp.addCommand(command);
                 for (GcodeMeta meta : points) {
                     if (meta.point != null) {
-                        VisualizerUtils.addLinesFromPointSegment(start, meta.point, arcSegmentLength, lines, spindleSpeed);
-                        spindleSpeed = meta.point.getSpindleSpeed();
+                        VisualizerUtils.addLinesFromPointSegment(start, meta.point, arcSegmentLength, lines);
 
                         // if the last set point is in a different or unknown unit, crate a new point-instance with the correct unit set
                         if (start.getUnits() != UnitUtils.Units.MM && gp.getCurrentState().isMetric) {

ugs-core/src/com/willwinder/universalgcodesender/visualizer/VisualizerUtils.java

@@ -22,10 +22,10 @@ This file is part of Universal Gcode Sender (UGS).
 package com.willwinder.universalgcodesender.visualizer;
 
 import com.willwinder.universalgcodesender.gcode.GcodePreprocessorUtils;
+import com.willwinder.universalgcodesender.gcode.util.GcodeParserException;
 import com.willwinder.universalgcodesender.gcode.util.PlaneFormatter;
 import com.willwinder.universalgcodesender.model.Position;
 import com.willwinder.universalgcodesender.types.PointSegment;
-import com.willwinder.universalgcodesender.utils.GUIHelpers;
 
 import java.io.BufferedReader;
 import java.io.DataInputStream;
@@ -35,14 +35,11 @@ This file is part of Universal Gcode Sender (UGS).
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * @author wwinder
  */
 public class VisualizerUtils {
-    private static final Logger LOGGER = Logger.getLogger(VisualizerUtils.class.getSimpleName());
 
     /**
      * Returns the maximum side dimension of a box containing two points.
@@ -54,24 +51,14 @@ public static double findMaxSide(Position min, Position max) {
         return Math.max(x, Math.max(y, z));
     }
 
-    /**
-     * Returns the aspect ratio from two points.
-     */
-    public static double findAspectRatio(Position min, Position max) {
-        double x = Math.abs(min.x) + Math.abs(max.x);
-        double y = Math.abs(min.y) + Math.abs(max.y);
-        return x / y;
-    }
-
     /**
      * Returns the center point on a line.
      */
     public static Position findCenter(Position min, Position max) {
-        Position center = new Position(
+        return new Position(
                 (min.x + max.x) / 2.0,
                 (min.y + max.y) / 2.0,
                 (min.z + max.z) / 2.0);
-        return center;
     }
 
     /**
@@ -132,7 +119,7 @@ public static double getRelativeMovementMultiplier(double objectMin, double obje
     /**
      * Helper to create a line segment with flags initialized.
      */
-    private static LineSegment createLineSegment(Position a, Position b, PointSegment meta, double spindleSpeed) {
+    private static LineSegment createLineSegment(Position a, Position b, PointSegment meta) {
         LineSegment ls = new LineSegment(a, b, meta.getLineNumber());
         ls.setIsArc(meta.isArc());
         ls.setIsFastTraverse(meta.isFastTraverse());
@@ -145,8 +132,10 @@ private static LineSegment createLineSegment(Position a, Position b, PointSegmen
 
     /**
      * Turns a point segment into one or more LineSegment. Arcs and rotations around axes are expanded
+     *
+     * @throws GcodeParserException if the lines could not be expanded
      */
-    public static void addLinesFromPointSegment(final Position start, final PointSegment endSegment, double arcSegmentLength, List<LineSegment> ret, double spindleSpeed) {
+    public static void addLinesFromPointSegment(final Position start, final PointSegment endSegment, double arcSegmentLength, List<LineSegment> ret) throws GcodeParserException {
         // For a line segment list ALL arcs must be converted to lines.
         double minArcLength = 0;
         endSegment.convertToMetric();
@@ -156,22 +145,21 @@ public static void addLinesFromPointSegment(final Position start, final PointSeg
             if (start != null) {
                 // Expand arc for graphics.
                 if (endSegment.isArc()) {
-                    expandArc(start, endSegment, arcSegmentLength, ret, minArcLength, spindleSpeed);
+                    expandArc(start, endSegment, arcSegmentLength, ret, minArcLength);
                 } else if (endSegment.isRotation()) {
-                    expandRotationalLineSegment(start, endSegment, ret, spindleSpeed);
+                    expandRotationalLineSegment(start, endSegment, ret);
                 } else {
                     // Line
-                    ret.add(createLineSegment(start, endSegment.point(), endSegment, spindleSpeed));
+                    ret.add(createLineSegment(start, endSegment.point(), endSegment));
                 }
             }
         } catch (Exception e) {
-            String message = endSegment.getLineNumber() + ": " + e.getMessage();
-            GUIHelpers.displayErrorDialog(message, true);
-            LOGGER.log(Level.SEVERE, message, e);
+            String message = "Line " + endSegment.getLineNumber() + ": " + e.getMessage();
+            throw new GcodeParserException(message, e);
         }
     }
 
-    private static void expandArc(Position start, PointSegment endSegment, double arcSegmentLength, List<LineSegment> ret, double minArcLength, double spindleSpeed) {
+    private static void expandArc(Position start, PointSegment endSegment, double arcSegmentLength, List<LineSegment> ret, double minArcLength) {
         List<Position> points =
                 GcodePreprocessorUtils.generatePointsAlongArcBDring(
                         start, endSegment.point(), endSegment.center(), endSegment.isClockwise(),
@@ -180,13 +168,13 @@ private static void expandArc(Position start, PointSegment endSegment, double ar
         if (!points.isEmpty()) {
             Position startPoint = start;
             for (Position nextPoint : points) {
-                ret.add(createLineSegment(startPoint, nextPoint, endSegment, spindleSpeed));
+                ret.add(createLineSegment(startPoint, nextPoint, endSegment));
                 startPoint = nextPoint;
             }
         }
     }
 
-    public static void expandRotationalLineSegment(Position start, PointSegment endSegment, List<LineSegment> ret, double spindleSpeed) {
+    public static void expandRotationalLineSegment(Position start, PointSegment endSegment, List<LineSegment> ret) {
         double maxDegreesPerStep = 5;
         double deltaX = defaultZero(endSegment.point().x) - defaultZero(start.x);
         double deltaY = defaultZero(endSegment.point().y) - defaultZero(start.y);
@@ -217,11 +205,11 @@ public static void expandRotationalLineSegment(Position start, PointSegment endS
             if (deltaC != 0) {
                 end.setC(defaultZero(start.c) + ((deltaC / steps) * i));
             }
-            ret.add(createLineSegment(startPoint, end, endSegment, spindleSpeed));
+            ret.add(createLineSegment(startPoint, end, endSegment));
             startPoint = end;
         }
 
-        ret.add(createLineSegment(startPoint, endSegment.point(), endSegment, spindleSpeed));
+        ret.add(createLineSegment(startPoint, endSegment.point(), endSegment));
     }
 
     /**