Skip to content
New issue

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

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

Already on GitHub? Sign in 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants