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 bug in single_image_client #103

Merged
merged 2 commits into from
Aug 17, 2021
Merged

fix bug in single_image_client #103

merged 2 commits into from
Aug 17, 2021

Conversation

lianghongzhuo
Copy link
Contributor

This fixes the bug in the single image client.
The camera_info was using K matrix to convert the camera_info msg.
However, it uses P matrix in the upstream code, see: https://github.com/ros-perception/vision_opencv/blob/noetic/image_geometry/src/pinhole_camera_model.cpp#L322-L327
By convert the K matrix to the P matrix, the apritag_ros can finally detect the tag pose. Otherwise, it will return a [0, 0, 0] pose instead.

@wxmerkt
Copy link
Collaborator

wxmerkt commented Aug 17, 2021

Thank you very much for the fix. While the fix is technically correct - image_geometry also returns from P for obtaining fx() etc. (cf. image_geometry/pinhole_camera_model.h#L311-L316) - the source of the issue is that both K and P should be filled in in the client. Particularly, the K[0]!=0.0 is used as a signal whether calibration parameters exist. Could you please also amend the client to fill in both K and P?

@lianghongzhuo
Copy link
Contributor Author

Sure, I added the K matrix back.
But curious to know where is the use of the K matrix though?

@wxmerkt
Copy link
Collaborator

wxmerkt commented Aug 17, 2021

Thank you! On a quick scan of image_geometry, we aren't using it. Based on the definition of the CameraInfo message, the presence/value of K[0] can be used to distinguish whether the camera is calibrated or not (https://github.com/ros/common_msgs/blob/20a833b56f9d7fd39655b8491a2ec1226d2639b3/sensor_msgs/msg/CameraInfo.msg#L22-L23)

@wxmerkt wxmerkt merged commit 3a8be33 into AprilRobotics:master Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants