-
Notifications
You must be signed in to change notification settings - Fork 6
RG-T133 Foregound service fix #282
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -737,13 +737,25 @@ export const useLiveKitStore = create<LiveKitState>((set, get) => ({ | |
| // that triggers the already-registered handler. | ||
| if (Platform.OS === 'android') { | ||
| try { | ||
| // connectedDevice type is only legal when a bluetooth PTT handset is actually | ||
| // connected AND a runtime prerequisite (BLUETOOTH_CONNECT) is held — Android 14+ | ||
| // validates both at FGS start and throws SecurityException otherwise, blocking | ||
| // the service. Manifest FOREGROUND_SERVICE_CONNECTED_DEVICE alone is not enough. | ||
| let bluetoothDeviceActive = useBluetoothAudioStore.getState().connectedDevice !== null; | ||
| if (bluetoothDeviceActive) { | ||
| bluetoothDeviceActive = await PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled async permission failure in src/stores/app/livekit-store.ts leaves await PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT) dependent on outer control flow and can produce nondeterministic foreground-service state if the Android permission API rejects. Guard the await with a dedicated try/catch and fall back safely when the check fails. Kody rule violation: Handle async operations with proper error handling try {
bluetoothDeviceActive = await PermissionsAndroid.check(
PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT,
);
} catch (err) {
logger.error('bluetooth permission check failed', {
op: 'PermissionsAndroid.check',
permission: 'BLUETOOTH_CONNECT',
err,
});
bluetoothDeviceActive = false;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Insufficient error context in src/stores/app/livekit-store.ts obscures failures from PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT) because a bare error does not identify the operation or permission. Log structured fields including op, permission, platform, and err in the catch path. Kody rule violation: Include error context in structured logs try {
bluetoothDeviceActive = await PermissionsAndroid.check(
PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT,
);
} catch (err) {
logger.error('bluetooth permission check failed', {
op: 'PermissionsAndroid.check',
permission: 'BLUETOOTH_CONNECT',
platform: 'android',
err,
});
bluetoothDeviceActive = false;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } | ||
| await notifee.displayNotification({ | ||
| title: 'Active PTT Call', | ||
| body: 'There is an active PTT call in progress.', | ||
| android: { | ||
| channelId: 'notif', | ||
| asForegroundService: true, | ||
| foregroundServiceTypes: [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE], | ||
| // microphone: keeps mic capture legal while backgrounded (Android 14+). | ||
| // Playback of remote audio needs no FGS type — any running FGS keeps the process alive. | ||
| foregroundServiceTypes: bluetoothDeviceActive | ||
| ? [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE, AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE] | ||
| : [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE], | ||
|
Comment on lines
+744
to
+758
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Foreground service type drift in src/stores/app/livekit-store.ts causes foregroundServiceTypes to use a one-time useBluetoothAudioStore.getState().connectedDevice snapshot, so later Bluetooth connect or disconnect events never update the active notification. Refresh the foreground notification when useBluetoothAudioStore.connectedDevice changes during an active call, or derive the service types in the Bluetooth connect and disconnect handlers so Android 14 connected-device compliance stays correct without requiring the user to rejoin the room. const showForegroundServiceNotification = async () => {
let bluetoothDeviceActive = useBluetoothAudioStore.getState().connectedDevice !== null;
if (bluetoothDeviceActive) {
bluetoothDeviceActive = await PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT);
}
await notifee.displayNotification({
title: 'Active PTT Call',
body: 'There is an active PTT call in progress.',
android: {
channelId: 'notif',
asForegroundService: true,
foregroundServiceTypes: bluetoothDeviceActive
? [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE, AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE]
: [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE],
smallIcon: 'ic_launcher',
},
});
};
await showForegroundServiceNotification();
const unsubscribe = useBluetoothAudioStore.subscribe(async (state, prev) => {
if (get().isConnected && state.connectedDevice !== prev.connectedDevice) {
await showForegroundServiceNotification();
}
});Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| smallIcon: 'ic_launcher', | ||
| }, | ||
| }); | ||
|
Comment on lines
748
to
761
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled platform API failure in src/stores/app/livekit-store.ts allows await notifee.displayNotification(...) to fail without deterministic application behavior or diagnostic context. Wrap the notification call in try/catch, log structured metadata such as op, notificationType, bluetoothDeviceActive, and err, and then recover safely or rethrow. Kody rule violation: Add try-catch blocks for external calls try {
await notifee.displayNotification({
title: 'Active PTT Call',
body: 'There is an active PTT call in progress.',
android: {
channelId: 'notif',
asForegroundService: true,
foregroundServiceTypes: bluetoothDeviceActive
? [
AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE,
AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE,
]
: [AndroidForegroundServiceType.FOREGROUND_SERVICE_TYPE_MICROPHONE],
smallIcon: 'ic_launcher',
},
});
} catch (err) {
logger.error('display notification failed', {
op: 'notifee.displayNotification',
notificationType: 'active-ptt-call',
bluetoothDeviceActive,
err,
});
throw err;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Report this failure at one boundary.
When
requestPermissions()rejects duringupdateRealtimeGeolocationSetting, this block callslogger.error()and rethrows. The caller at Lines 537-543 callslogger.error()for the same error. Becauselogger.error()captures exceptions in Sentry, this path submits duplicate error events. Keep the context at one boundary and make the other log non-reporting.🤖 Prompt for AI Agents