mvnmgrx / kiutils

Simple and SCM-friendly KiCad file parser based on Python dataclasses for KiCad 6.0 and up.
GNU General Public License v3.0
78 stars 25 forks source link

Ensure that string is passed to the re.match #91

Closed Frzoen closed 1 year ago

Frzoen commented 1 year ago

Regex r"^(.+?):(.+?)$ is failing to match ID which is just numbers.

If module/footprint name is composed only of digits re.match is failing with error:

parse_symbol_id = re.match(r"^(.+?):(.+?)$", symbol_id)
  File "/usr/lib/python3.10/re.py", line 190, in match
    return _compile(pattern, flags).match(string)
TypeError: expected string or bytes-like object

Change is ensuring that string object is passed to re.match

mvnmgrx commented 1 year ago

Is this a KiCad 5 thing? If i understand correctly, all footprint names are always quoted in the raw S-Expression and thus will always be of the string type in kiutils.

If so, could you provide some example footprint from which i can make a testcase from?

Frzoen commented 1 year ago

Yes. Footprints from KiCad 5 can be used without any "migration" in KiCad6. Sample footprint:

(module 0402 (layer F.Cu) (tedit 5E68CD62)
  (attr smd)
  (fp_text reference R24 (at 0 0 270) (layer B.SilkS) hide
    (effects (font (size 1.524 1.524) (thickness 0.05)) (justify mirror))
  )
  (fp_text value R_0R_0402 (at 0 0 270) (layer B.SilkS) hide
    (effects (font (size 1.524 1.524) (thickness 0.05)) (justify mirror))
  )
  (pad 2 smd rect (at 0.48 0 90) (size 0.62 0.57) (layers F.Cu F.Paste F.Mask))
  (pad 1 smd rect (at -0.48 0 90) (size 0.62 0.57) (layers F.Cu F.Paste F.Mask))
  (model ${KIPRJMOD}/lib/3d-models/0402-res.step
    (at (xyz 0 0 0))
    (scale (xyz 1 1 1))
    (rotate (xyz 0 0 180))
  )
)

It is properly loaded in KiCad6 as module keyword is supported. As You can see name 0402 is not quoted.

mvnmgrx commented 1 year ago

In your case, there is still the small problem of the leading "0" being removed as our S-Expression parser still treats this as a plain number initially.

I would accept your fix for backwards compatibilty to KiCad 5, if loosing a 0 here and there is acceptable.

I've also played around with some more logic in the parser but i haven't come up with a solution that doesn't break existing stuff.

Frzoen commented 1 year ago

Sounds good to me :) I can detect such a case in code and correct for it.

Sorry for the late response

mvnmgrx commented 1 year ago

Alright, i'm going to add a simple test case for this and release it in the next version.

Best regards,