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

Clean up lrauv_gazebo_plugins worlds #272

Merged
merged 2 commits into from
Nov 24, 2022
Merged

Clean up lrauv_gazebo_plugins worlds #272

merged 2 commits into from
Nov 24, 2022

Conversation

hidmic
Copy link
Collaborator

@hidmic hidmic commented Nov 14, 2022

Precisely what the title says. Incidentally closes #132.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Collaborator Author

hidmic commented Nov 14, 2022

@arjo129 @adityapande-1995 @caguero I'm seeing strange behavior. Running:

gz sim -v4 --gui-config install/share/lrauv_gazebo_plugins/config/gui.config portuguese_ledge.sdf

has the intended outcome, whereas:

gz sim -v4 --gui-config install/share/lrauv_gazebo_plugins/config/gui.config tethys_at_portuguese_ledge.sdf

yields nothing. Do you see the same?

@arjo129
Copy link
Member

arjo129 commented Nov 15, 2022

Neither are working for me. I'm getting the following error:

 [GUI] [Err] [Application.cc:290] Failed to load file [install/share/lrauv_gazebo_plugins/config/gui.config]: XMLError
[GUI] [Wrn] [Gui.cc:373] Failed to load config file[install/share/lrauv_gazebo_plugins/config/gui.config].

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 15, 2022

@arjo129 ahh, I wasn't explicit about how to build the workspace. It should be merged installed !

@arjo129
Copy link
Member

arjo129 commented Nov 15, 2022

Hmmm lots of weird behavior. I can see the gui but no hint of tiles or tethys.

@arjo129
Copy link
Member

arjo129 commented Nov 15, 2022

This is what I see for both:
image
image

Might I add that I believe the hooks file assumes a non-merge-install scenario.

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 15, 2022

Hmmm lots of weird behavior. I can see the gui but no hint of tiles or tethys.

It's less weird than having only one working 😅 I suspect some sort of race in how plugins are loaded, but it's just a hunch.

Might I add that I believe the hooks file assumes a non-merge-install scenario.

Hmm, shouldn't be the case as we're using cmake install prefix:

_colcon_prepend_unique_value GZ_SIM_RESOURCE_PATH @CMAKE_INSTALL_PREFIX@/share/@PROJECT_NAME@/worlds
_colcon_prepend_unique_value GZ_SIM_RESOURCE_PATH @CMAKE_INSTALL_PREFIX@/share/@PROJECT_NAME@/models
_colcon_prepend_unique_value GZ_SIM_RESOURCE_PATH @CMAKE_INSTALL_PREFIX@/share/@PROJECT_NAME@/data
_colcon_prepend_unique_value GZ_SIM_SYSTEM_PLUGIN_PATH @CMAKE_INSTALL_PREFIX@/lib

Edit: I get the exact same behavior with merged and symlink installed workspaces.

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 15, 2022

Alright, it was picking up another SDF file with the same name for me... Now we are seeing the same.

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 15, 2022

Curiously enough, removing ScienceSensorSystem and EnvironmentPreload plugins "fixes" the problem. Maybe not a race but either plugin failing to setup?

@arjo129
Copy link
Member

arjo129 commented Nov 15, 2022

We should be removing science sensor soon enough. I can take alook at EnvironmentPreload and see what happens.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic marked this pull request as ready for review November 21, 2022 19:05
@hidmic
Copy link
Collaborator Author

hidmic commented Nov 21, 2022

I gave up on trying to figure out what was going on. Merged everything back into each world in 7e72734. That works just fine.

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 21, 2022

@caguero re: number of worlds. Let me know if you want to go away with the worlds that start with a Tethys in them or not. Remember there is no generic mechanism to work with these worlds (everything is setup to work with MBARI LRAUVs only).

@arjo129
Copy link
Member

arjo129 commented Nov 22, 2022

Getting some weird behavior. Not sure exactly what went wrong. Need to investigate.
Screencast from 11-22-2022 11:36:08 AM.webm

@arjo129
Copy link
Member

arjo129 commented Nov 22, 2022

Seems like if a vehicle is spawned, EachNew() is not catching it here, hence buoyancy is not getting executed. I think this seems to be an issue with the ECM.

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 22, 2022

Seems like if a vehicle is spawned, EachNew() is not catching it here, hence buoyancy is not getting executed. I think this seems to be an issue with the ECM.

@arjo129 uh, good find. It is catching it though, but no collisions are found (at the time). A race?

@arjo129
Copy link
Member

arjo129 commented Nov 22, 2022

Yeah looks like it I'm trying to find the Entity Creation service. Are you able to reproduce the behaviour?

@arjo129
Copy link
Member

arjo129 commented Nov 22, 2022

OK so it makes sense. Seems like the UserCommand system responsible for creating the entities only creates the entities during the PreUpdate phase. At the same time the buoyancy plugin only handles new entities at creation time only during the PreUpdate phase. At the end of the time step new entities are all reset. Hence, if Buoyancy runs before UserCommand EachNew() will not catch anything new created and by the second time step all new creations will have been cleared. I have a fix for the buoyancy plugin that I will push now.

Update: The fix is taking a little longer than expected. Not the only unexpected thing that happened (:argentina: vs :saudi_arabia:).

@hidmic
Copy link
Collaborator Author

hidmic commented Nov 22, 2022

Not the only unexpected thing that happened

Someone went to bed in a jokey mood.

@caguero
Copy link
Collaborator

caguero commented Nov 22, 2022

@caguero re: number of worlds. Let me know if you want to go away with the worlds that start with a Tethys in them or not. Remember there is no generic mechanism to work with these worlds (everything is setup to work with MBARI LRAUVs only).

If we still don't have a generic way of launching vehicles I'm leaning towards keeping it as it is preserving the worlds.

arjo129 added a commit to gazebosim/gz-sim that referenced this pull request Nov 22, 2022
For reference see osrf/lrauv#272 (comment)

Essentially, if we spawn an entity using the `UserCommand` system, it will create the entity during the `PreUpdate` phase. However, the `Buoyancy` system only checks for new entities during the `PreUpdate` phase (It does this to precompute volume so it does not need to be computed on every run). This means the buoyancy system may or may not catch the newly created entities.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Copy link
Member

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Works for me once gazebosim/gz-sim#1808 is merged.

@hidmic hidmic mentioned this pull request Nov 23, 2022
38 tasks
arjo129 added a commit to gazebosim/gz-sim that referenced this pull request Nov 23, 2022
* Fixes buoyancy flakiness when spawning entities

For reference see osrf/lrauv#272 (comment)

Essentially, if we spawn an entity using the `UserCommand` system, it will create the entity during the `PreUpdate` phase. However, the `Buoyancy` system only checks for new entities during the `PreUpdate` phase (It does this to precompute volume so it does not need to be computed on every run). This means the buoyancy system may or may not catch the newly created entities.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
@arjo129 arjo129 merged commit 2c8ce78 into main Nov 24, 2022
@arjo129 arjo129 deleted the hidmic/world-cleanup branch November 24, 2022 08:26
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.

Install server configs instead of repeating plugins for each world
3 participants