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

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

Open
oysstu opened this issue Jun 16, 2024 · 3 comments

Comments

@oysstu
Copy link

oysstu commented Jun 16, 2024

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.

@fujitatomoya
Copy link
Contributor

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?

@MichaelOrlov
Copy link
Contributor

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.

@oysstu
Copy link
Author

oysstu commented Jun 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants