fix: timer runtime correctness, theming, and Shizuku UserService migration#39
Merged
Merged
Conversation
…ation Addresses a batch of bugs and improvements found in a full-project review. Timer runtime (core:service) - Anchor the countdown to a SystemClock.elapsedRealtime() deadline instead of accumulating delay() ticks, which pause during CPU deep-sleep and drift — a no-media timer no longer silently overruns. - Soft screen-off only injects KEYCODE_POWER while the screen is interactive, so "turn off display" can no longer wake an already-off device. - Harden service races: a generation counter stops a cancel teardown from killing a freshly started timer; ACTION_START over a running fade joins the old job and restores volume; the expiry teardown checks job identity so a "+ during fade" successor survives. - Post the notification only when the displayed minute changes (skip ~59 redundant re-posts/min); refresh +/- labels immediately when step size is edited mid-timer. Data (core:data) - ReplaceFileCorruptionHandler + IOException fallback so a corrupted settings file degrades to defaults instead of crashing the app at service start. - Single MAX_TIMER_MINUTES (240) source of truth; clamp persisted presets on read. Theming (feature:timer) - ProvideAppTheme derives a Material ColorScheme from the active AppTheme so dialogs/sliders/text-buttons follow the selected theme (incl. Light), and syncs system-bar icon contrast to the app theme rather than the system dark-mode flag. Dial (feature:timer) - Read time-varying values through rememberUpdatedState inside the pointerInput gesture lambdas, fixing stale captures when dragging a running timer. - Dial, hour-dots, and +/- caps all derive from MAX_TIMER_MINUTES (4h). Shizuku (core:service) - Migrate off the private, removal-announced Shizuku.newProcess to the official UserService API (IShellUserService AIDL + ShellUserService). ShizukuShell keeps its exec(vararg String): Boolean contract; command timeout is now enforced inside the user service. Proguard keep rules updated accordingly. Cleanups - Provide the device-admin ComponentName from the app module via a Hilt qualifier and inject it, removing three duplicated class-name string literals. - Delete the dead TimerPhase.FINISHED case; drop the unused lifecycle-service dep; ThemeId.values() -> entries; compareAndSet preset load to avoid a startup race. Verified: assembleDebug, lint, assembleRelease (R8 keeps verified in mapping), testDebugUnitTest all pass. Shizuku privileged actions need on-device verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-shizuku-hardening # Conflicts: # core/data/src/main/kotlin/dev/xitee/sleeptimer/core/data/repository/SettingsRepositoryImpl.kt # feature/timer/src/main/kotlin/dev/xitee/sleeptimer/feature/timer/settings/SettingsViewModel.kt
- ShizukuShell: unbind the pending ServiceConnection on bind timeout/cancel (invokeOnCancellation). A timed-out bind previously left the connection registered, so a late onServiceConnected orphaned a live binder — never cached, never unbound — for the app's lifetime. - SleepTimerService: ignore ACTION_ADD/SET that land in the post-expiry teardown window. With stopMediaPlayback off, onTimerExpired keeps the phase RUNNING through the Wi-Fi/BT/screen calls; a "+"/dial-commit there set a deadline no loop reads and was then wiped to IDLE. Guard on remainingMillis>0 (subtractStep was already covered by its <= stepMillis return). - ProvideAppTheme: key system-bar icon contrast off the rendered background luminance instead of the instant isDark flag, so the icons flip at the theme cross-fade midpoint rather than snapping washed-out at t=0. Equivalent at rest — every palette has bgSolid.luminance() < 0.5 exactly when isDark. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A batch of bug fixes and improvements from a full-project review. No architecture rewrite — the multi-module/Hilt/Compose/DataStore setup is already modern; these are targeted fixes to real defects plus supporting cleanups.
Bugs fixed
delay()ticks are uptime-based and pause during CPU deep-sleep, so a no-playback timer (e.g. only "screen off" enabled, unplugged) fired late. Now anchored to aSystemClock.elapsedRealtime()deadline and re-derived each tick.input keyevent 26(KEYCODE_POWER) is a toggle; if the screen was already off at expiry it turned it on. Now gated onPowerManager.isInteractive.pointerInput(Unit)never restarts, so its gesture lambdas froze theisRunning/runningMinutescaptures from first launch. Now read throughrememberUpdatedState.runBlocking { settings.first() }with no catch). AddedReplaceFileCorruptionHandler+IOException→ defaults.equals-identical UiState (admin flag wasn't in it), so a revoked grant never re-rendered. Admin flag is now part ofSettingsUiState; the Screen row shows a "permission missing" description when hard-lock is configured but revoked.ProvideAppThemederives a MaterialColorSchemefrom the activeAppThemeand syncs the system bars.Improvements
IMPORTANCE_HIGH+setOnlyAlertOnce) is intentionally unchanged. Step-size edits mid-timer now refresh the +/- labels immediately.Shizuku.newProcessis private and has an announced removal; replaced with anIShellUserServiceAIDL +ShellUserServiceper the official demo.ShizukuShellkeeps itsexec(vararg String): Booleancontract, so the Wi-Fi/Bluetooth/screen controllers are untouched. Command timeout is now enforced inside the user service. Proguard keeps updated (verified in the release mapping).MAX_TIMER_MINUTES = 240is now the single source for the dial, hour-dots, ViewModel, and all three service intents (previouslyADDwas unbounded and the running "+" never disabled).TimerPhase.FINISHEDcase; removed the unusedlifecycle-servicedependency;ThemeId.values()→entries;compareAndSeton preset load to avoid a startup race.Out of scope (per discussion)
AlarmManagerexact-alarm backstop for expiry in deep Doze — needs the exact-alarm permission + policy justification; the elapsedRealtime fix removes the drift, late-firing in deep Doze catches up at the next wake.Verification
assembleDebug,lint,assembleRelease(R8 keep rules confirmed inmapping.txt), andtestDebugUnitTestall pass locally.🤖 Generated with Claude Code