-
-
Notifications
You must be signed in to change notification settings - Fork 213
Add action to modify Android system settings via key mappings #1872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Co-authored-by: sds100 <[email protected]>
Co-authored-by: sds100 <[email protected]>
Co-authored-by: sds100 <[email protected]>
…access Co-authored-by: sds100 <[email protected]>
- Replace 3 separate ActionIds with single MODIFY_SETTING - Add SettingType enum (SYSTEM, SECURE, GLOBAL) - Update ActionData.ModifySetting to use enum instead of sealed class - Add dropdown for setting type in UI - Default to SYSTEM type (no PRO mode required) - Update all mappers and handlers accordingly Co-authored-by: sds100 <[email protected]>
Done in commit 927a3d6:
Note: I've created |
sds100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implement my changes
| open val settingKey: String, | ||
| open val value: String, | ||
| ) { | ||
| data class System( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use enum instead
| audioService.setRingerModeInternal(ringerMode, processPackageName) | ||
| } | ||
|
|
||
| override fun putSystemSetting(key: String?, value: String?): Boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake. Do not need to use the system bridge at all for writing settings. The system bridge will grant Key Mapper WRITE_SECURE_SETTINGS permission when its started. So for secure and global settings, ask for Permission.WRITE_SECURE_SETTINGS in ActionErrorSnapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 51b18eb. Removed all SystemBridge methods for writing settings. Now using SettingsUtils directly with permission checks in ActionErrorSnapshot:
- System settings: requires WRITE_SETTINGS permission
- Secure/Global settings: requires WRITE_SECURE_SETTINGS permission (granted by SystemBridge on start)
| return SettingsUtils.getSecureSetting<Int>(ctx, "doze_always_on") == 1 | ||
| } | ||
|
|
||
| override fun modifySystemSetting(key: String, value: String): KMResult<*> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the existing methods in SettingsUtils. And create an AndroidSettingsAdapter interface for it so PerformActionsUseCase can use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 51b18eb. Created AndroidSettingsAdapter interface that uses SettingsUtils methods for modifying settings. PerformActionsUseCase now uses SettingsAdapter instead of DisplayAdapter.
| ActionId.FORCE_STOP_APP -> R.string.action_force_stop_app | ||
| ActionId.CLEAR_RECENT_APP -> R.string.action_clear_recent_app | ||
|
|
||
| ActionId.MODIFY_SYSTEM_SETTING -> R.string.action_modify_system_setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a single ActionId and put it in the APPS category
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 51b18eb. Moved MODIFY_SETTING to APPS category.
| } | ||
|
|
||
| @Composable | ||
| private fun ModifySettingActionBottomSheetContent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a dropdown. Create another screen that lets the user select an existing setting and add a button to link to it. Each list item should have the key and current value. This screen should have a search box.
Look at ChooseActionsScreen for how this is done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commits 51b18eb and b57cdf8. Created ChooseSettingScreen similar to ChooseActionsScreen:
- Searchable list of all available settings
- Shows current value for each setting
- Dropdown to select setting type (System/Secure/Global)
- Each list item displays key and current value
- Returns selected setting to populate bottom sheet
Next commit will wire up the navigation to open this screen from the bottom sheet.
- Move MODIFY_SETTING to APPS category - Create AndroidSettingsAdapter interface implementing SettingsAdapter - Use SettingsUtils for all setting modifications (no SystemBridge needed) - Remove SystemBridge methods for settings (putSystemSetting, etc) - Add permission checks: WRITE_SETTINGS for System, WRITE_SECURE_SETTINGS for Secure/Global - Remove setting methods from DisplayAdapter Co-authored-by: sds100 <[email protected]>
- Created ChooseSettingViewModel with setting type selection and search - Created ChooseSettingScreen with dropdown for type and searchable list - Added ChooseSetting nav destination - Added string resources for choosesetting screen - Screen shows all available settings with current values - User can select from existing settings or enter custom key Next: Wire up navigation in CreateActionDelegate Co-authored-by: sds100 <[email protected]>
Changes implemented in commits 51b18eb and b57cdf8. Summary:
Still completing navigation wiring to connect the Choose Setting button to the screen. |
sds100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implement my review
| fun onSettingClick(key: String, currentValue: String?) { | ||
| viewModelScope.launch { | ||
| popBackStackWithResult( | ||
| kotlinx.serialization.json.Json.encodeToString( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use fully qualified names, import them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in c1f999f. Now importing SettingType, Json, and Serializable instead of using fully qualified names.
|
|
||
| @kotlinx.serialization.Serializable | ||
| data class ChooseSettingResult( | ||
| val settingType: io.github.sds100.keymapper.system.settings.SettingType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in c1f999f. Removed all fully qualified names throughout the file.
| modifier = Modifier.fillMaxWidth().menuAnchor(), | ||
| singleLine = true, | ||
| ) | ||
| ExposedDropdownMenu( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use KeyMapperDropdownMenu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in c1f999f. Replaced custom ExposedDropdownMenuBox implementation with KeyMapperDropdownMenu component. Also cleaned up unused imports.
…perDropdownMenu - Import SettingType instead of using fully qualified names - Import Json and Serializable for cleaner code - Replace custom dropdown with KeyMapperDropdownMenu in ChooseSettingScreen - Remove unused imports - Update NavDestination to use imported SettingType Co-authored-by: sds100 <[email protected]>
- Consolidate modify methods into single modifySetting(settingType, key, value) method - Import SettingType instead of using fully qualified name - Replace custom dropdown with KeyMapperDropdownMenu - Add onChooseSetting callback and Button to navigate to ChooseSettingScreen - Update PerformActionsUseCase to use new unified method - Add onChooseSettingClick method to CreateActionDelegate for navigation Co-authored-by: sds100 <[email protected]>
All review items implemented in c351c61:
Code is now cleaner and follows project conventions. |
sds100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more changes
| } | ||
|
|
||
| interface SettingsAdapter { | ||
| fun getSystemSettingKeys(): List<String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the same for getValue and getKeys methods. Can you refactor the getKeys methods to be called "getAll" and return a Map<String, String> with the keys and values of all settings.
|
|
||
| is ActionData.ModifySetting -> { | ||
| return when (action.settingType) { | ||
| io.github.sds100.keymapper.system.settings.SettingType.SYSTEM -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import SettingType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in b149abf. Now importing SettingType and using it throughout the file instead of fully qualified name.
…rrorSnapshot - Refactored getKeys methods into single getAll(settingType) returning Map<String, String?> - Consolidated getValue methods into single getValue(settingType, key) method - Updated ChooseSettingViewModel to use new unified methods - Imported SettingType in ActionErrorSnapshot instead of using fully qualified name - Simplified interface with only 3 methods: getAll, getValue, modifySetting Co-authored-by: sds100 <[email protected]>
All changes implemented in b149abf:
The interface is now fully consistent with all methods taking SettingType as a parameter. |
Task: Action to Modify System Settings
Implementation updated per review feedback:
Latest Changes (Interface Simplification)
SettingsAdapter Complete Refactoring
Map<String, String?>with both keys and values in one callgetAll,getValue,modifySettingActionErrorSnapshot
ChooseSettingViewModel
getAllcall to get both keys and valuesAll review feedback addressed. The API is now consistent with all methods taking SettingType as a parameter.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.