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

Mavros Update to Official ROS2 Port #128

Open
wants to merge 28 commits into
base: dev
Choose a base branch
from
Open

Mavros Update to Official ROS2 Port #128

wants to merge 28 commits into from

Conversation

mhl787156
Copy link
Member

This PR closes #59 by updating mavros to use the official ros2 port (https://github.com/mavlink/mavros/tree/ros2/mavros)

All settings and use should remain the same.

It removes the use of ROS1 mavros through the Ros1 parameter bridge and all of the difficulties due to this.

@mhl787156
Copy link
Member Author

Running into issues with set_mode service, as mirrored by this issue: mavlink/mavros#1588. All of the sys_status plugin services unfortunately hang.

@rob-clarke perhaps a workaround is to directly use CommandLong to pass a set mode request through? Maybe have the mavros container running a few of our nodes too - something which mirrors set_mode , emergency_stop (#127 ) and others

@mhl787156
Copy link
Member Author

@rob-clarke Is this ready to review and merge?

@rob-clarke
Copy link
Contributor

Not quite. It's a start. I think there are still some config bits that don't work properly, especially if multiple drones are involved. It also has some of the test stuff in there from #134 which I'd like to merge before this

@mhl787156 mhl787156 changed the base branch from master to dev November 24, 2022 09:55
@mhl787156 mhl787156 marked this pull request as ready for review December 3, 2022 11:06
@mhl787156
Copy link
Member Author

I believe this is now ready to merge in finally :)

@mhl787156 mhl787156 requested a review from rob-clarke December 3, 2022 11:06
Copy link
Contributor

@rob-clarke rob-clarke left a comment

Choose a reason for hiding this comment

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

Main comments are docs need more updating. Also the install_geographiclib_datasets.sh script should be installed with mavros, so shouldn't need to download separately

system/mavros2/Dockerfile Outdated Show resolved Hide resolved
system/mavros2/README.md Outdated Show resolved Hide resolved
system/mavros2/docker-compose.yml Show resolved Hide resolved
system/mavros2/mavros.launch.xml Show resolved Hide resolved
system/mavros2/README.md Outdated Show resolved Hide resolved
system/mavros2/mavros.launch.xml Show resolved Hide resolved
system/mavros2/mavros_setup.sh Show resolved Hide resolved
@mhl787156 mhl787156 requested a review from rob-clarke December 4, 2022 18:12
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.

Move to upstream MAVROS port
2 participants