From b66d186ffbf729cd8753f59cfc0102b76bd50269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0togl?= Date: Wed, 7 Jun 2023 22:47:57 +0200 Subject: [PATCH] Cleanup some tiny issues with the code. --- controller_manager/CMakeLists.txt | 2 +- .../controller_manager/controller_manager.hpp | 4 +--- controller_manager/src/controller_manager.cpp | 12 ++++++------ .../test/test_controller_manager_urdf_passing.cpp | 10 +++++----- hardware_interface/doc/mock_components_userdoc.rst | 2 +- .../include/hardware_interface/resource_manager.hpp | 4 ++-- hardware_interface/src/resource_manager.cpp | 4 ++-- hardware_interface/test/test_resource_manager.cpp | 10 +++++----- 8 files changed, 23 insertions(+), 25 deletions(-) diff --git a/controller_manager/CMakeLists.txt b/controller_manager/CMakeLists.txt index 3d38403332..1118856e48 100644 --- a/controller_manager/CMakeLists.txt +++ b/controller_manager/CMakeLists.txt @@ -145,7 +145,7 @@ if(BUILD_TESTING) controller_manager_msgs ) ament_add_gmock(test_controller_manager_urdf_passing - test/test_controller_manager_urdf_passing.cpp + test/test_controller_manager_urdf_passing.cpp ) target_link_libraries(test_controller_manager_urdf_passing controller_manager diff --git a/controller_manager/include/controller_manager/controller_manager.hpp b/controller_manager/include/controller_manager/controller_manager.hpp index f05de22d9c..631d5173aa 100644 --- a/controller_manager/include/controller_manager/controller_manager.hpp +++ b/controller_manager/include/controller_manager/controller_manager.hpp @@ -84,9 +84,6 @@ class ControllerManager : public rclcpp::Node CONTROLLER_MANAGER_PUBLIC virtual ~ControllerManager() = default; - CONTROLLER_MANAGER_PUBLIC - void subscribe_to_robot_description_topic(); - CONTROLLER_MANAGER_PUBLIC void robot_description_callback(const std_msgs::msg::String & msg); @@ -324,6 +321,7 @@ class ControllerManager : public rclcpp::Node std::vector get_controller_names(); std::pair split_command_interface( const std::string & command_interface); + void subscribe_to_robot_description_topic(); /** * Clear request lists used when switching controllers. The lists are shared between "callback" diff --git a/controller_manager/src/controller_manager.cpp b/controller_manager/src/controller_manager.cpp index 0111b6f67e..69a3bbb5f3 100644 --- a/controller_manager/src/controller_manager.cpp +++ b/controller_manager/src/controller_manager.cpp @@ -151,7 +151,7 @@ ControllerManager::ControllerManager( } std::string robot_description = ""; - // TODO(Manuel): robot_description parameter is deprecated and should be removed. + // TODO(destogl): remove support at the end of 2023 get_parameter("robot_description", robot_description); if (robot_description.empty()) { @@ -161,8 +161,8 @@ ControllerManager::ControllerManager( { RCLCPP_WARN( get_logger(), - "[Deprecated] Passing the robot description file directly to the control_manager node is " - "deprecated. Use robot_state_publisher instead."); + "[Deprecated] Passing the robot description parameter directly to the control_manager node " + "is deprecated. Use '~/robot_description' topic from 'robot_state_publisher' instead."); init_resource_manager(robot_description); } @@ -203,7 +203,7 @@ void ControllerManager::subscribe_to_robot_description_topic() { // set QoS to transient local to get messages that have already been published // (if robot state publisher starts before controller manager) - RCLCPP_INFO_STREAM( + RCLCPP_INFO( get_logger(), "Subscribing to '~/robot_description' topic for robot description file."); robot_description_subscription_ = create_subscription( "~/robot_description", rclcpp::QoS(1).transient_local(), @@ -219,7 +219,7 @@ void ControllerManager::robot_description_callback(const std_msgs::msg::String & // to die if a non valid urdf is passed. However, should maybe be fine tuned. try { - if (resource_manager_->load_urdf_called()) + if (resource_manager_->is_urdf_already_loaded()) { RCLCPP_WARN( get_logger(), @@ -249,7 +249,7 @@ void ControllerManager::init_resource_manager(const std::string & robot_descript std::vector configure_components_on_start = std::vector({}); if (get_parameter("configure_components_on_start", configure_components_on_start)) { - RCLCPP_WARN_STREAM( + RCLCPP_WARN( get_logger(), "[Deprecated]: Usage of parameter \"configure_components_on_start\" is deprecated. Use " "hardware_spawner instead."); diff --git a/controller_manager/test/test_controller_manager_urdf_passing.cpp b/controller_manager/test/test_controller_manager_urdf_passing.cpp index c73a45e291..102cbdfbd4 100644 --- a/controller_manager/test/test_controller_manager_urdf_passing.cpp +++ b/controller_manager/test/test_controller_manager_urdf_passing.cpp @@ -61,27 +61,27 @@ class TestControllerManagerWithTestableCM TEST_P(TestControllerManagerWithTestableCM, initial_no_load_urdf_called) { - ASSERT_FALSE(cm_->resource_manager_->load_urdf_called()); + ASSERT_FALSE(cm_->resource_manager_->is_urdf_already_loaded()); } TEST_P(TestControllerManagerWithTestableCM, load_urdf_called_after_callback) { - ASSERT_FALSE(cm_->resource_manager_->load_urdf_called()); + ASSERT_FALSE(cm_->resource_manager_->is_urdf_already_loaded()); // mimic callback auto msg = std_msgs::msg::String(); msg.data = ros2_control_test_assets::minimal_robot_urdf; cm_->robot_description_callback(msg); - ASSERT_TRUE(cm_->resource_manager_->load_urdf_called()); + ASSERT_TRUE(cm_->resource_manager_->is_urdf_already_loaded()); } TEST_P(TestControllerManagerWithTestableCM, load_urdf_called_after_invalid_urdf_passed) { - ASSERT_FALSE(cm_->resource_manager_->load_urdf_called()); + ASSERT_FALSE(cm_->resource_manager_->is_urdf_already_loaded()); // mimic callback auto msg = std_msgs::msg::String(); msg.data = ros2_control_test_assets::minimal_robot_missing_command_keys_urdf; cm_->robot_description_callback(msg); - ASSERT_TRUE(cm_->resource_manager_->load_urdf_called()); + ASSERT_TRUE(cm_->resource_manager_->is_urdf_already_loaded()); } INSTANTIATE_TEST_SUITE_P( diff --git a/hardware_interface/doc/mock_components_userdoc.rst b/hardware_interface/doc/mock_components_userdoc.rst index f143ea816c..573037a58d 100644 --- a/hardware_interface/doc/mock_components_userdoc.rst +++ b/hardware_interface/doc/mock_components_userdoc.rst @@ -28,7 +28,7 @@ Parameters ,,,,,,,,,, disable_commands (optional; boolean; default: false) - Disables mirroring commands to states. + Disables mirroring commands to states. This option is helpful to simulate an erroneous connection to the hardware when nothing breaks, but suddenly there is no feedback from a hardware interface. Or it can help you to test your setup when the hardware is running without feedback, i.e., in open loop configuration. diff --git a/hardware_interface/include/hardware_interface/resource_manager.hpp b/hardware_interface/include/hardware_interface/resource_manager.hpp index 1afd78b582..4ea3ae9a5f 100644 --- a/hardware_interface/include/hardware_interface/resource_manager.hpp +++ b/hardware_interface/include/hardware_interface/resource_manager.hpp @@ -98,7 +98,7 @@ class HARDWARE_INTERFACE_PUBLIC ResourceManager * @return true if resource manager's load_urdf() has been already called. * @return false if resource manager's load_urdf() has not been yet called. */ - bool load_urdf_called() const; + bool is_urdf_already_loaded() const; /// Claim a state interface given its key. /** @@ -424,7 +424,7 @@ class HARDWARE_INTERFACE_PUBLIC ResourceManager // Structure to store read and write status so it is not initialized in the real-time loop HardwareReadWriteStatus read_write_status; - bool load_urdf_called_ = false; + bool is_urdf_loaded__ = false; }; } // namespace hardware_interface diff --git a/hardware_interface/src/resource_manager.cpp b/hardware_interface/src/resource_manager.cpp index b24e812b07..f32d24f890 100644 --- a/hardware_interface/src/resource_manager.cpp +++ b/hardware_interface/src/resource_manager.cpp @@ -703,7 +703,7 @@ ResourceManager::ResourceManager( // CM API: Called in "callback/slow"-thread void ResourceManager::load_urdf(const std::string & urdf, bool validate_interfaces) { - load_urdf_called_ = true; + is_urdf_loaded__ = true; const std::string system_type = "system"; const std::string sensor_type = "sensor"; const std::string actuator_type = "actuator"; @@ -742,7 +742,7 @@ void ResourceManager::load_urdf(const std::string & urdf, bool validate_interfac resource_storage_->systems_.size()); } -bool ResourceManager::load_urdf_called() const { return load_urdf_called_; } +bool ResourceManager::is_urdf_already_loaded() const { return is_urdf_loaded__; } // CM API: Called in "update"-thread LoanedStateInterface ResourceManager::claim_state_interface(const std::string & key) diff --git a/hardware_interface/test/test_resource_manager.cpp b/hardware_interface/test/test_resource_manager.cpp index c1c054713c..aedf2b862d 100644 --- a/hardware_interface/test/test_resource_manager.cpp +++ b/hardware_interface/test/test_resource_manager.cpp @@ -215,7 +215,7 @@ TEST_F(ResourceManagerTest, initialization_with_urdf_unclaimed) TEST_F(ResourceManagerTest, no_load_urdf_function_called) { TestableResourceManager rm; - ASSERT_FALSE(rm.load_urdf_called()); + ASSERT_FALSE(rm.is_urdf_already_loaded()); } TEST_F(ResourceManagerTest, load_urdf_called_if_urdf_is_invalid) @@ -223,21 +223,21 @@ TEST_F(ResourceManagerTest, load_urdf_called_if_urdf_is_invalid) TestableResourceManager rm; EXPECT_THROW( rm.load_urdf(ros2_control_test_assets::minimal_robot_missing_state_keys_urdf), std::exception); - ASSERT_TRUE(rm.load_urdf_called()); + ASSERT_TRUE(rm.is_urdf_already_loaded()); } TEST_F(ResourceManagerTest, load_urdf_called_if_urdf_is_valid) { TestableResourceManager rm(ros2_control_test_assets::minimal_robot_urdf); - ASSERT_TRUE(rm.load_urdf_called()); + ASSERT_TRUE(rm.is_urdf_already_loaded()); } TEST_F(ResourceManagerTest, can_load_urdf_later) { TestableResourceManager rm; - ASSERT_FALSE(rm.load_urdf_called()); + ASSERT_FALSE(rm.is_urdf_already_loaded()); rm.load_urdf(ros2_control_test_assets::minimal_robot_urdf); - ASSERT_TRUE(rm.load_urdf_called()); + ASSERT_TRUE(rm.is_urdf_already_loaded()); } TEST_F(ResourceManagerTest, resource_claiming)