feat: Add Experimental CPU power control to Quick Menu#1698
Open
joshuatam wants to merge 4 commits into
Open
Conversation
Introduces a new "Power Control" tab in the Quick Menu, allowing users to manage CPU frequency scaling and governor settings. This feature is designed to optimize device performance and power consumption during gameplay, particularly on supported handhelds like AYN and Retroid devices. Key features include: - Display of current CPU governor, minimum, and maximum frequencies. - Predefined power profiles (e.g., Power Save, Balanced, Performance) generated dynamically based on available governors and frequencies. - Dropdown menus to select power profiles or individual CPU governors. - Utilizes the PServer service for root access to modify `/sys/devices/system/cpu` sysfs entries. - Saves original CPU settings upon app launch and restores them when the app environment shuts down, ensuring system integrity. - Provides a clear warning when the PServer service is unavailable, indicating that settings cannot be modified.
Contributor
📝 WalkthroughWalkthroughChangesCPU Power Control
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant QuickMenu
participant PowerControlQuickMenuTab
participant CpuFreqManager
participant PServerExecutor
User->>QuickMenu: Open Power tab
QuickMenu->>PowerControlQuickMenuTab: Render control UI
PowerControlQuickMenuTab->>CpuFreqManager: Read CPU information
PowerControlQuickMenuTab->>CpuFreqManager: Apply selected profile
CpuFreqManager->>PServerExecutor: Execute root sysfs writes
PServerExecutor-->>CpuFreqManager: Return command result
CpuFreqManager-->>PowerControlQuickMenuTab: Return updated CPU information
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt (2)
347-373: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCommented-out dead code references undeclared variables.
The commented "Available Frequencies" section references
selectedMinFreq,selectedMaxFreq, and aFrequencyChipcomposable that don't exist in this file. If uncommented, this would fail to compile. Either remove it or add a TODO with a tracking issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt` around lines 347 - 373, Remove the commented-out “Available Frequencies” block near the power-control menu content, since it references undeclared selectedMinFreq, selectedMaxFreq, and FrequencyChip symbols; do not add replacement UI or implementation.
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer safe scoping over
!!forcpuInfo.The
!!is technically safe due to theelse if (cpuInfo != null)guard, but a safeletscope is more idiomatic and robust against future refactoring:♻️ Proposed refactor
- } else if (cpuInfo != null) { - val info = cpuInfo!! - - if (!hasPServer) { + } else { + cpuInfo?.let { info -> + if (!hasPServer) {This requires adjusting the closing braces for the
letblock.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt` around lines 134 - 135, Update the cpuInfo branch in PowerControlQuickMenuTab to use a safe let-scoped value instead of the forced unwrap in the `else if (cpuInfo != null)` block, and adjust the closing braces accordingly while preserving the existing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/PluviaApp.kt`:
- Around line 241-243: Update the shutdownEnvironment restoration block around
CpuFreqManager.restoreOriginalSettings() to dispatch the synchronous
CPU-frequency restoration work to a background thread instead of running it on
the main thread. Preserve the existing runCatching failure logging within that
background task and keep shutdownEnvironment non-blocking.
In `@app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt`:
- Line 225: Secure writeSysfsFile by validating the value before executing the
root shell command and constructing the echo redirection with safe single-quoted
arguments, preventing shell metacharacters from being interpreted. In
setGovernor, validate the requested governor against getAvailableGovernors() and
only invoke writeSysfsFile for an allowed value.
In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt`:
- Line 333: Update QuickMenu’s isPowerControlAvailable initialization so
CpuFreqManager.isPServerAvailable() is not called inside remember during
composition. Load the flag from a LaunchedEffect running on Dispatchers.IO,
preserving the resulting state for the UI and its existing availability
behavior.
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`:
- Around line 219-227: Update the profile-selection onClick handler in
PowerControlQuickMenuTab so CpuFreqManager.setGovernor, setMinFrequency,
setMaxFrequency, and getCpuInfo run within a coroutine on Dispatchers.IO. Keep
selectedProfileName and isProfileDropdownExpanded state updates on the UI
thread, applying the refreshed cpuInfo through the appropriate UI-safe state
update after the background work completes.
- Around line 54-55: Change isInitialized in the quick-menu state declaration to
use remember rather than rememberSaveable, while leaving selectedProfileName
saveable. Ensure initialization can run again after process recreation so
CpuFreqManager.saveOriginalSettings() captures fresh in-memory CPU state.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`:
- Around line 347-373: Remove the commented-out “Available Frequencies” block
near the power-control menu content, since it references undeclared
selectedMinFreq, selectedMaxFreq, and FrequencyChip symbols; do not add
replacement UI or implementation.
- Around line 134-135: Update the cpuInfo branch in PowerControlQuickMenuTab to
use a safe let-scoped value instead of the forced unwrap in the `else if
(cpuInfo != null)` block, and adjust the closing braces accordingly while
preserving the existing logic.
🪄 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: 60e80b4f-3c29-49e6-a644-a89377cee860
📒 Files selected for processing (22)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/powercontrol/CpuFreqManager.ktapp/src/main/java/app/gamenative/powercontrol/CpuGovernor.ktapp/src/main/java/app/gamenative/powercontrol/PServerExecutor.ktapp/src/main/java/app/gamenative/powercontrol/PowerProfile.ktapp/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
| ) | ||
| } | ||
|
|
||
| val isPowerControlAvailable = remember { CpuFreqManager.isPServerAvailable() } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant symbols and files.
git ls-files | rg 'QuickMenu\.kt|PowerControlQuickMenuTab\.kt|CpuFreqManager'Repository: utkarshdalal/GameNative
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show AST outlines for the likely relevant files, then inspect the exact lines around the call sites.
for f in app/src/main/java/app/gamenative/ui/component/QuickMenu.kt app/src/main/java/app/gamenative/ui/component/PowerControlQuickMenuTab.kt; do
echo "===== $f ====="
wc -l "$f"
ast-grep outline "$f" --view expanded || true
done
echo "===== QuickMenu.kt around line 333 ====="
sed -n '300,360p' app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
echo "===== PowerControlQuickMenuTab.kt around the call site ====="
sed -n '1,220p' app/src/main/java/app/gamenative/ui/component/PowerControlQuickMenuTab.ktRepository: utkarshdalal/GameNative
Length of output: 1709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the implementation of CpuFreqManager.isPServerAvailable() and all call sites.
cpu_file=$(git ls-files | rg 'CpuFreqManager\.(kt|java)$' | head -n 1 || true)
echo "CpuFreqManager file: ${cpu_file:-<not found>}"
if [ -n "${cpu_file:-}" ]; then
wc -l "$cpu_file"
ast-grep outline "$cpu_file" --view expanded || true
echo "===== relevant implementation ====="
rg -n "isPServerAvailable|PServer|binder|IPC|ServiceManager|bind" "$cpu_file"
echo "===== surrounding lines ====="
line=$(rg -n "fun isPServerAvailable|boolean isPServerAvailable" "$cpu_file" | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-40)); [ "$start" -lt 1 ] && start=1
end=$((line+80))
sed -n "${start},${end}p" "$cpu_file"
fi
fi
echo "===== all call sites ====="
rg -n "isPServerAvailable\s*\(" app/src/main/javaRepository: utkarshdalal/GameNative
Length of output: 3895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect PServerExecutor to see whether isPServerAvailable can block.
executor_file=$(git ls-files | rg 'PServerExecutor\.(kt|java)$' | head -n 1 || true)
echo "PServerExecutor file: ${executor_file:-<not found>}"
if [ -n "${executor_file:-}" ]; then
wc -l "$executor_file"
ast-grep outline "$executor_file" --view expanded || true
echo "===== relevant symbols ====="
rg -n "isPServerAvailable|binder|ServiceManager|waitFor|connect|discover|root" "$executor_file"
echo "===== surrounding lines ====="
line=$(rg -n "class PServerExecutor|object PServerExecutor|val isPServerAvailable|fun isPServerAvailable" "$executor_file" | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-120)); [ "$start" -lt 1 ] && start=1
end=$((line+220))
sed -n "${start},${end}p" "$executor_file"
fi
fiRepository: utkarshdalal/GameNative
Length of output: 2932
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== QuickMenu.kt around line 333 ====="
sed -n '320,345p' app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
echo "===== PowerControlQuickMenuTab.kt around line 65 ====="
sed -n '1,110p' app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.ktRepository: utkarshdalal/GameNative
Length of output: 6061
Move the PServer check off the composition thread. CpuFreqManager lazily creates PServerExecutor on first access, and that constructor does the ServiceManager/binder lookup synchronously. Calling it from remember {} can block composition the first time this path runs; load the flag from LaunchedEffect on Dispatchers.IO instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/app/gamenative/ui/component/QuickMenu.kt` at line 333,
Update QuickMenu’s isPowerControlAvailable initialization so
CpuFreqManager.isPServerAvailable() is not called inside remember during
composition. Load the flag from a LaunchedEffect running on Dispatchers.IO,
preserving the resulting state for the UI and its existing availability
behavior.
Comment on lines
+54
to
+55
| var selectedProfileName by rememberSaveable { mutableStateOf(ProfileName.CUSTOM) } | ||
| var isInitialized by rememberSaveable { mutableStateOf(false) } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how saveOriginalSettings/restoreOriginalSettings store state
rg -n 'saveOriginalSettings|restoreOriginalSettings|originalGovernor|originalMin|originalMax|companion object' --type kotlin app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.ktRepository: utkarshdalal/GameNative
Length of output: 1157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CpuFreqManager declaration and state ==\n'
sed -n '1,140p' app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt | cat -n
printf '\n== PowerControlQuickMenuTab relevant section ==\n'
sed -n '1,140p' app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt | cat -nRepository: utkarshdalal/GameNative
Length of output: 12779
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== restoreOriginalSettings call sites ==\n'
rg -n 'restoreOriginalSettings\(' app/src/main/java
printf '\n== clearOriginalSettings call sites ==\n'
rg -n 'clearOriginalSettings\(' app/src/main/java
printf '\n== saveOriginalSettings call sites ==\n'
rg -n 'saveOriginalSettings\(' app/src/main/javaRepository: utkarshdalal/GameNative
Length of output: 892
Keep isInitialized out of saved state
CpuFreqManager.saveOriginalSettings() keeps the original governor/frequency only in in-memory fields, and restoreOriginalSettings() does nothing once those fields are null. If the process is recreated, rememberSaveable can restore isInitialized = true while CpuFreqManager starts fresh, so the original CPU settings never get captured and shutdown restore becomes a no-op. Use remember here, or persist the saved CPU state to disk.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`
around lines 54 - 55, Change isInitialized in the quick-menu state declaration
to use remember rather than rememberSaveable, while leaving selectedProfileName
saveable. Ensure initialization can run again after process recreation so
CpuFreqManager.saveOriginalSettings() captures fresh in-memory CPU state.
Comment on lines
+219
to
+227
| onClick = { | ||
| selectedProfileName = profile.name | ||
| CpuFreqManager.setGovernor(profile.governor.governorName) | ||
| CpuFreqManager.setMinFrequency(profile.minFreq) | ||
| CpuFreqManager.setMaxFrequency(profile.maxFreq) | ||
| // Refresh CPU info after applying profile | ||
| cpuInfo = CpuFreqManager.getCpuInfo() | ||
| isProfileDropdownExpanded = false | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== PowerControlQuickMenuTab outline ==\n'
ast-grep outline app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt --view expanded || true
printf '\n== Relevant lines around 219-227 and 277-282 ==\n'
sed -n '190,310p' app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt | cat -n
printf '\n== Locate CpuFreqManager declarations ==\n'
rg -n "object CpuFreqManager|class CpuFreqManager|fun setGovernor|fun setMinFrequency|fun setMaxFrequency|fun getCpuInfo" app/src/main/java -S
printf '\n== Files containing CpuFreqManager usages ==\n'
rg -n "CpuFreqManager\." app/src/main/java -SRepository: utkarshdalal/GameNative
Length of output: 10612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CpuFreqManager outline ==\n'
ast-grep outline app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt --view expanded || true
printf '\n== CpuFreqManager implementation ==\n'
sed -n '1,260p' app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt | cat -n
printf '\n== Search for coroutines/dispatchers in PowerControlQuickMenuTab.kt ==\n'
rg -n "rememberCoroutineScope|CoroutineScope|Dispatchers|launch\\s*\\{|withContext\\s*\\(" app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt -SRepository: utkarshdalal/GameNative
Length of output: 11333
Move the profile/governor writes off the main thread. Both onClick handlers call CpuFreqManager.setGovernor(), setMinFrequency(), setMaxFrequency(), and getCpuInfo() synchronously; those go through PServerExecutor.executeAsRoot(...) and sysfs I/O, so this can block the UI and trigger ANRs. Run the updates on Dispatchers.IO and keep only the dropdown state changes on the UI thread.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt`
around lines 219 - 227, Update the profile-selection onClick handler in
PowerControlQuickMenuTab so CpuFreqManager.setGovernor, setMinFrequency,
setMaxFrequency, and getCpuInfo run within a coroutine on Dispatchers.IO. Keep
selectedProfileName and isProfileDropdownExpanded state updates on the UI
thread, applying the refreshed cpuInfo through the appropriate UI-safe state
update after the background work completes.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
23 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:49">
P2: When Power is the initial tab, controller focus retries without reaching a control because the supplied `focusRequester` is never attached to a focusable node. Attach it to the first enabled dropdown row.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:55">
P1: `isInitialized` is stored via `rememberSaveable`, which survives process death. However, `CpuFreqManager.saveOriginalSettings()` only saves values in in-memory fields (`originalGovernor`, etc.). If the process is recreated, `isInitialized` restores as `true` while `CpuFreqManager`'s fields reset to `null` — so `saveOriginalSettings()` is never called again and `restoreOriginalSettings()` becomes a no-op, leaving the CPU in a modified state after shutdown. Use `remember` instead of `rememberSaveable` here.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:81">
P3: A device with a matching governor but non-profile frequency bounds is labeled Power Save/Balanced/Performance instead of Custom. Match governor and both current frequency bounds before selecting a named profile.</violation>
<violation number="4" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:88">
P3: CPU read failures and fallback labels bypass Android resources, so localized builds show English text. Move these messages and `N/A` to string resources.
(Based on your team's feedback about hardcoded UI strings.) [FEEDBACK_USED].</violation>
<violation number="5" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:129">
P3: Hardcoded English fallback string "Unknown error" displayed to users. Should use a string resource from strings.xml for localization.</violation>
<violation number="6" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:183">
P2: Devices with no PServer still expose enabled profile and governor controls despite the warning that edits cannot be made. Gate both click handlers with `hasPServer` so unavailable controls cannot be opened or selected.</violation>
<violation number="7" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:220">
P2: The profile label can report a new selection after one or more writes fail, hiding an unchanged or partially changed CPU configuration. Update selection only after all writes succeed and show a failure state otherwise.</violation>
<violation number="8" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:221">
P1: Blocking I/O calls (setGovernor, setMinFrequency, setMaxFrequency, getCpuInfo) inside the profile dropdown's onClick run on the main thread. Each call performs file I/O reading/writing sysfs files and blocking binder transactions via PServer, looping over all CPU cores. This will cause UI jank or a frozen quick menu while the operation completes.</violation>
<violation number="9" location="app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt:222">
P1: Switching from a low-cap profile to a higher-cap profile can leave the old minimum in place because the minimum write violates the current maximum. Choose write order from current/target bounds: raise maximum before minimum when expanding, and lower minimum before maximum when constraining.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt:333">
P2: `CpuFreqManager.isPServerAvailable()` triggers lazy initialization of `PServerExecutor`, whose constructor performs a synchronous `ServiceManager.getService()` binder lookup via reflection. Running this inside `remember {}` blocks the composition thread on first access. Load this flag from a `LaunchedEffect` on `Dispatchers.IO` instead, defaulting to `false` until the check completes.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt:529">
P2: Reopening after selecting Power restores tab 2 (Effects), not Power, because `quickMenuLastTab` still clamps persisted values to `0..2`. Expand the persisted tab range (and preserve this value) so the new tab can be restored correctly.</violation>
</file>
<file name="app/src/main/res/values-ja/strings.xml">
<violation number="1" location="app/src/main/res/values-ja/strings.xml:1591">
P3: 3 new string resources (`power_control_info`, `power_control_driver`, `power_control_frequencies`) are defined in values-ja but never referenced in any Kotlin/Java source code — no `R.string.power_control_info`, `R.string.power_control_driver`, or `R.string.power_control_frequencies` usage exists anywhere in the app. These are dead resources that increase APK size and maintenance burden.</violation>
<violation number="2" location="app/src/main/res/values-ja/strings.xml:1599">
P3: `power_control_available_frequencies` is defined but only referenced in a commented-out code block in `PowerControlQuickMenuTab.kt`. Unless its UI is about to be uncommented in this PR, adding translations for dead code produces unnecessary strings in resource files.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:29">
P2: `saveOriginalSettings()` uses a non-atomic check-then-act pattern on the `originalGovernor == null` guard. Since `CpuFreqManager` is a singleton object whose state is accessed from both the UI thread (first-use save) and the environment shutdown path (restore), two threads passing the null check before either writes would result in the second overwriting the first, or a write from one thread being invisible to the other due to the lack of a `@Volatile` or synchronization. This would cause `restoreOriginalSettings()` to restore a partial or incorrect set of original values.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:48">
P1: Restoring a higher original minimum after a low-power cap can fail because the current maximum is still lower. Apply limits in bound-safe order—raise max before raising min, and lower min before lowering max—and retain/rollback state on failures.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:60">
P1: After the first successful teardown, later sessions keep restoring the stale first-session baseline instead of their own original CPU state. Clear the saved values after a successful restore so the next `saveOriginalSettings()` records a new baseline.</violation>
<violation number="4" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:78">
P1: On multi-cluster devices, profile application and rollback can partially fail or overwrite clusters with policy-0 values. Track governor and limits per cpufreq policy (using each policy's related CPUs) and apply only values supported by that policy.</violation>
<violation number="5" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:80">
P2: Missing or unreadable cpufreq data is reported as a valid zero-valued CPU instead of an unsupported device. Require readable, valid governor/min/max values and return `null` otherwise, preventing controls and rollback from using empty/zero settings.</violation>
<violation number="6" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:113">
P2: Devices without the optional discrete-frequency attribute expose no power profiles even when current min/max limits are available. Fall back to readable scaling min/max limits (or another supported frequency source) before declaring the frequency list empty.</violation>
<violation number="7" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:190">
P3: `getNumCpus()` parses `/sys/devices/system/cpu/present` with a simple `split("-")` that assumes a single contiguous range like `"0-7"`. On devices where this file uses a comma-separated range format (e.g., `"0-2,4-6"`), the split produces incorrect results. This would cause the per-CPU write loops to attempt operations on CPU indices that don't exist, generating spurious failure logs. Consider reading `/sys/devices/system/cpu/online` and counting the online CPUs, or iterating until `File("$CPU_BASE_PATH/cpu$i").exists()` is false.</violation>
<violation number="8" location="app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt:225">
P1: A rejected sysfs write can be treated as successful because this checks Binder transport success, not the shell command result. Return/validate PServer's command exit status before reporting a write or restoration as successful.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/PServerExecutor.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/PServerExecutor.kt:38">
P1: Unsupported or rejected PServer transactions are reported as successful, so CPU controls can claim settings were applied when no command ran. Convert a `false` `transact` result to `Result.failure` before decoding the reply.</violation>
</file>
<file name="app/src/main/java/app/gamenative/powercontrol/PowerProfile.kt">
<violation number="1" location="app/src/main/java/app/gamenative/powercontrol/PowerProfile.kt:28">
P3: Profile generation has no focused tests for frequency-list boundaries or governor fallbacks, leaving device-dependent presets unguarded. Add unit cases for empty/small/large lists and each supported fallback governor.
(Based on your team's feedback about tests for complex logic.) [FEEDBACK_USED]</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| val reply = Parcel.obtain() | ||
| return try { | ||
| data.writeStringArray(arrayOf(cmd, "1")) | ||
| binder.transact(0, data, reply, 0) |
Contributor
There was a problem hiding this comment.
P1: Unsupported or rejected PServer transactions are reported as successful, so CPU controls can claim settings were applied when no command ran. Convert a false transact result to Result.failure before decoding the reply.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/PServerExecutor.kt, line 38:
<comment>Unsupported or rejected PServer transactions are reported as successful, so CPU controls can claim settings were applied when no command ran. Convert a `false` `transact` result to `Result.failure` before decoding the reply.</comment>
<file context>
@@ -0,0 +1,59 @@
+ val reply = Parcel.obtain()
+ return try {
+ data.writeStringArray(arrayOf(cmd, "1"))
+ binder.transact(0, data, reply, 0)
+ Result.success(decodeReply(reply))
+ } catch (throwable: Throwable) {
</file context>
| var availableGovernors by remember { mutableStateOf<List<String>>(emptyList()) } | ||
| var availableFrequencies by remember { mutableStateOf<List<Long>>(emptyList()) } | ||
| var selectedProfileName by rememberSaveable { mutableStateOf(ProfileName.CUSTOM) } | ||
| var isInitialized by rememberSaveable { mutableStateOf(false) } |
Contributor
There was a problem hiding this comment.
P1: isInitialized is stored via rememberSaveable, which survives process death. However, CpuFreqManager.saveOriginalSettings() only saves values in in-memory fields (originalGovernor, etc.). If the process is recreated, isInitialized restores as true while CpuFreqManager's fields reset to null — so saveOriginalSettings() is never called again and restoreOriginalSettings() becomes a no-op, leaving the CPU in a modified state after shutdown. Use remember instead of rememberSaveable here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt, line 55:
<comment>`isInitialized` is stored via `rememberSaveable`, which survives process death. However, `CpuFreqManager.saveOriginalSettings()` only saves values in in-memory fields (`originalGovernor`, etc.). If the process is recreated, `isInitialized` restores as `true` while `CpuFreqManager`'s fields reset to `null` — so `saveOriginalSettings()` is never called again and `restoreOriginalSettings()` becomes a no-op, leaving the CPU in a modified state after shutdown. Use `remember` instead of `rememberSaveable` here.</comment>
<file context>
@@ -0,0 +1,377 @@
+ var availableGovernors by remember { mutableStateOf<List<String>>(emptyList()) }
+ var availableFrequencies by remember { mutableStateOf<List<Long>>(emptyList()) }
+ var selectedProfileName by rememberSaveable { mutableStateOf(ProfileName.CUSTOM) }
+ var isInitialized by rememberSaveable { mutableStateOf(false) }
+ var isLoading by remember { mutableStateOf(true) }
+ var errorMessage by remember { mutableStateOf<String?>(null) }
</file context>
Suggested change
| var isInitialized by rememberSaveable { mutableStateOf(false) } | |
| var isInitialized by remember { mutableStateOf(false) } |
| onClick = { | ||
| selectedProfileName = profile.name | ||
| CpuFreqManager.setGovernor(profile.governor.governorName) | ||
| CpuFreqManager.setMinFrequency(profile.minFreq) |
Contributor
There was a problem hiding this comment.
P1: Switching from a low-cap profile to a higher-cap profile can leave the old minimum in place because the minimum write violates the current maximum. Choose write order from current/target bounds: raise maximum before minimum when expanding, and lower minimum before maximum when constraining.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt, line 222:
<comment>Switching from a low-cap profile to a higher-cap profile can leave the old minimum in place because the minimum write violates the current maximum. Choose write order from current/target bounds: raise maximum before minimum when expanding, and lower minimum before maximum when constraining.</comment>
<file context>
@@ -0,0 +1,377 @@
+ onClick = {
+ selectedProfileName = profile.name
+ CpuFreqManager.setGovernor(profile.governor.governorName)
+ CpuFreqManager.setMinFrequency(profile.minFreq)
+ CpuFreqManager.setMaxFrequency(profile.maxFreq)
+ // Refresh CPU info after applying profile
</file context>
| }, | ||
| onClick = { | ||
| selectedProfileName = profile.name | ||
| CpuFreqManager.setGovernor(profile.governor.governorName) |
Contributor
There was a problem hiding this comment.
P1: Blocking I/O calls (setGovernor, setMinFrequency, setMaxFrequency, getCpuInfo) inside the profile dropdown's onClick run on the main thread. Each call performs file I/O reading/writing sysfs files and blocking binder transactions via PServer, looping over all CPU cores. This will cause UI jank or a frozen quick menu while the operation completes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt, line 221:
<comment>Blocking I/O calls (setGovernor, setMinFrequency, setMaxFrequency, getCpuInfo) inside the profile dropdown's onClick run on the main thread. Each call performs file I/O reading/writing sysfs files and blocking binder transactions via PServer, looping over all CPU cores. This will cause UI jank or a frozen quick menu while the operation completes.</comment>
<file context>
@@ -0,0 +1,377 @@
+ },
+ onClick = {
+ selectedProfileName = profile.name
+ CpuFreqManager.setGovernor(profile.governor.governorName)
+ CpuFreqManager.setMinFrequency(profile.minFreq)
+ CpuFreqManager.setMaxFrequency(profile.maxFreq)
</file context>
| } | ||
|
|
||
| return try { | ||
| val result = pServerExecutor.executeAsRoot("echo $value > $path") |
Contributor
There was a problem hiding this comment.
P1: A rejected sysfs write can be treated as successful because this checks Binder transport success, not the shell command result. Return/validate PServer's command exit status before reporting a write or restoration as successful.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt, line 225:
<comment>A rejected sysfs write can be treated as successful because this checks Binder transport success, not the shell command result. Return/validate PServer's command exit status before reporting a write or restoration as successful.</comment>
<file context>
@@ -0,0 +1,247 @@
+ }
+
+ return try {
+ val result = pServerExecutor.executeAsRoot("echo $value > $path")
+ if (result.isFailure) {
+ Timber.tag("CpuFreqManager").e("Failed to write to $path: ${result.exceptionOrNull()?.message}")
</file context>
| * - Available frequencies: 307200 - 2016000 KHz (307 MHz - 2.02 GHz, 16 steps) | ||
| * - Frequency steps: 307, 441, 556, 672, 787, 902, 1017, 1113, 1228, 1344, 1459, 1555, 1670, 1785, 1900, 2016 MHz | ||
| */ | ||
| fun getDefaultProfiles(availableGovernors: List<String>, availableFrequencies: List<Long>): List<PowerProfile> { |
Contributor
There was a problem hiding this comment.
P3: Profile generation has no focused tests for frequency-list boundaries or governor fallbacks, leaving device-dependent presets unguarded. Add unit cases for empty/small/large lists and each supported fallback governor.
(Based on your team's feedback about tests for complex logic.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/PowerProfile.kt, line 28:
<comment>Profile generation has no focused tests for frequency-list boundaries or governor fallbacks, leaving device-dependent presets unguarded. Add unit cases for empty/small/large lists and each supported fallback governor.
(Based on your team's feedback about tests for complex logic.) </comment>
<file context>
@@ -0,0 +1,83 @@
+ * - Available frequencies: 307200 - 2016000 KHz (307 MHz - 2.02 GHz, 16 steps)
+ * - Frequency steps: 307, 441, 556, 672, 787, 902, 1017, 1113, 1228, 1344, 1459, 1555, 1670, 1785, 1900, 2016 MHz
+ */
+ fun getDefaultProfiles(availableGovernors: List<String>, availableFrequencies: List<Long>): List<PowerProfile> {
+ if (availableFrequencies.isEmpty()) return emptyList()
+
</file context>
| <string name="power_control_frequencies">周波数設定</string> | ||
| <string name="power_control_min_freq">最小周波数</string> | ||
| <string name="power_control_max_freq">最大周波数</string> | ||
| <string name="power_control_available_frequencies">利用可能な周波数</string> |
Contributor
There was a problem hiding this comment.
P3: power_control_available_frequencies is defined but only referenced in a commented-out code block in PowerControlQuickMenuTab.kt. Unless its UI is about to be uncommented in this PR, adding translations for dead code produces unnecessary strings in resource files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/res/values-ja/strings.xml, line 1599:
<comment>`power_control_available_frequencies` is defined but only referenced in a commented-out code block in `PowerControlQuickMenuTab.kt`. Unless its UI is about to be uncommented in this PR, adding translations for dead code produces unnecessary strings in resource files.</comment>
<file context>
@@ -1585,4 +1585,18 @@
+ <string name="power_control_frequencies">周波数設定</string>
+ <string name="power_control_min_freq">最小周波数</string>
+ <string name="power_control_max_freq">最大周波数</string>
+ <string name="power_control_available_frequencies">利用可能な周波数</string>
+ <string name="power_control_pserver_required">PServerが利用できません</string>
+ <string name="power_control_pserver_required_desc">この機能にはPServerサービスが必要です（AYNおよびRetroidデバイスで利用可能）。現在の設定を表示できますが、PServerなしでは変更できません。</string>
</file context>
|
|
||
| <!-- Power Control --> | ||
| <string name="power_control">パフォーマンス制御</string> | ||
| <string name="power_control_info">CPU周波数スケーリングとガバナー設定を制御します。PServerサービスが必要です。</string> |
Contributor
There was a problem hiding this comment.
P3: 3 new string resources (power_control_info, power_control_driver, power_control_frequencies) are defined in values-ja but never referenced in any Kotlin/Java source code — no R.string.power_control_info, R.string.power_control_driver, or R.string.power_control_frequencies usage exists anywhere in the app. These are dead resources that increase APK size and maintenance burden.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/res/values-ja/strings.xml, line 1591:
<comment>3 new string resources (`power_control_info`, `power_control_driver`, `power_control_frequencies`) are defined in values-ja but never referenced in any Kotlin/Java source code — no `R.string.power_control_info`, `R.string.power_control_driver`, or `R.string.power_control_frequencies` usage exists anywhere in the app. These are dead resources that increase APK size and maintenance burden.</comment>
<file context>
@@ -1585,4 +1585,18 @@
+
+ <!-- Power Control -->
+ <string name="power_control">パフォーマンス制御</string>
+ <string name="power_control_info">CPU周波数スケーリングとガバナー設定を制御します。PServerサービスが必要です。</string>
+ <string name="power_control_driver">ドライバー：%s</string>
+ <string name="power_control_profiles">パフォーマンスプロファイル</string>
</file context>
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text( | ||
| text = errorMessage ?: "Unknown error", |
Contributor
There was a problem hiding this comment.
P3: Hardcoded English fallback string "Unknown error" displayed to users. Should use a string resource from strings.xml for localization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/quickMenus/PowerControlQuickMenuTab.kt, line 129:
<comment>Hardcoded English fallback string "Unknown error" displayed to users. Should use a string resource from strings.xml for localization.</comment>
<file context>
@@ -0,0 +1,377 @@
+ contentAlignment = Alignment.Center
+ ) {
+ Text(
+ text = errorMessage ?: "Unknown error",
+ style = MaterialTheme.typography.bodyMedium,
+ color = MaterialTheme.colorScheme.error,
</file context>
| val cpuPresentFile = File("$CPU_BASE_PATH/present") | ||
| if (cpuPresentFile.exists()) { | ||
| val content = cpuPresentFile.readText().trim() | ||
| val parts = content.split("-") |
Contributor
There was a problem hiding this comment.
P3: getNumCpus() parses /sys/devices/system/cpu/present with a simple split("-") that assumes a single contiguous range like "0-7". On devices where this file uses a comma-separated range format (e.g., "0-2,4-6"), the split produces incorrect results. This would cause the per-CPU write loops to attempt operations on CPU indices that don't exist, generating spurious failure logs. Consider reading /sys/devices/system/cpu/online and counting the online CPUs, or iterating until File("$CPU_BASE_PATH/cpu$i").exists() is false.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/powercontrol/CpuFreqManager.kt, line 190:
<comment>`getNumCpus()` parses `/sys/devices/system/cpu/present` with a simple `split("-")` that assumes a single contiguous range like `"0-7"`. On devices where this file uses a comma-separated range format (e.g., `"0-2,4-6"`), the split produces incorrect results. This would cause the per-CPU write loops to attempt operations on CPU indices that don't exist, generating spurious failure logs. Consider reading `/sys/devices/system/cpu/online` and counting the online CPUs, or iterating until `File("$CPU_BASE_PATH/cpu$i").exists()` is false.</comment>
<file context>
@@ -0,0 +1,247 @@
+ val cpuPresentFile = File("$CPU_BASE_PATH/present")
+ if (cpuPresentFile.exists()) {
+ val content = cpuPresentFile.readText().trim()
+ val parts = content.split("-")
+ if (parts.size == 2) {
+ parts[1].toInt() + 1
</file context>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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
1 participant
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.
Description
Introduces a new "Power Control" tab in the Quick Menu, allowing users to manage CPU frequency scaling and governor settings. This feature is designed to optimize device performance and power consumption during gameplay, particularly on supported handhelds like AYN and Retroid devices.
Key features include:
/sys/devices/system/cpusysfs entries.All works are based on an open source project https://github.com/AurelioB/ClusterTune, thank you!
Recording
screen-20260711-171055.1.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds an experimental Power Control tab to the Quick Menu to view and change CPU governor and min/max frequency on supported handhelds. The tab only appears when
PServeris available, and original CPU settings are safely restored on environment shutdown.New Features
PServerto write sysfs; tab is shown only whenPServeris detected and Quick Menu falls back to HUD if the last tab was Power andPServeris unavailable.PluviaApp; localized strings added for the UI.Bug Fixes
Written for commit c80c2c6. Summary will update on new commits.
Summary by CodeRabbit