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

Merge HeatingNPN/HeatingPNP with NPN/PNP #3178

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

beutlich
Copy link
Member

This is the last missing conversion to finally close #2899.

@beutlich beutlich added L: Electrical.Analog Issue addresses Modelica.Electrical.Analog task General work that is not related to a bug or feature labels Oct 28, 2019
@beutlich beutlich added this to the MSL4.0.0 milestone Oct 28, 2019
@beutlich beutlich self-assigned this Oct 28, 2019
Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Looks good now.
I noticed that the manual conversion is slightly smarter than the automatic one in that useHeatPort=true is removed, but I don't see how we could handle that without an extension of the conversion - and I don't think that extension is worth the effort.

Copy link
Contributor

@christiankral christiankral left a comment

Choose a reason for hiding this comment

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

  • I tested the conversion script
  • I tested the examples Modelica.Electrical.Analog.Examples.HeatingNPN_NORGate and Modelica.Electrical.Analog.Examples.HeatingPNP_NORGate
  • I tested the old MSL 3.2.3 heating bipolar transistor models against the new models with non-default parameters
  • I checked the code

I found no discrepancy, so the PR is OK from my side

@beutlich beutlich merged commit c545a28 into modelica:master Oct 29, 2019
@beutlich beutlich deleted the merge-bjt branch October 29, 2019 18:40
@beutlich
Copy link
Member Author

I noticed that the manual conversion is slightly smarter than the automatic one in that useHeatPort=true is removed, but I don't see how we could handle that without an extension of the conversion

I thought that removing modifiers already is possible, see for example

convertModifiers({"Modelica.Mechanics.MultiBody.Joints.Prismatic"},
{"s_offset"}, fill("", 0), true);
convertModifiers({"Modelica.Mechanics.MultiBody.Joints.Revolute"},
{"phi_offset"}, fill("", 0), true);

@HansOlsson
Copy link
Contributor

I noticed that the manual conversion is slightly smarter than the automatic one in that useHeatPort=true is removed, but I don't see how we could handle that without an extension of the conversion

I thought that removing modifiers already is possible, see for example

I was unclear: removing useHeatPort-modifier in general would be easy, but removing it only when it was true is a bit trickier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: Electrical.Analog Issue addresses Modelica.Electrical.Analog task General work that is not related to a bug or feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove components having "Heating" in their name
3 participants