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

Commit 84a18be4 authored by Jesse Melhuish's avatar Jesse Melhuish Committed by Hsin-chen Chuang
Browse files

floss: Refactor BAS connection/disconnection

Floss connects to BAS automatically, this means in some cases Floss
needs to take responsibility to disconnect it:
- A user calls ClientConnect and then expects ClientDisconnect to drop
  the entire connection.
- A peer initiates some profile connections, then Floss connects to
  BAS. Since Floss is the initiator, the peer might not be disconnecting
  BAS from their end.

This patch does:
- Revise Message::OnDeviceConnectionStateChanged: It was not doing what
  its name told and was only fired in some specific cases
- Add Message::ProfileDisconnected: Disconnect BAS when it's the last
  active profile
- Remove BAS logic in ConnectAllEnabledProfiles as we would connect BAS
  on ACL/bond state changed
- Remove BAS logic in DisconnectAllEnabledProfiles as it already
  disconnects all GATT
- Cleanup BAS on GATT disconnected
- We would connect BAS to dual device after this patch

Bug: 369733365
Tag: #floss
Test: mmm packages/modules/Bluetooth
Test: Connect Sephero mini through app, then bond through btclient; Disconnecting through app would drop the entire connection after this patch
Test: LR10 (dual device), BAS connects and doesn't affect FastPair
Test: LE mouse, BAS works fine
Test: bluetooth_Adapter{CL,LE,AU}Health.all_floss
Test: CTSV Bluetooth LE Client Test
Test: SmartLock and FastPair works fine
Flag: EXEMPT, Floss-only change

Change-Id: I6339e7d649871142c7c55dc2e1a7f3f53d245c18
parent f4bac114
Loading
Loading
Loading
Loading
+26 −30
Original line number Diff line number Diff line
@@ -2,7 +2,6 @@ use crate::battery_manager::{Battery, BatterySet};
use crate::battery_provider_manager::{
    BatteryProviderManager, IBatteryProviderCallback, IBatteryProviderManager,
};
use crate::bluetooth::BluetoothDevice;
use crate::bluetooth_gatt::{
    BluetoothGatt, BluetoothGattService, IBluetoothGatt, IBluetoothGattCallback,
};
@@ -10,9 +9,9 @@ use crate::callbacks::Callbacks;
use crate::Message;
use crate::RPCProxy;
use crate::{uuid, APIMessage, BluetoothAPI};
use bt_topshim::btif::{BtAclState, BtBondState, BtTransport, DisplayAddress, RawAddress, Uuid};
use bt_topshim::btif::{BtTransport, DisplayAddress, RawAddress, Uuid};
use bt_topshim::profiles::gatt::{GattStatus, LePhy};
use log::debug;
use log::{debug, info};
use std::collections::HashMap;
use std::convert::TryInto;
use std::iter;
@@ -23,6 +22,10 @@ use tokio::sync::mpsc::Sender;
/// specification.
pub const CHARACTERISTIC_BATTERY_LEVEL: &str = "00002A1-9000-0100-0800-000805F9B34FB";

/// The app UUID BAS provides when connecting as a GATT client. Chosen at random.
/// TODO(b/233101174): make dynamic or decide on a static UUID
pub const BATTERY_SERVICE_GATT_CLIENT_APP_ID: &str = "e4d2acffcfaa42198f494606b7412117";

/// Represents the Floss BatteryService implementation.
pub struct BatteryService {
    gatt: Arc<Mutex<Box<BluetoothGatt>>>,
@@ -55,10 +58,6 @@ pub enum BatteryServiceActions {
    OnCharacteristicRead(RawAddress, GattStatus, i32, Vec<u8>),
    /// Params: addr, handle, value
    OnNotify(RawAddress, i32, Vec<u8>),
    /// Params: remote_device, transport
    Connect(BluetoothDevice, BtAclState, BtBondState, BtTransport),
    /// Params: remote_device
    Disconnect(BluetoothDevice),
}

/// API for Floss implementation of the Bluetooth Battery Service (BAS). BAS is built on GATT and
@@ -127,8 +126,7 @@ impl BatteryService {
    pub fn init(&self) {
        debug!("Registering GATT client for BatteryService");
        self.gatt.lock().unwrap().register_client(
            // TODO(b/233101174): make dynamic or decide on a static UUID
            String::from("e4d2acffcfaa42198f494606b7412117"),
            String::from(BATTERY_SERVICE_GATT_CLIENT_APP_ID),
            Box::new(GattCallback::new(self.tx.clone(), self.api_tx.clone())),
            false,
        );
@@ -144,6 +142,13 @@ impl BatteryService {

            BatteryServiceActions::OnClientConnectionState(status, _client_id, connected, addr) => {
                if !connected || status != GattStatus::Success {
                    info!(
                        "BAS: Dropping {} due to GATT state changed: connected={}, status={:?}",
                        DisplayAddress(&addr),
                        connected,
                        status
                    );
                    self.drop_device(addr);
                    return;
                }
                let client_id = match self.client_id {
@@ -157,8 +162,8 @@ impl BatteryService {

            BatteryServiceActions::OnSearchComplete(addr, services, status) => {
                if status != GattStatus::Success {
                    debug!(
                        "GATT service discovery for {} failed with status {:?}",
                    info!(
                        "BAS: Service discovery for {} failed: status={:?}",
                        DisplayAddress(&addr),
                        status
                    );
@@ -176,10 +181,16 @@ impl BatteryService {
                                )
                            });
                        }
                        info!(
                            "BAS: Failed to get handle from {}: status={:?}",
                            DisplayAddress(&addr),
                            status
                        );
                        self.drop_device(addr);
                        return;
                    }
                };
                info!("BAS: Found handle from {}", DisplayAddress(&addr));
                let client_id = match self.client_id {
                    Some(id) => id,
                    None => {
@@ -221,21 +232,6 @@ impl BatteryService {
                    callback.on_battery_info_updated(addr, battery_info.clone());
                });
            }

            BatteryServiceActions::Connect(device, acl_state, bond_state, transport) => {
                if transport != BtTransport::Le
                    || acl_state != BtAclState::Connected
                    || bond_state != BtBondState::Bonded
                {
                    return;
                }

                self.init_device(device.address, transport);
            }

            BatteryServiceActions::Disconnect(device) => {
                self.drop_device(device.address);
            }
        }
    }

@@ -258,23 +254,22 @@ impl BatteryService {
        battery_set.clone()
    }

    fn init_device(&self, remote_address: RawAddress, transport: BtTransport) {
    pub(crate) fn init_device(&self, remote_address: RawAddress) {
        let client_id = match self.client_id {
            Some(id) => id,
            None => return,
        };
        debug!("Attempting GATT connection to {}", DisplayAddress(&remote_address));
        self.gatt.lock().unwrap().client_connect(
            client_id,
            remote_address,
            false,
            transport,
            BtTransport::Le,
            false,
            LePhy::Phy1m,
        );
    }

    fn drop_device(&mut self, remote_address: RawAddress) {
    pub(crate) fn drop_device(&mut self, remote_address: RawAddress) {
        if self.handles.contains_key(&remote_address) {
            // Let BatteryProviderManager know that BAS no longer has a battery for this device.
            self.battery_provider_manager.lock().unwrap().remove_battery_info(
@@ -354,6 +349,7 @@ impl BatteryService {
}

/// Status enum for relaying the state of BAS or a particular device.
#[derive(Debug)]
pub enum BatteryServiceStatus {
    /// Device does not report support for BAS.
    BatteryServiceNotSupported,
+51 −88
Original line number Diff line number Diff line
@@ -43,7 +43,6 @@ use tokio::sync::mpsc::Sender;
use tokio::task::JoinHandle;
use tokio::time;

use crate::battery_service::BatteryServiceActions;
use crate::bluetooth_admin::BluetoothAdminPolicyHelper;
use crate::bluetooth_gatt::{
    BluetoothGatt, GattActions, IBluetoothGatt, IScannerCallback, ScanResult,
@@ -375,6 +374,7 @@ struct BluetoothDeviceContext {
    pub info: BluetoothDevice,
    pub last_seen: Instant,
    pub properties: HashMap<BtPropertyType, BluetoothProperty>,
    pub is_hh_connected: bool,

    /// If user wants to connect to all profiles, when new profiles are discovered we will also try
    /// to connect them.
@@ -398,6 +398,7 @@ impl BluetoothDeviceContext {
            info,
            last_seen,
            properties: HashMap::new(),
            is_hh_connected: false,
            connect_to_new_profiles: false,
        };
        device.update_properties(&properties);
@@ -1296,6 +1297,10 @@ impl Bluetooth {
            || self.pending_create_bond.is_some()
    }

    pub fn is_hh_connected(&self, device_address: &RawAddress) -> bool {
        self.remote_devices.get(&device_address).map_or(false, |context| context.is_hh_connected)
    }

    /// Checks whether the list of device properties contains some UUID we should connect now
    /// This function also connects those UUIDs.
    fn check_new_property_and_potentially_connect_profiles(
@@ -1417,37 +1422,6 @@ impl Bluetooth {
                                });
                            }

                            Profile::Bas => {
                                has_supported_profile = true;
                                let tx = self.tx.clone();
                                let device_context = match self.remote_devices.get(&addr) {
                                    Some(context) => context,
                                    None => continue,
                                };

                                let acl_state = device_context.ble_acl_state.clone();
                                let bond_state = device_context.bond_state.clone();
                                let device_to_send = device.clone();

                                let transport = match self.get_remote_type(device.clone()) {
                                    BtDeviceType::Bredr => BtTransport::Bredr,
                                    BtDeviceType::Ble => BtTransport::Le,
                                    _ => device_context.acl_reported_transport.clone(),
                                };
                                topstack::get_runtime().spawn(async move {
                                    let _ = tx
                                        .send(Message::BatteryService(
                                            BatteryServiceActions::Connect(
                                                device_to_send,
                                                acl_state,
                                                bond_state,
                                                transport,
                                            ),
                                        ))
                                        .await;
                                });
                            }

                            // We don't connect most profiles
                            _ => (),
                        }
@@ -1464,6 +1438,31 @@ impl Bluetooth {
            self.resume_discovery();
        }
    }

    fn fire_device_connection_or_bonded_state_changed(&self, addr: RawAddress) {
        if let Some(device) = self.remote_devices.get(&addr) {
            let tx = self.tx.clone();
            let bredr_acl_state = device.bredr_acl_state.clone();
            let ble_acl_state = device.ble_acl_state.clone();
            let bond_state = device.bond_state.clone();
            let transport = match self.get_remote_type(device.info.clone()) {
                BtDeviceType::Bredr => BtTransport::Bredr,
                BtDeviceType::Ble => BtTransport::Le,
                _ => device.acl_reported_transport.clone(),
            };
            tokio::spawn(async move {
                let _ = tx
                    .send(Message::OnDeviceConnectionOrBondStateChanged(
                        addr,
                        bredr_acl_state,
                        ble_acl_state,
                        bond_state,
                        transport,
                    ))
                    .await;
            });
        }
    }
}

#[btif_callbacks_dispatcher(dispatch_base_callbacks, BaseCallbacks)]
@@ -1933,40 +1932,22 @@ impl BtifBluetoothCallbacks for Bluetooth {
                        Instant::now(),
                        vec![],
                    ));
                    let acl_reported_transport = device.acl_reported_transport.clone();
                    let acl_state = device.ble_acl_state.clone();
                    let device_info = device.info.clone();

                    // Since this is a newly bonded device, we also need to trigger SDP on it.
                    self.fetch_remote_uuids(device_info.clone());
                    self.fetch_remote_uuids(device_info);
                    if self.get_wake_allowed_device_bonded() {
                        self.create_uhid_for_suspend_wakesource();
                    }
                    // Update the connectable mode since bonded list is changed.
                    self.update_connectable_mode();

                    let transport = match self.get_remote_type(device_info.clone()) {
                        BtDeviceType::Bredr => BtTransport::Bredr,
                        BtDeviceType::Ble => BtTransport::Le,
                        _ => acl_reported_transport,
                    };

                    let tx = self.tx.clone();
                    tokio::spawn(async move {
                        let _ = tx
                            .send(Message::OnDeviceConnectionStateChanged(
                                device_info.clone(),
                                acl_state,
                                BtBondState::Bonded,
                                transport,
                            ))
                            .await;
                    });
                }
                BtBondState::Bonding => {}
            }
        }

        // Modification to |self.remote_devices| has done, ok to fire the change event.
        self.fire_device_connection_or_bonded_state_changed(addr);

        // Resume discovery once the bonding process is complete. Discovery was paused before the
        // bond request to avoid ACL connection from interfering with active inquiry.
        if bond_state == BtBondState::NotBonded || bond_state == BtBondState::Bonded {
@@ -2073,7 +2054,6 @@ impl BtifBluetoothCallbacks for Bluetooth {
            return;
        }

        let txl = self.tx.clone();
        let device = self.remote_devices.entry(addr).or_insert(BluetoothDeviceContext::new(
            BtBondState::NotBonded,
            BtAclState::Disconnected,
@@ -2090,7 +2070,6 @@ impl BtifBluetoothCallbacks for Bluetooth {

        let info = device.info.clone();
        device.acl_reported_transport = link_type;
        let bond_state = device.bond_state.clone();

        metrics::acl_connection_state_changed(
            addr,
@@ -2103,26 +2082,10 @@ impl BtifBluetoothCallbacks for Bluetooth {

        match state {
            BtAclState::Connected => {
                let acl_reported_transport = device.acl_reported_transport;
                Bluetooth::send_metrics_remote_device_info(device);
                self.connection_callbacks.for_all_callbacks(|callback| {
                    callback.on_device_connected(info.clone());
                });
                let transport = match self.get_remote_type(info.clone()) {
                    BtDeviceType::Bredr => BtTransport::Bredr,
                    BtDeviceType::Ble => BtTransport::Le,
                    _ => acl_reported_transport,
                };
                tokio::spawn(async move {
                    let _ = txl
                        .send(Message::OnDeviceConnectionStateChanged(
                            info,
                            BtAclState::Connected,
                            bond_state,
                            transport,
                        ))
                        .await;
                });
            }
            BtAclState::Disconnected => {
                if !device.is_connected() {
@@ -2131,11 +2094,12 @@ impl BtifBluetoothCallbacks for Bluetooth {
                    });
                    device.connect_to_new_profiles = false;
                }
                tokio::spawn(async move {
                    let _ = txl.send(Message::OnDeviceDisconnected(info)).await;
                });
            }
        };

        // Modification to |self.remote_devices| has done, ok to fire the change event.
        self.fire_device_connection_or_bonded_state_changed(addr);

        // If we are bonding, skip the update here as we will update it after bonding complete anyway.
        // This is necessary for RTK controllers, which will break RNR after |Write Scan Enable|
        // command. Although this is a bug of RTK controllers, but as we could avoid unwanted page
@@ -2848,18 +2812,6 @@ impl IBluetooth for Bluetooth {
                                });
                            }

                            Profile::Bas => {
                                let tx = self.tx.clone();
                                let device_to_send = device.clone();
                                topstack::get_runtime().spawn(async move {
                                    let _ = tx
                                        .send(Message::BatteryService(
                                            BatteryServiceActions::Disconnect(device_to_send),
                                        ))
                                        .await;
                                });
                            }

                            // We don't connect most profiles
                            _ => (),
                        }
@@ -3010,7 +2962,7 @@ impl BtifSdpCallbacks for Bluetooth {
impl BtifHHCallbacks for Bluetooth {
    fn connection_state(
        &mut self,
        mut address: RawAddress,
        address: RawAddress,
        address_type: BtAddrType,
        transport: BtTransport,
        state: BthhConnectionState,
@@ -3047,6 +2999,16 @@ impl BtifHHCallbacks for Bluetooth {
            state as u32,
        );

        let tx = self.tx.clone();
        self.remote_devices.entry(address).and_modify(|context| {
            if context.is_hh_connected && state != BthhConnectionState::Connected {
                tokio::spawn(async move {
                    let _ = tx.send(Message::ProfileDisconnected(address)).await;
                });
            }
            context.is_hh_connected = state == BthhConnectionState::Connected;
        });

        if BtBondState::Bonded != self.get_bond_state_by_addr(&address)
            && (state != BthhConnectionState::Disconnecting
                && state != BthhConnectionState::Disconnected)
@@ -3057,6 +3019,7 @@ impl BtifHHCallbacks for Bluetooth {
            );
            // TODO(b/329837967): Determine correct reconnection
            // behavior based on device instead of the default
            let mut address = address;
            self.hh.as_ref().unwrap().disconnect(
                &mut address,
                address_type,
+17 −0
Original line number Diff line number Diff line
@@ -173,6 +173,14 @@ impl ContextMap {
            .collect()
    }

    fn get_connected_applications_from_address(&self, address: &RawAddress) -> Vec<Uuid> {
        self.get_client_ids_from_address(address)
            .into_iter()
            .filter_map(|id| self.get_by_client_id(id))
            .map(|client| client.uuid)
            .collect()
    }

    fn get_callback_from_callback_id(
        &mut self,
        callback_id: u32,
@@ -1872,6 +1880,10 @@ impl BluetoothGatt {
    pub fn handle_adv_action(&mut self, action: AdvertiserActions) {
        self.adv_manager.get_impl().handle_action(action);
    }

    pub(crate) fn get_connected_applications(&self, device_address: &RawAddress) -> Vec<Uuid> {
        self.context_map.get_connected_applications_from_address(device_address)
    }
}

#[derive(Debug, FromPrimitive, ToPrimitive)]
@@ -2865,6 +2877,11 @@ impl BtifGattClientCallbacks for BluetoothGatt {
                }
            }
        }

        let tx = self.tx.clone();
        tokio::spawn(async move {
            let _ = tx.send(Message::ProfileDisconnected(addr)).await;
        });
    }

    fn search_complete_cb(&mut self, conn_id: i32, _status: GattStatus) {
+8 −0
Original line number Diff line number Diff line
@@ -609,6 +609,10 @@ impl BluetoothMedia {
        false
    }

    pub(crate) fn get_connected_profiles(&self, device_address: &RawAddress) -> HashSet<Profile> {
        self.connected_profiles.get(device_address).cloned().unwrap_or_default()
    }

    fn add_connected_profile(&mut self, addr: RawAddress, profile: Profile) {
        if self.is_profile_connected(&addr, &profile) {
            warn!("[{}]: profile is already connected", DisplayAddress(&addr));
@@ -2302,6 +2306,10 @@ impl BluetoothMedia {
            self.connected_profiles.remove(&addr);
            states.remove(&addr);
            guard.remove(&addr);
            let tx = self.tx.clone();
            tokio::spawn(async move {
                let _ = tx.send(Message::ProfileDisconnected(addr)).await;
            });
            return;
        }

+49 −24
Original line number Diff line number Diff line
@@ -20,7 +20,7 @@ pub mod suspend;
pub mod uuid;

use bluetooth_qa::{BluetoothQA, IBluetoothQA};
use log::debug;
use log::{debug, info};
use num_derive::{FromPrimitive, ToPrimitive};
use std::sync::{Arc, Mutex};
use tokio::sync::mpsc::channel;
@@ -29,7 +29,9 @@ use tokio::time::{sleep, Duration};

use crate::battery_manager::{BatteryManager, BatterySet};
use crate::battery_provider_manager::BatteryProviderManager;
use crate::battery_service::{BatteryService, BatteryServiceActions};
use crate::battery_service::{
    BatteryService, BatteryServiceActions, BATTERY_SERVICE_GATT_CLIENT_APP_ID,
};
use crate::bluetooth::{
    dispatch_base_callbacks, dispatch_hid_host_callbacks, dispatch_sdp_callbacks, AdapterActions,
    Bluetooth, BluetoothDevice, IBluetooth,
@@ -45,7 +47,7 @@ use crate::dis::{DeviceInformation, ServiceCallbacks};
use crate::socket_manager::{BluetoothSocketManager, SocketActions};
use crate::suspend::Suspend;
use bt_topshim::{
    btif::{BaseCallbacks, BtAclState, BtBondState, BtTransport, RawAddress},
    btif::{BaseCallbacks, BtAclState, BtBondState, BtTransport, DisplayAddress, RawAddress, Uuid},
    profiles::{
        a2dp::A2dpCallbacks,
        avrcp::AvrcpCallbacks,
@@ -109,8 +111,14 @@ pub enum Message {

    // Follows IBluetooth's on_device_(dis)connected and bond_state callbacks
    // but doesn't require depending on Bluetooth.
    OnDeviceConnectionStateChanged(BluetoothDevice, BtAclState, BtBondState, BtTransport),
    OnDeviceDisconnected(BluetoothDevice),
    // Params: Address, BR/EDR ACL state, BLE ACL state, bond state, transport
    OnDeviceConnectionOrBondStateChanged(
        RawAddress,
        BtAclState,
        BtAclState,
        BtBondState,
        BtTransport,
    ),

    // Suspend related
    SuspendCallbackRegistered(u32),
@@ -167,6 +175,11 @@ pub enum Message {
    // UHid callbacks
    UHidHfpOutputCallback(RawAddress, u8, u8),
    UHidTelephonyUseCallback(RawAddress, bool),

    // This message is sent when either HID, media, or GATT client, is disconnected.
    // Note that meida sends this when the profiles are disconnected as a whole, that is, it will
    // not be called when AVRCP is disconnected but not A2DP, as an example.
    ProfileDisconnected(RawAddress),
}

/// Returns a callable object that dispatches a BTIF callback to Message
@@ -432,27 +445,19 @@ impl Stack {
                    bluetooth.lock().unwrap().handle_actions(action);
                }

                // Any service needing an updated list of devices can have an
                // update method triggered from here rather than needing a
                // reference to Bluetooth.
                Message::OnDeviceConnectionStateChanged(
                    device,
                    acl_state,
                // Any service needing an updated list of devices can have an update method
                // triggered from here rather than needing a reference to Bluetooth.
                Message::OnDeviceConnectionOrBondStateChanged(
                    addr,
                    _bredr_acl_state,
                    ble_acl_state,
                    bond_state,
                    transport,
                    _transport,
                ) => {
                    battery_service.lock().unwrap().handle_action(BatteryServiceActions::Connect(
                        device, acl_state, bond_state, transport,
                    ));
                    if ble_acl_state == BtAclState::Connected && bond_state == BtBondState::Bonded {
                        info!("BAS: Connecting to {}", DisplayAddress(&addr));
                        battery_service.lock().unwrap().init_device(addr);
                    }

                // For battery service, use this to clean up internal handles. GATT connection is
                // already dropped if ACL disconnect has occurred.
                Message::OnDeviceDisconnected(device) => {
                    battery_service
                        .lock()
                        .unwrap()
                        .handle_action(BatteryServiceActions::Disconnect(device));
                }

                Message::SuspendCallbackRegistered(id) => {
@@ -599,6 +604,26 @@ impl Stack {
                        .unwrap()
                        .dispatch_uhid_telephony_use_callback(addr, state);
                }

                Message::ProfileDisconnected(addr) => {
                    let bas_app_uuid =
                        Uuid::from_string(String::from(BATTERY_SERVICE_GATT_CLIENT_APP_ID))
                            .expect("BAS Uuid failed to be parsed");
                    // Ideally we would also check that there are no open sockets for this device
                    // but Floss does not manage socket state so there is no reasonable way for us
                    // to know whether a socket is open or not.
                    if bluetooth_gatt.lock().unwrap().get_connected_applications(&addr)
                        == vec![bas_app_uuid]
                        && !bluetooth.lock().unwrap().is_hh_connected(&addr)
                        && bluetooth_media.lock().unwrap().get_connected_profiles(&addr).is_empty()
                    {
                        info!(
                            "BAS: Disconnecting from {} since it's the last active profile",
                            DisplayAddress(&addr)
                        );
                        battery_service.lock().unwrap().drop_device(addr);
                    }
                }
            }
        }
    }