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

Commit b7c96798 authored by Hsin-chen Chuang's avatar Hsin-chen Chuang
Browse files

floss: Refactor btstack initialization order (4/4 BluetoothGatt)

This patch does:
- Move the scattered Gatt initialization (and the profiles depend on it)
  to lib.rs AdapterReady, so:
  - Gatt API ready delay can be removed
  - Bluetooth is now almost not depending on BluetoothGatt
- BluetoothGatt is now optional in Bluetooth. This makes more sense in
  the context of Bluetooth because it inits earlier than Gatt.
- Since BluetoothGatt is init-ed after Bluetooth we can save many
  redundant `Option` in it.

Some backgrounds:
  We want to get rid of the redundant `Option` of the topshim objects
  that is causing many verbosities and confusing snippets (We all know
  we can't do anything without the objects... Then why is it optional?).
  The overall direction is to remove all Bluetooth's dependencies on
  initialization, because the Bluetooth topshim object needs to be
  initialized first before all other topshim objects can be obtained.

Bug: 254870880
Bug: 247093293
Tag: #floss
Test: mmm packages/modules/Bluetooth
Test: bluetooth_AdapterQuickHealth.AVL.all_floss
Test: manual connect LE mouse, BAS works
Test: manual FastPair / SmartLock (GATT)
Test: manual Sphero mini / SPRK+
Test: manual NearbyShare on nami (SW Adv) and guybrush (Ext Adv)
Flag: EXEMPT, Floss-only changes
Change-Id: I14da61138c6baf56173aa88f75c0ef1f3ccd9a0f
parent adb4b458
Loading
Loading
Loading
Loading
+1 −23
Original line number Diff line number Diff line
@@ -7,8 +7,7 @@ use tokio::sync::mpsc::{channel, Receiver, Sender};

use btstack::{
    battery_manager::BatteryManager, battery_provider_manager::BatteryProviderManager,
    battery_service::BatteryService, bluetooth::Bluetooth, bluetooth::IBluetooth,
    bluetooth_admin::BluetoothAdmin, bluetooth_gatt::BluetoothGatt,
    bluetooth::Bluetooth, bluetooth_admin::BluetoothAdmin, bluetooth_gatt::BluetoothGatt,
    bluetooth_logging::BluetoothLogging, bluetooth_media::BluetoothMedia,
    bluetooth_qa::BluetoothQA, socket_manager::BluetoothSocketManager, suspend::Suspend,
    APIMessage, BluetoothAPI, Message,
@@ -59,7 +58,6 @@ impl InterfaceManager {
        bluetooth: Arc<Mutex<Box<Bluetooth>>>,
        bluetooth_admin: Arc<Mutex<Box<BluetoothAdmin>>>,
        bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,
        battery_service: Arc<Mutex<Box<BatteryService>>>,
        battery_manager: Arc<Mutex<Box<BatteryManager>>>,
        battery_provider_manager: Arc<Mutex<Box<BatteryProviderManager>>>,
        bluetooth_media: Arc<Mutex<Box<BluetoothMedia>>>,
@@ -201,19 +199,6 @@ impl InterfaceManager {
                            &[qa_iface],
                            bluetooth_qa.clone(),
                        );

                        // AdvertiseManager selects the stack per is_le_ext_adv_supported.
                        // Initialize it after Adapter is ready.
                        let bt_clone = bluetooth.clone();
                        let gatt_clone = bluetooth_gatt.clone();
                        tokio::spawn(async move {
                            let is_le_ext_adv_supported =
                                bt_clone.lock().unwrap().is_le_extended_advertising_supported();
                            gatt_clone
                                .lock()
                                .unwrap()
                                .init_adv_manager(bt_clone, is_le_ext_adv_supported);
                        });
                    }
                    BluetoothAPI::Admin => {
                        cr.lock().unwrap().insert(
@@ -228,13 +213,6 @@ impl InterfaceManager {
                            &[gatt_iface],
                            bluetooth_gatt.clone(),
                        );

                        // Battery service is on top of Gatt. Only initialize it after
                        // GATT is ready.
                        let bs = battery_service.clone();
                        tokio::spawn(async move {
                            bs.lock().unwrap().init();
                        });
                    }
                    BluetoothAPI::Media => {
                        cr.lock().unwrap().insert(
+19 −23
Original line number Diff line number Diff line
@@ -166,21 +166,7 @@ fn main() -> Result<(), Box<dyn Error>> {
        }

        // Construct btstack profiles.
        let intf = Arc::new(Mutex::new(get_btinterface().unwrap()));
        let bluetooth_gatt =
            Arc::new(Mutex::new(Box::new(BluetoothGatt::new(intf.clone(), tx.clone()))));
        let battery_provider_manager =
            Arc::new(Mutex::new(Box::new(BatteryProviderManager::new(tx.clone()))));
        let battery_service = Arc::new(Mutex::new(Box::new(BatteryService::new(
            bluetooth_gatt.clone(),
            battery_provider_manager.clone(),
            tx.clone(),
            api_tx.clone(),
        ))));
        let battery_manager = Arc::new(Mutex::new(Box::new(BatteryManager::new(
            battery_provider_manager.clone(),
            tx.clone(),
        ))));
        let intf = Arc::new(Mutex::new(get_btinterface()));
        let bluetooth = Arc::new(Mutex::new(Box::new(Bluetooth::new(
            virt_index,
            hci_index,
@@ -188,19 +174,14 @@ fn main() -> Result<(), Box<dyn Error>> {
            api_tx.clone(),
            sig_notifier.clone(),
            intf.clone(),
            bluetooth_gatt.clone(),
        ))));
        let bluetooth_qa = Arc::new(Mutex::new(Box::new(BluetoothQA::new(tx.clone()))));
        let dis = Arc::new(Mutex::new(Box::new(DeviceInformation::new(
            bluetooth_gatt.clone(),
            tx.clone(),
        ))));
        let battery_provider_manager =
            Arc::new(Mutex::new(Box::new(BatteryProviderManager::new(tx.clone()))));

        bluetooth.lock().unwrap().init(init_flags, hci_index);
        bluetooth.lock().unwrap().enable();

        bluetooth_gatt.lock().unwrap().init_profiles(tx.clone(), api_tx.clone());

        // These constructions require |intf| to be already init-ed.
        let bt_sock_mgr = Arc::new(Mutex::new(Box::new(BluetoothSocketManager::new(
            tx.clone(),
@@ -214,6 +195,8 @@ fn main() -> Result<(), Box<dyn Error>> {
            bluetooth.clone(),
            battery_provider_manager.clone(),
        ))));
        let bluetooth_gatt =
            Arc::new(Mutex::new(Box::new(BluetoothGatt::new(intf.clone(), tx.clone()))));

        // These constructions don't need |intf| to be init-ed, but just depend on those who need.
        let bluetooth_admin = Arc::new(Mutex::new(Box::new(BluetoothAdmin::new(
@@ -230,6 +213,20 @@ fn main() -> Result<(), Box<dyn Error>> {
            bluetooth_media.clone(),
            tx.clone(),
        ))));
        let battery_service = Arc::new(Mutex::new(Box::new(BatteryService::new(
            bluetooth_gatt.clone(),
            battery_provider_manager.clone(),
            tx.clone(),
            api_tx.clone(),
        ))));
        let battery_manager = Arc::new(Mutex::new(Box::new(BatteryManager::new(
            battery_provider_manager.clone(),
            tx.clone(),
        ))));
        let dis = Arc::new(Mutex::new(Box::new(DeviceInformation::new(
            bluetooth_gatt.clone(),
            tx.clone(),
        ))));

        // Run the stack main dispatch loop.
        topstack::get_runtime().spawn(Stack::dispatch(
@@ -264,7 +261,6 @@ fn main() -> Result<(), Box<dyn Error>> {
            bluetooth.clone(),
            bluetooth_admin.clone(),
            bluetooth_gatt.clone(),
            battery_service.clone(),
            battery_manager.clone(),
            battery_provider_manager.clone(),
            bluetooth_media.clone(),
+21 −20
Original line number Diff line number Diff line
@@ -590,7 +590,7 @@ pub struct Bluetooth {
    remote_devices: HashMap<RawAddress, BluetoothDeviceContext>,
    ble_scanner_id: Option<u8>,
    ble_scanner_uuid: Option<Uuid>,
    bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,
    bluetooth_gatt: Option<Arc<Mutex<Box<BluetoothGatt>>>>,
    bluetooth_media: Option<Arc<Mutex<Box<BluetoothMedia>>>>,
    callbacks: Callbacks<dyn IBluetoothCallback + Send>,
    connection_callbacks: Callbacks<dyn IBluetoothConnectionCallback + Send>,
@@ -638,7 +638,6 @@ impl Bluetooth {
        api_tx: Sender<APIMessage>,
        sig_notifier: Arc<SigData>,
        intf: Arc<Mutex<BluetoothInterface>>,
        bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,
    ) -> Bluetooth {
        Bluetooth {
            virt_index,
@@ -652,7 +651,7 @@ impl Bluetooth {
            hh: None,
            ble_scanner_id: None,
            ble_scanner_uuid: None,
            bluetooth_gatt,
            bluetooth_gatt: None,
            bluetooth_media: None,
            discovering_started: Instant::now(),
            intf,
@@ -688,6 +687,20 @@ impl Bluetooth {
        self.bluetooth_media = Some(bluetooth_media);
    }

    pub(crate) fn set_gatt_and_init_scanner(
        &mut self,
        bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,
    ) {
        self.bluetooth_gatt = Some(bluetooth_gatt.clone());

        // Initialize the BLE scanner for discovery.
        let callback_id = bluetooth_gatt
            .lock()
            .unwrap()
            .register_scanner_callback(Box::new(BleDiscoveryCallbacks::new(self.tx.clone())));
        self.ble_scanner_uuid = Some(bluetooth_gatt.lock().unwrap().register_scanner(callback_id));
    }

    fn update_connectable_mode(&mut self, is_sock_listening: bool) {
        // Set connectable if
        // - there is bredr socket listening, or
@@ -766,8 +779,6 @@ impl Bluetooth {
    }

    pub fn init_profiles(&mut self) {
        self.bluetooth_gatt.lock().unwrap().enable(true);

        self.sdp = Some(Sdp::new(&self.intf.lock().unwrap()));
        self.sdp.as_mut().unwrap().initialize(SdpCallbacksDispatcher {
            dispatch: make_message_dispatcher(self.tx.clone(), Message::Sdp),
@@ -1464,13 +1475,6 @@ impl BtifBluetoothCallbacks for Bluetooth {
                self.le_supported_states = controller.get_ble_supported_states();
                self.le_local_supported_features = controller.get_ble_local_supported_features();

                // Initialize the BLE scanner for discovery.
                let callback_id = self.bluetooth_gatt.lock().unwrap().register_scanner_callback(
                    Box::new(BleDiscoveryCallbacks::new(self.tx.clone())),
                );
                self.ble_scanner_uuid =
                    Some(self.bluetooth_gatt.lock().unwrap().register_scanner(callback_id));

                // Update connectable mode so that disconnected bonded classic device can reconnect
                self.trigger_update_connectable_mode();

@@ -1633,11 +1637,12 @@ impl BtifBluetoothCallbacks for Bluetooth {
        });

        // Start or stop BLE scanning based on discovering state
        if let Some(scanner_id) = self.ble_scanner_id {
        if let (Some(gatt), Some(scanner_id)) = (self.bluetooth_gatt.as_ref(), self.ble_scanner_id)
        {
            if is_discovering {
                self.bluetooth_gatt.lock().unwrap().start_active_scan(scanner_id);
                gatt.lock().unwrap().start_active_scan(scanner_id);
            } else {
                self.bluetooth_gatt.lock().unwrap().stop_active_scan(scanner_id);
                gatt.lock().unwrap().stop_active_scan(scanner_id);
            }
        }

@@ -2085,11 +2090,7 @@ impl IBluetooth for Bluetooth {
    }

    fn disable(&mut self) -> bool {
        let success = self.intf.lock().unwrap().disable() == 0;
        if success {
            self.bluetooth_gatt.lock().unwrap().enable(false);
        }
        success
        self.intf.lock().unwrap().disable() == 0
    }

    fn cleanup(&mut self) {
+2 −1
Original line number Diff line number Diff line
@@ -547,8 +547,9 @@ impl AdvertiseManager {
        &mut self,
        gatt: Arc<Mutex<Gatt>>,
        adapter: Arc<Mutex<Box<Bluetooth>>>,
        is_le_ext_adv_supported: bool,
    ) {
        let is_le_ext_adv_supported =
            adapter.lock().unwrap().is_le_extended_advertising_supported();
        self.adv_manager_impl = if is_le_ext_adv_supported {
            info!("AdvertiseManager: Selected extended advertising stack");
            Some(Box::new(AdvertiseManagerImpl::new(self.tx.clone(), gatt, adapter)))
+55 −129

File changed.

Preview size limit exceeded, changes collapsed.

Loading