ros2 / rosbag2

Apache License 2.0
285 stars 251 forks source link

Add record, stop, start_discovery, stop_discovery and is_discovery_stopped services for rosbag2_transport::Recorder #1840

Open sharminramli opened 1 month ago

sharminramli commented 1 month ago

Adds the following services to rosbag2_transport::Recorder:

Fixes some issues with starting record again after stopping:

Closes https://github.com/ros2/rosbag2/issues/1634

sharminramli commented 2 weeks ago

@MichaelOrlov Thank you for the review. I've unfortunately been busy this week but I'm starting to work on the changes.

I have a question regarding:

Also it would be more usefull if service request such as start_discovery, stop_dicovery, record and stop will return true or false as result.

I am right now reusing the existing Stop.srv in rosbag2_interfaces which is being used by the player. If I create a new srv e.g. StopRecorder.srv to have the boolean response, I would also rename the existing Stop.srv to e.g. StopPlayer.srv to avoid confusion. That however might break a lot of existing code for users. Alternatively I could add the bool to the existing Stop.srv (and also set it in the player when called). What do you think would be the best way?

MichaelOrlov commented 1 week ago

@sharminramli Adding a return value to the existing service requests looks like more safer option.