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 adaption #90

Closed
130s opened this issue Oct 24, 2018 · 11 comments
Closed

ros2 adaption #90

130s opened this issue Oct 24, 2018 · 11 comments
Labels

Comments

@130s
Copy link
Member

130s commented Oct 24, 2018

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
Copy link

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
Copy link
Member

mikeferguson commented Jun 12, 2020

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:
  • 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
Copy link
Member Author

130s commented Feb 22, 2023

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
Copy link

v4hn commented Feb 22, 2023

@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
Copy link
Member

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
Copy link
Member

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)

@mikeferguson
Copy link
Member

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:

@mikeferguson
Copy link
Member

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
Copy link
Member Author

130s commented Feb 23, 2023

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

#122 (comment)

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
Copy link
Member

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
Copy link
Member Author

130s commented Mar 2, 2023

I set the default branch to ros2.

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

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

No branches or pull requests

4 participants