collision_plugin_loader port to ROS 2

[vmayoral]

Authored originally by @anasarrak.

[mlautman]

Let's rebase this onto master so that we can start using travis

[vmayoral]

@mlautman rebased this and #71 to check CI. I see #71 failed.

Edit: Looking into why it failed.

[vmayoral]

On a second look, it looks like the CI is not building anything just yet right @mlautman? https://github.com/ros-planning/moveit2/blob/master/.docker/ci/Dockerfile

Update: It seems part of it is being tested indeed https://travis-ci.org/ros-planning/moveit2/builds/514743505#L1322, only not what's of interest for this PR. What's the flow you'd expect for new PRs? Shall we take care of updating the CI or will you guys take care of it?

[mlautman]

I think that building the sub-packages and passing travis is an essential part of the PR process. Ideally each PR would include the changes necessary for travis to build the changes.

In this case, there are dependencies that will need to be ported before this can be built. (moveit_core and moveit_ros_perception) If there are open PR's for the dependencies that are blocking this port, then I would recommend that you include links in the PR description. That will help direct our focus towards getting the underlying dependencies done first.

[vmayoral]

I think that building the sub-packages and passing travis is an essential part of the PR process. Ideally each PR would include the changes necessary for travis to build the changes.

In this case, there are dependencies that will need to be ported before this can be built. (moveit_core and moveit_ros_perception) If there are open PR's for the dependencies that are blocking this port, then I would recommend that you include links in the PR description. That will help direct our focus towards getting the underlying dependencies done first.

Great, thanks. Can you also indicate which changes would be required for travis? Maybe provide an example for a simpler module (e.g. #67)?

[mlautman]

@vmayoral Here are the steps:

• Remove the COLCON_IGNORE at moveit2/moveit_ros/COLCON_IGNORE
• Since the COLCON_IGNORE in this case covers all of the packages in moveit_ros, you will need to create empty files named COLCON_IGNORE in each of the non-ported packages within the moveit_ros sub-directory. (eg manipulation and move_group)
• Port the CMakeLists.txt in moveit2/moveit_ros/planning/CMakeLists.txt so that it works with colcon. You will want to comment out all of the add_subdirectory calls to packages that have yet to be ported, leaving only the ported packages.
• Port the CMakeLists.txt for the subpackage in question. In this case it would be
  moveit2/moveit_ros/planning/collision_plugin_loader/CMakeLists.txt
• When you remove the moveit2/moveit_ros/COLCON_IGNORE file, travis will automatically try to build any directory with both a CMakeLists.txt and a package.xml. So moveit2/moveit_ros/planning/CMakeLists.txt should get built automatically

Let me know if this works for you

[vmayoral]

@mlautman, I went ahead and performed the steps described over the weekend. Thanks for the guidelines. Unfortunately, the result wasn't satisfactory and @LanderU and I ended up having to modify the Docker image and moveit_ci quite heavily to make it work.

See AcutronicRobotics#34 for the complete discussion or moveit/moveit_ci#56 for the proposed changes. Returning to this PR, I'd advice we go ahead and merge these changes since they already pass CI (see #62 (comment)).

[mlautman]

Unfortunately, the result wasn't satisfactory

This is a good thing. The fact that CI failed shows us were there are still issues. Let's fix them in moveit/moveit_ci#56 and get these packages building :)

[henningkayser]

@mlautman CI seems to pass now, is there still something missing here? Also, I was wondering if we should delay migrating the collision plugins until moveit/moveit#1584 is synced. The plugin loader doesn't seem to be affected though.

[henningkayser]

@JafarAbdi could you reopen this one as well?

[JafarAbdi]

@henningkayser Absolutely - I'll finish addressing the review for the other PR and open a new one for this package

[henningkayser]

Closing in favor of #137