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

[WIP] Migration to Ignition #9

Open
wants to merge 4 commits into
base: gazebo-ng
Choose a base branch
from
Open

Conversation

gonzodepedro
Copy link
Collaborator

Signed-off-by: Gonzalo de Pedro [email protected]

Signed-off-by: Gonzalo de Pedro <[email protected]>
@gonzodepedro gonzodepedro self-assigned this Mar 30, 2022
@gonzodepedro gonzodepedro changed the title Migration to Ignition [WIP] Migration to Ignition Mar 30, 2022
Signed-off-by: Gonzalo de Pedro <[email protected]>
@jacobperron jacobperron changed the base branch from master to gazebo-ng April 20, 2022 18:06
Copy link
Owner

@jacobperron jacobperron 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 updates! I've left a few comments and questions below.

I've also re-targeted this to a new branch where we can add changes to support the next generation of Gazebo (versus Gazebo classic).

<topic>cmd_vel</topic>
<odom_topic>odom</odom_topic>
</plugin>
<!-- <plugin filename="libgazebo_ros_diff_drive.so" name="differential_drive_controller">
Copy link
Owner

Choose a reason for hiding this comment

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

I guess we can remove this commented plugin since we're replacing it will ignition-gazebo-diff-drive-system.

exec="parameter_bridge"
name="$(anon ros_ign_bridge)"
output="screen"
args="/model/vehicle/odometry@nav_msgs/[email protected] /cmd_vel@geometry_msgs/[email protected] ">
Copy link
Owner

Choose a reason for hiding this comment

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

@adityapande-1995 added support for OdometryWithCovariance and TwistWithCovariance in gazebosim/gz-msgs#224. I think those are the equivalent with the gazebo_ros_pkgs plugins, so maybe we should bridge those here instead?

Choose a reason for hiding this comment

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

ros_ign bridge doesn't support that yet, the PR is approved but is blocked on CI : gazebosim/ros_gz#222

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, thanks. Perhaps we can leave a TODO here to come back and update this once it is available.

Choose a reason for hiding this comment

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

Just an update, that PR has been merged here : gazebosim/ros_gz#222

</plugin>
<plugin filename="libgazebo_ros_p3d.so" name="p3d_base_controller">
</plugin> -->
<!-- <plugin filename="libgazebo_ros_p3d.so" name="p3d_base_controller">
Copy link
Owner

Choose a reason for hiding this comment

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

Can we just remove this plugin? Or is there something that's missing from Gazebo that we need here?

@@ -195,25 +208,24 @@
<stddev>0.007</stddev>
</noise>
</camera>
<plugin name="camera_controller" filename="libgazebo_ros_camera.so">
<ros>
<!-- <plugin name="camera_controller" filename="libgazebo_ros_camera.so"> -->
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can remove this and bridge the relevant camera topics from Gazebo. Maybe @WilliamLewww can provide pointers for anything we should add to this file and/or topics that can be bridged.

Signed-off-by: Gonzalo de Pedro <[email protected]>
config/robot.urdf Outdated Show resolved Hide resolved
config/robot.urdf Outdated Show resolved Hide resolved
config/robot.urdf Outdated Show resolved Hide resolved
Signed-off-by: Gonzalo de Pedro <[email protected]>
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.

3 participants