diff --git a/doc/migration.rst b/doc/migration.rst index 59a30b54b9..de4f4f1c10 100644 --- a/doc/migration.rst +++ b/doc/migration.rst @@ -20,3 +20,8 @@ diff_drive_controller ***************************** * 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 `_). + +mecanum_drive_controller +***************************** +* 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 ab8f4760f7..0fddeb7f5a 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -14,6 +14,11 @@ diff_drive_controller * 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 `_). +mecanum_drive_controller +***************************** +* 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 `_). + joint_state_broadcaster ************************ * Make all parameters read-only (the never got re-evaluated after initialization anyways). (`#2064 `_) diff --git a/mecanum_drive_controller/src/mecanum_drive_controller.cpp b/mecanum_drive_controller/src/mecanum_drive_controller.cpp index e468c8577b..b7c8c9c426 100644 --- a/mecanum_drive_controller/src/mecanum_drive_controller.cpp +++ b/mecanum_drive_controller/src/mecanum_drive_controller.cpp @@ -20,6 +20,7 @@ #include #include "controller_interface/helpers.hpp" +#include "controller_interface/tf_prefix.hpp" #include "hardware_interface/types/hardware_interface_type_values.hpp" #include "lifecycle_msgs/msg/state.hpp" #include "tf2/transform_datatypes.hpp" @@ -131,6 +132,17 @@ controller_interface::CallbackReturn MecanumDriveController::on_configure( reset_controller_reference_msg(current_ref_, get_node()); input_ref_.set(current_ref_); + // 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"); + } + try { // Odom state publisher @@ -146,24 +158,34 @@ controller_interface::CallbackReturn MecanumDriveController::on_configure( return controller_interface::CallbackReturn::ERROR; } - // Append the tf prefix if there is one + // resolve prefix: substitute tilde (~) with the namespace if contains and normalize slashes (/) std::string tf_prefix = ""; if (params_.tf_frame_prefix_enable) { - tf_prefix = params_.tf_frame_prefix != "" ? params_.tf_frame_prefix - : std::string(get_node()->get_namespace()); - - // Make sure prefix does not start with '/' and always ends with '/' - if (tf_prefix.back() != '/') + if (params_.tf_frame_prefix != "") { - tf_prefix = tf_prefix + "/"; + tf_prefix = controller_interface::resolve_tf_prefix( + params_.tf_frame_prefix, get_node()->get_namespace()); } - if (tf_prefix.front() == '/') + else { - tf_prefix.erase(0, 1); + RCLCPP_WARN( + get_node()->get_logger(), + "Please use tilde ('~') character in 'tf_frame_prefix' as it replaced with node namespace"); + + tf_prefix = std::string(get_node()->get_namespace()); + if (tf_prefix.back() != '/') + { + tf_prefix = tf_prefix + "/"; + } + if (tf_prefix.front() == '/') + { + tf_prefix.erase(0, 1); + } } } + // 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; diff --git a/mecanum_drive_controller/src/mecanum_drive_controller.yaml b/mecanum_drive_controller/src/mecanum_drive_controller.yaml index 058270fd7a..f789cdb19e 100644 --- a/mecanum_drive_controller/src/mecanum_drive_controller.yaml +++ b/mecanum_drive_controller/src/mecanum_drive_controller.yaml @@ -116,7 +116,7 @@ mecanum_drive_controller: tf_frame_prefix_enable: { type: bool, default_value: true, - description: "Enables or disables appending ``tf_frame_prefix`` to tf frame id's. See its parameter description for more information.", + description: "Deprecated: this parameter will be removed in a future release. Use 'tf_frame_prefix' instead.", } tf_frame_prefix: { type: string, diff --git a/mecanum_drive_controller/test/test_mecanum_drive_controller.cpp b/mecanum_drive_controller/test/test_mecanum_drive_controller.cpp index a1ec5fdba9..d7c90daeab 100644 --- a/mecanum_drive_controller/test/test_mecanum_drive_controller.cpp +++ b/mecanum_drive_controller/test/test_mecanum_drive_controller.cpp @@ -125,7 +125,7 @@ TEST_F(MecanumDriveControllerTest, when_controller_configured_expect_properly_ex } } -TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_false_no_namespace) +TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_prefix_false_covariance_test) { std::string odom_id = "odom"; std::string base_link_id = "base_link"; @@ -160,7 +160,7 @@ TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_false_no_na ASSERT_EQ(odometry_message.twist.covariance, twist_covariance); } -TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_no_namespace) +TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_prefix_no_namespace) { std::string odom_id = "odom"; std::string base_link_id = "base_link"; @@ -180,14 +180,12 @@ TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_no_nam std::string test_odom_frame_id = odometry_message.header.frame_id; std::string test_base_frame_id = odometry_message.child_frame_id; - /* 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(test_odom_frame_id, frame_prefix + "/" + odom_id); ASSERT_EQ(test_base_frame_id, frame_prefix + "/" + base_link_id); } -TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_no_namespace) +TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_no_namespace) { std::string odom_id = "odom"; std::string base_link_id = "base_link"; @@ -206,40 +204,13 @@ TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_no_na auto odometry_message = controller_->odom_state_msg_; std::string test_odom_frame_id = odometry_message.header.frame_id; std::string test_base_frame_id = odometry_message.child_frame_id; - /* tf_frame_prefix_enable is true but frame_prefix is blank so should not be appended to the - frame - * id's */ - ASSERT_EQ(test_odom_frame_id, odom_id); - ASSERT_EQ(test_base_frame_id, base_link_id); -} - -TEST_F(MecanumDriveControllerTest, 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"; - - auto node_options = controller_->define_custom_node_options(); - node_options.append_parameter_override("tf_frame_prefix_enable", rclcpp::ParameterValue(false)); - node_options.append_parameter_override("tf_frame_prefix", rclcpp::ParameterValue(frame_prefix)); - node_options.append_parameter_override("odom_frame_id", rclcpp::ParameterValue(odom_id)); - node_options.append_parameter_override("base_frame_id", rclcpp::ParameterValue(base_link_id)); - - SetUpController("test_mecanum_drive_controller", node_options, test_namespace); - - ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), NODE_SUCCESS); - auto odometry_message = controller_->odom_state_msg_; - std::string test_odom_frame_id = odometry_message.header.frame_id; - std::string test_base_frame_id = odometry_message.child_frame_id; - /* 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(test_odom_frame_id, odom_id); ASSERT_EQ(test_base_frame_id, base_link_id); } -TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_set_namespace) +TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_prefix_set_namespace) { std::string test_namespace = "/test_namespace"; @@ -261,19 +232,17 @@ TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_set_na std::string test_odom_frame_id = odometry_message.header.frame_id; std::string test_base_frame_id = odometry_message.child_frame_id; - /* 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(test_odom_frame_id, frame_prefix + "/" + odom_id); ASSERT_EQ(test_base_frame_id, frame_prefix + "/" + base_link_id); } -TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_set_namespace) +TEST_F(MecanumDriveControllerTest, 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 = "~"; auto node_options = controller_->define_custom_node_options(); node_options.append_parameter_override("tf_frame_prefix_enable", rclcpp::ParameterValue(true)); @@ -289,9 +258,8 @@ TEST_F(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_set_n std::string test_odom_frame_id = odometry_message.header.frame_id; std::string test_base_frame_id = odometry_message.child_frame_id; 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 */ + + // frame_prefix has tilde (~) character so node namespace should be prepended to the frame id's ASSERT_EQ(test_odom_frame_id, ns_prefix + odom_id); ASSERT_EQ(test_base_frame_id, ns_prefix + base_link_id); } diff --git a/mecanum_drive_controller/test/test_mecanum_drive_controller.hpp b/mecanum_drive_controller/test/test_mecanum_drive_controller.hpp index c519e00eb0..aaed8102d4 100644 --- a/mecanum_drive_controller/test/test_mecanum_drive_controller.hpp +++ b/mecanum_drive_controller/test/test_mecanum_drive_controller.hpp @@ -84,12 +84,11 @@ class TestableMecanumDriveController : public mecanum_drive_controller::MecanumD when_ref_timeout_zero_for_reference_callback_expect_reference_msg_being_used_only_once); FRIEND_TEST(MecanumDriveControllerTest, SideToSideAndRotationOdometryTest); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_false_no_namespace); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_no_namespace); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_no_namespace); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_false_set_namespace); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_test_prefix_true_set_namespace); - FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_true_set_namespace); + FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_prefix_false_covariance_test); + FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_prefix_no_namespace); + FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_blank_prefix_no_namespace); + FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_prefix_set_namespace); + FRIEND_TEST(MecanumDriveControllerTest, configure_succeeds_tf_tilde_prefix_set_namespace); public: controller_interface::CallbackReturn on_configure(