ros2 adaption

[130s]

trafficstars

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.)

[prasenjitdan]

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

[mikeferguson]

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 https://github.com/ros2/rclcpp/issues/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 https://github.com/ros-perception/image_common/pull/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.

[130s]

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]

@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.

[mikeferguson]

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

[mikeferguson]

Ok - it wasn't so bad to rebase - https://github.com/ros-drivers/openni2_camera/pull/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)

[mikeferguson]

Most of the key stuff is merged - still getting the build to be right in CI (there were some package.xml things wrong)

Remaining work is now in individual tickets:

• #119
• #120
• #121

[mikeferguson]

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

[130s]

Thanks @mikeferguson ! I set the default branch to ros2.

https://github.com/ros-drivers/openni2_camera/pull/122#issuecomment-1441092996

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.

[mikeferguson]

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)

[130s]

I set the default branch to ros2.

I finally did this (I thought I already did, but turned out I didn't).