ros1_bridge icon indicating copy to clipboard operation
ros1_bridge copied to clipboard

This specific mapping results in a build error

Open alexg-k opened this issue 1 year ago • 17 comments

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.

alexg-k avatar Jul 25 '22 06:07 alexg-k

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.

gbiggs avatar Jul 26 '22 13:07 gbiggs

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.

alexg-k avatar Jul 28 '22 11:07 alexg-k

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

gbiggs avatar Aug 18 '22 20:08 gbiggs

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.

alexg-k avatar Aug 23 '22 08:08 alexg-k

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.

gbiggs avatar Aug 23 '22 21:08 gbiggs

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.

alexg-k avatar Aug 24 '22 05:08 alexg-k

Does renaming your messages and fields make it work?

gbiggs avatar Aug 28 '22 22:08 gbiggs

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.

alexg-k avatar Sep 07 '22 09:09 alexg-k

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.

gbiggs avatar Sep 07 '22 23:09 gbiggs

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

alexg-k avatar Sep 09 '22 11:09 alexg-k

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?

gbiggs avatar Sep 13 '22 22:09 gbiggs

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.

alexg-k avatar Sep 15 '22 06:09 alexg-k

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?

gbiggs avatar Sep 22 '22 00:09 gbiggs

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]

alexg-k avatar Sep 28 '22 11:09 alexg-k

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!

gbiggs avatar Sep 28 '22 23:09 gbiggs

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.

quarkytale avatar Sep 30 '22 00:09 quarkytale

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.

alexg-k avatar Oct 19 '22 13:10 alexg-k