ros-tooling/setup-ros

Add support for installing ROS1/ROS2 releases

Closed this issue · 7 comments

thomas-moulard commented

Description

⚠️ Before starting, we need to understand if this is needed. If rosdep can properly pull dependencies, action-ros2-ci rosdep run may provide this to us "for free".

As a GH action user, I want to install a specific version of ROS1 using OSRF Debian packages.

This should be implemented by executing commands on the host machines, not through Docker (to keep the ROS1/ROS2 codepath similar for now, this can be re-evaluated down the road).

This requires splitting the current logic into multiple modules to prevent code duplication.

The API could look like this:

- uses: ros-tooling/setup-ros2@x.y.z
  with:
    - version: ros1-melodic
    - variant: desktop
    # etc.

Allowed values for version be: ros-melodic, ros-source, ros2-dashing, ros2-source, etc.
We would default to ros2-source to avoid breaking compatibility.

Test Plan

  • Unit test for new classes
  • End to end integration test

Documentation Plan

  • README.md update

Release Plan

  • setup-ros2 should be re-released

Acceptance Criteria

  • Code has been implemented, reviewed and merged.
  • Test plan has been completed
  • Release plan has been completed

Once all above items are checked, this story can be moved to done.

References

zmichaels11 commented

Duplicate? #20

thomas-moulard commented

ROS1 != ros1_bridge

zmichaels11 commented

Oh, I was assuming that setup_ros2 would only use ROS1 through `ros1_bridge

thomas-moulard commented

Unfortunately, ros1_bridge has a dependency on ROS1 and ROS2, and needs both installed.

zmichaels11 commented

Instead, should there be an action called "setup_ros1"?
So the workflow would look like:

steps:
- uses: ros-tooling/setup-ros2
- uses: ros-tooling/setup-ros1
- uses: ros-tooling/action-ros2-ci
thomas-moulard commented

Need to check, but I think that there is too much overlap between ROS1/ROS2 install to justify a different workflow, but I may be wrong (?).

thomas-moulard commented

This has been fixed by #30