search

ros-controls / ros_control

Generic and simple controls framework for ROS
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
469 stars 307 forks source link

Duplicate controller name in start and stop controllers list fails silently #506

Closed captain-yoshi closed 1 year ago

captain-yoshi commented 1 year ago

Fixes the case where the user would put the same controller name in the start and stop controllers request:

  switch_ctrl.request.start_controllers = { cartesian_ctrl_name };
  switch_ctrl.request.stop_controllers  = { cartesian_ctrl_name };
  switch_ctrl.request.timeout = 1.0;
  switch_ctrl.request.strictness = switch_ctrl.request.STRICT;

With latest version https://github.com/ros-controls/ros_control/commit/7f3a4064f8022bdcc577c443a595f42270154098:

[DEBUG] [1670293480.214267297]: switching service called
[DEBUG] [1670293480.214310895]: switching service locked
[DEBUG] [1670293480.214325558]: switching controllers:
[DEBUG] [1670293480.214344125]:  - starting controller 'my_cartesian_force_controller'
[DEBUG] [1670293480.214358538]:  - stopping controller 'my_cartesian_force_controller'
[DEBUG] [1670293480.214390714]: Found controller 'my_cartesian_force_controller' that needs to be stopped in list of controllers
[DEBUG] [1670293480.214420813]: Stop request vector has size 1
[DEBUG] [1670293480.214453121]: Found controller 'my_cartesian_force_controller' that needs to be started in list of controllers
[DEBUG] [1670293480.214484711]: Start request vector has size 1
[DEBUG] [1670293480.214549437]: Request atomic controller switch from realtime loop
[DEBUG] [1670293480.222428004]: Successfully switched controllers
[DEBUG] [1670293480.222473856]: switching service finished

and with this patch:

[DEBUG] [1670294793.065341693]: switching service called
[DEBUG] [1670294793.065438787]: switching service locked
[DEBUG] [1670294793.065490294]: switching controllers:
[DEBUG] [1670294793.065548651]:  - starting controller 'my_cartesian_force_controller'
[DEBUG] [1670294793.065602438]:  - stopping controller 'my_cartesian_force_controller'
[DEBUG] [1670294793.065659779]: Found controller 'my_cartesian_force_controller' that needs to be stopped in list of controllers
[DEBUG] [1670294793.065718912]: Stop request vector has size 1
[DEBUG] [1670294793.065763460]: Found controller 'my_cartesian_force_controller' that needs to be started in list of controllers
[DEBUG] [1670294793.065807345]: Start request vector has size 1
[ERROR] [1670294793.065873152]: Could no start or stop controller 'my_cartesian_force_controller' because of conflicting switching command
[DEBUG] [1670294793.065921385]: switching service finished

Maybe you accept this behaviour for someone who wants to restart the controller ? If so, does the manager really stops and restart the controller ?

bmagyar commented 1 year ago

I agree with you.

switch_controllers was never meant to be a tool to restart a single controller so having the same name in both start and stop list would be undefined behaviour in the best case.