grassjelly
commented
3 years ago Hey @PranshuTople . Thanks for the PR and great video tutorial.
I can't approve this PR with the Gazebo models being merged. I think this can be solved in a less intrusive manner and without causing any duplicated models in the user's machine by explicitly asking the users to download the models. Something similar to this: https://gist.github.com/awesomebytes/982166e825aa23ecdaf9acf34fa0a330 . All the models used in CHAMP's default Gazebo world can be found in the installed Gazebo folder, so this should work once downloaded.
Moreover, the issue you've mentioned might also be related to this https://github.com/ros/robot_state_publisher/pull/139 on Noetic since the bringup.launch uses tf_prefix to support spawning multiple quadruped robots and run applications healthily using namespaces.
However, your PR also contains fixes https://github.com/chvmp/champ/pull/61/files#diff-444377d83df7b0a3c941362ed8f41e2e405307f98f4aca463eb3ed9bb1030180 that I'd be happy to merge.
My suggestion is to create a new PR:
- Keeping the CMakeLists fixes
- Add a note on the README how to explicitly download the models shall this problem is encountered.
Thank you! :)
PranshuTople
@grassjelly I have seen multiple people facing this problem where the simulation was not loading whenever they use to launch it while online as gazebo was not able to download all the models and hence the Gazebo used to stuck on Black Screen. And when they used to launch the simulation offline, the simulation use to load without any models in it. So I fixed that problem by adding the models in the gazebo package of champ so everyone don't need to download those model individually. Also I have updated the readme by adding a link to video tutorial to use this package on Youtube along with proper timestamps so they can refer to parts which are necessary for them.
Let me know if you have any more suggestions!