Spawner check for currently running controllers on shutdown #364
Conversation
There was a problem hiding this comment.
I see the general idea with the PR but it is a little off. The spawner should only be responsible for stopping/unloading the controllers it loaded/started.
You should take the intersection of currently running vs loaded/started controller and only act on those.
controller_manager/src/controller_manager/controller_manager_interface.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The spawner should not deal with any controller that it was not configured for.
Previously, shutdown simply stopped and unloaded the controllers that were spawned on launch. This behavior caused errors if a different controller was running at the time shutdown was requested.
If this really causes errors, there is a bug that needs to be fixed.
The current PR would start controllers that have been stopped (or even unloaded) in between.
|
Error output from the original issue I was trying to resolve: I am running Kinetic on Ubuntu 16.04, and using the Why is |
controller_manager/src/controller_manager/controller_manager_interface.py
Outdated
Show resolved
Hide resolved
| @@ -191,9 +200,6 @@ def main(): | |||
|
|
|||
| rospy.loginfo("Controller Spawner: Loaded controllers: %s" % ', '.join(loaded)) | |||
|
|
|||
| if rospy.is_shutdown(): | |||
There was a problem hiding this comment.
whiy did you remove this block?
There was a problem hiding this comment.
It seemed redundant since a shutdown hook is defined, and rospy.spin() simply keeps python from exiting until rospy.core.is_shutdown() is set (see rospy/client.py).
The intended use case for Instead of adding the list call, it might be easier to just use |
|
Just to give some context, on a humanoid we'd usually have a separate set
of controllers for arms, legs/mobile base managed by different spawners but
within the same controller_manager as they belong to the same hw comm
system.
|
|
@megantuttle will you have time to revamps this or would you prefer to close this PR? |
@bmagyar I believe I've addressed all of the comments here, am I missing something? |
This PR improves controller shutdown by getting the list of currently running controllers before attempting to stop and unload them. Previously, shutdown simply stopped and unloaded the controllers that were spawned on launch. This behavior caused errors if a different controller was running at the time shutdown was requested.