Skip to content
This repository has been archived by the owner on Oct 2, 2020. It is now read-only.

Audio jack fix #1014

Merged
merged 4 commits into from
Dec 3, 2018
Merged

Audio jack fix #1014

merged 4 commits into from
Dec 3, 2018

Conversation

evanshultz
Copy link
Collaborator

@evanshultz evanshultz commented Oct 12, 2018

Fixes #1012.
Related to #191 (comment).

The intent here is the fix the pin numbers so CvPcb will at least find the footprints; I didn't check dimensions or do other work.

I couldn't find a datasheet for PJ311 but hopefully someone can; if so I'll be happy to include it here.

Since this is a bug, can I get a quick review?

image

Datasheets:
Jack_3.5mm_Ledino_KB3SPRS_Horizontal
https://www.reichelt.de/index.html?ACTION=7&LA=3&OPEN=0&INDEX=0&FILENAME=C160%252FKB3SPRS.pdf
Jack_3.5mm_Neutrik_NMJ6HCD2_Horizontal
NMJ6HCD2, TRS 1/4in (http://www.neutrik.com/en/audio/plugs-and-jacks/m-series/nmj6hcd2
Jack_3.5mm_PJ320D_Horizontal
http://www.qingpu-electronics.com/en/products/WQP-PJ320D-72.html
Jack_3.5mm_PJ320E_Horizontal
http://www.qingpu-electronics.com/en/products/WQP-PJ320E-177.html
Jack_3.5mm_PJ31060-I_Horizontal
http://www.china-bsun.com/Product48/1577.html
Jack_3.5mm_QingPu_WQP-PJ398SM_Vertical_CircularHoles
http://www.qingpu-electronics.com/en/products/WQP-PJ398SM-362.html


Thanks for creating a pull request to contribute to the KiCad libraries! To speed up integration of your PR, please check the following items:

  • Provide a URL to a datasheet for the footprint(s) you are contributing
  • An example screenshot image is very helpful
  • If there are matching symbol or 3D model pull requests, provide link(s) as appropriate
  • Check the output of the Travis automated check scripts - fix any errors as required

-Remove unknown jack
-Change pin designators on other to TRS
-Add datasheets where possible
-Fix typo of Neutrik
@evanshultz evanshultz added the Bug Fix footprint existing in the library label Oct 12, 2018
@evanshultz
Copy link
Collaborator Author

As 5.0.2 is going to be released soon, and I assume we can tag and update the libs, can I get a quick review here so this can be included?

@evanshultz
Copy link
Collaborator Author

Can I get a review on this? Otherwise the symbols aren't able to match with these footprints.

@evanshultz
Copy link
Collaborator Author

Can someone review and merge this?

@evanshultz
Copy link
Collaborator Author

evanshultz commented Nov 9, 2018

I found product pages from manufacturers that appear to match these footprints and updated them.

Can I please get someone to review this and push it to fix this bug? The 5.0.2 release is being planned and I would like this included since our current symbols don't match any of these valid footprints!

Edit: In the fullness of time (after 5.0.2 or 5.1 are released?) these footprints can be renamed to add the vendor name.

@Misca1234
Copy link
Collaborator

I can check it tomorrow night

@antoniovazquezblanco antoniovazquezblanco added this to the 5.0.2 milestone Nov 20, 2018
@Misca1234
Copy link
Collaborator

Just a minor detail on Jack_3.5mm_Neutrik_NMJ6HCD2_Horizontal,
I think the "boxes" at the ends are on wrong sides, pin SN and S is close to the side where the jack is mounted and this side, with the nut, is wider than the "back end", but in the foot print the widest box is closest to TN/T

PJ31060-I 6pin SMD 3.5mm headphones jack:
You have renamed pin 1 into S, but same pin in Jack_3.5mm_Ledino_KB3SPRS_Horizontal is named T

For Jack_3.5mm_PJ320D_Horizontal same pin (Jack_3.5mm_Ledino_KB3SPRS_Horizontal is named T) is named R1

Jack_3.5mm_PJ320E_Horizontal pin 5 have "lost" is re numbering (empty string)

@evanshultz
Copy link
Collaborator Author

Thanks for the review. There was no documentation for any of these connectors, so perhaps there are other datasheets which apply. If that is the case, then please help me to find them. The documentation I found matched the footprints and so I made them match the documents I found.

NMJ6HCD2:
I think you are right. Here is the drawing:
image

I fixed it:
image

PJ31060-I:
Here is the relevant snippet of the website showing the pinout and schematic:
image

I don't see any issue and I don't know why you're comparing this to the Ledino connector.

PJ320D:
image

I have corrected the footprint's pinout (swapping S and R2 pins):
image

If there is another issue please point it out. I don't see why the Ledino connector is relevant.

PJ320E:
image

Pin 5 is a dummy pin so it should not have a pin number:
image

@Misca1234
Copy link
Collaborator

Misca1234 commented Nov 24, 2018

I don't see any issue and I don't know why you're comparing this to the Ledino connector.

The reason is that almost all jackets have schematics the look like this

bild

Like the
https://cdn-reichelt.de/documents/datenblatt/C160/KB3SPRS.pdf

For the C160/KB3SPRS it is numbered like this
bild

In your make over, you have, for C160/KB3SPRS, re named pin one to 'T'.
So, when other symbols have the same thingy in their drawings,
bild

Should they not also be renamed into 'T' ?

@evanshultz
Copy link
Collaborator Author

The pin numbering means nothing and can vary from vendor to vendor. It's the part of the jack (on the cable) it connects to that is important, and we're moved to numbers to abstract that into a generic symbol (and of course the pin naming needs to match on the footprints).

Take a look at https://en.wikipedia.org/wiki/Phone_connector_(audio). The last image you posted above is of the Sleeve, so it should be "S".

For jacks which pass through when no cable is inserted, we use "*N" terminology where the asterisk is "S" or "T" as this is common. The "N" stands for "normalling".

So, in the middle screenshot you sent, here is the pinout we would use:

  1. S
  2. R
  3. T
  4. RN
  5. TN

Does that clear it up?

@evanshultz
Copy link
Collaborator Author

Is this ready to merge now?

@evanshultz
Copy link
Collaborator Author

@Misca1234 @antoniovazquezblanco
Are either of you able to take a look at this?

@antoniovazquezblanco
Copy link
Collaborator

@Misca1234 you had a look at this. Do you feel anything else is missing? I feel quite lazy about starting a review on this right now 😅

@evanshultz
Copy link
Collaborator Author

Guys. It's almost been a week since Wayne tagged 5.0.2 and if we're to release a new set of libs this should be fixed. It's already been reported by users.

@Misca1234
Please review the above as your highest priority task in the next day.

@antoniovazquezblanco
If there's no action would you please take a look? It's not too much to review.

@poeschlr
Copy link
Collaborator

poeschlr commented Nov 30, 2018

I really do not like rushing things just to get a release out the door.
If the symbol side of this has been merged after 5.0.1 then we have the option to re tag 5.0.1 as 5.0.2 (meaning no lib update for this minor release). This would give us time to do a proper review of everything and release it with 5.1.0.

If however 5.0.1 already includes the new symbols then we are forced to include this PR as well. (It would then fix an existing bug within 5.0.1)

Edit: I can look into this after work. (A few hours from now.)

@poeschlr
Copy link
Collaborator

I can not really see a symbol fitting the following footprints:

  • Jack_3.5mm_Ledino_KB3SPRS_Horizontal (nearest fitting are AudioJack3_Ground_SwitchTR or AudioJack3_Switch but both have one more pin than the footprint)
  • Jack_3.5mm_PJ31060-I_Horizontal (There is no symbol for switched jacks that have a pin R1)
  • Jack_3.5mm_QingPu_WQP-PJ398SM_Vertical_CircularHoles (Nearest fitting would be AudioJack2_Switch but that one has a second switch an therefore the pin SN is too much)

As i predicted there are simply too many different options to fill everything with generic symbols. So in this case i would leave the footprints that do not have a valid symbol alone. (And later create new fully specified symbols that can then use the numbering scheme as it is in the datasheet of these parts.)

@evanshultz
Copy link
Collaborator Author

I understand not wanting to rush things. As one can see above, though, a user reported this issue with footprints and symbols not matching and I posted this fix above 6 weeks ago. And then I repeatedly asked for someone to review it so that the library can get un-broken.

Yes, 5.0.1 already has symbols with letters. This is why users reported the broken footprints.

You are correct that Jack_3.5mm_PJ31060-I_Horizontal does not have a matching schematic symbol. (Did it ever have one??) I can create a symbol. But the other two you mention above DO have a symbol already in the library, these footprints match up in CvPcb with this PR merged, and they are shown here:
image

So... yeah. Please somebody double check this and merge it.

@robkam
Copy link
Contributor

robkam commented Dec 1, 2018

BTW
#1069 also makes a few changes/corrections to Jack_3.5mm_QingPu_WQP-PJ398SM_Vertical_CircularHoles (been waiting for this footprint to get reviewed/merged since August)
#1073 adds a footprint for the same jack with oval holes and a couple more for the stereo version

@poeschlr
Copy link
Collaborator

poeschlr commented Dec 2, 2018

@evanshultz why are your "Ground Switch" versions of the symbols missing the GND pin? If i look at them in kicad (current master) they have this additional pin. -> meaning they do not fit the footprints as these footprints don't have a ground pad.

Edit: To answer my own question: This is because all these symbols have changed since 5.0.1. (both got a ground pin added. The original without ground pin no longer exists)

@evanshultz
Copy link
Collaborator Author

So is there something overlooked here or can this be merged? I'll submit a PR for the missing symbol (T and R1 normalled but not R2 or S) in the next 24 hours (probably in around 12 hours).

@evanshultz
Copy link
Collaborator Author

evanshultz commented Dec 3, 2018

There should be non-ground versions of all of them. Check out the image at KiCad/kicad-symbols#1076. AudioJack3 has no "G" pin while the other AudioJack3 variants do.

Edit: Image reproduced here:
image

@GrantM11235
Copy link
Contributor

I am seeing some conflicting information about the PJ320D pinout. I found datasheets from two manufacturers (XKB and Korean Hroparts) that say that the lone pin on one side is the sleeve pin. I have confirmed that this is the case with a connector that I have (unknown manufacturer).

Is it possible that the Wenzhou QingPu datasheet is wrong?

@poeschlr
Copy link
Collaborator

poeschlr commented Dec 3, 2018

I checked the current master again. The symbols look different than in your screenshot. One example is the AudioJack2_Switch symbol: Edit: I confused two symbols. That one looks identical to the one in your screenshot. But the AudioJack2_SwitchT symbol is missing in the library (At least without ground pin. And this is the one we would need. It is even missing from your screenshot.)

My guess is that either something went wrong during the merging process or you forgot to push something.

@evanshultz
Copy link
Collaborator Author

@GrantM11235
Thanks for the info.

I really can't say for sure because there was no documentation associated with this footprint. Other connectors in the library matched with QingPu so my best guess was that someone submitted multiple footprints from that same company, but it was only a guess.

Also, the QingPu does not have a suffix after PJ320D (notice the -A and -B suffixes at https://datasheet.lcsc.com/szlcsc/Korean-Hroparts-Elec-PJ-320D-4A_C95562.pdf and -X at https://datasheet.lcsc.com/szlcsc/XKB-PJ-320D-X_C319111.pdf), but again this is not conclusive.

Pins named 1-4 don't tell us anything about the desired pinout either, so that's no help.

The earliest history I can see on GitHub for this footprint is at https://github.com/KiCad/Connectors.pretty/commits/master/PJ320D_3.5mm_Jack.kicad_mod, and it doesn't include the original import nor any vendor info. Maybe someone else can find more history?

It would be best to include the vendor name and the full MPN, but we do not want to change footprint names right now during the 5.0.x cycle. I think doing that when available, and adding these other variants too, would give users the coverage they desire since it appears the same physical connector can have different internal connections (I didn't check all dimensions carefully but they look the same).

(As an aside, this is why abstracted symbols with named pins works well, because different pinout variations can all use the same symbol. At least that's how I see it.)

@evanshultz
Copy link
Collaborator Author

@poeschlr
Alright. I'm working on that symbol now, but I don't see that stops this from getting merged if there are no other issues. Do you find anything else that needs to be changed?

@poeschlr
Copy link
Collaborator

poeschlr commented Dec 3, 2018

We are in this mess exactly because somebody did not require simultaneous changes to both symbols and footprints before merging. I am not going to make the same mistake with the same lib again. This one can be merged as soon as we can check it against symbols and then both should be merged at the same time.
Another option would be to split this PR into the parts that are already ok and the ones that are not. The QuinPu one might also be better left alone for now as we will need additional time to check it properly.

Edit: I have an additional complication today. My internet does not really seem to work. (very slow. and a lot of interruptions.)

@evanshultz
Copy link
Collaborator Author

Here is the original footprint:
image

I would naively assume the pin numbers mean the following:
1 = T
2 = R1
3 = R2
4 = S

This actually matches the XKB and Korean Hroparts if the pin numbering above is correct, but again I don't see any way to know for sure. I'm happy to update this PR to include either of those and the QingPu, both footprints including full manufacturer name and MPN, so we are covered either way. However, does that mean the existing footprint is removed? Or it's kept with the pinout as described above and then renamed later as either XKB or Hroparts?

Required symbol PR is posted at KiCad/kicad-symbols#1207.

Now, here are the symbols in master and the aforementioned PR required for these footprints:
image

And here they are in CvPcb (one is showing both potential options from this PR, but otherwise they are already matched):
image

So if the symbol PR is merged as well, I think there is nothing missing and all footprints are usable. Is there something still broken?

@GrantM11235
Copy link
Contributor

QingPu has another connector WQP-PJ320G which looks like it uses the exact same footprint, but the pinout matches the XKB and Korean Hroparts connectors.

I emailed QingPu to ask for clarification.

@poeschlr
Copy link
Collaborator

poeschlr commented Dec 3, 2018

You might really not have the current version of the master evan. The AudioJack2_Ground_Switch has a gnd pin in there. One without ground is even missing after merging your symbol pull request.

To be more precise: The symbols for J2 and J3 look different than in your screenshot

Here is what i have in the lib instead:
screenshot from 2018-12-03 20-21-48


Edit: another option would be that your project already has a rescue lib with the old symbols in it. This might be why you still have them. Maybe try with a completely new project.

poeschlr added a commit to poeschlr/kicad-symbols that referenced this pull request Dec 3, 2018
evanshultz pushed a commit to KiCad/kicad-symbols that referenced this pull request Dec 3, 2018
@evanshultz
Copy link
Collaborator Author

Indeed I did have a branch selected which wasn't up-to-date. My fault.

I merged your PR. Assuming that wraps up the symbols, how do you want to handle footprints? Merge this as-is so that we have matching footprints and we can tag 5.0.2 and then name the footprints properly in the near future? Change the pinout of the unattributed "PJ320D" footprint here? Something else?

@poeschlr
Copy link
Collaborator

poeschlr commented Dec 3, 2018

I think the major points are now fixed.
Thanks for your effort.

Everything else can be fixed at a later time.

@poeschlr poeschlr merged commit 471e1c0 into KiCad:master Dec 3, 2018
@GrantM11235 GrantM11235 mentioned this pull request Dec 4, 2018
4 tasks
@Misca1234
Copy link
Collaborator

Sorry for not following up, I was kinda 100% on the valve issues

@McColson
Copy link
Contributor

Hello,
I think the NMJ6HCD2 isn't a 3.5mm jack but a 6.35mm.

@evanshultz evanshultz deleted the audio-jack-fix branch February 27, 2019 16:35
@evanshultz
Copy link
Collaborator Author

@McColson
Yep, and my PR at #1145 will fix that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix footprint existing in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

audio jack symbols use letters; footprints use digits
7 participants