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

move tf to ros #698

Merged
merged 18 commits into from
Jul 26, 2023
Merged

move tf to ros #698

merged 18 commits into from
Jul 26, 2023

Conversation

j-herman
Copy link
Collaborator

This PR is an alternative to PR 664 which also resolves many of the TF issues in 681. It changes the way we publish TF information on the ROS side by:

  • adding a JointStatePublisher plugin to the wamv and bridging the topic to ROS to make dynamic joint states available on the ROS side
  • modifying the PosePublisher plugin to only publish sensor transforms, as those are only available via the sdf and will not be published when parsing the urdf
  • replacing most of the functionality in the custom pose_tf_broadcaster node with a robot_state_publisher node, which provides the robot description and tf tree in a format that can be ingested by ROS applications such as RViz

A launch file to start an instance of RViz with a custom config file is included for testing and for the tutorial.

UNKNOWN, help needed: Is any other process on the Gazebo side using the transforms from the sdf that PosePublisher was producing previously? I think not, as we have access to the truth data, but if so this PR will likely break something and needs to be reworked.

To test:

ros2 launch vrx_gz competition.launch.py world:=stationkeeping_task
ros2 launch vrx_gazebo rviz.launch.py

Confirm the wamv appears with all components drawn and in the right place.

ros2 topic pub /wamv/thrusters/left/pos std_msgs/msg/Float64 "{data: -1}"

Watch the left engine/propeller move in RViz as it moves in Gazebo.

ros2 run tf2_tools view_frames

Confirm the new TF tree is structured correctly.

I still haven't looked into the extra "wamv/" prepended on the PosePublisher's output, so this code adds the prefix to all ROS-generated link names instead, in order to make the sensor sdf links connect correctly. This is minor but annoying, so if we decide to implement this I will take another look at it.

@j-herman
Copy link
Collaborator Author

@M1chaelM @caguero
This is ready for a look, but we'll need to discuss. I do think it fixes the tf issues but I'm not sure if/when we want to make a significant change. Marked both this and the other option (tutorial hack only) as draft pending decisions.

@j-herman j-herman changed the title Jessica/move tf to ros move tf to ros Jun 30, 2023
@j-herman j-herman marked this pull request as ready for review July 11, 2023 17:45
@j-herman j-herman requested a review from caguero July 11, 2023 17:45
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.

Everything works for me as expected, I only committed a very minor tweak.

My only suggestion is about the rViz configuration: should we include the three cameras and the lidar?

@j-herman
Copy link
Collaborator Author

I don't think the extra cameras and lidar are necessary - it's super easy to add them, and one topic is shown as an example - but there's no reason not to have them all if you think that would look better for the demo. Whichever you prefer works for me!

@j-herman j-herman merged commit 76690b6 into main Jul 26, 2023
1 check passed
@j-herman j-herman deleted the jessica/move_tf_to_ros branch July 26, 2023 17:24
@j-herman j-herman mentioned this pull request Jul 26, 2023
This pull request was closed.
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