REP-136 compliance and compatibility with ROS1 & ROS2 using single branch

[wxmerkt]

In this PR, an exec_dependency on either catkin or ament_cmake is added per REP-136. The package format is updated to v3 to make use of condition arguments. This allows to only maintain a single devel branch and effectively deprecate ros2 going forward. ROS1 and ROS2 can now use devel.

@ruffsl @mikaelarguedas can you please test if this works for you?

Alternative to #303

[ruffsl]

can you please test if this works for you?

This still doesn't seem to register itself in the ament index if built as a ros2 package. See #303 (comment)

[wxmerkt]

I am not very familiar with ament or ROS2 yet unfortunately - since this is a pure CMake package, is it expected to register itself? Is there another required change to add?

[ahornung]

Anything missing to finish this PR?

[wxmerkt]

Thanks for the ping! I spotted an erroneous duplicated dependency and removed that now. I also added a build for both ROS1 and ROS2 using Industrial-CI to see if this config passes and builds using catkin-tools/colcon.

As for the registration of the package with the ament index, following ros-infrastructure/rosdep#724, we still need to create some extra conditionals or commands in the CMakeLists - do you have any insights or suggestions regarding this @ruffsl?

[wxmerkt]

The output of the new GitHub Actions CI can be seen here: https://github.com/wxmerkt/octomap/runs/1550182750

[spurnvoj]

Hi guys, any change to merge it?

[wxmerkt]

I believe this is ready to be merged and it passed Industrial-CI for both ROS1 and ROS2. I have not tested it in ament workspace, though, so if a community member could provide insights if it works as intended in a ROS2 environment that would be great.

[spurnvoj]

This is great because it will make the code compilable under ROS1 and also ROS2. However, as @ruffsl mentioned, the packages will not be registered in the ament index. Installing a package.xml is not enough for rosdep to detect the package in ROS2. It is enough for catkin for ROS1 but not for ROS2 (see the conversation here ros2/tinyxml_vendor#14). It looks like that the same problem is already on ros2 branch

[ahornung]

Does anyone know what's missing for a fix?

[ruffsl]

Perhaps we could copy and include what is used by ompl

ompl/ompl#754

[wxmerkt]

@ruffsl Thank you very much for the pointer. I added a commit following the OMPL example. Could you please test if this fixes the observed issue? This way we could retire the ros2 branch after merging this PR.

[wxmerkt]

@ruffsl @spurnvoj @tylerjw - did you get a chance to confirm whether this is working as intended for ROS2? If so we can go ahead and merge it and make releases for Foxy and Noetic. Many thanks :-).

[ruffsl]

I haven't had time to test this, but you could try and build octomap from a colcon underlay in a ros:foxy docker container, then source that underlay and try to build geometric_shapes in a separate colcon overlay. This should check to ensure that colcon can find octomap from the ament index in the underlay install folder and verify the octomap cmake is working as expected. Just make sure rosdpe doesn't accidently install octomap to the system, that would be below the underlay.

https://github.com/ros-planning/geometric_shapes/tree/ros2

[wxmerkt]

Thanks, I confirmed this as working using Docker.

@ahornung Can you please merge this PR and make a new release tag? I'll then release to Foxy, Rolling, and Galactic to address #335 and #300.

[ahornung]

Thanks for preparing all this - v1.9.7 is out!