Thruster plugin: add tests and velocity control

[chapulina]

Summary

The Thruster plugin was added in #749 without tests. This PR adds tests, and while at it, I also:

• Documentation
  • Documented examples/worlds/auv_controls.sdf
  • Documented missing XML fields in Thruster.hh
  • Updated variables and docs that mentioned RPM, because the plugin uses SI for angular velocity (rad / s)
• XML params
  • Some parameters were listed as "required", but also offered a default value. I made the implementation consistent with documentation by making parameters that have default values be optional.
  • Fail loading the plugin if the user gives a joint name that doesn't exist
  • Default the namespace to the model name instead of empty; empty is weird because the topic becomes /model/joint/... - this can potentially break users that are currently leaving the namespace empty.
• Implementation
  • Compute the desired propeller angular velocity only when the desired thrust changes
  • I think the cmd_pos topic is very confusing for this plugin, because the commands are forces in Newtons. So I'm proposing changing the topic to cmd_thrust. I didn't remove cmd_pos for backwards compatibility, but I think we should tick-tock it out.
  • Take the <joint><pose> into account
• Velocity control
  • Ported the alternative joint velocity control from downstream (osrf/lrauv@c20b44c).
  • The motivation for this feature was that the PID was accumulating errors that made the propellers drift over time
  • While writing tests, it became clear that using JointVelocityCmd makes the vehicle much more unstable than directly applying the wrench to the link. So there's a trade-off.

Test it

Follow the instructions in auv_controls.sdf.

Here are some handy commands to run the test worlds too:

ign gazebo -r -v 4 src/ign-gazebo/test/worlds/thruster_pid.sdf
ign topic -t /model/sub/joint/propeller_joint/cmd_thrust  -m ignition.msgs.Double -p 'data: 0.3'

ign gazebo -r -v 4 src/ign-gazebo/test/worlds/thruster_vel_cmd.sdf
ign topic -t /model/custom/joint/propeller_joint/cmd_thrust  -m ignition.msgs.Double -p 'data: 0.3'

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• 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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[codecov]

Codecov Report

Merging #1190 (0908925) into ign-gazebo5 (ba53127) will increase coverage by 0.13%.
The diff coverage is 90.90%.

@@               Coverage Diff               @@
##           ign-gazebo5    #1190      +/-   ##
===============================================
+ Coverage        65.87%   66.00%   +0.13%     
===============================================
  Files              254      255       +1     
  Lines            20026    20127     +101     
===============================================
+ Hits             13192    13285      +93     
- Misses            6834     6842       +8     

Impacted Files	Coverage Δ
src/systems/thruster/Thruster.cc	92.07% <90.90%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba53127...0908925. Read the comment docs.

[arjo129]

We should be OK to merge this as most of the issues are experienced at low velocities and I suspect due to issues with the hydrostatics. Appologies I've not yet tested this this review was for another PR.

facepalm commented on wrong PR!!!

[matosinho]

I see that @chapulina added TODO to configure max and min thrust in the sdf. This means that it will be done in another PR? I think that would be a good addition.

[chapulina]

I see that @chapulina added TODO to configure max and min thrust in the sdf. This means that it will be done in another PR? I think that would be a good addition.

I wasn't planning on tackling it anytime soon, but I agree it would be a nice addition

[arjo129]

Thanks for doing this! The tests and up-streaming are looking good. I've run it on my computer and it works.

[iche033]

I see that @chapulina added TODO to configure max and min thrust in the sdf. This means that it will be done in another PR? I think that would be a good addition.

here's a pull request for this: #1318

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1