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 component_config.yaml and thruster_config.yaml to be consistent with VRX WAM-V configuration #51

Merged
merged 12 commits into from
Apr 12, 2022

Conversation

Tinker-Twins
Copy link
Contributor

There were a few inconsistencies between the component_config.yaml and thruster_config.yaml of the vrx-docker repository as compared with example_component_config.yaml and example_thruster_config.yaml of the vrx repository.

This PR resolves the aforementioned inconsistencies and adds the pinger component to the component_config.yaml. For details, please refer osrf/vrx#448.

Copy link
Collaborator

@M1chaelM M1chaelM 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 this contribution! I have 1 small request and 1 question, but other than that it all looks good to me.

@@ -5,6 +5,3 @@ engine:
- prefix: "right"
position: "-2.373776 -1.027135 0.318237"
orientation: "0.0 0.0 0.0"
- prefix: "middle"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should keep the "middle" thruster to match the "T" configuration given in wamv_gazebo/urdf/wamv_gazebo.urdf.xacro, since this is the default for the 2022 competition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I have also created PR osrf/vrx#453 by updating the example_thruster_config.yaml of the vrx_gazebo package in order to match the "T" thruster configuration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK thanks. For what it's worth, I think the source of the problem here is probably that the thruster and component config tutorial does not do enough to clarify the difference between generic WAMV configuration while using the VRX Platform, and the configuration requirements for the VRX Competition. Ideally, these should be two separate (though related) things.

So, I may try to rework this tutorial before returning to your new PR osrf/vrx#453.

Copy link
Contributor Author

@Tinker-Twins Tinker-Twins Apr 13, 2022

Choose a reason for hiding this comment

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

@M1chaelM, I cannot clearly understand what is the "problem" in thruster_component_config tutorial with respect to PR osrf/vrx#453. Correct me if I am wrong here, but I have simply added a "middle" thruster to the example_thruster_config.yaml of the vrx_gazebo package in order to match the "T" thruster configuration with the one in this repository. This thruster configuration passes all the compliance checks and can be used for VRX Competition as well as generic USV simulations.

@Tinker-Twins
Copy link
Contributor Author

The requested changes have been made, kindly review them @M1chaelM.

@M1chaelM M1chaelM merged commit 1926287 into osrf:master Apr 12, 2022
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