Make the save and record dialogs work the same way #64
Conversation
Previously, the code for getting the filename to record a bag, got a prefix string fom the user and unconditionally appended a suffix based on the current date/time. In contrast, the code for saving simply let the user specify the output filename without further modification. To make these dialogs more uniform in their behavior, this change prepopulates both dialogs with a filename based on the current date/time. The user is then free to take the name as-is, or modify it. The result is then used as the filename for the bag file, resulting in consistent behavior for Save (export whole bag or region) and Record. Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
There was a problem hiding this comment.
Was able to reproduce the before and the after behaviors. The new behavior is a lot nicer! Fewer surprises.
Maybe we don't have CI for GUI. But do we have at least static analysis? Not familiar with what we do for ROS 1.
Reproducible example:
$ rosrun rqt_bag rqt_bag
Original behavior:
Record button will write to <user_value>_<timestamp>.bag
Restart rqt_bag. Open button, then Save button, will write to <user_value> (no extension).
New behavior:
Both Record button and Save button write to <user_value>.bag, where the user_value is prepopulated with the timestamp.
There was a problem hiding this comment.
After meeting discussions, I'll approve and leave it up to you whether you want to enable PR testing in rosdistro and see what happens. Since that is not the conventional practice for this package, I don't want it to block the PRs.
If we enable linting (roslint?) later on, we would probably fix all the linting of the entire package at once.
|
Another option for CI (that I just thought of) is to try ros-tooling/action-ros-ci with GitHub workflows. This could be enabled in parallel with rosdistro pull request CI. |
|
@mabelzhang Yes, I'd like to run the linting tool(s) and submit a separate PR for that. Also add a LICENSE file as Jacob recommended. Probably focus this cleanup on the ros2 branch. |
|
Sounds good! |
Previously, the code for getting the filename to record a bag, got
a prefix string from the user and unconditionally appended a suffix
based on the current date/time. And, in contrast, the code for saving simply
let the user specify the output filename without further modification.
Now, to make these dialogs more uniform in their behavior, this change
prepopulates both dialogs with a filename based on the current date/time.
The user is then free to take the name as-is, or modify it. The result
is then used as the filename for the bag file, resulting in consistent
behavior for Save (export whole bag or region) and Record.
Signed-off-by: Michael Jeronimo michael.jeronimo@openrobotics.org