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.

Sign up for GitHub

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

Jump to bottom

Rename Machines Controllers #3058

Merged
merged 9 commits into from
Jul 19, 2019
Merged

Rename Machines Controllers #3058

merged 9 commits into from
Jul 19, 2019

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Jul 17, 2019

Naming of controllers is misleading, has to be clarified:
Electrical.Machines.Utilities.CurrentController -> DQToThreePhase
Electrical.Machines.Utilities.CurrentController.{id_rms, iq_rms} -> DQToThreePhase.{d, q}
Electrical.Machines.Utilities.VoltageController -> CurrentController
Electrical.Machines.Utilities.VoltageController,{id_rms, iq_rms} -> CurrentController,{id, iq}
Renaming and conversion script done, test cases follow.

@AHaumer AHaumer added enhancement New feature or enhancement L: Electrical.Machines Issue addresses Modelica.Electrical.Machines P: high High priority issue labels Jul 17, 2019
@AHaumer AHaumer added this to the MSL4.0.0 milestone Jul 17, 2019
beutlich
beutlich reviewed Jul 17, 2019
View reviewed changes
Copy link
Member

@beutlich beutlich left a comment •
edited

Choose a reason for hiding this comment

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

I have no clue if it is advisable to rename VoltageController to an existing model CurrentController which also gets renamed. In case you run the conversion twice you'll get DQToThreePhase then. (I know, conversion is guarded by the uses annotation, but who knows?)

@christiankral
Copy link
Contributor

We could rename the "new" CurrentController to DQCurrentController.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 17, 2019

@beutlich good point ... followd @christiankral 's advice.

@AHaumer AHaumer mentioned this pull request Jul 17, 2019
Merged
@beutlich
Copy link
Member

After merging #3059 this is now conflicting.

@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 18, 2019 •
edited

@beutlich conflicts should be resolved.
At least I made the same adaptions in the two examples ...

Should be done now.

@AHaumer AHaumer requested a review from beutlich July 18, 2019 19:52
beutlich
beutlich requested changes Jul 18, 2019
View reviewed changes
Copy link
Member

@beutlich beutlich left a comment •
edited

Choose a reason for hiding this comment

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

There still is some issue, since the block SinCosEvaluation occurs twice in the library.

@beutlich beutlich self-assigned this Jul 18, 2019
@AHaumer
Copy link
Contributor Author

AHaumer commented Jul 18, 2019

@beutlich sorry don't know how this could happen, resolved.

@AHaumer AHaumer requested a review from beutlich July 18, 2019 21:21
@beutlich
Copy link
Member

Might be off-topic: Why are Modelica.Electrical.Machines.Utilities.SynchronousMachineData and Modelica.Electrical.Machines.Utilities.TransformerData not in Modelica.Electrical.Machines.Utilities.ParameterRecords?

beutlich
beutlich approved these changes Jul 18, 2019
View reviewed changes
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.

One conversion was missing in ModelicaTest.Electrical.Machines.SMPM_VoltageSourceWithLosses.

@AHaumer AHaumer merged commit 6f60a37 into modelica:master Jul 19, 2019
@AHaumer AHaumer deleted the RenameMachinesControllers branch July 19, 2019 04:52
@beutlich beutlich assigned AHaumer and unassigned beutlich Jul 19, 2019
@beutlich beutlich added task General work that is not related to a bug or feature and removed enhancement New feature or enhancement labels Jul 19, 2019
@beutlich beutlich mentioned this pull request Oct 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christiankral christiankral christiankral approved these changes

@beutlich beutlich beutlich approved these changes

Assignees

@AHaumer AHaumer

Labels
L: Electrical.Machines Issue addresses Modelica.Electrical.Machines P: high High priority issue task General work that is not related to a bug or feature
Projects
None yet
Milestone
MSL4.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AHaumer @christiankral @beutlich