Cleanup deprecations in ros_control (#2258)

Author: saikishor

controller_interface/include/controller_interface/helpers.hpp

@@ -81,15 +81,6 @@ inline bool interface_list_contains_interface_type(
          interface_type_list.end();
 }
 
-template <typename T>
-[[deprecated(
-  "Use ros2_control::add_item method instead. This method will be removed by the ROS 2 Kilted "
-  "Kaiju release.")]] void
-add_element_to_list(std::vector<T> & list, const T & element)
-{
-  ros2_control::add_item(list, element);
-}
-
 }  // namespace controller_interface
 
 #endif  // CONTROLLER_INTERFACE__HELPERS_HPP_

controller_manager/controller_manager/spawner.py

@@ -93,13 +93,6 @@ def main(args=None):
         action="append",
         required=False,
     )
-    parser.add_argument(
-        "-n",
-        "--namespace",
-        help="DEPRECATED Namespace for the controller_manager and the controller(s)",
-        default=None,
-        required=False,
-    )
     parser.add_argument(
         "--load-only",
         help="Only load the controller and leave unconfigured.",
@@ -202,21 +195,7 @@ def main(args=None):
 
         node = Node(spawner_node_name)
 
-        if node.get_namespace() != "/" and args.namespace:
-            raise RuntimeError(
-                f"Setting namespace through both '--namespace {args.namespace}' arg and the ROS 2 standard way "
-                f"'--ros-args -r __ns:={node.get_namespace()}' is not allowed!"
-            )
-
-        if args.namespace:
-            warnings.filterwarnings("always")
-            warnings.warn(
-                "The '--namespace' argument is deprecated and will be removed in future releases."
-                " Use the ROS 2 standard way of setting the node namespacing using --ros-args -r __ns:=<namespace>",
-                DeprecationWarning,
-            )
-
-        spawner_namespace = args.namespace if args.namespace else node.get_namespace()
+        spawner_namespace = node.get_namespace()
 
         if not spawner_namespace.startswith("/"):
             spawner_namespace = f"/{spawner_namespace}"

controller_manager/doc/userdoc.rst

@@ -168,8 +168,8 @@ There are two scripts to interact with controller manager from launch files:
 .. code-block:: console
 
     $ ros2 run controller_manager spawner -h
-    usage: spawner [-h] [-c CONTROLLER_MANAGER] [-p PARAM_FILE] [-n NAMESPACE] [--load-only] [--inactive] [-u] [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT]
-                  [--switch-timeout SWITCH_TIMEOUT] [--activate-as-group] [--service-call-timeout SERVICE_CALL_TIMEOUT] [--controller-ros-args CONTROLLER_ROS_ARGS]
+    usage: spawner [-h] [-c CONTROLLER_MANAGER] [-p PARAM_FILE] [--load-only] [--inactive] [-u] [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT] [--switch-timeout SWITCH_TIMEOUT] [--service-call-timeout SERVICE_CALL_TIMEOUT] [--activate-as-group]
+                  [--controller-ros-args CONTROLLER_ROS_ARGS]
                   controller_names [controller_names ...]
 
     positional arguments:
@@ -181,21 +181,18 @@ There are two scripts to interact with controller manager from launch files:
                             Name of the controller manager ROS node
       -p PARAM_FILE, --param-file PARAM_FILE
                             Controller param file to be loaded into controller node before configure. Pass multiple times to load different files for different controllers or to override the parameters of the same controller.
-      -n NAMESPACE, --namespace NAMESPACE
-                            DEPRECATED Namespace for the controller_manager and the controller(s)
       --load-only           Only load the controller and leave unconfigured.
       --inactive            Load and configure the controller, however do not activate them
       -u, --unload-on-kill  Wait until this application is interrupted and unload controller
       --controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT
                             Time to wait for the controller manager service to be available
+      --switch-timeout SWITCH_TIMEOUT
+                            Time to wait for a successful state switch of controllers. Useful when switching cannot be performed immediately, e.g., paused simulations at startup
       --service-call-timeout SERVICE_CALL_TIMEOUT
                             Time to wait for the service response from the controller manager
-      --switch-timeout SWITCH_TIMEOUT
-                            Time to wait for a successful state switch of controllers. Useful if controllers cannot be switched immediately, e.g., paused
-                            simulations at startup
       --activate-as-group   Activates all the parsed controllers list together instead of one by one. Useful for activating all chainable controllers altogether
       --controller-ros-args CONTROLLER_ROS_ARGS
-                            The --ros-args to be passed to the controller node for remapping topics etc
+                            The --ros-args to be passed to the controller node, e.g., for remapping topics. Pass multiple times for every argument.
 
 
 The parsed controller config file can follow the same conventions as the typical ROS 2 parameter file format. Now, the spawner can handle config files with wildcard entries and also the controller name in the absolute namespace. See the following examples on the config files:

controller_manager/test/test_spawner_unspawner.cpp

@@ -1144,97 +1144,6 @@ TEST_F(TestLoadControllerWithNamespacedCM, spawner_test_type_in_params_file)
     cm_->get_parameter(ctrl_2.info.name + ".params_file").as_string_array()[0], test_file_path);
 }
 
-TEST_F(
-  TestLoadControllerWithNamespacedCM, spawner_test_type_in_params_file_deprecated_namespace_arg)
-{
-  const std::string test_file_path =
-    std::string(PARAMETERS_FILE_PATH) + std::string("test_controller_spawner_with_type.yaml");
-
-  ControllerManagerRunner cm_runner(this);
-  // Provide controller type via the parsed file
-  EXPECT_EQ(
-    call_spawner(
-      "ns_ctrl_with_parameters_and_type ns_chainable_ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --controller-manager-timeout 1.0 -p " +
-      test_file_path),
-    256)
-    << "Should fail without the namespacing it";
-  EXPECT_EQ(
-    call_spawner(
-      "ns_ctrl_with_parameters_and_type ns_chainable_ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --namespace foo_namespace --controller-manager-timeout 1.0 -p " +
-      test_file_path + " --ros-args -r __ns:=/random_namespace"),
-    256)
-    << "Should fail when parsed namespace through both way with different namespaces";
-  EXPECT_EQ(
-    call_spawner(
-      "ns_ctrl_with_parameters_and_type ns_chainable_ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --namespace foo_namespace --controller-manager-timeout 1.0 -p" +
-      test_file_path + " --ros-args -r __ns:=/foo_namespace"),
-    256)
-    << "Should fail when parsed namespace through both ways even with same namespacing name";
-  EXPECT_EQ(
-    call_spawner(
-      "ns_ctrl_with_parameters_and_type ns_chainable_ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --namespace foo_namespace -p " +
-      test_file_path),
-    0)
-    << "Should work when parsed through the deprecated arg";
-
-  ASSERT_EQ(cm_->get_loaded_controllers().size(), 2ul);
-
-  auto ctrl_with_parameters_and_type = cm_->get_loaded_controllers()[0];
-  ASSERT_EQ(ctrl_with_parameters_and_type.info.name, "ns_ctrl_with_parameters_and_type");
-  ASSERT_EQ(ctrl_with_parameters_and_type.info.type, test_controller::TEST_CONTROLLER_CLASS_NAME);
-  ASSERT_EQ(
-    ctrl_with_parameters_and_type.c->get_lifecycle_state().id(),
-    lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
-  ASSERT_EQ(
-    cm_->get_parameter(ctrl_with_parameters_and_type.info.name + ".params_file")
-      .as_string_array()[0],
-    test_file_path);
-
-  auto chain_ctrl_with_parameters_and_type = cm_->get_loaded_controllers()[1];
-  ASSERT_EQ(
-    chain_ctrl_with_parameters_and_type.info.name, "ns_chainable_ctrl_with_parameters_and_type");
-  ASSERT_EQ(
-    chain_ctrl_with_parameters_and_type.info.type,
-    test_chainable_controller::TEST_CONTROLLER_CLASS_NAME);
-  ASSERT_EQ(
-    chain_ctrl_with_parameters_and_type.c->get_lifecycle_state().id(),
-    lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
-  ASSERT_EQ(
-    cm_->get_parameter(chain_ctrl_with_parameters_and_type.info.name + ".params_file")
-      .as_string_array()[0],
-    test_file_path);
-
-  EXPECT_EQ(
-    call_spawner(
-      "ns_ctrl_with_parameters_and_no_type -c test_controller_manager --namespace foo_namespace "
-      "-p " +
-      test_file_path),
-    256)
-    << "Should fail as no type is defined!";
-  // Will still be same as the current call will fail
-  ASSERT_EQ(cm_->get_loaded_controllers().size(), 2ul);
-
-  auto ctrl_1 = cm_->get_loaded_controllers()[0];
-  ASSERT_EQ(ctrl_1.info.name, "ns_ctrl_with_parameters_and_type");
-  ASSERT_EQ(ctrl_1.info.type, test_controller::TEST_CONTROLLER_CLASS_NAME);
-  ASSERT_EQ(
-    ctrl_1.c->get_lifecycle_state().id(), lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
-  ASSERT_EQ(
-    cm_->get_parameter(ctrl_1.info.name + ".params_file").as_string_array()[0], test_file_path);
-
-  auto ctrl_2 = cm_->get_loaded_controllers()[1];
-  ASSERT_EQ(ctrl_2.info.name, "ns_chainable_ctrl_with_parameters_and_type");
-  ASSERT_EQ(ctrl_2.info.type, test_chainable_controller::TEST_CONTROLLER_CLASS_NAME);
-  ASSERT_EQ(
-    ctrl_2.c->get_lifecycle_state().id(), lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
-  ASSERT_EQ(
-    cm_->get_parameter(ctrl_2.info.name + ".params_file").as_string_array()[0], test_file_path);
-}
-
 TEST_F(TestLoadControllerWithNamespacedCM, spawner_test_with_wildcard_entries_in_params_file)
 {
   const std::string test_file_path =
@@ -1318,16 +1227,14 @@ TEST_F(
     << "Should fail without the namespacing it";
   EXPECT_EQ(
     call_spawner(
-      "ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --namespace foo_namespace -p " +
-      test_file_path),
+      "ctrl_with_parameters_and_type --load-only -c test_controller_manager -p " + test_file_path +
+      " --ros-args -r __ns:=/foo_namespace"),
     256)
     << "Should fail even namespacing it as ctrl_with_parameters_and_type is not a wildcard entry";
   EXPECT_EQ(
     call_spawner(
-      "chainable_ctrl_with_parameters_and_type --load-only -c "
-      "test_controller_manager --namespace foo_namespace -p " +
-      test_file_path),
+      "chainable_ctrl_with_parameters_and_type --load-only -c test_controller_manager -p " +
+      test_file_path + " --ros-args -r __ns:=/foo_namespace"),
     0)
     << "Should work as chainable_ctrl_with_parameters_and_type is a wildcard entry";
 

controller_manager_msgs/msg/HardwareComponentState.msg

@@ -2,7 +2,6 @@ string name # Name of the hardware component
 string type # Type of the hardware component
 bool is_async # If the hardware component is running asynchronously
 uint16 rw_rate # read/write rate of the hardware component in Hz
-string class_type  # DEPRECATED
 string plugin_name # The name of the plugin that is used to load the hardware component
 lifecycle_msgs/State state # State of the hardware component
 HardwareInterface[] command_interfaces # Command interfaces of the hardware component

hardware_interface/include/hardware_interface/actuator_interface.hpp

@@ -112,23 +112,6 @@ class ActuatorInterface : public rclcpp_lifecycle::node_interfaces::LifecycleNod
 
   virtual ~ActuatorInterface() = default;
 
-  /// Initialization of the hardware interface from data parsed from the robot's URDF and also the
-  /// clock and logger interfaces.
-  /**
-   * \param[in] hardware_info structure with data from URDF.
-   * \param[in] logger Logger for the hardware component.
-   * \param[in] clock_interface pointer to the clock interface.
-   * \returns CallbackReturn::SUCCESS if required data are provided and can be parsed.
-   * \returns CallbackReturn::ERROR if any error happens or data are missing.
-   */
-  [[deprecated("Use init(HardwareInfo, rclcpp::Logger, rclcpp::Clock::SharedPtr) instead.")]]
-  CallbackReturn init(
-    const HardwareInfo & hardware_info, rclcpp::Logger logger,
-    rclcpp::node_interfaces::NodeClockInterface::SharedPtr clock_interface)
-  {
-    return this->init(hardware_info, logger, clock_interface->get_clock());
-  }
-
   /// Initialization of the hardware interface from data parsed from the robot's URDF and also the
   /// clock and logger interfaces.
   /**

hardware_interface/include/hardware_interface/handle.hpp

@@ -134,13 +134,6 @@ class Handle
 
   const std::string & get_interface_name() const { return interface_name_; }
 
-  [[deprecated(
-    "Replaced by get_name method, which is semantically more correct")]] const std::string &
-  get_full_name() const
-  {
-    return get_name();
-  }
-
   const std::string & get_prefix_name() const { return prefix_name_; }
 
   [[deprecated(
@@ -216,54 +209,6 @@ class Handle
     // END
   }
 
-  /**
-   * @brief Get the value of the handle.
-   * @tparam T The type of the value to be retrieved.
-   * @param value The value of the handle.
-   * @return true if the value is accessed successfully, false otherwise.
-   *
-   * @note The method is thread-safe and non-blocking.
-   * @note When different threads access the same handle at same instance, and if they are unable to
-   * lock the handle to access the value, the handle returns false. If the operation is successful,
-   * the value is updated and returns true.
-   */
-  template <typename T>
-  [[deprecated(
-    "Use std::optional<T> get_optional() instead to retrieve the value. This method will be "
-    "removed by the ROS 2 Kilted Kaiju release.")]] [[nodiscard]] bool
-  get_value(T & value) const
-  {
-    std::shared_lock<std::shared_mutex> lock(handle_mutex_, std::try_to_lock);
-    if (!lock.owns_lock())
-    {
-      return false;
-    }
-    // BEGIN (Handle export change): for backward compatibility
-    // TODO(Manuel) return value_ if old functionality is removed
-    if constexpr (std::is_same_v<T, double>)
-    {
-      // If the template is of type double, check if the value_ptr_ is not nullptr
-      THROW_ON_NULLPTR(value_ptr_);
-      value = *value_ptr_;
-    }
-    else
-    {
-      try
-      {
-        value = std::get<T>(value_);
-      }
-      catch (const std::bad_variant_access & err)
-      {
-        throw std::runtime_error(
-          fmt::format(
-            FMT_COMPILE("Invalid data type : '{}' access for interface : {} expected : '{}'"),
-            get_type_name<T>(), get_name(), data_type_.to_string()));
-      }
-    }
-    return true;
-    // END
-  }
-
   /**
    * @brief Set the value of the handle.
    * @tparam T The type of the value to be set.

hardware_interface/include/hardware_interface/loaned_command_interface.hpp

@@ -85,13 +85,6 @@ class LoanedCommandInterface
 
   const std::string & get_interface_name() const { return command_interface_.get_interface_name(); }
 
-  [[deprecated(
-    "Replaced by get_name method, which is semantically more correct")]] const std::string
-  get_full_name() const
-  {
-    return command_interface_.get_name();
-  }
-
   const std::string & get_prefix_name() const { return command_interface_.get_prefix_name(); }
 
   /**
@@ -179,36 +172,6 @@ class LoanedCommandInterface
     return std::nullopt;
   }
 
-  /**
-   * @brief Get the value of the command interface.
-   * @tparam T The type of the value to be retrieved.
-   * @param value The value of the command interface.
-   * @param max_tries The maximum number of tries to get the value.
-   * @return true if the value is accessed successfully, false otherwise.
-   *
-   * @note The method is thread-safe and non-blocking.
-   * @note When different threads access the internal handle at same instance, and if they are
-   * unable to lock the handle to access the value, the handle returns false. If the operation is
-   * successful, the value is updated and returns true.
-   * @note The method will try to get the value max_tries times before returning false. The method
-   * will yield the thread between tries. If the value is updated successfully, the method returns
-   * true immediately.
-   */
-  template <typename T>
-  [[deprecated(
-    "Use std::optional<T> get_optional() instead to retrieve the value. This method will be "
-    "removed by the ROS 2 Kilted Kaiju release.")]] [[nodiscard]] bool
-  get_value(T & value, unsigned int max_tries = 10) const
-  {
-    const auto opt_value = get_optional<T>(max_tries);
-    if (opt_value.has_value())
-    {
-      value = opt_value.value();
-      return true;
-    }
-    return false;
-  }
-
 protected:
   CommandInterface & command_interface_;
   Deleter deleter_;