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

[Gazebo 11] Allow gazebo to download models from Fuel in the sdf files #2822

Merged
merged 20 commits into from
Sep 22, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Aug 12, 2020

Some of the bullet point from the related issue: #2406

remove the need to set GAZEBO_MODEL_PATH

Models are recorded in ~/.ignition/fuel I don't need to set up this environment variable.

support Fuel URIs specified in SDF

This PR allows to load/download models from Fuel in the sdf files

I create this world to test these features:

  • Model
  • Files (just the mesh, for example dae files)

make sure both server and client is able to download from fuel

I used gzserver and the model are being downloaded

test saving world with models from fuel and opening the saved world

I saved the world with some fuel models. I reopenned the file again and models are shown properly. Some of the URIs are hardcoded to absolute path in my computer

test logging and verify playback on new machine

If I record a log and then I remove the fuel models from ~/.ignition/fuel some of the models are not visualize properly (meshes and textures are not being shown). I think It's because how the world is saved, it's using absolute paths.

support downloading models that contain references to other models

@chapulina can you give some examples of these worlds?

Signed-off-by: ahcorde ahcorde@gmail.com

chapulina and others added 3 commits June 25, 2020 20:14
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added the 11 Gazebo 11 label Aug 12, 2020
@ahcorde ahcorde requested a review from chapulina August 12, 2020 10:50
@ahcorde ahcorde self-assigned this Aug 12, 2020
@chapulina
Copy link
Contributor

chapulina commented Aug 12, 2020

It's because how the world is saved, it's using absolute paths.

That should have been fixed in gazebosim/gz-fuel-tools#85. We just need an ign-fuel-tools4 release to be able to use that on Gazebo 11. I'll work on that soon.

support downloading models that contain references to other models

After the discussion on gazebosim/gz-fuel-tools#77 (comment), the recommended pattern is using full URLs. And ign-fuel-tools should also be handling the model:// case referring to other assets as discussed here: gazebosim/gz-fuel-tools#85 (comment)

@ahcorde
Copy link
Contributor Author

ahcorde commented Aug 13, 2020

I compiled from sources ign-fuel-tools4, right now these three points are working too.

  • test saving world with models from fuel and opening the saved world

  • test logging and verify playback on new machine

  • support downloading models that contain references to other models

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Aug 13, 2020

With the last commit we can load worlds from URL using the command line:

gazebo --verbose https://staging-fuel.ignitionrobotics.org/1.0/chapulina/worlds/Shapes

are worlds fully compatible between Gazebo and Ignition ? I don't know if there any plan to move worlds to fuel too

@chapulina chapulina added the migration Helps with migration from Gazebo classic to Ignition label Aug 13, 2020
@chapulina
Copy link
Contributor

are worlds fully compatible between Gazebo and Ignition ?

We're working on it.

The main pending item now is that Ignition requires a few world plugins to be defined for simulation to work, while classic doesn't need that. This is being addressed by gazebosim/gz-sim#281.

After that, the only difference will be some SDF tags here and there, but the worlds should be largely compatible.

chapulina and others added 4 commits August 13, 2020 16:41
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@ahcorde
Copy link
Contributor Author

ahcorde commented Aug 26, 2020

friendly ping @chapulina

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Great work, all the use cases are working for me! Do you think you could add a few tests to make sure we don't break the functionality?

I create this world to test these features:

Mind committing that world to this repo? I could be called fuel.world.


I think that with this PR, we can remove the need for the USE_IGNITION_FUEL environment variable:

https://github.com/osrf/gazebo/blob/a9f093f2fc7fa7fc49efa73719f6536ad28f8af7/gazebo/gui/InsertModelWidget.cc#L52-L57

gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/common/CommonIface.cc Outdated Show resolved Hide resolved
gazebo/common/CommonIface.cc Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Aug 28, 2020

Added two tests

  • check that the model are downloaded and included in the world
  • check world from a URI

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/common/FuelModelDatabase.cc Show resolved Hide resolved
test/integration/factory.cc Outdated Show resolved Hide resolved
test/integration/factory.cc Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 3, 2020

@osrf-jenkins run tests again

gazebo/Server.cc Outdated Show resolved Hide resolved
gazebo/common/CommonIface.cc Show resolved Hide resolved
{
gzerr << "Unable to download model[" << _uri << "]" << std::endl;
return std::string();
if (path.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (path.empty()) {
if (path.empty())
{

gazebo/common/FuelModelDatabase.cc Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

@osrf-jenkins run tests please?

This looks good to me, now we just need to check CI.

@chapulina
Copy link
Contributor

@osrf-jenkins run tests maybe?

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 14, 2020

@osrf-jenkins run tests please

@chapulina
Copy link
Contributor

It looks like this breaks the Windows build:

C:\Jenkins\workspace\gazebo-ci-pr_any-windows7-amd64\ws\install\include\ignition\fuel_tools4\ignition/fuel_tools/Result.hh(48): error C2143: syntax error: missing '}' before '('

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 21, 2020

@osrf-jenkins run tests please

@chapulina
Copy link
Contributor

@ahcorde , the latest Windows build had the same issue. I vaguely remember seeing issues with fuel-tools enums on Windows. I see that the offending line on CI is this one, which has the DELETE enum.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 21, 2020

@osrf-jenkins run tests please

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 22, 2020

@chapulina, Fixed windows build, there are some tests failing. The tests that I have edited INTEGRATION_factory it's working.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Functionality works, the new tests pass and it looks like there are no regressions 👍

@chapulina chapulina merged commit 1aea1b3 into gazebo11 Sep 22, 2020
@chapulina chapulina deleted the ahcorde/fuel/world branch September 22, 2020 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Gazebo 11 migration Helps with migration from Gazebo classic to Ignition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants