rosbag2 icon indicating copy to clipboard operation
rosbag2 copied to clipboard

[SequentialWriter] Allow directory to exist as long as there is no metadata.yaml

Open oysstu opened this issue 1 year ago • 3 comments
trafficstars

https://github.com/ros2/rosbag2/blob/5062b6e7caaa04eeeaf2e9546df737a814d5533b/rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp#L117C1-L130C4

Currently SequentialWriter will throw an exception if the target logging directory already exists. Would you be open to a PR to make this a bit more permissive, such as only throwing an exception if a metadata.yaml file exists?

I have some auxiliary log files that would be nice to have in the same directory and permitting this would remove the need to ensure that the rosbag logger always starts first.

oysstu avatar Jun 16 '24 10:06 oysstu

i think this is reasonable, currently too strict. the directory must not exist. i think directory can exist but it needs to be empty or metadata.yaml is not there?

fujitatomoya avatar Jun 19 '24 05:06 fujitatomoya

I agree that a directory can exist but it must be empty.
The reason for this limitation is that previous recording could be abnormally terminated due to some circumstances and recorder didn't save metadata.yaml file. However, the recorded bag files with data are there. In this case, the user can restore the metadata.yaml file via the ros2 bag reindex path/to/the/folder/with/bag/files CLI command.

@oysstu For your use case with some thirdparty log files preexisting in the same directory. From the rosbag2 recorder side, it is difficult to distinguish if those files are log files or previously saved bag data files. Because we support storage plugins and have no prior knowledges on upper recorder level about bag format and file extension in general case.

MichaelOrlov avatar Jun 19 '24 05:06 MichaelOrlov

Ah, yes, metadata.yaml isn't created until the recording is complete. In that case the writer would have to check if any bag files are present in the folder by checking for files with extensions matching the storage formats (BaseInfoInterface::get_storage_identifier). I agree that this may be cumbersome.

What if an empty metadata.yaml is created at the start of the recording, and subsequently filled out or overwritten afterwards? That would give a single point to check if a folder contains an ongoing or previous recording, assuming that the user has not manually deleted it of course.

oysstu avatar Jun 19 '24 08:06 oysstu

Hello! I'm going to coopt this issue for a slightly more generalized use case that still meets the original requirement. Hopefully that's all right, and if not let me know and we'll discuss it!

Please see the updated title and description for details.

emersonknapp avatar Feb 21 '25 22:02 emersonknapp