Alarm: Simplify alarm alerting screen (and fix bug with alerting on time value change)

[ljahn]

Fixes #2206

When Alerting, the handlers for swiping and hard- and software buttons correctly deal with this.
The handler for time value changes did not.

Now a value change will also stop the alert.
For the scenario presented in #2206 (makeshift snooze), this means that after setting a new time, the new alarm also needs to be enabled again. However, I think this is not a problem, because the UI clearly presents that the alarm is disabled and it can be re-enabled exactly like setting an alarm always works.

Edit: Now fixing the problem by making the value change impossible in the first place by simplifying the alerting screen.

[github-actions]

Build size and comparison to main:

Section	Size	Difference
text	372880B	64B
data	948B	0B
bss	22536B	0B

[mark9064]

Though I hate to break the makeshift snooze, I think it's the right way to go here as the UI should be consistent with disabling the alarm anytime the parameters are changed

Sorry I misread what this is doing. I think it's a bad idea to stop the alert on changing the time values, since those buttons are large and maybe too easily activated by the sensitive touch panel. Maybe it would be best to disable all buttons other than the stop button when ringing? It doesn't make too much sense to me what the semantics of changing the time on a ringing alarm are

I'd like to understand why it's currently broken though - did you manage to get to the bottom of this?

[ljahn]

I think the reason is this:
SetAlerting creates an lv_task to automatically call StopAlerting after one minute. This task will call an invalid function reference and lead to a crash under the following condition:

All exit paths but the time value change (so not considering this fix) call StopAlerting themselves, which also terminates the lv_task. However, the value change callback only calls DisableAlarm, because its normal use case is for setting up an alarm, where you have to re-confirm enabling the alarm after every change you make. DisableAlarm still sets isAlerting in the alarmController to false, probably because someone thought a currently alerting but also disabled alarm makes no sense, this was introduced in a0cd439. That causes the destructor of Alarm to think there is nothing to do regarding the alerting when the alarm screen is dismissed. Therefore it does not call StopAlerting and the lv_task is left with an invalid function pointer, because Alarm does not exist anymore once the lv_task finally goes to call the callback function.

[ljahn]

I also think we should do what you said and fully disable the time change buttons while the alarm is ringing. While we are at it, lets go one step further and disable all other buttons as well. The info button does not do anything useful in that situation anyway, it shows some meaningless value, and why should you be able to change the recurrence when you are not allowed to change the time. So in line with #2082, I would make the alarm disable screen have only one button, the Stop button.

I updated the PR to reflect this. This is what it looks like:

Pressing the big red or the physical button brings you back to the normal alarm settings display and stops the alarm.

#2082 also calls for a snooze function, but this leads to additional questions that would warrant an own pull request and discussion.

[mark9064]

Really nice analysis of the root cause, thanks for writing it up :)

The new UI looks good to me, agree that snooze can go in a separate PR (and the stop button could easily be half width with snooze for the other half, so that should work)

Code at a glance looks sensible, I should be able to review it properly soon