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

Update Camera Intrinsics in camera_info topic #256

Closed
wants to merge 9 commits into from

Conversation

deepanshubansal01
Copy link
Contributor

@deepanshubansal01 deepanshubansal01 commented Aug 8, 2022

🦟 Bug fix

Fixes gazebosim/gz-sim#705

Summary

This PR solves the camera intrinsics not being reflected properly in /camera_info topic when the <intrinsics> tag is not provided. The PR uses the CameraIntrinsicMatrix() from gz-rendering package to get the camera intrinsics and update the sdf DOM object and /camera_info topic.

Depends on

gazebosim/gz-rendering#700

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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Aug 8, 2022
@deepanshubansal01 deepanshubansal01 changed the base branch from ign-sensors6 to main August 8, 2022 15:36
@deepanshubansal01 deepanshubansal01 self-assigned this Aug 8, 2022
@deepanshubansal01 deepanshubansal01 changed the title Deepanshu/optical center Update Camera Intrinsics in camera_info topic Aug 8, 2022
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
src/CameraSensor.cc Outdated Show resolved Hide resolved
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
test/integration/camera.cc Outdated Show resolved Hide resolved
test/integration/camera.cc Outdated Show resolved Hide resolved
test/integration/camera.cc Outdated Show resolved Hide resolved
test/integration/camera.cc Outdated Show resolved Hide resolved
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, I'll leave it up to @iche033 for final review.

test/integration/camera.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the analogous PR in gazebo classic, we have 3 instances of the camera : One with no intrinsics tag (which uses gazebo defaults), one with intrinsics with the same values as gazebo defaults, and one which has some custom values. It would be nice to keep the tests consistent here as well, though not a necessity.

adityapande-1995 added a commit that referenced this pull request Sep 2, 2022
Signed-off-by: Aditya <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor

Closing this in favour of #265 with the merge conflicts and signoff errors resolved.

@adityapande-1995 adityapande-1995 deleted the deepanshu/optical-center branch September 2, 2022 19:54
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.

CameraInfo's projection matrix not updating when resolution changes
3 participants