Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Icepak ECAD import example with Ansys board #221

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ansys-satyajeet
Copy link
Collaborator

Pull Request Template

Description

A new example for ECAD import into Icepak with Ansys board. It also covers stackup modification with material property changes.

Checklist

Please complete the following checklist before submitting your pull request:

  • [Y] I have followed the example template and guide lines to add/update an example.
  • [Y] I have tested the example locally and verified that it is working with the latest version of AEDT.
  • [Y] I have verified that these changes to the best of my knowledge do not introduce any security vulnerabilities.

@Samuelopez-ansys
Copy link
Member

Copy link
Member

@Samuelopez-ansys Samuelopez-ansys left a comment

Choose a reason for hiding this comment

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

@lorenzovecchietti lorenzovecchietti marked this pull request as draft August 29, 2024 07:35
@ansys-satyajeet
Copy link
Collaborator Author

@Samuelopez-ansys @lorenzovecchietti This example uses the Ansys board. As I understand correctly, we cannot distribute Galileo board and there are some restrictions on A1 board as well (this board is used in the existing example). Moreover, the old example was too long and combining IDF import with ECAD import. The new example aims to simplify the examples, addressing one specific topic, in this case importing and modify an ECAD in Icepak.

I understand the simplifications and shortening of examples can create more example that may have repeated code, and present testing and maintenance challenges, but for a user it would be very easy to quickly find what they are looking for and thus reduce the questions in Discussions.

@lorenzovecchietti
Copy link
Collaborator

Yes, that is perfectly fine. We can just remove the old example and keep just the one you pushed (removing the "v2" from the name)

I wanted also to showcase a bit the features of the pcb object. I couldn't work on this a lot today because the example is failing on my machine (I think the problem is just mine)

@ansys-satyajeet
Copy link
Collaborator Author

Yes, that is perfectly fine. We can just remove the old example and keep just the one you pushed (removing the "v2" from the name)

I wanted also to showcase a bit the features of the pcb object. I couldn't work on this a lot today because the example is failing on my machine (I think the problem is just mine)

Agree, showcasing the pcb component features is a good idea. Is there a way to add more to this existing pull request? Or do I need to create a new pull request for the additional code lines in this example? Not sure about the best practices.

@lorenzovecchietti
Copy link
Collaborator

lorenzovecchietti commented Aug 29, 2024 via email

Copy link
Collaborator Author

@ansys-satyajeet ansys-satyajeet left a comment

Choose a reason for hiding this comment

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

I have added a few lines to showcase PCB component features. I also modified the file names and tested the code a few times.

@lorenzovecchietti lorenzovecchietti marked this pull request as ready for review September 12, 2024 07:31
Copy link
Collaborator

@gmalinve gmalinve left a comment

Choose a reason for hiding this comment

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

The example doesn't follow the template.py format. It's not divided into sections and sections don't start with # ## Title followed by a blank line (#) and description (#).
Please look at other examples! :)

examples/05-electrothermal/ecad_import.py Outdated Show resolved Hide resolved
examples/05-electrothermal/ecad_import.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants