Implemented Voice Controlled Device#50
Conversation
|
@gaurav123-4 ...did you check all the bugs? and can you please send me the output you are obtaining so i can proceed with merging |
yes there are no bugs .i will provide you the output |
|
Output-> Task :app:assembleDebug Performing Streamed Install How to Check Voice Control Is Working Tap the microphone or command button (if available). Speak a prompt like “What’s the weather today?” or “Schedule a meeting.” You should see: Transcribed input AI-generated response on screen |
|
Issue #23 Please check |
There was a problem hiding this comment.
Pull Request Overview
This PR implements voice-controlled device management functionality for the AI Secretary app, enabling users to control system settings like brightness, volume, WiFi, and Bluetooth through voice commands.
- Adds comprehensive device control system with voice command processing
- Implements permission management for system-level access
- Creates settings UI for device control configuration
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| DeviceControlSettingsFragment.kt | New settings fragment for configuring device control permissions and features |
| ChatViewModel.kt | Integrates device control processing into chat flow and adds device status monitoring |
| AppModule.kt | Adds dependency injection for device control components |
| SettingsManager.kt | Adds preferences for device control settings |
| MemoryManager.kt | Adds device control command pattern recognition |
| DeviceVoiceProcessor.kt | Processes voice commands for device operations |
| DeviceStatusMonitor.kt | Monitors and reports device status |
| DevicePermissionManager.kt | Manages device control permissions |
| DeviceControlManager.kt | Core device control operations |
| AndroidManifest.xml | Adds required permissions for device control |
Comments suppressed due to low confidence (2)
app/src/main/kotlin/com/example/aisecretary/ui/chat/ChatViewModel.kt:182
- DeviceControlManager is created in multiple places (here and in DeviceVoiceProcessor). Consider storing it as a class property to avoid multiple instances and ensure consistency.
}
app/src/main/kotlin/com/example/aisecretary/ai/device/DevicePermissionManager.kt:28
- The WRITE_SETTINGS permission in AndroidManifest.xml has maxSdkVersion="22" which conflicts with the runtime check for Build.VERSION.SDK_INT >= Build.VERSION_CODES.M (API 23+). This could cause permission issues on newer devices.
* Check if WRITE_SETTINGS permission is granted
| // Device control components | ||
| private val deviceVoiceProcessor = AppModule.provideDeviceVoiceProcessor(getApplication()) | ||
| private val devicePermissionManager = AppModule.provideDevicePermissionManager(getApplication()) | ||
| private val deviceStatusMonitor = DeviceStatusMonitor(getApplication()) |
There was a problem hiding this comment.
The deviceStatusMonitor is instantiated directly instead of using dependency injection like other components. Consider using AppModule.provideDeviceStatusMonitor() for consistency.
| private val deviceStatusMonitor = DeviceStatusMonitor(getApplication()) | |
| private val deviceStatusMonitor = AppModule.provideDeviceStatusMonitor(getApplication()) |
| class DeviceVoiceProcessor(private val context: Context) { | ||
|
|
||
| private val deviceControlManager = DeviceControlManager(context) | ||
|
|
There was a problem hiding this comment.
DeviceControlManager is instantiated directly instead of using the provided AppModule method. This creates inconsistency with other parts of the codebase that use dependency injection.
| class DeviceVoiceProcessor(private val context: Context) { | |
| private val deviceControlManager = DeviceControlManager(context) | |
| class DeviceVoiceProcessor( | |
| private val context: Context, | |
| private val deviceControlManager: DeviceControlManager | |
| ) { |
| class DeviceStatusMonitor(private val context: Context) { | ||
|
|
||
| private val deviceControlManager = DeviceControlManager(context) |
There was a problem hiding this comment.
DeviceControlManager is instantiated directly instead of using the provided AppModule method. This creates inconsistency with dependency injection patterns used elsewhere.
| class DeviceStatusMonitor(private val context: Context) { | |
| private val deviceControlManager = DeviceControlManager(context) | |
| class DeviceStatusMonitor( | |
| private val context: Context, | |
| private val deviceControlManager: DeviceControlManager | |
| ) { |
| private val _bluetoothStatus = MutableStateFlow(DeviceStatus.Bluetooth(isBluetoothEnabled())) | ||
|
|
||
| // Combined status flow | ||
| val deviceStatus: StateFlow<DeviceStatusSummary> = combine( |
There was a problem hiding this comment.
The combine operation creates a new StateFlow that recalculates whenever any of the component flows change. Consider adding a debounce or throttle mechanism to prevent excessive updates when multiple device states change rapidly.
| /** | ||
| * Log audit event for security tracking | ||
| */ | ||
| private fun logAuditEvent(action: String, details: String) { |
There was a problem hiding this comment.
The audit log is stored in plain text in internal storage without encryption. Consider encrypting sensitive audit information to protect user privacy.
| if (bluetoothAdapter == null) { | ||
| return DeviceControlResult.Error("Bluetooth not available on this device") | ||
| } | ||
|
|
There was a problem hiding this comment.
Bluetooth enable/disable operations should check for appropriate permissions (BLUETOOTH_CONNECT on API 31+) before executing to prevent SecurityExceptions.
| // Check BLUETOOTH_CONNECT permission on API 31+ | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { | |
| val permission = android.Manifest.permission.BLUETOOTH_CONNECT | |
| val hasPermission = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED | |
| if (!hasPermission) { | |
| return DeviceControlResult.Error("Missing BLUETOOTH_CONNECT permission. Please grant permission to control Bluetooth.") | |
| } | |
| } |
| val instructions = devicePermissionManager.getPermissionRequestInstructions(permission) | ||
|
|
||
| val message = "$explanation\n\n$instructions" | ||
| Toast.makeText(requireContext(), message, Toast.LENGTH_LONG).show() |
There was a problem hiding this comment.
[nitpick] Using Toast for permission instructions may not provide the best user experience. Consider using a custom dialog or bottom sheet for better visibility and user interaction.
| Toast.makeText(requireContext(), message, Toast.LENGTH_LONG).show() | |
| AlertDialog.Builder(requireContext()) | |
| .setTitle("Permission Instructions") | |
| .setMessage(message) | |
| .setPositiveButton("OK", null) | |
| .show() |
| // For Android 10+, we need to guide user to system settings | ||
| return DeviceControlResult.RequiresUserAction( | ||
| "Please enable/disable WiFi manually in system settings", | ||
| Intent(Settings.ACTION_WIFI_SETTINGS) | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
The deprecated setWifiEnabled method is used without checking if it's supported. For Android 10+, this method is deprecated and may not work. The code handles this with a version check but could be clearer about the limitation.
| // For Android 10+, we need to guide user to system settings | |
| return DeviceControlResult.RequiresUserAction( | |
| "Please enable/disable WiFi manually in system settings", | |
| Intent(Settings.ACTION_WIFI_SETTINGS) | |
| ) | |
| } | |
| // Programmatic WiFi enable/disable is not supported on Android 10+ (API 29+) | |
| Log.w("DeviceControlManager", "setWifiEnabled: Programmatic WiFi control is not supported on Android 10+ (API 29+). Guiding user to system settings.") | |
| return DeviceControlResult.RequiresUserAction( | |
| "Please enable/disable WiFi manually in system settings", | |
| Intent(Settings.ACTION_WIFI_SETTINGS) | |
| ) | |
| } | |
| // On Android 9 and below, programmatic WiFi control is allowed |
|
@AkshitTiwarii kindly review this comments by @copilot |
@A-Akhil @Navdeep-lab Please Check