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

Common export directory for downloaded models #89

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

Common export directory for downloaded models #89

aaronchongth opened this issue Sep 10, 2021 · 5 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@aaronchongth
Copy link
Member

aaronchongth commented Sep 10, 2021

Feature request

Description

Related to open-rmf/rmf_traffic_editor#386.

  • Current split of MAP_NAME and MAP_NAME_ign directories makes install directory of rmf_demos_maps a little cluttered, with duplicated efforts.

  • We should have downloaded models as well as any common generated map yaml files be exported to a common MAP_NAME directory.

  • Both simulations from rmf_demos_gz and rmf_demos_ign should add this common directory to path for simulation needs.

Implementation considerations

@aaronchongth
Copy link
Member Author

aaronchongth commented Sep 13, 2021

@cnboonhan As mentioned during our VC, the issue of duplicated map names are here,

aaron@biryani:~/workspaces/rmf/install/rmf_demos_maps/share/rmf_demos_maps/maps$ ls
airport_terminal      battle_royale      clinic      hotel       hotel_full_ign  office      triple_H
airport_terminal_ign  battle_royale_ign  clinic_ign  hotel_full  hotel_ign       office_ign  triple_H_ign

The only difference seems to be the .world files between airport_terminal/airport_terminal.world and airport_terminal_ign/airport_terminal.world.

It would probably make sense to remove the use of install/rmf_demos_maps/share/rmf_demos_maps/maps/MAP_NAME_ign and consolidate everything into install/rmf_demos_maps/share/rmf_demos_maps/maps/MAP_NAME, with possibly just the world files named differently.

Let me know what you think.

@cnboonhan
Copy link
Contributor

i think thats fine idea 💯

@luca-della-vedova
Copy link
Member

Just some background and further thoughts.
The reason behind the original duplication of the meshes was that originally the sdf file for the building level was different since it had tags specific to ignition to deal with textures, this was fixed a long time ago.

I wonder if we can move this implementation further and have a common directory for all the models in all the maps, like a rmf_demos_maps/models folder. The idea is that if we have a lot of maps that use the same mesh we would still be duplicating the same mesh among different maps, for example on my machine:

luca@luxury:~/ws_avcs$ locate /MainTable/model.sdf
/home/luca/demos_ws/build/rmf_demos_maps/maps/airport_terminal/models/MainTable/model.sdf
/home/luca/demos_ws/build/rmf_demos_maps/maps/clinic/models/MainTable/model.sdf
/home/luca/demos_ws/build/rmf_demos_maps/maps/hotel/models/MainTable/model.sdf
/home/luca/demos_ws/build/rmf_demos_maps/maps/hotel_full/models/MainTable/model.sdf
/home/luca/demos_ws/install/rmf_demos_maps/share/rmf_demos_maps/maps/airport_terminal/models/MainTable/model.sdf
/home/luca/demos_ws/install/rmf_demos_maps/share/rmf_demos_maps/maps/clinic/models/MainTable/model.sdf
/home/luca/demos_ws/install/rmf_demos_maps/share/rmf_demos_maps/maps/hotel/models/MainTable/model.sdf
/home/luca/demos_ws/install/rmf_demos_maps/share/rmf_demos_maps/maps/hotel_full/models/MainTable/model.sdf

This means that we will have four of the same MainTable mesh that will both take a lot of time during build time (we need to download the model four times) plus make the size of the binaries go up quite a lot.
Does this make sense?

@cnboonhan
Copy link
Contributor

yes i would like that too. Since we pull from Fuel, its probably always true there is a one-one correspondence between model name and model mesh ( i believe ). Happy to move with that additional push as well

@aaronchongth
Copy link
Member Author

Yup, that would be the best way to avoid duplicated models across all maps in the whole rmf_demos_maps package. I'll update the description to reflect that.

@cnboonhan cnboonhan self-assigned this Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants