Skip to content
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

Improved alarm alert handling #945

Merged
merged 1 commit into from
Feb 13, 2022
Merged

Improved alarm alert handling #945

merged 1 commit into from
Feb 13, 2022

Conversation

Riksu9000
Copy link
Contributor

@Riksu9000 Riksu9000 commented Jan 18, 2022

The alert can now be stopped by pressing the physical button.
The Alarm app now can't be closed by swiping while alerting, so it won't be closed accidentally.
Closing the alarm app by long pressing the physical button will now stop the alerting.
The alarm will stop automatically after 60s.
Sleeping is disabled during alerting, which is also required for the LVGL task to stop the alert automatically.
Added ReloadIdleTimer() to the SystemTask message EnableSleeping to restart the idle timer when there is no user interaction. Already fixed.

Closes #935

@JF002 JF002 added this to the 1.9.0 milestone Jan 19, 2022
@Riksu9000
Copy link
Contributor Author

Should sleep be disabled, or should we use FreeRTOS timers instead, which can run while the screen is off?

@JF002
Copy link
Collaborator

JF002 commented Jan 26, 2022

Should sleep be disabled, or should we use FreeRTOS timers instead, which can run while the screen is off?

Do you mean disable sleep when the alert is alerting (vibrating during max 60s) ?

@Riksu9000
Copy link
Contributor Author

Riksu9000 commented Jan 26, 2022

Should sleep be disabled, or should we use FreeRTOS timers instead, which can run while the screen is off?

Do you mean disable sleep when the alert is alerting (vibrating during max 60s) ?

Yes. Currently (in this PR) sleep gets disabled only during the alerting, which allows the lvgl task to eventually stop the alerting. If I used a FreeRTOS timer instead, I believe I wouldn't need to disable sleep. I think that if in the future there was a special screen for the alarm and timer ending, then it shouldn't sleep during the alerting, but currently I'm not sure. It might be a bit weird for it to keep vibrating after the screen went blank though.

@JF002
Copy link
Collaborator

JF002 commented Jan 31, 2022

It might be a bit weird for it to keep vibrating after the screen went blank though.

That's also what I think... If the display is open on the alarm app, user will have a better chance to understand why it's vibrating.

@medeyko
Copy link
Contributor

medeyko commented Feb 5, 2022

I'd prefer to have an option to change period after the alert sleeps. Ok, let it be 60 seconds default, but for some important events I need to know that I won't miss them. It this case I wish to increase the period to infinity (in fact while there's a battery power) be absolutely sure.

I understand, that 60 seconds would almost certainly enough, but it's still uncomfortable sometimes.

On the other hand, some people suggested that for some environments it is preferable to have a shorter period.

So I think that settings allowing to set the period would be a right thing.

@JF002
Copy link
Collaborator

JF002 commented Feb 6, 2022

I understand that a single timeout value won't fit everyone's need, but we have to choose a default value that will fit most of the use-cases.

Settings might look as a good compromise to fit more use-cases, but I don't think we should add Settings for each and every apps and features in InfiniTime. Too many settings will make the whole system confusing and not easy to use. Also, we are constrained by the FLASH, RAM and display space, which are all already well packed.

@Riksu9000
Copy link
Contributor Author

Reading #935, I don't see anyone mention why the timeout should be adjustable, or why it should be any less than 60s. The only purpose of this timeout is to stop the alerting in case the watch is not worn, so it won't ring for the whole day. This is why I picked a very long duration, though 30 might also be enough. This is also the reason an infinite ringing option probably isn't a good idea.

@medeyko
Copy link
Contributor

medeyko commented Feb 6, 2022

@Riksu9000
I didn't find this at #935 as well, but I remember that there was some discussion, where someone mentioned a use case when one needs to know when the time is elapsed during a meeting (but doesn't wish to touch the watch). For this use case 10 seconds are enough, and a long vibration may be disturbing.

The only purpose of this timeout is to stop the alerting in case the watch is not worn

Just a thought: when the watch is not worn, the accelerometers show all zeroes for a long time. Even sleeping, people are moving a lot, so it's quite easy to distinguish the situations.

So it may be useful to have some system-wide function that says if watch is worn. It may be useful not only for alarms.

@JF002

Also, we are constrained by the FLASH, RAM and display space, which are all already well packed.

Understandably. Ok, you're right. I should have learned how to make my custom builds eons ago to be able to tweak generally subtle constants changes that however important for me personally. :)

@JF002 JF002 merged commit 4f649a8 into InfiniTimeOrg:develop Feb 13, 2022
@Riksu9000 Riksu9000 deleted the alarm_improvements branch February 13, 2022 10:38
@Itai-Nelken Itai-Nelken mentioned this pull request May 2, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stopping Alarm Vibration
3 participants