Adding thrust coefficient calculation

[marcoag]

Signed-off-by: Marco A. Gutierrez marco@openrobotics.org

🎉 New feature

Work towards osrf/lrauv#225

Summary

Thrust coefficient in Thruster plugin can only be set to a fixed number, by default 1. This PR adds a calculation that makes it depend on the velocity of the vehicle. The calculation implemented here is based Fossen's equations described in "Guidance and Control of Ocean Vehicles".

Test it

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[hidmic]

@marcoag meta: hmm, something's off here. If the thrust coefficient is zero, then no thrust should apply. If the propeller angular velocity is zero, then no thrust should apply and the thrust coefficient is not defined (and it doesn't matter anyways). I see this cause-effect relationship is reversed in this plugin: we assume thrust is always applicable and compute propeller angular velocity. I think we should set that straight. Then you would not need this conditional.

[marcoag]

When the plugin starts the angular velocity is zero, making the thrust coefficient nan. Since it's used later on to calculate the propeller angular velocity then the model would never start moving. The intent of this check was to break this loop and use the default thrust coefficient which is 1 and won't affect the final result. I think it might suffice with the isnan check for this or another option is to only update the thrust coefficient once the propeller angular velocity is not 0, maybe the later makes more sense?

[hidmic]

Ah, almost forgot to reply.

I guess my point is that the current implementation is not a good approximation. It takes thrust commands instead of propeller angular velocities, and applies forces instantaneously. That's why we are running into this odd corner cases. It makes me wonder, why use this improved thrust coefficient if the dynamics are not there.

Anyways, I think that if we want to keep everything as-is, we could (a) set propeller angular velocity to zero if thrust is zero, bypassing all calculations, and (b) solve the quadratic-like equation to get propeller angular velocity in all other cases, as if it had reached steady state instantaneously (like with forces).

[marcoag]

I don't know the reason to take thrust commands instead of propeller angular velocities here, the later would be even easier since it seems the thrust gets converter into an angular velocity anyway.

a) The propeller angular velocity would be zero if thrust is zero using calculations, so there doesn't seem a to be a need to bypass them.

Still the problem is a chicken egg dependency between velocity and thrust coefficient, due to a mutual dependency, if any of them is zero it would make the other one zero, preventing the vehicle to move when starting from stationary. With the conditional above I was trying to avoid this problem. I guess it makes more sense to only calculate when angular_velocity != 0 and use the default thrust coefficient (or latest) when that happens. Will add this change for now.

[hidmic]

As per offline discussion, let's go with this as-is. I ran a couple, very much simplified numerical experiments and the coefficient converges even if we start from a guess. I can't swear on convergence, but if the sim looks OK we should be fine.

solve the quadratic-like equation to get propeller angular velocity in all other cases, as if it had reached steady state instantaneously (like with forces).

Re: to myself. This is not super easy, I missed the linear velocity term in the thrust coefficient model.

[hidmic]

@marcoag bibliographic references here would be great.

[marcoag]

Added in 7351aab.

[hidmic]

LGTM but for two relatively minor comments

[hidmic]

@marcoag could the thrust coefficient ever become zero while moving (in other words, could either $\alpha_1$ or $\alpha_2$ be negative)? If so, maybe we should reset thrust coefficient to one if propeller angular velocity ever becomes zero. WDYT?

[hidmic]

Also, I'm surprised the compiler didn't complain about a floating point comparison to zero. AFAIK std::abs(this->propellerAngVel) < std::numeric_limits<double>::epsilon() would be a slightly more robust test.

[marcoag]

It did threw a warning, I assumed anything that is not a strict zero would work.

Changed to your suggestion: 51337cd.

[hidmic]

LGTM pending green CI

[codecov]

Codecov Report

Merging #1652 (b051ef0) into ign-gazebo6 (69e7757) will increase coverage by 0.03%.
The diff coverage is 85.18%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1652      +/-   ##
===============================================
+ Coverage        64.69%   64.73%   +0.03%     
===============================================
  Files              321      321              
  Lines            26062    26155      +93     
===============================================
+ Hits             16862    16931      +69     
- Misses            9200     9224      +24     

Impacted Files	Coverage Δ
src/systems/thruster/Thruster.cc	89.50% <85.18%> (-0.87%)	⬇️
src/SimulationRunner.cc	90.93% <0.00%> (-0.96%)	⬇️
src/gui/plugins/scene_manager/GzSceneManager.cc	17.58% <0.00%> (-0.60%)	⬇️
.../systems/triggered_publisher/TriggeredPublisher.cc	83.28% <0.00%> (-0.38%)	⬇️
.../systems/triggered_publisher/TriggeredPublisher.hh	100.00% <0.00%> (ø)
src/rendering/RenderUtil.cc	39.01% <0.00%> (+0.11%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[Crola1702]

👨‍🌾 Hi there!

I was taking a look at this issue on the buildfarm and I noticed we have a new gcc warning.

• Gcc warning:

comparing floating point with == or != is unsafe [-Wfloat-equal] 247 | if (_calculateCoefficient && angularVelocity != 0) | ^

On thruster.cc:247.

Do you mind taking a look at how to solve/supress it? Thanks

[j-rivero]

Do you mind taking a look at how to solve/supress it? Thanks

I think that @ahcorde arrived to this #1754