From 372fcd0a8f63599d3443a9e1942c305322617141 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 18 Mar 2026 11:05:52 +0100 Subject: [PATCH] Fix device update logic and more tests --- .../AndroidControlledAudioOutput.test.ts | 115 +++++- src/state/AndroidControlledAudioOutput.ts | 370 +++++++++++------- 2 files changed, 347 insertions(+), 138 deletions(-) diff --git a/src/state/AndroidControlledAudioOutput.test.ts b/src/state/AndroidControlledAudioOutput.test.ts index 0948f24d..12b74052 100644 --- a/src/state/AndroidControlledAudioOutput.test.ts +++ b/src/state/AndroidControlledAudioOutput.test.ts @@ -291,13 +291,96 @@ describe("Test select a device", () => { }); }); }); + + it(`manually switch then a bt headset is added`, () => { + withTestScheduler(({ cold, schedule, expectObservable, flush }) => { + const controlledAudioOutput = new AndroidControlledAudioOutput( + cold("a--b", { + a: BASE_DEVICE_LIST, + b: BT_HEADSET_BASE_DEVICE_LIST, + }), + testScope, + "audio", + mockControls, + ); + + // Default was earpiece (audio call), let's switch to speaker + schedule("-a--", { + a: () => controlledAudioOutput.select(SPEAKER_DEVICE.id), + }); + + expectObservable(controlledAudioOutput.selected$).toBe("ab-c", { + // virtualEarpiece is always false on android. + // Initially the BT_HEADSET is selected. + a: { id: EARPIECE_DEVICE.id, virtualEarpiece: false }, + b: { id: SPEAKER_DEVICE.id, virtualEarpiece: false }, + c: { id: BT_HEADSET_DEVICE.id, virtualEarpiece: false }, + }); + + flush(); + + [ + mockControls.onOutputDeviceSelect, + mockControls.onAudioDeviceSelect, + ].forEach((mockFn) => { + expect(mockFn).toHaveBeenCalledTimes(3); + expect(mockFn).toHaveBeenNthCalledWith(1, EARPIECE_DEVICE.id); + expect(mockFn).toHaveBeenNthCalledWith(2, SPEAKER_DEVICE.id); + expect(mockFn).toHaveBeenNthCalledWith(3, BT_HEADSET_DEVICE.id); + }); + }); + }); + + it(`Go back to the previously selected after the auto-switch device goes away`, () => { + withTestScheduler(({ cold, schedule, expectObservable, flush }) => { + const controlledAudioOutput = new AndroidControlledAudioOutput( + cold("a--b-c", { + a: BASE_DEVICE_LIST, + b: BT_HEADSET_BASE_DEVICE_LIST, + c: BASE_DEVICE_LIST, + }), + testScope, + "audio", + mockControls, + ); + + // Default was earpiece (audio call), let's switch to speaker + schedule("-a---", { + a: () => controlledAudioOutput.select(SPEAKER_DEVICE.id), + }); + + expectObservable(controlledAudioOutput.selected$).toBe("ab-c-d", { + // virtualEarpiece is always false on android. + // Initially the BT_HEADSET is selected. + a: { id: EARPIECE_DEVICE.id, virtualEarpiece: false }, + b: { id: SPEAKER_DEVICE.id, virtualEarpiece: false }, + c: { id: BT_HEADSET_DEVICE.id, virtualEarpiece: false }, + d: { id: SPEAKER_DEVICE.id, virtualEarpiece: false }, + }); + + flush(); + + [ + mockControls.onOutputDeviceSelect, + mockControls.onAudioDeviceSelect, + ].forEach((mockFn) => { + expect(mockFn).toHaveBeenCalledTimes(4); + expect(mockFn).toHaveBeenNthCalledWith(1, EARPIECE_DEVICE.id); + expect(mockFn).toHaveBeenNthCalledWith(2, SPEAKER_DEVICE.id); + expect(mockFn).toHaveBeenNthCalledWith(3, BT_HEADSET_DEVICE.id); + expect(mockFn).toHaveBeenNthCalledWith(4, SPEAKER_DEVICE.id); + }); + }); + }); }); describe("Available device changes", () => { let availableSource$: Subject; - const createAudioControlledOutput = (intent: RTCCallIntent): void => { - new AndroidControlledAudioOutput( + const createAudioControlledOutput = ( + intent: RTCCallIntent, + ): AndroidControlledAudioOutput => { + return new AndroidControlledAudioOutput( availableSource$, testScope, intent, @@ -309,7 +392,7 @@ describe("Available device changes", () => { availableSource$ = new Subject(); }); - it("When a BT speaker is added, control should switch to use it", () => { + it("When a BT headset is added, control should switch to use it", () => { createAudioControlledOutput("video"); // Emit the base device list, the speaker should be selected @@ -334,11 +417,14 @@ describe("Available device changes", () => { }); }); - it("When a wired headset speaker is added, control should switch to use it", () => { - createAudioControlledOutput("video"); + // Android does not set `isExternalHeadset` to true for wired headphones, so we can't test this case.' + it.skip("When a wired headset is added, control should switch to use it", async () => { + const controlledAudioOutput = createAudioControlledOutput("video"); // Emit the base device list, the speaker should be selected availableSource$.next(BASE_DEVICE_LIST); + + await firstValueFrom(controlledAudioOutput.selected$.pipe(take(1))); // Initially speaker would be selected [ mockControls.onOutputDeviceSelect, @@ -409,6 +495,25 @@ describe("Available device changes", () => { expect(mockFn).toHaveBeenLastCalledWith(SPEAKER_DEVICE.id); }); }); + + it("Do not repeatidly set the same device", () => { + createAudioControlledOutput("video"); + + availableSource$.next(BT_HEADSET_BASE_DEVICE_LIST); + availableSource$.next(BT_HEADSET_BASE_DEVICE_LIST); + availableSource$.next(BT_HEADSET_BASE_DEVICE_LIST); + availableSource$.next(BT_HEADSET_BASE_DEVICE_LIST); + availableSource$.next(BT_HEADSET_BASE_DEVICE_LIST); + + // Initially bt headset would be selected + [ + mockControls.onOutputDeviceSelect, + mockControls.onAudioDeviceSelect, + ].forEach((mockFn) => { + expect(mockFn).toHaveBeenCalledTimes(1); + expect(mockFn).toHaveBeenCalledWith(BT_HEADSET_DEVICE.id); + }); + }); }); describe("Scope management", () => { diff --git a/src/state/AndroidControlledAudioOutput.ts b/src/state/AndroidControlledAudioOutput.ts index 9c2950b5..8c223685 100644 --- a/src/state/AndroidControlledAudioOutput.ts +++ b/src/state/AndroidControlledAudioOutput.ts @@ -6,14 +6,13 @@ Please see LICENSE in the repository root for full details. */ import { logger as rootLogger } from "matrix-js-sdk/lib/logger"; import { - combineLatest, - EMPTY, + distinctUntilChanged, map, + merge, type Observable, - pairwise, + scan, startWith, Subject, - switchMap, tap, } from "rxjs"; @@ -25,7 +24,30 @@ import { import type { ObservableScope } from "./ObservableScope.ts"; import type { RTCCallIntent } from "matrix-js-sdk/lib/matrixrtc"; import { type Controls, type OutputDevice } from "../controls.ts"; +import { type Behavior } from "./Behavior.ts"; +type ControllerState = { + /** + * The list of available output devices, ordered by preference order (most preferred first). + */ + devices: OutputDevice[]; + /** + * Explicit user preference for the selected device. + */ + preferredDeviceId: string | undefined; + /** + * The effective selected device, always valid against available devices. + */ + selectedDeviceId: string | undefined; +}; + +/** + * The possible actions that can be performed on the controller, + * either by the user or by the system. + */ +type ControllerAction = + | { type: "selectDevice"; deviceId: string | undefined } + | { type: "deviceUpdated"; devices: OutputDevice[] }; /** * The implementation of the audio output media device for Android when using the controlled audio output mode. * @@ -48,6 +70,30 @@ export class AndroidControlledAudioOutput implements MediaDevice< "[MediaDevices AndroidControlledAudioOutput]", ); + // STATE stream: the current state of the controller, including the list of available devices and the selected device. + private readonly controllerState$: Behavior; + + /** + * @inheritdoc + */ + public readonly available$: Behavior>; + + /** + * Effective selected device, always valid against available devices. + * + * On android, we don't listen to the selected device from native code (control.setAudioDevice). + * Instead, we determine the selected device ourselves based on the available devices and the user's selection (if any). + */ + public readonly selected$: Behavior; + + // COMMAND stream: user asks to select a device + private readonly selectDeviceCommand$ = new Subject(); + + public select(id: string): void { + this.logger.info(`select device: ${id}`); + this.selectDeviceCommand$.next(id); + } + /** * Creates an instance of AndroidControlledAudioOutput. * @@ -63,139 +109,196 @@ export class AndroidControlledAudioOutput implements MediaDevice< private initialIntent: RTCCallIntent | undefined = undefined, controls: Controls, ) { - this.selected$.pipe(scope.bind()).subscribe((device) => { - // Let the hosting application know which output device has been selected. - if (device !== undefined) { - this.logger.info("onAudioDeviceSelect called:", device); - controls.onAudioDeviceSelect?.(device.id); - // Also invoke the deprecated callback for backward compatibility - // TODO: it appears that on Android the hosting application is only using the deprecated callback (onOutputDeviceSelect) - // and not the new one (onAudioDeviceSelect), we should clean this up and only have one callback for audio device selection. - controls.onOutputDeviceSelect?.(device.id); - } - }); + this.controllerState$ = this.startObservingState$(); - this.selected$ - .pipe( - switchMap((selected, index) => { - if (selected == undefined) { - // If we don't have a selected device, - // we don't need to listen to changes in the available devices - return EMPTY; - } - // For a given selected device, we want to listen to changes in the available devices - // and determine if we need to switch the selected device based on that. - return this.controlledDevices$.pipe(pairwise()).pipe( - map(([previous, current]) => { - // If a device is added we might want to switch to it if it's more preferred than the currently selected device. - // If a device is removed, and it was the currently used, we want to switch to the next best device. - const newDeviceWasAdded = current.some( - (device) => !previous.some((d) => d.id === device.id), - ); + this.selected$ = this.effectiveSelectionFromState$(this.controllerState$); - if (newDeviceWasAdded) { - // check if the currently selected device is the most preferred one, if not switch to the most preferred one. - const mostPreferredDevice = current[0]; - if (mostPreferredDevice.id !== selected.id) { - // Given this is automatic switching, we want to be careful and only switch to a more private device - // (e.g. from speaker to a BT headset) but not switch from a more private device to a less private one - // (e.g. from a BT headset to the speaker), as that can be disruptive for the user if it happens unexpectedly. - const candidate = current.find( - (device) => device.id === selected.id, - ); - if (candidate?.isExternalHeadset == true) { - // Let's switch as it is a more private device. - this.deviceSelection$.next(mostPreferredDevice.id); - return; - } - } - } + this.available$ = scope.behavior( + this.controllerState$.pipe( + map((state) => { + this.logger.info("available devices updated:", state.devices); - const isSelectedDeviceStillAvailable = current.some( - (device) => device.id === selected.id, - ); - - if (!isSelectedDeviceStillAvailable) { - // The currently selected device is no longer available, switch to the most preferred available device. - // we can do this by switching back to the default device selection logic (by setting the preferred device to undefined), - // which will pick the most preferred available device. - this.logger.info( - `The currently selected device ${selected.id} is no longer available, switching to the most preferred available device.`, - ); - this.deviceSelection$.next(undefined); - } + return new Map( + state.devices.map((outputDevice) => { + return [outputDevice.id, mapDeviceToLabel(outputDevice)]; }), ); }), - ) + ), + ); + + // Effect 1: notify host when effective selection changes + this.selected$ + // It is a behavior so it has built-in distinct until change .pipe(scope.bind()) - .subscribe(); + .subscribe((device) => { + // Let the hosting application know which output device has been selected. + if (device !== undefined) { + this.logger.info("onAudioDeviceSelect called:", device); + controls.onAudioDeviceSelect?.(device.id); + // Also invoke the deprecated callback for backward compatibility + // TODO: it appears that on Android the hosting application is only using the deprecated callback (onOutputDeviceSelect) + // and not the new one (onAudioDeviceSelect), we should clean this up and only have one callback for audio device selection. + controls.onOutputDeviceSelect?.(device.id); + } + }); } - private readonly deviceSelection$ = new Subject(); + private startObservingState$(): Behavior { + const initialState: ControllerState = { + devices: [], + preferredDeviceId: undefined, + selectedDeviceId: undefined, + }; - public select(id: string): void { - this.logger.info(`select device: ${id}`); - this.deviceSelection$.next(id); - } + // Merge the two possible inputs observable as a single + // stream of actions that will update the state of the controller. + const actions$ = merge( + this.controlledDevices$.pipe( + map( + (devices) => + ({ type: "deviceUpdated", devices }) satisfies ControllerAction, + ), + ), + this.selectDeviceCommand$.pipe( + map( + (deviceId) => + ({ type: "selectDevice", deviceId }) satisfies ControllerAction, + ), + ), + ); - public readonly available$ = this.scope.behavior( - this.controlledDevices$.pipe( - map((availableRaw) => { - this.logger.info("available raw devices:", availableRaw); + const initialAction: ControllerAction = { + type: "deviceUpdated", + devices: [], + }; - const available = new Map( - availableRaw.map((outputDevice) => { - return [outputDevice.id, mapDeviceToLabel(outputDevice)]; - }), - ); + return this.scope.behavior( + actions$.pipe( + startWith(initialAction), + scan((state, action): ControllerState => { + if (action.type === "deviceUpdated") { + const chosenDevice = this.chooseEffectiveSelection({ + previousDevices: state.devices, + availableDevices: action.devices, + currentSelectedId: state.selectedDeviceId, + preferredDeviceId: state.preferredDeviceId, + }); - this.logger.info("available devices mapped:", available); - - return available; - }), - ), - // start with an empty map - new Map(), - ); - - /** - * On android, we don't listen to the selected device from native code (control.setAudioDevice). - * Instead, we determine the selected device ourselves based on the available devices and the user's selection (if any). - */ - public readonly selected$ = this.scope.behavior( - combineLatest([ - this.available$, - this.deviceSelection$.pipe(startWith(undefined)), - ]) - .pipe( - map(([available, preferredId]) => { - this.logger.debug( - `selecting device: Preferred:${preferredId}: intent:${this.initialIntent}: Available: ${Array.from(available.keys()).join(",")}`, - ); - if (preferredId !== undefined) { return { - id: preferredId, - virtualEarpiece: false /** This is an iOS thing, always false for android*/, + ...state, + devices: action.devices, + selectedDeviceId: chosenDevice, + }; + } else if (action.type === "selectDevice") { + const chosenDevice = this.chooseEffectiveSelection({ + previousDevices: state.devices, + availableDevices: state.devices, + currentSelectedId: state.selectedDeviceId, + preferredDeviceId: action.deviceId, + }); + + return { + ...state, + preferredDeviceId: action.deviceId, + selectedDeviceId: chosenDevice, }; } else { - // No preferred device, so pick a default. - const selected = this.chooseDefaultDeviceId(available); - return selected - ? { - id: selected, - virtualEarpiece: false /** This is an iOS thing, always false for android*/, - } - : undefined; + return state; } - }), - ) - .pipe( - tap((selected) => { - this.logger.debug(`selected device: ${selected?.id}`); - }), + }, initialState), ), - ); + ); + } + + private effectiveSelectionFromState$( + state$: Observable, + ): Behavior { + return this.scope.behavior( + state$ + .pipe( + map((state) => { + if (state.selectedDeviceId) { + return { + id: state.selectedDeviceId, + /** This is an iOS thing, always false for android*/ + virtualEarpiece: false, + }; + } + return undefined; + }), + distinctUntilChanged((a, b) => a?.id === b?.id), + ) + .pipe( + tap((selected) => { + this.logger.debug(`selected device: ${selected?.id}`); + }), + ), + ); + } + + private chooseEffectiveSelection(args: { + previousDevices: OutputDevice[]; + availableDevices: OutputDevice[]; + currentSelectedId: string | undefined; + preferredDeviceId: string | undefined; + }): string | undefined { + const { + previousDevices, + availableDevices, + currentSelectedId, + preferredDeviceId, + } = args; + + this.logger.debug(`chooseEffectiveSelection with args:`, args); + + // Take preferredDeviceId in priority or default to the last effective selection. + const activeSelectedDeviceId = preferredDeviceId || currentSelectedId; + const isAvailable = availableDevices.some( + (device) => device.id === activeSelectedDeviceId, + ); + + // If there is no current device, or it is not available anymore, + // choose the default device selection logic. + if (activeSelectedDeviceId === undefined || !isAvailable) { + this.logger.debug( + `No current device or it is not available, using default selection logic.`, + ); + // use the default selection logic + return this.chooseDefaultDeviceId(availableDevices); + } + + // Is there a new added device? + // If a device is added, we might want to switch to it if it's more preferred than the currently selected device. + const newDeviceWasAdded = availableDevices.some( + (device) => !previousDevices.some((d) => d.id === device.id), + ); + + if (newDeviceWasAdded) { + // TODO only want to check from the added device, not all devices.? + // check if the currently selected device is the most preferred one, if not switch to the most preferred one. + const mostPreferredDevice = availableDevices[0]; + this.logger.debug( + `A new device was added, checking if we should switch to it.`, + mostPreferredDevice, + ); + if (mostPreferredDevice.id !== activeSelectedDeviceId) { + // Given this is automatic switching, we want to be careful and only switch to a more private device + // (e.g. from speaker to a BT headset) but not switch from a more private device to a less private one + // (e.g. from a BT headset to the speaker), as that can be disruptive for the user if it happens unexpectedly. + if (mostPreferredDevice.isExternalHeadset == true) { + this.logger.info( + `The currently selected device ${mostPreferredDevice.id} is not the most preferred one, switching to the most preferred one ${activeSelectedDeviceId} instead.`, + ); + // Let's switch as it is a more private device. + return mostPreferredDevice.id; + } + } + } + + // no changes + return activeSelectedDeviceId; + } /** * The logic for the default is different based on the call type. @@ -206,40 +309,41 @@ export class AndroidControlledAudioOutput implements MediaDevice< * @param available the available audio output devices to choose from, keyed by their id, sorted by likelihood of it being used for communication. * */ - private chooseDefaultDeviceId( - available: Map, - ): string | undefined { + private chooseDefaultDeviceId(available: OutputDevice[]): string | undefined { this.logger.debug( `Android routing logic intent: ${this.initialIntent} finding best default...`, ); if (this.initialIntent === "audio") { - const systemProposed = available.keys().next().value; + const systemProposed = available[0]; // If no headset is connected, android will route to the speaker by default, // but for a voice call we want to route to the earpiece instead, // so override the system proposed routing in that case. - if (systemProposed && available.get(systemProposed)?.type === "speaker") { - // search for the headset - const headsetEntry = Array.from(available.entries()).find( - ([_, label]) => label.type === "earpiece", + if (systemProposed?.isSpeaker == true) { + // search for the earpiece + const earpieceDevice = available.find( + (device) => device.isEarpiece == true, ); - if (headsetEntry) { + if (earpieceDevice) { this.logger.debug( `Android routing: Switch to earpiece instead of speaker for voice call`, ); - return headsetEntry[0]; + return earpieceDevice.id; } else { this.logger.debug( `Android routing: no earpiece found, cannot switch, use system proposed routing`, ); - return systemProposed; + return systemProposed.id; } } else { - this.logger.debug(`Android routing: Use system proposed routing`); - return systemProposed; + this.logger.debug( + `Android routing: Use system proposed routing `, + systemProposed, + ); + return systemProposed?.id; } } else { // Use the system best proposed best routing. - return available.keys().next().value; + return available[0]?.id; } } }