-
-
Notifications
You must be signed in to change notification settings - Fork 1k
timer: Add ringing and counter #1971
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
base: main
Are you sure you want to change the base?
Conversation
|
Build size and comparison to main:
|
0033e6c to
9136594
Compare
6bd64b5 to
ebc5b50
Compare
|
Works great, huge usability improvement for the timer. Been running this for a while and it makes me a more consistent cook for sure! I think it would be nice if the alarm and timer had different vibration patterns, but vibration needs to be overhauled anyway so this is good to go for now IMO |
mark9064
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see why you changed this (count up after ringing), but we probably want to avoid relying on UB
|
Tested this today while cooking and this is indeed a great usability improvement for the timer. Until now i missed a lot of the alarms, the vibration is just too short. Great work. |
4ba3e90 to
b3a6d74
Compare
9de08c1 to
6930303
Compare
4352f03 to
d2e548a
Compare
11fd07e to
ec4838b
Compare
|
It's a bit hidden so just in case I responded to the timer method semantics problem here: https://github.com/InfiniTimeOrg/InfiniTime/pull/1971/files#r2492311746 Would be super great to get this in for 1.16 which we're hoping to release in the next few days :) |
8ad9ed8 to
7002f68
Compare
|
I've just tested this PR and it looks very good, thanks @vkareh ! See this video : https://github.com/user-attachments/assets/2d6ba5c3-43c0-42fa-92e8-1779267c42b2 |
|
I think this is the correct behaviour since the time always rounds to zero. so you should expect it to show 0 for 2 seconds (from time +1 to -1) |
|
@mark9064 Good point, that makes sense! 👍 |
|
Could it be possible to add an option to ring indefinitely (in many cases people will not notice the 10 seconds ringing, especially if the timer is used during physical activity/sports)? |
|
I totally get where you're coming from but I think it'd be best to choose one sensible default here. At least how I've been using the timer 10s ringing has felt good to me, even if I'm moving about I definitely notice it before it stops. I'm not sure if you've had the chance to try this PR yet, but if not maybe give it a go and see how you find 10s? Maybe it's more noticeable than you'd think :) |
7002f68 to
3a25944
Compare
|
@mark9064 - just rebased. Had to fix some conflicts, but it all led to basically dropping the last commit, as it became a no-op 🎉 |
The timer app issues a short buzz once and then disappears. There is no trace left that the timer finished or how long ago. This change makes the motor start ringing and presents a timer counter. The timer stops buzzing after 10 seconds, and finally resets after 1 minute.
This prevents the motorController from buzzing infinitely while the watch is sleeping.
3a25944 to
1d98a9a
Compare
mark9064
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd really like to see this use the expired field of the timer rather than tracking that state externally. I've pointed out a few consequences of doing that but there's definitely more changes that would follow
However I don't think this is a release blocker, it could be refactored later
These are really good suggestions. I've implemented them and it made the code a lot cleaner. Thanks! |
Consolidate timer ringing logic and use Timer component as single source of truth for expired state.
0c74315 to
1b72a3b
Compare
The timer app currently issues a short buzz once and then disappears. There is no trace left that the timer finished or how long ago. This change makes the motor start ringing and presents a timer counter. The motor will continue the ringing pattern for 10 seconds and the timer counter will finally reset after 60 seconds.