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

Fix frame_id in rgbd_camera #458

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

fmrico
Copy link
Contributor

@fmrico fmrico commented Aug 20, 2024

🦟 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

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
@fmrico fmrico requested a review from iche033 as a code owner August 20, 2024 08:32
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Aug 20, 2024
Copy link
Contributor

@iche033 iche033 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 the fix. I was able to reproduce the problem and confirms that this fixes it for Harmonic when the frame id is specified via <optical_frame_id>.

Just one minor cpplint issue

src/RgbdCameraSensor.cc Outdated Show resolved Hide resolved
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
@ahcorde ahcorde merged commit f2fe577 into gazebosim:gz-sensors8 Aug 21, 2024
9 checks passed
@ahcorde
Copy link
Contributor

ahcorde commented Aug 21, 2024

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

@fmrico fmrico deleted the fix_rgbd_camera_frame branch August 21, 2024 10:01
@sea-bass
Copy link

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 #259

@fmrico
Copy link
Contributor Author

fmrico commented Aug 21, 2024

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:

  • To add a new method getCameraFrame() in RgbdCameraSensor to get the last part of what FrameId() returns.
  • A more general method, maybe in the base class, getSimpleFrame() or similar, to get the same.

Best

@iche033
Copy link
Contributor

iche033 commented Aug 23, 2024

I created a backport PR in #461 that includes the fix from this PR.

#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 #259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants