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

Implemented FreeWheel #4004

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

Implemented FreeWheel #4004

wants to merge 17 commits into from

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Jun 26, 2022

As mentioned in #3977:
I see that we have a rather sophisticated Mechanics.Rotational.Components.OneWayClutch, but no simple ideal freewheel (like the Electrical.Analog.Ideal.Diode). We could introduce such a component for usage e.g. in bicycle models.

@AHaumer AHaumer added enhancement New feature or enhancement L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational labels Jun 26, 2022
@AHaumer AHaumer self-assigned this Jun 26, 2022
@AHaumer AHaumer enabled auto-merge June 26, 2022 16:48
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.

@AHaumer Please check the following changes and re-invite me for a review afterwards.

  1. I added SI units the parameters in 8c6ba79
  2. I updated the documentation of the example in 73078a6

@AHaumer AHaumer requested a review from christiankral July 4, 2022 16:57
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.

  • The new model is missing in ModelicaTest.Rotational.AllComponents.
  • The new example misses the required file Modelica/Resources/Reference/Modelica/Mechanics/Rotational/Examples/DemoFreeWheel/comparisonSignals.txt

@AHaumer AHaumer requested a review from beutlich July 5, 2022 06:29
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.

  • The new model is missing in ModelicaTest.Rotational.AllComponents.
  • The new example misses the required file Modelica/Resources/Reference/Modelica/Mechanics/Rotational/Examples/DemoFreeWheel/comparisonSignals.txt

Copy link
Contributor

@tobolar tobolar left a comment

Choose a reason for hiding this comment

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

Is there any reason to not extend from Modelica.Mechanics.Rotational.Interfaces.PartialCompliantWithRelativeStates as e.g. Rotational.Components.Clutch does?

@AHaumer AHaumer requested review from beutlich and tobolar July 29, 2022 16:17
…alCompliant

Thus, the sign convention of phi_rel (and w_rel) established for "rotational" components is assured.
Note: doing so, the sign of w_rel and tau changes - compared to the original implementation.
@tobolar
Copy link
Contributor

tobolar commented Aug 3, 2022

Is there any reason to not extend from Modelica.Mechanics.Rotational.Interfaces.PartialCompliantWithRelativeStates as e.g. Rotational.Components.Clutch does?

With 7e6f667, the freewheel extends now from Mechanics.Rotational.Interfaces.PartialCompliant (without the state selection since it could be problematic). Thus, the sign convention of phi_rel (and w_rel) established for all rotational components is assured now. Consequently, the sign of w_rel and tau changes - compared to the original implementation. The example models work fine.

Copy link
Contributor

@tobolar tobolar left a comment

Choose a reason for hiding this comment

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

Fine for me now. Nice work!

@christiankral
Copy link
Contributor

christiankral commented Aug 3, 2022

  1. Some time ago we were trying to get rid of all the "Diagram" drawings that explain the model. Instead, the "Diagram" diagram shall be added to the documentation. I will add it.
  2. The diagram tau vs w_rel has to be adapted, because:

With 7e6f667, the freewheel extends now from Mechanics.Rotational.Interfaces.PartialCompliant (without the state selection since it could be problematic). Thus, the sign convention of phi_rel (and w_rel) established for all rotational components is assured now. Consequently, the sign of w_rel and tau changes - compared to the original implementation. The example models work fine.

  1. The currently implemented diagram is a bit misleading, as it is not clear what the parameters tauRes and wRes really mean. The current "Diagram" view shows:

image

@christiankral
Copy link
Contributor

Before implementing a change I would like to discuss the meaning of the parameters tauRes and wRes:

The current implementation is:

  • if free = false the characteristic is indicated by tau = tauRes @ w_rel = 1 (1/s)
  • if free = true the characteristic is indicated by w_rel = -wRes @ tau = -1 (Nm)

Hmmm.... the actual implementatioin is somehow avoiding the units that specify the slope of the two branches in the diagram. The diode models uses Ron (unit V/A) anf Goff (unit A/V): this makes a lot more sense to me.

