-
-
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
e955f54 to
44389c6
Compare
44389c6 to
684acc3
Compare
54d0f5d to
4352f03
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 :) |
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.
The timer was calling xTimerGetExpiryTime() after the FreeRTOS timer had expired, which results in undefined behavior according to the FreeRTOS documentation. This change captures the expiry time in the TimerCallback when the timer expires, using xTaskGetTickCount() instead of the undefined xTimerGetExpiryTime() on a stopped timer. The expiry time is now captured before the timer stops running.
|
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 :) |
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.