search

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

service getting multiply defined #243

Closed knickels closed 4 years ago

knickels commented 4 years ago

Steps to reproduce issue

  1. build ros1 message packages (XXX_nav_msgs) in ros1/melodic. Note these have both msg/ and srv/ files in the package 
    source /opt/ros/melodic/setup.bash
catkin build
  2. build ros2 message packages (XXX_nav_msgs) in ros2/eloquent. Note these have both msg/ and srv/ files in the package 
    source /opt/ros/eloquent/setup.bash
colcon build --symlink-install
  3. attempt to build ros1_bridge 
    source ~/code/ros1_XXX_ws/develop/setup.bash
source ~/code/ros2_XXX_ws/install/setup.bash
cd ~/code/ros1_bridge
$ colcon build --symlink-install --cmake-force-configure

Expected behavior

bridge compiles without errors

Actual behavior

~/code/ros1_bridge_ws/build/ros1_bridge/generated/XXX_nav_msgssrvSvcName__factories.cpp:175:6: error: redefinition of ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_2_to_1(const ROS2Response&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response&) [with ROS1_T = XXX_nav_msgs::SvcName; ROS2_T = XXX_nav_msgs::srv::SvcName; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = XXX_nav_msgs::srv::SvcNameResponse<std::allocator >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = XXX_navmsgs::SvcNameResponse<std::allocator >]’

~/code/ros1_bridge_ws/build/ros1_bridge/generated/XXX_nav_msgssrvSvcName__factories.cpp:116:6: note: ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_2_to_1(const ROS2Response&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response&) [with ROS1_T = XXX_nav_msgs::SvcName; ROS2_T = XXX_nav_msgs::srv::SvcName; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = XXX_nav_msgs::srv::SvcNameResponse<std::allocator >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = XXX_navmsgs::SvcNameResponse<std::allocator >]’ previously declared here

Additional information

If I look at the generated C++ file, I see that it looks like the services are getting pulled in twice:


// include builtin interfaces
#include <ros1_bridge/convert_builtin_interfaces.hpp>

// include ROS 1 services
#include <XXX_nav_msgs/SvcName.h>
#include <XXX_nav_msgs/SvcName.h>

// include ROS 2 services
#include <XXX_nav_msgs/srv/SvcName.hpp>
#include <XXX_nav_msgs/srv/SvcName.hpp>

I only see the ROS1 packages once each time on the CMAKE_PREFIX_PATH, and
the ROS2 packages once each time on the AMEND_PREFIX_PATH.
dirk-thomas commented 4 years ago

To make your steps reproducible please include the ROS 1 and ROS 2 packages for XXX_nav_msgs.

knickels commented 4 years ago

I'll have to find non-proprietary packages that exhibit the same flaw, before I can do that. Looks (to me) the same issue as PR#242.

dirk-thomas commented 4 years ago

Looks (to me) the same issue as #242.

Does your service has a fields with a fixed size array type?

knickels commented 4 years ago

No, but I notice the services have fields which are custom messages. So srv/SvcName.srv has:

std_msgs/Header header
CustomParameters parameters
---
XXX_common_msgs/ServiceResult result

and CustomParameters.msg and ServiceResult.msg both only have standard elements (string, flaot64, bool)

dirk-thomas commented 4 years ago

Does your service has a fields with a fixed size array type?

No, but I notice the services have fields which are custom messages.

Then the referenced PR is not related.

I'll have to find non-proprietary packages that exhibit the same flaw, before I can do that.

Please still provide a complete set of sets to reproduce. It should be easy to distill a set of msgs / srvs from your proprietary package which can be publically shared.

knickels commented 4 years ago

Ok, I've narrowed it down to a single service that exhibits this behavior.

cat ../ros1_msg/src/xx_nav_msgs/srv/GetDbwCapabilities.srv 
# Service call to get the capabilities of the DBW system.

------ 
float64 min_curvature
float64 max_curvature
float64 max_curvature_rate_of_change 
cat ../ros2_msg/src/xx_nav_msgs/srv/GetDbwCapabilities.srv 
# Service call to get the capabilities of the DBW system.

---

float64 min_curvature
float64 max_curvature
float64 max_curvature_rate_of_change

With these services, I get the following error building the bridge.

[33.075s] /home/knickels/doc/army-mars/custom-msg/ros1_bridge_ws/build/ros1_bridge/generated/xx_nav_msgs__srv__GetDbwCapabilities__factories.cpp:169:6: error: redefinition of ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_2_to_1(const ROS2Response&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response&) [with ROS1_T = xx_nav_msgs::GetDbwCapabilities; ROS2_T = xx_nav_msgs::srv::GetDbwCapabilities; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = xx_nav_msgs::srv::GetDbwCapabilities_Response_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = xx_nav_msgs::GetDbwCapabilitiesResponse_<std::allocator<void> >]’
[33.076s] /home/knickels/doc/army-mars/custom-msg/ros1_bridge_ws/build/ros1_bridge/generated/xx_nav_msgs__srv__GetDbwCapabilities__factories.cpp:110:6: note: ‘void ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::translate_2_to_1(const ROS2Response&, ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response&) [with ROS1_T = xx_nav_msgs::GetDbwCapabilities; ROS2_T = xx_nav_msgs::srv::GetDbwCapabilities; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = xx_nav_msgs::srv::GetDbwCapabilities_Response_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = xx_nav_msgs::GetDbwCapabilitiesResponse_<std::allocator<void> >]’ previously declared here

and indeed it's in build/ros1_bridge/generated/xx_nav_msgs__srv__GetDbwCapabilities__factories.cpp twice, both included twice as #includes (which should be ok), and in the C++ code itself (lines 110 and 169).

knickels commented 4 years ago

I noticed and fixed the ------ in the ros1 service file, recompiled the ros1_msg package using catkin (different shell), then the ros1_bridge using colcon again (new shell, sourced as above). No change.

dirk-thomas commented 4 years ago

I don't see how this service is any different than all the other services we have and which can be used successfully by the ros1_bridge. Since I would need to create a ROS 1 and ROS 2 package for you to try out your specific file I won't go through that effort but assume the problem is somewhere in your process or the parts you haven't shared.

The duplicate includes shouldn't be there which is probably due to the same reason the functions are defined twice. That is likely due to a bug somewhere else in your package.

If you want me to investigate the problem you need to provide a SSCCE - a ready-to-run set of commands reproducing your problem.

knickels commented 4 years ago

That's fair, neither do I.

I'll look into doing the SSCCE. Would a pointer to a github repo with ros1_msg, ros2_msg, and a script for building the packages, cloning ros1_bridge, and building give you what you need?

(BTW, thanks for helping with this!)

dirk-thomas commented 4 years ago

I'll look into doing the SSCCE. Would a pointer to a github repo with ros1_msg, ros2_msg

Yes.

a script for building the packages, cloning ros1_bridge, and building give you what you need?

It doesn't have to be a script. Just a bullet list of command you ran will be good enough.

knickels commented 4 years ago

While developing the isolated SSCCE, I noticed in stderr that 

 Cannot generate a safe runtime search path for target ros1_bridge because
[16.784s]   files in some directories may conflict with libraries in implicit
[16.785s]   directories:
[16.785s]     runtime library [libconsole_bridge.so.0.4] in /usr/lib/x86_64-linux-gnu may be hidden by files in:
[16.785s]       /opt/ros/eloquent/lib
[16.785s]   Some of these libraries may not be found correctly.

And saw in another thread that this is something we should not ignore. I see that LD_LIBRARY_PATH=/opt/ros/eloquent/opt/yaml_cpp_vendor/lib:/opt/ros/eloquent/opt/rviz_ogre_vendor/lib:/opt/ros/eloquent/lib:/opt/ros/melodic/lib:/usr/lib/x86_64-linux-gnu:/usr/lib/i386-linux-gnu:/usr/local/nvidia/lib:/usr/local/nvidia/lib64

I see who owns each copy:

$ dpkg -S /opt/ros/eloquent/lib/libconsole_bridge.so.0.4
ros-eloquent-console-bridge-vendor: /opt/ros/eloquent/lib/libconsole_bridge.so.0.4
$ dpkg -S /usr/lib/x86_64-linux-gnu/libconsole_bridge.so.0.4 
libconsole-bridge0.4:amd64: /usr/lib/x86_64-linux-gnu/libconsole_bridge.so.0.4

I can't remove either package without removing the respective version of ros (melodic/eloquent).

Is this related, or should I open another issue here, or is this a generic (a.k.a answers.ros.org) question?

dirk-thomas commented 4 years ago

Is this related, or should I open another issue here, or is this a generic (a.k.a answers.ros.org) question?

This doesn't seem related. ROS 2 needs at least version 0.4.1 of console_bridge. As long as that one is first in the search path it should be fine. ROS 1 uses the console_bridge package from upstream Ubuntu. In Ubuntu Bionic that is only version 0.4 so ROS 2 needs to build a newer version. In newer Ubuntu distros console_bridge is new enough so ROS 2 doesn't need to build a custom version but also uses the package from upstream Ubuntu.

knickels commented 4 years ago

OK, @dirk-thomas , I'm going to close this out. I'm pretty sure that I had message paths from the original ros2_msg folder as well as the 'isolated' one that I was using to build the bridge, when I went to build the bridge. ros2 seems to be more apt to just add new paths rather than replacing them as seemed to happen in ros1. Thanks for the debugging help.