Add functionality to check and make app restricted for powerSavingMode #137
Conversation
WalkthroughThis pull request introduces functionality related to power-saving mode within the application. It includes the addition of a Changes
Sequence DiagramsequenceDiagram
participant Device as Device Power Manager
participant App as YourSpace App
participant Receiver as PowerSavingModeObserver
participant Service as AuthService
participant UI as MainActivity
Device->>Receiver: Power Save Mode Changed
Receiver->>Service: Update Power Saving Status
Receiver->>UI: Trigger Power Saving Dialog
UI->>User: Display Power Saving Alert
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
app/src/main/java/com/canopas/yourspace/ui/flow/home/map/component/SelectedUserDetail.kt (3)
164-164: Switch to a more appropriate logging level.
You are logging the battery save mode status at the error level (Timber.e). Since it is not an error, it might be preferable to useTimber.dorTimber.ito avoid misleading error logs.
165-174: Localize or externalize the string for battery saver mode.
Instead of hardcoding "Battery saver mode is on," consider placing this in a string resource for localization and maintainability.-} else { - "Battery saver mode is on" +} else { + stringResource(id = R.string.battery_saver_mode_on)
176-185: Review color logic for maintainability.
Your color assignments for different user states remain clear but might be cleaner if refactored. Consider extracting the color-logic into a helper function for easier testing and reuse, especially as more states are introduced.data/src/main/java/com/canopas/yourspace/data/models/user/ApiUser.kt (1)
51-51: Add clarifying documentation for new property
Consider adding a brief comment to explain howpower_save_mode_enabledintegrates into the user session and whether it directly reflects or overrides the device’s power-saving status. This helps avoid confusion for future contributors.app/src/main/java/com/canopas/yourspace/domain/receiver/PowerSavingModeObserver.kt (1)
25-29: Consider extensibility for future power manager actions
TheinitReceiverfunction is narrowly tailored forACTION_POWER_SAVE_MODE_CHANGED. Consider centralizing intent actions if you plan to observe additional power manager events (e.g.,ACTION_BATTERY_CHANGED).app/src/main/java/com/canopas/yourspace/domain/utils/PowerSavingObserver.kt (2)
17-49: Optimize user messaging and logs
ThesendPowerSavingNotificationmethod logs its status with Timber. If errors occur during notification creation or system constraints prevent showing notifications, consider catching and logging those events as well for troubleshooting.
51-56: Incorporate fallback logic in cancellation
When callingcancelPowerSavingNotification, confirm that theNOTIFICATION_ID_POWER_SAVINGis the sole ID or whether multiple notifications might exist. In a multi-notification scenario, you may want to adopt a design that identifies specific notifications or uses grouped IDs.app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt (1)
15-63: Scope manufacturer-specific logic into utility functions
Thewhenblock elegantly handles manufacturer differences. Consider modularizing each branch into smaller helper methods to separate the logic for readability and easier maintenance if more special-cases arise.data/src/main/java/com/canopas/yourspace/data/service/user/ApiUserService.kt (1)
170-180: Robust session-level update.
updatePowerSaveModeStatusproperly fetches the active session and updates it. Add logging for cases where no active session is found, to help diagnose user states.+ } else { + Timber.w("No active session found for user $currentUserId. Power-saving mode status not updated.") + }app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (1)
80-84: Check for potential repeated checks.
You are immediately checking for power-saving mode and showing the dialog upon activity creation (lines 80-84). Ensure that if the activity is re-created frequently (e.g., on configuration changes), you’re not repeatedly triggering the dialog. You may want to include logic to avoid redundant calls.app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt (1)
193-208: Use localized strings for battery optimization text.
Currently, the text for battery optimization instructions is hardcoded in lines 197-207. Consider using string resources to streamline localization and maintain consistency.app/src/main/res/values/strings.xml (2)
308-309: Consider relocating the notification channel string.For better organization and maintainability, consider moving this string near other notification channel strings (around line 40-50).
- <string name="notification_channel_power_saving">Power saving notification</string>Move the above string near other notification channel strings and enhance its description:
+ <string name="title_notification_channel_power_saving">Power Saving Mode</string> + <string name="description_notification_channel_power_saving">Notifications about device power-saving mode affecting app performance</string>
310-319: Enhance the Chinese devices array for better maintainability.The array could benefit from consistent casing and alphabetical ordering. Also, consider renaming it to better reflect its purpose.
- <string-array name="chinese_devices_name"> + <string-array name="power_saving_device_manufacturers"> <item>huawei</item> <item>xiaomi</item> <item>oppo</item> <item>vivo</item> <item>realme</item> <item>lenovo</item> <item>asus</item> <item>oneplus</item> </string-array>Consider applying these improvements:
- Use proper casing for brand names:
- <item>huawei</item> - <item>xiaomi</item> - <item>oppo</item> - <item>vivo</item> - <item>realme</item> - <item>lenovo</item> - <item>asus</item> - <item>oneplus</item> + <item>ASUS</item> + <item>Huawei</item> + <item>Lenovo</item> + <item>OnePlus</item> + <item>OPPO</item> + <item>Realme</item> + <item>Vivo</item> + <item>Xiaomi</item>
- Add a comment to document the array's purpose:
+ <!-- List of device manufacturers that require special handling for power-saving mode --> <string-array name="power_saving_device_manufacturers">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/canopas/yourspace/YourSpaceApplication.kt(3 hunks)app/src/main/java/com/canopas/yourspace/domain/receiver/PowerSavingModeObserver.kt(1 hunks)app/src/main/java/com/canopas/yourspace/domain/utils/PowerSavingObserver.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt(6 hunks)app/src/main/java/com/canopas/yourspace/ui/MainViewModel.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/home/map/MapViewModel.kt(2 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/home/map/component/SelectedUserDetail.kt(2 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt(5 hunks)app/src/main/res/values/strings.xml(1 hunks)data/src/main/java/com/canopas/yourspace/data/models/user/ApiUser.kt(1 hunks)data/src/main/java/com/canopas/yourspace/data/service/auth/AuthService.kt(1 hunks)data/src/main/java/com/canopas/yourspace/data/service/user/ApiUserService.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.7)
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt
[warning] 324-324: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (22)
app/src/main/java/com/canopas/yourspace/ui/flow/home/map/component/SelectedUserDetail.kt (1)
58-58: Validate your logging dependency usage.
While the Timber import is fine, ensure Timber is properly added to your Gradle dependencies to avoid runtime errors. If not already done, consider including implementation "com.jakewharton.timber:timber:<latest_version>" in your build.gradle file.
app/src/main/java/com/canopas/yourspace/domain/receiver/PowerSavingModeObserver.kt (3)
18-21: Confirm receiver lifecycle management
The @AndroidEntryPoint annotation injects dependencies, but ensure consumers of this class are aware of precisely how and when the BroadcastReceiver is registered/unregistered to avoid memory leaks or orphan receivers.
31-52: Ensure robust error handling for broadcast-based updates
The onReceive method updates the user’s power-save mode status. While the coroutine usage is suitable, ensure tasks can’t overlap or cancel each other if multiple power-save-mode toggles occur rapidly.
54-56: Double-check receiver unregistration
Ensure unregisterReceiver is called if the application transitions into a state where the observer is no longer needed (e.g., user logs out or the app is closed).
app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt (1)
65-79: Handle potential indefinite fallback
If both the default route and battery saver settings fail to launch, there’s a final fallback to ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS. Confirm that this is still usable on all targeted devices or versions.
app/src/main/java/com/canopas/yourspace/YourSpaceApplication.kt (5)
23-23: No issues with the added import.
The import statement for PowerSavingModeObserver is necessary and correctly placed.
52-54: Injecting PowerSavingModeObserver is appropriate.
Ensures the observer can be set up using your existing DI framework.
60-60: Verify the disabling of Crashlytics.
Disabling Crashlytics collection might hamper gathering crash reports in production. Confirm that this is deliberate or controlled by build variants.
63-63: Initialization of the power-saving observer.
Initialization is triggered at the right point in the application lifecycle. Include additional error handling only if the observer’s registration can fail or block the main thread.
76-76: Unregister call looks good.
Ensures a clean tear-down of the observer.
app/src/main/java/com/canopas/yourspace/ui/MainViewModel.kt (3)
147-149: Clear naming and behavior.
showPowerSavingDialog() sets the appropriate state flag, which keeps the UI logic straightforward for showing the dialog.
151-153: Consistent approach to hiding the dialog.
dismissPowerSavingDialog() properly resets isPowerSavingEnabled. Consider logging or analytics to see how often users dismiss.
161-162: New flag aligns with the existing state pattern.
isPowerSavingEnabled is placed consistently with other MainScreenState flags.
app/src/main/java/com/canopas/yourspace/ui/flow/home/map/MapViewModel.kt (2)
122-123: Storing battery saving flag is appropriate.
Supports reacting to a user’s power-saving mode changes in the map UI.
233-233: batterySaveModeValue property extends UI state.
This boolean aligns with the rest of your state model, preserving clarity.
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (2)
109-112: Method validates power-saving status correctly.
This function is straightforward and accurately checks power-saving mode. Make sure that consumer code calling this method properly handles the returned boolean, especially on devices running older Android versions (if minSdk is significantly lower).
121-124: Conditional composable rendering is appropriate.
Displaying the PowerSavingAlertPopup based on isPowerSavingEnabled is logical. Verify that state.isPowerSavingEnabled is always in sync with the real-time device setting, or whether any background broadcast might not toggle it promptly.
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt (2)
209-214: Navigate to battery optimization settings.
The code triggers navigateToSettings(context), presumably to the app's detail or battery settings page. Confirm that this method consistently opens the right settings panel on all devices—some devices might require different URIs for battery optimization screens.
231-235: HorizontalDivider for better UI separation.
This divider is well-placed to separate the permissions section from the footer. Ensure the color and thickness are consistent with your design system.
app/src/main/AndroidManifest.xml (2)
12-12: Permission for notification policy access.
Adding ACCESS_NOTIFICATION_POLICY is valid if you manage Do Not Disturb or similar features. Confirm that you indeed need this permission and that your usage is documented for the Play Store (if required).
63-71: Receiver configuration for PowerSavingModeObserver.
The manifest properly declares the receiver with ACTION_POWER_SAVE_MODE_CHANGED. Good job setting exported="false". This helps keep this broadcast internal to your application.
data/src/main/java/com/canopas/yourspace/data/service/auth/AuthService.kt (1)
138-141: Method to update power-saving mode status is well integrated.
This method updates the user’s power-saving mode status in the backend (line 138-141). Ensure you handle potential API errors (network disruptions, Firestore constraints, etc.) and keep currentUserSession in sync if needed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/src/main/res/values/strings.xml (1)
313-322:⚠️ Potential issueAdd Samsung and Motorola to the device manufacturers list.
Based on the previous review feedback and market share, Samsung and Motorola should be included in the list as they have their own power-saving mechanisms that can affect location sharing.
Apply this diff to add the missing manufacturers:
<string-array name="chinese_devices_name"> <item>huawei</item> <item>xiaomi</item> <item>oppo</item> <item>vivo</item> <item>realme</item> <item>lenovo</item> <item>asus</item> <item>oneplus</item> + <item>samsung</item> + <item>motorola</item> </string-array>Additionally, consider renaming the array to better reflect its purpose:
- <string-array name="chinese_devices_name"> + <string-array name="device_manufacturers_with_custom_power_settings">
🧹 Nitpick comments (7)
app/src/test/java/com/canopas/yourspace/ui/flow/auth/permission/EnablePermissionViewModelTest.kt (1)
14-15: Consider verifying interactions with these mock dependencies.The
userPreferencesandappDispatchermocks are declared, but there are no explicit tests to ensure correct usage. You may want to confirm that theviewModelleverages these dependencies as expected (e.g., verifying calls touserPreferences, checking that coroutines run on the desired dispatcher, etc.).app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt (2)
197-199: Move device manufacturer check to ViewModelThe device manufacturer check logic should be moved to the ViewModel to maintain separation of concerns and make the UI more testable.
- val chineseDeviceList = stringArrayResource(R.array.chinese_devices_name) - val manufacturer = Build.MANUFACTURER.lowercase() - val isChineseDevice = chineseDeviceList.contains(manufacturer)Add this to your ViewModel:
private fun isChineseDevice(context: Context): Boolean { val chineseDeviceList = context.resources.getStringArray(R.array.chinese_devices_name) return chineseDeviceList.contains(Build.MANUFACTURER.lowercase()) }
213-219: Improve logging and string formatting
- Use debug level logging instead of error level for state tracking
- Use string formatting instead of concatenation
- Timber.e("Screen Value ${state.isBatteryOptimized}") + Timber.d("Battery optimization state: ${state.isBatteryOptimized}") - description = "$subtitle1\n\n$subTitle2", + description = stringResource( + R.string.battery_optimization_description_format, + if (isChineseDevice) + stringResource(R.string.battery_optimization_subtitle_chinese) + else + stringResource(R.string.battery_optimization_subtitle_other), + if (isChineseDevice) + stringResource(R.string.battery_optimization_path_chinese) + else + stringResource(R.string.battery_optimization_path_other) + )Add to strings.xml:
<string name="battery_optimization_description_format">%1$s\n\n%2$s</string>app/src/main/res/values/strings.xml (1)
309-311: Consider improving dialog text for better UX.The dialog text could be more user-friendly and actionable:
- <string name="battery_saver_dialog_title">Turn off Battery Saver Mode</string> - <string name="battery_saver_dialog_description">When Battery Saver is on, GroupTrack cannot access location data, preventing the app from working correctly. Please turn off the power saving mode.</string> + <string name="battery_saver_dialog_title">Battery Saver Affects Location Sharing</string> + <string name="battery_saver_dialog_description">To ensure accurate location sharing with your group members, GroupTrack needs unrestricted access to location services. Please turn off battery saver mode to continue.</string>app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt (1)
45-47: Prefer read-only properties in data classes for safer concurrency.
MakingisBatteryOptimizedimmutable can help prevent unintended modifications.-data class EnablePermissionState( - var isBatteryOptimized: Boolean = false -) +data class EnablePermissionState( + val isBatteryOptimized: Boolean = false +)app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt (2)
66-66: Use a more appropriate log level for routine messages.
Switching fromTimber.etoTimber.dorTimber.iis recommended for non-critical events.-Timber.e("Navigating to settings: ${intent.component}") +Timber.d("Navigating to settings: ${intent.component}")
68-79: CatchingExceptionis too broad.
Consider catching more specific exception types to handle errors gracefully and improve debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt(7 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt(7 hunks)app/src/main/res/values/strings.xml(1 hunks)app/src/test/java/com/canopas/yourspace/ui/flow/auth/permission/EnablePermissionViewModelTest.kt(2 hunks)data/src/main/java/com/canopas/yourspace/data/storage/UserPreferences.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt
🔇 Additional comments (11)
app/src/test/java/com/canopas/yourspace/ui/flow/auth/permission/EnablePermissionViewModelTest.kt (2)
3-4: New imports look correct.
The added imports align with the new dependencies being introduced to the ViewModel. No issues here.
17-17: Validate coverage of new ViewModel constructor parameters.
The constructor now includes userPreferences and appDispatcher, but the test only exercises popBack(). Consider adding tests covering scenarios that rely on these new parameters (e.g., the changeBatteryOptimizationValue logic).
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt (2)
106-107: LGTM: State management implementation follows best practices
The state collection from ViewModel using collectAsState() is implemented correctly.
236-240: LGTM: UI layout implementation follows Material 3 guidelines
The HorizontalDivider implementation with custom styling is correct and consistent with Material 3 design system.
app/src/main/res/values/strings.xml (2)
308-308: LGTM: Notification channel string looks good.
The notification channel name is clear and follows the existing pattern in the file (e.g., lines 73-78).
313-322: Verify device manufacturer names are lowercase.
Let's verify that all device manufacturer names in the codebase are consistently lowercase.
✅ Verification successful
Device manufacturer names are consistently lowercase in code comparisons
Based on the search results, all device manufacturer name comparisons in the codebase are consistently using lowercase strings, matching the string array definition. The main usage is in NavigateToDeviceSetting.kt where manufacturer names are used in case statements, and they all follow the lowercase convention.
Let me verify one more aspect to ensure we haven't missed any other comparisons:
Device manufacturer names are consistently lowercase throughout the codebase
The verification confirms that all device manufacturer names are consistently using lowercase across the codebase:
- In the string resources (
strings.xml), all manufacturer names are defined in lowercase - In the code (
NavigateToDeviceSetting.kt), all string comparisons use lowercase manufacturer names - No instances of inconsistent casing were found in other XML resources or in direct
Build.MANUFACTURERcomparisons
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistent casing of manufacturer names in the codebase
# This helps ensure that string comparisons will work correctly
# Search for manufacturer names with different casing
rg -i "(?i)(huawei|xiaomi|oppo|vivo|realme|lenovo|asus|oneplus|samsung|motorola)" \
--type java --type kotlin \
--glob '!**/build/**' \
--glob '!**/generated/**'
# Search specifically for Build.MANUFACTURER comparisons
ast-grep --pattern 'if ($expr == Build.MANUFACTURER) {
$$$
}'
Length of output: 2047
Script:
#!/bin/bash
# Check for direct Build.MANUFACTURER comparisons and string literals
rg -i "Build\.MANUFACTURER.*(?i)(huawei|xiaomi|oppo|vivo|realme|lenovo|asus|oneplus)" \
--type java --type kotlin \
--glob '!**/build/**' \
--glob '!**/generated/**' -A 1
# Also check other string resources that might contain manufacturer names
fd -e xml -t f --exec rg -i "(?i)(huawei|xiaomi|oppo|vivo|realme|lenovo|asus|oneplus)" {}
Length of output: 479
data/src/main/java/com/canopas/yourspace/data/storage/UserPreferences.kt (2)
48-48: LGTM! The preference key is well-defined.
The new boolean preference key follows the existing naming convention and is appropriately placed within the PreferencesKey object.
170-178:
Critical: Avoid blocking operations and clarify property naming.
-
Using
runBlockingin property accessors is problematic as it blocks the calling thread for I/O operations. This can lead to UI freezes or ANRs (Application Not Responding) in Android. -
The property name
isBatteryOptimizationEnabledis ambiguous. Based on the PR objectives, we want to track if the app is excluded from battery optimization.
Consider these changes:
- var isBatteryOptimizationEnabled: Boolean
- get() = runBlocking {
- preferencesDataStore.data.first()[PreferencesKey.KEY_BATTERY_OPTIMIZATION] ?: false
- }
- set(value) = runBlocking {
- preferencesDataStore.edit { preferences ->
- preferences[PreferencesKey.KEY_BATTERY_OPTIMIZATION] = value
- }
- }
+ // Tracks if the app is excluded from battery optimization
+ suspend fun isExcludedFromBatteryOptimization(): Boolean {
+ return preferencesDataStore.data.first()[PreferencesKey.KEY_BATTERY_OPTIMIZATION] ?: false
+ }
+
+ suspend fun setExcludedFromBatteryOptimization(value: Boolean) {
+ preferencesDataStore.edit { preferences ->
+ preferences[PreferencesKey.KEY_BATTERY_OPTIMIZATION] = value
+ }
+ }Let's verify the current usage of this property:
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt (2)
16-18: Dependency injection approach looks good.
The constructor updates properly integrate userPreferences and appDispatcher, following best DI practices.
24-32: Initialization logic is well-structured.
Emitting _state in an init block ensures the UI is aware of the most recent user preference.
app/src/main/java/com/canopas/yourspace/ui/flow/home/activity/NavigateToDeviceSetting.kt (1)
80-80: Confirm timing of changeBatteryOptimizationValue invocation.
Calling viewModel.changeBatteryOptimizationValue(true) before the user actually changes anything in their settings might produce a false-positive state. If the intent fails or the user cancels halfway, the state may not match reality.
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/res/values/strings.xml (1)
322-331: 🛠️ Refactor suggestionAdd Samsung and Motorola to the device list.
While the current list covers major Chinese manufacturers, consider adding:
<string-array name="chinese_devices_name"> <item>huawei</item> <item>xiaomi</item> <item>oppo</item> <item>vivo</item> <item>realme</item> <item>lenovo</item> <item>asus</item> <item>oneplus</item> + <item>samsung</item> + <item>motorola</item> </string-array>
🧹 Nitpick comments (4)
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt (2)
46-48: Consider using immutable properties in data classes.The
isBatteryOptimizedproperty is declared as mutable (var). For better state management and to prevent unexpected mutations, consider making it immutable (val).data class EnablePermissionState( - var isBatteryOptimized: Boolean = false + val isBatteryOptimized: Boolean = false )
24-32: Consider handling potential errors in the coroutine scope.The
refreshBatteryOptimizationStatefunction performs I/O operations but doesn't handle potential errors. Consider adding error handling to prevent crashes and improve user experience.fun refreshBatteryOptimizationState() { viewModelScope.launch(appDispatcher.IO) { + try { _state.emit( state.value.copy( isBatteryOptimized = userPreferences.isBatteryOptimizationEnabled ) ) + } catch (e: Exception) { + Timber.e(e, "Failed to refresh battery optimization state") + } } }app/src/main/java/com/canopas/yourspace/ui/MainViewModel.kt (1)
157-160: Consider caching the PowerManager service reference.The
isPowerSavingModeEnabledfunction gets the system service on each call. Consider caching the PowerManager reference to improve performance.+private var powerManager: PowerManager? = null fun isPowerSavingModeEnabled(context: Context): Boolean { - val powerManager = context.getSystemService(Context.POWER_SERVICE) as PowerManager - return powerManager.isPowerSaveMode + if (powerManager == null) { + powerManager = context.getSystemService(Context.POWER_SERVICE) as PowerManager + } + return powerManager?.isPowerSaveMode ?: false }app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (1)
321-321: Remove debug log statement.The "XXX" prefixed log statement should be removed before production.
- Timber.d("XXX :- PowerSavingAlertPopup: Displaying power saving dialog")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/main/java/com/canopas/yourspace/domain/receiver/PowerSavingModeObserver.kt(1 hunks)app/src/main/java/com/canopas/yourspace/domain/utils/PowerSavingObserver.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt(5 hunks)app/src/main/java/com/canopas/yourspace/ui/MainViewModel.kt(2 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/home/map/component/SelectedUserDetail.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt(1 hunks)app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt(7 hunks)app/src/main/res/values/strings.xml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/canopas/yourspace/ui/flow/home/map/component/SelectedUserDetail.kt
- app/src/main/java/com/canopas/yourspace/domain/utils/PowerSavingObserver.kt
- app/src/main/java/com/canopas/yourspace/domain/receiver/PowerSavingModeObserver.kt
🔇 Additional comments (10)
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionViewModel.kt (1)
34-39: LGTM: Proper state management implementation.
The changeBatteryOptimizationValue function correctly updates both the UI state and persists the value in preferences.
app/src/main/java/com/canopas/yourspace/ui/MainViewModel.kt (1)
149-155: LGTM: Clean implementation of power saving state management.
The state management functions are concise and follow proper practices for updating immutable state.
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (2)
105-106: LGTM: Proper initialization of power saving state.
The initial power saving state is correctly set using LaunchedEffect.
329-334: 🛠️ Refactor suggestion
Consider providing user feedback for settings navigation failure.
When the settings navigation fails, the error is logged but the user receives no feedback.
try {
val intent = Intent(Settings.ACTION_BATTERY_SAVER_SETTINGS)
context.startActivity(intent)
} catch (e: Exception) {
Timber.e("PowerSavingAlertPopup", "Failed to open battery saver settings", e)
+ Toast.makeText(
+ context,
+ context.getString(R.string.error_opening_settings),
+ Toast.LENGTH_SHORT
+ ).show()
}Likely invalid or redundant comment.
app/src/main/java/com/canopas/yourspace/ui/flow/permission/EnablePermissionsScreen.kt (4)
25-25: LGTM: Import changes are appropriate.
The added imports support the new UI components and state management functionality.
Also applies to: 34-34
106-107: LGTM: State management is well implemented.
The state collection and LaunchedEffect for battery optimization refresh are properly implemented.
Also applies to: 113-115
201-204: Consider adding Samsung and Motorola to the device list.
While the device detection logic is sound, Samsung and Motorola, which have their own power-saving mechanisms, are missing from the list.
Run this script to verify device-specific handling:
#!/bin/bash
# Description: Verify device-specific power-saving handling
# Search for any device-specific power-saving implementations
rg -i "samsung|motorola" -A 3
# Check if there are any device-specific settings navigation
ast-grep --pattern 'when ($manufacturer) {
$$$
}'217-222: LGTM: UI implementation is clean and consistent.
The battery optimization permission UI is well-integrated with proper string resources and visual separation.
Also applies to: 239-243
app/src/main/res/values/strings.xml (2)
110-114: LGTM: Battery optimization strings are clear and informative.
The strings provide clear, device-specific instructions for battery optimization settings.
314-320: LGTM: Power-saving notification strings are well-crafted.
The strings effectively communicate the impact of power-saving mode and provide clear actions for users.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (4)
77-82: Add nullable annotation for better null safety.The BroadcastReceiver's
onReceiveparameters should be marked as nullable for better Kotlin null safety:- override fun onReceive(context: Context?, intent: Intent?) { + override fun onReceive(context: Context?, intent: Intent?) {
89-91: Add error handling for receiver registration.The receiver registration could fail if the system is unable to register the broadcast receiver. Add try-catch block:
- val filter = IntentFilter(PowerManager.ACTION_POWER_SAVE_MODE_CHANGED) - registerReceiver(powerSaveReceiver, filter) + try { + val filter = IntentFilter(PowerManager.ACTION_POWER_SAVE_MODE_CHANGED) + registerReceiver(powerSaveReceiver, filter) + } catch (e: Exception) { + Timber.e(e, "Failed to register power save receiver") + }Also applies to: 105-106
120-123: Add error handling for receiver unregistration.The unregisterReceiver call could throw if the receiver wasn't registered successfully. Add try-catch block:
override fun onDestroy() { super.onDestroy() - unregisterReceiver(powerSaveReceiver) + try { + unregisterReceiver(powerSaveReceiver) + } catch (e: Exception) { + Timber.e(e, "Failed to unregister power save receiver") + } }
326-326: Remove debug logging prefix.The "XXX" prefix in the log statement suggests this is temporary debugging code that should be cleaned up.
- Timber.d("XXX :- PowerSavingAlertPopup: Displaying power saving dialog") + Timber.d("PowerSavingAlertPopup: Displaying power saving dialog")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt(6 hunks)
🔇 Additional comments (2)
app/src/main/java/com/canopas/yourspace/ui/MainActivity.kt (2)
132-134: LGTM!
The conditional rendering of PowerSavingAlertPopup based on power saving state is implemented correctly.
334-339: 🛠️ Refactor suggestion
Add user feedback for settings navigation failure.
When the settings navigation fails, the user should be informed. Add a Toast message:
try {
val intent = Intent(Settings.ACTION_BATTERY_SAVER_SETTINGS)
context.startActivity(intent)
} catch (e: Exception) {
Timber.e("PowerSavingAlertPopup", "Failed to open battery saver settings", e)
+ Toast.makeText(
+ context,
+ context.getString(R.string.error_opening_settings),
+ Toast.LENGTH_SHORT
+ ).show()
}Likely invalid or redundant comment.
|
Why its named as Phase-I? Is there any work remaining in this? |
cp-sneh-s
changed the title
|
…notification # Conflicts: # app/src/main/res/values/strings.xml
|
I think you need to show a full message in notification. Here it is showing only one line not showing the full message or making it short.
|
cp-megh-l
changed the title
Changelog
Breaking Changes
New Features
Battery Optimization Check:- On the first launch of the app, users are prompted to ensure the app is excluded from power-saving mode for optimal performance.
Device-Specific Instructions:-
Battery.Saver.mp4
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores