-
-
Notifications
You must be signed in to change notification settings - Fork 4
Added custom alarm behavior to show the custom activity based on App/Device active/inactive state #23
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
Conversation
…tom activity when they're in the foreground state or not
…value of `shouldShowAlarmActivityWhenAppIsActive`
…hen the user is using the App
…the awake state or not
…ser's current App is running or not.
WalkthroughThis update introduces two new configuration options to control whether the custom alarm activity should be shown when the device is active or when the app is in the foreground. The options are integrated into the configuration, persistence, and service logic. Additionally, version numbers are incremented to 1.0.0, and minor documentation and comment improvements are made. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant TriggerXConfig
participant TriggerXPreference
participant TriggerXForegroundService
App->>TriggerXConfig: Set shouldShowAlarmActivityWhenDeviceIsActive / showAlarmActivityWhenAppIsActive
App->>TriggerXPreference: Save config (including new flags)
Note right of TriggerXPreference: Persist new Boolean flags
TriggerXForegroundService->>TriggerXPreference: Load config
TriggerXForegroundService->>TriggerXConfig: Read flags
alt Device is interactive and flag disallows
TriggerXForegroundService-->>TriggerXForegroundService: Stop service early
else App is foreground and flag disallows
TriggerXForegroundService-->>TriggerXForegroundService: Stop service early
else
TriggerXForegroundService->>TriggerXForegroundService: Continue alarm processing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (6)
triggerx/build.gradle.kts (1)
103-110: Version bump looks correct, but consider removing hard-coded duplicates.
coordinates(... "1.0.0")and the explicitpom.version = "1.0.0"will need to be kept in sync with every future release, in addition toVERSION_NAMEin the workflow. A single source of truth (e.g.,VERSION_NAMEdeclared in a gradle property and referenced here) eliminates the risk of drifting values..github/workflows/release.yaml (1)
9-10: Workflow still hard-codes the version string.Now that the Gradle build script also carries a literal
"1.0.0", the release process relies on two separate places being updated together. To avoid the inevitable mismatch at some point, fetch the version fromgradle.properties(already created in the “Setup gradle.properties” step) and reuse it as the env variable instead of duplicating it here.triggerx/src/main/java/com/meticha/triggerx/dsl/TriggerX.kt (1)
74-78: Well-implemented internal helper functions.The functions correctly provide access to configuration flags with sensible fallback defaults. The
truedefaults ensure backward compatibility when configurations are not explicitly set.Consider adding brief KDoc comments for better maintainability:
+ /** + * Returns whether alarm activity should be shown when app is in the foreground. + * Defaults to true if not configured. + */ internal fun showAlarmActivityWhenAppIsActive(): Boolean = config?.showAlarmActivityWhenAppIsActive ?: true + /** + * Returns whether alarm activity should be shown when device is active/unlocked. + * Defaults to true if not configured. + */ internal fun showAlarmActivityWhenDeviceIsActive(): Boolean = config?.shouldShowAlarmActivityWhenDeviceIsActive ?: truetriggerx/src/main/java/com/meticha/triggerx/dsl/TriggerXConfig.kt (2)
62-66: Fix documentation typos and improve clarity.There are grammatical errors and the documentation could be clearer about what "device is active" means.
/** - * This is used to know if the user's device is in the unlocked state, then weather they - * want to show the custom alarm dialog or not. + * Controls whether the custom alarm activity should be shown when the device is active/unlocked. + * When false, the alarm activity will not be shown if the device is currently interactive. */ var shouldShowAlarmActivityWhenDeviceIsActive: Boolean = true
68-72: Fix documentation typos and improve naming consistency.There are grammatical errors and inconsistent naming with the other property.
/** - * This is used to know that if the user's App is running (in the foreground state), - * then weather they want to show the custom dialog or not. + * Controls whether the custom alarm activity should be shown when the app is in the foreground. + * When false, the alarm activity will not be shown if the app is currently visible to the user. */ - var showAlarmActivityWhenAppIsActive: Boolean = true + var shouldShowAlarmActivityWhenAppIsActive: Boolean = trueNote: This naming change would require updating the corresponding references in other files for consistency.
triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt (1)
87-87: Update comment to be more comprehensive.The comment mentions checking "if their App is open or not" but doesn't mention the device active state check that's also being added. Consider updating it to cover both conditions.
- * - Checks if the user wants to show the activity if their App is open or not + * - Checks user preferences for showing activity based on app foreground state and device active state
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/release.yaml(1 hunks)app/src/main/java/com/meticha/triggerxexample/TriggerXApplication.kt(1 hunks)triggerx/build.gradle.kts(1 hunks)triggerx/src/main/java/com/meticha/triggerx/TriggerXAlarmScheduler.kt(2 hunks)triggerx/src/main/java/com/meticha/triggerx/dsl/TriggerX.kt(1 hunks)triggerx/src/main/java/com/meticha/triggerx/dsl/TriggerXConfig.kt(2 hunks)triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXPreference.kt(4 hunks)triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt(5 hunks)
🔇 Additional comments (6)
app/src/main/java/com/meticha/triggerxexample/TriggerXApplication.kt (1)
36-36: LGTM! Clean demonstration of the new configuration option.The addition of
shouldShowAlarmActivityWhenDeviceIsActive = falseproperly demonstrates the usage of the new feature, showing how developers can configure the alarm behavior based on device state.triggerx/src/main/java/com/meticha/triggerx/TriggerXAlarmScheduler.kt (2)
60-61: Improved comment clarity.The updated comments provide better context about optional user notification and permission handling flow.
79-79: More precise AlarmClockInfo documentation.The clarification that alarm clock behavior is "shown in the status bar" is more accurate and helpful.
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXPreference.kt (1)
22-22: Proper import addition.The
booleanPreferencesKeyimport is correctly added for the new boolean preference functionality.triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt (2)
19-19: LGTM: Import added for ActivityManager.The import is correctly added to support the new functionality for checking if the app is in foreground.
201-201: LGTM: Improved NotificationManager service retrieval.Using
getSystemService(NotificationManager::class.java)is more type-safe than casting from string service name.
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXPreference.kt
Outdated
Show resolved
Hide resolved
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXPreference.kt
Outdated
Show resolved
Hide resolved
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXPreference.kt
Outdated
Show resolved
Hide resolved
triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt
Outdated
Show resolved
Hide resolved
triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt
Outdated
Show resolved
Hide resolved
triggerx/src/main/java/com/meticha/triggerx/services/TriggerXForegroundService.kt
Show resolved
Hide resolved
…es where the && conditions were not properly setup
|
Fixes #21 |
Summary by CodeRabbit
New Features
Improvements
Other