ros2 adaption
130s opened this issue · 11 comments
Continued from a discussion.
For those who are interested in making this happen, please voice up here.
(As a maintainer personally, I'm not planning to spend any of my time on this in the forseeable future.)
Thanks, Isaac for creating this post.
All,
Please let me know if any of you have any concern for my team to work on openni2_launch porting to ROS2.
Thanks,
Dan
For anyone looking at this - I've got a port of openni2_camera started here: https://github.com/mikeferguson/openni2_camera/tree/ros2
Status:
Split services IDL into openni2_camera_msgs package. I may revert this now that I've found out how to properly specify dependencies on messages in the same package.This has been reverted- The nodelet has been refactored into an rclcpp component called "openni2_wrapper::OpenNI2Driver".
- There are no subscriber connect/disconnect callbacks yet in ROS2. This means that lazy publishers can't really be implemented properly. I've done a hacky version by:
- modifying the monitor_connection timer to always run (used to only run if we were using reconnect logic, now the callback checks the reconnect logic flag).
- within the monitor_connection callback we check the subscriber counts and decide whether to start the camera (so it's a lazy publisher that takes ~1s to start up after a connection).
Due to ros2/rclcpp#1174 this logic FAILS if you remap the topic. So for now, don't remap things-- remapping is now working by changing how we get the subscriber count - requires ros-perception/image_common#163 to be merged.
- Also due to lack of subscriber connect callbacks, most (all?) of the components in depth_image_proc and image_proc do not implement lazy pub/sub. This means we cannot setup a large graph like rgbd_launch/openni2_launch did - since you will end up with active subscribers to all camera topics in the openni2_camera driver and that violates the USB bandwidth.
- For now, I recommend creating a launch file for your specific setup desired. An example that creates rectified color/depth and publishes an XYZRGB point cloud can be found in the ubr_reloaded repo
This has been tested with a Primesense Carmine 1.09 and an Asus Xtion on Ubuntu 20.04 and ROS Foxy. I'm planning to resolve the remap problem before opening a PR to incorporate things here.
I'm getting interested (finally) in getting openni2 to work on ros2. Just pushed ros2 branched off of ros1 branch (so no work has been added regarding ros2 yet).
@mikeferguson Is your branch ready to be merged into main repo (i.e. here), or would you keep your fork (if so why)?
@v4hn I saw in ros-o a patch https://github.com/ros-o/openni2_camera/commits/obese-devel C++17 patches. Is that meant to be generic I'd be happy to merge in here.
@v4hn I saw in ros-o a patch https://github.com/ros-o/openni2_camera/commits/obese-devel C++17 patches. Is that meant to be generic I'd be happy to merge in here.
Yes, to the best of my knowledge they should be compatible with C++11 (they mostly remove annotations nobody relied on). That said I did not test them on anything pre-c++17 yet. Feel free to cherry-pick/merge them from the ros-o branch,
I did not get around yet to file a PR.
My branch could certainly come over here - I was mainly waiting to be "feature complete" - which is currently blocked on lazy subscribers (which hopefully are coming in Iron turtle). That said, it looks like I'm going to have to manually merge a bunch of stuff since it was forked off like 2+ years ago - so it will take a bit of time
Ok - it wasn't so bad to rebase - #115 still needs testing, but is open for comment/review. Changes from my earlier ros2 branch:
- Reverted the messages to be part of the openni2_camera package rather than adding a new openni2_camera_msg package
- Reverted the boost removal for now - we can open that as a separate PR so that @christianrauch gets proper attribution
I'm not actually sure how to switch the CI over to ROS2 - maybe @130s you can open a PR for that and then I can rebase again (It seems like it uses the default branch config rather than what is in the PR)
CI is working - I think we've got as much done as we can for now - really waiting on lazy subscribers and image_proc/depth_proc stuff to complete openni2_launch
Thanks @mikeferguson ! I set the default branch to ros2.
I'll need to create a new ros2-gbp in the shared org so that we can release to rolling
I have never made a bloom release yet. Does bloom want a new GBP repo for ROS2 release?
Looks like it does according to What if my existing release repo isn’t on ros2-gbp? (docs.ros.org), and I don't have a right to create a new repo:
If you are porting a ROS 1 package to ROS 2 and planning on releasing your packages into ROS 2 for the first time, follow standard procedure to request for a new release repository for your ROS 2 releases.
It's not really that bloom requires it - it's the other automation around rolling releases - we want to create a GBP that the upstream ROS2 team has access to, so that they can bloom the first releases from rolling into the next ROS2 release (basically, in April/May there will be a day where they create the first Iron releases for everything in Rolling - that way they don't have to wait for every maintainer to do it in dependency order)
I've created several teams/gbps for ros2 - it's fully automated, I'll tag you on the PR when I create it (and add you as a team member so you'll have commit rights once the repo is created)
I set the default branch to ros2.
I finally did this (I thought I already did, but turned out I didn't).