a. Creating the new units "Nm.s" and "1/(s.Nm)" may be one way to resolve this point.

b. We could improve the description to better explain the meaning of tauRes and wRes: this, however, seems not to be a sound way of dealing with this problem, as we avoid the slopes -- which indicate the physical nature of the model.

@AHaumer @tobolar any ideas on how to resolve this "unit" issue?

@christiankral
Copy link
Contributor

OK, possibly the unit SI.RotationalDampingConstant could resolve this issue...

@tobolar
Copy link
Contributor

tobolar commented Aug 4, 2022

The current implementation is:

  • if free = false the characteristic is indicated by tau = tauRes @ w_rel = 1 (1/s)
  • if free = true the characteristic is indicated by w_rel = -wRes @ tau = -1 (Nm)

To my understanding, the implementation is:

  • if free = false then tau = (-)flange_a.tau -> s -> w_rel = s*tauRes
  • if free = true then w_rel -> s -> tau = s*wRes

Hmm, the documentation says:

  • free = false: torque is transferred with a residual difference wRes of relative angular velocity of the flanges.
  • free = true: the flanges move independently except a residual friction torque tauRes.

It seems this fits not well.

@AHaumer AHaumer dismissed beutlich’s stale review August 5, 2022 08:16

already solved

@AHaumer
Copy link
Contributor Author

AHaumer commented Aug 5, 2022

@tobolar and @christiankral thanks for taking care!
I need some free hours to consider the changed sign convention ... then I'll propose the following:

  • Keep the "Diagram" drawing, but check whether it has to be adapted to the changed signs.
  • Check the documentation whether it has to be adapted to the changed signs.
  • Replace the parameter tauRes by RotationalDampingCoefficientFree.
  • Replace the parameter wRes by 1/RotationalDampingCoefficientLocked.

@AHaumer
Copy link
Contributor Author

AHaumer commented Aug 5, 2022

@tobolar and @christiankral I think a have a sound solution (looks like a translation of the electrical diode):

  parameter SI.RotationalDampingConstant residualFriction=1e-5 "Residual friction coefficient (free = true)";
  parameter SI.RotationalDampingConstant torqueTransmission=1e5 "Torque transmission coefficient (free = false)";
  • free = false: flange_a is driving flange_b; residual slip is defined by w_rel = tau/torqueTransmission.
  • free = true: flange_b rotates (nearly) independent of flange_a; residual friction is defined by tau = w_rel*residualFriction.
    grafik

@AHaumer AHaumer requested a review from tobolar August 5, 2022 17:18
@tobolar
Copy link
Contributor

tobolar commented Aug 18, 2022

@AHaumer , @christiankral
Thinking again about the two "residual" parameters.
One can understand tauRes at free=true being a residual torque due to some friction occurency between the two flanges. Thus, for me both tauRes (N.m) and also residualFriction (no SI unit!) could be acceptable.
Regarding wRes (rad/s), this is a sort of slip velocity ("Schlupf") which occurs at free=false and forces w_b being smaller then w_a.
So to me, the current documentation of the model is fine and provides an explanation everybody can follow.
(I know, you both clarified this already. ;-) )

I'm still stumbling about the two parameters. Using tauRes and wRes has an advantage that user can use its own terms to get they values, e.g. the friction. In contrast, torqueTransmission is somehow fancy.

Regarding units - both tauand w_rel depend, in fact, on the unitless variable s which only "transfers" value, not unit.
So I would tend to b) in #4004. Of course, this makes plotting of the two branches in the diagram difficult.

@dietmarw dietmarw removed their request for review September 5, 2022 13:03
@beutlich beutlich removed their request for review September 24, 2022 10:20
@christiankral
Copy link
Contributor

@tobolar Would the following wording make it better on your opinion:

parameter Real(final unit="rad/N/m/s") residualAngularVelocity =1e-5 "Residual angular velocity coefficient (free = false)";
...
  w_rel = s*unitTorque         *(if free then 1 else residualAngularVelocity);

@tobolar
Copy link
Contributor

tobolar commented Nov 8, 2022

Would the following wording make it better on your opinion:..

Sounds good. The following could be a reasonable implementation now:

parameter Real residualFriction=1e-5 "Residual friction coefficient (if free = true)";
parameter Real residualAngularVelocity=1e-5 "Residual angular velocity coefficient (if free = false)";
...
w_rel = s*unitAngularVelocity * (if free then 1 else residualAngularVelocity);
tau   = s*unitTorque          * (if free then residualFriction else 1);

The documentation shall be modified then:

free = false: flange_a is driving ...
...
tau = w_rel / residualAngularVelocity;

Regarding the two equations for tauin the documentation (where neither unitAngularVelocity nor unitTorque is used), the correct units of the two parameters shall be:

  • residualFriction (unit="N.s")
  • residualAngularVelocity (unit="1/N/s")

but it seems to be strange since one could, in general, expect both friction and a coefficient being unitless.

@christiankral
Copy link
Contributor

christiankral commented Nov 15, 2022

w_rel = s*unitAngularVelocity * (if free then 1 else residualAngularVelocity);
tau   = s*unitTorque          * (if free then residualFriction else 1);

On my understanding the units of the proposed equations are not correct. They shall rather be:

parameter residualFriction (unit="N.m.s")=1e-5 "Residual friction coefficient (if free = true)";
parameter residualAngularVelocity (unit="1/N/m/s")=1e-5 "Residual angular velocity coefficient (if free = false)";
w_rel = s*unitTorque          * (if free then 1 else residualAngularVelocity);
tau   = s*unitAngularVelocity * (if free then residualFriction else 1);

Note, that I swaped unitTorque and unitAngularVelocity in the two equations.

@christiankral
Copy link
Contributor

  • residualFriction (unit="N.s")
  • residualAngularVelocity (unit="1/N/s")

@tobolar As we deal with torques (not forces) the units shall rather be N.m.s and 1/N/m/s, isn't is?

@christiankral
Copy link
Contributor

Alternatively we could consider the unit rad, but I am not sure if this makes sense.

parameter residualFriction (unit="N.m.s/rad")=1e-5 "Residual friction coefficient (if free = true)";
parameter residualAngularVelocity (unit="rad/N/m/s")=1e-5 "Residual angular velocity coefficient (if free = false)";

@tobolar
Copy link
Contributor

tobolar commented Nov 16, 2022

@christiankral
IMO, one would expect regarding a mechanical component:

  1. "friction coefficient" (mu overall in MSL) is generally a unitless quantity,
  2. and so is probably also with "angular velocity coefficient".

This is why I tend to have no units. Optionally, we schould think again of changing "residualFriction" name and description. Not sure if this also holds for residualAngularVelocity.

@HansOlsson
Copy link
Contributor

Note that unitless can mean two different things:

  • A variable without a specified unit.
  • A variable with unit="1", also known as dimensionless.

To me the friction coefficient should be of the second kind for translational friction, but I notice that often in MSL it is of the first kind. (And it is generally not possible to deduce its unit because it appears as mu*cgeo where the geometric coefficient also is without unit.)

For rotational friction it is not clear to me if mu or cgeo should handle the conversion between force and torque.

@tobolar
Copy link
Contributor

tobolar commented Nov 16, 2022

@HansOlsson Need to open a new issue?
(See Sec. 2.3.3 Dimensions of quantities of the BIPM: The International System of Units – 9th edition (2019) - probably you know it already. )

@HansOlsson
Copy link
Contributor

@HansOlsson Need to open a new issue?

Possibly. You decide.

@christiankral
Copy link
Contributor

@HansOlsson Is using unitAngularVelocity and unitTorque still "state of the art" in how we handle the diode and freewheel implementation?

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: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants