Remove deprecations from CLI and controller_manager (ros-controls#948)

Co-authored-by: Bence Magyar <[email protected]>
henrygerardmoore · Feb 25, 2023 · b9d30d8 · b9d30d8
1 parent a676d3c
commit b9d30d8
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 102 deletions.
diff --git a/controller_manager/controller_manager/spawner.py b/controller_manager/controller_manager/spawner.py
@@ -156,12 +156,6 @@ def main(args=None):
         action="store_true",
         required=False,
     )
-    parser.add_argument(
-        "--stopped",
-        help="Load and configure the controller, however do not activate them",
-        action="store_true",
-        required=False,
-    )
     parser.add_argument(
         "--inactive",
         help="Load and configure the controller, however do not activate them",
@@ -298,7 +292,7 @@ def main(args=None):
                 node.get_logger().error("Failed to configure controller")
                 return 1
 
-            if not args.stopped and not args.inactive:
+            if not args.inactive:
                 ret = switch_controllers(
                     node, controller_manager_name, [], [controller_name], True, True, 5.0
                 )
@@ -313,8 +307,6 @@ def main(args=None):
                     + prefixed_controller_name
                     + bcolors.ENDC
                 )
-            elif args.stopped:
-                node.get_logger().warn('"--stopped" flag is deprecated use "--inactive" instead')
 
         if not args.unload_on_kill:
             return 0
@@ -324,7 +316,7 @@ def main(args=None):
             while True:
                 time.sleep(1)
         except KeyboardInterrupt:
-            if not args.stopped and not args.inactive:
+            if not args.inactive:
                 node.get_logger().info("Interrupt captured, deactivating and unloading controller")
                 ret = switch_controllers(
                     node, controller_manager_name, [controller_name], [], True, True, 5.0
@@ -335,9 +327,6 @@ def main(args=None):
 
                 node.get_logger().info("Deactivated controller")
 
-            elif args.stopped:
-                node.get_logger().warn('"--stopped" flag is deprecated use "--inactive" instead')
-
             ret = unload_controller(node, controller_manager_name, controller_name)
             if not ret.ok:
                 node.get_logger().error("Failed to unload controller")

diff --git a/controller_manager/doc/userdoc.rst b/controller_manager/doc/userdoc.rst
@@ -81,21 +81,23 @@ 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] [--load-only] [--stopped] [-t CONTROLLER_TYPE] [-u]
+    usage: spawner [-h] [-c CONTROLLER_MANAGER] [-p PARAM_FILE] [-n NAMESPACE] [--load-only] [--inactive] [-t CONTROLLER_TYPE] [-u]
                       [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT]
                       controller_name
 
     positional arguments:
       controller_name       Name of the controller
 
-    optional arguments:
+    options:
       -h, --help            show this help message and exit
       -c CONTROLLER_MANAGER, --controller-manager CONTROLLER_MANAGER
                             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
+      -n NAMESPACE, --namespace NAMESPACE
+                            Namespace for the controller
       --load-only           Only load the controller and leave unconfigured.
-      --stopped             Load and configure the controller, however do not start them
+      --inactive            Load and configure the controller, however do not activate them
       -t CONTROLLER_TYPE, --controller-type CONTROLLER_TYPE
                             If not provided it should exist in the controller manager namespace
       -u, --unload-on-kill  Wait until this application is interrupted and unload controller

diff --git a/controller_manager/src/controller_manager.cpp b/controller_manager/src/controller_manager.cpp
@@ -1521,46 +1521,10 @@ void ControllerManager::switch_controller_service_cb(
   std::lock_guard<std::mutex> guard(services_lock_);
   RCLCPP_DEBUG(get_logger(), "switching service locked");
 
-  //   response->ok = switch_controller(
-  //     request->activate_controllers, request->deactivate_controllers, request->strictness,
-  //     request->activate_asap, request->timeout) == controller_interface::return_type::OK;
-  // TODO(destogl): remove this after deprecated fields are removed from service and use the
-  // commented three lines above
-  // BEGIN: remove when deprecated removed
-  auto activate_controllers = request->activate_controllers;
-  auto deactivate_controllers = request->deactivate_controllers;
-
-  if (!request->start_controllers.empty())
-  {
-    RCLCPP_WARN(
-      get_logger(),
-      "'start_controllers' field is deprecated, use 'activate_controllers' field instead!");
-    activate_controllers.insert(
-      activate_controllers.end(), request->start_controllers.begin(),
-      request->start_controllers.end());
-  }
-  if (!request->stop_controllers.empty())
-  {
-    RCLCPP_WARN(
-      get_logger(),
-      "'stop_controllers' field is deprecated, use 'deactivate_controllers' field instead!");
-    deactivate_controllers.insert(
-      deactivate_controllers.end(), request->stop_controllers.begin(),
-      request->stop_controllers.end());
-  }
-
-  auto activate_asap = request->activate_asap;
-  if (request->start_asap)
-  {
-    RCLCPP_WARN(
-      get_logger(), "'start_asap' field is deprecated, use 'activate_asap' field instead!");
-    activate_asap = request->start_asap;
-  }
-
-  response->ok = switch_controller(
-                   activate_controllers, deactivate_controllers, request->strictness, activate_asap,
-                   request->timeout) == controller_interface::return_type::OK;
-  // END: remove when deprecated removed
+  response->ok =
+    switch_controller(
+      request->activate_controllers, request->deactivate_controllers, request->strictness,
+      request->activate_asap, request->timeout) == controller_interface::return_type::OK;
 
   RCLCPP_DEBUG(get_logger(), "switching service finished");
 }

diff --git a/controller_manager_msgs/srv/SwitchController.srv b/controller_manager_msgs/srv/SwitchController.srv
@@ -21,12 +21,9 @@
 
 string[] activate_controllers
 string[] deactivate_controllers
-string[] start_controllers       # DEPRECATED: Use activate_controllers filed instead
-string[] stop_controllers        # DEPRECATED: Use deactivate_controllers filed instead
 int32 strictness
 int32 BEST_EFFORT=1
 int32 STRICT=2
-bool start_asap                 # DEPRECATED: Use activate_asap filed instead
 bool activate_asap
 builtin_interfaces/Duration timeout
 ---

diff --git a/ros2controlcli/ros2controlcli/verb/load_controller.py b/ros2controlcli/ros2controlcli/verb/load_controller.py
@@ -50,11 +50,6 @@ def main(self, *, args):
             if not response.ok:
                 return "Error configuring controller"
 
-            # TODO(destogl): remove in humble+
-            if args.set_state == "start":
-                print('Setting state "start" is deprecated "activate" instead!')
-                args.set_state == "activate"
-
             if args.set_state == "active":
                 response = switch_controllers(
                     node, args.controller_manager, [], [args.controller_name], True, True, 5.0

diff --git a/ros2controlcli/ros2controlcli/verb/set_controller_state.py b/ros2controlcli/ros2controlcli/verb/set_controller_state.py
@@ -58,14 +58,6 @@ def main(self, *, args):
             #          #      print(f'successfully cleaned up {args.controller_name}')
             #     return 0
 
-            if args.state == "configure":
-                args.state = "inactive"
-                print('Setting state "configure" is deprecated, use "inactive" instead!')
-
-            if args.state == "stop":
-                args.state = "inactive"
-                print('Setting state "stop" is deprecated, use "inactive" instead!')
-
             if args.state == "inactive":
                 if matched_controller.state == "unconfigured":
                     response = configure_controller(
@@ -93,10 +85,6 @@ def main(self, *, args):
                         f"from its current state {matched_controller.state}"
                     )
 
-            if args.state == "start":
-                args.state = "active"
-                print('Setting state "start" is deprecated, use "active" instead!')
-
             if args.state == "active":
                 if matched_controller.state != "inactive":
                     return (

diff --git a/ros2controlcli/ros2controlcli/verb/switch_controllers.py b/ros2controlcli/ros2controlcli/verb/switch_controllers.py
@@ -26,27 +26,13 @@ class SwitchControllersVerb(VerbExtension):
 
     def add_arguments(self, parser, cli_name):
         add_arguments(parser)
-        arg = parser.add_argument(
-            "--stop",
-            nargs="*",
-            default=[],
-            help="Name of the controllers to be deactivated",
-        )
-        arg.completer = LoadedControllerNameCompleter(["active"])
         arg = parser.add_argument(
             "--deactivate",
             nargs="*",
             default=[],
             help="Name of the controllers to be deactivated",
         )
         arg.completer = LoadedControllerNameCompleter(["active"])
-        arg = parser.add_argument(
-            "--start",
-            nargs="*",
-            default=[],
-            help="Name of the controllers to be activated",
-        )
-        arg.completer = LoadedControllerNameCompleter(["inactive"])
         arg = parser.add_argument(
             "--activate",
             nargs="*",
@@ -55,7 +41,6 @@ def add_arguments(self, parser, cli_name):
         )
         arg.completer = LoadedControllerNameCompleter(["inactive"])
         parser.add_argument("--strict", action="store_true", help="Strict switch")
-        parser.add_argument("--start-asap", action="store_true", help="Start asap controllers")
         parser.add_argument("--activate-asap", action="store_true", help="Start asap controllers")
         parser.add_argument(
             "--switch-timeout",
@@ -67,24 +52,14 @@ def add_arguments(self, parser, cli_name):
         add_controller_mgr_parsers(parser)
 
     def main(self, *, args):
-        if args.stop:
-            print('"--stop" flag is deprecated, use "--deactivate" instead!')
-            args.deactivate = args.stop
-        if args.start:
-            print('"--start" flag is deprecated, use "--activate" instead!')
-            args.activate = args.start
-        if args.start_asap:
-            print('"--start-asap" flag is deprecated, use "--activate-asap" instead!')
-            args.activate_asap = args.start_asap
-
         with NodeStrategy(args) as node:
             response = switch_controllers(
                 node,
                 args.controller_manager,
                 args.deactivate,
                 args.activate,
                 args.strict,
-                args.start_asap,
+                args.activate_asap,
                 args.switch_timeout,
             )
             if not response.ok: