BangleApps icon indicating copy to clipboard operation
BangleApps copied to clipboard

recorder: resume logs recorded within 4 hours

Open bobrippling opened this issue 2 years ago • 5 comments

We now look at the latest log's last timestamp - if it was within four hours, we prompt the user to resume recording to that file.

This change also adds (manually runnable) tests for setRecording()

Fixes #3135

bobrippling avatar Dec 20 '23 22:12 bobrippling

Should it be clarified that the file name is in UTC not local time? This unlikely to affect people not living in GMT+-1

Very good point - I think that since we now look at the times within the file, the filename is less important (so long as it still sorts by date), so we could make it that the filename is in localtime (but keep the timestamps themselves in UTC), so when you look at the files they're in your timezone.


Additionally, I feel like forcing the recorder to append should always append to the latest file, regardless of the time difference

That makes sense - how does this sound as a proposed flow? Any tweaks/additions?

on recording start:

if any previous recorder log (containing data):
  if within 4 hours:
    prompt the user: append, new, overwrite
  else:
    prompt the user: append, new

"new" will create a filename based on the current date (in localtime)

(the prompt will also have the existing cancel option)

(I'll leave this PR open here - feel free to add changes on if you like, I'm not sure when I'll next be free to pick it up)

bobrippling avatar Dec 21 '23 18:12 bobrippling

I like the idea, but have you actually tested this in practise? Because iterating over the entire file to find the last line could take quite a long time (10+ seconds depending on the track length) and I think that would provide quite a poor experience if you user just wants to start a run.

At minimum it should at least check the filename date and if it's different, skip the check.

... plus this feels like quite a lot of extra complexity? I'm not sure really sure if it's worth it?

gfwilliams avatar Mar 14 '24 11:03 gfwilliams

Good point - @Mineinjava is this an issue you still see?

Either way I'd like to keep the tests (and fix the lint warnings), I'll split out to another PR if necessary (and if you agree the tests are good to have)

bobrippling avatar Mar 16 '24 10:03 bobrippling

Good point - @Mineinjava is this an issue you still see?

Either way I'd like to keep the tests (and fix the lint warnings), I'll split out to another PR if necessary (and if you agree the tests are good to have)

Honestly I think the best solution is to

  • tell the user the name of the last log
  • if they say "append," append regardless of timestamps

Mineinjava avatar Mar 25 '24 18:03 Mineinjava

Yeah that seems reasonable, I'll make some progress towards that as part of this PR

bobrippling avatar Mar 25 '24 22:03 bobrippling

This has been subsumed by #3431, closing

bobrippling avatar Jul 04 '24 17:07 bobrippling