Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

Frzoen
Copy link
Contributor

@Frzoen Frzoen commented May 3, 2023

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

 r"^(.+?):(.+?)$ is failing to match ID which is just numbers
@mvnmgrx
Copy link
Owner

mvnmgrx commented May 23, 2023

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
Copy link
Contributor Author

Frzoen commented Jun 2, 2023

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 mvnmgrx added this to the Legacy Stuff milestone Jun 4, 2023
@mvnmgrx
Copy link
Owner

mvnmgrx commented Jun 20, 2023

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
Copy link
Contributor Author

Frzoen commented Jul 17, 2023

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

Sorry for the late response

@mvnmgrx
Copy link
Owner

mvnmgrx commented Aug 15, 2023

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

Best regards,

@mvnmgrx mvnmgrx merged commit 30d92b0 into mvnmgrx:master Aug 15, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants