Skip to content

Conversation

doom-fr
Copy link

@doom-fr doom-fr commented Oct 7, 2019

So I have added ATmega324PB in TQFP package.
I have not added the VQFN version as I haven't found the footprint.
I made both functional symbol (default one) and physical symbol (using physical pin placement).
I have also created a new category called "AVR 8bits Microcontrollers" and put this one and also ATtiny85 inside.

Finally, I have fixed a little mistake in the ATtiny85 description (8 kbytes instead of 8 bytes of flash).

Please tell me if this is ok for you and I will add ATmegaxx8PB (48/88/168/328).

Thanks.

@doom-fr
Copy link
Author

doom-fr commented Oct 11, 2019

Added also ATmega328PB and its memory variations (48/88/168)

Copy link
Contributor

@ubruhin ubruhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

My comments:

  • Maybe better rename the symbols/components to e.g. "ATmega48/88/168/328PB", or at least mention it in the description to which models the symbols/components apply?
  • Remove the underscore from symbol names (i.e. "ATmegaxx8PB_functional" -> "ATmegaxx8PB Functional").
  • Remove the "Reset = PC6" label from symbols. Rename the component signal to "PC6/nRESET" instead.
  • Move the "{{NAME}}" and "{{VALUE}}" labels out of the symbol body, e.g. name to the top left and value to the bottom left (or right, if it looks better).
  • The component "ATmegaxx4PB" has no default value set.
  • In the "ATmegaxx8PB_functional" symbol, pins are the other way around as in "ATmegaxx4PB_functional" -> change pin order (Px0 at top, Px7 at bottom).
  • GND1 and GND2 should be combined into one component signal "GND", and only one GND pin added to the functional symbols.
  • Actually I think the "physical" symbols could be made generic and added to the LibrePCB_ICs library (like the "Generic IC" symbols there). Then the symbols could be used by other 32/44-pin components too. And the term "functional" resp. "physical" is no longer needed in the symbol names. Or what do you think @dbrgn?

@doom-fr
Copy link
Author

doom-fr commented Oct 27, 2019

  • GND1 and GND2 should be combined into one component signal "GND", and only one GND pin added to the functional symbols.

I was waiting for that observation !
Here is an extract of the ATmega324PB datasheet.
image
As you can see, this gnd is near analog pins and on the analog gnd plane.
Depending how you want to manage planes, it can be useful to have gnd pins separate.

  • Actually I think the "physical" symbols could be made generic and added to the LibrePCB_ICs library (like the "Generic IC" symbols there). Then the symbols could be used by other 32/44-pin components too. And the term "functional" resp. "physical" is no longer needed in the symbol names.

The physical symbol can't be generic as power pins and port are not always in the same place in different micro-controllers

Here is a Microchip PIC TQFP44
image

And here ATmega324PB
image

@ubruhin ubruhin added addition New library element. ready for review Waiting for review by maintainers. labels Nov 3, 2019
@rnestler
Copy link

rnestler commented Nov 3, 2019

@doom-fr @ubruhin What is the status on that one? Was the feedback applied?

@doom-fr Do you have the available time to do the fixes @ubruhin suggested? Otherwise could somebody take over the PR to get it merged?

@rnestler
Copy link

rnestler commented Nov 3, 2019

  • GND1 and GND2 should be combined into one component signal "GND", and only one GND pin added to the functional symbols.

I was waiting for that observation !
Here is an extract of the ATmega324PB datasheet.
As you can see, this gnd is near analog pins and on the analog gnd plane.
Depending how you want to manage planes, it can be useful to have gnd pins separate.

So how should we handle this case? It is quite common to have a different Analog GND (AGND) and the GND for the rest.
Should we add an AGND pin in the symbol for that?

@doom-fr
Copy link
Author

doom-fr commented Nov 3, 2019

@rnestler , I do not know the position of @ubruhin on the GND point.
So for now, I am waiting ...

@ubruhin
Copy link
Contributor

ubruhin commented Nov 25, 2019

  • GND1 and GND2 should be combined into one component signal "GND", and only one GND pin added to the functional symbols.

I was waiting for that observation !
Here is an extract of the ATmega324PB datasheet.
As you can see, this gnd is near analog pins and on the analog gnd plane.
Depending how you want to manage planes, it can be useful to have gnd pins separate.

Hmm that's a tricky thing. Since the two GND pins are (probably) electrically identical (otherwise Atmel would name them differently), I'm not sure if it makes sense to handle them as separate signals in the component. With the names GND1 and GND2 you still don't know (in schematics) which one is which.

If you want to draw a separate GND plane around all the analog pins, shouldn't this be a topic related only to the board editor?

  • Actually I think the "physical" symbols could be made generic and added to the LibrePCB_ICs library (like the "Generic IC" symbols there). Then the symbols could be used by other 32/44-pin components too. And the term "functional" resp. "physical" is no longer needed in the symbol names.

The physical symbol can't be generic as power pins and port are not always in the same place in different micro-controllers

That's why our generic IC symbol have their pins names just "1", "2", "3" etc, so they can be used for whatever signal you like. For example pin "5" is used as "GND" in one component, and as "VCC" in another component, that doesn't matter.

@rnestler
Copy link

rnestler commented Dec 9, 2019

If you want to draw a separate GND plane around all the analog pins, shouldn't this be a topic related only to the board editor?

Usually one wants at least different net names to be able to easily highlight the different GNDs. Also sometimes GND and AGND are connected via some kind of decoupling (L, C or whatever), or just a 0-Ohm resistor / bridge to have a "star" distribution of the GND signal. See https://www.analog.com/en/analog-dialogue/raqs/raq-issue-159.html for example.

@doom-fr
Copy link
Author

doom-fr commented Dec 10, 2019

@rnestler I am not sure the link you provide can represent the situation.
Current output of micro-controller is not so high (100mA max), so I don't think it can be compared to power circuit.

@rnestler
Copy link

Current output of micro-controller is not so high (100mA max), so I don't think it can be compared to power circuit.

It is relevant if you want to separate the GND for the analog parts from the GND of other parts of the circuit which may draw more power. But you are right that it doesn't represent the issue perfectly. Maybe https://www.analog.com/en/analog-dialogue/articles/staying-well-grounded.html# is a better resource.

@doom-fr
Copy link
Author

doom-fr commented Dec 29, 2019

I have made all changes except the GND prob.

First we are OK that if I put only one GND pin in functional symbol, I should make the same to Physical symbols in order to use them in the same component ?

And when I rename GND1 to GND and suppress GND2, I encounter some problems :
image

I get an orphelin pin-signal-mapping
image

How can I delete this without breaking everything ?

And also in pad-signal-mapping of devices, I still get old name :
image

@ubruhin
Copy link
Contributor

ubruhin commented Feb 8, 2020

I have made all changes except the GND prob.

There is still an unresolved issue: #8 (comment) 😉 And the symbols still have an underscore in their name (see #8 (review)).

First we are OK that if I put only one GND pin in functional symbol, I should make the same to Physical symbols in order to use them in the same component ?

Hmm I'm not sure if I understand correctly. The physical symbol should still have all pins since it represents the package. And if you make the symbol generic (pins named "1", "2", ...) as I suggested in #8 (review), there is no conflict between the two GND pins.

FYI, to simplify things and thus getting this merged sooner, you could even remove the physical symbol variant for now and add it in a separate PR later ;)

@ubruhin ubruhin added needs corrections Pull request needs corrections before next review. and removed ready for review Waiting for review by maintainers. labels Mar 30, 2020
@doom-fr
Copy link
Author

doom-fr commented May 17, 2020

Should I update version from 0.2.1 to 0.2.2 ?

And for the name, I think that AVR are more Atmel than Microchip.
So naming category "Microchip AVR Microcontrollers" is a bit strange.
I think that "AVR 8 bits Microcontrollers" is a better category name.

@doom-fr
Copy link
Author

doom-fr commented May 17, 2020

Seems there also a problem because I have used the nightly build to make changes.

How should I solve that ?

@doom-fr
Copy link
Author

doom-fr commented Jun 2, 2020

@doom-fr doom-fr requested a review from ubruhin June 2, 2020 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition New library element. needs corrections Pull request needs corrections before next review.
Development

Successfully merging this pull request may close these issues.

3 participants