InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Timer: let timer go overtime (with ringing)

Open hatmajster opened this issue 4 years ago • 16 comments

In short: Now timer can go overtime - after ringing counter will turn red and start counting up to show when exactly did the timer ring. This way if I didn't feel alarm,or just had other stuff to do, I can just look at the watch and see "oh, so the alarm rang 2 minutes ago, fine".

Description: When timer has ended and vibrated, the counter turns red and start counting up. It will turn off if user will press "stop" or close Timer application. It will not turn off if the screen will just go blank (but I've found during testing that notifications can turn red timer off - I don't know how to gracefully go around it, but it should be doable. For now left it as is). The timer will also turn off after 99:59 overtime, just to not mess with display.

This is actually most common behavior in modern timers I think. For example all smartphones have this I think.

This PR will conflict with https://github.com/JF002/InfiniTime/pull/554 and https://github.com/JF002/InfiniTime/pull/407 as clang-format reformatted all files, and also I did myself some minor cleanings.

PS1: I changed GetTimeRemaining to give seconds instead of milliseconds as that's what was used anyway, and changed the function name to GetSecondsRemaining. Hopefully with the name changed it won't break anyone's custom code.

PS2: currently counter in Timer shows floor(remainingSeconds), wouldn't it be better if it would show ceil(remainingSeconds)? I think that's what @Riksu9000 also did in https://github.com/JF002/InfiniTime/pull/554

Movie of awesome quality, but You should see slight red in the overtime (and "see" ringing as Pinetime moves each ring):

https://user-images.githubusercontent.com/85759394/140383812-65d13928-0915-4225-b559-ed6fcabb7f53.mp4

hatmajster avatar Aug 04 '21 16:08 hatmajster

PS2: currently counter in Timer shows floor(remainingSeconds), wouldn't it be better if it would show ceil(remainingSeconds)? I think that's what @Riksu9000 also did in #554

Correct.

Ringing will be added in #342/#544, which can be used here to make timer ending more obvious and less likely to get missed. Still I do think that showing overtime is a good idea.

Riksu9000 avatar Aug 04 '21 17:08 Riksu9000

Rebased, fixed conflicts, tested - works as before. Nothing changed since the beginning, as I don't really want to make this change any bigger.

hatmajster avatar Oct 28 '21 15:10 hatmajster

@Riksu9000 I think I've done all You asked, but I am not sure about time blinking, so I left it for now. In summary:

  • implemented stop() function
  • added ringing
  • matched app layout to Alarm

EDIT: I can write the blinking too, I just thought that it was unnecessary complicating code. But if more folks want it, I can implement that too ofc.

@JF002 I think this is one of the simplest most thumbed up change;) Can You review/give Your opinion (or both);) I've even added kinda crappy movie now, so hopefully its visible what it does ^^

hatmajster avatar Nov 04 '21 16:11 hatmajster

The layouts nice, but in the alarm app the stop button turns red when it's ringing. This is what I meant.

IMG_20211105_104210

This is what I had in mind about the overtime, and have just the text blink. But that's just an idea. I haven't tested it if it would be any good.

overtime

Riksu9000 avatar Nov 05 '21 08:11 Riksu9000

Works fine and is a great improvement. It would be nice if ringing continued after closing the app like the alarm app does.

Itai-Nelken avatar Dec 01 '21 20:12 Itai-Nelken

I think the ringing might actually make the overtime redundant. It's unlikely that the user would miss the timer ending now since the watch is worn on the wrist, compared to a timer on a phone which could get misplaced. So maybe we only need the ringing after all.

Riksu9000 avatar Jan 01 '22 15:01 Riksu9000

I'd be interested in the overtime thing nevertheless.

On January 1, 2022 4:30:59 PM GMT+01:00, Riku Isokoski @.***> wrote:

I think the ringing might actually make the overtime redundant. It's unlikely that the user would miss the timer ending now since the watch is worn on the wrist, compared to a timer on a phone which could get misplaced. So maybe we only need the ringing after all.

