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

Explicitly mapping packages with the same package name and service names causes a redefinition error on build #257

Closed suddrey-qut closed 4 years ago

suddrey-qut commented 4 years ago

Operating System: Description: Ubuntu 18.04.4 LTS

Installation type: eloquent binaries, ros1_bridge source

Version or commit hash: On eloquent branch of ros1_bridge: ± git rev-parse HEAD 60f903cb159ad4ef59b9b09badf71caa601cba04

DDS implementation: Fast-RTPS

Client library (if applicable): N/A

Steps to reproduce issue

Following the instructions found on the ros1_bridge docs.

Clone the ROS1 version of the messages into a ROS1 workspace and build it

git clone git@github.com:RoboticVisionOrg/rv_msgs.git -b ros1_bridge_example

Clone the ROS2 version of the messages into a ROS2 workspace and build it

git clone git@github.com:RoboticVisionOrg/rv_msgs.git -b eloquent-devel

In a new terminal, ensuring no work spaces are sourced clone the eloquent branch of ros1_bridge into a new empty workspace

git clone https://github.com/ros2/ros1_bridge.git -b eloquent
colcon build --packages-select ros1_bridge --cmake-force-configure --cmake-args -DBUILD_TESTING=FALSE

Expected behavior

Bridge compiles without errors

Actual behavior

generate_services adds a pair for both the implicit match derived from rospack and the explicit match provided in the mapping.yaml file causing gcc to throw redefinition errors.

~/ros2/bridge_ws/build/ros1_bridge/generated/rv_msgs__srv__GetDoorStatus__factories.cpp:124:6: error: redefinition of ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_1_to_2(const ROS1Request&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request&) [with ROS1_T = rv_msgs::GetDoorStatus; ROS2_T = rv_msgs::srv::GetDoorStatus; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Request = rv_msgs::GetDoorStatusRequest_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request = rv_msgs::srv::GetDoorStatus_Request_<std::allocator<void> >]’
 void ServiceFactory<
      ^~~~~~~~~~~~~~~
   rv_msgs::GetDoorStatus,
   ~~~~~~~~~~~~~~~~~~~~~~~
   rv_msgs::srv::GetDoorStatus
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 >::translate_1_to_2(
 ~     
~/ros2/bridge_ws/build/ros1_bridge/generated/rv_msgs__srv__GetDoorStatus__factories.cpp:71:6: note: ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_1_to_2(const ROS1Request&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request&) [with ROS1_T = rv_msgs::GetDoorStatus; ROS2_T = rv_msgs::srv::GetDoorStatus; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Request = rv_msgs::GetDoorStatusRequest_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request = rv_msgs::srv::GetDoorStatus_Request_<std::allocator<void> >]’ previously declared here
 void ServiceFactory<
      ^~~~~~~~~~~~~~~
   rv_msgs::GetDoorStatus,
   ~~~~~~~~~~~~~~~~~~~~~~~
   rv_msgs::srv::GetDoorStatus
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 >::translate_1_to_2(
 ~     
~/ros2/bridge_ws/build/ros1_bridge/generated/rv_msgs__srv__GetDoorStatus__factories.cpp:137:6: error: redefinition of ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_1_to_2(const ROS1Response&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response&) [with ROS1_T = rv_msgs::GetDoorStatus; ROS2_T = rv_msgs::srv::GetDoorStatus; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = rv_msgs::GetDoorStatusResponse_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = rv_msgs::srv::GetDoorStatus_Response_<std::allocator<void> >]’
 void ServiceFactory<
      ^~~~~~~~~~~~~~~
   rv_msgs::GetDoorStatus,
   ~~~~~~~~~~~~~~~~~~~~~~~
   rv_msgs::srv::GetDoorStatus
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 >::translate_1_to_2(
 ~     

Additional information

This is the same issue as #243.

dirk-thomas commented 4 years ago

Since packages with the same name as in this example are mapped automatically why do you specific it explicitly in the mapping file?

suddrey-qut commented 4 years ago

At the time I was skimming the tutorials and did not notice that it would implicitly generate a bridge for packages with the same name. It also seems that for messages it does not care if I provide an explicit mapping.

In hindsight I know what I did was technically redundant, but it's hard to tell from the error messages how I had caused the issue.

I have made a pull request to bring services in line with messages, which do handle duplication between explicit and implicit pairs.

suddrey-qut commented 4 years ago

For sanity, I have created a valid example of where this issue would arise

I have created a slimmed down version of my messages package that has a single message and service. The service name is the same between ROS1 and ROS2 but the message has been renamed from ROS1Message to ROS2Message respectively.

I have also amended the mapping file such that it has a mapping between ROS1Message and ROS2Message, but has no explicit mapping for the service.

Eloquent branch: https://github.com/RoboticVisionOrg/rv_msgs/tree/eloquent_bridge_example

ROS1 branch: https://github.com/RoboticVisionOrg/rv_msgs/tree/ros1_bridge_example

In this example, I still get the redefinition error.

peterpena commented 4 years ago

When I compile your example repositories, there are no compilation errors. When I add the mapping for the GetDoorStatus service, I do get redefinition errors. I made some comments on the pull request referenced.

kei-mo commented 4 years ago

I encountered the same error and solved it with your patch! thanks a lot!