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

Add unit and displayUnit to signal input and output representing an angle #2488

Closed
christiankral opened this issue Mar 13, 2018 · 8 comments · Fixed by #3267
Closed

Add unit and displayUnit to signal input and output representing an angle #2488

christiankral opened this issue Mar 13, 2018 · 8 comments · Fixed by #3267
Assignees
Labels
enhancement New feature or enhancement
Milestone

Comments

@christiankral
Copy link
Contributor

There are a couple models in the MSL which provide an angle signal input or output. However, when simulating and displaying such angles, the displayUnit can often NOT be switched to "deg", since the output is just a RealOutput with no unit. Therefore, the angle signal input and output shall be equipped with (unit="rad",displayUnit="deg").

This issue applies to the following models (list may not yet be complete):

  • Modelica.Blocks.Math.Sin
  • ...
  • Modelica.Blocks.Math.Atan2
  • Modelica.Blocks.Math.WrapAngle
  • Modelica.Blocks.Math.RectangularToPolar
  • Modelica.Blocks.Math.RectangularToPolar
  • Modelica.Blocks.Math.Harmonic
  • Modelica.ComplexBlocks.ComplexMath.PolarToComplex
  • Modelica.ComplexBlocks.ComplexMath.ComplexToPolar
  • Modelica.ComplexBlocks.ComplexMath.Bode
  • Modelica.Electrical.Machines.SpacePhasors.Components.Rotator
  • Modelica.Electrical.Machines.SpacePhasors.Blocks.Rotator
  • Modelica.Electrical.Machines.SpacePhasors.Blocks.ToPolar
  • Modelica.Electrical.Machines.SpacePhasors.Blocks.FromPolar
  • Modelica.Electrical.Machines.Utilities.ToDQ
  • Modelica.Electrical.Machines.Utilities.FromDQ
  • Modelica.Magnetic.QuasiStatic.FundamentalWave.Utilities.CurrentController
  • Modelica.Electrical.Machines.Utilities.CurrentController
  • Modelica.Electrical.Machines.Utilities.VoltageController
@christiankral christiankral added the enhancement New feature or enhancement label Mar 13, 2018
@christiankral christiankral added this to the MSL3.2.3 milestone Mar 13, 2018
@christiankral christiankral self-assigned this Mar 13, 2018
@HansOlsson
Copy link
Contributor

There are a couple models in the MSL which provide an angle signal input or output. However, when simulating and displaying such angles, the displayUnit can often NOT be switched to "deg", since the output is just a RealOutput with no unit. Therefore, the angle signal input and output shall be equipped with (unit="rad",displayUnit="deg").

I can understand the need for unit="rad", and I would also hope that we could derive that unit in more cases. However, it may be excessive to use displayUnit="deg" - when looking at polar representation I don't necessarily consider degrees a better choice.

@AHaumer
Copy link
Contributor

AHaumer commented Mar 18, 2018

see #2494 (and #2492), let's continue the discussion on a generalized level in #2494.

@christiankral
Copy link
Contributor Author

OK, to me this sounds like adding the angle unit "rad" is possible without getting the MSL backwards incompatible, right? The displayUnit shall not be included -- so this makes sense to me. I can create a pull request, if we agree to go for adding units to angle inputs and outputs.

@AHaumer AHaumer modified the milestones: MSL3.2.3, MSL_next-MINOR-version May 20, 2018
@AHaumer
Copy link
Contributor

AHaumer commented May 20, 2018

I agree with Hans, displayUnit might be overdone.
It seems that unit (without diplayUnit) won't cause problems or incompatiblities.
I suggest starting implementation, but I think we won't get all models done for 3.2.3
Therefore shifting the milestone.

@AHaumer
Copy link
Contributor

AHaumer commented May 21, 2018

But don't forget angular velocity ;-)

@AHaumer
Copy link
Contributor

AHaumer commented May 21, 2018

I'm just working on the initially mentioned.
For two blocks this isn't possible:

  • Modelica.Electrical.Machines.SpacePhasors.Blocks.ToPolar
  • Modelica.Electrical.Machines.SpacePhasors.
    since the output is a vector of lenght 2, the first element representing the length and the second the angle. Don't know how to set the unit of just 1 element of an arry.

To be sure: We should go through the whole standard library a look for similar occurenes.
Blocks.FromPola

@AHaumer
Copy link
Contributor

AHaumer commented May 21, 2018

To be sure: We should go through the whole standard library a look for similar occurenes.

@AHaumer AHaumer reopened this May 21, 2018
@AHaumer AHaumer self-assigned this May 21, 2018
@AHaumer
Copy link
Contributor

AHaumer commented May 21, 2018

To be sure: We should go through the whole standard library a look for similar occurenes.

@AHaumer AHaumer reopened this May 21, 2018
@beutlich beutlich modified the milestones: MSL_next-MINOR-version, MSL_next-MAJOR-version Jun 24, 2018
beutlich added a commit to beutlich/ModelicaStandardLibrary that referenced this issue Dec 15, 2019
beutlich added a commit to beutlich/ModelicaStandardLibrary that referenced this issue Dec 15, 2019
@beutlich beutlich self-assigned this Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants