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

RotationalCoupling #4019

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Aug 5, 2022

Address #4012:

  • Moved RotationalCopling to Modelica.Electrical.Machines.Utilities.
  • Used IdealGear(ratio=-1) inside the coupling.
  • Imrpoved documentation.

@AHaumer AHaumer added enhancement New feature or enhancement L: Electrical.Machines Issue addresses Modelica.Electrical.Machines L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational labels Aug 5, 2022
Copy link
Member

@dietmarw dietmarw left a comment

Choose a reason for hiding this comment

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

I think this implementation is much cleaner. However, the comparisonSignals and test models introduced in #3786 should get removed/updated

@tobolar
Copy link
Contributor

tobolar commented Sep 15, 2022

I'm convinced with this solution.

But sorry for bother you again - I'm still stumbling upon the icon. To me, it makes an impression of a jaw coupling (#4012 (comment)) which in fact enables some very small relative rotation between the two flanges a and b - in fact, it is limited mechanically.
In contrast, considering the example Modelica.Electrical.Machines.Examples.DCMachines.DCPM_Drive, the relative angle between the two machines dcpm reaches several hunderts of radian.

We have obviously different points of view - while I'm focused on that one component only, you consider the whole drive test rig. Located in the Electrical library now, consider this as my last comment on this component and feel free to design the icon on your own.

@tobolar tobolar removed their request for review September 15, 2022 15:12
@beutlich beutlich removed their request for review September 24, 2022 10:19
@christiankral
Copy link
Contributor

christiankral commented Sep 27, 2022

To have the coupling more obvious in the simulation example it would make sense to add two rotational speed sensors. The doumentation could then refer to speedSensor1.w and speedSensor2.w.

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.

OK for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement L: Electrical.Machines Issue addresses Modelica.Electrical.Machines L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants