Add fastrtps (Fast-DDS) as supported dds middleware #148

henriksod · 2023-07-29T07:13:40Z

As requested in this discussion #145

This is an attempt to add Fast-DDS as a supported dds middleware to this repo. I am using versions compatible with ROS2 Humble.

Added repo dependencies:

foonathan_memory 0.7-3
fastcdr 1.1.0
fastrtps 2.6.2
rmw_fastrtps 6.2.3
rosidl_typesupport_fastrtps 2.2.1

I have added a build flag dds_vendor, which will be cyclonedds by default. It controls a switch in rmw_implementation.BUILD.bazel.
This is how you can set it to use fastdds instead:

# To use FastDDS, invoke Bazel with `--config=fastdds`.
build:fastdds --@com_github_mvukov_rules_ros2//ros2:dds_vendor=fastdds

henriksod · 2023-07-29T08:36:54Z

I tried to fix the typesupport generation in interfaces.bzl, but getting errors of detail__* and time__* files not generated. Typesupport for fastrtps doesn't have those.

mvukov · 2023-07-29T09:06:22Z

repositories/rmw_fastrtps.BUILD.bazel

+)
+
+
+ros2_cpp_binary(


So, fastdds supports two modes per https://fast-dds.docs.eprosima.com/en/latest/fastdds/ros2/ros2.html#ros-2-using-fast-dds-middleware:

rmw_fastrtps_dynamic_cpp that can use introspection-based type-support that we already have. This one should be easy to integrate. cyclonedds rmw uses introspection-based type-support.

rmw_fastrtps_cpp that uses their custom type-support. This is involved as fastdds-specific IDL code generation needs to be implemented.

This is for rclc/rclcpp part, I think. I'm not 100% sure, should be checked, which type-support rclpy expects/can work with.

My take: as a first step, if it suits your needs, just integrate rmw_fastrtps_dynamic_cpp.

If we want to go with rmw_fastrtps_cpp then we will also need to use dds_vendor switch in IDL code-generation, to only generate what's necessary. This might get involved -- haven't studied what generated code fastdds needs.

Please let me know what you think.

I will switch it to dynamic instead. I tried this before, but didn't have improving results. Let's see if I can make it work!

mvukov · 2023-07-29T09:16:10Z

repositories/rmw_implementation.BUILD.bazel

 load("@com_github_mvukov_rules_ros2//ros2:cc_defs.bzl", "ros2_cpp_library")

+string_flag(


Please move this and the two config settings to //ros2/BUILD.bazel. Where this is actually used is an implementation detail from user perspective, IMO.

mvukov · 2023-07-29T09:17:39Z

.github/workflows/main.yml

@@ -45,11 +45,22 @@ jobs:
        toolchain:
          - "gcc"
          - "clang"
+        dds_vendor:


CI changes are OK, but lets leave this for a follow-up. For instance, I have to check how much buildbuddy remote cache we use and not overspend on it.

mvukov · 2023-07-29T09:18:47Z

repositories/rmw_implementation.BUILD.bazel

+    data = select(
+        {
+            ":use_cyclonedds": ["@ros2_rmw_cyclonedds//:rmw_cyclonedds"],
+            ":use_fastdds": ["@ros2_rmw_fastrtps//:rmw_fastrtps"],
+        },
+        no_match_error = "Unsupported dds vendor",
+    ),


henriksod · 2023-07-29T16:39:37Z

Switched to rmw_fastrtps_dynamic_cpp.

Getting a new, a bit more descriptive error:

Executing tests from //ros2/test:generic_publisher_tests
-----------------------------------------------------------------------------
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestGenericPublisher
[ RUN      ] TestGenericPublisher.WhenPublisherDeletedInThread_EnsureCleanNodeCleanup

>>> [rcutils|error_handling.c:108] rcutils_set_error_state()
This error state is being overwritten:

  'Unknown typesupport identifier, at external/ros2_rmw_fastrtps/rmw_fastrtps_dynamic_cpp/src/type_support_registry.cpp:93'

with this new error message:

  'create_publisher() failed to get message_type_support, at external/ros2_rmw_fastrtps/rmw_fastrtps_dynamic_cpp/src/publisher.cpp:181'

rcutils_reset_error() should be called after error handling to avoid this.
<<<
unknown file: Failure
C++ exception with description "failed to initialize rcl node: error not set, at external/ros2_rcl/rcl/src/rcl/publisher.c:116" thrown in the test body.
[  FAILED  ] TestGenericPublisher.WhenPublisherDeletedInThread_EnsureCleanNodeCleanup (5 ms)
[----------] 1 test from TestGenericPublisher (5 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (5 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] TestGenericPublisher.WhenPublisherDeletedInThread_EnsureCleanNodeCleanup

mvukov · 2023-07-29T16:44:30Z

Hi, this is somewhat likely related to #2 -- I suspect you'll need to patch rmw_fastrtps_dynamic_cpp in a similar way we patched rmw_cyclone_dds.

henriksod · 2023-07-29T16:56:40Z

Aha! I actually just came to the exact same conclusion haha 😄

https://github.com/ros2/rmw_fastrtps/blob/b45e73500bc0a2056fdc31197a6f94434b327d55/rmw_fastrtps_dynamic_cpp/src/type_support_common.cpp#L32

== does not work for c strings. It passed when I used strcmp instead!

We should raise issues in the corresponding repos, if not done already.

henriksod · 2023-07-29T17:56:26Z

@mvukov tests are passing and chatter example ran with fastdds 🚀

mvukov

When I run tests, I get errors related to

==================== Test output for //ros2/test:test_rclcpp:
terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  failed to initialize rcl init options: failed to load shared library 'external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so' due to dlopen error: external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so: undefined symbol: shm_open, at external/ros2_rcutils/src/shared_library.c:99, at external/ros2_rmw_implementation/rmw_implementation/src/functions.cpp:88, at external/ros2_rcl/rcl/src/rcl/init_options.c:75

Please check what shm_open expects and document it. Perhaps add SHM under a flag, like we do for iceoryx.

You should not manually edit repositories/ros2_repositories_impl.bzl, but edit repositories/ros2_repo_mappings.yaml and then run bazel run //repositories/private:resolver (if all up-to-date git diff must be empty for this the _impl.bzl file). If needed, you can override a package version from the yaml file.

mvukov · 2023-07-30T08:21:01Z

repositories/fastcdr.BUILD.bazel

+    "EPROSIMA_INSTALLER": "OFF",
+    "EPROSIMA_BUILD": "OFF",
+    "EPROSIMA_BUILD_TESTS": "OFF",
+    "APPEND_PROJECT_NAME_TO_INCLUDEDIR": "OFF",
+    "BUILD_DOCUMENTATION": "OFF",
+    "CHECK_DOCUMENTATION": "OFF",


Please alph-sort by keys.

mvukov · 2023-07-30T08:21:12Z

repositories/fastrtps.BUILD.bazel

+    "SECURITY": "OFF",
+    "NO_TLS": "ON",
+    "SHM_TRANSPORT_DEFAULT": "ON",
+    "COMPILE_TOOLS": "OFF",
+    "INSTALL_TOOLS": "OFF",
+    "FASTDDS_STATISTICS": "OFF",
+    "COMPILE_EXAMPLES": "OFF",
+    "INSTALL_EXAMPLES": "OFF",
+    "BUILD_DOCUMENTATION": "OFF",
+    "CHECK_DOCUMENTATION": "OFF",


Please alph-sort by keys.

mvukov · 2023-07-30T08:22:34Z

repositories/patches/rmw_fastrtps-fix-typesupport-conditions-bug.patch

+ {
+-  return typesupport_identifier == rosidl_typesupport_introspection_c__identifier;
+  return !std::string(typesupport_identifier)
+         .compare(rosidl_typesupport_introspection_c__identifier);


Would std::strcmp work if you would cast rosidl_typesupport_introspection_c__identifier to char*?

strcmp works, I just wanted to use the same method as the patch for cyclonedds to be consistent

As pointed out in ros2/rmw_cyclonedds#320, std::strcmp is a bit of an optimization and will avoid an extra mem. alloc. (Eventually we should also refactor the rwm_cyclonedds patch, but that's for a follow-up.)

mvukov · 2023-07-30T08:23:07Z

repositories/rmw_fastrtps.BUILD.bazel

+
+ros2_cpp_library(
+    name = "rmw_fastrtps_shared_cpp",
+    srcs = glob([


Please split into hdrs and srcs.

I think header files in src belong in srcs. They don't look like they're intended for downstream rules to #include directly.

repositories/rmw_fastrtps.BUILD.bazel

mvukov · 2023-07-30T08:27:25Z

ros2/BUILD.bazel

@@ -64,3 +66,25 @@ py_binary(
        "@ros2cli//:ros2lifecycle",
    ],
 )
+
+# DDS vendor settings


Obvious comments, please rm.

mvukov · 2023-07-30T08:27:44Z

ros2/BUILD.bazel

+
+config_setting(
+    name = "use_fastdds",
+    flag_values = {":dds_vendor": "fastdds"},


Should be fastdds_dynamic IMO.

henriksod · 2023-07-30T14:39:48Z

When I run tests, I get errors related to

==================== Test output for //ros2/test:test_rclcpp:
terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  failed to initialize rcl init options: failed to load shared library 'external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so' due to dlopen error: external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so: undefined symbol: shm_open, at external/ros2_rcutils/src/shared_library.c:99, at external/ros2_rmw_implementation/rmw_implementation/src/functions.cpp:88, at external/ros2_rcl/rcl/src/rcl/init_options.c:75

Please check what shm_open expects and document it. Perhaps add SHM under a flag, like we do for iceoryx.

Strange, I do not get this error when running the tests. I will investigate a bit.

henriksod · 2023-07-30T15:46:24Z

You should not manually edit repositories/ros2_repositories_impl.bzl, but edit repositories/ros2_repo_mappings.yaml and then run bazel run //repositories/private:resolver (if all up-to-date git diff must be empty for this the _impl.bzl file). If needed, you can override a package version from the yaml file.

I ran this command and then the urls were removed from the http_archive rules.

henriksod · 2023-07-30T15:47:16Z

@mvukov fixed comments

bazelisk test //ros2/test:test_rclcpp --config=fastdds --@fastrtps//:enable_shm=True passes for me.

mvukov · 2023-07-30T19:56:33Z

You should not manually edit repositories/ros2_repositories_impl.bzl, but edit repositories/ros2_repo_mappings.yaml and then run bazel run //repositories/private:resolver (if all up-to-date git diff must be empty for this the _impl.bzl file). If needed, you can override a package version from the yaml file.

I ran this command and then the urls were removed from the http_archive rules.

OK, I will investigate this.

henriksod · 2023-07-30T21:10:57Z

Strange, I am getting the same error as you when running a regular listener node with fastdds active:

[micro_ros_listener_example_task-2] terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
[micro_ros_listener_example_task-2]   what():  failed to initialize rcl init options: failed to load shared library 
'external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so' due to dlopen error: 
external/ros2_rmw_fastrtps/librmw_fastrtps_dynamic_cpp.so: undefined symbol: shm_open, at 
external/ros2_rcutils/src/shared_library.c:99, at 
external/ros2_rmw_implementation/rmw_implementation/src/functions.cpp:88, at 
external/ros2_rcl/rcl/src/rcl/init_options.c:75

It works when enabling shared memory transport, but not without...

henriksod · 2023-08-01T08:32:43Z

@mvukov I have learned that we should link all binaries and tests (using e.g rosidl_typesupport_introspection_c and std_msgs, etc.) dynamically, see ros2/rmw_fastrtps#703.

This is because some pointers are shared between libraries and static linking creates separate object files which breaks this:
ros2/rmw_fastrtps#703 (comment)

linking dynamically means we do not need to patch this for example:
https://github.com/mvukov/rules_ros2/blob/main/repositories/patches/rmw_cyclonedds-fix-typesupport-conditions-bug.patch

henriksod · 2023-08-01T10:06:09Z

Okay so this works

ros2_cpp_test(
    name = "generic_publisher_tests",
    srcs = ["generic_publisher_tests.cc"],
    linkstatic = False,
    idl_deps = [
        "@ros2_common_interfaces//:std_msgs",
    ],
    deps = [
        "@com_google_googletest//:gtest",
        "@ros2_rclcpp//:rclcpp",
    ],
)

since @ros2_common_interfaces//:std_msgs is loaded as a plugin.

However, it still doesnt work when @ros2_common_interfaces//:std_msgs is used as a cpp library for a node, e.g.:

ros2_cpp_binary(
    name = "publisher",
    srcs = ["publisher.cc"],
    linkstatic = False,
    deps = [
        "@ros2_common_interfaces//:cpp_std_msgs",
        "@ros2_rclcpp//:rclcpp",
    ],
)

I believe something is not correct with interface generation

henriksod · 2023-08-01T12:53:59Z

Okay so this is strange,

When I run bazelisk run //chatter:chatter --config=fastdds without sestting linkstatic = False, it throws the same error about

'Unknown typesupport identifier, at external/ros2_rmw_fastrtps/rmw_fastrtps_dynamic_cpp/src/type_support_registry.cpp:93'

But if I do set linkstatic = False for both talker and listener, it works!

However, the test does not work, it throws the same error despite linkstatic = False:
bazelisk test //chatter:tests --config=fastdds

Could it be the extra dependency "@ros2_common_interfaces//:py_std_msgs",?

As a result from this issue: ros2/rmw_fastrtps#700

… as a dependency externally

… always ON

…fastrtps

mvukov · 2023-08-01T20:35:06Z

I'm afraid linkstatic = False won't be sufficient, but, you can try out whether mechanism like in https://github.com/mvukov/rules_ros2/blob/main/repositories/class_loader.BUILD.bazel#L6 would work for rosidl_typesupport_introspection_*.

Interesting discussion in ros2/rmw_fastrtps#703. FWIW, my take: relying on dynamic linking for correction operation of the code if fragile. Are there other places where == -> strcmp could be applied?

mvukov · 2023-09-01T05:11:54Z

@henriksod Any plans to finish this up?

henriksod · 2023-09-01T05:16:39Z

Sorry I haven't had time. Best to put it on hold if no one else wants to take it up.

brians-neptune · 2024-02-29T20:40:02Z

I'm going to try finishing this up. I'm going to respond to comments in this PR, but I don't think I can push to henriksod's branch so I'll open a new PR from my fork.

I'm planning to use the strcmp patch approach instead of #185 to avoid blocking this on the discussion there.

mvukov · 2024-04-09T19:57:49Z

Now that #299 is merge I hope adding fastrtps support will be way easier.

henriksod force-pushed the main branch 2 times, most recently from dda45f1 to 49ce3de Compare July 29, 2023 07:24

mvukov reviewed Jul 29, 2023

View reviewed changes

henriksod mentioned this pull request Jul 29, 2023

String comparison with equal-to operator does not work for c strings, in type_support_common.cpp ros2/rmw_fastrtps#703

Closed

henriksod force-pushed the main branch from 071ccd8 to 47ca620 Compare July 29, 2023 17:49

henriksod force-pushed the main branch from d996c79 to 0ef0327 Compare July 29, 2023 18:40

mvukov reviewed Jul 30, 2023

View reviewed changes

henriksod and others added 10 commits August 1, 2023 14:57

Attempt to add fastrtps as dds vendor alternative

0b383a7

Update main.yml

5299e28

Changed Fast-DDS version from 2.7.1 to 2.6.2

66b2d4a

As a result from this issue: ros2/rmw_fastrtps#700

Removed changes to rclcpp.BUILD.bazel

aeb70e3

Switched from rmw_fastrtps_cpp to rmw_fastrtps_dynamic_cpp

4e339eb

Patched rmw_fastrtps so string comparison is correct

0f5c362

Ran pre-commit hooks

72819cd

removed dependency openssl from fastrtps since it is not used

7a4246e

Making rmw_fastrtps_shared_cpp visibility public since it can be used…

36602a1

… as a dependency externally

Activate shared memory transport for fastrtps

631b597

henriksod added 4 commits August 1, 2023 14:57

Fix shared memory for fastrtps

0720bca

Fixed mistake in fastrtps.BUILD.bazel where SHM_TRANSPORT_DEFAULT was…

bdecf5d

… always ON

make rosidl_typesupporte_introspection_cXX shared libraries

30186fa

making //ros2/test:generic_publisher_tests pass without patching rmw_…

502d125

…fastrtps

henriksod force-pushed the main branch from 74256d6 to 502d125 Compare August 1, 2023 12:57

henriksod mentioned this pull request Aug 1, 2023

Support cross compilation to aarch64 #151

Open

mvukov mentioned this pull request Nov 29, 2023

Refactor/exported symbols #185

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fastrtps (Fast-DDS) as supported dds middleware #148

Add fastrtps (Fast-DDS) as supported dds middleware #148

henriksod commented Jul 29, 2023 •

edited

Loading

henriksod commented Jul 29, 2023

mvukov Jul 29, 2023

henriksod Jul 29, 2023

mvukov Jul 29, 2023

mvukov Jul 29, 2023

mvukov Jul 29, 2023

henriksod commented Jul 29, 2023

mvukov commented Jul 29, 2023

henriksod commented Jul 29, 2023 •

edited

Loading

henriksod commented Jul 29, 2023

mvukov left a comment

mvukov Jul 30, 2023

mvukov Jul 30, 2023

mvukov Jul 30, 2023

henriksod Jul 30, 2023

mvukov Jul 30, 2023

mvukov Jul 30, 2023

brians-neptune Feb 29, 2024

mvukov Jul 30, 2023

mvukov Jul 30, 2023

henriksod commented Jul 30, 2023

henriksod commented Jul 30, 2023

henriksod commented Jul 30, 2023 •

edited

Loading

mvukov commented Jul 30, 2023

henriksod commented Jul 30, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading

mvukov commented Aug 1, 2023

mvukov commented Sep 1, 2023

henriksod commented Sep 1, 2023

brians-neptune commented Feb 29, 2024

mvukov commented Apr 9, 2024

		load("@com_github_mvukov_rules_ros2//ros2:cc_defs.bzl", "ros2_cpp_library")

		string_flag(

Add fastrtps (Fast-DDS) as supported dds middleware #148

Are you sure you want to change the base?

Add fastrtps (Fast-DDS) as supported dds middleware #148

Conversation

henriksod commented Jul 29, 2023 • edited Loading

henriksod commented Jul 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henriksod commented Jul 29, 2023

mvukov commented Jul 29, 2023

henriksod commented Jul 29, 2023 • edited Loading

henriksod commented Jul 29, 2023

mvukov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henriksod commented Jul 30, 2023

henriksod commented Jul 30, 2023

henriksod commented Jul 30, 2023 • edited Loading

mvukov commented Jul 30, 2023

henriksod commented Jul 30, 2023 • edited Loading

henriksod commented Aug 1, 2023 • edited Loading

henriksod commented Aug 1, 2023 • edited Loading

henriksod commented Aug 1, 2023 • edited Loading

mvukov commented Aug 1, 2023

mvukov commented Sep 1, 2023

henriksod commented Sep 1, 2023

brians-neptune commented Feb 29, 2024

mvukov commented Apr 9, 2024

henriksod commented Jul 29, 2023 •

edited

Loading

henriksod commented Jul 29, 2023 •

edited

Loading

henriksod commented Jul 30, 2023 •

edited

Loading

henriksod commented Jul 30, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading

henriksod commented Aug 1, 2023 •

edited

Loading