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

Deprecate option to use http request for model downloading #388

Open
aaronchongth opened this issue Sep 10, 2021 · 3 comments
Open

Deprecate option to use http request for model downloading #388

aaronchongth opened this issue Sep 10, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@aaronchongth
Copy link
Member

aaronchongth commented Sep 10, 2021

Deprecation

Related to #386.

  • Remove the option to use http requests.
  • Use ign-fuel-tools moving forwards and support upstream development if we need any new features.
@aaronchongth aaronchongth added enhancement New feature or request help wanted Extra attention is needed labels Sep 10, 2021
@cnboonhan
Copy link
Contributor

cnboonhan commented Sep 13, 2021

happy to work on this, but I would like to get a clear idea of the ideal result. From here:

A real fix would be to throw all the models, including the building and floor generated by rmf_building_map_tools into a common folder, to also remove the current mesh duplication.

I am guessing, would the model download process be something like

  • download from fuel to ~/.ignition, ignoring duplicates
  • export models to dedicated install folder, perhaps install/rmf_fuel_models
  • Link all launch files to this folder

Some thoughts I had:

  • Ignition seems to halt if some models are missing. This might not be ideal for CI, which might want to omit some models for the sake of efficiency. Is there a way to let ignition continue running with some missing models?
  • Would there be value in improving the old models we have to remove whitespace, etc? @aaronchongth mentioned some stuff about some standards to enforce on fuel models, which we could spend some time to tidy up if ideal.

@aaronchongth
Copy link
Member Author

As per VC, this issue will only be related to refactoring and deprecating the option to download models using the http request feature.

Regarding, the discussion about exporting models, I have created a separate issue open-rmf/rmf_demos#89.

As for reducing the number of workarounds we apply during model downloading, e.g. handling white space, changing names, etc, as you surmised, the ideal solution would be to improve the older models on Fuel, however I would expect plenty of users are using them as it is, and any major changes would break lots of simulation behaviors across the board. We should definitely revisit this once we get a better understanding on our simulation model needs some time soon. Thanks!

@aaronchongth aaronchongth removed the help wanted Extra attention is needed label Sep 13, 2021
@cnboonhan
Copy link
Contributor

yes thanks for clarifying! will dive in 🤽

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

No branches or pull requests

2 participants