From 90630211dd9751bd7c3ee1bf00d8f264c7c5b6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Tue, 28 Nov 2023 07:10:47 +0100 Subject: [PATCH] Fix issue with custom messages (#317) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix issue with custom messages Signed-off-by: Alejandro Hernández Cordero --------- Signed-off-by: Alejandro Hernández Cordero Co-authored-by: Eric Cousineau --- bazel_ros2_rules/ros2/rosidl.bzl | 145 +++++++++++++++++- bazel_ros2_rules/ros2/tools/dload.bzl | 15 ++ ros2_example_bazel_installed/BUILD.bazel | 10 ++ ros2_example_bazel_installed/WORKSPACE | 1 + .../ros2_example_apps/BUILD.bazel | 30 ++-- .../test/custom_message_echo_test.py | 2 +- .../test/custom_message_list_test.py | 2 +- .../test/custom_message_rosbag_test.py | 99 ++++++++++++ .../tools/BUILD.bazel | 22 +++ ros2_example_bazel_installed/tools/ros2.py | 22 +++ 10 files changed, 331 insertions(+), 17 deletions(-) create mode 100644 ros2_example_bazel_installed/ros2_example_apps/test/custom_message_rosbag_test.py create mode 100644 ros2_example_bazel_installed/tools/ros2.py diff --git a/bazel_ros2_rules/ros2/rosidl.bzl b/bazel_ros2_rules/ros2/rosidl.bzl index f017f864..5fb90f11 100644 --- a/bazel_ros2_rules/ros2/rosidl.bzl +++ b/bazel_ros2_rules/ros2/rosidl.bzl @@ -1083,6 +1083,56 @@ def rosidl_typesupport_cc_library( **kwargs ) +def _symlink_typesupport_workaround_issue311_impl(ctx): + filename = paths.join( + ctx.attr.prefix, + "lib" + ctx.attr.pkgname + ".so", + ) + + library = ctx.actions.declare_file(filename) + ctx.actions.symlink( + output = library, + target_file = ctx.executable.executable, + is_executable = True, + ) + + runfiles_symlinks = { + filename: library, + } + + return [ + AmentIndex(prefix = ctx.attr.prefix), + DefaultInfo( + executable = library, + files = depset(direct = [library]), + runfiles = ctx.runfiles(root_symlinks = runfiles_symlinks), + ), + ] + +""" +Symlinks a shared library. + +This is done as a workaround to drake-ros#311, where applications like +`ros2 bag record` may look for typesupport via ${AMENT_PREFIX_PATH}/lib instead +of using ${LD_LIBRARY_PATH}. +""" + +_symlink_typesupport_workaround_issue311 = rule( + _symlink_typesupport_workaround_issue311_impl, + attrs = { + "executable": attr.label( + executable = True, + cfg = "target", + ), + "prefix": attr.string( + default = "rosidl_generate_ament_index_entry/lib", + ), + "pkgname": attr.string(mandatory = True), + }, + executable = True, + doc = "Creates a new target for the given executable.", +) + def rosidl_cc_support( name, interfaces, @@ -1120,6 +1170,7 @@ def rosidl_cc_support( **kwargs ) + data = list(data) typesupports = {} # NOTE: typesupport binary files must not have any leading @@ -1145,6 +1196,20 @@ def rosidl_cc_support( typesupports["rosidl_typesupport_introspection_cpp"] = \ _make_public_label(name, "__rosidl_typesupport_introspection_cpp") + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_introspection_cpp", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_introspection_cpp", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_introspection_cpp", + ), + **kwargs + ) + data += [name + "_symlink_introspection_cpp"] + if "rosidl_typesupport_fastrtps_cpp" in AVAILABLE_TYPESUPPORT_LIST: rosidl_typesupport_fastrtps_cc_library( name = _make_public_name( @@ -1165,6 +1230,20 @@ def rosidl_cc_support( typesupports["rosidl_typesupport_fastrtps_cpp"] = \ _make_public_label(name, "__rosidl_typesupport_fastrtps_cpp") + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_fastrtps_cpp", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_fastrtps_cpp", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_fastrtps_cpp", + ), + **kwargs + ) + data += [name + "_symlink_fastrtps_cpp"] + rosidl_typesupport_cc_library( name = _make_public_name(name, "__rosidl_typesupport_cpp"), typesupports = typesupports, @@ -1179,6 +1258,20 @@ def rosidl_cc_support( **kwargs ) + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_typesupport_cpp", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_cpp", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_cpp", + ), + **kwargs + ) + data += [name + "_symlink_typesupport_cpp"] + cc_library_rule( name = _make_public_name(name, "_cc"), srcs = [ @@ -1229,6 +1322,7 @@ def rosidl_py_support( **kwargs ) + data = list(data) typesupports = {} # NOTE: typesupport binary files must not have any leading @@ -1254,6 +1348,20 @@ def rosidl_py_support( typesupports["rosidl_typesupport_introspection_c"] = \ _make_public_label(name, "__rosidl_typesupport_introspection_c") + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_introspection_c", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_introspection_c", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_introspection_c", + ), + **kwargs + ) + data += [name + "_symlink_introspection_c"] + if "rosidl_typesupport_fastrtps_c" in AVAILABLE_TYPESUPPORT_LIST: rosidl_typesupport_fastrtps_c_library( name = _make_public_name( @@ -1274,6 +1382,20 @@ def rosidl_py_support( typesupports["rosidl_typesupport_fastrtps_c"] = \ _make_public_label(name, "__rosidl_typesupport_fastrtps_c") + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_fastrtps_c", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_fastrtps_c", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_fastrtps_c", + ), + **kwargs + ) + data += [name + "_symlink_fastrtps_c"] + rosidl_typesupport_c_library( name = _make_public_name(name, "__rosidl_typesupport_c"), typesupports = typesupports, @@ -1290,6 +1412,20 @@ def rosidl_py_support( typesupports["rosidl_typesupport_c"] = \ _make_public_label(name, "__rosidl_typesupport_c") + _symlink_typesupport_workaround_issue311( + name = name + "_symlink_typesupport_c", + executable = ":" + _make_public_name( + name, + "__rosidl_typesupport_c", + ), + pkgname = _make_public_name( + name, + "__rosidl_typesupport_c", + ), + **kwargs + ) + data += [name + "_symlink_typesupport_c"] + cc_library_rule( name = _make_public_name(name, "_c"), srcs = typesupports.values(), @@ -1331,8 +1467,11 @@ def rosidl_interfaces_group( """ Generates and builds C++ and Python ROS 2 interfaces. + To depend on IDL definitions, use the `_defs` target. To depend on C++ interfaces, use the `_cc` target. - To depend on Python interfaces, use the `_py` target. + To depend on Python interfaces, use the `_py` target. You should + depend on this target for tools like `ros2 bag record` and + `ros2 topic echo` to work. Args: name: interface group name, used as prefix for target names @@ -1381,11 +1520,13 @@ def rosidl_interfaces_group( cc_library_rule = cc_library_rule, **kwargs ) + cc_name = _make_public_name(name, "_cc") rosidl_py_support( name, interfaces = interfaces, - data = data, + # Add the C++ target so that we can support `ros2 bag record`. + data = data + [cc_name], deps = deps, group = group, cc_binary_rule = cc_binary_rule, diff --git a/bazel_ros2_rules/ros2/tools/dload.bzl b/bazel_ros2_rules/ros2/tools/dload.bzl index a4192268..2d3ea2e2 100644 --- a/bazel_ros2_rules/ros2/tools/dload.bzl +++ b/bazel_ros2_rules/ros2/tools/dload.bzl @@ -61,6 +61,19 @@ def get_dload_shim_attributes(): "env_changes": attr.string_list_dict(), } +def _workaround_issue311(ament_prefixes, env_changes): + # Work around drake-ros#311 and ensure that we have our + # ${AMEND_PREFIX_PATH}/lib libraries also exposed on LD_LIBRARY_PATH. + lib_folders = [] + for prefix in ament_prefixes: + if prefix.endswith("/lib"): + lib_folders.append(prefix) + if len(lib_folders) > 0: + if "LD_LIBRARY_PATH" not in env_changes: + env_changes["LD_LIBRARY_PATH"] = ["path-prepend"] + lib_folders = depset(lib_folders).to_list() + env_changes["LD_LIBRARY_PATH"].extend(lib_folders) + def do_dload_shim(ctx, template, to_list): """ Implements common dload_shim rule functionality. @@ -108,6 +121,8 @@ def do_dload_shim(ctx, template, to_list): ament_prefixes = depset(ament_prefixes).to_list() env_changes["AMENT_PREFIX_PATH"].extend(ament_prefixes) + _workaround_issue311(ament_prefixes, env_changes) + envvars = env_changes.keys() # TODO(eric.cousineau): Should deduplicate entries for path-prepend and diff --git a/ros2_example_bazel_installed/BUILD.bazel b/ros2_example_bazel_installed/BUILD.bazel index 01257a15..29407957 100644 --- a/ros2_example_bazel_installed/BUILD.bazel +++ b/ros2_example_bazel_installed/BUILD.bazel @@ -85,3 +85,13 @@ ros_cc_test( srcs = ["test/use_console_bridge.cc"], deps = ["@ros2//:console_bridge_vendor_cc"], ) + +# Provide a roll-up of all generated IDL types for `//tools:ros2`. +py_library( + name = "ros_msgs_all_py", + visibility = ["__subpackages__"], + deps = [ + "//ros2_example_apps:ros2_example_apps_msgs_py", + "//ros2_example_common:ros2_example_common_msgs_py", + ], +) diff --git a/ros2_example_bazel_installed/WORKSPACE b/ros2_example_bazel_installed/WORKSPACE index b0a663ff..eb32431b 100644 --- a/ros2_example_bazel_installed/WORKSPACE +++ b/ros2_example_bazel_installed/WORKSPACE @@ -72,6 +72,7 @@ ROS2_PACKAGES = [ "rclpy", "ros2cli", "ros2cli_common_extensions", + "rosbag2", "rosidl_default_generators", "tf2_py", ] + [ diff --git a/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel b/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel index c0604769..92183db5 100644 --- a/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel +++ b/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel @@ -12,6 +12,7 @@ rosidl_interfaces_group( interfaces = [ "msg/Status.msg", ], + visibility = ["//:__pkg__"], deps = [ "//ros2_example_common:ros2_example_common_msgs", "@ros2//:builtin_interfaces", @@ -201,6 +202,20 @@ ros_py_binary( ], ) +ros_py_test( + name = "custom_message_rosbag_test", + srcs = ["test/custom_message_rosbag_test.py"], + data = [ + ":simple_talker", + "//tools:ros2", + ], + main = "test/custom_message_rosbag_test.py", + deps = [ + "@bazel_tools//tools/python/runfiles", + "@ros2//resources/rmw_isolation:rmw_isolation_py", + ], +) + # This shows how to ensure a Bazel-provided `ros2` CLI can print out custom # messages (#118). See note below. ros_py_test( @@ -208,14 +223,10 @@ ros_py_test( srcs = ["test/custom_message_echo_test.py"], data = [ ":simple_talker", - "@ros2", + "//tools:ros2", ], main = "test/custom_message_echo_test.py", deps = [ - # This is the main key - you must provide the generated messages as a - # Python dependency. Otherwise, attempting to use `ros2 topic ...` will - # indicate the message type is invalid. - ":ros2_example_apps_msgs_py", "@bazel_tools//tools/python/runfiles", "@ros2//resources/rmw_isolation:rmw_isolation_py", ], @@ -227,14 +238,7 @@ ros_py_test( name = "custom_message_list_test", srcs = ["test/custom_message_list_test.py"], data = [ - # This is the main key. You must provide the generated definitions as - # a dependency. It can come through data such as the C++ (*_cc) or - # Python (*_py) targets as is done here, or directly via the - # definitions targets (*_defs). - # Otherwise, `ros2 interface ...` will indicate the message type is - # invalid. - ":ros2_example_apps_msgs_py", - "@ros2", + "//tools:ros2", ], main = "test/custom_message_list_test.py", deps = [ diff --git a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py index 7867c29c..719baf9b 100644 --- a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py +++ b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py @@ -11,7 +11,7 @@ def main(): os.environ["ROS_HOME"] = os.path.join(os.environ["TEST_TMPDIR"]) manifest = runfiles.Create() - ros2_bin = manifest.Rlocation("ros2/ros2") + ros2_bin = manifest.Rlocation("ros2_example_bazel_installed/tools/ros2") talker_bin = manifest.Rlocation( "ros2_example_bazel_installed/ros2_example_apps/simple_talker") diff --git a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py index 83f6b740..4c3db25d 100644 --- a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py +++ b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py @@ -11,7 +11,7 @@ def main(): os.environ["ROS_HOME"] = os.path.join(os.environ["TEST_TMPDIR"]) manifest = runfiles.Create() - ros2_bin = manifest.Rlocation("ros2/ros2") + ros2_bin = manifest.Rlocation("ros2_example_bazel_installed/tools/ros2") interfaces = subprocess.run( [ros2_bin, "interface", "list"], diff --git a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_rosbag_test.py b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_rosbag_test.py new file mode 100644 index 00000000..fb92fd27 --- /dev/null +++ b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_rosbag_test.py @@ -0,0 +1,99 @@ +""" +To run: + export ROS2_DISTRO_PREFIX=/opt/ros/humble + bazel test --nocache_test_results --test_output=streamed //ros2_example_apps:custom_message_rosbag_test +OR + export ROS2_DISTRO_PREFIX=/opt/ros/humble + bazel build //ros2_example_apps:custom_message_rosbag_test + bazel-bin/ros2_example_apps/custom_message_rosbag_test +WARNING: `bazel run` does not work as expected +""" + +import os +import shutil +import select +import subprocess +import time + +from bazel_tools.tools.python.runfiles import runfiles +from rmw_isolation import isolate_rmw_by_path + + +def read_available(f, timeout=0.0, chunk_size=1024): + """ + Reads all available data on a given file. Useful for using PIPE with Popen. + """ + readable, _, _ = select.select([f], [], [f], timeout) + out = None + if f in readable: + while True: + cur = os.read(f.fileno(), chunk_size) + if out is None: + out = cur + else: + out += cur + if len(cur) < chunk_size: + break + text = out.decode("utf-8") + return text + + +def open_process(args): + return subprocess.Popen( + args, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + stdin=subprocess.PIPE, + text=True, + ) + + +def attempt_record(): + if "TEST_TMPDIR" in os.environ: + tmp_dir = os.environ["TEST_TMPDIR"] + isolate_rmw_by_path(tmp_dir) + os.environ["ROS_HOME"] = os.path.join(tmp_dir) + else: + tmp_dir = "/tmp" + + manifest = runfiles.Create() + ros2_bin = manifest.Rlocation("ros2_example_bazel_installed/tools/ros2") + assert ros2_bin is not None + talker_bin = manifest.Rlocation( + "ros2_example_bazel_installed/ros2_example_apps/simple_talker") + bag_dir = os.path.join(tmp_dir, "test_bag") + if os.path.exists(bag_dir): + shutil.rmtree(bag_dir) + + try: + talker = open_process([talker_bin]) + rosbag = open_process( + [ros2_bin, "bag", "record", "--all", "-o", bag_dir] + ) + + time.sleep(1.0) + + assert talker.returncode is None, read_available(talker.stdout) + assert rosbag.returncode is None, read_available(rosbag.stdout) + + text = read_available(rosbag.stdout) + return text + finally: + talker.kill() + rosbag.kill() + + +def main(): + text = attempt_record() + print(text) + + assert "Recording..." in text + assert "Subscribed to topic '/status'" in text + + # This is the error we're running into. + assert "has unknown type" not in text + assert "Failure in topics discovery" not in text + + +if __name__ == "__main__": + main() diff --git a/ros2_example_bazel_installed/tools/BUILD.bazel b/ros2_example_bazel_installed/tools/BUILD.bazel index 9a5c8f51..6262bb33 100644 --- a/ros2_example_bazel_installed/tools/BUILD.bazel +++ b/ros2_example_bazel_installed/tools/BUILD.bazel @@ -1,3 +1,25 @@ # -*- python -*- exports_files(["cmd_exec.sh"]) + +load( + "@ros2//:ros_py.bzl", + "ros_py_binary", +) + +ros_py_binary( + name = "ros2", + srcs = ["ros2.py"], + data = [ + "@ros2", + ], + visibility = ["//:__subpackages__"], + deps = [ + # This is the main key for tools like `ros2 topic echo` and + # `ros2 bag record` to work with custom messages. You must provide the + # generated definitions as a dependency. Ideally, it should come + # through a "roll-up" target. + "//:ros_msgs_all_py", + "@bazel_tools//tools/python/runfiles", + ], +) diff --git a/ros2_example_bazel_installed/tools/ros2.py b/ros2_example_bazel_installed/tools/ros2.py new file mode 100644 index 00000000..b400a072 --- /dev/null +++ b/ros2_example_bazel_installed/tools/ros2.py @@ -0,0 +1,22 @@ +""" +Bazel-based entry point for `ros2` binary, e.g. + ros2 interfaces list + ros2 topic list + ros2 bag record +""" + +import os +import sys + +from bazel_tools.tools.python.runfiles import runfiles + + +def main(): + manifest = runfiles.Create() + bin_file = manifest.Rlocation("ros2/ros2") + argv = [bin_file] + sys.argv[1:] + os.execv(bin_file, argv) + + +assert __name__ == "__main__" +main()