Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration to ROS2 #22

Open
Rayman opened this issue Dec 2, 2020 · 1 comment
Open

Migration to ROS2 #22

Rayman opened this issue Dec 2, 2020 · 1 comment

Comments

@Rayman
Copy link

Rayman commented Dec 2, 2020

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:
ros-drivers/omron@6046bdb

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
Copy link
Member

reinzor commented Dec 2, 2020

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants