ros1_bridge deb depends on rospy_tutorials but not on the ros2 equivalent

[mikaelarguedas]

Bug report

The fix will happen on the release repository but opened this issue here for visibility and because that's where the readme is hosted.

Required Info:

• Operating System:
  • Ubuntu bionic
• Installation type:
  • debs
• Version or commit hash:
  • dashing (and eloquent)
• DDS implementation:
  • N/A
• Client library (if applicable):
  • N/A

ros1_bridge deb pulls the ros1 talker listener examples (roscpp_tutorials and rospy_tutorials) but not the ROS2 equivalents

The README says to use demo_nodes_{cpp,py} on the ROS 2 side and ros{cpp,py}_tutorials on the ROS 1 side.
For consistency it would be better to either pull neither or all of these packages as dependencies.

Steps to reproduce issue

docker run -it --rm osrf/ros:dashing-ros1-bridge
# apt -qq update && apt-cache depends ros-dashing-ros1-bridge | grep demo-node

# apt -qq update && apt-cache depends ros-dashing-ros1-bridge | grep tutorials
  Depends: ros-melodic-roscpp-tutorials
  Depends: ros-melodic-rospy-tutorials

Expected behavior

# apt -qq update && apt-cache depends ros-dashing-ros1-bridge | grep demo-node
  Depends: ros-dashing-demo-nodes-cpp
  Depends: ros-dashing-demo-nodes-py

[dirk-thomas]

The ros1_bridge doesn't depend on the rospy_tutorials package (at least directly) since the bridge can be used without that package just fine.

The talker / listener demonstration is only an example how the bridge can be used. In order to run that demo you will just need to make sure that both packages on the ROS 1 as well as the ROS 2 side are installed independently of the bridge.

It probably makes sense to install those packages in the mentioned docker container. For that please open a ticket in the repository defining these docker images.

[mikaelarguedas]

The ros1_bridge doesn't depend on the rospy_tutorials package (at least directly) since the bridge can be used without that package just fine.

The ros1_bridge deb depends directly on these ROS 1 packages as pointed out in the issue description.

apt-cache depends ros-eloquent-ros1-bridge | grep tutorials
  Depends: <ros-melodic-roscpp-tutorials>
  Depends: <ros-melodic-rospy-tutorials>

From issue description:

For consistency it would be better to either pull neither or all of these packages as dependencies.

It looks like you're advocating for the former and to remove the direct dependency on the ros{py,cpp}_tutorials packages. Is that correct?

[dirk-thomas]

The ros1_bridge deb depends directly on these ROS 1 packages as pointed out in the issue description.

The package manifest doesn't contain a dependency on either of these two packages: see https://github.com/ros2/ros1_bridge/blob/f69a1dfc42e591fed1a213af486d9f89c9efa127/package.xml

[dirk-thomas]

These Debian dependencies are injected in the release repo: https://github.com/ros2-gbp/ros1_bridge-release/blob/91ca9fe4c869394dc22ac0a3471fddb935d0772d/debian/control#L11

• Update: the reason is that these packages contain a service the bridge needs to know about at compile to in order to bridge it.

Therefore I will move the ticket to ros2-gbp/ros1_bridge-release. Nvm, that doesn't work across org units. It should be resolved in conjunction with ros2-gbp/ros1_bridge-release#6.

[nuclearsandwich]

@mikaelarguedas Doesn't the bridge need both ROS 1 and ROS 2 deps at build time to enable communication?

If we remove the ROS 1 dependencies when adding their ROS 2 counterparts, won't that also result in unbridgeable topics since the ROS 1 deps won't be there?

[dirk-thomas]

Why should the Debian package contain these dependencies? A user might want to install the bridge but not use it with the demo talker / listener.

I would argue the ROS 1 tutorial dependencies should be removed (instead of adding the ROS 2 demo dependencies for symmetry) to avoid having non-message dependencies which are not strictly necessary for the bridge to work.

• Update: the reason the bridge depends on the ROS 1 tutorials is that they contain a service definition.

[nuclearsandwich]

Why should the Debian package contain these dependencies?

The intent is to provide a ros1_bridge binary that is maximally useful.

From the ros1_bridge README:

The bridge is currently implemented in C++ as at the time the Python API for ROS 2 had not been developed. Because of this its support is limited to only the message/service types available at compile time of the bridge. The bridge provided with the prebuilt ROS 2 binaries includes support for common ROS interfaces (messages/services), such as the interface packages listed in the ros2/common_interfaces repository and tf2_msgs.

I haven't been following the development of the bridge so I don't know if that statement is still accurate.

I can't find anything in writing, but starting with Beta 3 we were manually adding interfaces packages so that the bridge binary is built with support for them. The number of packages added has definitely increased since beta 3. So if the suggestion is for culling back to just the basic messages described in the README that's also an option.

[dirk-thomas]

The discussion here is not about message packages. We do want to list as many message packages out of the box to provide maximum usability for users (to avoid them having to build the bridge from source).

This is about the ROS 2 demo / ROS 1 tutorial packages.demo_nodes_cpp / demo_nodes_py define no messages and therefore don't need to be a dependency of the bridge.

Looking at the ROS 1 tutorial packages roscpp_tutorials / rospy_tutorials they actually do define a service. And that service is being mapped to the service provided by the ROS 2 example_interfaces package. So that is why the dependency on the ROS 1 tutorial pkgs makes sense.

With that rational this ticket as well as the two PRs should be closed.

[nuclearsandwich]

demo_nodes_cpp / demo_nodes_py define no messages and therefore don't need to be a dependency of the bridge.

That makes sense to me. I don't know whether @mikaelarguedas had some other rationale for their inclusion.

[mikaelarguedas]

No rationale for inclusion 👍 . Inclusion in the docker images has been added in osrf/docker_images#341 so no need for inclusion in the bridge.

From @dirk-thomas comment above it looks like there is a service defined in roscpp_tutorials that is being mapped so it should be kept. Not sure about rospy_tutorials, as it defines the same interface my guess is that it should be kept for consistency, maybe with an additional mapping in example_interfaces to be able to bridge that one as well.

Closing this