[KLC] Clarify NC pin handling #377

poeschlr · 2019-01-25T22:48:18Z

Clarify rules regarding NC pins
- Clarify when they may be omitted
- Clarify that they must be hidden when included.

Inspiration: KiCad/kicad-library-utils#285

* Clarify rules regarding NC pins * Clarify when they may be omitted * Clarify that they must be hidden when included.

content/klc/S4.5.adoc

diegoherranz · 2019-02-12T23:18:12Z

content/klc/S4.6.adoc


 **Exceptions**

-. Unused NC pins can be set to invisible. In this case the electrical type must be `Not Connected`. The end of the pin should lie on the symbols outline box to prevent unwanted connections to the invisible pin (see screenshot below).
+. Power input pins must not be invisible unless used in power symbols. (Hidden power input pins are global labels.) Refer to the link:/libraries/klc/S4.3[requirements for pin stacking]


Punctuation is a bit odd on this sentence.
Otherwise, I think this PR looks OK.

The reason for the strange punctuation might be because i tried to use this strange RFC defined standardized language. #289

Do you have a better suggestion?

Something like this maybe?

Power input pins must not be invisible unless used in power symbols (hidden power input pins are global labels). Refer to the link:/libraries/klc/S4.3[requirements for pin stacking].

Am i blind or is yours word for word the same?

The wording is the same. It's just the punctuation and capitalisation.

fixed (forgot to mark it as such)

poeschlr · 2019-03-22T05:36:36Z

Fixed

marekr · 2019-03-22T14:01:57Z

@poeschlr ok to merge?

poeschlr · 2019-03-22T14:36:34Z

In my opinion yes. But i might be biased here ;)

poeschlr · 2019-05-04T06:42:50Z

@marekr i think this is ready to go. Nobody complained about it for a month or so and this is more an update of the KLC to document how everything is done in the library already than a new rule.

diegoherranz · 2019-05-04T07:32:48Z

Thanks for having fixed that picky comment of mine.
I think this PR makes the KLC clear so happy to merge.
I've just noticed one thing I hadn't noticed before.

Symbol pins must not be hidden unless they are not connected, part of a stack or power symbols

Maybe "unless they are not connected or part of a stack or power symbols"

Sorry I didn't come back on this earlier.

Thanks!

diegoherranz · 2019-05-04T07:57:20Z

Also, it's been so long that we may want to update the date on the revision history.
Thanks!

poeschlr · 2019-05-04T09:12:52Z

Since when would one use a construction like A or B or C instead of A, B or C?

… time

diegoherranz · 2019-05-04T09:31:17Z

Since when would one use a construction like A or B or C instead of A, B or C?

Please ignore. In my mind I had read it as:

"Symbol pins must not be hidden unless they are not connected, part of a stack of power symbols"

And by stack of power symbols I understood stack of power pins in a symbol, so I misread that too.
So I was missing an "or" in place ofthe comma.
Just ignore, sorry for the noise.

After the date update, I think this can be merged.

Thanks!

nickoe · 2019-05-04T21:52:31Z

@marekr it looks like the link checker keeps failing for the digikey links, but I can access those fine in my own browser. I wonder if there is anything to do about this. It is not 404, but 403.

poeschlr · 2019-05-05T07:44:20Z

I did not touch these links. So i suggest to open a separate issue about it as nobody will find a discussion about them in here and the result of such an discussion should not impact this change anyways.

nickoe · 2019-05-06T22:33:33Z

@poeschlr yes I know. It seems like you accidently pushed 29c33d5 to your klc_nc_pins branch. I guess you don't want that merge commit here.

poeschlr · 2019-05-07T06:37:58Z

I needed a merge commit as i had a conflict on the klc version page.

diegoherranz · 2019-05-07T21:16:21Z

klc.adoc needs to be updated too. Otherwise, I think it's all ready to go.
Thanks!

diegoherranz · 2019-05-08T05:58:40Z

Thanks, @poeschlr. Looks good to go.

poeschlr · 2019-06-02T06:58:53Z

@nickoe, @marekr are there any open problems with this? Should the library team start to fully handle KLC parts of the website including merging?

evanshultz · 2019-07-01T15:12:30Z

content/klc/S4.5.adoc

-. If the pin-count in the schematic does not match the pad-count on the footprint, the footprint filter must include the pad-count of the footprint (see also link:/libraries/klc/S5.2[requirements for footprint filters])
-* `SOT?23?5`
-* `SOIC?8`
+. The footprint filter must include the pad-count of the footprint if the pin-count in the schematic does not match the pad-count on the footprint (see also link:/libraries/klc/S5.2[requirements for footprint filters])


I believe it sounds better to use "match the pad-count of the footprint".

evanshultz · 2019-07-01T15:16:16Z

@poeschlr
I have seen, and then subsequently requested, to have test pins visible. Some ICs have test pins which don't have value for the end user in a normal application but are be used during IC testing or programming or other validation. Sometimes these pins are marked as "NC" so it seems to fit here. Are these pins to be visible? If so, can it be noted with these changes?

I know you're generally out for a couple weeks but when you have time can you please update this to 3.0.21, now the next available revision?

@nickoe @marekr
Any input on Rene's question above?

evanshultz · 2019-07-15T20:15:12Z

content/libraries/klc_history.adoc

+== 3.0.19 - 2019-05-04
+* Clarify rules regarding NC pins
+   * Clarify when they may be omitted
+   * Clarify that they must be hidden when


Not correct phrase. Is to missing something at the end or just need wordsmithing?

yes something went wrong here. fixed

evanshultz · 2019-07-15T20:16:31Z

@poeschlr
See my two comments above, fix the branch conflict, and update the KLC revision/date. Then I'd be happy to see this merged.

@nickoe @marekr
Please see Rene's question above and respond. Thank you.

evanshultz · 2019-07-15T20:17:55Z

@poeschlr
Oh, and while it's related but not required, http://kicad-pcb.org/libraries/klc/S4.6/ doesn't mention that NC pins can be inside the symbol body. Yet we do that for non-rectangular symbols (opamps and comparators) or if the symbol outline is too small to fit all NC pins.

If you wanted to include that, since it's also about NC pins, nice. If not, that's OK too.

poeschlr · 2019-08-19T06:25:16Z

@evanshultz i think the non response to our question can be seen as the go ahead for us to be responsible for merging website content for the KLC.

Regarding test pins: Depends on how they are marked in the datasheet. If the datasheet mentions them as do not connect (without further information) then it is to be handled as a NC pin as described here.
If the datasheet calls it Test (or similar) but states "do not connect" then it is to be handled as a normal NC pin (we could allow the name test in this case.)

Only if the function of the test pin is described in the datasheet then we should handle it as a normal pin.

Programming pins are normally not described as "do not connect" in datasheets and are therefore not really handled by this ruleset. (otherwise same as with test pin: normal pin if datasheet describes it otherwise NC)

nickoe · 2019-08-19T07:37:34Z

@nickoe, @marekr are there any open problems with this? Should the library team start to fully handle KLC parts of the website including merging?

I don't really follow the KLC discussions so I let you handle this. I guess the same goes for @marekr. It would be nice if you just mention me in a comment on the pull request with a clear message that it should be merged. For example: "Please merge this now."

And please use the github review feature then it is easier to see if there are pending inline comments. Right now @evanshultz has some outstanding comments.

poeschlr · 2019-08-19T07:45:02Z

In my mind the person reviewing should be the person merging. (It makes zero sense to have a "ready please merge" comment at the end unless the person that would respond to that also makes a review.)

nickoe · 2019-08-19T07:48:29Z

In my mind the person reviewing should be the person merging. (It makes zero sense to have a "ready please merge" comment at the end unless the person that would respond to that also makes a review.)

I do review it, but not in detail with regard to the KLC, but only the technicalites about the website. I don't think it is a real impediment, we do merge KLC patches very fast when it is clear that it should be merged.

poeschlr · 2019-09-01T07:28:01Z

I fixed all suggestions made. @evanshultz have i answered your inquiry regarding test pins to your satisfaction? I do not believe i would need to add anything to the KLC as the behaviour i describe above is already covered by it.

poeschlr · 2019-09-08T07:55:25Z

@evanshultz could you take a quick look? I really would like to work on other sections of the KLC but this should really be merged first.

evanshultz · 2019-09-09T22:23:02Z

content/klc/S4.5.adoc

-. If the pin-count in the schematic does not match the pad-count on the footprint, the footprint filter must include the pad-count of the footprint (see also link:/libraries/klc/S5.2[requirements for footprint filters])
-* `SOT?23?5`
-* `SOIC?8`
+. The footprint filter must include the pad-count of the footprint if the pin-count in the schematic does not match the pad-count of the footprint (see also link:/libraries/klc/S5.2[requirements for footprint filters])


Sounds better to me if in the schematic changes to of the symbol. Also, why hyphenate pincount?

found an even better wording

evanshultz · 2019-09-09T22:26:17Z

content/klc/S4.6.adoc

@@ -1,14 +1,15 @@
 +++
-brief = "Hidden pins"
+brief = "Symbol pins must not be hidden unless they are of type 'not connected', part of a stack or the label pin of a power symbol"


Below Not Connected is used with regards to pin type. Should that be consistent? It feels like it to me.

I could have been more clear that there were two parts I was talking about being missing here but used below. Both capitalization and the code format (enclosed in grace accent marks) is used below. Capitalization is done but code style is still missing.

I am not sure if the brief part can use utf8 chars. My guess is that the normal quote mark is used on purpose here. Maybe @nickoe or @marekr know more about that. (I am not even sure where the brief part is used to be honest.)

I see. It appears to be the section header.

What about retaining the section header Hidden pins and then moving the sentence down into this rule? As it is, that appears to be an awfully long name to section S4.6.

Ah you are right this is the header. (It is so long ago that i started this that i already forgot half of what i intended.)

I assume i had a good reason why i changed it. Is it possible that this wording matches travis output? I will check that tomorrow as it is way too late for me for today.

Perhaps. I wonder if such a verbose section header is a good idea, though? We already have the rule number to cross-reference with check script output.

i reverted the title as i am currently not finding something better that fulfils your requirement for a short one.

evanshultz · 2019-09-09T22:30:18Z

@poeschlr
Sure!

I made two more comments inline after re-reading the change log.

Regarding test pins, see KiCad/kicad-symbols#1719 (datasheet at http://www.ti.com/lit/ds/symlink/tusb4020bi.pdf). This is an example what I had in mind. The pins aren't named NC, but the datasheet says do not connect them. They clearly have some internal connection and may not be used in normal applications but are used for testing, debug, service, etc. Is this general type of pin to be shown or not?

poeschlr · 2019-09-09T22:45:32Z

Do you mean the reserved pins? These state "do not connect on pcb" so they fall under

Pins that are not intended to be connected must be set to invisible. In this case the electrical type must be Not Connected. The end of the pin should lie within or on the symbols outline to prevent unwanted connections to the invisible pin (see screenshot below).

And yes the manufacturer of the part will most likely use these pins for final testing. But our symbols are not meant for this usecase but for the more common usecase of the enduser who will want to follow the advice of "do not connect".

The test pin however says "it is recommended to be pulled to ground" so it should be included like a normal pin with electrical type "input".

evanshultz · 2019-09-09T22:47:49Z

Alright. Just the one note above using code style and then I'm good to merge this.

evanshultz · 2019-09-09T22:50:48Z

And it doesn't really hurt anything, but the KLC changelog is broken down awfully fine. Just having the two major bullets would be satisfactory for me.

poeschlr · 2019-09-09T22:54:24Z

history simplified

evanshultz · 2019-09-10T17:56:13Z

OK. Works for me.

@nickoe @marekr
This is ready to merge!

Edit: Note that @diegoherranz made requests above but already indicated this is ready to merge.

evanshultz · 2019-09-12T15:56:18Z

@nickoe @marekr
Can you please merge?

