-
Notifications
You must be signed in to change notification settings - Fork 742
Analog ADC: create ADS8684 and ADS8688 #1197
Conversation
|
@evanshultz thanks for the review, I agree with most of your points (I have lean a lot since this PR by reviewing and reading the KLC !) I will do the modifications asap. Joel |
Comment by comment:
Agree. Was copy/paste from datasheet.
OK, no modification.
Done.
Done.
Done.
Agree. New arrangement of the pins looking at datasheet page 35.
Agree. I keep an extra space because that's not the same block.
Still the same behavior and still my point of view, you can't do anything you want with this pin so it can't be Passive.
Agree, done.
Agree for pins 8, 28, 29, 31, 32.
Agree, splitting in two devices. Cheers, |
Please change "Ranges" in the description to "Range". Also please update the screenshot in the first post or add fresh ones with all the visual changes done above. I don't understand your objection to Passive type, which is what we've done elsewhere in the library. Especially for pin 7 since it only goes to a cap and should not really factor into any ERC checking. The ERC engine is extremely basic and making these pins Passive will avoid false positives (such as REFIO being an input coming from a linear regulator output which is of type Power Output). Input, Output, and Bidirectional types can catch a few issues with logic pins but it doesn't help with analog pins like this with the current capability of the KiCad ERC engine. I'm making suggestions so this PR conforms to the method that's been used on other recent symbols and thus will be consistent and require less discussion. #381 and KiCad/kicad-website#342 are a couple of discussions about pin types which would allow conversation about pin types to stay in one place and also avoid bogging down a PR with a bigger topic. |
@evanshultz I understand the reason and I agree it will be better for ERC checking and consistency with other devices if already used like that. |
@evanshultz additional comments ? Cheers |
@myfreescalewebpage |
@evanshultz no worries :) I have just checked again, on my side this is ready to merge. |
Sounds good. Thanks and sorry for the delay! |
Creation of TI ADC ADS8688 and ADS8644.
Datasheet : http://www.ti.com/lit/ds/symlink/ads8688.pdf
Screenshot:
No error with checklib.py, using existing TSSOP38 footprint.