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

Commit bbb1f64d authored by Michael Sun's avatar Michael Sun
Browse files

Floss: pause discovery during bond and connect

This patch introduces a mechanism that temporarily prevents clients
from restarting discovery, rather than just canceling it. The patch
prevents clients and Floss from repeatedly starting and stopping
discovery.

Bug: 277977567
Tag: #floss
Test: Manual - Pairing don't triggers the start/stop discovery loop
Change-Id: Iea707981ca1a38d463d461bb1696e98a5b29ecba
parent 384e2240
Loading
Loading
Loading
Loading
+3 −3
Original line number Original line Diff line number Diff line
@@ -623,10 +623,10 @@ impl CommandHandler {


        match &command[..] {
        match &command[..] {
            "start" => {
            "start" => {
                self.lock_context().adapter_dbus.as_ref().unwrap().start_discovery();
                self.lock_context().adapter_dbus.as_mut().unwrap().start_discovery();
            }
            }
            "stop" => {
            "stop" => {
                self.lock_context().adapter_dbus.as_ref().unwrap().cancel_discovery();
                self.lock_context().adapter_dbus.as_mut().unwrap().cancel_discovery();
            }
            }
            _ => return Err(CommandError::InvalidArgs),
            _ => return Err(CommandError::InvalidArgs),
        }
        }
@@ -661,7 +661,7 @@ impl CommandHandler {
                let success = self
                let success = self
                    .lock_context()
                    .lock_context()
                    .adapter_dbus
                    .adapter_dbus
                    .as_ref()
                    .as_mut()
                    .unwrap()
                    .unwrap()
                    .create_bond(device.clone(), BtTransport::Auto);
                    .create_bond(device.clone(), BtTransport::Auto);


+3 −3
Original line number Original line Diff line number Diff line
@@ -719,12 +719,12 @@ impl IBluetooth for BluetoothDBus {
    }
    }


    #[dbus_method("StartDiscovery")]
    #[dbus_method("StartDiscovery")]
    fn start_discovery(&self) -> bool {
    fn start_discovery(&mut self) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


    #[dbus_method("CancelDiscovery")]
    #[dbus_method("CancelDiscovery")]
    fn cancel_discovery(&self) -> bool {
    fn cancel_discovery(&mut self) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


@@ -739,7 +739,7 @@ impl IBluetooth for BluetoothDBus {
    }
    }


    #[dbus_method("CreateBond")]
    #[dbus_method("CreateBond")]
    fn create_bond(&self, device: BluetoothDevice, transport: BtTransport) -> bool {
    fn create_bond(&mut self, device: BluetoothDevice, transport: BtTransport) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


+3 −3
Original line number Original line Diff line number Diff line
@@ -482,12 +482,12 @@ impl IBluetooth for IBluetoothDBus {
    }
    }


    #[dbus_method("StartDiscovery")]
    #[dbus_method("StartDiscovery")]
    fn start_discovery(&self) -> bool {
    fn start_discovery(&mut self) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


    #[dbus_method("CancelDiscovery")]
    #[dbus_method("CancelDiscovery")]
    fn cancel_discovery(&self) -> bool {
    fn cancel_discovery(&mut self) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


@@ -502,7 +502,7 @@ impl IBluetooth for IBluetoothDBus {
    }
    }


    #[dbus_method("CreateBond")]
    #[dbus_method("CreateBond")]
    fn create_bond(&self, device: BluetoothDevice, transport: BtTransport) -> bool {
    fn create_bond(&mut self, device: BluetoothDevice, transport: BtTransport) -> bool {
        dbus_generated!()
        dbus_generated!()
    }
    }


+66 −12
Original line number Original line Diff line number Diff line
@@ -118,10 +118,10 @@ pub trait IBluetooth {
    fn is_le_extended_advertising_supported(&self) -> bool;
    fn is_le_extended_advertising_supported(&self) -> bool;


    /// Starts BREDR Inquiry.
    /// Starts BREDR Inquiry.
    fn start_discovery(&self) -> bool;
    fn start_discovery(&mut self) -> bool;


    /// Cancels BREDR Inquiry.
    /// Cancels BREDR Inquiry.
    fn cancel_discovery(&self) -> bool;
    fn cancel_discovery(&mut self) -> bool;


    /// Checks if discovery is started.
    /// Checks if discovery is started.
    fn is_discovering(&self) -> bool;
    fn is_discovering(&self) -> bool;
@@ -130,7 +130,7 @@ pub trait IBluetooth {
    fn get_discovery_end_millis(&self) -> u64;
    fn get_discovery_end_millis(&self) -> u64;


    /// Initiates pairing to a remote device. Triggers connection if not already started.
    /// Initiates pairing to a remote device. Triggers connection if not already started.
    fn create_bond(&self, device: BluetoothDevice, transport: BtTransport) -> bool;
    fn create_bond(&mut self, device: BluetoothDevice, transport: BtTransport) -> bool;


    /// Cancels any pending bond attempt on given device.
    /// Cancels any pending bond attempt on given device.
    fn cancel_bond_process(&self, device: BluetoothDevice) -> bool;
    fn cancel_bond_process(&self, device: BluetoothDevice) -> bool;
@@ -459,8 +459,10 @@ pub struct Bluetooth {
    is_connectable: bool,
    is_connectable: bool,
    is_discovering: bool,
    is_discovering: bool,
    is_discovering_before_suspend: bool,
    is_discovering_before_suspend: bool,
    is_discovery_paused: bool,
    discovery_suspend_mode: SuspendMode,
    discovery_suspend_mode: SuspendMode,
    local_address: Option<RawAddress>,
    local_address: Option<RawAddress>,
    pending_discovery: bool,
    properties: HashMap<BtPropertyType, BluetoothProperty>,
    properties: HashMap<BtPropertyType, BluetoothProperty>,
    profiles_ready: bool,
    profiles_ready: bool,
    found_devices: HashMap<String, BluetoothDeviceContext>,
    found_devices: HashMap<String, BluetoothDeviceContext>,
@@ -505,8 +507,10 @@ impl Bluetooth {
            is_connectable: false,
            is_connectable: false,
            is_discovering: false,
            is_discovering: false,
            is_discovering_before_suspend: false,
            is_discovering_before_suspend: false,
            is_discovery_paused: false,
            discovery_suspend_mode: SuspendMode::Normal,
            discovery_suspend_mode: SuspendMode::Normal,
            local_address: None,
            local_address: None,
            pending_discovery: false,
            properties: HashMap::new(),
            properties: HashMap::new(),
            profiles_ready: false,
            profiles_ready: false,
            found_devices: HashMap::new(),
            found_devices: HashMap::new(),
@@ -990,6 +994,23 @@ impl Bluetooth {


        return BtStatus::Success;
        return BtStatus::Success;
    }
    }

    /// Temporarily stop the discovery process and mark it as paused so that clients cannot restart
    /// it.
    fn pause_discovery(&mut self) {
        self.cancel_discovery();
        self.is_discovery_paused = true;
    }

    /// Remove the paused flag to allow clients to begin discovery, and if there is already a
    /// pending request, start discovery.
    fn resume_discovery(&mut self) {
        if self.pending_discovery {
            self.pending_discovery = false;
            self.start_discovery();
        }
        self.is_discovery_paused = false;
    }
}
}


#[btif_callbacks_dispatcher(dispatch_base_callbacks, BaseCallbacks)]
#[btif_callbacks_dispatcher(dispatch_base_callbacks, BaseCallbacks)]
@@ -1399,6 +1420,12 @@ impl BtifBluetoothCallbacks for Bluetooth {
                .and_modify(|d| d.bond_state = bond_state.clone());
                .and_modify(|d| d.bond_state = bond_state.clone());
        }
        }


        // 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 {
            self.resume_discovery();
        }

        // Send bond state changed notifications
        // Send bond state changed notifications
        self.callbacks.for_all_callbacks(|callback| {
        self.callbacks.for_all_callbacks(|callback| {
            callback.on_bond_state_changed(
            callback.on_bond_state_changed(
@@ -1490,6 +1517,10 @@ impl BtifBluetoothCallbacks for Bluetooth {
        conn_direction: BtConnectionDirection,
        conn_direction: BtConnectionDirection,
        _acl_handle: u16,
        _acl_handle: u16,
    ) {
    ) {
        // If discovery was previously paused at connect_all_enabled_profiles to avoid an outgoing
        // ACL connection colliding with an ongoing inquiry, resume it.
        self.resume_discovery();

        if status != BtStatus::Success {
        if status != BtStatus::Success {
            warn!(
            warn!(
                "Connection to [{}] failed. Status: {:?}, Reason: {:?}",
                "Connection to [{}] failed. Status: {:?}, Reason: {:?}",
@@ -1788,12 +1819,19 @@ impl IBluetooth for Bluetooth {
        }
        }
    }
    }


    fn start_discovery(&self) -> bool {
    fn start_discovery(&mut self) -> bool {
        // Short-circuit to avoid sending multiple start discovery calls.
        // Short-circuit to avoid sending multiple start discovery calls.
        if self.is_discovering {
        if self.is_discovering {
            return true;
            return true;
        }
        }


        // Short-circuit if paused and add the discovery intent to the queue.
        if self.is_discovery_paused {
            self.pending_discovery = true;
            debug!("Queue the discovery request during paused state");
            return true;
        }

        let discovery_suspend_mode = self.get_discovery_suspend_mode();
        let discovery_suspend_mode = self.get_discovery_suspend_mode();
        if discovery_suspend_mode != SuspendMode::Normal
        if discovery_suspend_mode != SuspendMode::Normal
            && discovery_suspend_mode != SuspendMode::Resuming
            && discovery_suspend_mode != SuspendMode::Resuming
@@ -1805,7 +1843,13 @@ impl IBluetooth for Bluetooth {
        self.intf.lock().unwrap().start_discovery() == 0
        self.intf.lock().unwrap().start_discovery() == 0
    }
    }


    fn cancel_discovery(&self) -> bool {
    fn cancel_discovery(&mut self) -> bool {
        // Client no longer want to discover, clear the request
        if self.is_discovery_paused {
            self.pending_discovery = false;
            debug!("Cancel the discovery request during paused state");
        }

        // Reject the cancel discovery request if the underlying stack is not in a discovering
        // Reject the cancel discovery request if the underlying stack is not in a discovering
        // state. For example, previous start discovery was enqueued for ongoing discovery.
        // state. For example, previous start discovery was enqueued for ongoing discovery.
        if !self.is_discovering {
        if !self.is_discovering {
@@ -1841,7 +1885,7 @@ impl IBluetooth for Bluetooth {
        }
        }
    }
    }


    fn create_bond(&self, device: BluetoothDevice, transport: BtTransport) -> bool {
    fn create_bond(&mut self, device: BluetoothDevice, transport: BtTransport) -> bool {
        let addr = RawAddress::from_string(device.address.clone());
        let addr = RawAddress::from_string(device.address.clone());


        if addr.is_none() {
        if addr.is_none() {
@@ -1869,8 +1913,7 @@ impl IBluetooth for Bluetooth {
        metrics::bond_create_attempt(address, device_type.clone());
        metrics::bond_create_attempt(address, device_type.clone());


        // BREDR connection won't work when Inquiry is in progress.
        // BREDR connection won't work when Inquiry is in progress.
        self.cancel_discovery();
        self.pause_discovery();

        let status = self.intf.lock().unwrap().create_bond(&address, transport);
        let status = self.intf.lock().unwrap().create_bond(&address, transport);


        if status != 0 {
        if status != 0 {
@@ -2225,20 +2268,21 @@ impl IBluetooth for Bluetooth {
            }
            }
        };
        };


        // log ACL connection attempt if it's not already connected.
        let is_connected = self
        let is_connected = self
            .get_remote_device_if_found(&device.address)
            .get_remote_device_if_found(&device.address)
            .map_or(false, |d| d.acl_state == BtAclState::Connected);
            .map_or(false, |d| d.acl_state == BtAclState::Connected);
        if !is_connected {
        if !is_connected {
            // log ACL connection attempt if it's not already connected.
            metrics::acl_connect_attempt(addr, BtAclState::Connected);
            metrics::acl_connect_attempt(addr, BtAclState::Connected);
            // Pause discovery before connecting, or the ACL connection request may conflict with
            // the ongoing inquiry.
            self.pause_discovery();
        }
        }


        // Cancel discovery before attempting to connect (or we'll get connection failures).
        self.cancel_discovery();

        // Check all remote uuids to see if they match enabled profiles and connect them.
        // Check all remote uuids to see if they match enabled profiles and connect them.
        let mut has_enabled_uuids = false;
        let mut has_enabled_uuids = false;
        let mut has_media_profile = false;
        let mut has_media_profile = false;
        let mut has_supported_profile = false;
        let uuids = self.get_remote_uuids(device.clone());
        let uuids = self.get_remote_uuids(device.clone());
        for uuid in uuids.iter() {
        for uuid in uuids.iter() {
            match UuidHelper::is_known_profile(uuid) {
            match UuidHelper::is_known_profile(uuid) {
@@ -2246,6 +2290,7 @@ impl IBluetooth for Bluetooth {
                    if UuidHelper::is_profile_supported(&p) {
                    if UuidHelper::is_profile_supported(&p) {
                        match p {
                        match p {
                            Profile::Hid | Profile::Hogp => {
                            Profile::Hid | Profile::Hogp => {
                                has_supported_profile = true;
                                let status = self.hh.as_ref().unwrap().connect(&mut addr);
                                let status = self.hh.as_ref().unwrap().connect(&mut addr);
                                metrics::profile_connection_state_changed(
                                metrics::profile_connection_state_changed(
                                    addr,
                                    addr,
@@ -2267,6 +2312,7 @@ impl IBluetooth for Bluetooth {
                            Profile::A2dpSink | Profile::A2dpSource | Profile::Hfp
                            Profile::A2dpSink | Profile::A2dpSource | Profile::Hfp
                                if !has_media_profile =>
                                if !has_media_profile =>
                            {
                            {
                                has_supported_profile = true;
                                has_media_profile = true;
                                has_media_profile = true;
                                let txl = self.tx.clone();
                                let txl = self.tx.clone();
                                let address = device.address.clone();
                                let address = device.address.clone();
@@ -2278,6 +2324,7 @@ impl IBluetooth for Bluetooth {
                            }
                            }


                            Profile::Bas => {
                            Profile::Bas => {
                                has_supported_profile = true;
                                let tx = self.tx.clone();
                                let tx = self.tx.clone();
                                let transport =
                                let transport =
                                    match self.get_remote_device_if_found(&device.address) {
                                    match self.get_remote_device_if_found(&device.address) {
@@ -2323,6 +2370,13 @@ impl IBluetooth for Bluetooth {
            }
            }
        }
        }


        // If the SDP has not been completed or the device does not have a profile that we are
        // interested in connecting to, resume discovery now. Other cases will be handled in the
        // ACL connection state or bond state callbacks.
        if !has_enabled_uuids || !has_supported_profile {
            self.resume_discovery();
        }

        return true;
        return true;
    }
    }