From 89f601a4fb010fd82d55de8cbf0dce98da778ff3 Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Sun, 13 Oct 2024 13:43:15 +0000 Subject: [PATCH 1/7] Add cm switch controller service timeout a parameter --- .../controller_manager/spawner.py | 34 ++++++++++++++++--- .../controller_manager/unspawner.py | 16 ++++++++- controller_manager/doc/userdoc.rst | 23 +++++++------ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/controller_manager/controller_manager/spawner.py b/controller_manager/controller_manager/spawner.py index d6df3be01f..46aae894f8 100644 --- a/controller_manager/controller_manager/spawner.py +++ b/controller_manager/controller_manager/spawner.py @@ -112,7 +112,14 @@ def main(args=None): "--controller-manager-timeout", help="Time to wait for the controller manager", required=False, - default=0, + default=0.0, + type=float, + ) + parser.add_argument( + "--controller-manager-switch-timeout", + help="Time to wait for the controller manager to switch controllers", + required=False, + default=10.0, type=float, ) parser.add_argument( @@ -129,6 +136,7 @@ def main(args=None): controller_manager_name = args.controller_manager param_file = args.param_file controller_manager_timeout = args.controller_manager_timeout + controller_manager_switch_timeout = args.controller_manager_switch_timeout if param_file and not os.path.isfile(param_file): raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), param_file) @@ -206,7 +214,13 @@ def main(args=None): if not args.inactive and not args.activate_as_group: ret = switch_controllers( - node, controller_manager_name, [], [controller_name], True, True, 5.0 + node, + controller_manager_name, + [], + [controller_name], + True, + True, + controller_manager_switch_timeout, ) if not ret.ok: node.get_logger().error( @@ -224,7 +238,13 @@ def main(args=None): if not args.inactive and args.activate_as_group: ret = switch_controllers( - node, controller_manager_name, [], controller_names, True, True, 5.0 + node, + controller_manager_name, + [], + controller_names, + True, + True, + controller_manager_switch_timeout, ) if not ret.ok: node.get_logger().error( @@ -250,7 +270,13 @@ def main(args=None): node.get_logger().info("Interrupt captured, deactivating and unloading controller") # TODO(saikishor) we might have an issue in future, if any of these controllers is in chained mode ret = switch_controllers( - node, controller_manager_name, controller_names, [], True, True, 5.0 + node, + controller_manager_name, + controller_names, + [], + True, + True, + controller_manager_switch_timeout, ) if not ret.ok: node.get_logger().error( diff --git a/controller_manager/controller_manager/unspawner.py b/controller_manager/controller_manager/unspawner.py index e42d85aee9..7034379096 100644 --- a/controller_manager/controller_manager/unspawner.py +++ b/controller_manager/controller_manager/unspawner.py @@ -36,17 +36,31 @@ def main(args=None): default="/controller_manager", required=False, ) + parser.add_argument( + "--controller-manager-switch-timeout", + help="Time to wait for the controller manager to switch controllers", + required=False, + default=10.0, + type=float, + ) command_line_args = rclpy.utilities.remove_ros_args(args=sys.argv)[1:] args = parser.parse_args(command_line_args) controller_names = args.controller_names controller_manager_name = args.controller_manager + controller_manager_switch_timeout = args.controller_manager_switch_timeout node = Node("unspawner_" + controller_names[0]) try: # Ignore returncode, because message is already printed and we'll try to unload anyway ret = switch_controllers( - node, controller_manager_name, controller_names, [], True, True, 5.0 + node, + controller_manager_name, + controller_names, + [], + True, + True, + controller_manager_switch_timeout, ) node.get_logger().info("Deactivated controller") diff --git a/controller_manager/doc/userdoc.rst b/controller_manager/doc/userdoc.rst index bc9f75384e..15d1bf63de 100644 --- a/controller_manager/doc/userdoc.rst +++ b/controller_manager/doc/userdoc.rst @@ -145,9 +145,9 @@ 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] [-t CONTROLLER_TYPE] [-u] - [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT] - controller_name + usage: spawner [-h] [-c CONTROLLER_MANAGER] [-p PARAM_FILE] [-n NAMESPACE] [--load-only] [--inactive] [-u] [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT] + [--controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT] [--activate-as-group] + controller_names [controller_names ...] positional arguments: controller_names List of controllers @@ -165,10 +165,9 @@ There are two scripts to interact with controller manager from launch files: -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 + --controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT + Time to wait for the controller manager to switch controllers --activate-as-group Activates all the parsed controllers list together instead of one by one. Useful for activating all chainable controllers altogether - --fallback_controllers FALLBACK_CONTROLLERS [FALLBACK_CONTROLLERS ...] - Fallback controllers list are activated as a fallback strategy when the spawned controllers fail. When the argument is provided, it takes precedence over the fallback_controllers list in the - param file ``unspawner`` @@ -177,15 +176,17 @@ There are two scripts to interact with controller manager from launch files: .. code-block:: console $ ros2 run controller_manager unspawner -h - usage: unspawner [-h] [-c CONTROLLER_MANAGER] controller_name + usage: unspawner [-h] [-c CONTROLLER_MANAGER] [--controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT] controller_names [controller_names ...] positional arguments: - controller_name Name of the controller + controller_names 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 + --controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT + Time to wait for the controller manager to switch controllers ``hardware_spawner`` ^^^^^^^^^^^^^^^^^^^^^^ @@ -193,7 +194,7 @@ There are two scripts to interact with controller manager from launch files: .. code-block:: console $ ros2 run controller_manager hardware_spawner -h - usage: hardware_spawner [-h] [-c CONTROLLER_MANAGER] (--activate | --configure) hardware_component_name + usage: hardware_spawner [-h] [-c CONTROLLER_MANAGER] [--controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT] (--activate | --configure) hardware_component_name positional arguments: hardware_component_name @@ -203,6 +204,8 @@ There are two scripts to interact with controller manager from launch files: -h, --help show this help message and exit -c CONTROLLER_MANAGER, --controller-manager CONTROLLER_MANAGER Name of the controller manager ROS node + --controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT + Time to wait for the controller manager --activate Activates the given components. Note: Components are by default configured before activated. --configure Configures the given components. From e26c00c64afa81c06ba6177413a940b11cbc30f5 Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Sun, 13 Oct 2024 13:47:32 +0000 Subject: [PATCH 2/7] Update release notes --- doc/release_notes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 7d28b4390d..0c11f55ce6 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -74,6 +74,7 @@ controller_manager * The support for the ``description`` parameter for loading the URDF was removed (`#1358 `_). * The ``--controller-type`` or ``-t`` spawner arg is removed. Now the controller type is defined in the controller configuration file with ``type`` field (`#1639 `_). * The ``--namespace`` or ``-n`` spawner arg is deprecated. Now the spawner namespace can be defined using the ROS 2 standard way (`#1640 `_). +* ``--controller-manager-switch-timeout`` was added as parameter to the helper scripts ``spawner.py`` and ``unspawner.py`` (`#1790 `_). hardware_interface ****************** From 2cc492439f88d87f15b327e2ff89d7f04411926f Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Mon, 14 Oct 2024 08:40:18 +0000 Subject: [PATCH 3/7] Rename param and set to the same default as CM --- controller_manager/controller_manager/spawner.py | 16 +++++++++------- .../controller_manager/unspawner.py | 12 +++++++----- controller_manager/doc/userdoc.rst | 14 ++++++++------ 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/controller_manager/controller_manager/spawner.py b/controller_manager/controller_manager/spawner.py index 46aae894f8..7cc8652782 100644 --- a/controller_manager/controller_manager/spawner.py +++ b/controller_manager/controller_manager/spawner.py @@ -116,10 +116,12 @@ def main(args=None): type=float, ) parser.add_argument( - "--controller-manager-switch-timeout", - help="Time to wait for the controller manager to switch controllers", + "--switch-timeout", + help="Time to wait for a successful state switch of controllers." + " Useful if the controller_manager is not immediately ready, e.g.," + " paused simulations at startup", required=False, - default=10.0, + default=1.0, type=float, ) parser.add_argument( @@ -136,7 +138,7 @@ def main(args=None): controller_manager_name = args.controller_manager param_file = args.param_file controller_manager_timeout = args.controller_manager_timeout - controller_manager_switch_timeout = args.controller_manager_switch_timeout + switch_timeout = args.switch_timeout if param_file and not os.path.isfile(param_file): raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), param_file) @@ -220,7 +222,7 @@ def main(args=None): [controller_name], True, True, - controller_manager_switch_timeout, + switch_timeout, ) if not ret.ok: node.get_logger().error( @@ -244,7 +246,7 @@ def main(args=None): controller_names, True, True, - controller_manager_switch_timeout, + switch_timeout, ) if not ret.ok: node.get_logger().error( @@ -276,7 +278,7 @@ def main(args=None): [], True, True, - controller_manager_switch_timeout, + switch_timeout, ) if not ret.ok: node.get_logger().error( diff --git a/controller_manager/controller_manager/unspawner.py b/controller_manager/controller_manager/unspawner.py index 7034379096..e7ee8dddc1 100644 --- a/controller_manager/controller_manager/unspawner.py +++ b/controller_manager/controller_manager/unspawner.py @@ -37,10 +37,12 @@ def main(args=None): required=False, ) parser.add_argument( - "--controller-manager-switch-timeout", - help="Time to wait for the controller manager to switch controllers", + "--switch-timeout", + help="Time to wait for a successful state switch of controllers." + " Useful if the controller_manager is not immediately ready, e.g.," + " paused simulations at startup", required=False, - default=10.0, + default=1.0, type=float, ) @@ -48,7 +50,7 @@ def main(args=None): args = parser.parse_args(command_line_args) controller_names = args.controller_names controller_manager_name = args.controller_manager - controller_manager_switch_timeout = args.controller_manager_switch_timeout + switch_timeout = args.switch_timeout node = Node("unspawner_" + controller_names[0]) try: @@ -60,7 +62,7 @@ def main(args=None): [], True, True, - controller_manager_switch_timeout, + switch_timeout, ) node.get_logger().info("Deactivated controller") diff --git a/controller_manager/doc/userdoc.rst b/controller_manager/doc/userdoc.rst index 15d1bf63de..61fb4ca755 100644 --- a/controller_manager/doc/userdoc.rst +++ b/controller_manager/doc/userdoc.rst @@ -146,7 +146,7 @@ There are two scripts to interact with controller manager from launch files: $ 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] - [--controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT] [--activate-as-group] + [--switch-timeout SWITCH_TIMEOUT] [--activate-as-group] controller_names [controller_names ...] positional arguments: @@ -165,8 +165,9 @@ There are two scripts to interact with controller manager from launch files: -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 - --controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT - Time to wait for the controller manager to switch controllers + --switch-timeout SWITCH_TIMEOUT + Time to wait for a successful state switch of controllers. Useful if the controller_manager is not immediately ready, 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 @@ -176,7 +177,7 @@ There are two scripts to interact with controller manager from launch files: .. code-block:: console $ ros2 run controller_manager unspawner -h - usage: unspawner [-h] [-c CONTROLLER_MANAGER] [--controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT] controller_names [controller_names ...] + usage: unspawner [-h] [-c CONTROLLER_MANAGER] [--switch-timeout SWITCH_TIMEOUT] controller_names [controller_names ...] positional arguments: controller_names Name of the controller @@ -185,8 +186,9 @@ There are two scripts to interact with controller manager from launch files: -h, --help show this help message and exit -c CONTROLLER_MANAGER, --controller-manager CONTROLLER_MANAGER Name of the controller manager ROS node - --controller-manager-switch-timeout CONTROLLER_MANAGER_SWITCH_TIMEOUT - Time to wait for the controller manager to switch controllers + --switch-timeout SWITCH_TIMEOUT + Time to wait for a successful state switch of controllers. Useful if the controller_manager is not immediately ready, e.g., paused + simulations at startup ``hardware_spawner`` ^^^^^^^^^^^^^^^^^^^^^^ From 68ae082fe4b7d1079b0894c8e1109c3a3308bcc6 Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Mon, 14 Oct 2024 08:46:35 +0000 Subject: [PATCH 4/7] Use the same behavior than before --- controller_manager/controller_manager/spawner.py | 2 +- controller_manager/controller_manager/unspawner.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controller_manager/controller_manager/spawner.py b/controller_manager/controller_manager/spawner.py index 7cc8652782..e28ecabfd9 100644 --- a/controller_manager/controller_manager/spawner.py +++ b/controller_manager/controller_manager/spawner.py @@ -121,7 +121,7 @@ def main(args=None): " Useful if the controller_manager is not immediately ready, e.g.," " paused simulations at startup", required=False, - default=1.0, + default=5.0, type=float, ) parser.add_argument( diff --git a/controller_manager/controller_manager/unspawner.py b/controller_manager/controller_manager/unspawner.py index e7ee8dddc1..8917b10c29 100644 --- a/controller_manager/controller_manager/unspawner.py +++ b/controller_manager/controller_manager/unspawner.py @@ -42,7 +42,7 @@ def main(args=None): " Useful if the controller_manager is not immediately ready, e.g.," " paused simulations at startup", required=False, - default=1.0, + default=5.0, type=float, ) From 8ec163f5fd8af5892c4877659c2cb7e54dfeb0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Fr=C3=B6hlich?= Date: Mon, 14 Oct 2024 10:48:44 +0200 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: Felix Exner (fexner) --- controller_manager/controller_manager/spawner.py | 2 +- controller_manager/controller_manager/unspawner.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controller_manager/controller_manager/spawner.py b/controller_manager/controller_manager/spawner.py index e28ecabfd9..7e9fe3443b 100644 --- a/controller_manager/controller_manager/spawner.py +++ b/controller_manager/controller_manager/spawner.py @@ -118,7 +118,7 @@ def main(args=None): parser.add_argument( "--switch-timeout", help="Time to wait for a successful state switch of controllers." - " Useful if the controller_manager is not immediately ready, e.g.," + " Useful when switching cannot be performed immediately, e.g.," " paused simulations at startup", required=False, default=5.0, diff --git a/controller_manager/controller_manager/unspawner.py b/controller_manager/controller_manager/unspawner.py index 8917b10c29..9e380f5086 100644 --- a/controller_manager/controller_manager/unspawner.py +++ b/controller_manager/controller_manager/unspawner.py @@ -39,7 +39,7 @@ def main(args=None): parser.add_argument( "--switch-timeout", help="Time to wait for a successful state switch of controllers." - " Useful if the controller_manager is not immediately ready, e.g.," + " Useful when switching cannot be performed immediately, e.g.," " paused simulations at startup", required=False, default=5.0, From 40d9a32d9fde2dd0e1956c6293ac5f154c9e66c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Fr=C3=B6hlich?= Date: Mon, 14 Oct 2024 10:49:01 +0200 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Felix Exner (fexner) --- controller_manager/doc/userdoc.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controller_manager/doc/userdoc.rst b/controller_manager/doc/userdoc.rst index 61fb4ca755..0a3bb0a99a 100644 --- a/controller_manager/doc/userdoc.rst +++ b/controller_manager/doc/userdoc.rst @@ -166,7 +166,7 @@ There are two scripts to interact with controller manager from launch files: --controller-manager-timeout CONTROLLER_MANAGER_TIMEOUT Time to wait for the controller manager --switch-timeout SWITCH_TIMEOUT - Time to wait for a successful state switch of controllers. Useful if the controller_manager is not immediately ready, e.g., paused + 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 @@ -187,7 +187,7 @@ There are two scripts to interact with controller manager from launch files: -c CONTROLLER_MANAGER, --controller-manager CONTROLLER_MANAGER Name of the controller manager ROS node --switch-timeout SWITCH_TIMEOUT - Time to wait for a successful state switch of controllers. Useful if the controller_manager is not immediately ready, e.g., paused + Time to wait for a successful state switch of controllers. Useful if controllers cannot be switched immediately, e.g., paused simulations at startup ``hardware_spawner`` From 2cd2b2daa0a311964d5ab4094fd6182be368f08d Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Mon, 14 Oct 2024 09:36:51 +0000 Subject: [PATCH 7/7] Update release notes --- doc/release_notes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 0c11f55ce6..738add2492 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -74,7 +74,7 @@ controller_manager * The support for the ``description`` parameter for loading the URDF was removed (`#1358 `_). * The ``--controller-type`` or ``-t`` spawner arg is removed. Now the controller type is defined in the controller configuration file with ``type`` field (`#1639 `_). * The ``--namespace`` or ``-n`` spawner arg is deprecated. Now the spawner namespace can be defined using the ROS 2 standard way (`#1640 `_). -* ``--controller-manager-switch-timeout`` was added as parameter to the helper scripts ``spawner.py`` and ``unspawner.py`` (`#1790 `_). +* ``--switch-timeout`` was added as parameter to the helper scripts ``spawner.py`` and ``unspawner.py``. Useful if controllers cannot be switched immediately, e.g., paused simulations at startup (`#1790 `_). hardware_interface ******************