ROS2 port - Githubissues

clydemcqueen commented 3 years ago

This is a first pass. I briefly tested this in Eloquent.

It does not compile in Dashing, rclcpp stream logging was added in Eloquent.

I'd like to do the following:

remove support for gstreamer-0.10 -- this hasn't been supported since 2012
run ament linters (ament_lint_common) -- however, this will cause many formatting changes, so we can make this a separate PR

Feedback welcome!

clydemcqueen commented 3 years ago

Additional TODOs:

remove scripts/gscam_node.in
remove nodelet_plugins.xml
remove src/gscam_nodelet.cpp
remove include /gscam/gscam_nodelet.h
port examples/v4l.launch
port examples/gscam_nodelet.launch
port examples/nodelet_pipeline.launch

clydemcqueen commented 2 years ago

The port is complete.

There is CI using Github Actions. It builds on Foxy, Galactic and Rolling.
I ported all of the launch files, but was only able to test the v4l and component launches.

Please take a look.

wep21 commented 2 years ago

@clydemcqueen I created a small fix toward your repository. Could you review this? https://github.com/clydemcqueen/gscam/pull/3

wep21 commented 2 years ago

@k-okada friendly ping

wep21 commented 2 years ago

@k-okada friendly ping

wep21 commented 2 years ago

@jbohren Could you review and merge this?

jbohren commented 2 years ago

@clydemcqueen This looks great, and a continuation of the tradition of completely different people pulling gscam forward.

I just tested it with v4l. I did make a slight mod here to address some deprecation warnings, so you should pull that into your fork: https://github.com/clydemcqueen/gscam/pull/5

If you just add a note in the README regarding which sources / examples you tested, this looks good to merge, but someone else will have to manage releasing it.

wep21 commented 2 years ago

@nuclearsandwich Could you create gscam-release in https://github.com/ros2-gbp? @jbohren Will you manage releasing? If there is no one to manage releasing, can I manage releasing?

jbohren commented 2 years ago

@wep21 I don't have the bandwidth to release it, so I'm happy if you want to do it.

clydemcqueen commented 2 years ago

@jbohren Thank you for jumping in!

The latest set of changes is on https://github.com/clydemcqueen/gscam/pull/6, so when that lands I would love to see this released. I have never done that, so I am happy to leave this to @wep21 or others, and I will learn.

clydemcqueen commented 2 years ago

@jbohren @wep21 All changes are in. Thanks.

nuclearsandwich commented 2 years ago

@nuclearsandwich Could you create gscam-release in https://github.com/ros2-gbp? @jbohren Will you manage releasing? If there is no one to manage releasing, can I manage releasing?

https://github.com/ros2-gbp/gscam-release has been created and both @wep21 and @jbohren have been invited to the release team for it.

wep21 commented 2 years ago

@jbohren Could you review additional changes and merge this PR if there is no issue to fix?

wep21 commented 2 years ago

@jbohren Do you have any resource to check this? If you don't have any time, is it possible to provide some write access to merge this PR for me?

jbohren commented 2 years ago

@wep21 a quick evaluation shows that it's working for me on galactic with a v4l2 source, and it looks like you've put a bunch of time into reviewing and enhancing @clydemcqueen's initial PR. I read through the cpp source changes for anything untoward, and it looks like all changes are well-intentioned.

This looks like a great first rev for ros2 support. Thank you all for your contributions!

@wep21 I'm open to expanding maintainers, I need to check with current standards for review.

ros-drivers / gscam

ROS2 port #66