-- Reply to this email directly or view it on GitHub: https://github.com/InfiniTimeOrg/InfiniTime/pull/557#issuecomment-1003574313 You are receiving this because you are subscribed to this thread.

Message ID: @.***>

M4xTheMan avatar Jan 01 '22 16:01 M4xTheMan

I think the ringing might actually make the overtime redundant. It's unlikely that the user would miss the timer ending now since the watch is worn on the wrist, compared to a timer on a phone which could get misplaced. So maybe we only need the ringing after all.

i thinks the overtime is useful because it allow to see how much have overtime (if you cook something , overtime of 1 minute is not thze same than 15 or more)... it even more true if it's something that need to know how much overtime have been done in order to adjust the rest of the steps

trman avatar Jan 01 '22 17:01 trman

I'm thinking that because the timer ending is now very obvious, the user wouldn't miss it in the first place. Because of this, the timer won't be left going into overtime accidentally.

As for letting it go to overtime intentionally, I don't think people will want to keep the watch vibrating on their wrist for minutes just to keep track of overtime. Though if this is important, maybe there should be a timeout and/or a mute button, but let's leave that for another PR.

Riksu9000 avatar Jan 01 '22 17:01 Riksu9000

Maybe simply stop the vibrations after a certain amount of time, for example 10 seconds.

Itai-Nelken avatar Jan 01 '22 18:01 Itai-Nelken

To me overtime is an important feature like trman said, during cooking, baking I take it into consideration constantly. Its maybe even more important than missing the timer as I usually just check it constantly. I added now the 10 seconds for stopping vibrations requested by Itai-Nelken, as indeed ringing all the time was a bit of annoyance. I am still wondering if overtime timer should stop after closing application. It is sometimes happening and then the information lost... On the other hand I don't like clicking on watch that much, and everything currently is just so clean;)

EDIT: uh actually just noticed that 10 seconds limit doesn't work when the screen goes blank. Have to figure something different...

hatmajster avatar Jan 25 '22 17:01 hatmajster

Okay, I'm willing to approve this PR with or without the 10 second timeout, as it's an improvement either way. I may make further improvents to this in the future.

If you want to fix the timeout, you'll need to disable sleeping while the timer is ringing. This is because the apps don't run while the screen is off, so Refresh() isn't called. Alternatively I think you can use FreeRTOS timers, which keep running while the screen is off. You can take a look at #945, where I made something similar for the Alarm app. Now I'm wondering which solution is better..

Riksu9000 avatar Jan 26 '22 20:01 Riksu9000

To me it doesn't matter much if the screen should be on or off during vibration, I can have/do it either way. Right now in newest commit, I implemented vibration for 10 times in motorController. Seems to be working good, and I like this way. You, Riksu could reuse it in Alarm or any other app the same way. However, it introduces a little rare race, so it maybe better to do it differently (like You already did), do outside of this PR for now or scrap completely. The problem is when the timer is in overtime and a call will happen, it will often stop ringing. This is because Timer app and Notifications app stops vibration on closing. If timer/notification don't sync we have stopped ringing and can miss a call:/ Now that I think of it I don't know if previous implementation was secured against this anyway... I think not.

hatmajster avatar Jan 27 '22 20:01 hatmajster

Hmm, I think I would rather have the app control the ringing directly rather than leave it being controlled separately in MotorController without feedback.

There is already a bug where the ringing stops when receiving two call notifications for example, so I don't think it was caused by your changes.

Riksu9000 avatar Jan 28 '22 10:01 Riksu9000

Note: This PR is open so long that in the meantime the alarm apps design got matched to the timer. @hatmajster do you plan to finish the work on this? @Riksu9000 Are you happy with the current implementation?

ghost avatar Jul 28 '22 01:07 ghost

I would like to see some changes made, but there are also a lot of conflicts making it difficult to update and I'd have preferred the overtime feature to be separate.

Riksu9000 avatar Jul 28 '22 05:07 Riksu9000