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 invalid camera parameters in gazebo_depthCamera plugin #408

Merged
merged 2 commits into from
Jun 5, 2019

Conversation

jchevrie
Copy link

Fix for issue #407

Fix focalLengthX and focalLengthY values returned by the plugin when asking the intrinsic parameters of the camera through rpc via the command visr get intp.
The new returned values are now pixel/meter ratios instead of meters, to be similar to the interface of the hardware cameras.

@traversaro
Copy link
Member

Thanks @jchevrie . I think we need to first sort out what is the actually intended unit of measurements, as discussed in #407 (comment) .

@traversaro
Copy link
Member

Given the fixes in robotology/yarp#1960, how this PR should be updated (if it needs to be updated at all)?

Jason Chevrie added 2 commits June 4, 2019 15:49
Fix `focalLengthX` and `focalLengthY` values. When asking the parameters through rpc (via the command `visr get intp`), the `focalLengthX` and `focalLengthY` parameters should be expressed in pixel size instead of meters.
Fix typo.
@jchevrie jchevrie changed the base branch from master to devel June 4, 2019 15:27
@jchevrie
Copy link
Author

jchevrie commented Jun 4, 2019

In the end we chose pixels as unit instead of meters, so this fix is still relevant.

I updated the code by adding the new mandatory parameter physFocalLength that was introduced.
However I set the returned value to 0.0 (meaning invalid) because I don't think we set it in gazebo for now (it has a default value 1.0 which should probably not be used as is).

@traversaro
Copy link
Member

Great, thanks a lot @jchevrie . If @Nicogene or @barbalberto could review this PR given their expertise on this interface it would be great, thanks!

Copy link
Collaborator

@barbalberto barbalberto left a comment

Choose a reason for hiding this comment

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

LGTM

@barbalberto barbalberto merged commit 6ea5b58 into robotology:devel Jun 5, 2019
@barbalberto
Copy link
Collaborator

Merged, thanks @jchevrie for your work on this topic

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

3 participants