ros2 / ros1_bridge

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

This specific mapping results in a build error #372

Open alexg-k opened 2 years ago

alexg-k commented 2 years ago

The following service should map fine in ros1_bridge. All my other messages and services are structured like this and build just fine. For some reason this specific service produces a build error. I could fix the error by manually modifying the generated factory, as can be seen below. Is this a bug or did I use an unexpected naming scheme somewhere? Any advice is appreciated.

ros1 service called "MySrv.srv":

BasicRequest basicRequest
---
BasicResponse basicResponse

ros2 service called "MySrv.srv":

BasicRequest basic_request
---
BasicResponse basic_response

my_msgs_mapping.yaml:

-
  ros1_package_name: 'my_msgs'
  ros1_service_name: 'MySrv'
  ros2_package_name: 'my_msgs'
  ros2_service_name: 'MySrv'
  request_fields_1_to_2:
    basicRequest: 'basic_request'
  response_fields_1_to_2:
    basicResponse: 'basic_response'

These are the resulting build errors:

/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp: In member function ‘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 = my_msgs::MySrv; ROS2_T = my_msgs::srv::MySrv; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Request = my_msgs::MySrvRequest_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request = my_msgs::srv::MySrv_Request_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:62:101: error: ‘basic_request1’ was not declared in this scope; did you mean ‘basic_request2’?
   62 |   Factory<my_msgs::BasicRequest,my_msgs::msg::BasicRequest>::convert_1_to_2(basic_request1, basicRequest2);
      |                                                                                                     ^~~~~~~~~~~~~~
      |                                                                                                     basic_request2
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:62:117: error: ‘basicRequest2’ was not declared in this scope; did you mean ‘basicRequest1’?
   62 |   Factory<my_msgs::BasicRequest,my_msgs::msg::BasicRequest>::convert_1_to_2(basic_request1, basicRequest2);
      |                                                                                                                     ^~~~~~~~~~~~~
      |                                                                                                                     basicRequest1
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:60:10: warning: unused variable ‘basicRequest1’ [-Wunused-variable]
   60 |   auto & basicRequest1 = req1.basicRequest;
      |          ^~~~~~~~~~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:61:10: warning: unused variable ‘basic_request2’ [-Wunused-variable]
   61 |   auto & basic_request2 = req2.basic_request;
      |          ^~~~~~~~~~~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp: In member function ‘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 = my_msgs::MySrv; ROS2_T = my_msgs::srv::MySrv; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = my_msgs::MySrvResponse_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = my_msgs::srv::MySrv_Response_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:75:103: error: ‘basic_response1’ was not declared in this scope; did you mean ‘basic_response2’?
   75 |   Factory<my_msgs::BasicResponse,my_msgs::msg::BasicResponse>::convert_1_to_2(basic_response1, basicResponse2);
      |                                                                                                       ^~~~~~~~~~~~~~~
      |                                                                                                       basic_response2
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:75:120: error: ‘basicResponse2’ was not declared in this scope; did you mean ‘basicResponse1’?
   75 |   Factory<my_msgs::BasicResponse,my_msgs::msg::BasicResponse>::convert_1_to_2(basic_response1, basicResponse2);
      |                                                                                                                        ^~~~~~~~~~~~~~
      |                                                                                                                        basicResponse1
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:73:10: warning: unused variable ‘basicResponse1’ [-Wunused-variable]
   73 |   auto & basicResponse1 = req1.basicResponse;
      |          ^~~~~~~~~~~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_msgs__srv__MySrv__factories.cpp:74:10: warning: unused variable ‘basic_response2’ [-Wunused-variable]
   74 |   auto & basic_response2 = req2.basic_response;
      |   

After I modify the auto-generated code at line 62 to: Factory<my_msgs::BasicRequest,my_msgs::msg::BasicRequest>::convert_1_to_2(basicRequest1, basic_request2); and line 75 to: Factory<my_msgs::BasicResponse,my_msgs::msg::BasicResponse>::convert_1_to_2(basicResponse1, basic_response2); the build finishes succesfully.

I use a docker container running Ubuntu 22.04 with ros1 noetic and ros2 humble built from source. The ros1_bridge is built from the latest master branch of this repository.

gbiggs commented 2 years ago

I can't immediately see anything wrong with your definitions. Here are a couple of things you could try:

  1. Naming the fields differently from their types, e.g.:

    BasicRequest req
    ---
    BasicResponse resp
  2. Making the names used in ROS 1 and ROS 2 very different, rather than similar.

Although I don't expect either of those to fix it, they will at least make it easier to debug.

If you can provide a repository containing your code, or a link to your docker image where the error occurs, I can help you debug it further.

alexg-k commented 2 years ago

Thank you, I really appreciate the offer, but I am not able to hand out the involved messages. Like you said, renaming the messages did not resolve the error. I will try to reproduce the error with generic messages next.

gbiggs commented 1 year ago

@alexg-k Have you had any luck reproducing the error?

alexg-k commented 1 year ago

What I found out is that the custom mapping rule for MySrv.srv in the mapping yaml-file has no effect on the build error. The mapping file is getting parsed during the build process (I checked it using an invalid rule with a wrong syntax), which resulted in an error output: Ignoring a rule without a ros1_package_name and/or ros2_package_name. However, the build process gives no such output when a faulty mapping (non-existing parameter names, non-existing message names, ...) are being mapped in the mapping file. This makes the mapping file very hard to debug and I don't know if my (correct) mapping rule is being used.

if it is any help, I can show you the relevant, faulty excerpt of the generated my_msgs__srv__MySrv__factories.cpp:

52 template <>
53 void ServiceFactory<
54   my_msgs::MySrv,
55   my_msgs::srv::MySrv
56 >::translate_1_to_2(
57   const my_msgs::MySrv::Request& req1,
58   my_msgs::srv::MySrv::Request& req2
59 ) {
60   auto & basicRequest1 = req1.basicRequest;
61   auto & basic_request2 = req2.basic_request;
62   Factory<my_msgs::BasicRequest,my_msgs::msg::BasicRequest>::convert_1_to_2(basic_request1, basicRequest2);
63 }
64 
65 template <>
66 void ServiceFactory<
67   my_msgs::MySrv,
68   my_msgs::srv::MySrv
69 >::translate_1_to_2(
70   const my_msgs::MySrv::Response& req1,
71   my_msgs::srv::MySrv::Response& req2
72 ) {
73   auto & basicResponse1 = req1.basicResponse;
74   auto & basic_response2 = req2.basic_response;
75   Factory<my_msgs::BasicResponse,my_msgs::msg::BasicResponse>::convert_1_to_2(basic_response1, basicResponse2);
76 }

EDIT: Another interesting finding: I switched the datatypes for the parameters in MySrv.srv from BasicRequest and BasicResponse to int8 for the ROS1 and ROS2 service, as shown below: "MySrv.srv":

int8 basic_request
---
int8 basic_response

The build error is then resolved. This is obviously not a solution to my problem, but is interesting nonetheless.

gbiggs commented 1 year ago

One possible cause is something that's turned up recently in a couple of other issues. In ROS 2, interface definitions (messages, services and actions) are, by convention, stored in their own package that has _interfaces (new) or _msgs as the suffix of its name. This is only a convention: in general things will work just fine if you don't do this. However the ros1_bridge has this convention hard-coded in when it searches for matching interface definitions between ROS 1 and ROS 2.

I don't think this is the cause of your problem because it looks like you have your messages in a package named my_msgs, but it's the only thing I can think of without being able to see your code.

alexg-k commented 1 year ago

Yup, all my interface packages are named *_msgs. I suspect it has something to do with the parameter type name (BasicRequest) and the parameter name (basicRequest) and the name of the above mentioned message (BasicRequest.msg), that is being recursively used in another message (MySrv.srv), looking identical for case-insensitive software.

gbiggs commented 1 year ago

Does renaming your messages and fields make it work?

alexg-k commented 1 year ago

First of all, sorry for the long wait.

Secondly, renaming the messages, parameters did not resolve the issue or changed the outcome.

Thirdly, I would like to focus on my remark from before:

What I found out is that the custom mapping rule for MySrv.srv in the mapping yaml-file has no effect on the build error. The mapping file is getting parsed during the build process (I checked it using an invalid rule with a wrong syntax), which resulted in an error output: Ignoring a rule without a ros1_package_name and/or ros2_package_name. However, the build process gives no such output when a faulty mapping (non-existing parameter names, non-existing message names, ...) are being mapped in the mapping file. This makes the mapping file very hard to debug and I don't know if my (correct) mapping rule is being used.

Changing line 7 in my my_msgs_mapping.yamlfile from

- basicRequest: 'basic_request'
+ basicRequest: 'basic_request123'

does indeed no effect on the error message. Even fully commenting out the mapping rule for the MySrv interface has no effect. Like said before, the mapping file (yaml) is getting parsed for sure as introducing a syntax error here, raises a different build error from yaml/scanner.py.

gbiggs commented 1 year ago

There's clearly something broken somewhere, but I'm not able to pin it down because I can't reproduce it locally. If you can produce a simple example that shows the problem and share the repository with me then I'll be able to help you figure this out.

alexg-k commented 1 year ago

Ok, I have a docker container ready that I can share with you. I uploaded it to a random storage site due to its size. It should produce the following build error:

--- stderr: ros1_bridge
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp: In member function ‘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 = my_pkg1::MySrv1; ROS2_T = my_pkg2::srv::MySrv2; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Request = my_pkg1::MySrv1Request_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request = my_pkg2::srv::MySrv2_Request_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:62:83: error: ‘req21’ was not declared in this scope; did you mean ‘req22’?
   62 |   Factory<my_pkg1::MyFirstMessage1,my_pkg2::msg::MyFirstMessage2>::convert_1_to_2(req21, req12);
      |                                                                                   ^~~~~
      |                                                                                   req22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:62:90: error: ‘req12’ was not declared in this scope; did you mean ‘req22’?
   62 |   Factory<my_pkg1::MyFirstMessage1,my_pkg2::msg::MyFirstMessage2>::convert_1_to_2(req21, req12);
      |                                                                                          ^~~~~
      |                                                                                          req22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:60:10: warning: unused variable ‘req11’ [-Wunused-variable]
   60 |   auto & req11 = req1.req1;
      |          ^~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:61:10: warning: unused variable ‘req22’ [-Wunused-variable]
   61 |   auto & req22 = req2.req2;
      |          ^~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp: In member function ‘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 = my_pkg1::MySrv1; ROS2_T = my_pkg2::srv::MySrv2; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = my_pkg1::MySrv1Response_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = my_pkg2::srv::MySrv2_Response_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:75:85: error: ‘resp21’ was not declared in this scope; did you mean ‘resp22’?
   75 |   Factory<my_pkg1::MySecondMessage1,my_pkg2::msg::MySecondMessage2>::convert_1_to_2(resp21, resp12);
      |                                                                                     ^~~~~~
      |                                                                                     resp22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:75:93: error: ‘resp12’ was not declared in this scope; did you mean ‘resp22’?
   75 |   Factory<my_pkg1::MySecondMessage1,my_pkg2::msg::MySecondMessage2>::convert_1_to_2(resp21, resp12);
      |                                                                                             ^~~~~~
      |                                                                                             resp22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:73:10: warning: unused variable ‘resp11’ [-Wunused-variable]
   73 |   auto & resp11 = req1.resp1;
      |          ^~~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2__srv__MySrv2__factories.cpp:74:10: warning: unused variable ‘resp22’ [-Wunused-variable]
   74 |   auto & resp22 = req2.resp2;
      |          ^~~~~~
gmake[2]: *** [CMakeFiles/ros1_bridge.dir/build.make:1161: CMakeFiles/ros1_bridge.dir/generated/my_pkg2__srv__MySrv2__factories.cpp.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
gmake[1]: *** [CMakeFiles/Makefile2:290: CMakeFiles/ros1_bridge.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< ros1_bridge [48.2s, exited with code 2]

when executing:

cd ~/bridge_ws && 
source ~/ros1_noetic_ws/install_isolated/setup.bash && 
source ~/ros1_msgs_ws/install_isolated/setup.bash &&
source ~/ros2_humble_ws/install/setup.bash && 
source ~/ros2_msgs_ws/install/setup.bash && 
echo "add_compile_definitions(BOOST_BIND_GLOBAL_PLACEHOLDERS)" >> ~/bridge_ws/src/ros1_bridge/CMakeLists.txt && 
colcon build --symlink-install --packages-select ros1_bridge --cmake-force-configure
gbiggs commented 1 year ago

I'm not much of a Docker expert (I use a different container system), and am unable to start a shell in your container. Can you please provide instructions beginning from importing the image into Docker?

alexg-k commented 1 year ago

Sure.

  1. docker load < image.tar
  2. docker imagesto find the image ID of the freshly imported image (it should be: 4636eaa77fd7)
  3. docker run -it <imageID> bash
  4. You should have a root shell. Enter the command from above to start the build process.
gbiggs commented 1 year ago

I notice that the interfaces package in the ros2_msgs_ws workspace is named my_pkg2. This package needs to end with _msgs or _interfaces for the bridge to find the messages during the build. Does renaming it fix your problem?

alexg-k commented 1 year ago

At this point I feel like a massive thank you is appropriate for your patience, even before the issue is resolved! So, thanks!

While abstracting and simplifying the messages, I forgot about this requirement. I renamed both packages to my_pkg{1,2}_msgs and compiled again. However, the error remains:

Starting >>> ros1_bridge
--- stderr: ros1_bridge
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp: In member function ‘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 = my_pkg1_msgs::MySrv1; ROS2_T = my_pkg2_msgs::srv::MySrv2; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Request = my_pkg1_msgs::MySrv1Request_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Request = my_pkg2_msgs::srv::MySrv2_Request_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:62:93: error: ‘req21’ was not declared in this scope; did you mean ‘req22’?
   62 |   Factory<my_pkg1_msgs::MyFirstMessage1,my_pkg2_msgs::msg::MyFirstMessage2>::convert_1_to_2(req21, req12);
      |                                                                                             ^~~~~
      |                                                                                             req22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:62:100: error: ‘req12’ was not declared in this scope; did you mean ‘req22’?
   62 |   Factory<my_pkg1_msgs::MyFirstMessage1,my_pkg2_msgs::msg::MyFirstMessage2>::convert_1_to_2(req21, req12);
      |                                                                                                    ^~~~~
      |                                                                                                    req22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:60:10: warning: unused variable ‘req11’ [-Wunused-variable]
   60 |   auto & req11 = req1.req1;
      |          ^~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:61:10: warning: unused variable ‘req22’ [-Wunused-variable]
   61 |   auto & req22 = req2.req2;
      |          ^~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp: In member function ‘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 = my_pkg1_msgs::MySrv1; ROS2_T = my_pkg2_msgs::srv::MySrv2; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS1Response = my_pkg1_msgs::MySrv1Response_<std::allocator<void> >; ros1_bridge::ServiceFactory<ROS1_T, ROS2_T>::ROS2Response = my_pkg2_msgs::srv::MySrv2_Response_<std::allocator<void> >]’:
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:75:95: error: ‘resp21’ was not declared in this scope; did you mean ‘resp22’?
   75 |   Factory<my_pkg1_msgs::MySecondMessage1,my_pkg2_msgs::msg::MySecondMessage2>::convert_1_to_2(resp21, resp12);
      |                                                                                               ^~~~~~
      |                                                                                               resp22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:75:103: error: ‘resp12’ was not declared in this scope; did you mean ‘resp22’?
   75 |   Factory<my_pkg1_msgs::MySecondMessage1,my_pkg2_msgs::msg::MySecondMessage2>::convert_1_to_2(resp21, resp12);
      |                                                                                                       ^~~~~~
      |                                                                                                       resp22
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:73:10: warning: unused variable ‘resp11’ [-Wunused-variable]
   73 |   auto & resp11 = req1.resp1;
      |          ^~~~~~
/root/bridge_ws/build/ros1_bridge/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp:74:10: warning: unused variable ‘resp22’ [-Wunused-variable]
   74 |   auto & resp22 = req2.resp2;
      |          ^~~~~~
gmake[2]: *** [CMakeFiles/ros1_bridge.dir/build.make:1227: CMakeFiles/ros1_bridge.dir/generated/my_pkg2_msgs__srv__MySrv2__factories.cpp.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
gmake[1]: *** [CMakeFiles/Makefile2:290: CMakeFiles/ros1_bridge.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---
Failed   <<< ros1_bridge [48.8s, exited with code 2]
gbiggs commented 1 year ago

At this point I feel like a massive thank you is appropriate for your patience, even before the issue is resolved! So, thanks!

You're welcome! We'll figure this out, sooner or later!

quarkytale commented 1 year ago

Hi @alexg-k, thanks for sharing the docker image! (Though its a very large file, please consider modifying this Dockerfile for faster porting.)

I could reproduce your issue. But I noticed an discrepancy in the ros2_msgs_ws/src/my_pkg2/my_msgs_mapping_rule.yaml, line 20-21:

request_fields_1_to_2:
    MyFirstMessage1: 'MySecondMessage2'

It should be MyFirstMessage1: 'MyFirstMessage2' right?

Still looking into it.

alexg-k commented 1 year ago

Sorry, I took some time off. Yes, @quarkytale, that's correct. The build error remains however.

Sorry also about the large image file. I wanted to use ros2 humble for the bridge and ended up compiling ros1 and ros2 from source.