Add JST SH generator script

[nbes4]

This PR adds a generator script for JST SH type connectors.
For more info about the connector and its variants, please see the official datasheet

The generator script generates packages and devices according to the specs and names in the datasheet. The generated devices consist of the generated packages which are linked to PINHEADER components (from the LibrePCB Connectors library).

The output of the generator script generally looks like this:

3D/Silkscreen View (no models yet):

The naming of the packages (e.g. JST SH-BM-05) was based on the package from the Molex LibrePCB library (Molex 53261-06) but I am not sure if this is inline with the naming conventions mentioned in this post on the LibrePCB forum.

[...] but we recommend using IPC naming (uppercase, no spaces) for the package name.

For the device and package category I chose "Direct Wire to Board connector" although the Molex library uses "Pin Headers (Male)" (devcat) and "Molex" (pkgcat) so I'm not sure which one to choose for the JST pkg/dev. Maybe someone could clarify? 😉

Furthermore, I placed a "pad one marking" (silkscreen dot) next to the pads that are defined as "circuit no. 1" in the datasheets. Is this common practice or are there other ways of identifying the no. 1 pad? For example by making the pad corners have a radius of 0 while all others have a radius of 0.5? And speaking of silkscreen, some silkscreen markings extend over the courtyard, is that ok?

Possible improvements (from the top of my head):

• 3D Models
• More detailed documentation layer drawings

Regarding the 3D Models, maybe JST could provide some of them since I saw a "Request 3D Models" button on a webpage of theirs:

@ubruhin maybe you could shoot them an E-Mail as the main dev and explain why and what we'd use them for! 😄

I apologize for any oversights or convention violations, still new around here!
Any feedback/criticism is welcome!

Credits:
Some code is based on the PR from @ollelogdahl which provided a very good starting point for this PR 👍

Edit: Fixed typos

[ubruhin]

Thanks for working on this. I did a very quick look at it, but really not in detail.

The output itself looks not bad. However:

• The side entry package should be rotated 180° to keep a consistent (and intuitive) pad order from top to bottom.
• Legend polygons should be moved tightly to the package outline (no clearance).
• Pin-1 marking should be drawn according IPC7351C, see for example our IC packages. Or just remove it, since the part cannot be assembled in wrong rotation.
• Typically for documentation we use 0.2mm line width, not 0.25.
• The library editor warnings "suspicious function of pad" need to be approved to make our CI pass (the "No 3D model" too, if we still don't have 3D models when this PR is ready to merge).
• The package should be added to the "JST" package category in addition to the current category.
• The keyword "j" is not useful (not sure what it should mean, too short to be meaningful).
• The package names should use the prefix "JST_" (IPC) and the devices should use the prefix "JST " (with space). Ignore the Molex library, it was created a loooong time ago where we didn't have conventions at all.
• Not mandatory, but it would be nice to have at least the pad spacing noted in the descriptions to give users some more information helping to decide for which connector to add to the schematic.
• The concrete MPNs should be added to the devices (not absolutely mandatory, but I think it should be easy and is very valuable information).

Regarding code, to be honest I'm really not happy that you copied all the concepts/style of #98 🙈 It is so completely different to our other generators, I really don't like to have every generator using a very different style because it makes our life hard to maintain them all in the future. I'm not sure how we should proceed now regarding this topic...

@ubruhin maybe you could shoot them an E-Mail as the main dev and explain why and what we'd use them for! 😄

Done. No comment about their response 😉

Vielen Dank für Ihre Nachricht.

Wir stellen unseren Kunden (welche Teile über uns oder unsere Partner beziehen) die STEP Dateien direkt zu.

[nbes4]

Thanks @ubruhin for having a look! 😄

I fixed/implemented almost all of your feedback:

• The side entry package should be rotated 180° to keep a consistent (and intuitive) pad order from top to bottom.
• Legend polygons should be moved tightly to the package outline (no clearance).
• Pin-1 marking should be drawn according IPC7351C, see for example our IC packages. Or just remove it, since the part cannot be assembled in wrong rotation. removed the dot entirely
• Typically for documentation we use 0.2mm line width, not 0.25.
• The library editor warnings "suspicious function of pad" need to be approved to make our CI pass (the "No 3D model" too, if we still don't have 3D models when this PR is ready to merge).
• The package should be added to the "JST" package category in addition to the current category.
• The keyword "j" is not useful (not sure what it should mean, too short to be meaningful). removed it
• The package names should use the prefix "JST_" (IPC) and the devices should use the prefix "JST " (with space).
• Not mandatory, but it would be nice to have at least the pad spacing noted in the descriptions to give users some more information helping to decide for which connector to add to the schematic.
• The concrete MPNs should be added to the devices (not absolutely mandatory, but I think it should be easy and is very valuable information).

Regarding code, to be honest I'm really not happy that you copied all the concepts/style of #98 🙈 It is so completely different to our other generators, I really don't like to have every generator using a very different style because it makes our life hard to maintain them all in the future. I'm not sure how we should proceed now regarding this topic...

Could you provide more detail for this? I don't really see the resemblance to #98 at all anymore apart from some helper functions 🙈 I actually think it looks and works more like generate_axial_tht.py and the likes, the only real difference I see might be that I outsourced more code into functions to help readability a bit. I totally understand your point with maintainability and uniformity though, but as I said I'd appreciate more details as to what concepts/styles you think differ from the other generators 😄

Done. No comment about their response 😉

Vielen Dank für Ihre Nachricht.

Wir stellen unseren Kunden (welche Teile über uns oder unsere Partner beziehen) die STEP Dateien direkt zu.

Well I guess that's a no, which is bit disappointing but kind of expected... Thank you for trying anyways! 🤝

Let me know what you think of the changes if you find time! Output now looks like this:

Top entry:

Side entry:

Edit: Fixed typo, check item no. 5 in list

[rnestler]

CI fails. Please run tests and linter locally and fix the issues they show.

[nbes4]

@rnestler Thanks, we should be good now 👍

[ubruhin]

Awesome, thanks for all the updates! 🎉 I didn't review them yet in detail but I'll try to do so soon.

Could you provide more detail for this? I don't really see the resemblance to #98 at all anymore apart from some helper functions

Yes the helper functions are one thing which I consider as difference to the other generators - in my opinion they actually decrease readability but I guess this is a personal taste 🙈 As a concrete example, honestly I see no reason why to factor out functions like write_pkg() - it is used at only one place and veeery trivial (no conditionals etc) so factoring out only adds more lines of code (function signature, empty lines) without any real benefit.

Let me share my thoughts:

• The component UUID lookup seems to be way more complicated than needed. I'd suggest to check how it is done in generate_idc.py as it looks much simpler. Note that we don't need foolproof error messages like "could not find connector symbols..." as this is a developer tool (not for end-users, we can read exception stacktraces). It's totally fine to just call dict[key] to make the code simpler and shorter (and even less error-prone).
• Same applies to raising exceptions with human readable messages, generally asserts are good enough and simpler.
• There are some helper functions like sanitize_rotation() or align_by_rotation() which don't seem to be related to this generator. If we really need them, it might make more sense to put them in common code so other generators can use them too. However, I'm not sure if we really need them - as far as I remember, we always directly generate the values as appearing in the output file, which sounds much more reasonable to me than generating some intermediate (possibly invalid) value and then making it valid by some additional logic.
• Something similar applies to footprint_shift_to_center() and footprint_rotate_around_center(): I understand why you added them, but to be honest it makes the generator much easier to understand (and maintain) if the final coordinates are generated directly instead of transforming them afterwards. For me, this is actually the main thing I'm struggling with. Maybe @dbrgn or @rnestler have different opinion about that(?).
• Also the footprint_add_*() functions don't look like the concept we use in other generators. If I remember correctly, when we factor out such functions, we make the function returning the generated object (e.g. a polygon) instead of passing the footprint into it. But my memory might be wrong ;) Also this is not that relevant...

Please don't get me wrong - I don't want to be too strict and insist in changing all these things, generally I'm open for other opinions (also from other maintainers).

[nbes4]

Thank you for taking the time - always happy to get more feedback! 😄

I agree with and implemented your points, except for the last two which I would like to justify/discuss.

Regarding the footprint_shift_to_center() and footprint_rotate_around_center(). I somewhat agree with you here (using final coordinates), however it was easier for me to imagine and calculate what I want to draw in my head when using the images from the datasheet as reference and putting them into a "mental" coordinate system. I'd also argue that for future maintenance it is easier to think this way, since we don't have to do any center offset calculations for any intermediate steps in our heads. Furthermore, we don't have to worry about center-positioning even or odd numbered connector pads since everything just expands to the right.

Please see the image below for further clarification about putting the pictures from the datasheet in the coordinate system I'm talking about:

For the footprint_add_* functions, I found the following functions in the current generator scripts:

• generate_qfp.py -> _create_outline_vertices() (returns Vertex list)
• generate_connectors.py -> generate_silkscreen_female() (returns Polygon)
• generate_led.py -> _add_flattened_circle() (returns None, adds circle directly to footprint)

I am happy to change these functions to return the objects they create instead, if wanted. As for why I even factored them out, I think for anyone trying to figure out how the footprint gets generated, the generate_footprint() function is very readable and self-describing and shows step-by-step what's happening. If I want to know how something gets created (e.g. silkscreen) I can just click on the function or easily search and jump there to get the implementation details instead of scrolling through one very big function. This is all just my personal opinion and taste so I guess we could also change this if required. 👍

Opinions welcome 😃

[ubruhin]

That was fast 😃

For the footprint_add_* functions, I found the following functions in the current generator scripts:

Ah good you checked it as I was not completely sure - in this case I'm fine with it anyway 👍

Regarding the coordinate transformations I see your point and maybe it is indeed a valid case. Generally we don't follow datasheet coordinate systems because every manufacturer does it differently while we want a consistent system across all libraries. But so far probably we never had the case that two very similar footprints need to be generated with partially 180° rotated content. Let me think about it... Maybe we also get feedback from other maintainers in the mean time.

Btw. a small side-note (still not done a detailed review): I was confused about the manufacturer name "J.S.T. Mfg. Co., Ltd." which did not look familiar for me so I did some research. It seems this company name is used on their website so it is for sure not wrong. However, in the end this is the name which ends up in the BOM so it is helpful to use the same/similar naming as suppliers use (to increase chances of correct automatic matching). Also for example the upcoming feature LibrePCB/LibrePCB#1313 relies on manufacturer name matching. Unfortunately manufacturer names are often not consistent and JST is not different:

• DigiKey: "JST Sales America Inc."
• WIN SOURCE: "JST Sales America Inc."
• Octopart: "JST"
• Partstack: "J S T Mfg" (sometimes just "JST")

Often I go with the naming of DigiKey because I think it is the de-facto standard and their data quality usually is very good. In this case, maybe just "JST" would be even better. But is seems "J.S.T. Mfg. Co., Ltd." is not used by suppliers so it might not match good with them.

[nbes4]

Regarding the coordinate transformations I see your point and maybe it is indeed a valid case. Generally we don't follow datasheet coordinate systems because every manufacturer does it differently while we want a consistent system across all libraries. But so far probably we never had the case that two very similar footprints need to be generated with partially 180° rotated content. Let me think about it... Maybe we also get feedback from other maintainers in the mean time.

Okay! 👌

[...] I was confused about the manufacturer name "J.S.T. Mfg. Co., Ltd." [...]

Yeah, I agree. I just copied it from the offical site, but to actually use the name used by large suppliers is definitely a better idea. I have never actually seen "J.S.T. Mfg. Co., Ltd." it in the wild 🙈

Regarding LibrePCB/LibrePCB#1313, if the system uses name matching, wouldn't it make sense to add all known manufacturer name variations to the part somehow to get the best chance of automatic matching? I have no clue how the system works underneath, so this is just an idea, but maybe this would be worth considering? Since we know manufacturer names are not consistent and we also know the different variations of the name. Kind of like aliases or "aka.", we have the most common variation used by suppliers as "main name" that gets displayed in the BOM but in the background we have these aliases/variations to help matching.

[ubruhin]

Regarding LibrePCB/LibrePCB#1313, if the system uses name matching, wouldn't it make sense to add all known manufacturer name variations to the part somehow to get the best chance of automatic matching?

Yes it makes sense to store manufacturer aliases, but not on client (i.e. application) side 🙂 The application sends the MPN + manufacturer name to the server and the server replies with the part information so the matching is done server side. Unfortunately these aliases are not available yet so the matching capability is limited. But for long therm this is the goal.

Anyway, this is only one use-case. The most important use-case is exporting the BOM manually, and there the user expects the most common manufacturer name to be used to help finding the correct parts (for example, by CSV upload to a webshop). There aliases also don't help on our side since only one name can (and should) be listed in the BOM.

[ubruhin]

Sorry for the long delay @nbes4 - the LibrePCB 1.1 release had priority... 🙈

I reviewed the changes and the output looks really good!

I think these are the last small things which should be done:

• The generated_by property should be set to an empty string. It is intended for the generator name (not the author of the generator) but we don't use it yet.
• The create_date needs to be specified explicitly, otherwise it will change with every additional generation step.
• The category "Direct Wire to Board Connector" is not correct as it's not a direct connection. I think "Pin Headers (male)" should be OK for these connectors.
• I saw on Digikey they also sell the top-entry connectors with suction cap (TBT suffix in MPN), so it would be nice to add these MPNs too, including an attribute e.g. "FEATURES=Suction Cap" to clarify the difference. Some MPNs for the gold contact variants ("-G-" in MPN) are also on Digikey but not in stock at all so I'm fine with ignoring them for now...
• Afterwards, could you please open a pull request on the JST library to add the newly generated elements? This will also make the CI run to check if there are no invalid files generated.

[nbes4]

@ubruhin All good. I totally understand this, congratulations on the 1.1 release! 🎉 Thank you for your feedback and the review, I'll adjust the PR accordingly in the upcoming days 👍

[nbes4]

@ubruhin I implemented your first two points and changed the category to "Pin Header (Male)". I originally put them in the "Direct Wire to Board Connector" since it's marketed that way on DigiKey (without the direct though)

JST’s SH series wire-to-board connectors offer a high-density solution in a small outline for a multitude of applications. [...]

but I agree, at the end of the day they are pin headers.

However, I am unsure on how to add an attribute to a Part (Python) Entity as the API only exposes mpn and manufacturer in the constructor. Am I missing something here?

Afterwards, could you please open a pull request on the JST library to add the newly generated elements? This will also make the CI run to check if there are no invalid files generated.

Will do once I implemented all of your feedback. 👍

[nbes4]

I apologize, I had to rebase and force-push this branch since I messed something up in my git config. Should not affect anything though. Please delete your local copy of this branch (if you have one) and pull it again. Thanks

[ubruhin]

I originally put them in the "Direct Wire to Board Connector" since it's marketed that way on DigiKey (without the direct though)

Yes I know, but the direct is exactly the thing which makes a big difference 😉 A direct w2b connector is not really a connector but a fixed connection which cannot be plugged off anymore -- thus a separate category for this very special kind of connection.

However, I am unsure on how to add an attribute to a Part (Python) Entity as the API only exposes mpn and manufacturer in the constructor. Am I missing something here?

Oh indeed, seems even attributes in general are not implemented in the generator yet 🙁 Hmm in that case I'd be OK to either add the additional MPNs without attributes (preferred) or even ignore them for now... Up to you.

I apologize, I had to rebase and force-push this branch since I messed something up in my git config.

No problem, I'm okay with rebasing anyway.

[nbes4]

@ubruhin Thanks for the clarification on the W2B connectors - never really paid too much attention to the subtle details! 😅

Hmm in that case I'd be OK to either add the additional MPNs without attributes (preferred) or even ignore them for now... Up to you.

I'll ignore them for this PR, but I opened up another PR for the missing Attributes if you are interested 😄 I'll add them once Attributes are implemented (regardless by which PR).

[ubruhin]

I'll ignore them for this PR, but I opened up another PR for the missing Attributes if you are interested 😄 I'll add them once Attributes are implemented (regardless by which PR).

Awesome! 😃

So then I'm looking forward for the PR in the JST library.

I triggered CI on this PR again because there was a temporary failure on the last run, now it succeeded.