search

KiCad / kicad-library-utils

Some scripts for helping with library development
GNU General Public License v3.0
128 stars 92 forks source link

Check for duplicate lines #286

Closed cpresser closed 5 years ago

cpresser commented 5 years ago

Related #278

cpresser commented 5 years ago

This checks for duplicate/overlapping lines in silkscreen and fab layers. Courtyard layers are checked by #284 and not included in this patch.

poeschlr commented 5 years ago

Merging this would require documenting this also on the website. Would you be up to extending rule 5.1 with this? We can not include any check against undocumented rules ;)

cpresser commented 5 years ago

Imho it would require changes in 5.1 and 5.2 And yes, I can provide a patch for KLC. But I can't seem to find the respository that has the KLC souce.

evanshultz commented 5 years ago

https://github.com/KiCad/kicad-website/tree/master/content/klc

cpresser commented 5 years ago

I just figured that this patch could be considered as incomplete. It only works for lines, but not arcs and circles. Circles are simple; arcs could be done, there is similar logic in #284 that I could reuse.

What do you think?

evanshultz commented 5 years ago

Should this also check courtyard line overlaps? That is the other layer that typically includes lines.

Definitely circles should be included if they're relatively simple. Arcs would be great too but I don't see a reason not to merge lines and circles if they're quick and then do arcs as a second step if you prefer.

poeschlr commented 5 years ago

If we extend this to courtyard (and other layers) then we might want to write a new separate rule that clarifies this instead of adding this to every rule.

This would then mean that this check should also be in a separate file.

cpresser commented 5 years ago

I made another patch to check the courtyard (#284). That's why I did not include the courtyard check in this patch. I was about to postulate that this is already enough. But there are cases where this is not true. A rectangular courtyard which is fully duplicated (8 lines total) will be considered as closed. Right now I am lacking ideas on how to change #284 to detect that as well.

That means this #286 should also check the courtyard layers. Adding a check for circles is trivial; a update for this PR will be ready soon. Arcs seem to be more complicated; as proposed, I will look into that in a second step.

Making a F5.4 rule that checks all layers seems like a good idea. I started a pad to coordinate the text: https://pads.ccc.ac/p/KLC_F5.4

cpresser commented 5 years ago

A changed patch has been (force) pushed. It now checks for duplicate/overlapping lines and for duplicate circles. I have some Ideas for Arcs as well, but would like to do that in a separate PR.

KLC (website) has a PR as well: https://github.com/KiCad/kicad-website/pull/384

poeschlr commented 5 years ago

I think this is ok now. lets test it on the real world.

poeschlr commented 5 years ago

@cpresser it seems your script has false positives if a two lines meet at a point and are in the same angle (so if a long line is split into two smaller lines.)

cpresser commented 5 years ago

@poeschlr Good catch. This is a case were the check failed. A patch to fix this is pretty simple. A PR is here: https://github.com/KiCad/kicad-library-utils/pull/301