Skip to content
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.

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

On shutdown hooks #244

Closed
stonier opened this issue Oct 20, 2018 · 5 comments
Closed

On shutdown hooks #244

stonier opened this issue Oct 20, 2018 · 5 comments
Labels
enhancement New feature or request

Comments

@stonier
Copy link
Contributor

stonier commented Oct 20, 2018

Feature request

Feature description

Something like the rospy.on_shutdown() handle.

Implementation considerations

Primary use case here is so that authors of scripts/applications do not need to remember a long sequence of magic shutdown calls. e.g.

try:
    rclpy.spin(node)
except KeyboardInterrupt:
     pass
foo.shutdown()
bar.shutdown()
node.destroy_node()
rclpy.shutdown()

Instead, the shutdown methods would register themselves in a setup phase inside the class and subsequently the author of the main script need not worry about the shutdown methods at all.

@sloretz sloretz added the enhancement New feature or request label Oct 22, 2018
@stonier
Copy link
Contributor Author

stonier commented Oct 24, 2018

I wonder if this was just my immediate reaction to not having something that was there in ROS1. Looking at the situation with fresh eyes, I wonder if it really is the right thing to have the application take care of the shutdown. It is only the application that will know that foo.shutdown() might need to happen before bar.shutdown() lest something catastrophic happen and the world end.

In this case it would be each class's responsibility to agnostically provide an on_shutdown method.

@clalancette
Copy link
Contributor

@stonier Based on your latest comment, I'm going to close this out for now. If you feel like this is something that core rclpy needs to provide in the future, please feel free to reopen.

@stonier
Copy link
Contributor Author

stonier commented Oct 26, 2018

I'd be interested to know if there was an explicit reason it was made available in the ROS 1 api. We could perhaps trawl the rospy issues and commit history to see if there is anything there.

@clalancette
Copy link
Contributor

Unfortunately, it looks like in ros_comm that code comes from this commit: ros/ros_comm@5ac90fafe3 ,which looks like it is probably the port from sourceforge -> github in 2010. To go further back than that, we'd have to go trawling through the original sourceforge stuff, which I don't think is worth it. Maybe @tfoote has some recollection on why it was originally added.

@chapulina
Copy link
Contributor

On gazebo_ros_pkgs we have a use case where we want to make a service call right before shutdown. On ROS 1 the on_shutdown hook was being used:

https://github.com/ros-simulation/gazebo_ros_pkgs/blob/3cd100e20ba09245be01ad674248aa31a46ad271/gazebo_ros/scripts/spawn_model#L315-L317

If the hook isn't ported, is there an alternative way to achieve this?

That script is being migrated to ROS 2 on ros-simulation/gazebo_ros_pkgs#948

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants