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

ros2 camera calibration, tests are strangely failing #376

Closed
wants to merge 11 commits into from
Closed

ros2 camera calibration, tests are strangely failing #376

wants to merge 11 commits into from

Conversation

klintan
Copy link
Contributor

@klintan klintan commented Dec 30, 2018

Started port of camera calibration to ROS2, would love some initial feedback and thoughts on stuff that is still missing (mostly commented out in the code)

  • cv2.imshow() should not be run in a thread as per https://stackoverflow.com/questions/11623566/python-opencv-threading and http://opencv-users.1802565.n2.nabble.com/imshow-and-waitkey-from-within-a-thread-gt-not-updating-the-display-td6918493.html. The discussion that is referenced in the display thread solution implemented for ROS1 Camera calibration segfault when displaying images from thread #85 does not work on Mojave (Apple implemented stricter control on writing to the UI from non-main-threads, can't find a reference link though). Further trying to create the cv2.namedWindow in the main thread and update the imshow in a different one didn't work either, see the first sentence.
    This all meant that I put the spin in a separate thread and the imshow while loop in the main thread. This could perhaps be a custom executor or something more fancy, would love some feedback on this new approach if it needs major changes.

  • Tests are failing on non-ROS2-related stuff (calibration errors) which is quite serious, but the only way I can see it being related is because its now running on python 3.7 (instead of 2.x). Will try to look into it, but any pointers here are of interest as well. I will look at the CV2 calibration implementation and see if I can see any differences since I guess its based on that code.

  • The service check is not implemented yet because it uses some functions I was uncertain about, such as the rospy.remap_name() and rospy.resolve_name().

  • Did some restructuring of the projects as to the recommendations of ros2 py-packages. But would love feedback on all of this as well.

Just wanted to start this PR as a heads up as well as to get some early feedback since I new to Ros2 (and only have limited experience in ROS1 as well)

mjcarroll and others added 8 commits October 17, 2018 22:20
* Port depth_image_proc on ROS2

* Port depth_image_proc of image_pipeline on ROS2

* rename Nodelets as Node

* add launch examples, such as "ros2 launch depth_image_proc point_cloud_xyzi.launch.py"

* verified point_cloud_xyzrgb, point_cloud_xyz, convert_metric based on Realsense camera(https://github.com/intel/ros2_intel_realsense).

Signed-off-by: Chris Ye <chris.ye@intel.com>

* add ament_lint_auto test and adjust code style

Signed-off-by: Chris Ye <chris.ye@intel.com>

* update test example for depth_image_proc

- rename raw topic as image_transport fixed the issue (ros-perception/image_common#96)
- update test example of point_cloud_xyzrgb.launch

Signed-off-by: Chris Ye <chris.ye@intel.com>

* remove unused dependence in cmakelist

* remove boost which is unused on ROS2

* remove cv_bridge version check logic as setted in find_package

* update maintainer in package.xml

Signed-off-by: Chris Ye <chris.ye@intel.com>

* added all example launchers for demo test

pass test:
 ros2 launch depth_image_proc point_cloud_xyzrgb.launch.py
 ros2 launch depth_image_proc point_cloud_xyz.launch.py
 ros2 launch depth_image_proc convert_metric.launch.py
 ros2 launch depth_image_proc crop_foremost.launch.py
 ros2 launch depth_image_proc point_cloud_xyz_radial.launch.py
 ros2 launch depth_image_proc disparity.launch.py
 ros2 launch depth_image_proc register.launch.py
 ros2 launch depth_image_proc point_cloud_xyzi.launch.py
 ros2 launch depth_image_proc point_cloud_xyzi_radial.launch.py

Signed-off-by: Chris Ye <chris.ye@intel.com>

* @wip update to use raw pointers.

* continue to update to use raw pointer

Signed-off-by: Chris Ye <chris.ye@intel.com>
this may be remarked while code debugging, should be enabled to build node plugin file
and added points remap in point_cloud_xyzrgb.launch.py
* port image_publisher on ROS2
* switch to use cmake 3.5
* change nodelet to classloader
* change ros::param to ros2 parameter APIs
* use ros2 code style
* enable ros2 camera_info_manager
@yechun1 yechun1 mentioned this pull request Jan 2, 2019
7 tasks
@klintan
Copy link
Contributor Author

klintan commented Feb 6, 2019

  • The service check is not implemented yet because it uses some functions I was uncertain about, such as the rospy.remap_name() and rospy.resolve_name().

Is done, but ugly and non-ROS2 solution. As stated earlier, if someone could take a look at that, that would be awesome. Will see why the calibration tests are failing, I could be having the incorrect test files.
Would be awesome to try to merge this into the ROS2 branch eventually so that another package in the image_pipeline is available.

@klintan
Copy link
Contributor Author

klintan commented Feb 9, 2019

Updated some missed things from ahuizxc@5e2b4ef

@yechun1
Copy link
Contributor

yechun1 commented Feb 26, 2019

@klintan, may I know how do you use stereo images as input resources on ROS2? Have you enabled ucv_camera to publish stereo raw image (left and right) or is there any solution to provide images for stereo calibration tests?

@klintan
Copy link
Contributor Author

klintan commented Feb 27, 2019

@klintan, may I know how do you use stereo images as input resources on ROS2? Have you enabled ucv_camera to publish stereo raw image (left and right) or is there any solution to provide images for stereo calibration tests?

Unfortunately I have not tried this yet :/ but it's on my roadmap to buy a simple stereocamera and try out the calibration and everything that comes with it as well.

For the tests I used the old ROS1 data, at least I think its that one, its available here http://download.ros.org/data/camera_calibration/camera_calibration.tar.gz

@yechun1
Copy link
Contributor

yechun1 commented Feb 28, 2019

Would you please share me the steps how you run camera_calibration tests with those [left/right]*.pgm files?

@klintan
Copy link
Contributor Author

klintan commented Apr 9, 2019

Would you please share me the steps how you run camera_calibration tests with those [left/right]*.pgm files?

Sorry missed this, will run it through and get back to you.

@yechun1
Copy link
Contributor

yechun1 commented Apr 9, 2019

I have found another solution to publish left/right camera by image_publisher, with two USB camera devices and run two image_publisher node.
here is #386 for reference.

launch stereo camera (/dev/video0 and /dev/video1)
 ros2 launch image_publisher image_publisher_stereo.launch.py

@clydemcqueen
Copy link

It appears that the fork is gone, is this PR dead? Is there an active ros2 port for camera_calibration? Thanks.

@JWhitleyWork
Copy link
Collaborator

@clydemcqueen - There is a ros2 port that is in-progress. It's on the ros2 branch of this repo.

@klintan
Copy link
Contributor Author

klintan commented Jul 10, 2019

Sorry I removed it because I wanted to do some other work. I'll create a branch on my new fork and add the changes from this. I don't think anything is done on the image calibration on the ROS2 branch just yet.

@clydemcqueen
Copy link

I am happy to help review & test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants