InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Added repeat option to timer

Open clay53 opened this issue 4 years ago • 10 comments

Closes #918 I moved the play/pause button to the left and put a "Repeat" button next to it which just toggles repeating when it's pressed. When repeating is disabled, it follows the default theme for buttons and the timer behaves exactly as it did before but when repeating is enabled, it turns green and when the timer ends, the timer is started again at whatever duration it was started at last. I thought about modifying the TimerController to get repeating working by changing the timer mode to APP_TIMER_MODE_REPEATED but I has trouble getting it working. So currently, the timer app itself restarts the timer when repeating is enabled. I made it so that repeating can be toggled even when the timer is enabled since having the button not work but still visible as if it did when the timer was running seemed weird to me. I also didn't want to make it not visible because then it would seem the play/pause button was off to the side for no reason when the timer was running and I didn't want the play/pause button to move when the timer started. I also didn't have another good place to put the repeat button.

I tested this on a sealed device so I don't have any debugging info for it and I haven't done C++ in years but as far as I can tell, everything is well so this can be merged once reviewed.

https://user-images.githubusercontent.com/16981283/148719309-8b7bca0b-d34f-4b71-9bec-4ade939901fc.mp4

clay53 avatar Jan 10 '22 04:01 clay53

@clay53 Hi. Can you take a look at my review?

Riksu9000 avatar Feb 08 '22 09:02 Riksu9000

@clay53 Hi. Can you take a look at my review?

Sorry, I've been very busy. I'll try for this Saturday.

clay53 avatar Feb 08 '22 11:02 clay53

@Riksu9000 Did what you said, just wanted to note that (for both iterations) I noticed that the repeat option would be lost if the app was exited. Do you think it should be persistent (like is that worth it or even desired)?

clay53 avatar Feb 13 '22 05:02 clay53

Because the setting doesn't persist, if I exit the app while a repeating timer is running, it will notify once and won't repeat anymore. I also think that for the use case you suggested in #918, the watch shouldn't wake up when repeating is on. Thus I think this option should be added in TimerController, so in SystemTask it can be made to not wake up when repeating is enabled. https://github.com/InfiniTimeOrg/InfiniTime/blob/4f649a85448fc79b55202d9db0e8f4c8828975e3/src/systemtask/SystemTask.cpp#L313

We also need to figure out if the repeating timer should keep running after the app is closed. If it doesn't, someone might accidentally close the timer they meant to set, but if it does keep running, should the timer app open each time the timer ends or not?

A repeating timer seems like a completely separate use case from a normal timer, so maybe it could be a separate app? Maybe the Metronome app could be used for this actually.

Riksu9000 avatar Feb 14 '22 13:02 Riksu9000

That would be a great feature for interval training. How is the status here, is it possible that this feature is part of the next versions of PineTime?

PetersOtto avatar Feb 18 '22 08:02 PetersOtto

That would be a great feature for interval training. How is the status here, is it possible that this feature is part of the next versions of PineTime?

The current version works without any trouble on my fork. You can clone it from there and figure out how to build it. Only thing I had trouble with after following the instructions on the GitHub was finding the actual build script. I created my own script to overlay everything that had to be done for it (you'll need Linux, pyenv, make, and adafruit-nrfutil from pip for my script - other deps in instructions and might be duplicated here):

source .venv/bin/activate
cd build
make -j pinetime-mcuboot-app
cd ..
python ./tools/mcuboot/imgtool.py create --align 4 --version 1.8.0 --header-size 32 --slot-size 475136 --pad-header ./build/src/pinetime-mcuboot-app-1.8.0.bin image.bin
adafruit-nrfutil dfu genpkg --dev-type 0x0052 --application image.bin ~/Downloads/DELETE/dfu.zip

change the ~/Downloads/DELETE/dfu.zip if you want it to put the package somewhere else. You can then take that package and flash your watch with it. I just copy it over to my Android phone and flash it with Gadgetbridge. Probably a better system but that's what I've been doing.

I imagine it'll be included in future versions.

Edit: Slipped my mind that I could just like, attach the package and you could flash it (generally don't do this - recommended you build it yourself or get it from a source you trust): dfu.zip .

clay53 avatar Feb 18 '22 23:02 clay53

I think this option should be added in TimerController, so in SystemTask it can be made to not wake up when repeating is enabled.

I agree with this

should the timer app open each time the timer ends or not?

I don't see much of a reason for it too keep running but if it does, the only reason, I'd think, someone would close the app while it's running to like check a notification or something it opening again would be interrupting. But, maybe there should be some kind of UI thing to indicate that in case someone doesn't realize they've set it to repeat.

A repeating timer seems like a completely separate use case from a normal timer, so maybe it could be a separate app?

Making it a separate app could be better so that we can have different "gestures" and flow for it. One thing I'd like to have is to have it so the physical button would start/stop the timer especially when it's cold and I have to take gloves off to press the touch screen or we're starting as a group and I need to start my timer quickly. Another thing is that it currently remembers the time you pause it at instead of the time it last started at.

When I made this app for my PebbleTime (also my first C project), I used a tap of the middle button to start/stop and a long press to reset it to the time it was previously started at. I also had it work as a stopwatch when you set the interval to zero so maybe it would be better integrated into the stopwatch app? The back button was used exit the app but the PineTime doesn't have a back button so maybe this could be an onscreen button? I worry about sweat accidentally exiting though - maybe have a confirmation screen?

Maybe the Metronome app could be used for this actually.

I feel like the Metronome app is also a completely separate use case. A lot of things would have to be changed in it to make it work like a proper interval timer.

clay53 avatar Feb 18 '22 23:02 clay53

I do think the repeating timer should stop when closing the app, but my main concern is that someone makes a 10 min timer, closes the app and forgets about it, but they accidentally left repeat mode on, so it would be stopped. If use cases of an independently repeating timer only require at most 60 seconds for example, we could disable the minutes when repeating is enabled. Then the user would have to disable the repeating to set the timer to more than a minute. What do you think?

@clay53 Did you enable -DBUILD_DFU=1 when generating the project with cmake? Github also builds these PRs as checks and the DFUs can be grabbed from there.

Riksu9000 avatar Feb 19 '22 10:02 Riksu9000

Yesterday and today I test the repeat-function at the training. It is a great feature. There is no need to have an eye on the clock and it is possible to here music during the training because the timer use the vibration-alarm.

PetersOtto avatar Feb 20 '22 09:02 PetersOtto

I just want to make it clear that at this point, I am not working on this feature so someome else is clear to work on it. I haven't been keeping up with the latest updates to InfiniTime (iirc there was a timer rework) since I've switched back to my Pebble Time for more reasons than just this feature so maybe something similar's already been added idk.

clay53 avatar Sep 05 '22 00:09 clay53