From e2f2bbbaf85441f05322a8dd82ed23c61d16c1b5 Mon Sep 17 00:00:00 2001 From: kuralme Date: Sun, 9 Nov 2025 10:38:10 -0500 Subject: [PATCH 1/6] tf prefix helper utilized - control toolbox pkg already linked --- .../src/diff_drive_controller.cpp | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/diff_drive_controller/src/diff_drive_controller.cpp b/diff_drive_controller/src/diff_drive_controller.cpp index 4057121895..046970b6a2 100644 --- a/diff_drive_controller/src/diff_drive_controller.cpp +++ b/diff_drive_controller/src/diff_drive_controller.cpp @@ -22,6 +22,7 @@ #include #include +#include "control_toolbox/tf_utils.hpp" #include "diff_drive_controller/diff_drive_controller.hpp" #include "hardware_interface/types/hardware_interface_type_values.hpp" #include "lifecycle_msgs/msg/state.hpp" @@ -411,32 +412,17 @@ controller_interface::CallbackReturn DiffDriveController::on_configure( std::make_shared>( odometry_publisher_); - // Append the tf prefix if there is one - std::string tf_prefix = ""; - if (params_.tf_frame_prefix_enable) - { - if (params_.tf_frame_prefix != "") - { - tf_prefix = params_.tf_frame_prefix; - } - else - { - tf_prefix = std::string(get_node()->get_namespace()); - } - - // Make sure prefix does not start with '/' and always ends with '/' - if (tf_prefix.back() != '/') - { - tf_prefix = tf_prefix + "/"; - } - if (tf_prefix.front() == '/') - { - tf_prefix.erase(0, 1); - } - } - - const auto odom_frame_id = tf_prefix + params_.odom_frame_id; - const auto base_frame_id = tf_prefix + params_.base_frame_id; + // apply the TF prefix if not empty, otherwise use the node namespace + const auto odom_frame_id = + params_.tf_frame_prefix_enable + ? control_toolbox::apply_tf_prefix( + params_.tf_frame_prefix, get_node()->get_namespace(), params_.odom_frame_id) + : params_.odom_frame_id; + const auto base_frame_id = + params_.tf_frame_prefix_enable + ? control_toolbox::apply_tf_prefix( + params_.tf_frame_prefix, get_node()->get_namespace(), params_.base_frame_id) + : params_.base_frame_id; odometry_message_.header.frame_id = odom_frame_id; odometry_message_.child_frame_id = base_frame_id; From 0f564ffa7dbffd19deb2e18ef03d6480f4e3c31c Mon Sep 17 00:00:00 2001 From: kuralme Date: Sun, 9 Nov 2025 10:40:44 -0500 Subject: [PATCH 2/6] TEMP: my fork used for CI test - this commit is to be deleted later --- ros2_controllers.rolling.repos | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ros2_controllers.rolling.repos b/ros2_controllers.rolling.repos index 067271620f..86539a0bc4 100644 --- a/ros2_controllers.rolling.repos +++ b/ros2_controllers.rolling.repos @@ -13,8 +13,8 @@ repositories: version: master control_toolbox: type: git - url: https://github.com/ros-controls/control_toolbox.git - version: master + url: https://github.com/kuralme/control_toolbox.git + version: tf_prefix_helper/diff_drive kinematics_interface: type: git url: https://github.com/ros-controls/kinematics_interface.git From e683c9830792911ccc3d0d8a5ab5ea953c478bf4 Mon Sep 17 00:00:00 2001 From: kuralme Date: Thu, 13 Nov 2025 11:42:07 -0500 Subject: [PATCH 3/6] tf prefix helper update - enable flag removed from args - tilde substitution with ns added - tests updated - verbose comments --- .../src/diff_drive_controller.cpp | 20 ++--- .../test/test_diff_drive_controller.cpp | 79 +++---------------- 2 files changed, 21 insertions(+), 78 deletions(-) diff --git a/diff_drive_controller/src/diff_drive_controller.cpp b/diff_drive_controller/src/diff_drive_controller.cpp index 046970b6a2..e9ab9d3855 100644 --- a/diff_drive_controller/src/diff_drive_controller.cpp +++ b/diff_drive_controller/src/diff_drive_controller.cpp @@ -22,7 +22,7 @@ #include #include -#include "control_toolbox/tf_utils.hpp" +#include "controller_interface/helpers.hpp" #include "diff_drive_controller/diff_drive_controller.hpp" #include "hardware_interface/types/hardware_interface_type_values.hpp" #include "lifecycle_msgs/msg/state.hpp" @@ -412,17 +412,13 @@ controller_interface::CallbackReturn DiffDriveController::on_configure( std::make_shared>( odometry_publisher_); - // apply the TF prefix if not empty, otherwise use the node namespace - const auto odom_frame_id = - params_.tf_frame_prefix_enable - ? control_toolbox::apply_tf_prefix( - params_.tf_frame_prefix, get_node()->get_namespace(), params_.odom_frame_id) - : params_.odom_frame_id; - const auto base_frame_id = - params_.tf_frame_prefix_enable - ? control_toolbox::apply_tf_prefix( - params_.tf_frame_prefix, get_node()->get_namespace(), params_.base_frame_id) - : params_.base_frame_id; + // resolve prefix: substitute tilde (~) with the namespace if contains and normalize slashes (/) + const auto tf_prefix = + controller_interface::resolve_tf_prefix(params_.tf_frame_prefix, get_node()->get_namespace()); + + // prepend resolved TF prefix to frame ids + const auto odom_frame_id = tf_prefix + params_.odom_frame_id; + const auto base_frame_id = tf_prefix + params_.base_frame_id; odometry_message_.header.frame_id = odom_frame_id; odometry_message_.child_frame_id = base_frame_id; diff --git a/diff_drive_controller/test/test_diff_drive_controller.cpp b/diff_drive_controller/test/test_diff_drive_controller.cpp index 49a7493e42..6d5d86487a 100644 --- a/diff_drive_controller/test/test_diff_drive_controller.cpp +++ b/diff_drive_controller/test/test_diff_drive_controller.cpp @@ -74,12 +74,10 @@ class TestableDiffDriveController : public diff_drive_controller::DiffDriveContr return realtime_odometry_publisher_; } // Declare these tests as friends so we can access odometry_message_ - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_test_prefix_false_no_namespace); - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_no_namespace); - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_no_namespace); - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_test_prefix_false_set_namespace); - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_set_namespace); - FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_set_namespace); + FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_prefix_no_namespace); + FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_blank_prefix_no_namespace); + FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_prefix_set_namespace); + FRIEND_TEST(TestDiffDriveController, configure_succeeds_tf_tilde_prefix_set_namespace); // Declare these tests as friends so we can access controller_->reference_interfaces_ FRIEND_TEST(TestDiffDriveController, chainable_controller_unchained_mode); FRIEND_TEST(TestDiffDriveController, chainable_controller_chained_mode); @@ -310,29 +308,7 @@ TEST_F( EXPECT_EQ(cmd_if_conf.type, controller_interface::interface_configuration_type::INDIVIDUAL); } -TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_false_no_namespace) -{ - std::string odom_id = "odom"; - std::string base_link_id = "base_link"; - std::string frame_prefix = "test_prefix"; - - ASSERT_EQ( - InitController( - left_wheel_names, right_wheel_names, - {rclcpp::Parameter("tf_frame_prefix_enable", rclcpp::ParameterValue(false)), - rclcpp::Parameter("tf_frame_prefix", rclcpp::ParameterValue(frame_prefix)), - rclcpp::Parameter("odom_frame_id", rclcpp::ParameterValue(odom_id)), - rclcpp::Parameter("base_frame_id", rclcpp::ParameterValue(base_link_id))}), - controller_interface::return_type::OK); - - ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); - - /* tf_frame_prefix_enable is false so no modifications to the frame id's */ - ASSERT_EQ(controller_->odometry_message_.header.frame_id, odom_id); - ASSERT_EQ(controller_->odometry_message_.child_frame_id, base_link_id); -} - -TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_no_namespace) +TEST_F(TestDiffDriveController, configure_succeeds_tf_prefix_no_namespace) { std::string odom_id = "odom"; std::string base_link_id = "base_link"; @@ -349,13 +325,12 @@ TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_no_namesp ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); - /* tf_frame_prefix_enable is true and frame_prefix is not blank so should be appended to the frame - * id's */ + // frame_prefix is not blank so should be prepended to the frame id's ASSERT_EQ(controller_->odometry_message_.header.frame_id, frame_prefix + "/" + odom_id); ASSERT_EQ(controller_->odometry_message_.child_frame_id, frame_prefix + "/" + base_link_id); } -TEST_F(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_no_namespace) +TEST_F(TestDiffDriveController, configure_succeeds_tf_blank_prefix_no_namespace) { std::string odom_id = "odom"; std::string base_link_id = "base_link"; @@ -372,38 +347,12 @@ TEST_F(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_no_names ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); - /* tf_frame_prefix_enable is true but frame_prefix is blank so should not be appended to the frame - * id's */ - ASSERT_EQ(controller_->odometry_message_.header.frame_id, odom_id); - ASSERT_EQ(controller_->odometry_message_.child_frame_id, base_link_id); -} - -TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_false_set_namespace) -{ - std::string test_namespace = "/test_namespace"; - - std::string odom_id = "odom"; - std::string base_link_id = "base_link"; - std::string frame_prefix = "test_prefix"; - - ASSERT_EQ( - InitController( - left_wheel_names, right_wheel_names, - {rclcpp::Parameter("tf_frame_prefix_enable", rclcpp::ParameterValue(false)), - rclcpp::Parameter("tf_frame_prefix", rclcpp::ParameterValue(frame_prefix)), - rclcpp::Parameter("odom_frame_id", rclcpp::ParameterValue(odom_id)), - rclcpp::Parameter("base_frame_id", rclcpp::ParameterValue(base_link_id))}, - test_namespace), - controller_interface::return_type::OK); - - ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); - - /* tf_frame_prefix_enable is false so no modifications to the frame id's */ + // frame_prefix is blank so nothing added to the frame id's ASSERT_EQ(controller_->odometry_message_.header.frame_id, odom_id); ASSERT_EQ(controller_->odometry_message_.child_frame_id, base_link_id); } -TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_set_namespace) +TEST_F(TestDiffDriveController, configure_succeeds_tf_prefix_set_namespace) { std::string test_namespace = "/test_namespace"; @@ -423,18 +372,17 @@ TEST_F(TestDiffDriveController, configure_succeeds_tf_test_prefix_true_set_names ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); - /* tf_frame_prefix_enable is true and frame_prefix is not blank so should be appended to the frame - * id's instead of the namespace*/ + // frame_prefix is not blank so should be prepended to the frame id's instead of the namespace ASSERT_EQ(controller_->odometry_message_.header.frame_id, frame_prefix + "/" + odom_id); ASSERT_EQ(controller_->odometry_message_.child_frame_id, frame_prefix + "/" + base_link_id); } -TEST_F(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_set_namespace) +TEST_F(TestDiffDriveController, configure_succeeds_tf_tilde_prefix_set_namespace) { std::string test_namespace = "/test_namespace"; std::string odom_id = "odom"; std::string base_link_id = "base_link"; - std::string frame_prefix = ""; + std::string frame_prefix = "~"; ASSERT_EQ( InitController( @@ -448,9 +396,8 @@ TEST_F(TestDiffDriveController, configure_succeeds_tf_blank_prefix_true_set_name ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), CallbackReturn::SUCCESS); + // frame_prefix has tilde (~) character so node namespace should be prepended to the frame id's std::string ns_prefix = test_namespace.erase(0, 1) + "/"; - /* tf_frame_prefix_enable is true but frame_prefix is blank so namespace should be appended to the - * frame id's */ ASSERT_EQ(controller_->odometry_message_.header.frame_id, ns_prefix + odom_id); ASSERT_EQ(controller_->odometry_message_.child_frame_id, ns_prefix + base_link_id); } From 0e253321f91b6fcf92be24538ee8f33e69e1a133 Mon Sep 17 00:00:00 2001 From: kuralme Date: Thu, 13 Nov 2025 11:48:57 -0500 Subject: [PATCH 4/6] temp commit: used my fork for CI --- ros2_controllers.rolling.repos | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ros2_controllers.rolling.repos b/ros2_controllers.rolling.repos index 86539a0bc4..c2342b18d2 100644 --- a/ros2_controllers.rolling.repos +++ b/ros2_controllers.rolling.repos @@ -1,8 +1,8 @@ repositories: ros2_control: type: git - url: https://github.com/ros-controls/ros2_control.git - version: master + url: https://github.com/kuralme/ros2_control.git + version: tf_prefix_helper realtime_tools: type: git url: https://github.com/ros-controls/realtime_tools.git @@ -13,8 +13,8 @@ repositories: version: master control_toolbox: type: git - url: https://github.com/kuralme/control_toolbox.git - version: tf_prefix_helper/diff_drive + url: https://github.com/ros-controls/control_toolbox.git + version: master kinematics_interface: type: git url: https://github.com/ros-controls/kinematics_interface.git From 55ac4411b08d075643dd899f61842c231807b540 Mon Sep 17 00:00:00 2001 From: kuralme Date: Sat, 15 Nov 2025 11:09:59 -0500 Subject: [PATCH 5/6] deprecated tf prefix flag - log message added if tf_frame_prefix_enable is not set default - relevant info added to yaml description, migration and release notes --- diff_drive_controller/src/diff_drive_controller.cpp | 11 +++++++++++ .../src/diff_drive_controller_parameter.yaml | 2 +- doc/migration.rst | 5 +++++ doc/release_notes.rst | 5 +++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/diff_drive_controller/src/diff_drive_controller.cpp b/diff_drive_controller/src/diff_drive_controller.cpp index e9ab9d3855..cd0293d863 100644 --- a/diff_drive_controller/src/diff_drive_controller.cpp +++ b/diff_drive_controller/src/diff_drive_controller.cpp @@ -405,6 +405,17 @@ controller_interface::CallbackReturn DiffDriveController::on_configure( } }); + // deprecation warning if tf_frame_prefix_enable set to non-default value + const bool default_tf_frame_prefix_enable = true; + if (params_.tf_frame_prefix_enable != default_tf_frame_prefix_enable) + { + RCLCPP_WARN( + get_node()->get_logger(), + "Parameter 'tf_frame_prefix_enable' is DEPRECATED and set to a non-default value (%s). " + "Please migrate to 'tf_frame_prefix'.", + params_.tf_frame_prefix_enable ? "true" : "false"); + } + // initialize odometry publisher and message odometry_publisher_ = get_node()->create_publisher( DEFAULT_ODOMETRY_TOPIC, rclcpp::SystemDefaultsQoS()); diff --git a/diff_drive_controller/src/diff_drive_controller_parameter.yaml b/diff_drive_controller/src/diff_drive_controller_parameter.yaml index 1fc8b66e88..81e8bc645b 100644 --- a/diff_drive_controller/src/diff_drive_controller_parameter.yaml +++ b/diff_drive_controller/src/diff_drive_controller_parameter.yaml @@ -49,7 +49,7 @@ diff_drive_controller: tf_frame_prefix_enable: { type: bool, default_value: true, - description: "Enables or disables appending tf_prefix to tf frame id's.", + description: "Deprecated: this parameter will be removed in a future release. Use 'tf_frame_prefix' instead.", } tf_frame_prefix: { type: string, diff --git a/doc/migration.rst b/doc/migration.rst index 6b0875f73d..b21e52fa89 100644 --- a/doc/migration.rst +++ b/doc/migration.rst @@ -4,3 +4,8 @@ Migration Guides: Kilted Kaiju to Lyrical Luth ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This list summarizes important changes between Kilted Kaiju (previous) and Lyrical Luth (current) releases, where changes to user code might be necessary. + +diff_drive_controller +***************************** +* Remove unused parameter ``tf_frame_prefix_enable``, use ``tf_frame_prefix`` instead. (`#1997 `_). +* Now any tilde ("~") character in ``tf_frame_prefix`` is substituted with node namespace. (`#1997 `_). diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 8622620620..d06d411977 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -4,3 +4,8 @@ Release Notes: Kilted Kaiju to Lyrical Luth ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This list summarizes important changes between Kilted Kaiju (previous) and Lyrical Luth (current) releases. + +diff_drive_controller +***************************** +* Remove unused parameter ``tf_frame_prefix_enable``, use ``tf_frame_prefix`` instead. (`#1997 `_). +* Now any tilde ("~") character in ``tf_frame_prefix`` is substituted with node namespace. (`#1997 `_). From 05a578b3ca98c3324511b89244a41d778e38d3a5 Mon Sep 17 00:00:00 2001 From: kuralme Date: Sun, 16 Nov 2025 18:37:25 -0500 Subject: [PATCH 6/6] deprecation notes updated --- doc/migration.rst | 4 ++-- doc/release_notes.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/migration.rst b/doc/migration.rst index b21e52fa89..391e96859a 100644 --- a/doc/migration.rst +++ b/doc/migration.rst @@ -7,5 +7,5 @@ This list summarizes important changes between Kilted Kaiju (previous) and Lyric diff_drive_controller ***************************** -* Remove unused parameter ``tf_frame_prefix_enable``, use ``tf_frame_prefix`` instead. (`#1997 `_). -* Now any tilde ("~") character in ``tf_frame_prefix`` is substituted with node namespace. (`#1997 `_). +* Instead of using ``tf_frame_prefix_enable:=false``, set an empty ``tf_frame_prefix:=""`` parameter instead. (`#1997 `_). +* For using node namespace as tf prefix: Set ``tf_frame_prefix:="~"``, where the ("~") character is substituted with node namespace. (`#1997 `_). diff --git a/doc/release_notes.rst b/doc/release_notes.rst index d06d411977..1272709927 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -7,5 +7,5 @@ This list summarizes important changes between Kilted Kaiju (previous) and Lyric diff_drive_controller ***************************** -* Remove unused parameter ``tf_frame_prefix_enable``, use ``tf_frame_prefix`` instead. (`#1997 `_). +* Parameter ``tf_frame_prefix_enable`` got deprecated and will be removed in a future release (`#1997 `_). * Now any tilde ("~") character in ``tf_frame_prefix`` is substituted with node namespace. (`#1997 `_).