ros-perception / vision_msgs

Algorithm-agnostic computer vision message types for ROS.
Apache License 2.0
155 stars 74 forks source link

[ros2] ros1_bridge can't map ObjectHypothesis between before-noetic ROS 1 and ROS 2 #39

Open norro opened 3 years ago

norro commented 3 years ago

ros1_bridge can't map ObjectHypothesis between before-noetic ROS 1 and ROS 2 because of two conflicting types in ObjectHypothesis' id field. The field is int64 in until ROS 1 melodic and string in noetic and ROS 2 (thanks @mintar for the clarification). During compilation of ros1_bridge, the automatic mapping attempt will fail with the following error:

.../ros1_bridge/generated/vision_msgs__msg__ObjectHypothesis__factories.cpp: In static member function ‘static void ros1_bridge::Factory<ROS1_T, ROS2_T>::convert_2_to_1(const ROS2_T&, ROS1_T&) [with ROS1_T = vision_msgs::ObjectHypothesis_<std::allocator<void> >; ROS2_T = vision_msgs::msg::ObjectHypothesis_<std::allocator<void> >]’:
.../ros1_bridge/generated/vision_msgs__msg__ObjectHypothesis__factories.cpp:79:26: error: cannot convert ‘const _id_type {aka const std::__cxx11::basic_string<char>}’ to ‘vision_msgs::ObjectHypothesis_<std::allocator<void> >::_id_type {aka long int}’ in assignment
   ros1_msg.id = ros2_msg.id;
                          ^~
make[2]: *** [CMakeFiles/ros1_bridge.dir/generated/vision_msgs__msg__ObjectHypothesis__factories.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
.../ros1_bridge/generated/vision_msgs__msg__ObjectHypothesisWithPose__factories.cpp: In static member function ‘static void ros1_bridge::Factory<ROS1_T, ROS2_T>::convert_2_to_1(const ROS2_T&, ROS1_T&) [with ROS1_T = vision_msgs::ObjectHypothesisWithPose_<std::allocator<void> >; ROS2_T = vision_msgs::msg::ObjectHypothesisWithPose_<std::allocator<void> >]’:
.../ros1_bridge/generated/vision_msgs__msg__ObjectHypothesisWithPose__factories.cpp:87:26: error: cannot convert ‘const _id_type {aka const std::__cxx11::basic_string<char>}’ to ‘vision_msgs::ObjectHypothesisWithPose_<std::allocator<void> >::_id_type {aka long int}’ in assignment
   ros1_msg.id = ros2_msg.id;
                          ^~

This probably requires a custom mapping rule (which is possible for ros1_bridge).

norro commented 3 years ago

Probably relates to this recent commit: https://github.com/Kukanani/vision_msgs/commit/82dda4ab3affb6e7c142054f86c7a897f0b8b41e

mintar commented 3 years ago

To clarify: The field is int64 in ROS1 kinetic and melodic, and string in ROS1 noetic and ROS2.

About the mapping rules: It's easy to convert an int64 into a string, but how would you do the opposite direction?

norro commented 3 years ago

I'd say that in a setup where it bridges the old (int64) world and the new (string) world, it is fair to assume that IDs are (string-ified) numbers.

mintar commented 3 years ago

I'd say that in a setup where it bridges the old (int64) world and the new (string) world, it is fair to assume that IDs are (string-ified) numbers.

Fair enough. The bridge could just try whether the conversion works and throw an exception if the string cannot be converted to an int.

norro commented 3 years ago

Might be a delicate situation, though. Custom mapping rules for the ros1_bridge are supposed to live in the ROS 2 package, IMHO, but if it is required or not is depending on the ROS 1 version in use.

Kukanani commented 3 years ago

Thanks for the bug report!

Unfortunately I don't have the bandwidth to work on this and come up with a good solution right now. I'm happy to review PRs or advocate for a PR on the bridge repo, though. I'm going to create a "help wanted" label and apply it to this issue.

Also, note that while Melodic is not EOL until 2023, the ROS2 distros that also target Ubuntu 18 (Dashing and Eloquent) are EOL in May 2021 and November 2020. So any fix for these distros that involves ros2_bridge fixes would have to come in before then.

kisg commented 2 years ago

PR #57 broke this for noetic as well, because it reverted the id to integer instead of a string and ros1_bridge cannot handle this.