-
Notifications
You must be signed in to change notification settings - Fork 55
fix(audio): Defer Bluetooth audio route changes until connection is r… #694
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
base: development
Are you sure you want to change the base?
fix(audio): Defer Bluetooth audio route changes until connection is r… #694
Conversation
104d7b4 to
453db19
Compare
...ava/com/amazonaws/services/chime/sdk/meetings/internal/audio/DefaultAudioClientController.kt
Show resolved
Hide resolved
amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/device/MediaDevice.kt
Show resolved
Hide resolved
| * @param state The new SCO audio state | ||
| * @param previousState The previous SCO audio state | ||
| */ | ||
| private fun onScoStateChanged(state: Int, previousState: Int) { |
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.
It's hard for me to gauge, but is there any way to break this logic out and if so, whether it could be an interface with two implementations? Understandable if not possible.
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.
Yeah it kinda grew to a big chunk without me realizing as we're supporting two APIs. The callback logic makes it a bit tricky to cleanly extract this out but I can give it a shot and get back
1d24b0f to
1332a57
Compare
…eady When switching to a Bluetooth audio device, the SDK was changing the audio route before the Bluetooth connection was established. This caused a race condition where Android would reject Bluetooth routing and fall back to Handset. This fix introduces the BluetoothAudioRouter abstraction to encapsulate Bluetooth connection management. The router defers route changes until the Bluetooth connection is confirmed ready, with implementations selected via BluetoothAudioRouterFactory based on API level. This design isolates the complex asynchronous Bluetooth handling from DefaultDeviceController and allows each platform path to be tested independently. ScoBluetoothAudioRouter (API < 31): - Monitor ACTION_SCO_AUDIO_STATE_UPDATED broadcasts for connection state - Execute deferred setRoute() only when SCO reaches CONNECTED state - Handle SCO connection failures and timeouts (3 seconds) - Distinguish connection failure from device switching for multi-device support CommunicationDeviceBluetoothAudioRouter (API 31+): - Use OnCommunicationDeviceChangedListener for connection confirmation - Retry on mismatch when setCommunicationDevice() succeeds but listener reports a different device (500ms delay, max 3 retries) - 5-second timeout for device switching per Android documentation Additional changes: - Update MediaDevice to track device IDs for API 31+ routing - Add test coverage for both API paths
1332a57 to
013f631
Compare
ℹ️ Description
When switching to a Bluetooth audio device, the SDK was changing the audio
route before the Bluetooth connection was established. This caused a race
condition where Android would reject Bluetooth routing and fall back to
Handset.
This fix introduces the BluetoothAudioRouter abstraction to encapsulate Bluetooth
connection management. The router defers route changes until the Bluetooth
connection is confirmed ready, with implementations selected via
BluetoothAudioRouterFactory based on API level. This design isolates the
complex asynchronous Bluetooth handling from DefaultDeviceController and
allows each platform path to be tested independently.
ScoBluetoothAudioRouter (API < 31):
CommunicationDeviceBluetoothAudioRouter (API 31+):
reports a different device (500ms delay, max 3 retries)
Additional changes:
Issue #, if available
Type of change
🧪 How Has This Been Tested?
Manually verified with a Pixel 8 device running Android
Unit test coverage
Added comprehensive unit tests
Additional Manual Test
📱 Screenshots, if available
provide screenshots/video record if there's a UI change in demo app
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.