matthewlai / JLCKicadTools

Tool for using JLCPCB assembly service with KiCad
GNU General Public License v3.0
346 stars 62 forks source link

Require documentation/examples when changing rotation database? #26

Closed polwel closed 3 years ago

polwel commented 4 years ago

I have used this tool a few times already, and I am loving it.

I still frequently come across components, that are incorrectly rotated. For instance, just now I see a tantalum cap that is reversed (C7171). There is a particular rule about this one package in the database already:

"^CP_Tantalum_Case-A_EIA-3216-18",180

So either

  1. the original rule was incorrect;
  2. JLC are inconsistent even among the same package;
  3. JLC have updated their orientation.

Neither strikes confidence in JLC or JLCKicadTools. The last one would be particularly disastrous for when I'd like to reorder an old design.

I suggest that in the future all changes to cpl_rotations_db.csv are required to be documented with an example component. This could just be a additional column (AFAICT, that is acceptable even without any changes to the current script).

This way, I can at least verify that particular rule.

What's more, there should be a reference board maintained somewhere (perhaps even in this same repo), that includes all of those components. This is important for regression testing: The database features both generic rules (^QFN-) and more specific ones (^(.*?_|V)?QFN-(16|20|24|28|40)(-|_|$)). I can easily see how changes could cascade and break components the author did not intend to touch.

Ideally testing would be automated, but I don't see how that could happen :)

polwel commented 4 years ago

Ok, I just realised that the footprint from the rule above does not even exist in the current library: https://github.com/KiCad/kicad-footprints/tree/master/Capacitor_Tantalum_SMD.pretty

It should be ^CP_EIA-3216-18_*.

It seems like the problem is not on JLC's end. My original point still stands however.

matthewlai commented 4 years ago

JLC changed many of their rotations recently. We changed the database file to match. For example: https://github.com/matthewlai/JLCKicadTools/pull/22

There are also cases where different pin counts of the same footprint have different rotations: https://github.com/matthewlai/JLCKicadTools/pull/23

When JLC changes their rotation, there's really not much we can do about that, until someone notices and fixes JLCKicadTools.

With different pin counts, we can either require each pin count to be verified, which means many people will need to add their own entry, even though most will be consistent.

Our current approach to verification is users are supposed to check the orientation of each component on their board on the JLC site before ordering, and (hopefully) submit a pull request to update the database.

Yes, a reference board would be good. Contribution welcomed :).

polwel commented 4 years ago

Yeah, I have seen those PRs. I appreciate the effort! :)

So far, I've only had one single board where the rotations as provided were good without any tuning. What keeps me from submitting PRs is the lack of confidence that it wouldn't break other parts.

Unless there is a way to demonstrate that my changes are indeed going to improve coverage as a whole, I am going to keep my hacks to myself.

matthewlai commented 4 years ago

Yeah, I have seen those PRs. I appreciate the effort! :)

So far, I've only had one single board where the rotations as provided were good without any tuning. What keeps me from submitting PRs is the lack of confidence that it wouldn't break other parts.

Unless there is a way to demonstrate that my changes are indeed going to improve coverage as a whole, I am going to keep my hacks to myself.

Sure, that is up to you. I don't think any of us are offering any guarantees, and all changes are on a best effort basis. We do still want people to visually check every board before ordering, because JLC changes their rotations, if nothing else.

You can also just submit rotations for specific footprints, if you don't want it to be applied to other variants of the footprint.

JLC (at least for now) is also having a human look at every order, and change rotations where they are obviously wrong. When I contacted them, their advice is to "just order" if in doubt, and their engineer will verify/change, and contact you if the correct orientation is not obvious from silkscreen.

polwel commented 4 years ago

Ok, at least for the tantalum cap in question I am confident I identified the problem 👍

See PR #27 .