Would it make sense to have the head librarian (@poeschlr ) as part of the website team so they can merge KLC PRs? That would only be one person to manage with hopefully good stability. If the head librarian authors the PR someone else can sign off, but otherwise they'd be trusted to handle KLC tasks.

evanshultz · 2019-09-14T21:11:31Z

@nickoe @marekr @stambaughw
Merge, please.

This is a perfect example why having the head librarian with merge rights to this repo, or another solution that allows the library team to manage the KLC page, would be a good idea. 4 days isn't an eternity, and maybe this is extremely unusual, but this has been ready for merge and yet waiting in purgatory...

nickoe · 2019-09-18T21:37:58Z

@evanshultz @marekr I was waiting for go on #355 (comment)

[KLC] Clarify NC pin handling

f606040

* Clarify rules regarding NC pins * Clarify when they may be omitted * Clarify that they must be hidden when included.

poeschlr added enhancement libraries labels Jan 25, 2019

poeschlr mentioned this pull request Feb 9, 2019

add Pulse_H1100NL ethernet transformer KiCad/kicad-symbols#1511

Open

6 tasks

diegoherranz reviewed Feb 10, 2019

View reviewed changes

content/klc/S4.5.adoc Outdated Show resolved Hide resolved

fix link to hidden pins section

46b7103

diegoherranz approved these changes Feb 12, 2019

View reviewed changes

Changed punctuation as suggested.

6b3a61e

Update date in history document as this one got not merged for a long…

eec8a8b

… time

merge master into klc_nc_pin

29c33d5

evanshultz reviewed Jul 1, 2019

View reviewed changes

evanshultz reviewed Jul 15, 2019

View reviewed changes

poeschlr mentioned this pull request Aug 19, 2019

Add the TI TS3A27518E 6-channel 1:2 analog mux KiCad/kicad-symbols#2092

Merged

6 tasks

Merge remote-tracking branch 'upstream/master' into klc_nc_pins

78e237e

poeschlr force-pushed the klc_nc_pins branch from 392f56a to 78e237e Compare September 1, 2019 07:14

Fixes as suggested plus further clarifications

3d8e8da

evanshultz reviewed Sep 9, 2019

View reviewed changes

removed unnecessary hyphenation and better wording

cb38038

simplify history

db1f4a6

revert title

d66fb52

marekr merged commit 411a156 into KiCad:master Sep 14, 2019

[KLC] Clarify NC pin handling #377

[KLC] Clarify NC pin handling #377

Conversation

poeschlr commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poeschlr commented Mar 22, 2019

marekr commented Mar 22, 2019

poeschlr commented Mar 22, 2019

poeschlr commented May 4, 2019

diegoherranz commented May 4, 2019

diegoherranz commented May 4, 2019

poeschlr commented May 4, 2019

diegoherranz commented May 4, 2019

nickoe commented May 4, 2019

poeschlr commented May 5, 2019

nickoe commented May 6, 2019

poeschlr commented May 7, 2019

diegoherranz commented May 7, 2019

diegoherranz commented May 8, 2019

poeschlr commented Jun 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanshultz commented Jul 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanshultz commented Jul 15, 2019

evanshultz commented Jul 15, 2019

poeschlr commented Aug 19, 2019

nickoe commented Aug 19, 2019 • edited Loading

poeschlr commented Aug 19, 2019

nickoe commented Aug 19, 2019

poeschlr commented Sep 1, 2019 • edited Loading

poeschlr commented Sep 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poeschlr Sep 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanshultz commented Sep 9, 2019

poeschlr commented Sep 9, 2019

evanshultz commented Sep 9, 2019

evanshultz commented Sep 9, 2019

poeschlr commented Sep 9, 2019

evanshultz commented Sep 10, 2019 • edited Loading

evanshultz commented Sep 12, 2019

evanshultz commented Sep 14, 2019

nickoe commented Sep 18, 2019

nickoe commented Aug 19, 2019 •

edited

Loading

poeschlr commented Sep 1, 2019 •

edited

Loading

poeschlr Sep 9, 2019 •

edited

Loading

evanshultz commented Sep 10, 2019 •

edited

Loading