-
Notifications
You must be signed in to change notification settings - Fork 132
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 camera coord frames for all cameras and all robots for hw and sim #720
fix camera coord frames for all cameras and all robots for hw and sim #720
Conversation
73e8655
to
ed8f911
Compare
in 51439d9 I added a |
ToDo:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on cob4-12: OK
I've opened an issue for the wrong transformations in the sick_3dcs PointCloud here: https://github.com/SICKAG/sick_visionary_t/issues/19 |
<arg name="camera_name" value="$(arg name)"/> | ||
<arg name="pointcloud_in" value="/$(arg name)/depth/points"/> | ||
<arg name="pointcloud_out" value="/$(arg name)_upright/depth/points"/> | ||
<!-- Sick3DCS does not have color image, and image flip cannot process images that are not CV_8UC3 --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ipa-rmb FYI
I don't know whether this is intended...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not care about that camera at the moment. Image flip would need to be extended to handle other types of images as well, the current code fpr 8UC3 images is speed optimized and not general. We can later add a slower general fall back solution for other image types if anyone needs that at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new issue for this: ipa320/cob_perception_common#85
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except for the flip_color_image: false
for cob4-10, I'm OK with this PR.
@@ -12,7 +12,7 @@ reference_frame: base_link | |||
|
|||
# flip color image | |||
# bool | |||
flip_color_image: true | |||
flip_color_image: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sick_3dcs does not have a color_image...@ipa-rmb @ipa-josh
and image_flip complained about not being able to flip the depth/image topic (see #720 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also fine with us, see comment above.
@ipa-rmb @ipa-fmw |
Yes, because the pixel matrix should be the same for the color image and the point cloud. If I move pixels on the color image to a different location, then I would expect to find the corresponding XYZ pixel at the same pixel coordiates that I used in the color image. That is why the point cloud needs to be re-arranged as well. |
Transforms and images for cob4-10 look fine. |
Can/shall I merge? |
As I opened issues for all remaining issues, I'll merge all related PRs for now... |
fixes #669
fixes ipa320/cob4#553
requires https://github.com/ipa320/cob_common/pull/233
includes #715
@ipa-nhg @cagbal @ipa-fmw @ipa-rmb FYI