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

Conversation

zoonman
Copy link
Contributor

@zoonman zoonman commented Jul 27, 2019

Adding AD831 mixer from Analog Devices

Official page https://www.analog.com/en/products/ad831.html
Datasheet https://www.analog.com/media/en/technical-documentation/data-sheets/AD831.pdf

Symbol Screenshot:

Screen Shot 2019-07-26 at 23 37 07

The datasheet one:

Screen Shot 2019-07-26 at 23 43 35

I reused existing footprint because there is generic one that matches described in the datasheet.

@CLAassistant
Copy link

CLAassistant commented Jul 27, 2019

CLA assistant check
All committers have signed the CLA.

@myfreescalewebpage myfreescalewebpage added Addition Adds new symbols to library Pending reviewer A pull request waiting for a reviewer labels Jul 29, 2019
@myfreescalewebpage
Copy link
Collaborator

@zoonman thanks for this contribution.
In KiCad, pins should be grouped by function, i.e. GND at the bottom, power at the top, etc, not by the physical position of thep ins.
Please review.
Thanks,
Joel

@evanshultz
Copy link
Collaborator

In addition, the description is dramatically verbose.

@zoonman
Copy link
Contributor Author

zoonman commented Jul 29, 2019

Thank you, guys. I was reading the contribution guidelines and found it pretty confusing. Please, don't mind, I am not electronics engineer. I will try to improve this PR.

@zoonman
Copy link
Contributor Author

zoonman commented Jul 30, 2019

@myfreescalewebpage @evanshultz I made updates to description and pins layout.

Here is updated screenshot:

Screen Shot 2019-07-29 at 22 11 21

@myfreescalewebpage myfreescalewebpage self-assigned this Jul 30, 2019
@myfreescalewebpage myfreescalewebpage removed the Pending reviewer A pull request waiting for a reviewer label Jul 30, 2019
@myfreescalewebpage
Copy link
Collaborator

myfreescalewebpage commented Jul 30, 2019

Hi @zoonman , thanks for contributing,

A few comments I have during my review:

  • AD831 does not exist and APZ is the ROHS version, only "AD831AP"' is expcted, please remove the others
  • The description should end with ", PLCC-20", no dot, I suggest Doubly Balanced monolithic Mixer, 500 MHz BW, +24 dBm, IP3, LNA and LO driver, PLCC-20
  • No comma required in the keywords
  • Symbol is not centered (see travis log)
  • Pin offset should be 20mil (see travis log)
  • Pin length should be 100mil
  • Name of pins 1, 9 and 12 is "VP", name of pin 2 is IFN, etc etc. You should use the exact names from the datasheet to avoid confusion
  • Pin 14 (BIAS) should not be at the top of the symbol. I suggest in the bottom right corner just below IFN
  • IFN and IFP should be Open Collector
  • VN pins should be stacked
  • GND pins should be stacked
  • Probably grouping AP/AN with IFP/IFN is betetr to simplify wiring in the schematics

Will do a complete review after these points are corrected.

Cheers,
Joel

@zoonman
Copy link
Contributor Author

zoonman commented Jul 31, 2019

@myfreescalewebpage I think I got everything cleaned up. No complaints from Travis.
The only one thing bothers me is the stacked pins. I hope I did this correctly.

Screen Shot 2019-07-30 at 22 18 28

@myfreescalewebpage
Copy link
Collaborator

myfreescalewebpage commented Jul 31, 2019

@zoonman thanks for all the fixes!

  • One comment remain: we do not add ROHS versions in KiCad, please remove APZ version
  • VP should not be stacked, only GND should be stacked
  • VN should not be stacked (sorry it's a mistake in the previous comment)
  • One point forgotten previously: footprint filter should not contain the number of pins, it should be PLCC* only for this symbol
  • Looking at the datasheet I propose you the following modification of the design to simplify the wiring, let me known:

Capture

Joel

@zoonman
Copy link
Contributor Author

zoonman commented Aug 2, 2019

@myfreescalewebpage, I followed your instructions and got it updated:

Screen Shot 2019-08-01 at 22 10 09

I hope this is final version.

@myfreescalewebpage
Copy link
Collaborator

Perfect, thanks. No more comment to do, merging.
Joel

@myfreescalewebpage myfreescalewebpage merged commit c115722 into KiCad:master Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Addition Adds new symbols to library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants