Make excellon tests pass

10 files changed, 149 insertions, 72 deletions

diff --git a/gerbonara/gerber/aperture_macros/parse.py b/gerbonara/gerber/aperture_macros/parse.py
index 43af309..c1aa2d0 100644
--- a/gerbonara/gerber/aperture_macros/parse.py
+++ b/gerbonara/gerber/aperture_macros/parse.py
@@ -121,10 +121,10 @@ class ApertureMacro:
 
     def to_graphic_primitives(self, offset, rotation, parameters : [float], unit=None):
         variables = dict(self.variables)
-        for number, value in enumerate(parameters):
-            if i in variables:
+        for number, value in enumerate(parameters, start=1):
+            if number in variables:
                 raise SyntaxError(f'Re-definition of aperture macro variable {i} through parameter {value}')
-            variables[i] = value
+            variables[number] = value
 
         for primitive in self.primitives:
             yield from primitive.to_graphic_primitives(offset, rotation, variables, unit)
diff --git a/gerbonara/gerber/aperture_macros/primitive.py b/gerbonara/gerber/aperture_macros/primitive.py
index 4de19c4..76268d2 100644
--- a/gerbonara/gerber/aperture_macros/primitive.py
+++ b/gerbonara/gerber/aperture_macros/primitive.py
@@ -232,9 +232,9 @@ class Outline(Primitive):
             bound_radii = [None] * len(bound_coords)
 
             rotation += deg_to_rad(calc.rotation)
-            bound_coords = [ rotate_point(*p, rotation, 0, 0) for p in bound_coords ]
+            bound_coords = [ gp.rotate_point(*p, rotation, 0, 0) for p in bound_coords ]
 
-            return gp.ArcPoly(bound_coords, bound_radii, polarity_dark=calc.exposure)
+            return [gp.ArcPoly(bound_coords, bound_radii, polarity_dark=calc.exposure)]
 
     def dilate(self, offset, unit):
         # we would need a whole polygon offset/clipping library here
diff --git a/gerbonara/gerber/apertures.py b/gerbonara/gerber/apertures.py
index b3e462c..872de9f 100644
--- a/gerbonara/gerber/apertures.py
+++ b/gerbonara/gerber/apertures.py
@@ -96,9 +96,6 @@ class ExcellonTool(Aperture):
     plated : bool = None
     depth_offset : Length(float) = 0
     
-    def __post_init__(self):
-        print('created', self)
-
     def primitives(self, x, y, unit=None):
         return [ gp.Circle(x, y, self.unit.convert_to(unit, self.diameter/2)) ]
 
diff --git a/gerbonara/gerber/cam.py b/gerbonara/gerber/cam.py
index bbcd042..9c9baf0 100644
--- a/gerbonara/gerber/cam.py
+++ b/gerbonara/gerber/cam.py
@@ -80,6 +80,10 @@ class FileSettings:
         if '.' in value:
             return float(value)
 
+        # TARGET3001! exports zeros as "00" even when it uses an explicit decimal point everywhere else.
+        if int(value) == 0:
+            return 0
+
         # Format precision
         integer_digits, decimal_digits = self.number_format
         if integer_digits is None or decimal_digits is None:
diff --git a/gerbonara/gerber/excellon.py b/gerbonara/gerber/excellon.py
index 4887245..2e0add3 100755
--- a/gerbonara/gerber/excellon.py
+++ b/gerbonara/gerber/excellon.py
@@ -50,7 +50,7 @@ class ExcellonContext:
             yield 'G05'
 
     def route_mode(self, unit, x, y):
-        x, y = self.unit(x, unit), self.unit(y, unit)
+        x, y = self.settings.unit(x, unit), self.settings.unit(y, unit)
 
         if self.mode == ProgramState.ROUTING and (self.x, self.y) == (x, y):
             return # nothing to do
@@ -369,7 +369,12 @@ class ExcellonParser(object):
                 warnings.warn('Commands found following end of program statement.', SyntaxWarning)
             # TODO check first command in file is "start of header" command.
 
-            self.exprs.handle(self, line)
+            try:
+                if not self.exprs.handle(self, line):
+                    raise ValueError('Unknown excellon statement:', line)
+            except:
+                print('Original line was:', line)
+                raise
 
     exprs = RegexMatcher()
 
@@ -447,7 +452,7 @@ class ExcellonParser(object):
 
         self.active_tool = self.tools[index]
 
-    coord = lambda name, key=None: fr'{name}(?P<{key or name}>[+-]?[0-9]*\.?[0-9]*)?'
+    coord = lambda name, key=None: fr'({name}(?P<{key or name}>[+-]?[0-9]*\.?[0-9]*))?'
     xy_coord = coord('X') + coord('Y')
 
     @exprs.match(r'R(?P<count>[0-9]+)' + xy_coord)
@@ -455,8 +460,8 @@ class ExcellonParser(object):
         if self.program_state == ProgramState.HEADER:
             return
 
-        dx = int(match['x'] or '0')
-        dy = int(match['y'] or '0')
+        dx = int(match['X'] or '0')
+        dy = int(match['Y'] or '0')
 
         for i in range(int(match['count'])):
             self.pos[0] += dx
@@ -473,9 +478,9 @@ class ExcellonParser(object):
             def wrapper(self, *args, **kwargs):
                 nonlocal name
                 if self.program_state is None:
-                    warnings.warn(f'{name} header statement found before start of header')
+                    warnings.warn(f'{name} header statement found before start of header', SyntaxWarning)
                 elif self.program_state != ProgramState.HEADER:
-                    warnings.warn(f'{name} header statement found after end of header')
+                    warnings.warn(f'{name} header statement found after end of header', SyntaxWarning)
                 fun(self, *args, **kwargs)
             return wrapper
         return wrap
@@ -485,7 +490,7 @@ class ExcellonParser(object):
         if self.program_state == ProgramState.HEADER:
             # It seems that only fritzing puts both a '%' start of header thingy and an M48 statement at the beginning
             # of the file.
-            self.generator_hints('fritzing')
+            self.generator_hints.append('fritzing')
         elif self.program_state is not None:
             warnings.warn(f'M48 "header start" statement found in the middle of the file, currently in {self.program_state}', SyntaxWarning)
         self.program_state = ProgramState.HEADER
@@ -585,7 +590,7 @@ class ExcellonParser(object):
         if self.program_state != ProgramState.ROUTING:
             return
 
-        if not self.drill_down or not (match['x'] or match['y']) or not self.ensure_active_tool():
+        if not self.drill_down or not (match['X'] or match['Y']) or not self.ensure_active_tool():
             return
 
         if self.interpolation_mode == InterpMode.LINEAR:
@@ -627,38 +632,37 @@ class ExcellonParser(object):
 
             self.objects.append(Arc(*start, *end, i, j, True, self.active_tool, unit=self.settings.unit))
 
-    @exprs.match('M71|METRIC') # XNC uses "METRIC"
-    @header_command('M71')
-    def handle_metric_mode(self, match):
-        self.settings.unit = MM
+    @exprs.match(r'(M71|METRIC|M72|INCH)(,LZ|,TZ)?(,0*\.0*)?')
+    def parse_easyeda_format(self, match):
+        metric = match[1] in ('METRIC', 'M71')
 
-    @exprs.match('M72|INCH') # XNC uses "INCH"
-    @header_command('M72')
-    def handle_inch_mode(self, match):
-        self.settings.unit = Inch
+        self.settings.unit = MM if metric else Inch
 
-    @exprs.match(r'(METRIC|INCH)(,LZ|,TZ)?(0*\.0*)?')
-    def parse_easyeda_format(self, match):
-        # geda likes to omit the LZ/TZ
-        self.settings.unit = MM if match[1] == 'METRIC' else Inch
         if match[2]:
-            self.settings.zeros = 'leading' if match[2] == ',LZ' else 'trailing'
+            self.settings.zeros = 'trailing' if match[2] == ',LZ' else 'leading'
+
         # Newer EasyEDA exports have this in an altium-like FILE_FORMAT comment instead. Some files even have both.
         # This is used by newer autodesk eagles, fritzing and diptrace
         if match[3]:
-            if self.generator is None:
-                # newer eagles identify themselvees through a comment, and fritzing uses this wonky double-header-start
-                # with a "%" line followed  by an "M48" line. Thus, thus must be diptrace.
-                self.generator_hints.append('diptrace')
-            integer, _, fractional = match[3].partition('.')
+            integer, _, fractional = match[3][1:].partition('.')
             self.settings.number_format = len(integer), len(fractional)
-        self.generator_hints.append('easyeda')
+
+        elif self.settings.number_format == (None, None) and not metric:
+            warnings.warn('Using implicit number format from naked "INCH" statement. This is normal for Fritzing, Diptrace, Geda and pcb-rnd.', SyntaxWarning)
+            self.settings.number_format = (2,4)
     
     @exprs.match('G90')
     @header_command('G90')
     def handle_absolute_mode(self, match):
         self.settings.notation = 'absolute'
 
+    @exprs.match('G93' + xy_coord)
+    def handle_absolute_mode(self, match):
+        if int(match['X'] or 0) != 0 or int(match['Y'] or 0) != 0:
+            # Siemens tooling likes to include a meaningless G93X0Y0 after its header.
+            raise SyntaxError('G93 zero set command is not supported.')
+        self.generator_hints.append('siemens')
+
     @exprs.match('ICI,?(ON|OFF)')
     def handle_incremental_mode(self, match):
         self.settings.notation = 'absolute' if match[1] == 'OFF' else 'incremental'
@@ -702,14 +706,14 @@ class ExcellonParser(object):
 
     @exprs.match(r'; Format\s*: ([0-9]+\.[0-9]+) / (Absolute|Incremental) / (Inch|MM) / (Leading|Trailing)')
     def parse_siemens_format(self, match):
-        x, _, y = match[1].split('.')
+        x, _, y = match[1].partition('.')
         self.settings.number_format = int(x), int(y)
         # NOTE: Siemens files seem to always contain both this comment and an explicit METRIC/INC statement. However,
         # the meaning of "leading" and "trailing" is swapped in both: When this comment says leading, we get something
         # like "INCH,TZ".
-        self.settings.notation = {'Leading': 'trailing', 'Trailing': 'leading'}[match[2]]
+        self.settings.notation = match[2].lower()
         self.settings.unit = to_unit(match[3])
-        self.settings.zeros = match[4].lower()
+        self.settings.zeros = {'Leading': 'trailing', 'Trailing': 'leading'}[match[4]]
         self.generator_hints.append('siemens')
 
     @exprs.match('; Contents: (Thru|.*) / (Drill|Mill) / (Plated|Non-Plated)')
diff --git a/gerbonara/gerber/layer_rules.py b/gerbonara/gerber/layer_rules.py
index e0a6954..4969d36 100644
--- a/gerbonara/gerber/layer_rules.py
+++ b/gerbonara/gerber/layer_rules.py
@@ -58,6 +58,19 @@ MATCH_RULES = {
     'drill plated':     r'.*\.(drl)', # diptrace has unplated drills on the outline layer
     },
 
+'target': {
+    'top copper':       r'.*\.Top',
+    'top mask':         r'.*\.StopTop',
+    'top silk':         r'.*\.PosiTop',
+    'top paste':        r'.*\.PasteTop',
+    'bottom copper':    r'.*\.Bot',
+    'bottop mask':      r'.*\.StopBot',
+    'bottop silk':      r'.*\.PosiBot',
+    'bottop paste':     r'.*\.PasteBot',
+    'drill outline':    r'.*\.Outline',
+    'drill plated':     r'.*\.Drill',
+    },
+
 'orcad': {
     'top copper':       r'.*\.top',
     'top mask':         r'.*\.smt',
diff --git a/gerbonara/gerber/rs274x.py b/gerbonara/gerber/rs274x.py
index e986225..42e4230 100644
--- a/gerbonara/gerber/rs274x.py
+++ b/gerbonara/gerber/rs274x.py
@@ -464,7 +464,7 @@ class GerberParser:
         'image_rotation': fr"IR(?P<rotation>{NUMBER})",
         'mirror_image': r"MI(A(?P<a>0|1))?(B(?P<b>0|1))?",
         'scale_factor': fr"SF(A(?P<a>{DECIMAL}))?(B(?P<b>{DECIMAL}))?",
-        'aperture_definition': fr"ADD(?P<number>\d+)(?P<shape>C|R|O|P|{NAME})[,]?(?P<modifiers>[^,%]*)",
+        'aperture_definition': fr"ADD(?P<number>\d+)(?P<shape>C|R|O|P|{NAME})(?P<modifiers>,[^,%]*)?$",
         'aperture_macro': fr"AM(?P<name>{NAME})\*(?P<macro>[^%]*)",
         'region_start': r'G36',
         'region_end': r'G37',
@@ -536,7 +536,12 @@ class GerberParser:
 
             for name, le_regex in self.STATEMENT_REGEXES.items():
                 if (match := le_regex.match(line)):
-                    getattr(self, f'_parse_{name}')(match.groupdict())
+                    try:
+                        getattr(self, f'_parse_{name}')(match.groupdict())
+                    except:
+                        print('Original line was:', line)
+                        print('    match:', match)
+                        raise
                     line = line[match.end(0):]
                     break
 
@@ -627,7 +632,7 @@ class GerberParser:
 
     def _parse_aperture_definition(self, match):
         # number, shape, modifiers
-        modifiers = [ float(val) for val in match['modifiers'].split('X') ] if match['modifiers'].strip() else []
+        modifiers = [ float(val) for val in match['modifiers'].strip(' ,').split('X') ] if match['modifiers'] else []
 
         aperture_classes = {
                 'C': apertures.CircleAperture,
diff --git a/gerbonara/gerber/tests/test_excellon.py b/gerbonara/gerber/tests/test_excellon.py
index 9aa5232..fb0ebb8 100644
--- a/gerbonara/gerber/tests/test_excellon.py
+++ b/gerbonara/gerber/tests/test_excellon.py
@@ -4,50 +4,94 @@
 import math
 
 import pytest
+from scipy.spatial import KDTree
 
 from ..excellon import ExcellonFile
+from ..rs274x import GerberFile
 from ..cam import FileSettings
+from ..graphic_objects import Flash
 
 from .image_support import *
 from .utils import *
 from ..utils import Inch, MM
 
-REFERENCE_FILES = [
-        'easyeda/Gerber_Drill_NPTH.DRL',
-        'easyeda/Gerber_Drill_PTH.DRL',
-        'altium-composite-drill/NC Drill/LimeSDR-QPCIe_1v2-SlotHoles.TXT',
-        'altium-composite-drill/NC Drill/LimeSDR-QPCIe_1v2-RoundHoles.TXT',
-        'pcb-rnd/power-art.xln',
-        'siemens/80101_0125_F200_ThruHoleNonPlated.ncd',
-        'siemens/80101_0125_F200_ThruHolePlated.ncd',
-        'siemens/80101_0125_F200_ContourPlated.ncd',
-        'Target3001/IRNASIoTbank1.2.Drill',
-        'altium-old-composite-drill.txt',
-        'fritzing/combined.txt',
-        'ncdrill.DRD',
-        'upverter/design_export.drl',
-        'diptrace/mainboard.drl',
-        'diptrace/panel.drl',
-        'diptrace/keyboard.drl',
-        ]
+REFERENCE_FILES = {
+        'easyeda/Gerber_Drill_NPTH.DRL': (None, None),
+        'easyeda/Gerber_Drill_PTH.DRL': (None, 'easyeda/Gerber_TopLayer.GTL'),
+    # Altium uses an excellon format specification format that gerbv doesn't understand, so we have to fix that.
+        'altium-composite-drill/NC Drill/LimeSDR-QPCIe_1v2-SlotHoles.TXT': (('mm', 'leading', 4), None),
+        'altium-composite-drill/NC Drill/LimeSDR-QPCIe_1v2-RoundHoles.TXT': (('mm', 'leading', 4), 'altium-composite-drill/Gerber/LimeSDR-QPCIe_1v2.GTL'),
+        'pcb-rnd/power-art.xln': (None, 'pcb-rnd/power-art.gtl'),
+        'siemens/80101_0125_F200_ThruHoleNonPlated.ncd': (None, None),
+        'siemens/80101_0125_F200_ThruHolePlated.ncd': (None, 'siemens/80101_0125_F200_L01_Top.gdo'),
+        'siemens/80101_0125_F200_ContourPlated.ncd': (None, None),
+        'Target3001/IRNASIoTbank1.2.Drill': (None, 'Target3001/IRNASIoTbank1.2.Top'),
+        'altium-old-composite-drill.txt': (None, None),
+        'fritzing/combined.txt': (None, 'fritzing/combined.gtl'),
+        'ncdrill.DRD': (None, None),
+        'upverter/design_export.drl': (None, 'upverter/design_export.gtl'),
+        'diptrace/mainboard.drl': (None, 'diptrace/mainboard_Top.gbr'),
+        'diptrace/panel.drl': (None, None),
+        'diptrace/keyboard.drl': (None, 'diptrace/keyboard_Bottom.gbr'),
+        }
 
 @filter_syntax_warnings
-@pytest.mark.parametrize('reference', REFERENCE_FILES, indirect=True)
+@pytest.mark.parametrize('reference', list(REFERENCE_FILES.items()), indirect=True)
 def test_round_trip(reference, tmpfile):
+    reference, (unit_spec, _) = reference
     tmp = tmpfile('Output excellon', '.drl')
-    # Altium uses an excellon format specification format that gerbv doesn't understand, so we have to fix that.
-    unit_spec = ('mm', 'leading', 4) if 'altium-composite-drill' in str(reference) else None 
-    # pcb-rnd does not include any unit specification at all
-    if 'pcb-rnd' in str(reference):
-        settings = FileSettings(unit=Inch, zeros='leading', number_format=(2,4))
-    else:
-        settings = None
 
-    ExcellonFile.open(reference, settings=settings).save(tmp)
+    ExcellonFile.open(reference).save(tmp)
 
     mean, _max, hist = gerber_difference(reference, tmp, diff_out=tmpfile('Difference', '.png'), ref_unit_spec=unit_spec)
     assert mean < 5e-5
     assert hist[9] == 0
     assert hist[3:].sum() < 5e-5*hist.size
 
+@filter_syntax_warnings
+@pytest.mark.parametrize('reference', list(REFERENCE_FILES.items()), indirect=True)
+def test_gerber_alignment(reference, tmpfile, print_on_error):
+    reference, (unit_spec, gerber) = reference
+    tmp = tmpfile('Output excellon', '.drl')
+
+    if gerber is None:
+        pytest.skip()
+
+    excf = ExcellonFile.open(reference)
+    gerf_path = reference_path(gerber)
+    print_on_error('Reference gerber file:', gerf_path)
+    gerf = GerberFile.open(gerf_path)
+    print('bounds excellon:', excf.bounding_box(MM))
+    print('bounds gerber:', gerf.bounding_box(MM))
+    excf.save('/tmp/test.xnc')
+
+    flash_coords = []
+    for obj in gerf.objects:
+        if isinstance(obj, Flash):
+            x, y = obj.unit.convert_to(MM, obj.x), obj.unit.convert_to(MM, obj.y)
+            if abs(x - 121.525) < 2 and abs(y - 64) < 2:
+                print(obj)
+            flash_coords.append((x, y))
+
+    tree = KDTree(flash_coords, copy_data=True)
+
+    tolerance = 0.05 # mm
+    matches, total = 0, 0
+    for obj in excf.objects:
+        if isinstance(obj, Flash):
+            if obj.plated in (True, None):
+                total += 1
+                x, y = obj.unit.convert_to(MM, obj.x), obj.unit.convert_to(MM, obj.y)
+                print((x, y), end=' ')
+                if abs(x - 121.525) < 2 and abs(y - 64) < 2:
+                    print(obj)
+                    print('   ', tree.query_ball_point((x, y), r=tolerance))
+                if tree.query_ball_point((x, y), r=tolerance):
+                    matches += 1
+
+    # Some PCB tools, notably easyeda, are dumb and export certain pads as regions, not apertures. Thus, we have to
+    # tolerate some non-matches.
+    assert matches > 10
+    assert matches/total > 0.5
+
 
diff --git a/gerbonara/gerber/tests/utils.py b/gerbonara/gerber/tests/utils.py
index f63f3c6..bf30a1f 100644
--- a/gerbonara/gerber/tests/utils.py
+++ b/gerbonara/gerber/tests/utils.py
@@ -61,8 +61,16 @@ def tmpfile(request):
 
 @pytest.fixture
 def reference(request, print_on_error):
-    ref = reference_path(request.param)
-    yield ref
+    ref = request.param
+    if isinstance(ref, tuple):
+        ref, args = ref
+        ref = reference_path(ref)
+        yield ref, args
+
+    else:
+        ref = reference_path(request.param)
+        yield ref
+
     print_on_error(f'Reference file: {ref}')
 
 def filter_syntax_warnings(fun):
diff --git a/gerbonara/gerber/utils.py b/gerbonara/gerber/utils.py
index fded04a..0627a6b 100644
--- a/gerbonara/gerber/utils.py
+++ b/gerbonara/gerber/utils.py
@@ -44,7 +44,9 @@ class RegexMatcher:
             if (match := re.fullmatch(regex, line)):
                 #print('  handler', handler.__name__)
                 handler(inst, match)
-                break
+                return True
+        else:
+            return False
 
 class LengthUnit:
     def __init__(self, name, shorthand, this_in_mm):