Fix IPC-356 tests

4 files changed, 230 insertions, 49 deletions

diff --git a/gerbonara/gerber/cam.py b/gerbonara/gerber/cam.py
index 83155bd..51612f4 100644
--- a/gerbonara/gerber/cam.py
+++ b/gerbonara/gerber/cam.py
@@ -80,12 +80,16 @@ class FileSettings:
 
     def format_ipc_number(self, value, digits, key='', sign=False):
         if value is None:
-            return ' ' * (digits + 1 + len(key))
+            return ' ' * (digits + int(bool(sign)) + len(key))
 
         if isinstance(value, Enum):
             value = value.value
+        num = format(round(value), f'{"+" if sign else ""}0{digits+int(bool(sign))}d')
 
-        return key + format(round(value), f'{"+" if sign else ""}0{digits+1}d')
+        if len(num) > digits + int(bool(sign)):
+            raise ValueError('Error: Number {num} to wide for IPC-356 field of width {digits}')
+
+        return key + num
 
     def format_ipc_length(self, value, digits, key='', unit=None, sign=False):
         if value is not None:
diff --git a/gerbonara/gerber/ipc356.py b/gerbonara/gerber/ipc356.py
index 940ed72..043f2ab 100644
--- a/gerbonara/gerber/ipc356.py
+++ b/gerbonara/gerber/ipc356.py
@@ -39,21 +39,28 @@ class Netlist(CamFile):
         self.adjacency = adjacency or {}
         self.params = params or {}
 
-    def merge(self, other, our_net_prefix=None, their_net_prefix=None):
+    def merge(self, other, our_prefix=None, their_prefix=None):
         ''' Merge other netlist into this netlist. The respective net names are prefixed with the given prefixes
         (default: None). Garbles other. '''
         if not isinstance(other, Netlist):
             raise TypeError(f'Can only merge Netlist with other Netlist, not {type(other)}')
 
-        self.prefix_nets(our_net_prefix)
-        other.prefix_nets(our_net_prefix)
+        self.prefix_nets(our_prefix)
+        other.prefix_nets(our_prefix)
 
         self.test_records.extend(other.test_records)
         self.conductors.extend(other.conductors)
         self.outlines.extend(other.outlines)
         self.comments.extend(other.comments)
         self.adjacency.update(other.adjacency)
-        self.params.extend(other.params)
+        self.params.update(other.params)
+
+        self.params['JOB'] = 'Gerbonara IPC-356 merge'
+        self.params['TITLE'] = 'Gerbonara IPC-356 merge'
+
+        for key in 'CODE', 'NUM', 'REV', 'VER':
+            if key in self.params:
+                del self.params[key]
 
     def prefix_nets(self, prefix):
         if not prefix:
@@ -68,10 +75,18 @@ class Netlist(CamFile):
                 conductor.net_name = prefix + conductor.net_name
 
         new_adjacency = {}
-        for key in adjacency:
-            new_adjacency[prefix + key] = [ prefix + name for name in adjacency[key] ]
+        for key in self.adjacency:
+            new_adjacency[prefix + key] = [ prefix + name for name in self.adjacency[key] ]
         self.adjacency = new_adjacency
 
+    def offset(self, dx=0,  dy=0, unit=MM):
+        # FIXME
+        pass
+
+    def rotate(self, angle:'radian', center=(0,0), unit=MM):
+        # FIXME
+        pass
+
     @property
     def objects(self):
         yield from self.test_records
@@ -117,18 +132,18 @@ class Netlist(CamFile):
             yield f'P  {name} {value!s}'
 
         net_name_map = {
-                f'NNAME{i}': name for i, name in enumerate(
+                name: f'NNAME{i}' for i, name in enumerate(
                     name for name in self.net_names() if len(name) > 14
                     ) }
 
         yield 'C'
         yield 'C  Net name mapping:'
         yield 'C'
-        for name, value in net_name_map.items():
-            yield f'P  {name} {value!s}'
+        for name, alias in net_name_map.items():
+            yield f'P  {alias} {name}'
 
         yield 'C'
-        yield 'C Test records:'
+        yield 'C  Test records:'
         yield 'C'
 
         for record in self.test_records:
@@ -136,21 +151,21 @@ class Netlist(CamFile):
 
         if self.conductors:
             yield 'C'
-            yield 'C Conductors:'
+            yield 'C  Conductors:'
             yield 'C'
             for conductor in self.conductors:
                 yield from conductor.format(settings, net_name_map)
 
         if self.outlines:
             yield 'C'
-            yield 'C Outlines:'
+            yield 'C  Outlines:'
             yield 'C'
             for outline in self.outlines:
                 yield from outline.format(settings)
 
         if self.adjacency:
             yield 'C'
-            yield 'C Adjacency data:'
+            yield 'C  Adjacency data:'
             yield 'C'
             done = set()
             for net, others in self.adjacency.items():
@@ -217,6 +232,7 @@ class NetlistParser(object):
         self.adjacency = {}
         self.outlines = []
         self.eof = False
+        self.generator = None
 
     def warn(self, msg, kls=SyntaxWarning):
         warnings.warn(f'{self.filename}:{self.start_line}: {msg}', kls)
@@ -255,16 +271,34 @@ class NetlistParser(object):
             return
 
         if self.eof:
-            warnings.warn('Data following IPC-356 End Of File marker')
+            self.warn('Data following IPC-356 End Of File marker')
 
         if line[0] == 'C':
             line = line[2:].strip()
+            #     +-- sic!
+            #     v
+            if 'Ouptut' in line and 'Allegro' in line:
+                self.generator = 'allegro'
+
+            elif 'Ouptut' not in line and 'Allegro' in line:
+                self.warn('This seems to be a file generated by a newer allegro version. Please raise an issue on our '
+                          'issue tracker with your Allegro version and if possible please provide an example file '
+                          'so we can improve Gerbonara!')
+
+            elif 'EAGLE' in line and 'CadSoft' in line:
+                self.generator = 'eagle'
 
             if line.strip().startswith('NNAME'):
                 name, *value = line.strip().split()
                 value = ' '.join(value)
-                warnings.warn('File contains non-standard Allegro-style net name alias definitions in comments.')
-                self.net_names[name] = value
+                self.warn('File contains non-standard Allegro-style net name alias definitions in comments.')
+                if self.generator == 'allegro':
+                    # it's amazing how allegro always seems to have found a way to do the same thing everyone else is
+                    # doing just in a different, slightly more messed up, completely incompatible way.
+                    self.net_names[name] = value[5:] # strip NNAME because Allegro
+
+                else:
+                    self.net_names[name] = value
 
             else:
                 self.comments.append(line)
@@ -294,7 +328,11 @@ class NetlistParser(object):
                     raise SyntaxError(f'Unsupported IPC-356 netlist unit specification "{line}"')
 
             elif name.startswith('NNAME'):
-                self.net_names[name] = value
+                if self.generator == 'allegro':
+                    self.net_names[name] = value[5:]
+
+                else:
+                    self.net_names[name] = value
 
             else:
                 self.params[name] = value
@@ -319,10 +357,10 @@ class NetlistParser(object):
 
         elif line[0:3] == '389':
             self.assert_unit()
-            self.outlines.append(Outline.parse(line, self.settings))
+            self.outlines.extend(Outline.parse(line, self.settings))
 
         else:
-            warnings.warn(f'Unknown IPC-356 record type {line[0:3]}')
+            self.warn(f'Unknown IPC-356 record type {line[0:3]}')
 
 
 class PadType(Enum):
@@ -345,6 +383,7 @@ class TestRecord:
     __test__ = False # tell pytest to ignore this class
     pad_type : PadType = None
     net_name : str = None
+    is_connected : bool = True # None, True or False.
     ref_des : str = None # part reference designator, e.g. "C1" or "U69"
     is_via : bool = False
     pin_num : int = None
@@ -356,10 +395,11 @@ class TestRecord:
     y : float = None
     w : float = None
     h : float = None
-    rotation : float = None
+    rotation : float = 0
     solder_mask : SoldermaskInfo = None
     lefover : str = None
-    unit: KW_ONLY = None
+    _ : KW_ONLY
+    unit : LengthUnit = None
 
     def __str__(self):
         x = self.unit.format(self.x)
@@ -373,9 +413,23 @@ class TestRecord:
 
         obj.unit = settings.unit
         obj.pad_type = PadType(int(line[1]))
+
         net_name = line[3:17].strip() or None
-        obj.net_name = net_name_map.get(net_name, net_name)
-        obj.ref_des = line[20:26].strip() or None
+        if net_name == 'N/C':
+            obj.net_name = None
+            obj.is_connected = False
+        else:
+            obj.net_name = net_name_map.get(net_name, net_name)
+            obj.is_connected = True
+
+        ref_des = line[20:26].strip() or None
+        if ref_des == 'VIA':
+            obj.is_via = True
+            obj.ref_des = None
+        else:
+            obj.is_via = False
+            obj.ref_des = ref_des
+
         obj.pin = line[27:31].strip() or None
 
         if line[31] == 'M':
@@ -395,21 +449,26 @@ class TestRecord:
         if line[62] == 'Y':
             obj.h = settings.parse_ipc_length(line[63:67])
         if line[67] == 'R':
-            obj.h = math.radians(int(line[68:71]))
+            obj.rotation = math.radians(int(line[68:71]))
+        else:
+            obj.rotation = 0
         if line[72] == 'S':
             obj.solder_mask = SoldermaskInfo(int(line[73]))
         obj.leftover = line[74:].strip() or None
 
         return obj
 
-    def format(self, settings, net_name_map):
+    def format(self, settings, net_name_map={}):
         x = settings.unit(self.x, self.unit)
         y = settings.unit(self.y, self.unit)
         w = settings.unit(self.w, self.unit)
         h = settings.unit(self.h, self.unit)
         # TODO: raise warning if any string is too long
         ref_des = 'VIA' if self.is_via else (self.ref_des or '')
-        net_name = net_name_map.get(self.net_name, self.net_name)
+        if self.is_connected:
+            net_name = net_name_map.get(self.net_name, self.net_name)
+        else:
+            net_name = 'N/C'
 
         yield ''.join((
             '3',
@@ -427,7 +486,7 @@ class TestRecord:
             settings.format_ipc_length(self.x, 6, 'X', self.unit, sign=True),
             settings.format_ipc_length(self.y, 6, 'Y', self.unit, sign=True),
             settings.format_ipc_length(self.w, 4, 'X', self.unit),
-            settings.format_ipc_length(self.y, 4, 'Y', self.unit),
+            settings.format_ipc_length(self.h, 4, 'Y', self.unit),
             settings.format_ipc_number(math.degrees(self.rotation) if self.rotation is not None else None, 3, 'R'),
             ' ',
             settings.format_ipc_number(self.solder_mask, 1, 'S'),
@@ -445,7 +504,7 @@ def parse_coord_chain(line, settings):
     for segment in line.split('*'):
         coords = []
         for coord in segment.strip().split():
-            if not (m := re.match(r'(X[+-]?[0-9]+)?(Y[+-]?[0-9]+)?', coord)):
+            if not (match := re.match(r'(X[+-]?[0-9]+)?(Y[+-]?[0-9]+)?', coord)):
                 raise SyntaxError(f'Invalid IPC-356 coordinate {coord}')
 
             x = settings.parse_ipc_length(match[1], x)
@@ -457,17 +516,17 @@ def parse_coord_chain(line, settings):
             coords.append((x, y))
         yield coords
 
-def format_coord_chain(line, settings, coords, cont):
+def format_coord_chain(line, settings, coords, cont, unit):
     for x, y in coords:
-        coord = settings.format_ipc_length(x, 6, 'X', unit=self.unit, sign=True)
-        coord += settings.format_ipc_length(y, 6, 'Y', unit=self.unit, sign=True)
+        coord = settings.format_ipc_length(x, 6, 'X', unit=unit, sign=True)
+        coord += settings.format_ipc_length(y, 6, 'Y', unit=unit, sign=True)
 
         if len(line) + len(coord) <= 80:
-            line += coord
+            line = (line + coord + ' ')[:80]
 
         else:
             yield line
-            line = f'{cont} {coord}'
+            line = f'{cont} {coord} '
     yield line
 
 
@@ -475,17 +534,20 @@ def format_coord_chain(line, settings, coords, cont):
 class Outline:
     outline_type : OutlineType
     outline : [(float,)]
-    unit : KW_ONLY
+    _ : KW_ONLY
+    unit : LengthUnit = None
 
     @classmethod
     def parse(kls, line, settings):
+        print('parsing outline', line)
         outline_type = OutlineType[line[3:17].strip()]
         for outline in parse_coord_chain(line[22:], settings):
+            print(' ->', outline)
             yield kls(outline_type, outline, unit=settings.unit)
 
     def format(self, settings):
         line = f'389{self.outline_type.name:<14}     '
-        yield from format_coord_chain(line, settings, self.outline, '089')
+        yield from format_coord_chain(line, settings, self.outline, '089', self.unit)
 
     def __str__(self):
         return f'<IPC-356 {self.outline_type.name} outline with {len(self.outline)} points>'
@@ -497,7 +559,8 @@ class Conductor:
     layer : int
     aperture : (float,)
     coords : [(float,)]
-    unit : KW_ONLY
+    _ : KW_ONLY
+    unit : LengthUnit = None
 
     @classmethod
     def parse(kls, line, settings, net_name_map={}):
@@ -520,7 +583,7 @@ class Conductor:
         net_name = net_name_map.get(self.net_name, self.net_name)
         net_name = f'{net_name:<14}[:14]'
         line = f'378{net_name} L{self.layer:02d} '
-        yield from format_coord_chain(line, settings, self.outline, '078')
+        yield from format_coord_chain(line, settings, self.outline, '078', self.unit)
 
     def __str__(self):
         return f'<IPC-356 conductor {self.net_name} with {len(self.coords)} points>'
diff --git a/gerbonara/gerber/tests/test_ipc356.py b/gerbonara/gerber/tests/test_ipc356.py
index 2a39a79..a49b243 100644
--- a/gerbonara/gerber/tests/test_ipc356.py
+++ b/gerbonara/gerber/tests/test_ipc356.py
@@ -32,10 +32,18 @@ def test_idempotence(reference, tmpfile):
     tmp_1 = tmpfile('First generation output', '.ipc')
     tmp_2 = tmpfile('Second generation output', '.ipc')
 
-    Netlist.open(reference).save(tmp_1)
-    Netlist.open(tmp_1).save(tmp_2)
+    a = Netlist.open(reference)
+    a.save(tmp_1)
+    b = Netlist.open(tmp_1)
+    b.save(tmp_2)
+
+    print(f'{a.outlines=}')
+    print(f'{b.outlines=}')
 
-    assert tmp_1.read_text() == tmp_2.read_text()
+    res = tmp_1.read_text() == tmp_2.read_text()
+    # Confuse pytest so it doesn't try to print out a diff. pytest's potato diff algorithm is wayyyy to slow and would
+    # hang for several minutes.
+    assert res
 
 @filter_syntax_warnings
 @pytest.mark.parametrize('reference', REFERENCE_FILES, indirect=True)
@@ -50,12 +58,118 @@ def test_bells_and_whistles(reference):
     netlist.conductors_by_layer(0)
 
 @filter_syntax_warnings
-@pytest.mark.parametrize('reference', REFERENCE_FILES, indirect=True)
-@pytest.mark.parametrize('other', REFERENCE_FILES, indirect=True)
-def test_merge(reference, other):
-    other = reference_path(other)
-    a = Netlist.open(reference)
-    b = Netlist.open(other)
+@pytest.mark.parametrize('a', REFERENCE_FILES)
+@pytest.mark.parametrize('b', REFERENCE_FILES)
+def test_merge(a, b):
+    a, b = reference_path(a), reference_path(b)
+    print('File A:', a)
+    print('File B:', a)
+
+    a = Netlist.open(a)
+    b = Netlist.open(b)
     a.merge(b, our_prefix='A')
     # FIXME asserts
 
+def test_record_semantics():
+    settings = FileSettings()
+    r = TestRecord.parse('327m0002            CPU1  -AY30       A01X+020114Y+014930X0120Y    R090 S1      ', settings)
+    assert r.pad_type == PadType.SMD_PAD
+    assert r.net_name == 'm0002'
+    assert r.is_connected
+    assert r.ref_des == 'CPU1'
+    assert not r.is_via
+    assert r.pin == 'AY30'
+    assert not r.is_middle
+    assert r.hole_dia is None
+    assert r.is_plated is None
+    assert r.access_layer == 1
+    assert math.isclose(r.x, 20114/1000) and math.isclose(r.y, 14930/1000)
+    assert math.isclose(r.w, 120/1000) and r.h is None
+    assert math.isclose(r.rotation, math.pi/2)
+    assert r.solder_mask == SoldermaskInfo.PRIMARY
+    assert r.unit == settings.unit
+
+    r = TestRecord.parse('327m0002            U15   -D3         A01X+011545Y+003447X0090Y    R090 S1      ', settings)
+    assert r.pad_type == PadType.SMD_PAD
+    assert r.net_name == 'm0002'
+    assert r.is_connected
+    assert r.ref_des == 'U15'
+    assert r.pin == 'D3'
+    assert not r.is_middle
+    assert r.hole_dia is None
+    assert r.is_plated is None
+    assert r.access_layer == 1
+    assert math.isclose(r.w, 90/1000) and r.h is None
+
+    r = TestRecord.parse('327VSUMPG           C39   -2   M      A01X+013050Y+020050X0350Y0320R270 S1      ', settings)
+    assert r.pad_type == PadType.SMD_PAD
+    assert r.net_name == 'VSUMPG'
+    assert r.is_connected
+    assert r.ref_des == 'C39'
+    assert r.pin == '2'
+    assert r.is_middle
+    assert r.hole_dia is None
+    assert r.is_plated is None
+    assert r.access_layer == 1
+    assert math.isclose(r.w, 350/1000) and math.isclose(r.h, 320/1000)
+    assert math.isclose(r.rotation, math.pi*3/2)
+
+    r = TestRecord.parse('327N/C              CPU1  -AD2        A01X+023191Y+020393X0110Y    R090 S1      ', settings)
+    assert r.pad_type == PadType.SMD_PAD
+    assert r.net_name == None
+    assert not r.is_connected
+    assert r.ref_des == 'CPU1'
+    assert r.pin == 'AD2'
+    assert r.hole_dia is None
+    assert r.is_plated is None
+    assert r.access_layer == 1
+    assert math.isclose(r.w, 110/1000) and r.h is None
+
+    r = TestRecord.parse('317m0002            VIA   -    MD0080PA00X+011900Y+004000X0160Y         S3      ', settings)
+    assert r.pad_type == PadType.THROUGH_HOLE
+    assert r.net_name == 'm0002'
+    assert r.is_connected
+    assert r.ref_des is None
+    assert r.is_via
+    assert r.pin is None
+    assert r.is_middle
+    assert r.hole_dia == 80/1000
+    assert r.is_plated
+    assert r.access_layer == 0
+    assert math.isclose(r.w, 160/1000) and r.h is None
+    assert r.rotation == 0
+    assert r.solder_mask == SoldermaskInfo.BOTH
+
+    r = TestRecord.parse('317GND              VIA   -    MD0080PA00X+023800Y+010100X0160Y         S0      ', settings)
+    assert r.pad_type == PadType.THROUGH_HOLE
+    assert r.net_name == 'GND'
+    assert r.is_connected
+    assert r.is_via
+    assert r.pin is None
+    assert r.hole_dia == 80/1000
+    assert r.is_plated
+    assert r.access_layer == 0
+    assert r.solder_mask == SoldermaskInfo.NONE
+
+def test_record_idempotence():
+    records = [
+            '327m0002            CPU1  -AY30       A01X+020114Y+014930X0120Y    R090 S1      ',
+            '327m0002            U15   -D3         A01X+011545Y+003447X0090Y    R090 S1      ',
+            '327VSUMPG           C39   -2   M      A01X+013050Y+020050X0350Y0320R270 S1      ',
+            '317m0002            VIA   -    MD0080PA00X+011900Y+004000X0160Y         S3      ',
+            '317GND              VIA   -    MD0080PA00X+023800Y+010100X0160Y         S0      ',]
+
+    for unit in MM, Inch:
+        settings = FileSettings(unit=unit)
+        for record in records:
+            ra = TestRecord.parse(record, settings)
+            a = list(ra.format(settings))[0]
+            rb = TestRecord.parse(a, settings)
+            b = list(rb.format(settings))[0]
+            print('ra', ra)
+            print('rb', rb)
+            print('0', record)
+            print('a', a)
+            print('b', b)
+            assert a == b
+
diff --git a/gerbonara/gerber/utils.py b/gerbonara/gerber/utils.py
index 1aad900..6ea8bcb 100644
--- a/gerbonara/gerber/utils.py
+++ b/gerbonara/gerber/utils.py
@@ -76,7 +76,7 @@ class LengthUnit:
         return unit.convert_from(self, value)
 
     def format(self, value):
-        return f'{value:.3f}{self.shorthand}'
+        return f'{value:.3f}{self.shorthand}' if value is not None else ''
 
     def __call__(self, value, unit):
         return self.convert_from(unit, value)