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

Convert "HeatingResistor" to "Resistor" in Electrical.Analog #2983

Merged
merged 3 commits into from
Jun 22, 2019

Conversation

christiankral
Copy link
Contributor

@christiankral christiankral commented Jun 21, 2019

see #2899

@christiankral christiankral added the L: Electrical.Analog Issue addresses Modelica.Electrical.Analog label Jun 21, 2019
@beutlich
Copy link
Member

Do you want me to exclude the "invalid" commit on V.A?

@beutlich beutlich added this to the MSL4.0.0 milestone Jun 21, 2019
@christiankral
Copy link
Contributor Author

The conversion script proposed in #2899 (comment) does not work in Dymola. I therefore changed it in c31d6a8 -- which triggers Dymola to convert Issue2899 as intended.

However, I do not understand why the performed change in c31d6a8 works, as this makes only sense to me if

convertElement("Modelica.Electrical.Analog.Basic.HeatingResistor",
               {"R_ref"},
               {"R"});

is performed before

convertClass("Modelica.Electrical.Analog.Basic.HeatingResistor",
              "Modelica.Electrical.Analog.Basic.Resistor");

even though the two conversions are implemented in the conversion script in the opposite order.

@beutlich Can you please double check if the conversion script is correct?

@beutlich
Copy link
Member

@christiankral Can only check in Dymola, too, which is the only kown tool to support conversion currently.

We could also rename R to R_ref in Resistor.

@beutlich
Copy link
Member

Conversion of ModelicaTestConversion4.Electrical.Analog.Issue2899 works as expected.

Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

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

Please update Modelica.Electrical.Analog.Examples.HeatingResistor to no longer use the HeatingResistor.

@christiankral
Copy link
Contributor Author

Do you want me to exclude the "invalid" commit on V.A?

@beutlich My fault. I guess I shall "undo" the change, as the change is handled in #2982, correct

@beutlich
Copy link
Member

Yes, you can do so.

@christiankral
Copy link
Contributor Author

Please update Modelica.Electrical.Analog.Examples.HeatingResistor to no longer use the HeatingResistor.

Thanks, first I thought about it then then I forgot about it... I will change the example.

@beutlich beutlich added the task General work that is not related to a bug or feature label Jun 21, 2019
Previous conversion did not work as expected, as parameter "R_ref" was not converted to "R" in Dymola; the updated conversion script now works in Dymola
@beutlich beutlich merged commit cf4ceff into modelica:master Jun 22, 2019
@beutlich beutlich removed the request for review from AHaumer June 22, 2019 12:10
@HansOlsson
Copy link
Contributor

The conversion script proposed in #2899 (comment) does not work in Dymola. I therefore changed it
...
is performed before

The script-commands don't convert models one by one, but they sort of create a conversion table that is then applied in one go - and that's why the order doesn't matter.

@christiankral christiankral deleted the removeHeatingResistor branch June 24, 2019 14:21
christiankral added a commit to christiankral/ModelicaStandardLibrary that referenced this pull request Jul 1, 2019
…r examples

The heating resistor of Modelica.Thermal.HeatTransfer.Examples.ControlledTemperature has not been converted by modelica#2983
beutlich pushed a commit that referenced this pull request Jul 1, 2019
…es (#3019)

The heating resistor of Modelica.Thermal.HeatTransfer.Examples.ControlledTemperature has not been converted by #2983
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.

None yet

4 participants