Skip to content

Remove deprecations from CLI and controller_manager #948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bmagyar merged 2 commits into from
Feb 25, 2023
Merged

Remove deprecations from CLI and controller_manager #948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b675ba
Remove deprecations from CLI and controller_manager
christophfroehlich Feb 20, 2023
e447b2a
Merge branch 'master' into remove_cli_deprecations
bmagyar Feb 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions controller_manager/controller_manager/spawner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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")
Expand Down
8 changes: 5 additions & 3 deletions controller_manager/doc/userdoc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 4 additions & 40 deletions controller_manager/src/controller_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
3 changes: 0 additions & 3 deletions controller_manager_msgs/srv/SwitchController.srv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
---
Expand Down
5 changes: 0 additions & 5 deletions ros2controlcli/ros2controlcli/verb/load_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions ros2controlcli/ros2controlcli/verb/set_controller_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 (
Expand Down
27 changes: 1 addition & 26 deletions ros2controlcli/ros2controlcli/verb/switch_controllers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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="*",
Expand All @@ -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",
Expand All @@ -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:
Expand Down