Conversation
… This works by changing the screen timeout setting to a very short value, and clearing the FLAG_KEEP_SCREEN_ON flag, so the power manager turns the screen off. When the screen is to turn on, a wake activity turns it back on, and the keyguard lockscreen is dismissed, and the user's original screen timeout value is restored.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persisted screen timeout, dim/undim and wake handling with snooze logic (MainActivity + Settings), new preferences and UI tweaks (screenTimeout, keepScreenOn, screenDimmingCategory), device-admin XML, manifest WRITE_SETTINGS and exported WidgetConfigActivity, Gradle plugin addition, and app version bump. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant MainActivity
participant SystemSettings as "Settings.System"
participant PowerManager
participant KeyguardManager
participant WindowManager
User->>MainActivity: trigger dim (screenDim = true)
MainActivity->>WindowManager: clear FLAG_KEEP_SCREEN_ON
MainActivity->>SystemSettings: read SCREEN_OFF_TIMEOUT
SystemSettings-->>MainActivity: original timeout
MainActivity->>SystemSettings: set SCREEN_OFF_TIMEOUT = 3000
MainActivity->>WindowManager: show dim overlay / blank WebView
MainActivity->>MainActivity: schedule snooze/redim timer
User->>MainActivity: trigger undim (screenDim = false)
MainActivity->>SystemSettings: restore original SCREEN_OFF_TIMEOUT
MainActivity->>PowerManager: acquire short WakeLock
PowerManager-->>MainActivity: device wakes
MainActivity->>WindowManager: add FLAG_KEEP_SCREEN_ON, hide dim overlay
alt API >= O_MR1
MainActivity->>MainActivity: setShowWhenLocked / setTurnScreenOn
MainActivity->>KeyguardManager: requestDismissKeyguard()
else
MainActivity->>MainActivity: set window flags to turn screen on / show when locked
MainActivity->>KeyguardManager: requestDismissKeyguard()
end
KeyguardManager-->>MainActivity: keyguard dismissed / screen visible
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/AndroidManifest.xml`:
- Around line 9-10: Remove the unused permission declaration for
REQUEST_IGNORE_BATTERY_OPTIMIZATIONS from the Android manifest: delete the
<uses-permission
android:name="android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS"/> entry
in AndroidManifest.xml and ensure no code references that permission string
elsewhere (search for "REQUEST_IGNORE_BATTERY_OPTIMIZATIONS") before committing
the change.
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt`:
- Line 82: The current in-memory backup originalScreenTimeout is overwritten on
repeated screenDim(true) calls and lost on process death; make the backup
lifecycle-safe and idempotent by: only assign originalScreenTimeout when it is
the unset sentinel (e.g., -1) inside the method handling screenDim(true) so
repeated calls do not overwrite the real user value, persist the backup to
SharedPreferences when first saving it, and on restore (screenDim(false)) read
and apply the saved value only if a valid backup exists then clear both the
in-memory sentinel and the SharedPreferences key; also load any persisted backup
into originalScreenTimeout during startup (e.g., in onCreate) so crashes/process
death can still restore the user’s timeout.
- Around line 852-875: You set wake/unlock flags (setShowWhenLocked,
setTurnScreenOn, requestDismissKeyguard and window flags FLAG_TURN_SCREEN_ON /
FLAG_SHOW_WHEN_LOCKED / FLAG_DISMISS_KEYGUARD) but never clear them; update
MainActivity to explicitly reset these after the wake sequence completes (or in
onDestroy/onStop) by calling setShowWhenLocked(false) and setTurnScreenOn(false)
for API>=O_MR1, and clearing the window flags (window.clearFlags(...)) for the
older branches, and ensure any requestDismissKeyguard cleanup is handled; add
the cleanup logic to the same class (MainActivity) in a lifecycle method
(onDestroy or onStop) so the flags do not persist beyond the intended wake
period.
- Around line 779-788: The setScreenTimeout function calls
Settings.System.putInt without checking Settings.System.canWrite; before
attempting the write, check Settings.System.canWrite(contentResolver) and if it
returns false, avoid calling putInt and instead trigger the WRITE_SETTINGS flow
(e.g., launch an Intent with ACTION_MANAGE_WRITE_SETTINGS for the app) or
surface a user-facing prompt to request the permission; keep the existing
try/catch for putInt but only execute it when canWrite is true to prevent silent
failures and log or handle the denied-permission path accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/immichframe/immichframe/MainActivity.ktsettings.gradle.kts
| private var previousImage: Helpers.ImageResponse? = null | ||
| private var currentImage: Helpers.ImageResponse? = null | ||
| private var portraitCache: Helpers.ImageResponse? = null | ||
| private var originalScreenTimeout: Int = -1 |
There was a problem hiding this comment.
Make timeout backup lifecycle-safe and idempotent.
Line 82 keeps backup only in memory, and Lines 795-797 can overwrite it on repeated screenDim(true) calls (e.g., duplicate RPCs). That risks restoring 3000 instead of the user’s real value.
Proposed fix (idempotent backup/restore)
private fun screenDim(dim: Boolean) {
+ val isCurrentlyDimmed = dimOverlay.isVisible
+ if (dim == isCurrentlyDimmed) return
+
if (dim) {
// save user's screen timeout and set to 3s to show "going to sleep" message
- originalScreenTimeout = Settings.System.getInt(
- contentResolver, Settings.System.SCREEN_OFF_TIMEOUT, 30000
- )
+ if (originalScreenTimeout == -1) {
+ originalScreenTimeout = Settings.System.getInt(
+ contentResolver, Settings.System.SCREEN_OFF_TIMEOUT, 30000
+ )
+ }
setScreenTimeout(3000)
@@
if (originalScreenTimeout != -1) {
setScreenTimeout(originalScreenTimeout)
+ originalScreenTimeout = -1
}Also persist the backup (e.g., SharedPreferences) so it can be restored after process death/crash.
Also applies to: 795-799, 829-831
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` at line 82,
The current in-memory backup originalScreenTimeout is overwritten on repeated
screenDim(true) calls and lost on process death; make the backup lifecycle-safe
and idempotent by: only assign originalScreenTimeout when it is the unset
sentinel (e.g., -1) inside the method handling screenDim(true) so repeated calls
do not overwrite the real user value, persist the backup to SharedPreferences
when first saving it, and on restore (screenDim(false)) read and apply the saved
value only if a valid backup exists then clear both the in-memory sentinel and
the SharedPreferences key; also load any persisted backup into
originalScreenTimeout during startup (e.g., in onCreate) so crashes/process
death can still restore the user’s timeout.
| private fun setScreenTimeout(timeout: Int) { | ||
| try { | ||
| Settings.System.putInt( | ||
| contentResolver, | ||
| Settings.System.SCREEN_OFF_TIMEOUT, | ||
| timeout | ||
| ) | ||
| } catch (e: Exception) { | ||
| Log.e("Settings", "Could not set screen timeout: ${e.message}") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify timeout writes and permission patterns
echo "=== Checking SCREEN_OFF_TIMEOUT and Settings API usage ==="
rg -n -C3 'SCREEN_OFF_TIMEOUT|Settings\.System\.putInt|Settings\.System\.canWrite|ACTION_MANAGE_WRITE_SETTINGS' app/src/main/java/com/immichframe/immichframe/MainActivity.kt
echo ""
echo "=== Checking AndroidManifest.xml for WRITE_SETTINGS permission ==="
rg -n 'WRITE_SETTINGS' app/src/main/AndroidManifest.xmlRepository: immichFrame/ImmichFrame_Android
Length of output: 953
🌐 Web query:
Android Settings.System.putInt WRITE_SETTINGS permission canWrite requirement documentation
💡 Result:
Settings.System.putInt() modifies values in the system settings table, so on Android 6.0 (API 23) and higher you must have the special WRITE_SETTINGS authorization (not a normal runtime permission prompt).
What the official docs say:
- Declare the permission in the manifest:
android.permission.WRITE_SETTINGS. If your app targets API 23+, the user must explicitly grant this via a management screen (Settings.ACTION_MANAGE_WRITE_SETTINGS). You can verify access withSettings.System.canWrite(context). [1] Settings.System.canWrite(Context)“checks if the specified app can modify system settings” and reiterates the requirement: manifest declaration plus user grant viaACTION_MANAGE_WRITE_SETTINGS. [2]- This is considered a special permission that’s granted from Settings > Special app access, not via
requestPermissions(). [3]
Minimal reference flow (API 23+):
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && !Settings.System.canWrite(context)) {
Intent i = new Intent(Settings.ACTION_MANAGE_WRITE_SETTINGS);
i.setData(Uri.parse("package:" + context.getPackageName()));
context.startActivity(i);
} else {
Settings.System.putInt(context.getContentResolver(), Settings.System.SCREEN_BRIGHTNESS, 100);
}Sources:
[1] Android Manifest.permission.WRITE_SETTINGS reference (note about API 23+, manage screen, and canWrite()). (developer.android.com)
[2] Android Settings.System.canWrite() reference (requirements and ACTION_MANAGE_WRITE_SETTINGS). (developer.android.com)
[3] Android “Request special permissions” guide (special app access workflow). (developer.android.com)
Citations:
- 1: https://developer.android.com/reference/android/Manifest.permission?utm_source=openai
- 2: https://developer.android.com/reference/android/provider/Settings.System?utm_source=openai
- 3: https://developer.android.com/training/permissions/requesting-special?utm_source=openai
Guard SCREEN_OFF_TIMEOUT writes behind Settings.System.canWrite(...).
The WRITE_SETTINGS permission requires explicit user grant through device Settings on Android 6.0+. Without checking Settings.System.canWrite() before calling putInt(), these writes will fail when the user hasn't granted the permission, breaking the screen timeout feature.
Proposed fix
- private fun setScreenTimeout(timeout: Int) {
+ private fun setScreenTimeout(timeout: Int): Boolean {
+ if (!Settings.System.canWrite(this)) {
+ Log.w("Settings", "WRITE_SETTINGS not granted; cannot set screen timeout")
+ return false
+ }
try {
- Settings.System.putInt(
+ return Settings.System.putInt(
contentResolver,
Settings.System.SCREEN_OFF_TIMEOUT,
timeout
)
} catch (e: Exception) {
Log.e("Settings", "Could not set screen timeout: ${e.message}")
+ return false
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines
779 - 788, The setScreenTimeout function calls Settings.System.putInt without
checking Settings.System.canWrite; before attempting the write, check
Settings.System.canWrite(contentResolver) and if it returns false, avoid calling
putInt and instead trigger the WRITE_SETTINGS flow (e.g., launch an Intent with
ACTION_MANAGE_WRITE_SETTINGS for the app) or surface a user-facing prompt to
request the permission; keep the existing try/catch for putInt but only execute
it when canWrite is true to prevent silent failures and log or handle the
denied-permission path accordingly.
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | ||
| setShowWhenLocked(false) | ||
| } | ||
| } | ||
| .start() | ||
| } | ||
|
|
||
| // turn screen back on, dismissing keyguard lockscreen | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | ||
| setShowWhenLocked(true) | ||
| setTurnScreenOn(true) | ||
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | ||
| keyguardManager.requestDismissKeyguard(this, null) | ||
| } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | ||
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | ||
| keyguardManager.requestDismissKeyguard(this, null) | ||
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | ||
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED) | ||
|
|
||
| } else { | ||
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | ||
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or | ||
| WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'setTurnScreenOn\(|setShowWhenLocked\(|FLAG_TURN_SCREEN_ON|FLAG_SHOW_WHEN_LOCKED|FLAG_DISMISS_KEYGUARD|addFlags|clearFlags' app/src/main/java/com/immichframe/immichframe/MainActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 3420
🏁 Script executed:
rg -n 'onPause|onDestroy|onStop' app/src/main/java/com/immichframe/immichframe/MainActivity.kt | head -20Repository: immichFrame/ImmichFrame_Android
Length of output: 209
🏁 Script executed:
sed -n '924,940p' app/src/main/java/com/immichframe/immichframe/MainActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 516
Clear wake/unlock flags after wake completes.
You set multiple wake/unlock flags, but only partially reset them. In the O_MR1 branch, setShowWhenLocked(false) is called but setTurnScreenOn(true) persists. In the Android 8.0 and earlier branches, the window flags are never cleared. These flags remain active throughout the activity lifecycle with no cleanup in onDestroy(), potentially causing unintended screen wake behavior later.
Proposed fix
.withEndAction {
dimOverlay.visibility = View.GONE
loadSettings()
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) {
setShowWhenLocked(false)
+ setTurnScreenOn(false)
+ } else {
+ window.clearFlags(
+ WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or
+ WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or
+ WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD
+ )
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | |
| setShowWhenLocked(false) | |
| } | |
| } | |
| .start() | |
| } | |
| // turn screen back on, dismissing keyguard lockscreen | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | |
| setShowWhenLocked(true) | |
| setTurnScreenOn(true) | |
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | |
| keyguardManager.requestDismissKeyguard(this, null) | |
| } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | |
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | |
| keyguardManager.requestDismissKeyguard(this, null) | |
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | |
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED) | |
| } else { | |
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | |
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or | |
| WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD) | |
| } | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | |
| setShowWhenLocked(false) | |
| setTurnScreenOn(false) | |
| } else { | |
| window.clearFlags( | |
| WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | |
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or | |
| WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD | |
| ) | |
| } | |
| } | |
| .start() | |
| } | |
| // turn screen back on, dismissing keyguard lockscreen | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { | |
| setShowWhenLocked(true) | |
| setTurnScreenOn(true) | |
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | |
| keyguardManager.requestDismissKeyguard(this, null) | |
| } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | |
| val keyguardManager = getSystemService(Context.KEYGUARD_SERVICE) as KeyguardManager | |
| keyguardManager.requestDismissKeyguard(this, null) | |
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | |
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED) | |
| } else { | |
| window.addFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or | |
| WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or | |
| WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines
852 - 875, You set wake/unlock flags (setShowWhenLocked, setTurnScreenOn,
requestDismissKeyguard and window flags FLAG_TURN_SCREEN_ON /
FLAG_SHOW_WHEN_LOCKED / FLAG_DISMISS_KEYGUARD) but never clear them; update
MainActivity to explicitly reset these after the wake sequence completes (or in
onDestroy/onStop) by calling setShowWhenLocked(false) and setTurnScreenOn(false)
for API>=O_MR1, and clearing the window flags (window.clearFlags(...)) for the
older branches, and ensure any requestDismissKeyguard cleanup is handled; add
the cleanup logic to the same class (MainActivity) in a lifecycle method
(onDestroy or onStop) so the flags do not persist beyond the intended wake
period.
|
Thanks for the PR! While I appreciate the effort, I cannot merge these changes because this app goes through the Google Play Store. Modifying system-level settings and using privileged APIs introduces several risks that would likely trigger Google's Automated Scanners and lead to a Play Store rejection:
Once it is flagged the whole app must be approved again which is a nightmare. If you wanted to maintain this branch (and fix the system integrity issue above), we could put a note in our docs that for full screen off support see |
|
Thanks for the response! But now I have a bunch of new questions. I'm still working on fixing the issues you brought up, and which the coderabbit checker flagged, but in the meantime, I have no objection with maintaining
a separate fork and just letting people use that if they want this feature, but I'm wondering: who is this app really meant for?
Personally, as detailed in the PR, I'm running this on a Frameo photo frame, which of course is based on Android, but has no access to the Google Play store, so the app must be side-loaded there, and of course Play Store
restrictions don't apply here. I guess I just assumed that most users were doing the same thing, but now it seems that maybe it's really meant for people using generic Android tablets (which have access to the Play
store). So I'm wondering if it would make any sense to simply maintain two forks of this app, one aimed at users who install through the Play Store, and another aimed at users who are side-loading and using dedicated
Android-based photo frames.
A related question: the only reason I added code to modify the timeout to 3 seconds was because the feature exists in the app to allow the timeout in the first place (or turn it off and keep the screen on). But do users
actually use this? And if they do, it seems to me that allowing the screen to timeout after 10 or 30 minutes or whatever is at odds with the "screen dimming" feature: I don't see why a user would use both. If they just want
to press the power button and let it show photos for 10 minutes, and turn itself off, there's no reason for it to dim the screen at night, and vice-versa. Would it make sense to make these two settings mutually
exclusive?
While I have your attention, I was planning to add another feature that really only makes sense for a side-loaded version of the app: self-updating. If you're familiar with "SmartTube", it has this feature, since it's also side-loaded (Google won't accept it since it shows YouTube without the ads). In it, when it detects a new version released on Github, an "Update" menu item appears in the main menu, and the user can see the release notes and press the "update" button to update the app to the newest version. I want to add this feature to this app because my use-case is that I have not only one frame for my own use at home, but two additional frames at my and my wife's parents' homes, in far-away locations where I can't easily update them myself, so making it reasonably easy to update the app from within the app would be a really useful feature here. But here too, it seems this feature would only be useful on the fork that's aimed at side-loaders.
Anyway, thanks again for the response and sincere consideration. I'll work on resolving the coding issues, and again I have no problem with anyone pointing users to my branch.
Dan
…On Mon, Mar 2, 2026 at 10:33 PM Rob Rogers ***@***.***> wrote:
*3rob3* left a comment (immichFrame/ImmichFrame_Android#46)
<#46 (comment)>
Thanks for the PR! While I appreciate the effort, I cannot merge these
changes because this app goes through the Google Play Store. Modifying
system-level settings and using privileged APIs introduces several risks
that would likely trigger Google's Automated Scanners and lead to a Play
Store rejection:
- *Privileged Access Abuse:* Requesting WRITE_SETTINGS to modify
SCREEN_OFF_TIMEOUT is considered "Special App Access." Google's
automated scanners flag this unless it is the core functionality of the app
(like a system utility).
- *System Integrity Risk:* This code modifies a global system value.
If the app crashes after setting the timeout to 3 seconds but before
restoring it, the user's device becomes effectively unusable until they
manually find and fix the setting.
- *Special Permissions*
- *Policy Non-Compliance:* Using deprecated WakeLock flags and
programmatically dismissing the keyguard is a common reason for "Malicious
Behavior" flags during the automated review process (I have had this happen
already).
Once it is flagged the whole app must be approved again which is a
nightmare. If you wanted to maintain this branch (and fix the system
integrity issue above), we could put a note in our docs that for full
screen off support see LINK_TO_YOUR_REPO?
Thanks again for the contribution!
—
Reply to this email directly, view it on GitHub
<#46 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BQHEBS453IW7M32FHS4G6BD4OWESZAVCNFSM6AAAAACWCTNIQCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTSOBUGQZDMNJWHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
This app was made long before we started using Frameo devices, then I adapted it to support them. Believe it or not it is most popular (by far) on Android TV devices. This is likely due to the screensaver functionality. I have been considering splitting them (would make maintaining the Play store version much simpler pulling out all the non webview code), but then I would have to maintain two branches, and I just don't have the time for that. I don't understand your question/comment about a 3 second timeout. Are you talking about the Keep Screen On toggle? If so no that is not at odds with the screen dim. Keep screen on is so the device doesn't auto turn off, thereby locking the device. |
|
I have been considering splitting them (would make maintaining the Play
store version much simpler pulling out all the non webview code), but then
I would have to maintain two branches, and I just don't have the time for
that.
I'm going to take a crack at making my fork just for the Frameo devices
(and perhaps other similar devices). We'll see how it goes (this is still
early), but if it goes well, you're welcome to just point to mine for this
use-case.
I don't understand your question/comment about a 3 second timeout. Are you
talking about the Keep Screen On toggle? If so no that is not at odds with
the screen dim. Keep screen on is so the device doesn't auto turn off,
thereby locking the device.
I'm sorry, I didn't explain adequately. There's two configuration options:
1) the Keep Screen On toggle, and 2) the Screen Dimming function. With
Keep Screen On set to "on", then the Screen Dimming function is very useful
(IMO), because it lets us have a black screen at night when people are
asleep, and also saves some energy. But if the Keep Screen On toggle is
set to "off", I don't see how the Screen Dimming function is useful any
more: the screen is going to turn off and lock on its own after a
relatively short time anyway, unless I suppose if you have the timeout set
to something really long and the screen dimming time is coming up quickly.
It seems to me that users would either use Screen Dimming with KSO set to
"on", or would leave KSO "off" and not use Screen Dimming.
…On Thu, Mar 5, 2026 at 10:16 PM Rob Rogers ***@***.***> wrote:
*3rob3* left a comment (immichFrame/ImmichFrame_Android#46)
<#46 (comment)>
This app was made long before we started using Frameo devices, then I
adapted it to support them. Believe it or not it is most popular (by far)
on Android TV devices. This is likely due to the screensaver functionality.
I have been considering splitting them (would make maintaining the Play
store version much simpler pulling out all the non webview code), but then
I would have to maintain two branches, and I just don't have the time for
that.
I don't understand your question/comment about a 3 second timeout. Are you
talking about the Keep Screen On toggle? If so no that is not at odds with
the screen dim. Keep screen on is so the device doesn't auto turn off,
thereby locking the device.
—
Reply to this email directly, view it on GitHub
<#46 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BQHEBS5S33HTCWW4VX57BFL4PF437AVCNFSM6AAAAACWCTNIQCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMBUHE4TGMRWGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
KeepScreenOn can probably be removed (this is the default behavior in all other clients). You are right, without that set screen dimming is kinda useless. |
…ection to keep screen on is set, and if it is not set, added selection to set timeout value in any number of minutes rather than requiring user to set it in Android settings. Fix so user timeout value not lost
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (3)
780-790:⚠️ Potential issue | 🟠 MajorCheck
Settings.System.canWrite(...)before every timeout write.Declaring
WRITE_SETTINGSis not enough on Android 6.0+. Without the special-access grant theseputInt()calls fail, so the timeout feature silently stops working unless you surfaceACTION_MANAGE_WRITE_SETTINGSfirst.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines 780 - 790, In setScreenTimeout(timeout: Int) ensure you call Settings.System.canWrite(context) before Settings.System.putInt — if it returns false, launch the ACTION_MANAGE_WRITE_SETTINGS intent to prompt the user for the special WRITE_SETTINGS grant (or otherwise surface an error/notification) and skip/abort the putInt call; update the catch to handle/re-log refusal. Target the setScreenTimeout function and the surrounding code that invokes Settings.System.putInt to implement this permission check and user flow.
878-900:⚠️ Potential issue | 🟠 MajorClear the temporary wake and unlock flags after wake completes.
The O_MR1+ path only calls
setShowWhenLocked(false), sosetTurnScreenOn(true)stays latched. The pre-O window flags are never cleared at all. Later resumes can then wake or unlock the device unexpectedly.Suggested fix
.withEndAction { dimOverlay.visibility = View.GONE loadSettings() if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) { setShowWhenLocked(false) + setTurnScreenOn(false) + } else { + window.clearFlags( + WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or + WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or + WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD + ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines 878 - 900, The wake/unlock flags are never cleared which leaves setTurnScreenOn(true) and the window flags latched; update MainActivity so that after the wake/unlock sequence completes (e.g., after the start()/animation or once keyguard dismissal finishes) you clear the flags: for API >= O_MR1 call setTurnScreenOn(false) and setShowWhenLocked(false); for API >= O and older call window.clearFlags(...) to remove WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON, FLAG_SHOW_WHEN_LOCKED and FLAG_DISMISS_KEYGUARD as appropriate (while keeping the keyguardManager.requestDismissKeyguard(...) call). Ensure these cleanup calls run reliably after the wake completes.
82-82:⚠️ Potential issue | 🟠 MajorPersist and eagerly restore the original timeout.
originalScreenTimeoutstill lives only in memory, andapplyScreenTimeout()never restores it whenkeepScreenOnflips back totrue. A crash or process kill aftersetScreenTimeout(...), or simply re-enablingkeepScreenOn, can leave the device stuck on the app-specific global timeout. Persist the backup, load it on startup, and funnel every restore path through one helper.Also applies to: 797-808, 818-856, 951-954
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` at line 82, originalScreenTimeout is only in-memory and applyScreenTimeout()/setScreenTimeout(...) doesn't reliably restore it when keepScreenOn toggles or after process restart; persist the original timeout (e.g., SharedPreferences) when you first back it up, load it during startup (in MainActivity.onCreate) to repopulate originalScreenTimeout, and centralize all restore logic into a single helper method (e.g., restoreOriginalScreenTimeout()) that both applyScreenTimeout() and the keepScreenOn toggle call; ensure every code path that wants to restore uses that helper and that the helper clears the persisted backup after successful restore.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/AndroidManifest.xml`:
- Around line 67-70: WidgetConfigActivity currently proceeds to read/write
widget preferences without validating the incoming EXTRA_APPWIDGET_ID; add an
early guard in WidgetConfigActivity (after extracting appWidgetId from the
intent) that checks if appWidgetId == AppWidgetManager.INVALID_APPWIDGET_ID and
if so calls setResult(RESULT_CANCELED), finish(), and returns to prevent further
preference access and accidental RESULT_OK for invalid requests.
- Line 9: Remove the unused WRITE_SETTINGS permission declaration from
AndroidManifest.xml: delete the line declaring android.permission.WRITE_SETTINGS
since the app uses WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON for screen
timeout and does not interact with system Settings; if you actually need to
modify system settings instead, implement that logic where appropriate and add
the corresponding rationale and documentation, otherwise simply remove the
<uses-permission android:name="android.permission.WRITE_SETTINGS" /> entry.
In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt`:
- Around line 62-67: When chkKeepScreenOn preference is turned off, you must
also clear the persisted screen dimming flag and ensure
MainActivity.loadSettings() ignores/stops any schedule when keepScreenOn is
false: in the chkKeepScreenOn.setOnPreferenceChangeListener block (symbols:
chkKeepScreenOn, screenDimmingCategory, txtScreenTimeout) write the preference
change to remove or set the stored "screenDim" flag to false via the app
SharedPreferences (e.g., sharedPreferences.edit().remove("screenDim") or
.putBoolean("screenDim", false).apply()), and in MainActivity.loadSettings()
gate reading/activation of the screen dimming schedule by first checking the
current keepScreenOn value (do not rely on a stale screenDim value when
keepScreenOn is false) so the schedule is not resumed if keepScreenOn is
disabled.
- Around line 69-78: The preference listener in SettingsFragment.kt accepts any
positive integer which can overflow when MainActivity.applyScreenTimeout()
multiplies minutes by 60*1000; clamp the parsed value to a safe maximum before
accepting it: compute a maxMinutes based on the same timeout unit used in
MainActivity (e.g., derive maxMinutes = SCREEN_OFF_TIMEOUT / (60*1000) or use a
named constant) and validate newValue.toString().toIntOrNull() to be between 1
and maxMinutes; if out of range, show the existing Toast and return false,
otherwise return true. Reference txtScreenTimeout preference listener in
SettingsFragment and MainActivity.applyScreenTimeout / SCREEN_OFF_TIMEOUT to
keep limits consistent.
---
Duplicate comments:
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt`:
- Around line 780-790: In setScreenTimeout(timeout: Int) ensure you call
Settings.System.canWrite(context) before Settings.System.putInt — if it returns
false, launch the ACTION_MANAGE_WRITE_SETTINGS intent to prompt the user for the
special WRITE_SETTINGS grant (or otherwise surface an error/notification) and
skip/abort the putInt call; update the catch to handle/re-log refusal. Target
the setScreenTimeout function and the surrounding code that invokes
Settings.System.putInt to implement this permission check and user flow.
- Around line 878-900: The wake/unlock flags are never cleared which leaves
setTurnScreenOn(true) and the window flags latched; update MainActivity so that
after the wake/unlock sequence completes (e.g., after the start()/animation or
once keyguard dismissal finishes) you clear the flags: for API >= O_MR1 call
setTurnScreenOn(false) and setShowWhenLocked(false); for API >= O and older call
window.clearFlags(...) to remove WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON,
FLAG_SHOW_WHEN_LOCKED and FLAG_DISMISS_KEYGUARD as appropriate (while keeping
the keyguardManager.requestDismissKeyguard(...) call). Ensure these cleanup
calls run reliably after the wake completes.
- Line 82: originalScreenTimeout is only in-memory and
applyScreenTimeout()/setScreenTimeout(...) doesn't reliably restore it when
keepScreenOn toggles or after process restart; persist the original timeout
(e.g., SharedPreferences) when you first back it up, load it during startup (in
MainActivity.onCreate) to repopulate originalScreenTimeout, and centralize all
restore logic into a single helper method (e.g., restoreOriginalScreenTimeout())
that both applyScreenTimeout() and the keepScreenOn toggle call; ensure every
code path that wants to restore uses that helper and that the helper clears the
persisted backup after successful restore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea726cff-9067-4560-be55-c9d2e9b07fa8
📒 Files selected for processing (7)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.ktapp/src/main/res/xml/device_admin.xmlapp/src/main/res/xml/settings_view.xmlgradlew
✅ Files skipped from review due to trivial changes (2)
- app/src/main/res/xml/device_admin.xml
- app/build.gradle.kts
| <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" /> | ||
| <uses-permission android:name="android.permission.INTERNET" /> | ||
| <uses-permission android:name="android.permission.WAKE_LOCK" /> | ||
| <uses-permission android:name="android.permission.WRITE_SETTINGS" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(build\.gradle|AndroidManifest|flavor)" | head -20Repository: immichFrame/ImmichFrame_Android
Length of output: 145
🏁 Script executed:
fd -t f "AndroidManifest" -o "build.gradle"Repository: immichFrame/ImmichFrame_Android
Length of output: 218
🏁 Script executed:
cd app && find . -name "AndroidManifest.xml" -o -name "build.gradle*" | head -20Repository: immichFrame/ImmichFrame_Android
Length of output: 124
🏁 Script executed:
cat -n app/build.gradle.kts | head -100Repository: immichFrame/ImmichFrame_Android
Length of output: 3658
🏁 Script executed:
cat -n app/src/main/AndroidManifest.xmlRepository: immichFrame/ImmichFrame_Android
Length of output: 4149
🏁 Script executed:
rg "WRITE_SETTINGS" -B 3 -A 3Repository: immichFrame/ImmichFrame_Android
Length of output: 663
🏁 Script executed:
rg "WRITE_SETTINGS\|Settings\.ACTION_MANAGE_WRITE_SETTINGS" app/src -B 2 -A 2Repository: immichFrame/ImmichFrame_Android
Length of output: 57
🏁 Script executed:
grep -r "flavors\|productFlavors\|flavorDimensions" app/build.gradle.ktsRepository: immichFrame/ImmichFrame_Android
Length of output: 57
🏁 Script executed:
find app/src -name "*.kt" -o -name "*.java" | xargs grep -l "WRITE_SETTINGS\|Settings\|ACTION_MANAGE_WRITE_SETTINGS" 2>/dev/nullRepository: immichFrame/ImmichFrame_Android
Length of output: 456
🏁 Script executed:
cat app/src/main/java/com/immichframe/immichframe/SettingsActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 494
🏁 Script executed:
cat app/src/main/java/com/immichframe/immichframe/SettingsFragment.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 7814
🏁 Script executed:
rg "ACTION_MANAGE_WRITE_SETTINGS\|canWrite\|WRITE_SETTINGS" app/src/main/java --type kt -B 3 -A 3Repository: immichFrame/ImmichFrame_Android
Length of output: 101
🏁 Script executed:
rg "ACTION_MANAGE_WRITE_SETTINGS|canWrite|WRITE_SETTINGS" app/src/main/javaRepository: immichFrame/ImmichFrame_Android
Length of output: 57
🏁 Script executed:
rg "keepScreenOn|screenTimeout|dim" app/src/main/java --type-add "kt:*.kt" -B 1 -A 1Repository: immichFrame/ImmichFrame_Android
Length of output: 11002
🏁 Script executed:
cat app/src/main/java/com/immichframe/immichframe/ScreenSaverService.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 23994
Remove unused WRITE_SETTINGS permission.
WRITE_SETTINGS is declared in the manifest but not used anywhere in the codebase. Screen timeout management is handled via app-level WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON, not system Settings. This unused permission declaration will likely trigger Play Store policy review or rejection. Either remove it or implement and document its actual usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/AndroidManifest.xml` at line 9, Remove the unused WRITE_SETTINGS
permission declaration from AndroidManifest.xml: delete the line declaring
android.permission.WRITE_SETTINGS since the app uses
WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON for screen timeout and does not
interact with system Settings; if you actually need to modify system settings
instead, implement that logic where appropriate and add the corresponding
rationale and documentation, otherwise simply remove the <uses-permission
android:name="android.permission.WRITE_SETTINGS" /> entry.
| <activity android:name=".WidgetConfigActivity" | ||
| android:label="Widget Configuration" | ||
| android:theme="@style/Theme.WidgetConfigDialog" /> | ||
| android:theme="@style/Theme.WidgetConfigDialog" | ||
| android:exported="true" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'EXTRA_APPWIDGET_ID|INVALID_APPWIDGET_ID|RESULT_CANCELED|RESULT_OK|widgetBackground' app/src/main/java/com/immichframe/immichframe/WidgetConfigActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 1048
🏁 Script executed:
cat app/src/main/java/com/immichframe/immichframe/WidgetConfigActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 2214
Reject invalid appWidgetId before accessing preferences.
WidgetConfigActivity reads and writes widget preferences without validating EXTRA_APPWIDGET_ID. If called by an external app without providing a valid widget ID, the activity defaults to INVALID_APPWIDGET_ID and proceeds to access shared preferences anyway, returning RESULT_OK regardless. Add an early check to return RESULT_CANCELED and finish if the widget ID is invalid:
if (appWidgetId == AppWidgetManager.INVALID_APPWIDGET_ID) {
setResult(RESULT_CANCELED)
finish()
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/AndroidManifest.xml` around lines 67 - 70, WidgetConfigActivity
currently proceeds to read/write widget preferences without validating the
incoming EXTRA_APPWIDGET_ID; add an early guard in WidgetConfigActivity (after
extracting appWidgetId from the intent) that checks if appWidgetId ==
AppWidgetManager.INVALID_APPWIDGET_ID and if so calls
setResult(RESULT_CANCELED), finish(), and returns to prevent further preference
access and accidental RESULT_OK for invalid requests.
| chkKeepScreenOn?.setOnPreferenceChangeListener { _, newValue -> | ||
| val value = newValue as Boolean | ||
| screenDimmingCategory?.isVisible = value | ||
| txtScreenTimeout?.isVisible = !value | ||
| true | ||
| } |
There was a problem hiding this comment.
Disable screenDim when keepScreenOn is turned off.
This only hides the category. MainActivity.loadSettings() still honors the persisted screenDim flag, so a previously enabled schedule keeps running while the option is invisible. Clear the flag here, and also gate the read in MainActivity so stale prefs from older installs do not survive.
Suggested fix
chkKeepScreenOn?.setOnPreferenceChangeListener { _, newValue ->
val value = newValue as Boolean
screenDimmingCategory?.isVisible = value
txtScreenTimeout?.isVisible = !value
+ if (!value) {
+ chkScreenDim?.isChecked = false
+ txtDimTime?.isVisible = false
+ }
true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| chkKeepScreenOn?.setOnPreferenceChangeListener { _, newValue -> | |
| val value = newValue as Boolean | |
| screenDimmingCategory?.isVisible = value | |
| txtScreenTimeout?.isVisible = !value | |
| true | |
| } | |
| chkKeepScreenOn?.setOnPreferenceChangeListener { _, newValue -> | |
| val value = newValue as Boolean | |
| screenDimmingCategory?.isVisible = value | |
| txtScreenTimeout?.isVisible = !value | |
| if (!value) { | |
| chkScreenDim?.isChecked = false | |
| txtDimTime?.isVisible = false | |
| } | |
| true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt` around
lines 62 - 67, When chkKeepScreenOn preference is turned off, you must also
clear the persisted screen dimming flag and ensure MainActivity.loadSettings()
ignores/stops any schedule when keepScreenOn is false: in the
chkKeepScreenOn.setOnPreferenceChangeListener block (symbols: chkKeepScreenOn,
screenDimmingCategory, txtScreenTimeout) write the preference change to remove
or set the stored "screenDim" flag to false via the app SharedPreferences (e.g.,
sharedPreferences.edit().remove("screenDim") or .putBoolean("screenDim",
false).apply()), and in MainActivity.loadSettings() gate reading/activation of
the screen dimming schedule by first checking the current keepScreenOn value (do
not rely on a stale screenDim value when keepScreenOn is false) so the schedule
is not resumed if keepScreenOn is disabled.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (1)
523-525:⚠️ Potential issue | 🟠 MajorDeduplicate
dimCheckRunnablescheduling.
loadSettings()runs on startup, after returning from settings, and again after undim. PostingdimCheckRunnableeach time without removing the previous callback creates multiple self-rescheduling loops, so dim checks multiply over time. Remove the callback before everypost(...)or guard scheduling with a dedicated flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines 523 - 525, The dimCheckRunnable is being posted repeatedly (from loadSettings and other flows) causing duplicate self-rescheduling loops; before calling handler.post(dimCheckRunnable) in loadSettings (and any other place that schedules it), call handler.removeCallbacks(dimCheckRunnable) to clear existing callbacks, or alternatively introduce and check a boolean (e.g., isDimRunnableScheduled) that you set true when posting and false when removing to prevent double-scheduling; update the scheduling logic around screenDim, handler.post(dimCheckRunnable), and wherever dimCheckRunnable is removed/rescheduled so only one active loop exists.
♻️ Duplicate comments (2)
app/src/main/java/com/immichframe/immichframe/MainActivity.kt (2)
879-901:⚠️ Potential issue | 🟠 MajorClear all wake/unlock flags after the wake completes.
This still leaves
setTurnScreenOn(true)latched on API 27+, and the pre-27 branches never clearFLAG_TURN_SCREEN_ON,FLAG_SHOW_WHEN_LOCKED, orFLAG_DISMISS_KEYGUARD. Those flags persist past this wake cycle and can cause later resumes to wake/unlock unexpectedly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines 879 - 901, The wake/unlock flags are being set (setTurnScreenOn, setShowWhenLocked and window.addFlags with FLAG_TURN_SCREEN_ON/FLAG_SHOW_WHEN_LOCKED/FLAG_DISMISS_KEYGUARD) but never cleared, leaving the device latched awake; after the wake completes (e.g., at the end of the wake/animation completion handler where you currently call .start() or after requestDismissKeyguard succeeds) explicitly clear them: for API >= O_MR1 call setTurnScreenOn(false) and setShowWhenLocked(false); for older APIs call window.clearFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD) so the flags do not persist across resumes.
82-82:⚠️ Potential issue | 🟠 MajorMake the
SCREEN_OFF_TIMEOUToverride survivable across failed writes and process loss.
originalScreenTimeoutstill only lives in memory,setScreenTimeout()gives callers no success signal, and Lines 855-857 clear the backup as if the restore succeeded. If the system write is rejected or the process dies while the 3s/custom timeout is active, the device-global timeout can be stranded with no value left to restore. Persist the original timeout before the first write, gate writes behindSettings.System.canWrite(...), and only clear/reset the backup after a confirmed restore.Also applies to: 781-790, 803-809, 819-824, 855-857, 953-956
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` at line 82, Persist the originalScreenTimeout to durable storage (e.g. SharedPreferences) before performing the first write to Settings.System.SCREEN_OFF_TIMEOUT, require Settings.System.canWrite(...) to be true before any write, change setScreenTimeout(...) to provide a boolean success/failure result (or throw) so callers can react to failed writes, and only clear the persisted backup after a confirmed successful restore in the restore path (do not clear in the code paths that assume success). Locate and update the fields/methods originalScreenTimeout, setScreenTimeout, and the restore routine that currently clears the backup (the code around the restoreOriginalTimeout/clear backup logic) to read/write the persisted backup and to keep it intact on write/restore failures or process death so the system timeout can always be recovered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt`:
- Line 815: The short-circuit check "if (dim == dimOverlay.isVisible) return"
prevents reapplying the sleep/FLAG_KEEP_SCREEN_ON state while the dim overlay is
visible; change the condition so it only returns when there is truly no change
and the overlay is not active (e.g., only return when dim ==
dimOverlay.isVisible && !dimOverlay.isVisible), so onResume() (and the other
affected block around the applyScreenTimeout() calls) will still reassert the
configured short timeout during an active dim session; update both the check at
onResume() (where applyScreenTimeout() is called) and the similar check in the
block around lines 940-944 to use this refined condition.
- Around line 817-824: The current dim flow uses Handler.postDelayed in
checkDimTime()/dim loop and reduces the timeout via setScreenTimeout(3000), but
the Handler stops when the device sleeps so screenDim(false) may never run;
replace the periodic Handler scheduling with an AlarmManager
RTC_WAKEUP/ELAPSED_REALTIME_WAKEUP scheduled for the exact end of the dim window
so the system will wake and run your undim logic. Specifically: when entering
dim mode (where originalScreenTimeout is captured and setScreenTimeout(3000) is
called), schedule an AlarmManager alarm for the dim-end timestamp; cancel that
alarm when leaving dim early; in the alarm receiver or PendingIntent invoke the
same code path currently used for undimming (call screenDim(false) or the
existing wake-and-undim logic) and ensure any WakeLock acquisition remains in
that undim path; remove or stop relying on Handler.postDelayed/checkDimTime for
the final wake-up.
---
Outside diff comments:
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt`:
- Around line 523-525: The dimCheckRunnable is being posted repeatedly (from
loadSettings and other flows) causing duplicate self-rescheduling loops; before
calling handler.post(dimCheckRunnable) in loadSettings (and any other place that
schedules it), call handler.removeCallbacks(dimCheckRunnable) to clear existing
callbacks, or alternatively introduce and check a boolean (e.g.,
isDimRunnableScheduled) that you set true when posting and false when removing
to prevent double-scheduling; update the scheduling logic around screenDim,
handler.post(dimCheckRunnable), and wherever dimCheckRunnable is
removed/rescheduled so only one active loop exists.
---
Duplicate comments:
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt`:
- Around line 879-901: The wake/unlock flags are being set (setTurnScreenOn,
setShowWhenLocked and window.addFlags with
FLAG_TURN_SCREEN_ON/FLAG_SHOW_WHEN_LOCKED/FLAG_DISMISS_KEYGUARD) but never
cleared, leaving the device latched awake; after the wake completes (e.g., at
the end of the wake/animation completion handler where you currently call
.start() or after requestDismissKeyguard succeeds) explicitly clear them: for
API >= O_MR1 call setTurnScreenOn(false) and setShowWhenLocked(false); for older
APIs call window.clearFlags(WindowManager.LayoutParams.FLAG_TURN_SCREEN_ON or
WindowManager.LayoutParams.FLAG_SHOW_WHEN_LOCKED or
WindowManager.LayoutParams.FLAG_DISMISS_KEYGUARD) so the flags do not persist
across resumes.
- Line 82: Persist the originalScreenTimeout to durable storage (e.g.
SharedPreferences) before performing the first write to
Settings.System.SCREEN_OFF_TIMEOUT, require Settings.System.canWrite(...) to be
true before any write, change setScreenTimeout(...) to provide a boolean
success/failure result (or throw) so callers can react to failed writes, and
only clear the persisted backup after a confirmed successful restore in the
restore path (do not clear in the code paths that assume success). Locate and
update the fields/methods originalScreenTimeout, setScreenTimeout, and the
restore routine that currently clears the backup (the code around the
restoreOriginalTimeout/clear backup logic) to read/write the persisted backup
and to keep it intact on write/restore failures or process death so the system
timeout can always be recovered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec2ccae8-b297-4808-897f-2d5ebd868c31
📒 Files selected for processing (2)
app/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt
| if (dim) { | ||
| lp.screenBrightness = 0.01f | ||
| window.attributes = lp | ||
| // save user's screen timeout and set to 3s to show "going to sleep" message | ||
| if (originalScreenTimeout == -1) { | ||
| originalScreenTimeout = Settings.System.getInt( | ||
| contentResolver, Settings.System.SCREEN_OFF_TIMEOUT, 30000 | ||
| ) | ||
| } | ||
| setScreenTimeout(3000) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the MainActivity.kt file
find . -name "MainActivity.kt" -type fRepository: immichFrame/ImmichFrame_Android
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Get file size first
wc -l ./app/src/main/java/com/immichframe/immichframe/MainActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Read the context around lines 817-824
sed -n '800,840p' ./app/src/main/java/com/immichframe/immichframe/MainActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 1786
🏁 Script executed:
#!/bin/bash
# Read the context around lines 850-910
sed -n '850,910p' ./app/src/main/java/com/immichframe/immichframe/MainActivity.ktRepository: immichFrame/ImmichFrame_Android
Length of output: 2914
🏁 Script executed:
#!/bin/bash
# Search for screenDim calls and how they're scheduled
rg "screenDim" ./app/src/main/java/com/immichframe/immichframe/MainActivity.kt -A 2 -B 2Repository: immichFrame/ImmichFrame_Android
Length of output: 1104
🏁 Script executed:
#!/bin/bash
# Search for dimCheckRunnable definition
rg "dimCheckRunnable" ./app/src/main/java/com/immichframe/immichframe/MainActivity.kt -A 10 -B 2Repository: immichFrame/ImmichFrame_Android
Length of output: 1004
🏁 Script executed:
#!/bin/bash
# Let me verify the complete flow by checking when setScreenTimeout(3000) actually lets device sleep
# and whether there's any other wake-up mechanism
rg "setScreenTimeout|AlarmManager|WakeLock" ./app/src/main/java/com/immichframe/immichframe/MainActivity.kt -B 2 -A 2Repository: immichFrame/ImmichFrame_Android
Length of output: 1369
Replace Handler-based scheduling with AlarmManager for reliable undim triggering.
The dim-to-undim flow relies on Handler.postDelayed() to periodically call checkDimTime() and trigger screenDim(false). However, once the device sleeps after 3 seconds (line 824 sets setScreenTimeout(3000)), the Handler loop stops executing—it is not a wake source. The device will remain asleep until something else wakes it, making the end-of-window auto-undim unreliable.
While the screenDim(false) code path (lines 863-869) correctly uses WakeLock with ACQUIRE_CAUSES_WAKEUP to wake the device, this code never executes if screenDim(false) is never called. To ensure reliable undim at the scheduled end time, use AlarmManager to schedule a wake-up alarm at the end of the dim window instead of relying on the Handler loop.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/immichframe/immichframe/MainActivity.kt` around lines
817 - 824, The current dim flow uses Handler.postDelayed in checkDimTime()/dim
loop and reduces the timeout via setScreenTimeout(3000), but the Handler stops
when the device sleeps so screenDim(false) may never run; replace the periodic
Handler scheduling with an AlarmManager RTC_WAKEUP/ELAPSED_REALTIME_WAKEUP
scheduled for the exact end of the dim window so the system will wake and run
your undim logic. Specifically: when entering dim mode (where
originalScreenTimeout is captured and setScreenTimeout(3000) is called),
schedule an AlarmManager alarm for the dim-end timestamp; cancel that alarm when
leaving dim early; in the alarm receiver or PendingIntent invoke the same code
path currently used for undimming (call screenDim(false) or the existing
wake-and-undim logic) and ensure any WakeLock acquisition remains in that undim
path; remove or stop relying on Handler.postDelayed/checkDimTime for the final
wake-up.
… where the user can press the power button while the screen is turned off in dimming mode, and the screen will come back on for 30s, then turn back off.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt (1)
43-48:⚠️ Potential issue | 🟠 MajorHandle stale
screenDimon initial load whenkeepScreenOnis already false.On Line 43–48, visibility is toggled, but persisted
screenDimis not reset on first render. If older prefs still havescreenDim=true,MainActivitycan still resume dim scheduling while the option is hidden. This is the same concern previously raised and is still partially open in this path.Proposed fix
val keepScreenOn = chkKeepScreenOn?.isChecked ?: false screenDimmingCategory?.isVisible = keepScreenOn txtScreenTimeout?.isVisible = !keepScreenOn +if (!keepScreenOn) { + chkScreenDim?.isChecked = false + txtDimTime?.isVisible = false +} val screenDim = chkScreenDim?.isChecked ?: false txtDimTime?.isVisible = screenDim🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt` around lines 43 - 48, When initializing visibility in SettingsFragment, currently txtDimTime visibility uses the persisted chkScreenDim value even when keepScreenOn is false; update the block that reads chkKeepScreenOn/chkScreenDim (references: chkKeepScreenOn, chkScreenDim, screenDimmingCategory, txtScreenTimeout, txtDimTime) so that when keepScreenOn is true you restore screenDim visibility from prefs, but when keepScreenOn is false you explicitly set screenDim to false, persist that change (update the preference backing chkScreenDim) and notify/call the existing MainActivity dim scheduling cancel method so any scheduled dimming is stopped; ensure UI elements (screenDimmingCategory, txtScreenTimeout, txtDimTime) reflect the cleared state.
🧹 Nitpick comments (1)
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt (1)
155-157: Reuse the existingtxtDimTimereference instead of re-fetching the same preference.This is minor, but it reduces duplication and keeps preference wiring in one place.
Proposed refactor
- val timePref = findPreference<EditTextPreference>("dim_time_range") - timePref?.setOnPreferenceChangeListener { _, newValue -> + txtDimTime?.setOnPreferenceChangeListener { _, newValue -> val timeRange = newValue.toString().trim()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt` around lines 155 - 157, The listener re-fetches the same EditTextPreference instead of reusing the existing txtDimTime reference; update the setOnPreferenceChangeListener block to use the already-declared txtDimTime (instead of calling findPreference<EditTextPreference>("dim_time_range") again) so the code uses the single preference instance (txtDimTime) when trimming/validating newValue and setting values, keeping preference wiring centralized in the txtDimTime variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt`:
- Around line 43-48: When initializing visibility in SettingsFragment, currently
txtDimTime visibility uses the persisted chkScreenDim value even when
keepScreenOn is false; update the block that reads chkKeepScreenOn/chkScreenDim
(references: chkKeepScreenOn, chkScreenDim, screenDimmingCategory,
txtScreenTimeout, txtDimTime) so that when keepScreenOn is true you restore
screenDim visibility from prefs, but when keepScreenOn is false you explicitly
set screenDim to false, persist that change (update the preference backing
chkScreenDim) and notify/call the existing MainActivity dim scheduling cancel
method so any scheduled dimming is stopped; ensure UI elements
(screenDimmingCategory, txtScreenTimeout, txtDimTime) reflect the cleared state.
---
Nitpick comments:
In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt`:
- Around line 155-157: The listener re-fetches the same EditTextPreference
instead of reusing the existing txtDimTime reference; update the
setOnPreferenceChangeListener block to use the already-declared txtDimTime
(instead of calling findPreference<EditTextPreference>("dim_time_range") again)
so the code uses the single preference instance (txtDimTime) when
trimming/validating newValue and setting values, keeping preference wiring
centralized in the txtDimTime variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b899fcc9-3513-4bd6-91b5-c54e18afc85a
📒 Files selected for processing (2)
app/src/main/java/com/immichframe/immichframe/MainActivity.ktapp/src/main/java/com/immichframe/immichframe/SettingsFragment.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/immichframe/immichframe/MainActivity.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt`:
- Around line 158-159: Replace the misspelled reference txtFimeTime with the
correctly declared txtDimTime in the preference change hook so the dimming time
validation runs; update the call to setOnPreferenceChangeListener from
txtFimeTime?.setOnPreferenceChangeListener { ... } to
txtDimTime?.setOnPreferenceChangeListener { _, newValue -> ... } (keeping the
existing listener logic intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: faef687f-df70-4b56-a9b0-f8da5a6b98a5
📒 Files selected for processing (1)
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt
app/src/main/java/com/immichframe/immichframe/SettingsFragment.kt
Outdated
Show resolved
Hide resolved
2 participants
This PR allows the screen to turn off when screen dimming is activated, rather than simply showing a black screen.
Currently, the app has a "Screen Dimming" feature which allows the user to specify a time range in which the screen will be dimmed, for instance at night when users would be asleep. However, this feature only displays a black screen and turns off the WebView, saving a small amount of power. On my 10.5-inch device, it brings power consumption down from 7W to 4W.
This PR changes the logic to turn off the flag keeping the screen on, and sets a very short (3s) time so the screen actually turns off. When the dimming function is activated, it turns off the WebView as before, but also saves the user's screen timeout setting value (in case they actually use this), changes it to 3s, and clears the screen keep-on flag, so a quick "Going to sleep" message is displayed and then the screen turns off. On my device, this brings power consumption down to less than 1W.
When the screen is to wake up again, it uses a wakeLock to turn the screen on, restores the original screen timeout value, sets the screen keep-on flag, and restores the WebView.
There was some help done with Android Studio's Gemini assistant, but all changes were done and reviewed by myself. Unfortunately, this has only been tested on my one device, a SSA ZN-DP1101 which runs Android 13.0. I attempted to ensure the code still runs properly on devices with older Android versions, but I do not have any such devices to test on.
Summary by CodeRabbit
New Features
Bug Fixes
Chores