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

Backport fix for optical_frame_id in Depth and RGBD camera sensors #461

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Aug 23, 2024

🦟 Bug fix

Summary

Port <optical_frame_id> fixes from #458 and #362 for DepthCamera and RGBD camera sensors.

To test:

Run with this exampel optical_frame_id.sdf world and check the frame_id field in various topics.

ign gazebo -v 4 -s -r optical_frame_id.sdf
# check rgbd camera topics: the `frame_id` value should be `rgbd_optical_frame_id`
ign topic -e -t /rgbd/camera_info  | grep -A 2 frame_id
ign topic -e -t /rgbd/image  | grep -A 2 frame_id
ign topic -e -t /rgbd/depth_image  | grep -A 2 frame_id
ign topic -e -t /rgbd/points  | grep -A 2 frame_id

# check depth camera topics: the `frame_id` value should be `depth_optical_frame_id`
ign topic -e -t /camera_info  | grep -A 2 frame_id
ign topic -e -t /depth/points | grep -A 2 frame_id
ign topic -e -t /depth | grep -A 2 frame_id

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: Ian Chen <ichen@openrobotics.org>
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Aug 23, 2024
@iche033 iche033 mentioned this pull request Aug 23, 2024
8 tasks
@iche033 iche033 changed the title Fix optical_frame_id in Depth and RGBD camera sensors Backport fix for optical_frame_id in Depth and RGBD camera sensors Aug 23, 2024
@fmrico
Copy link
Contributor

fmrico commented Aug 23, 2024

Hi @iche033

As I commented in #458 (comment), the correct frame for this message is actually the last part of frameId(). Otherwise, you'll have the point cloud turned as shown in the above link.

@iche033
Copy link
Contributor Author

iche033 commented Aug 23, 2024

Hi @iche033

As I commented in #458 (comment), the correct frame for this message is actually the last part of frameId(). Otherwise, you'll have the point cloud turned as shown in the above link.

Not sure if I understand you correctly. In you example in #458 (comment), FrameId returns: tiago/head_2_link/head_front_camera_frame_sensor so when you say the last part of FrameId(), do you expect it to return head_front_camera_frame_sensor?

@fmrico
Copy link
Contributor

fmrico commented Aug 26, 2024

Hi @iche033

Not sure if I understand you correctly. In you example in #458 (comment), FrameId returns: tiago/head_2_link/head_front_camera_frame_sensor so when you say the last part of FrameId(), do you expect it to return head_front_camera_frame_sensor?

Yes, that is what I expected, but I changed nothing in FrameId() because I didn't know if that was an agreement in Gazebo. I preferred to fix in #458 to a valid frame to use the bridge to ROS, in which that frame was definitively wrong.

@iche033
Copy link
Contributor Author

iche033 commented Oct 4, 2024

merging this as it fixes the use of <optical_frame_id> and so that the behavior is consistent with newer versions. We can tackle other issues separately.

@iche033 iche033 merged commit 3081ba0 into ign-sensors6 Oct 4, 2024
11 checks passed
@iche033 iche033 deleted the optical_frame_id branch October 4, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants