Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 91868c52 authored by En-Shuo Hsu's avatar En-Shuo Hsu
Browse files

floss: Fix AvrcpDeviceConnected race

We wait for a period of time for both A2DP and HFP's capability to be
updated before sending the device added event. If AvrcpDeviceConnected
is triggered while we are waiting, we'll trigger the absolute volume
support change event to audio stack. However, audio stack will regard
it as an abnormal event as there is no device connected yet from its
point of view.

The pending device added event triggered after the volume support event
won't help as the absolute_volume value in the device added event is
out-of-sync and wrong.

Fix the race by canceling the pending added device event and add a new
one with the correct absolute_volume information to solve the race and
just don't trigger the absolute volume support change event when the
device is not added.

Bug: 243083916
Tag: #floss
Test: Build and verify volume control works
BYPASS_LONG_LINES_REASON: Bluetooth likes 120 char lines

Change-Id: Ib76651a77fc3116c2d5b90dbabffe98dfb27424e
parent c2d8341a
Loading
Loading
Loading
Loading
+35 −6
Original line number Diff line number Diff line
@@ -29,7 +29,10 @@ use crate::bluetooth::{Bluetooth, BluetoothDevice, IBluetooth};
use crate::callbacks::Callbacks;
use crate::{Message, RPCProxy};

const DEFAULT_PROFILE_DISCOVERY_TIMEOUT_SEC: u64 = 5;
// The timeout we have to wait for all supported profiles to connect after we
// receive the first profile connected event. In the worst scenario, we'll have
// 2 * PROFILE_DISCOVERY_TIMEOUT_SEC of waiting time.
const PROFILE_DISCOVERY_TIMEOUT_SEC: u64 = 5;

pub trait IBluetoothMedia {
    ///
@@ -214,11 +217,37 @@ impl BluetoothMedia {

    pub fn dispatch_avrcp_callbacks(&mut self, cb: AvrcpCallbacks) {
        match cb {
            AvrcpCallbacks::AvrcpDeviceConnected(_addr, supported) => {
            AvrcpCallbacks::AvrcpDeviceConnected(addr, supported) => {
                if self.absolute_volume == supported {
                    return;
                }

                self.absolute_volume = supported;
                self.callbacks.lock().unwrap().for_all_callbacks(|callback| {
                let mut guard = self.device_added_tasks.lock().unwrap();
                if let Some(task) = guard.get(&addr) {
                    match task {
                        // There is a device added event waiting for other
                        // profiles (A2DP or HFP) to connect. We need to cancel
                        // the pending event to update the absolute volume
                        // capability.
                        // This refreshes the timeout waiting for potential
                        // profile connection and makes the worst case total
                        // waiting time to 2 * PROFILE_DISCOVERY_TIMEOUT_SEC.
                        Some(handler) => {
                            handler.abort();
                            guard.remove(&addr);
                            drop(guard);
                            self.notify_media_capability_added(addr);
                        }
                        // This addr has been added so trigger the absolute
                        // volume supported changed callback.
                        None => self.callbacks.lock().unwrap().for_all_callbacks(|callback| {
                            callback.on_absolute_volume_supported_changed(supported);
                });
                        }),
                    }
                } else {
                    info!("[{}]: Device's avrcp connected before a2dp and hfp", addr.to_string());
                }
            }
            AvrcpCallbacks::AvrcpDeviceDisconnected(_addr) => {}
            AvrcpCallbacks::AvrcpAbsoluteVolumeUpdate(volume) => {
@@ -409,7 +438,7 @@ impl BluetoothMedia {
                        absolute_volume,
                    );
                    let task = topstack::get_runtime().spawn(async move {
                        sleep(Duration::from_secs(DEFAULT_PROFILE_DISCOVERY_TIMEOUT_SEC)).await;
                        sleep(Duration::from_secs(PROFILE_DISCOVERY_TIMEOUT_SEC)).await;
                        if dedup_added_cb(device_added_tasks, addr, callbacks, device, true) {
                            warn!(
                                "[{}]: Add a device with only hfp or a2dp capability after timeout.",