search

stereolabs / zed-ros2-wrapper

ROS 2 wrapper for the ZED SDK
https://www.stereolabs.com/docs/ros2/
Apache License 2.0
134 stars 139 forks source link

Rolling Support #188

Open peterdavidfagan opened 6 months ago

peterdavidfagan commented 6 months ago

Preliminary Checks

Proposal

Support ROS rolling in order to enable applications that leverage the latest versions of ROS packages.

Use-Case

Leveraging rolling versions of existing ROS packages. Currently I am looking to build a Zed container leveraging rolling for the following repository.

Myzhar commented 6 months ago

@peterdavidfagan This is not scheduled, but I agree this could be useful for R&D. We will consider this Feature Request for future development.

peterdavidfagan commented 6 months ago

Thanks @Myzhar much appreciated, for now I will remove these checks and test the launch files.

Myzhar commented 6 months ago

Maybe you can modify this part in all the CMakeLists.txt files too: https://github.com/stereolabs/zed-ros2-wrapper/blob/66e6d123615294c19c710de4813f7e6268107dc8/zed_wrapper/CMakeLists.txt#L7-L44 so you can add officially the ROLLING support

Myzhar commented 6 months ago

a nice and good 

   elseif(${FOUND_ROS2_DISTRO} STREQUAL "rolling") 
     #message("* ROS2 ${FOUND_ROS2_DISTRO} is officially supported by this package.") 
     add_definitions(-DFOUND_ROLLING)

at line 38 before the else() and you are ready

peterdavidfagan commented 6 months ago

Ok yes this is good advice, also a lot better than the quick solution I was going for to test things. Shall I open a PR with these changes?

Update: so far so good, I guess for opening a PR I would also need to update the CMakeLists.txt of the submodule dependency: https://github.com/stereolabs/zed-ros2-interfaces/blob/6dfb08531c324481418342e789adb2c3d836c917/CMakeLists.txt.

For my current purposes I think the above changes are ideal and better than simply deleting the check as I was doing.

Myzhar commented 6 months ago

Yes, you're welcome to open a PR