Skip to content

Run on background#33

Open
imiziuk wants to merge 10 commits intoChicoState:mainfrom
imiziuk:main
Open

Run on background#33
imiziuk wants to merge 10 commits intoChicoState:mainfrom
imiziuk:main

Conversation

@imiziuk
Copy link
Copy Markdown
Contributor

@imiziuk imiziuk commented Apr 13, 2026

No description provided.

@SynthNibbler SynthNibbler self-requested a review April 13, 2026 16:52
Copy link
Copy Markdown
Contributor

@smennings smennings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works! Looking good

Copy link
Copy Markdown
Contributor

@SynthNibbler SynthNibbler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The permissions popups look very clean and make sense to ask the user for permissions before forcing the features. The permissions are separated and chained one after another so the user can set these up quickly at the start and then continue using the app. This flow makes sense. The code is well commented with descriptions of what the code is doing.

This PR appears to be combining a feature addition with a large refactoring of most of the project. It is also bundled with some unrelated maintenance changes, such as two python scripts. One of these scripts is described as locating android/settings.gradle for fixing a Notifee build error, but it is unclear why this solution was needed. The other python script is deleting some dependencies and editing the .gitignore file.

After running the app to test the behavior, it has overlapping popups when creating an alarm, this can be replicated consistently. When closing the app it stalls for a few seconds with a "saving state" popup before actually closing.

Before this gets merged the feature related code should be separated from the maintenance code. The overlapping popups would also need to be fixed.

@imiziuk imiziuk reopened this Apr 27, 2026
@SynthNibbler SynthNibbler self-requested a review April 29, 2026 16:46
@SynthNibbler
Copy link
Copy Markdown
Contributor

This is a much tighter PR with a better focus of what features are requesting to be merged, it is still several features packaged though so not all features have been reviewed yet. I did test 3 of them so far, creating a new batch and the interval alarms triggering and lockscreen alerts.

This version has a bug with the battery optimization permissions. It offers to update settings but triggers the notification on each "Create Alarm" no matter what the battery optimization permission is set to. I included a video attachment to demonstrate this behavior.

The Notification for the alarms works, it does notify on each interval for the set, but it only pings like a notification, it does not ring an alarm. The behavior for the alarm triggering a pop up and sound until dismissed is removed. The alarm needs to ring until the user dismisses it, not just a ping like a text message. They do correctly work if the app is in the background.

The battery optimization flow needs to be fixed so the user can set it or skip it and it does not trigger again.
The alarms need to ring until ended not just ping notifications.

BatteryOpt_loop.mp4

Copy link
Copy Markdown
Contributor

@SynthNibbler SynthNibbler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! New features seem to work as intended and works well consistently.

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.

3 participants