Add patch to fix typesupport bug in rmw_cyclonedds.

[EvanLiUAV]

As we discuss in #1 ,the problem is solved
Fix this bug by modified rmw_cyclonedds package. the patch is as below based on foxy branch:

diff --git a/rmw_cyclonedds_cpp/src/serdata.cpp b/rmw_cyclonedds_cpp/src/serdata.cpp
index 05b3286..b3382cc 100644
--- a/rmw_cyclonedds_cpp/src/serdata.cpp
+++ b/rmw_cyclonedds_cpp/src/serdata.cpp
@@ -60,14 +60,16 @@ using ResponseTypeSupport_cpp = rmw_cyclonedds_cpp::ResponseTypeSupport<
   rosidl_typesupport_introspection_cpp::ServiceMembers,
   rosidl_typesupport_introspection_cpp::MessageMembers>;
 
-static bool using_introspection_c_typesupport(const char * typesupport_identifier)
-{
-  return typesupport_identifier == rosidl_typesupport_introspection_c__identifier;
+static bool using_introspection_c_typesupport(
+    const char* typesupport_identifier) {
+  return !std::string(typesupport_identifier)
+      .compare(rosidl_typesupport_introspection_c__identifier);
 }
 
-static bool using_introspection_cpp_typesupport(const char * typesupport_identifier)
-{
-  return typesupport_identifier == rosidl_typesupport_introspection_cpp::typesupport_identifier;
+static bool using_introspection_cpp_typesupport(
+    const char* typesupport_identifier) {
+  return !std::string(typesupport_identifier)
+      .compare(rosidl_typesupport_introspection_cpp::typesupport_identifier);
 }
 
 void * create_message_type_support(

The problem is solved when the conditions are correctly determined.

talker

evan@in_docker:~/rules_ros2$ bazel run //examples/chatter:talker 
INFO: Analyzed target //examples/chatter:talker (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //examples/chatter:talker up-to-date:
  bazel-bin/examples/chatter/talker
INFO: Elapsed time: 0.113s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
1623155607.229638 [0]     talker: using network interface eno1 (udp/172.18.20.17) selected arbitrarily from: eno1, docker0
[INFO] [1623155607.736482142] [minimal_publisher]: Publishing: 'Hello, world! 0'
[INFO] [1623155608.236450915] [minimal_publisher]: Publishing: 'Hello, world! 1'
[INFO] [1623155608.736465676] [minimal_publisher]: Publishing: 'Hello, world! 2'
[INFO] [1623155609.236454210] [minimal_publisher]: Publishing: 'Hello, world! 3'
[INFO] [1623155609.736454613] [minimal_publisher]: Publishing: 'Hello, world! 4'
[INFO] [1623155610.236439533] [minimal_publisher]: Publishing: 'Hello, world! 5'
^C[INFO] [1623155610.466806625] [rclcpp]: signal_handler(signal_value=2)

listener

evan@in_docker:~/rules_ros2$ bazel run //examples/chatter:listener 
INFO: Analyzed target //examples/chatter:listener (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //examples/chatter:listener up-to-date:
  bazel-bin/examples/chatter/listener
INFO: Elapsed time: 0.096s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
1623155596.586644 [0]   listener: using network interface eno1 (udp/172.18.20.17) selected arbitrarily from: eno1, docker0
[INFO] [1623155607.736783469] [minimal_subscriber]: I heard: 'Hello, world! 0'
[INFO] [1623155608.236732927] [minimal_subscriber]: I heard: 'Hello, world! 1'
[INFO] [1623155608.736722786] [minimal_subscriber]: I heard: 'Hello, world! 2'
[INFO] [1623155609.236693576] [minimal_subscriber]: I heard: 'Hello, world! 3'
[INFO] [1623155609.736732649] [minimal_subscriber]: I heard: 'Hello, world! 4'
[INFO] [1623155610.236678127] [minimal_subscriber]: I heard: 'Hello, world! 5'
^C[INFO] [1623155631.231753811] [rclcpp]: signal_handler(signal_value=2)

rqt

[mvukov]

This is great! Thank you so much 💯 Looks like you knew where to look for a problem. And I was unlucky a bit to start the journey with a problematic DDS-rmw binding :) Tested this and it works as expected.

I have some minor comments, otherwise LGTM!

Please rename repositories/patches/BUILD to repositories/patches/BUILD.bazel for consistency with the rest of the repo.

[mvukov]

Please add a new line at the end of the file.

[EvanLiUAV]

Fixed.

[mvukov]

@eboasson FYI.

[eboasson]

And I was unlucky a bit to start the journey with a problematic DDS-rmw binding :)

Ouch that hurts! 😉

I just grepped the sources and this pattern is used by all the (DDS-based) RMW implementations, so this fix might be a bit bigger than originally thought. Anyway, would you be willing to open a PR to https://github.com/ros2/rmw_cyclonedds? I'm a little bit surprised it is needed (though I do have some idea why) and I'd like the input from others who have been around for a long time and might know the reason why a pointer comparison was used. If they also approve, it would only be fair that your commit goes into the main repository.

[mvukov]

LGTM!

[mvukov]

@eboasson Thanks for prompt reply. Yeah, it totally makes sense to have this fix in rmw_cyclonedds repo. I can do it when I find some spare cycles, or @EvanLiUAV can do that -- he found the bug and made a fix here. Just for my understanding: is usage of using_introspection_x_typesupport(.) uncommon?

[EvanLiUAV]

And I was unlucky a bit to start the journey with a problematic DDS-rmw binding :)

Ouch that hurts!

I just grepped the sources and this pattern is used by all the (DDS-based) RMW implementations, so this fix might be a bit bigger than originally thought. Anyway, would you be willing to open a PR to https://github.com/ros2/rmw_cyclonedds? I'm a little bit surprised it is needed (though I do have some idea why) and I'd like the input from others who have been around for a long time and might know the reason why a pointer comparison was used. If they also approve, it would only be fair that your commit goes into the main repository.

I will add a pull request to rmw_cyclonedds

[eboasson]

Just for my understanding: is usage of using_introspection_x_typesupport(.) uncommon?

What I see is that the Cyclone, Connext and FastRTPS code all contain a return typesupport_identifier == rosidl_typesupport_introspection_c__identifier somewhere in the code to see if they are dealing with C or C++ when using the introspection type supports. Cyclone always uses that one, the others only if you if you use the ..._dynamic_cpp variant of the RMW implementation. So the visibility of the problem with the other RMW implementations would be less, I suppose.