gazebosim / gz-sensors

Provides numerous sensor models designed to generate realistic data from simulation environments.
https://gazebosim.org
Apache License 2.0
63 stars 58 forks source link

Fix frame_id in rgbd_camera #458

Closed fmrico closed 3 months ago

fmrico commented 3 months ago

🦟 Bug fix

Fixes [No number]

Summary

When using rgbd_camera as sensor, frame_id is filled with FrameId() (for example tiago/head_2_link/head_front_camera_frame_sensor), instead of OpticalFrameId() (for example, head_front_camera_rgb_optical_frame). So, the frame_id in the messages is wrong.

This PR fixes it.

I hope it helps. Any comment is welcome :)

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

ahcorde commented 3 months ago

@iche033 do we need to backport this to other distros?

sea-bass commented 3 months ago

Doing a drive-by comment here since I saw the merge notification. I'm not entirely sure if this is a bug or a feature needed to realign images from Gazebo to show up correctly in RViz through TF lookups.

Before going down the backport path, I would validate that this does not break the example used in the original PR https://github.com/gazebosim/gz-sensors/pull/259

fmrico commented 3 months ago

Hi,

Actually, this PR fixes to publish in a valid frame, but this is not the correct frame.

image

If I use FrameId() I get the wrong frame (tiago/head_2_link/head_front_camera_frame_sensor), so the options could be:

Best

iche033 commented 3 months ago

I created a backport PR in https://github.com/gazebosim/gz-sensors/pull/461 that includes the fix from this PR.

https://github.com/gazebosim/gz-sensors/pull/259 fixes the issue for the camera_info topic, while this PR fixes it for the frame_id field in the msg header in other topics. So it should not break the example used in https://github.com/gazebosim/gz-sensors/pull/259