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

Fluid added mass #1592

Merged
merged 30 commits into from
Feb 2, 2023
Merged

Fluid added mass #1592

merged 30 commits into from
Feb 2, 2023

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Jul 14, 2022

🎉 New feature

Summary

Uses the APIs added as part of the Fluid Added Mass Proposal.

TODO

  • Check that added mass is correctly passed to physics engine
  • Create example world that demonstrates the added mass effect
  • Display added mass on component inspector
  • Add tests

Test it

Check out the example world once the PR is ready.

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.

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

@chapulina chapulina added the MBARI buoy Sponsored by MBARI buoy sim project: https://github.com/osrf/buoy_sim label Jul 14, 2022
@chapulina chapulina mentioned this pull request Jul 14, 2022
5 tasks
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Jul 14, 2022
@chapulina chapulina added the physics Involves Ignition Physics label Jul 14, 2022
@chapulina chapulina added the 🌱 garden Ignition Garden label Jul 25, 2022
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina self-assigned this Jul 25, 2022
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina added the enhancement New feature or request label Jul 29, 2022
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina added 🌱 garden Ignition Garden and removed 🌱 garden Ignition Garden labels Aug 10, 2022
@chapulina chapulina changed the base branch from main to gz-sim7 August 10, 2022 18:17
@chapulina
Copy link
Contributor Author

We're past feature freeze ❄️ . The plan is to not merge this PR into gz-sim7 until after the stable release (i.e. October).

@JoanAguilar JoanAguilar marked this pull request as ready for review November 15, 2022 00:22
@JoanAguilar JoanAguilar assigned JoanAguilar and unassigned chapulina Nov 15, 2022
@JoanAguilar
Copy link
Contributor

I just updated the branch and added the second test for added mass, I have also made the PR ready for review. I don't expect CI to pass, as the changes here depend on gazebosim/gz-physics#384, which hasn't been merged yet.

On a separate note, the "expected values" in both tests come from an implementation of the equations of motion (in python) that has gone through somewhat of a validation process. I have not pushed that code, but we should probably find a home for it and reference it from the tests, just in case these tests need to be revisited in the future.

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>
@mjcarroll
Copy link
Contributor

@osrf-jenkins retest this please

@mjcarroll mjcarroll removed the needs upstream release Blocked by a release of an upstream library label Dec 5, 2022
@mjcarroll
Copy link
Contributor

@JoanAguilar still a few Windows issues here:

C:\Jenkins\workspace\ign_gazebo-pr-win\ws\gz-sim\test\integration\added_mass.cc(50,33): error C2065: 'M_PI': undeclared identifier [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\gz-sim7\test\integration\INTEGRATION_added_mass.vcxproj]

C:\Jenkins\workspace\ign_gazebo-pr-win\ws\gz-sim\test\integration\added_mass.cc(50,14): error C2737: 'kForceAngVel': const object must be initialized [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\gz-sim7\test\integration\INTEGRATION_added_mass.vcxproj]

C:\Jenkins\workspace\ign_gazebo-pr-win\ws\gz-sim\test\integration\added_mass.cc(56,34): error C2065: 'M_PI': undeclared identifier [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\gz-sim7\test\integration\INTEGRATION_added_mass.vcxproj]

C:\Jenkins\workspace\ign_gazebo-pr-win\ws\gz-sim\test\integration\added_mass.cc(56,14): error C2737: 'kTorqueAngVel': const object must be initialized [C:\Jenkins\workspace\ign_gazebo-pr-win\ws\build\gz-sim7\test\integration\INTEGRATION_added_mass.vcxproj]

Signed-off-by: Joan Aguilar Mayans <joan@openrobotics.org>
@mjcarroll
Copy link
Contributor

@osrf-jenkins retest this please

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Merging #1592 (0ba6831) into gz-sim7 (af73ebe) will increase coverage by 0.12%.
The diff coverage is 22.38%.

@@             Coverage Diff             @@
##           gz-sim7    #1592      +/-   ##
===========================================
+ Coverage    64.64%   64.77%   +0.12%     
===========================================
  Files          343      344       +1     
  Lines        27542    27773     +231     
===========================================
+ Hits         17805    17989     +184     
- Misses        9737     9784      +47     
Impacted Files Coverage Δ
src/Util.cc 92.53% <ø> (ø)
src/gui/plugins/component_inspector/Inertial.cc 11.76% <11.76%> (ø)
src/systems/apply_link_wrench/ApplyLinkWrench.cc 77.77% <46.15%> (ø)
src/Conversions.cc 83.66% <100.00%> (-0.34%) ⬇️
.../plugins/component_inspector/ComponentInspector.cc 5.61% <100.00%> (+0.14%) ⬆️
...rc/systems/ackermann_steering/AckermannSteering.hh 100.00% <0.00%> (ø)
...rc/systems/odometry_publisher/OdometryPublisher.cc 88.75% <0.00%> (+0.87%) ⬆️
src/SimulationRunner.cc 92.15% <0.00%> (+1.88%) ⬆️
...rc/systems/ackermann_steering/AckermannSteering.cc 84.07% <0.00%> (+3.62%) ⬆️

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

@mjcarroll mjcarroll merged commit ad5fe37 into gz-sim7 Feb 2, 2023
@mjcarroll mjcarroll deleted the chapulina/7/added_mass branch February 2, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🌱 garden Ignition Garden MBARI buoy Sponsored by MBARI buoy sim project: https://github.com/osrf/buoy_sim physics Involves Ignition Physics
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants