Skip to content

Conversation

kaatrasa
Copy link
Member

Tested on Ubuntu 20.04 LTS with OAK-D (Python), Orbbec Femto Mega and Azure Kinect.

@kaatrasa kaatrasa requested a review from oseiskar January 19, 2024 14:06
if args.auto_subfolders:
import datetime
autoFolderName = datetime.datetime.now().strftime("%Y%m%dT%H%M%S")
autoFolderName = datetime.datetime.now().strftime("%Y-%m-%d_%H-%M-%S")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the Python recording tool's datetime -> string formatting a little (to match other wrappers). Or do you prefer the old version, and I should use that in the other wrappers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. The old format is the standard ISO8601 datetime format and can be parsed in many programming languages with some method that mentions that identifier (it can also optionally handle time zones etc. unambiguously). However, the proposed format is easier to read (by humans) and I think this is more relevant in this context.

Comment on lines +127 to +128
else if (argument == "--auto_subfolders")
autoSubfolders = true;
Copy link
Member

@pekkaran pekkaran Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the automatic creation of timestamped folders is the most commonly desired mode of operation, so it's a bit awkward that you need to set this parameter to get that behavior. Instead, the argument could be removed by making it work like this:

  • If recordingFolder is set, record to that directory without any subfolders (overwrites if run twice). This allows the user to save data exactly where he wants, as he can create the parent folder himself.
  • If recordingFolder is not set, save in data/%Y-%m-%d_%H-%M-%S. It probably does not matter much that the user can't set name of the parent folder, but you could also add an optional argument to set that path.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it works like this:
If --output is set, then recordings are saved to output
If --output is not set, then recordings are saved to data/timestamp (default)
If --output --auto_subfolders is set, then recordings are saved to output/timestamp

@kaatrasa kaatrasa merged commit 91207c1 into main Jan 22, 2024
@kaatrasa kaatrasa deleted the recording-tool-default-directory branch January 22, 2024 12:43
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

Successfully merging this pull request may close these issues.

3 participants