ros-drivers / odva_ethernetip

Library implementing ODVA EtherNet/IP (Industrial Protocol).
94 stars 48 forks source link

Migration to ROS2 #22

Open Rayman opened 3 years ago

Rayman commented 3 years ago

Hi, I've been working on migrating the omron driver to ROS2 (https://github.com/ruvu/omron/tree/ros2). The first task is deciding how to migrate odva_ethernetip to ros2.

Fortunately the code does not depend on ros, so that is nice. We only have to worry about the CMakeLists.txt. I've come up with two possible migration paths:

What are the preferences of the maintainers? I could provide a PR for both variants, but I have a plain cmake one ready.

1. Convert odva_ethernetip to plain cmake

By converting to a plain cmake package, the same codebase can be used both by ROS1 as ROS2 (and without ROS). For maintainers this is useful because there is only one branch, no need to backport fixes.

The downside is that all ros1 packages that depend on this package, should do a small migration. To do this, I would recommend to do this migration from noetic onwards as to not break any code. So it would need a noetic-devel branch.

Before:

find_package(catkin REQUIRED COMPONENTS odva_ethernetip)

After:

find_package(odva_ethernetip REQUIRED)

See also this branch for the plain cmake variant: https://github.com/ruvu/odva_ethernetip/tree/feature/plain-cmake

See this branch for the required migration of the omron driver: https://github.com/ros-drivers/omron/commit/6046bdb1fc0f4c5fe7c2b0dc62014482fc2d41f3

2. Convert odva_ethernetip to use ament_cmake

By converting to ament_cmake the code is ideomatic ROS2, and a little simpler. The downside is that maintainers will have to maintain two branches, one for each ROS version. Backporting changes will take a little bit more time.

reinzor commented 3 years ago

@Rayman , we are not actively using this package anymore so if you would like to maintain both packages; you are more than welcome. Also, I am not able to test the suggested changes at the moment.