ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
765 stars 913 forks source link

[rosbag] Expose snapshot functionality #1941

Open gabrielSoudry opened 4 years ago

gabrielSoudry commented 4 years ago

Unlike #1414 I don't need more features, I just want the return of the deprecated command rosrecord -s like this topic in ros forum. The functionnality seem to yet exist https://github.com/ros/ros_comm/blob/fad51345f1937a8fca036b264a4c6fb034970a5f/tools/rosbag/src/recorder.cpp#L182-L188 But not in any options https://github.com/ros/ros_comm/blob/d82f3f09f2b9b55cbbee68ba25f36174ec2c7002/tools/rosbag/src/record.cpp#L50-L72

Just add : ("snapshot,s","(EXPERIMENTAL) Enable snapshot recording (don't write to file unless triggered)") ("trigger,t","(EXPERIMENTAL) Trigger snapshot recording") And if (vm.count("snapshot")) { opts.snapshot = true; } if (vm.count("trigger")) { opts.trigger = true; } And it's fixed.

Do you have any idea why it's not implemented ?

dirk-thomas commented 4 years ago

Do you have any idea why it's not implemented ?

There is no need for a trigger option. That is being done through the topic snapshot_trigger shown in the above snippet.

For a snapshot option please consider to provide a pull request for this. I assume it wasn't added when the feature was implemented because the author only used the API rather than the CLI.