dvc94ch / pykicad

Library for working with KiCAD file formats
ISC License
64 stars 18 forks source link

parse error loading this module #11

Closed wez closed 7 years ago

wez commented 7 years ago

Here's the error:

Traceback (most recent call last):
  File "./kbdmakerutils", line 1667, in <module>
    circuit = kicad_schematic(physical_matrix, shapes)
  File "./kbdmakerutils", line 1180, in kicad_schematic
    cdiode = circuit.diode()
  File "/Users/wez/src/kbdmakerutils/circuitlib.py", line 296, in diode
    'Diodes_THT:D_DO-41_SOD81_P7.62mm_Horizontal')
  File "/Users/wez/src/kbdmakerutils/circuitlib.py", line 229, in part
    module = self.footprintpcb.parseFootprintModule(footprint)
  File "/Users/wez/src/kbdmakerutils/kicadpcb.py", line 128, in parseFootprintModule
    module = pykicad.module.Module.from_file('%s/%s.kicad_mod' % (lib, compname))
  File "pykicad/pykicad/module.py", line 362, in from_file
    return cls.parse(module)
  File "pykicad/pykicad/sexpr.py", line 293, in parse
    parse_result = cls._parser.parseString(string)
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1622, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 3395, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 3672, in parseImpl
    loc,results = e._parse(instring,loc,doActions)
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 3983, in parseImpl
    loc, tokens = self.expr._parse( instring, loc, doActions, callPreParse=False )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1405, in _parseNoCache
    tokens = fn( instring, tokensStart, retTokens )
  File "./deps/usr/local/lib/python2.7/site-packages/pyparsing.py", line 1049, in wrapper
    ret = func(*args[limit[0]:])
  File "pykicad/pykicad/sexpr.py", line 50, in action
    return {attr: ast(**parse_action(tokens[0]))}
TypeError: __init__() got an unexpected keyword argument 'xyz'
make: *** [all] Error 1

Here's the module:

(module D_DO-41_SOD81_P7.62mm_Horizontal (layer F.Cu) (tedit 5877C982)
  (descr "D, DO-41_SOD81 series, Axial, Horizontal, pin pitch=7.62mm, , length*diameter=5.2*2.7mm^2, , http://www.diodes.com/_files/packages/DO-41%20(Plastic).pdf")
  (tags "D DO-41_SOD81 series Axial Horizontal pin pitch 7.62mm  length 5.2mm diameter 2.7mm")
  (fp_text reference REF** (at 3.81 -2.41) (layer F.SilkS)
    (effects (font (size 1 1) (thickness 0.15)))
  )
  (fp_text value D_DO-41_SOD81_P7.62mm_Horizontal (at 3.81 2.41) (layer F.Fab)
    (effects (font (size 1 1) (thickness 0.15)))
  )
  (fp_line (start 1.21 -1.35) (end 1.21 1.35) (layer F.Fab) (width 0.1))
  (fp_line (start 1.21 1.35) (end 6.41 1.35) (layer F.Fab) (width 0.1))
  (fp_line (start 6.41 1.35) (end 6.41 -1.35) (layer F.Fab) (width 0.1))
  (fp_line (start 6.41 -1.35) (end 1.21 -1.35) (layer F.Fab) (width 0.1))
  (fp_line (start 0 0) (end 1.21 0) (layer F.Fab) (width 0.1))
  (fp_line (start 7.62 0) (end 6.41 0) (layer F.Fab) (width 0.1))
  (fp_line (start 1.99 -1.35) (end 1.99 1.35) (layer F.Fab) (width 0.1))
  (fp_line (start 1.15 -1.28) (end 1.15 -1.41) (layer F.SilkS) (width 0.12))
  (fp_line (start 1.15 -1.41) (end 6.47 -1.41) (layer F.SilkS) (width 0.12))
  (fp_line (start 6.47 -1.41) (end 6.47 -1.28) (layer F.SilkS) (width 0.12))
  (fp_line (start 1.15 1.28) (end 1.15 1.41) (layer F.SilkS) (width 0.12))
  (fp_line (start 1.15 1.41) (end 6.47 1.41) (layer F.SilkS) (width 0.12))
  (fp_line (start 6.47 1.41) (end 6.47 1.28) (layer F.SilkS) (width 0.12))
  (fp_line (start 1.99 -1.41) (end 1.99 1.41) (layer F.SilkS) (width 0.12))
  (fp_line (start -1.35 -1.7) (end -1.35 1.7) (layer F.CrtYd) (width 0.05))
  (fp_line (start -1.35 1.7) (end 9 1.7) (layer F.CrtYd) (width 0.05))
  (fp_line (start 9 1.7) (end 9 -1.7) (layer F.CrtYd) (width 0.05))
  (fp_line (start 9 -1.7) (end -1.35 -1.7) (layer F.CrtYd) (width 0.05))
  (pad 1 thru_hole rect (at 0 0) (size 2.2 2.2) (drill 1.1) (layers *.Cu *.Mask))
  (pad 2 thru_hole oval (at 7.62 0) (size 2.2 2.2) (drill 1.1) (layers *.Cu *.Mask))
  (model Diodes_THT.3dshapes/D_DO-41_SOD81_P7.62mm_Horizontal.wrl
    (at (xyz 0 0 0))
    (scale (xyz 0.393701 0.393701 0.393701))
    (rotate (xyz 0 0 0))
  )
)
wez commented 7 years ago

This seems to make things get further along:

$ git diff
diff --git a/pykicad/sexpr.py b/pykicad/sexpr.py
index e66b497..2bb50e1 100644
--- a/pykicad/sexpr.py
+++ b/pykicad/sexpr.py
@@ -109,7 +109,7 @@ def generate_parser(tag, schema, attr=None, optional=False):
         children = []
         for key, value in schema.items():
             if not (key.isdigit() or key[0] == '_'):
-                attr = None
+                attr = schema.get('_attr', None)
                 if type(value) == type:
                     attr = key
                 children.append(generate_parser(key, value, attr=attr, optional=True))
dvc94ch commented 7 years ago

that is really weird, are you using the latest commit? I'm not suppressing anything anymore...

The testfile parses fine for me...?!

dvc94ch commented 7 years ago

I'm hitting an issue with python2. Let me see if I can reproduce with python2...

dvc94ch commented 7 years ago

Here's a generated pcb file using python2 and your module:

screenshot from 2017-04-22 22-42-51

I created a test environment using guix environment --ad-hoc python@2 python2-pyparsing kicad --pure to make sure there is nothing in my path messing with things

wez commented 7 years ago

I'm based on rev 0d15e39 (looks like that is still the latest)

wez commented 7 years ago

This is with the macOS python, not sure if that matters:

$ /usr/bin/python
Python 2.7.10 (default, Jul 30 2016, 19:40:32)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

adding in some debugging showed that the Model was seeing (at (xyz 0 0 0)) as xyz: [0,0,0]; in other words, the attribute name was missing, which is why I tried adding that explicit check for _attr in the comment above.

dvc94ch commented 7 years ago

The schema parser is a pain to debug and understand I know. Probably was not worth the complexity, probably won't be writing parser generators again. Anyway I think it's nearly there, so starting from scratch at this point is probably a bad idea.

What happens when you run the unit tests? They are really good at detecting errors and are a life saver with this kind of code.

wez commented 7 years ago

I added a test case:

diff --git a/tests/test_module.py b/tests/test_module.py
index 7f0d0e4..d58f496 100644
--- a/tests/test_module.py
+++ b/tests/test_module.py
@@ -1,3 +1,4 @@
+from __future__ import unicode_literals
 import unittest
 from pytest import *
 from pykicad.module import *
@@ -112,3 +113,13 @@ class NetTests(unittest.TestCase):
         assert n1.code == 1
         assert n2.code == 2
         assert n3.code == 3
+
+class ModelTests(unittest.TestCase):
+    def test_model(self):
+        model_string = (
+            '\n(model Diodes_THT.3dshapes/D_DO-41_SOD81_P7.62mm_Horizontal.wrl \n'
+            '    (at (xyz 0 0 0)) \n'
+            '    (scale (xyz 0.3937010000 0.3937010000 0.3937010000)) \n'
+            '    (rotate (xyz 0 0 0)))''')
+        model = Model.parse(model_string)
+        self.assertEqual(model_string, model.to_string())
diff --git a/tests/test_sexpr.py b/tests/test_sexpr.py
index 80f8e2e..ce2abde 100644
--- a/tests/test_sexpr.py
+++ b/tests/test_sexpr.py
@@ -1,3 +1,5 @@
+# vim: set fileencoding=utf-8
+from __future__ import unicode_literals
 import unittest
 from pytest import *
 from pyparsing import ParseException
wez commented 7 years ago

This makes it easier to see the error:

$ git diff
diff --git a/pykicad/sexpr.py b/pykicad/sexpr.py
index 27004b4..ae37c4a 100644
--- a/pykicad/sexpr.py
+++ b/pykicad/sexpr.py
@@ -308,7 +308,10 @@ class AST(object):
                     result[key] = [result[key]]
                 result[key].append(res[key])

-        return cls(**result)
+        try:
+            return cls(**result)
+        except TypeError as e:
+            raise TypeError('%r: %s (%r)' % (cls, e, result))

     @classmethod
     def from_schema(cls, tag, schema):
ERROR: test_model (test_module.ModelTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/wez/src/kbdmakerutils/pykicad/tests/test_module.py", line 124, in test_model
    model = Model.parse(model_string)
  File "pykicad/sexpr.py", line 314, in parse
    raise TypeError('%r: %s (%r)' % (cls, e, result))
TypeError: <class 'pykicad.module.Model'>: __init__() got an unexpected keyword argument 'xyz' ({'path': u'Diodes_THT.3dshapes/D_DO-41_SOD81_P7.62mm_Horizontal.wrl', 'xyz': [0.0, 0.0, 0.0, [0.393701, 0.393701, 0.393701], [0.0, 0.0, 0.0]]})
wez commented 7 years ago

Note that the change I suggested in https://github.com/dvc94ch/pykicad/issues/11#issuecomment-296396424 allows the file to be compiled, but when you dump out the resulting module, the model stanza loses the xyz level of nesting and the resultant pcb file fails to parse when loaded by kicad.

wez commented 7 years ago

For now, I'm working around this with the the one-liner I suggested above, and to make sure that I get a valid pcb, I'm setting module.model = None before I save it.

dvc94ch commented 7 years ago

If you submit a PR for the test I'll fix it on Wednesday... I still don't understand why I wasn't getting that error dough...

dvc94ch commented 7 years ago

I think this is fixed now, could you let me know if you are still having this issue?

wez commented 7 years ago

Finally got around to looking at this again; with master in github this error is resolved, but not with the currently released package installed via pip