ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
436 stars 283 forks source link

Parametrize Quality of Service in `parameter_bridge`. #331

Closed LoyVanBeek closed 2 years ago

LoyVanBeek commented 2 years ago

Had some trouble using the parameter_bridge with /tf_static Turns out, I/m not the only one: https://answers.ros.org/question/349230/problem-passing-tf_static-through-ros1_bridge-to-ros2/ . The dynamic_bridge has an special case for this topic introduced in https://github.com/ros2/ros1_bridge/pull/282, but not in the parameter_bridge.

To tackle that and be the most general I could be, I extended the parameter_bridge to set specific QoS settings per topic, allowing one the implement that special case if required.

I've used this config for testing (also with different and illegal values for the settings):

bridge.yaml.

topics:
  - 
    topic: /chatter  # ROS1 topic name
    type: std_msgs/msg/String  # ROS2 type name
    queue_size: 1  # For the publisher back to ROS1
  - 
    topic: /joint_states
    type: sensor_msgs/msg/JointState
    queue_size: 1
  - 
    topic: /message_to_ros
    type: std_msgs/msg/String
    queue_size: 1
  - 
    topic: /message_from_ros
    type: std_msgs/msg/String
    queue_size: 1
    qos:
      history: keep_last
      depth: 10
      reliability: reliable
      durability: transient_local
      deadline: 
          secs: 10
          nsecs: 2345
      lifespan: 
          secs: 20
          nsecs: 3456
      liveliness: LIVELINESS_AUTOMATIC
      liveliness_lease_duration: 
          secs: 40
          nsecs: 5678
  - 
    topic: /tf
    type: tf2_msgs/msg/TFMessage
    queue_size: 1
  - 
    topic: /tf_static
    type: tf2_msgs/msg/TFMessage
    queue_size: 1
    qos:
      history: keep_all
      durability: transient_local

Usage of the parameter_bridge is explained in https://github.com/ros2/ros1_bridge/pull/330, this bridge.yaml is can be used in the same way.

LoyVanBeek commented 2 years ago

Anything I can do to improve this PR and maybe get it merged eventually?

LoyVanBeek commented 2 years ago

Anything I can do to get this merged?

gbiggs commented 2 years ago

CI: Build Status

gbiggs commented 2 years ago

Let's try that again.

Build Status

methylDragon commented 2 years ago

Unstable build seems to be coming from unrelated file: https://github.com/ros2/ros1_bridge/blob/master/include/ros1_bridge/builtin_interfaces_factories.hpp

gbiggs commented 2 years ago

Indeed, the failures look unrelated, so I'm going to go ahead and merge this.

jacobperron commented 2 years ago

Because of the added dependency on xmlrpcpp, this PR breaks building ros1_bridge on a system with only ROS 2 installed. Previously, I was able to build this package without ROS 1 installed (well, it kind of just skipped the build).

We might want to make xmlrpcpp a conditional dependency, like roscpp, emit a warning and skip the build, e.g.

https://github.com/ros2/ros1_bridge/blob/4af421df41c0b162e115c9e04979f6aeb19425c4/CMakeLists.txt#L34

Maybe this is not super important since we've removed ros1_bridge from the default ros2.repos file.

gbiggs commented 2 years ago

In the interest of not having to deal with bug reports about that problem, I've gone ahead and made a PR that moves the xmlrpcpp search to after searching for ROS 1.

371

LoyVanBeek commented 2 years ago

Ah, my bad. I had assumed (the mother of ...) that xmlrpcpp would always be available on a system where ros1_bridge would be built, given the ROS 1 dependencies.