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

Commit 08ffb7a7 authored by Sonny Sasaka's avatar Sonny Sasaka
Browse files

Floss: Update filter policy based on MSFT filters 

* When MSFT filter is used, scan parameter's filter policy should be
  "ignore not in accept list".
* When there is at least one scanner that doesn't use filter, filter
  policy should be "accept all advertisements".

Bug: 265215220
Tag: #floss
Test: Manual - Tested with btclient and chrome-btclient

Change-Id: I1c1c5e39dcefdd1ee8c5da5d27cdde52570418d0
parent b82d5d32

system/gd/hci/le_scanning_manager.cc

+12 −1
Original line number Diff line number Diff line
@@ -504,7 +504,10 @@ struct LeScanningManager::impl : public LeAddressManagerCallback { 
      return;

    }



    if (is_scannable && !is_scan_response) {

    // Don't wait for scan response if it's a filtered scan since the filter might miss the scan

    // response.

    if (is_scannable && !is_scan_response &&

        filter_policy_ != LeScanningFilterPolicy::FILTER_ACCEPT_LIST_ONLY) {

      // Waiting for scan response

      return;

    }
@@ -710,6 +713,10 @@ struct LeScanningManager::impl : public LeAddressManagerCallback { 
    scanning_callbacks_->OnSetScannerParameterComplete(scanner_id, ScanningCallback::SUCCESS);

  }



  void set_scan_filter_policy(LeScanningFilterPolicy filter_policy) {

    filter_policy_ = filter_policy;

  }



  void scan_filter_enable(bool enable) {

    if (!is_filter_supported_) {

      LOG_WARN("Advertising filter is not supported");
@@ -1686,6 +1693,10 @@ void LeScanningManager::SetScanParameters( 
  CallOn(pimpl_.get(), &impl::set_scan_parameters, scanner_id, scan_type, scan_interval, scan_window);

}



void LeScanningManager::SetScanFilterPolicy(LeScanningFilterPolicy filter_policy) {

  CallOn(pimpl_.get(), &impl::set_scan_filter_policy, filter_policy);

}



void LeScanningManager::ScanFilterEnable(bool enable) {

  CallOn(pimpl_.get(), &impl::scan_filter_enable, enable);

}

system/gd/hci/le_scanning_manager.h

+2 −0
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ class LeScanningManager : public bluetooth::Module { 
  virtual void SetScanParameters(

      ScannerId scanner_id, LeScanType scan_type, uint16_t scan_interval, uint16_t scan_window);



  virtual void SetScanFilterPolicy(LeScanningFilterPolicy filter_policy);



  /* Scan filter */

  virtual void ScanFilterEnable(bool enable);

system/gd/rust/linux/stack/src/bluetooth_gatt.rs

+39 −7
Original line number Diff line number Diff line
@@ -942,6 +942,11 @@ impl GattAsyncIntf { 
    /// May be converted into real async in the future if btif supports it.

    async fn update_scan(&mut self) {

        if self.scanners.lock().unwrap().values().find(|scanner| scanner.is_active).is_some() {

            // Toggle the scan off and on so that we reset the scan parameters based on whether

            // we have active scanners using hardware filtering.

            // TODO(b/266752123): We can do more bookkeeping to optimize when we really need to

            // toggle. Also improve toggling API into 1 operation that guarantees correct ordering.

            self.gatt.as_ref().unwrap().lock().unwrap().scanner.stop_scan();

            self.gatt.as_ref().unwrap().lock().unwrap().scanner.start_scan();

        } else {

            self.gatt.as_ref().unwrap().lock().unwrap().scanner.stop_scan();
@@ -1343,9 +1348,17 @@ impl IBluetoothGatt for BluetoothGatt { 
            }

        }



        let has_active_unfiltered_scanner = self

            .scanners

            .lock()

            .unwrap()

            .iter()

            .any(|(_uuid, scanner)| scanner.is_active && scanner.filter.is_none());



        let gatt_async = self.gatt_async.clone();

        let scanners = self.scanners.clone();

        let is_msft_supported = self.is_msft_supported();



        tokio::spawn(async move {

            // The three operations below (monitor add, monitor enable, update scan) happen one

            // after another, and cannot be interleaved with other GATT async operations.
@@ -1373,14 +1386,18 @@ impl IBluetoothGatt for BluetoothGatt { 
                }



                log::debug!("Added adv monitor handle = {}", monitor_handle);

            }



            if !gatt_async

                    .msft_adv_monitor_enable(true)

                .msft_adv_monitor_enable(!has_active_unfiltered_scanner)

                .await

                .map_or(false, |status| status == 0)

            {

                    log::error!("Error enabling Advertisement Monitor");

                }

                // TODO(b/266752123):

                // Intel controller throws "Command Disallowed" error if we tried to enable/disable

                // filter but it's already at the same state. This is harmless but we can improve

                // the state machine to avoid calling enable/disable if it's already at that state

                log::error!("Error updating Advertisement Monitor enable");

            }



            gatt_async.update_scan().await;
@@ -1403,6 +1420,13 @@ impl IBluetoothGatt for BluetoothGatt { 
            }

        };



        let has_active_unfiltered_scanner = self

            .scanners

            .lock()

            .unwrap()

            .iter()

            .any(|(_uuid, scanner)| scanner.is_active && scanner.filter.is_none());



        let gatt_async = self.gatt_async.clone();

        tokio::spawn(async move {

            // The two operations below (monitor remove, update scan) happen one after another, and
@@ -1415,6 +1439,14 @@ impl IBluetoothGatt for BluetoothGatt { 
                let _res = gatt_async.msft_adv_monitor_remove(handle).await;

            }



            if !gatt_async

                .msft_adv_monitor_enable(!has_active_unfiltered_scanner)

                .await

                .map_or(false, |status| status == 0)

            {

                log::error!("Error updating Advertisement Monitor enable");

            }



            gatt_async.update_scan().await;

        });

system/main/shim/ble_scanner_interface_impl.h

+1 −1
Original line number Diff line number Diff line
@@ -134,7 +134,7 @@ class BleScannerInterfaceImpl : public ::BleScannerInterface, 
  void OnMsftAdvMonitorAdd(uint8_t monitor_handle,

                           bluetooth::hci::ErrorCode status);

  void OnMsftAdvMonitorRemove(bluetooth::hci::ErrorCode status);

  void OnMsftAdvMonitorEnable(bluetooth::hci::ErrorCode status);

  void OnMsftAdvMonitorEnable(bool enable, bluetooth::hci::ErrorCode status);

  MsftCallbacks msft_callbacks_;



 private:

system/main/shim/le_scanning_manager.cc

+9 −2
Original line number Diff line number Diff line
@@ -283,7 +283,7 @@ void BleScannerInterfaceImpl::MsftAdvMonitorEnable( 
  msft_callbacks_.Enable = cb;

  bluetooth::shim::GetMsftExtensionManager()->MsftAdvMonitorEnable(

      enable, base::Bind(&BleScannerInterfaceImpl::OnMsftAdvMonitorEnable,

                         base::Unretained(this)));

                         base::Unretained(this), enable));

}



/** Callback of adding MSFT filter */
@@ -302,8 +302,15 @@ void BleScannerInterfaceImpl::OnMsftAdvMonitorRemove( 


/** Callback of enabling / disabling MSFT scan filter */

void BleScannerInterfaceImpl::OnMsftAdvMonitorEnable(

    bluetooth::hci::ErrorCode status) {

    bool enable, bluetooth::hci::ErrorCode status) {

  LOG_INFO("in shim layer");



  if (status == bluetooth::hci::ErrorCode::SUCCESS) {

    bluetooth::shim::GetScanning()->SetScanFilterPolicy(

        enable ? bluetooth::hci::LeScanningFilterPolicy::FILTER_ACCEPT_LIST_ONLY

               : bluetooth::hci::LeScanningFilterPolicy::ACCEPT_ALL);

  }



  msft_callbacks_.Enable.Run((uint8_t)status);

}