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

Buoyancy plugin #485

Open
wants to merge 3 commits into
base: gazebo_classic
Choose a base branch
from
Open

Buoyancy plugin #485

wants to merge 3 commits into from

Conversation

acxz
Copy link

@acxz acxz commented Jul 20, 2022

Solves #484 and #167

The buoyancy computation code is removed from usv_dynamics and the generic BuoyancyPlugin is used.

The buoyant force has been verified to be within 50 N of the previous code. Let me know if you would like videos for verification.

@bsb808
Copy link
Collaborator

bsb808 commented Jul 26, 2022

@acxz This is a really interesting idea. When we wrote this plugin the standard Gazebo buoyancy plugin wasn't able to deal with items at the surface - only fully submerged items.

I tested the modification with an exaggerated sea-state. There seems to be issues with synchronization with the visual wave field and lack of induced vessel pitch and roll. Those are critical capabilities for our use-case.

I'm not very familiar with the latest buoyancy. Perhaps the standard plugin can be used for this application. Is there documentation on how the plugin uses the <wave_model> and "buoyancy" tags?

@bsb808
Copy link
Collaborator

bsb808 commented Jul 26, 2022

Peek 2022-07-25 17-38

@acxz
Copy link
Author

acxz commented Jul 26, 2022

@bsb808 thanks for looking at this PR!

I tested the modification with an exaggerated sea-state.

Can you provide the files you changed, so that I can test the scenario on my local machine.

There seems to be issues with synchronization with the visual wave field and lack of induced vessel pitch and roll.

This is a problem of the wave field visuals vs the actual simulated wave field height. You will also see similar behavior with the existing buoyancy code, without the plugin. Have you compared the two in your exaggerated scenario? I ran my own exaggerated scenario setting scale to 1000 here

and here

and observed the same behavior. A different issue should be opened for the discrepancy between the wavemodel and wavevisuals.

As for the lack of induced pitch and roll, the previous code also does not handle it. This PR is only meant to be a generalization to vessel's buoyancy computation, instead of hardcoding it as two half cyclinders. A different issue should be opened for the lack of induced pitch and roll.

Perhaps the standard plugin can be used for this application. Is there documentation on how the plugin uses the <wave_model> and "buoyancy" tags?

Here is the current upstream BuoyancyPlugin code: https://github.com/osrf/gazebo/blob/2e42eb91e7638da54b36dbb8fdd4ae5e3a310d6d/plugins/BuoyancyPlugin.cc

As you can tell, the upstream BuoyancyPlugin is not at all as fancy as the one we have here in VRX. I don't think this is an option when trying to improve the buoyancy code we have in VRX.

It also does not use the <wave_model> tag.

acxz added 3 commits August 3, 2022 09:43
remove buoyancy code inside usv dyanmics, model wamv buoyancy as two buoyant cylinders in location of pontoons
@acxz acxz force-pushed the buoyancy_plugin branch from b6295ea to cbc9bad Compare August 3, 2022 14:43
@acxz acxz mentioned this pull request Sep 2, 2022
@bsb808
Copy link
Collaborator

bsb808 commented Sep 2, 2022

See https://github.com/osrf/vrx/wiki/changing_plugin_params_tutorial to change wave parameters. I probably used period=10 and gain=1

@acxz
Copy link
Author

acxz commented Sep 2, 2022

Thanks for sending the params you tested on. Just tested this PR and the existing codebase and the same behavior is observed.

For this PR in particular is there any closure that I can expect, now that the master branch has been put in maintenance mode.

cc'ing @M1chaelM since he has made issue #487 which this PR helps tackle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants