PoseWithCovarianceStamped transform doesn't transform covariance part

[ksuszka]

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04, but it doesn't really matter
• Installation type:
  • source code analysis
• Version or commit hash:
  • a4f152f
• DDS implementation:
  • N/A
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

Expected behavior

doTransform on PoseWithCovarianceStamped should also transform covariance.

Actual behavior

doTransform on PoseWithCovarianceStamped copies covariance without transforming it.

Additional information

The culprit seems to be line

geometry2/tf2_geometry_msgs/include/tf2_geometry_msgs/tf2_geometry_msgs.h

Line 309 in d65b7eb

t_out.pose.covariance = t_in.pose.covariance;

It seems that this functionality is implemented in ROS1 version https://github.com/ros/geometry2/blob/c73b5939723db078c9bbe18523230ad54f859682/tf2_geometry_msgs/include/tf2_geometry_msgs/tf2_geometry_msgs.h#L927

[clalancette]

Yep, agreed. A pull request to port that functionality over from the ROS 1 version would be welcome.

[aprotyas]

On that note, do_transform_pose_with_covariance_stamped in tf2_geometry_msgs.py does not transform the covariance either. Same pattern.

geometry2/tf2_geometry_msgs/src/tf2_geometry_msgs/tf2_geometry_msgs.py

Line 143 in 6fa2001

res.pose.covariance = pose.pose.covariance

Since I'm working on this issue, should I fold above changes into the same PR? @clalancette

[clalancette]

Since I'm working on this issue, should I fold above changes into the same PR? @clalancette

It's worthwhile to fix it both places, but I'll suggest two separate PRs, one for C++ and one for Python.

[msmcconnell]

@clalancette Is there any chance the fix for this in #430 gets backported to Foxy?

[clalancette]

@clalancette Is there any chance the fix for this in #430 gets backported to Foxy?

I think that is viable; it doesn't change API or ABI. Mind opening a backport PR?

[aprotyas]

@clalancette Is there any chance the fix for this in #430 gets backported to Foxy?

I think that is viable; it doesn't change API or ABI. Mind opening a backport PR?

I've opened #488 and #489 through mergify.

[msmcconnell]

Thanks for putting those up @aprotyas ! Hope they can make it in soon.

[aprotyas]

FYI: both backports have landed.

[msmcconnell]

@aprotyas Awesome thanks for doing that!