From a898d73aa8466313793ad04351b9808ed6b4aca2 Mon Sep 17 00:00:00 2001 From: Tamaki Nishino Date: Sun, 14 Jun 2026 18:17:21 +0900 Subject: [PATCH] Isolate TfBuffer internal node from global __node remap The TfBuffer TF listener node was created with default NodeOptions (use_global_arguments == true) and a fixed name "_tf_listener". When a component container is launched with a global node-name remap, e.g. --ros-args -r __node:=my_container every TfBuffer-created listener inherited that remap and was renamed to the container's name. With one listener per TfBuffer-using component this caused duplicate node names and "Publisher already registered for node name" rosout warnings. Give the internal node use_global_arguments(false) so it ignores the CLI __node remap, and derive its name from the host node (_tf_listener) so concurrent listeners don't collide. Add a regression test that launches with a global __node remap and asserts, via the node graph, that the internal node is uniquely named after the host and does not adopt the remapped name. --- rclcpp_async/CMakeLists.txt | 3 + .../include/rclcpp_async/tf_buffer.hpp | 13 ++- .../test/test_tf_buffer_node_name.cpp | 90 +++++++++++++++++++ 3 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 rclcpp_async/test/test_tf_buffer_node_name.cpp diff --git a/rclcpp_async/CMakeLists.txt b/rclcpp_async/CMakeLists.txt index ee6b8fa..e52e787 100644 --- a/rclcpp_async/CMakeLists.txt +++ b/rclcpp_async/CMakeLists.txt @@ -142,6 +142,9 @@ if(BUILD_TESTING) ament_add_ros_isolated_gtest(test_tf_buffer test/test_tf_buffer.cpp) target_link_libraries(test_tf_buffer ${PROJECT_NAME}) + ament_add_ros_isolated_gtest(test_tf_buffer_node_name test/test_tf_buffer_node_name.cpp) + target_link_libraries(test_tf_buffer_node_name ${PROJECT_NAME}) + ament_add_ros_isolated_gtest(test_task_destroy test/test_task_destroy.cpp) target_link_libraries(test_task_destroy ${PROJECT_NAME} ${example_interfaces_TARGETS}) diff --git a/rclcpp_async/include/rclcpp_async/tf_buffer.hpp b/rclcpp_async/include/rclcpp_async/tf_buffer.hpp index 54b5070..3dba84b 100644 --- a/rclcpp_async/include/rclcpp_async/tf_buffer.hpp +++ b/rclcpp_async/include/rclcpp_async/tf_buffer.hpp @@ -43,6 +43,11 @@ namespace rclcpp_async { +// Asynchronous TF2 buffer with a coroutine-friendly lookup_transform(). +// +// Creates an internal listener node named "_tf_listener". That name is +// unique per host node, not per instance, so create at most one TfBuffer per +// host node (CoContext); a second one collides on the node name. class TfBuffer { public: @@ -59,9 +64,11 @@ class TfBuffer explicit TfBuffer(CoContext & ctx) : ctx_(ctx), tf_node_(std::make_shared( - "_tf_listener", ctx.node().get_namespace(), - rclcpp::NodeOptions().start_parameter_services(false).start_parameter_event_publisher( - false))), + std::string(ctx.node().get_name()) + "_tf_listener", ctx.node().get_namespace(), + rclcpp::NodeOptions() + .use_global_arguments(false) + .start_parameter_services(false) + .start_parameter_event_publisher(false))), buffer_(std::make_shared(ctx.node().get_clock())) { auto cb = [this](tf2_msgs::msg::TFMessage::ConstSharedPtr msg) { diff --git a/rclcpp_async/test/test_tf_buffer_node_name.cpp b/rclcpp_async/test/test_tf_buffer_node_name.cpp new file mode 100644 index 0000000..adfa32d --- /dev/null +++ b/rclcpp_async/test/test_tf_buffer_node_name.cpp @@ -0,0 +1,90 @@ +// Copyright 2025 Tamaki Nishino +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Regression test for the TfBuffer internal TF listener node inheriting a +// global `__node` remap. +// +// A component container is launched with e.g. +// --ros-args -r __node:=pfr_nav_container +// which is a *global* node-name remap. Before the fix, every TfBuffer created +// its internal listener with default NodeOptions (use_global_arguments == true) +// and a fixed name "_tf_listener", so the global remap renamed each one to the +// container name. With one such node per TfBuffer-using component, this produced +// duplicate node names and "Publisher already registered for node name" rosout +// warnings. +// +// The fix gives the internal node use_global_arguments(false) (so it ignores the +// CLI `__node` remap) and a name derived from the host node (so concurrent +// listeners do not collide). This test reproduces the global remap and asserts +// both properties via the node graph. + +#include + +#include +#include +#include +#include +#include + +#include + +#include "rclcpp_async/rclcpp_async.hpp" + +using namespace rclcpp_async; // NOLINT(build/namespaces) +using namespace std::chrono_literals; // NOLINT(build/namespaces) + +TEST(TfBufferNodeName, IgnoresGlobalNodeRemapAndIsUnique) +{ + // Simulate a container launched with a global `__node` remap. + const char * argv[] = { + "test_tf_buffer_node_name", "--ros-args", "-r", "__node:=globally_remapped"}; + const int argc = static_cast(sizeof(argv) / sizeof(argv[0])); + rclcpp::init(argc, argv); + + // Host node with a known name, itself shielded from the global remap so its + // name is deterministic for the assertions below. + auto host = std::make_shared( + "host_under_test", rclcpp::NodeOptions().use_global_arguments(false)); + CoContext ctx(*host); + TfBuffer tf_buffer(ctx); + + rclcpp::executors::SingleThreadedExecutor executor; + executor.add_node(host); + + // Let the graph discover the internal TF listener node. + const auto deadline = std::chrono::steady_clock::now() + 5s; + bool found_unique = false; + bool found_remapped = false; + while (std::chrono::steady_clock::now() < deadline) { + executor.spin_some(); + std::this_thread::sleep_for(20ms); + + const std::vector names = host->get_node_names(); + found_unique = + std::find(names.begin(), names.end(), "/host_under_test_tf_listener") != names.end(); + // The internal node must NOT have adopted the global __node remap. + found_remapped = + std::find(names.begin(), names.end(), "/globally_remapped") != names.end(); + if (found_unique) { + break; + } + } + + EXPECT_TRUE(found_unique) + << "internal TF listener node should be uniquely named after the host node"; + EXPECT_FALSE(found_remapped) + << "internal TF listener node must ignore the global __node remap"; + + rclcpp::shutdown(); +}