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

Stop hardcoding absolute paths into downloaded models #77

Closed
chapulina opened this issue Jun 12, 2020 · 4 comments
Closed

Stop hardcoding absolute paths into downloaded models #77

chapulina opened this issue Jun 12, 2020 · 4 comments
Assignees
Labels
🏰 citadel Ignition Citadel enhancement New feature or request migration Helps with migration from Gazebo classic to Ignition

Comments

@chapulina
Copy link
Contributor

When ign-fuel-tools finds a model:// within a file that it's downloading, it replaces that with the full path on disk where the file is being downloaded to. This is done by the LocalCachePrivate::FixPaths* functions.

This is undesirable because it makes those models no longer be relocatable after they've been downloaded.

Moreover, in general, modifying a resource while downloading it makes for poor user experience, because the user is expecting to get the resource as it is on the server. Downloading a resource from the web UI or making direct REST calls to the server doesn't modify resources, and users are using these to circumvent this behaviour (among others) (pit_crew, untouched).

There is a reason for the current behaviour though. Initially Ignition Gazebo didn't support model:// URIs. And even now that it's about to support them as first-class citizens (gazebosim/gz-sim#172), that doesn't map well to models in the Fuel cache, because:

  • The cache directory structure, <model name>/version/model.sdf, doesn't match the <model name>/model.sdf pattern used with the environment variable.
  • Most of the models in the OpenRobotics org have been transferred from gazebo_models, renamed on the process, but the URIs inside the SDF file remained the same. This is an issue with the resources though, not the library.

Possible ways forward

Relative paths

Modify all models in Fuel that are using model:// to use relative paths instead.

Problems with this approach:

  • We do support model:// though (actually any scheme://), so users may upload that and it has to work without overwriting.
  • Relative paths don't support sharing resources across several models.

Fuel cache + model://

Make it possible to find a resource on the cache, such as:

~/.ignition/fuel/fuel.ignitionrobotics.org/OpenRobotics/models/Post\ office/2/meshes/post_office.dae

from

model://Post office/meshes/post_office.dae

The main challenge is, what if multiple owners have models named Post office, which one do we get?

@chapulina chapulina added enhancement New feature or request 📜 blueprint Ignition Blueprint labels Jun 12, 2020
@chapulina chapulina self-assigned this Jun 12, 2020
@chapulina
Copy link
Contributor Author

chapulina commented Jun 24, 2020

The more I think about this, I think that any implementation of Fuel cache + model:// will come with so many caveats that we might as well not recommend it.

So I propose circling back to the proposals on this PR. Essentially:

  1. Strongly ✨ recommend the following inside Fuel models:
    • relative paths for files within a resource (i.e. <uri>meshes/mesh.dae</uri> inside model.sdf)
    • full URLs for files from another resource (i.e. <uri>https://fuel.ignitionrobotics.org/...dae</uri> inside model.sdf)
  2. Strongly ✨ discourage the use of any other URI scheme inside Fuel. And if users do, the model will work, but with the caveat that once downloaded, the model:// is converted to a full path Fuel URI. Ignition does support URIs like model://, but they should only be used for projects installing their own models and setting environment variables, not by models uploaded to Fuel.

Action items:

@chapulina
Copy link
Contributor Author

Closed too quickly 🙂

@chapulina
Copy link
Contributor Author

Tutorial recommending the use of relative paths: gazebosim/gz-sim#400

@chapulina chapulina added 🏰 citadel Ignition Citadel and removed 📜 blueprint Ignition Blueprint labels Dec 16, 2020
@chapulina
Copy link
Contributor Author

Update all models in the OpenRobotics org to use relative paths / full URLs

Follow up on #194

Closing this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel enhancement New feature or request migration Helps with migration from Gazebo classic to Ignition
Projects
None yet
Development

No branches or pull requests

1 participant