diff --git a/system/gd/rust/linux/stack/src/bluetooth_media.rs b/system/gd/rust/linux/stack/src/bluetooth_media.rs index e5abf5c5d89274beb4951a8146fed51847509d04..bcebb0f9062b169a4a6e8e25d1660d1588914400 100644 --- a/system/gd/rust/linux/stack/src/bluetooth_media.rs +++ b/system/gd/rust/linux/stack/src/bluetooth_media.rs @@ -40,12 +40,14 @@ use crate::uuid::Profile; use crate::{Message, RPCProxy}; // The timeout we have to wait for all supported profiles to connect after we -// receive the first profile connected event. -const PROFILE_DISCOVERY_TIMEOUT_SEC: u64 = 5; +// receive the first profile connected event. The host shall disconnect the +// device after this many seconds of timeout. +const PROFILE_DISCOVERY_TIMEOUT_SEC: u64 = 10; // The timeout we have to wait for the initiator peer device to complete the // initial profile connection. After this many seconds, we will begin to // connect the missing profiles. -const ACCEPTOR_CONNECT_MISSING_PROFILES_TIMEOUT_SEC: u64 = 2; +// 6s is set to align with Android's default. See "btservice/PhonePolicy". +const CONNECT_MISSING_PROFILES_TIMEOUT_SEC: u64 = 6; pub trait IBluetoothMedia { /// @@ -166,6 +168,14 @@ pub enum MediaActions { Disconnect(String), } +#[derive(Debug, Clone, PartialEq)] +enum DeviceConnectionStates { + ConnectingBeforeRetry, // Some profile is connected, initiated from either side + ConnectingAfterRetry, // Host initiated requests to missing profiles after timeout + FullyConnected, // All profiles (excluding AVRCP) are connected + Disconnecting, // Working towards disconnection of each connected profile +} + pub struct BluetoothMedia { intf: Arc<Mutex<BluetoothInterface>>, battery_provider_manager: Arc<Mutex<Box<BatteryProviderManager>>>, @@ -184,12 +194,12 @@ pub struct BluetoothMedia { hfp_audio_state: HashMap<RawAddress, BthfAudioState>, a2dp_caps: HashMap<RawAddress, Vec<A2dpCodecConfig>>, hfp_cap: HashMap<RawAddress, HfpCodecCapability>, - device_added_tasks: Arc<Mutex<HashMap<RawAddress, Option<(JoinHandle<()>, Instant)>>>>, + fallback_tasks: Arc<Mutex<HashMap<RawAddress, Option<(JoinHandle<()>, Instant)>>>>, absolute_volume: bool, uinput: UInput, delay_enable_profiles: HashSet<uuid::Profile>, connected_profiles: HashMap<RawAddress, HashSet<uuid::Profile>>, - disconnecting_devices: HashSet<RawAddress>, + device_states: Arc<Mutex<HashMap<RawAddress, DeviceConnectionStates>>>, } impl BluetoothMedia { @@ -223,12 +233,12 @@ impl BluetoothMedia { hfp_audio_state: HashMap::new(), a2dp_caps: HashMap::new(), hfp_cap: HashMap::new(), - device_added_tasks: Arc::new(Mutex::new(HashMap::new())), + fallback_tasks: Arc::new(Mutex::new(HashMap::new())), absolute_volume: false, uinput: UInput::new(), delay_enable_profiles: HashSet::new(), connected_profiles: HashMap::new(), - disconnecting_devices: HashSet::new(), + device_states: Arc::new(Mutex::new(HashMap::new())), } } @@ -372,6 +382,7 @@ impl BluetoothMedia { self.a2dp_caps.remove(&addr); self.a2dp_audio_state.remove(&addr); self.rm_connected_profile(addr, uuid::Profile::A2dpSink, true); + self.disconnect(addr.to_string()); } _ => { self.a2dp_states.insert(addr, state); @@ -406,7 +417,7 @@ impl BluetoothMedia { // Notify change via callback if device is added. if self.absolute_volume != supported { - let guard = self.device_added_tasks.lock().unwrap(); + let guard = self.fallback_tasks.lock().unwrap(); if let Some(task) = guard.get(&addr) { if task.is_none() { self.callbacks.lock().unwrap().for_all_callbacks(|callback| { @@ -536,6 +547,7 @@ impl BluetoothMedia { self.hfp_cap.remove(&addr); self.hfp_audio_state.remove(&addr); self.rm_connected_profile(addr, uuid::Profile::Hfp, true); + self.disconnect(addr.to_string()); } BthfConnectionState::Connecting => { info!("[{}]: hfp connecting.", addr.to_string()); @@ -614,11 +626,19 @@ impl BluetoothMedia { } fn notify_critical_profile_disconnected(&mut self, addr: RawAddress) { - if self.disconnecting_devices.insert(addr) { - let mut guard = self.device_added_tasks.lock().unwrap(); + info!( + "[{}]: Device connection state: {:?}.", + addr.to_string(), + DeviceConnectionStates::Disconnecting + ); + + let mut states = self.device_states.lock().unwrap(); + let prev_state = states.insert(addr, DeviceConnectionStates::Disconnecting).unwrap(); + if prev_state != DeviceConnectionStates::Disconnecting { + let mut guard = self.fallback_tasks.lock().unwrap(); if let Some(task) = guard.get(&addr) { match task { - // Abort pending task if it hasn't been notified. + // Abort pending task if there is any. Some((handler, _ts)) => { warn!( "[{}]: Device disconnected a critical profile before it was added.", @@ -643,97 +663,34 @@ impl BluetoothMedia { } fn notify_media_capability_updated(&mut self, addr: RawAddress) { - fn device_added_cb( - device_added_tasks: Arc<Mutex<HashMap<RawAddress, Option<(JoinHandle<()>, Instant)>>>>, - addr: RawAddress, - callbacks: Arc<Mutex<Callbacks<dyn IBluetoothMediaCallback + Send>>>, - device: BluetoothAudioDevice, - missing_profiles: HashSet<uuid::Profile>, - ) { - // Once it gets here, either it will win the lock and run the task - // or be aborted and potentially get replaced. - let mut guard = device_added_tasks.lock().unwrap(); - guard.insert(addr, None); - - if !missing_profiles.is_empty() { - warn!( - "Notify media capability added with missing profiles: {:?}", - missing_profiles - ); - } - - callbacks.lock().unwrap().for_all_callbacks(|callback| { - callback.on_bluetooth_audio_device_added(device.clone()); - }); - } - - let cur_a2dp_caps = self.a2dp_caps.get(&addr); - let cur_hfp_cap = self.hfp_cap.get(&addr); - let name = self.adapter_get_remote_name(addr); - let absolute_volume = self.absolute_volume; - let device = BluetoothAudioDevice::new( - addr.to_string(), - name.clone(), - cur_a2dp_caps.unwrap_or(&Vec::new()).to_vec(), - *cur_hfp_cap.unwrap_or(&HfpCodecCapability::UNSUPPORTED), - absolute_volume, - ); - - let mut guard = self.device_added_tasks.lock().unwrap(); + let mut guard = self.fallback_tasks.lock().unwrap(); + let mut states = self.device_states.lock().unwrap(); let mut first_conn_ts = Instant::now(); let is_profile_cleared = self.connected_profiles.get(&addr).unwrap().is_empty(); - if is_profile_cleared { - self.connected_profiles.remove(&addr); - self.disconnecting_devices.remove(&addr); - } - match guard.get(&addr) { - Some(task) => match task { - // There is a handler that hasn't fired. - // Abort the task and later replace it if the device isn't disconnecting. - Some((handler, ts)) => { - handler.abort(); - first_conn_ts = *ts; - if is_profile_cleared { - warn!( - "[{}]: Device disconnected all profiles before it was added.", - addr.to_string() - ); - guard.remove(&addr); - return; - } else { - guard.insert(addr, None); - } - } - // The handler was fired or aborted (due to critical profile disconnection). - // Ignore if it's a late "insert" event. - // Also ignore if it's a "remove" event unless all have been removed. - None => { - if is_profile_cleared { - info!("[{}]: Device disconnected all profiles.", addr.to_string()); - guard.remove(&addr); - } - return; - } - }, - // First update since the last moment with no connection. - // Note it's possible that a device (e.g., Motorola S10) requests - // disconnection at start (i.e., when nothing is connected). - None => { - if is_profile_cleared { - warn!( - "[{}]: Trying to remove capability of an unknown device.", - addr.to_string() - ); + if let Some(task) = guard.get(&addr) { + if let Some((handler, ts)) = task { + // Abort the pending task. It may be updated or + // removed depending on whether all profiles are cleared. + handler.abort(); + first_conn_ts = *ts; + guard.insert(addr, None); + } else { + // The device is already added or is disconnecting. + // Ignore unless all profiles are cleared. + if !is_profile_cleared { return; } } } - // If the device has disconnected a critical profile, wait until all - // profiles have disconnected and refrain from adding the task. - if self.disconnecting_devices.contains(&addr) { + // Cleanup if transitioning to empty set. + if is_profile_cleared { + info!("[{}]: Device connection state: Disconnected.", addr.to_string()); + self.connected_profiles.remove(&addr); + states.remove(&addr); + guard.remove(&addr); return; } @@ -742,30 +699,118 @@ impl BluetoothMedia { let missing_profiles = available_profiles.difference(&connected_profiles).cloned().collect::<HashSet<_>>(); - let callbacks = self.callbacks.clone(); - let device_added_tasks = self.device_added_tasks.clone(); - let txl = self.tx.clone(); - let task = topstack::get_runtime().spawn(async move { - if !missing_profiles.is_empty() { - // When the headset initiates profile connection, it will not share the same - // path as that of the other way around, and may selectively connect to - // certain profiles while missing out others. - // Therefore here we want to connect the missing profiles. However, we must not do - // so immediately, since by convention the initiating device should be given chance - // to connect the profiles first. Here we yield for a couple of seconds before - // attempting to connect the missing profiles. - sleep(Duration::from_secs(ACCEPTOR_CONNECT_MISSING_PROFILES_TIMEOUT_SEC)).await; - let _ = txl.send(Message::Media(MediaActions::Connect(addr.to_string()))).await; - - let now_ts = Instant::now(); - let total_wait_duration = Duration::from_secs(PROFILE_DISCOVERY_TIMEOUT_SEC); - let remaining_wait_duration = - (first_conn_ts + total_wait_duration).saturating_duration_since(now_ts); - sleep(remaining_wait_duration).await; + // Update device states + if states.get(&addr).is_none() { + states.insert(addr, DeviceConnectionStates::ConnectingBeforeRetry); + } + if missing_profiles.is_empty() + || missing_profiles == HashSet::from([Profile::AvrcpController]) + { + states.insert(addr, DeviceConnectionStates::FullyConnected); + } + + info!("[{}]: Device connection state: {:?}.", addr.to_string(), states.get(&addr).unwrap()); + + // React on updated device states + match states.get(&addr).unwrap() { + DeviceConnectionStates::ConnectingBeforeRetry => { + let fallback_tasks = self.fallback_tasks.clone(); + let device_states = self.device_states.clone(); + let txl = self.tx.clone(); + let task = topstack::get_runtime().spawn(async move { + let now_ts = Instant::now(); + let total_duration = Duration::from_secs(CONNECT_MISSING_PROFILES_TIMEOUT_SEC); + let sleep_duration = + (first_conn_ts + total_duration).saturating_duration_since(now_ts); + sleep(sleep_duration).await; + + { + let mut states = device_states.lock().unwrap(); + states.insert(addr, DeviceConnectionStates::ConnectingAfterRetry); + } + + info!( + "[{}]: Device connection state: {:?}.", + addr.to_string(), + DeviceConnectionStates::ConnectingAfterRetry + ); + + let _ = txl.send(Message::Media(MediaActions::Connect(addr.to_string()))).await; + + let now_ts = Instant::now(); + let total_duration = Duration::from_secs(PROFILE_DISCOVERY_TIMEOUT_SEC); + let sleep_duration = + (first_conn_ts + total_duration).saturating_duration_since(now_ts); + sleep(sleep_duration).await; + + { + let mut states = device_states.lock().unwrap(); + let mut guard = fallback_tasks.lock().unwrap(); + states.insert(addr, DeviceConnectionStates::Disconnecting); + guard.insert(addr, None); + } + + info!( + "[{}]: Device connection state: {:?}.", + addr.to_string(), + DeviceConnectionStates::Disconnecting + ); + + let _ = + txl.send(Message::Media(MediaActions::Disconnect(addr.to_string()))).await; + }); + guard.insert(addr, Some((task, first_conn_ts))); + } + DeviceConnectionStates::ConnectingAfterRetry => { + let fallback_tasks = self.fallback_tasks.clone(); + let device_states = self.device_states.clone(); + let txl = self.tx.clone(); + let task = topstack::get_runtime().spawn(async move { + let now_ts = Instant::now(); + let total_duration = Duration::from_secs(PROFILE_DISCOVERY_TIMEOUT_SEC); + let sleep_duration = + (first_conn_ts + total_duration).saturating_duration_since(now_ts); + sleep(sleep_duration).await; + + { + let mut states = device_states.lock().unwrap(); + let mut guard = fallback_tasks.lock().unwrap(); + states.insert(addr, DeviceConnectionStates::Disconnecting); + guard.insert(addr, None); + } + + info!( + "[{}]: Device connection state: {:?}.", + addr.to_string(), + DeviceConnectionStates::Disconnecting + ); + + let _ = + txl.send(Message::Media(MediaActions::Disconnect(addr.to_string()))).await; + }); + guard.insert(addr, Some((task, first_conn_ts))); + } + DeviceConnectionStates::FullyConnected => { + let cur_a2dp_caps = self.a2dp_caps.get(&addr); + let cur_hfp_cap = self.hfp_cap.get(&addr); + let name = self.adapter_get_remote_name(addr); + let absolute_volume = self.absolute_volume; + let device = BluetoothAudioDevice::new( + addr.to_string(), + name.clone(), + cur_a2dp_caps.unwrap_or(&Vec::new()).to_vec(), + *cur_hfp_cap.unwrap_or(&HfpCodecCapability::UNSUPPORTED), + absolute_volume, + ); + + self.callbacks.lock().unwrap().for_all_callbacks(|callback| { + callback.on_bluetooth_audio_device_added(device.clone()); + }); + + guard.insert(addr, None); } - device_added_cb(device_added_tasks, addr, callbacks, device, missing_profiles); - }); - guard.insert(addr, Some((task, first_conn_ts))); + DeviceConnectionStates::Disconnecting => {} + } } fn adapter_get_remote_name(&self, addr: RawAddress) -> String { @@ -963,7 +1008,7 @@ impl IBluetoothMedia for BluetoothMedia { let missing_profiles = available_profiles.difference(&connected_profiles).collect::<HashSet<_>>(); - for profile in missing_profiles { + for profile in &missing_profiles { match profile { uuid::Profile::A2dpSink => { metrics::profile_connection_state_changed( @@ -1026,6 +1071,13 @@ impl IBluetoothMedia for BluetoothMedia { }; } uuid::Profile::AvrcpController => { + // Fluoride will resolve AVRCP as a part of A2DP connection request. + // Explicitly connect to it only when it is considered missing, and don't + // bother about it when A2DP is not connected. + if missing_profiles.contains(&Profile::A2dpSink) { + continue; + } + metrics::profile_connection_state_changed( addr, Profile::AvrcpController as u32, @@ -1068,6 +1120,9 @@ impl IBluetoothMedia for BluetoothMedia { true } + // TODO(b/263808543): Currently this is designed to be called from both the + // UI and via disconnection callbacks. Remove this workaround once the + // proper fix has landed. fn disconnect(&mut self, address: String) { let addr = match RawAddress::from_string(address.clone()) { None => { @@ -1091,6 +1146,11 @@ impl IBluetoothMedia for BluetoothMedia { for profile in connected_profiles { match profile { uuid::Profile::A2dpSink => { + // Some headsets (b/263808543) will try reconnecting to A2DP + // when HFP is running but (requested to be) disconnected. + if connected_profiles.contains(&Profile::Hfp) { + continue; + } metrics::profile_connection_state_changed( addr, Profile::A2dpSink as u32, @@ -1151,6 +1211,9 @@ impl IBluetoothMedia for BluetoothMedia { }; } uuid::Profile::AvrcpController => { + if connected_profiles.contains(&Profile::A2dpSink) { + continue; + } metrics::profile_connection_state_changed( addr, Profile::AvrcpController as u32,