InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.76k stars 943 forks source link

Alarm gets dismissed by Notification #1223

Open ghost opened 2 years ago

ghost commented 2 years ago

Verification

What happened?

While an alarm was playing, a incoming notifications vibration dismissed the alarm.

What should happen instead?

The alarm should not be interrupted or even dismissed by the notification

Reproduction steps

  1. Set an alarm
  2. wait until alarm is vibrating
  3. get a notification on a paired device
  4. the alarm gets deactivated and the notification is shown on the watch

More details?

No response

Version

1.10.0

Companion app

No response

SuperPrower commented 2 years ago

I've tried looking into this, and here is how I see it going:

  1. We get an alarm, this starts an alarm app.
  2. We get a notification. Notification event starts a notification preview app, which causes current app (Alarm) to be destroyed: https://github.com/InfiniTimeOrg/InfiniTime/blob/95ff285991d399498d9bd7f60a503ef7665822ce/src/displayapp/DisplayApp.cpp#L305
  3. Alarm app destructor stops the alarm: https://github.com/InfiniTimeOrg/InfiniTime/blob/7f45538eb53235ab4015fcf13533796c8759c7bc/src/displayapp/screens/Alarm.cpp#L132-L135

Doesn't look like a bug for me, rather just an unintended interaction. Not sure how to make them play nicely.

minacode commented 2 years ago

The watch could have a global state that captures that the alarm is going. Going to the alarm app and turning off the alarm would reset that state. As long as it is on, the watch is vibrating. The problem right now is, that the alarm is only allowed to ring as long as the app is open.

We might need a clear hint to the alarm app though, or people would go crazy about how to turn off the vibrations 😄

SuperPrower commented 2 years ago

Going to the alarm app and turning off the alarm would reset that state.

I think a simpler variant could be to have app loader check if alarm is playing and not start notification preview app if it does. I don't believe any other app could hijack the screen like it. However, both this and the variable and checks on it sound like a complication that could be somehow avoided with some smart engineering.

minacode commented 2 years ago

Then the notification would go unnoticed. I don't like this either.

I think, the general problem is bigger than this issue, because notifications, phone calls, alarm, timer and metronome all use vibrations and we need a general strategy to resolve conflicts.

I think, we should decouple vibrations from the apps, save which patterns are active and run the one with the highest priority. Maybe, we could also jump to the according screen.

Example:

I would use the following priorities (high to low):

Timer and alarm are nearly equal for me, though.

Riksu9000 commented 1 year ago

The notifcation would not go unnoticed, if it is only delayed until the highest priority alert, alarm in this example, is dismissed. Vibrations should definitely not be decoupled from apps. Instead I think vibrations should be strictly screen specific. The vibration is alerting the user to pay attention to what is on screen. Decoupling this doesn't make sense.

thibaultmol commented 1 year ago

i'm very scared of missing an alarm because of this

minacode commented 1 year ago

I think the "new" app stack can solve this problem quite elegantly, if we have consistency for all relevant apps (e.g. alarm) implemented.

After the notification alert screen closes, the alarm app would be reopened and would continue to vibrate.

We only need to ensure that the vibration continues until it is explicitly turned off.