Upgraded extract_images_sync script to ROS 2

[artineering]

Change Description: The extract_images_sync python script was upgraded to ROS 2.
Testing: Upgraded node tested against Iron on Ubuntu 22.04.3 LTS
Issue: fixes #860

[mikeferguson]

Mostly good - a couple of minor issues noted. I'll also have to test this a bit later in the week once I'm back in the office.

[artineering]

I had inadvertently changed the api signature for cv_bridge.cvtColorForDisplay. Have reverted that change.

[artineering]

Thank you, @ahcorde for catching that mistake.

[mikeferguson]

3 notes from me:

• Can you pop the last commit off of this? We'll end up squashing the history when we merge this, so I suggest we make a separate PR afterwards for the sec_per_frame update - that will make the history far more clear for anyone coming back and looking at those changes in the future.
• I think we need to add an install for the script (I don't see anything in CMake - I presume "ros2 run" doesn't actually work? Unless there is some magic in the auto_ament commands that I'm not aware of)
• Can you give a quick description of your "testing"? I didn't get a chance to try this out today - if you can describe how you tested it, that might give me the confidence to merge without actually testing it myself

[mikeferguson]

And one other question if you don't mind - how did you find out about these issues and decide to work on them? (this is more for calibrating my notions of "what gets people involved in ROS")

[artineering]

3 notes from me:

• Can you pop the last commit off of this? We'll end up squashing the history when we merge this, so I suggest we make a separate PR afterwards for the sec_per_frame update - that will make the history far more clear for anyone coming back and looking at those changes in the future.

I agree. I did not intend to this change to get included in this. But, since I am fairly new to github, I didn't realize that submitting it to my fork would automatically update the PR. I will try to squash the change on my side and resubmit once this PR is merged.

• I think we need to add an install for the script (I don't see anything in CMake - I presume "ros2 run" doesn't actually work? Unless there is some magic in the auto_ament commands that I'm not aware of)

Yes, the script does not run by itself. In order to run it, you need to create a ros package and copy the script into it. Thats what I did to test the script upto the point where I was able to build and run. I tried out all ros arguments.

• Can you give a quick description of your "testing"? I didn't get a chance to try this out today - if you can describe how you tested it, that might give me the confidence to merge without actually testing it myself

I tested the build and run part of the script. I also tested whether all the ros arguments are getting picked up as expected. I couldn't get the camera/image_raw to publish properly the last time I ran the code in ros2, so that part is pending.

[artineering]

And one other question if you don't mind - how did you find out about these issues and decide to work on them? (this is more for calibrating my notions of "what gets people involved in ROS")

My background is in Image Processing and have started my PhD in Computer Science recently. In discussions with my professor and other TAs, contributing to ROS was one of the things recommended to get ourselves familarized with the ecosystem. ROS-perception felt like the obvious choice for me. Your tag ('good first issue') helped identify the issues that I felt confident enough to handle.

I do plan to contribute to this repository as best I can by taking on more issues and hopefully contributing some new features as well.

[mikeferguson]

I just pushed a commit with the install directive and tested this locally - was able to save images from a Primesense camera with:

ros2 launch openni2_camera camera_with_cloud.launch.py
ros2 run image_view extract_images_sync --ros-args -p inputs:='[/camera/rgb/image_raw, /camera/depth/image_raw]' -p approximate_sync:=True

I did have to slightly patch the encoding stuff (cv_bridge says 8UC3 isn't valid) - I'll put that in a follow on PR once I look into it in more detail.