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

Update pinger.yaml to comply with acoustic_pinger_tutorial #394

Merged
merged 6 commits into from
Nov 8, 2021

Conversation

Tinker-Twins
Copy link

Updated the pinger.yaml file to include pinger position [65.0, -47.5, -2] in order to comply with the results produced by Visualize sensor measurements in RViz section in acoustic_pinger_tutorial.

@Tinker-Twins Tinker-Twins changed the title Update pinger.yaml Update pinger.yaml to comply with acoustic_pinger_tutorial Nov 7, 2021
Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

Thanks for detecting this inconsistency in the tutorial. How about we create a solution that works for the 2019 world and the current one (2022)?

I suggest to create a new pinger.launch under vrx_2019/launch pointing to a config file under vrx_2019/config/pinger.yaml? Then, you can set the new yaml file with the old values [65.0, -47.5, -2].

We should be able to launch both alternatives with:

roslaunch vrx_gazebo pinger.launch

or

roslaunch vrx_2019 pinger.launch

Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

You also need to install the config directory. I suggest to add this block to vrx_2019/CMakeLists.txt:

# Install all the config files
install(DIRECTORY config/
  DESTINATION ${CATKIN_PACKAGE_SHARE_DESTINATION}/config)

@Tinker-Twins
Copy link
Author

Made the necessary changes and tested locally by cloning the updated repository.

@caguero caguero merged commit f6a5040 into osrf:master Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants