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

remove duplicate services and msgs #295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

v-lopez
Copy link

@v-lopez v-lopez commented Nov 18, 2020

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

v-lopez commented Nov 18, 2020

I have a similar fix for actions for when #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)

# iterate_packages might return ROS2 paths for already found ROS1 pkgs
msg = Message(package_name, message_name, path)
if msg not in msgs:
msgs.append(msg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it guaranteed that the first message found will be the ROS 1 version and not the ROS 2 version? If not then this check might not be enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's guaranteed, so you're right.

I've reimplented this, please take a look.

@v-lopez
Copy link
Author

v-lopez commented May 20, 2021

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

@Timple
Copy link
Contributor

Timple commented Mar 3, 2023

Probably not, galactic is EOL already...

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

Successfully merging this pull request may close these issues.

3 participants