-
Notifications
You must be signed in to change notification settings - Fork 205
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 example_component_config.yaml and numeric.yaml for generating compliant WAM-V URDFs #448
Conversation
@caguero Note that our example team at https://github.com/osrf/vrx-docker/tree/master/team_config/example_team is also missing a pinger, so we may way to update that also. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me! Thanks for the contribution! Just one more minor request. As @M1chaelM pointed out, could you add the pinger to the components file of our team example?
https://github.com/osrf/vrx-docker/tree/master/team_config/example_team
Sure, no problem! |
Created osrf/vrx-docker#51 by adding the pinger component to the Besides this, we found that there were a few inconsistencies between the |
Thanks for this contribution. Since this is approved I'm going to go ahead and merge. |
@M1chaelM, PR osrf/vrx-docker#51 is still pending. Kindly review and merge it so that there wouldn't be any inconsistency between the two repositories. |
The current
example_component_config.yaml
lacks two things:1. Functional aspect: It does not include the
wamv_pinger
component! If a custom WAM-V URDF is generated without this component, it still passes the compliance checks, but the sensor is not instantiated. When the Gymkhana task is launched, no data is published to/wamv/sensors/pingers/pinger/range_bearing
topic. Consequently, it is impossible to localize the black box, and hence complete the task.2. Readability: The example config sets a standard in front of developers, and hence it is common to assume that it is self-contained to its fullest abilities. However, the current version does not include all the allowable parameters for most of the components.
The current
numeric.yaml
lacks two things:1. Functional aspect:
wamv_gps
andwamv_imu
do nat have the parameterpost_Y
.2. Readability: Sequence of the components does not match that in the
example_component_config.yaml
file.Changes:
The said files have been updated to address the concerns raised above. Particularly, the
example_component_config.yaml
file now contains defination forwamv_pinger
component, and includes all the allowable parameters for all the components. Thenumeric.yaml
file is rearranged to match component sequence as in theexample_component_config.yaml
file, and unnecessary parameters have been removed.