Skip to content

Conversation

@naveensingh
Copy link
Member

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Fixed IllegalStateException in ClockFragment
  • Added a check to ask for notification permission on startup but only when there is at least one enabled alarm. This should help with cases like this.
  • Removed invalid check in MainActivity so that alarms are properly rescheduled on startup. Before this change:
    • Set an alarm
    • Force stop the app (or any perform any action that could clear the scheduled alarms)
    • Reopen the app (so alarms are rescheduled)
    • Notice that the alarm won't ring at all i.e. not rescheduled by the previous step. More details in the commit message.

Fixes the following issue(s)

Acknowledgement

Fixed crash `java.lang.IllegalStateException` in `updateAlarm()``
It seems faulty code was originally introduced in SimpleMobileTools/Simple-Clock#469

Before that PR, the code rescheduled enabled alarms when there were no scheduled alarms according to the system APIs.

After that PR, the code rescheduled enabled alarms when there were NO enabled alarms enabled in the database i.e. a contradiction.

See #89
Previously, the app didn't ask for notification permission unless the user tried to enable/disable an alarm. This is a problem in case the notifications were turned off (for whatever reason) after an alarm was configured. The user would think that everything is ok and miss their alarm.

#89 (comment)
Same issue as 7d35b04.

Original code relied on the scheduled alarms returned by the system as a filter but the updated code by SimpleMobileTools/Simple-Clock#469 clears/disables expired alarms when there are no enabled alarms which doesn't make much sense. One can not disable already disabled alarms
 - Moved duplicated code to an extension
 - Removed alarm clearing logic from alarms fragment as it's not necessary there. Instead, the alarms fragment is refreshed by `disableExpiredAlarm()`.
 - Disabled expired alarms when alarms are auto dismissed as well.
Adapter was being passed the same list of alarms over and over
@naveensingh naveensingh added testers needed We need testers for this issue or pull request and removed testers needed We need testers for this issue or pull request labels Mar 17, 2025
@naveensingh naveensingh merged commit fc82299 into master Mar 18, 2025
4 of 6 checks passed
@naveensingh naveensingh deleted the ask_notification_permission branch March 18, 2025 11:01
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.

2 participants