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

python node with Recorder does not exit with CTRL-C #1458

Closed
tonynajjar opened this issue Sep 5, 2023 · 5 comments · Fixed by #1464
Closed

python node with Recorder does not exit with CTRL-C #1458

tonynajjar opened this issue Sep 5, 2023 · 5 comments · Fixed by #1464
Labels
bug Something isn't working

Comments

@tonynajjar
Copy link

Description

A rclpy node instantiating a rosbag2_py.Recorder does not exit with CTRL-C

Expected Behavior

It should exit cleanly

Actual Behavior

It just hangs

To Reproduce

import rclpy

from rclpy.node import Node
from rosbag2_py import Recorder

class RosBagRecorder(Node):
    def __init__(self):
        super().__init__("rosbag_recorder")
        self.recorder = Recorder()

def main():
    rclpy.init()
    node = RosBagRecorder()
    rclpy.spin(node)


if __name__ == "__main__":
    main()

System (please complete the following information)

  • OS: Ubuntu 22
  • ROS 2 Distro: Iron
  • Install Method: source
  • Version: iron latest

Additional context

** Add any other context about the problem here **

@MichaelOrlov
Copy link
Contributor

@tonynajjar The code behave as expected by design.
It doesn't make sense to create rosbag2 recorder and spin node directly.
We have SingleThreadedExecuter inside Recorder::record(..) method which will spin the node and gracefully handle SIGINT and SIGTERM signals.

in your example, you are missing call for the

recorder.record(storage_options, record_options, "node_name")

Please refer to the

recorder = Recorder()
try:
recorder.record(storage_options, record_options, args.node_name)
except KeyboardInterrupt:
pass

as an example of how to properly use it.

@MichaelOrlov MichaelOrlov added the wontfix This will not be worked on label Sep 7, 2023
@tonynajjar
Copy link
Author

The instantiation of a Recorder in any python script will block CTRL-C

from rosbag2_py import Recorder
import time

recorder = Recorder()
time.sleep(10)

Try to exit this script with CTRL-C before the sleep ends -> doesn't exit. Maybe CTRL-C will work if recorder.record is called but that shouldn't be a requirement

@MichaelOrlov
Copy link
Contributor

@tonynajjar Yes it is true. We override signal handlers in the recorder constructor to handle them further on in blocking recorder::record(..) method.
Perhaps we can change this behavior and install signal handlers at the beginning of the recorder::record(..) method.

Out of curiosity what would be a use case for instantiating Recorder() instance and not using recorder::record(..) method after that?

@tonynajjar
Copy link
Author

The use case is I want to have a node with a service server that when called, will start recording a bag with the help of the Recorder. A bit like what is asked in #685

So the Recorder exists but we only start recording when the service is called

@fujitatomoya
Copy link
Contributor

The use case is I want to have a node with a service server that when called, will start recording a bag with the help of the Recorder.

@tonynajjar https://github.com/ros2/rosbag2?tab=readme-ov-file#controlling-recordings-via-services does not work for your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants