ros2 / ros1_bridge

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

remove duplicate services and msgs #295

Open v-lopez opened 3 years ago

v-lopez commented 3 years ago

iterate_packages might return ROS2 paths for already found ROS1 pkgs

This results in duplicated entries in data and double definitions in the generated cpp code.

This happens to me building for instance this package: https://github.com/pal-robotics/pal_msgs/tree/ros2-tests/pal_common_msgs

I am trying to have both ROS1 and ROS2 msgs on the same branch, and need to do some package.xml and CMakeLists hacking, which might be tricking iterate_packages.

I have 3 workspaces:

ros1_ws extending /opt/ros/noetic, built with caktin_make install ros2_ws extending /opt/ros/foxy, built with colcon build bridge_ws, extending ros1_ws and ros2_ws install dirs.

And rosmsg.iterate_packages(rospack, rosmsg.MODE_MSG) returns:

 ('pal_common_msgs', '/root/msgs/ros1_ws/install/share/pal_common_msgs/msg'),
 ('pal_common_msgs',
  '/root/msgs/ros2_ws/install/pal_common_msgs/share/pal_common_msgs/msg'),

Am I doing something wrong? I was surprised that this did not happen with other messages that are both installed in ROS1 and ROS2.

v-lopez commented 3 years ago

I have a similar fix for actions for when https://github.com/ros2/ros1_bridge/pull/256 is merged, or if this is merged first I'll push there.

-        if match:
-            actions.append({
+        action_data = {
                 'ros1_name': pair[0].message_name,
                 'ros2_name': pair[1].message_name,
                 'ros1_package': pair[0].package_name,
                 'ros2_package': pair[1].package_name,
                 'fields': output
-            })
+            }
+        if match and action_data not in actions:
+            actions.append(action_data)
v-lopez commented 3 years ago

Friendly ping, could we move this? Is it going to make it to foxy or just galactic an newer?

Timple commented 1 year ago

Probably not, galactic is EOL already...