Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
norro opened this issue Sep 23, 2020 · 7 comments

Comments

@norro
Copy link

norro commented Sep 23, 2020

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
Copy link
Author

norro commented Sep 23, 2020

Probably relates to this recent commit: 82dda4a

@mintar
Copy link
Contributor

mintar commented Sep 23, 2020

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
Copy link
Author

norro commented Sep 23, 2020

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.

@norro norro changed the title [ros2] ros1_bridge can't map ObjectHypothesis between ROS 1 and ROS 2 [ros2] ros1_bridge can't map ObjectHypothesis between before-noetic and after-noetic ROS'es Sep 23, 2020
@norro norro changed the title [ros2] ros1_bridge can't map ObjectHypothesis between before-noetic and after-noetic ROS'es [ros2] ros1_bridge can't map ObjectHypothesis between before-noetic ROS 1 and ROS 2 Sep 23, 2020
@mintar
Copy link
Contributor

mintar commented Sep 23, 2020

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
Copy link
Author

norro commented Sep 23, 2020

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
Copy link
Collaborator

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
Copy link

kisg commented Oct 27, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants