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

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

floss: Refactor btstack initialization order (3/4 BluetoothMedia) 

This patch does:
- Reverse the dependency - Now BluetoothMedia depends on Bluetooth and
  media is optional in Bluetooth. This makes more sense in the context
  of Bluetooth because it inits earlier than media.
- Since the dependency is reversed we can now init BluetoothMedia later,
  and save many redundant `Option` in BluetoothMedia.

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
Tag: #floss
Test: mmm packages/modules/Bluetooth
Test: bluetooth_AdapterQuickHealth.AVL.all_floss
Test: manual Allegro HFP and A2DP
Flag: EXEMPT, Floss-only changes
Change-Id: If1b81374363755d1a826d8b60c32d41453f5b4a2
parent a0c9568d

system/gd/rust/linux/service/src/main.rs

+16 −17
Original line number Diff line number Diff line
@@ -181,11 +181,6 @@ fn main() -> Result<(), Box<dyn Error>> { 
            battery_provider_manager.clone(),

            tx.clone(),

        ))));

        let bluetooth_media = Arc::new(Mutex::new(Box::new(BluetoothMedia::new(

            tx.clone(),

            intf.clone(),

            battery_provider_manager.clone(),

        ))));

        let bluetooth = Arc::new(Mutex::new(Box::new(Bluetooth::new(

            virt_index,

            hci_index,
@@ -194,14 +189,6 @@ fn main() -> Result<(), Box<dyn Error>> { 
            sig_notifier.clone(),

            intf.clone(),

            bluetooth_gatt.clone(),

            bluetooth_media.clone(),

        ))));

        let suspend = Arc::new(Mutex::new(Box::new(Suspend::new(

            bluetooth.clone(),

            intf.clone(),

            bluetooth_gatt.clone(),

            bluetooth_media.clone(),

            tx.clone(),

        ))));

        let bluetooth_qa = Arc::new(Mutex::new(Box::new(BluetoothQA::new(tx.clone()))));

        let dis = Arc::new(Mutex::new(Box::new(DeviceInformation::new(
@@ -209,21 +196,26 @@ fn main() -> Result<(), Box<dyn Error>> { 
            tx.clone(),

        ))));



        bluetooth_media.lock().unwrap().set_adapter(bluetooth.clone());



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

        bluetooth.lock().unwrap().enable();



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



        // This construction requires |intf| to be already init-ed.

        // These constructions require |intf| to be already init-ed.

        let bt_sock_mgr = Arc::new(Mutex::new(Box::new(BluetoothSocketManager::new(

            tx.clone(),

            bt_sock_mgr_runtime,

            intf.clone(),

        ))));

        let bluetooth_media = Arc::new(Mutex::new(Box::new(BluetoothMedia::new(

            tx.clone(),

            api_tx.clone(),

            intf.clone(),

            bluetooth.clone(),

            battery_provider_manager.clone(),

        ))));



        // This construction doesn't need |intf| to be init-ed, but just depends on those who need.

        // 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(

            String::from(ADMIN_SETTINGS_FILE_PATH),

            tx.clone(),
@@ -231,6 +223,13 @@ fn main() -> Result<(), Box<dyn Error>> { 
            bluetooth_media.clone(),

            bt_sock_mgr.clone(),

        ))));

        let suspend = Arc::new(Mutex::new(Box::new(Suspend::new(

            bluetooth.clone(),

            intf.clone(),

            bluetooth_gatt.clone(),

            bluetooth_media.clone(),

            tx.clone(),

        ))));



        // Run the stack main dispatch loop.

        topstack::get_runtime().spawn(Stack::dispatch(

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

+27 −18
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ use crate::bluetooth_admin::BluetoothAdminPolicyHelper; 
use crate::bluetooth_gatt::{

    BluetoothGatt, GattActions, IBluetoothGatt, IScannerCallback, ScanResult,

};

use crate::bluetooth_media::{BluetoothMedia, IBluetoothMedia, MediaActions, LEA_UNKNOWN_GROUP_ID};

use crate::bluetooth_media::{BluetoothMedia, MediaActions, LEA_UNKNOWN_GROUP_ID};

use crate::callbacks::Callbacks;

use crate::socket_manager::SocketActions;

use crate::uuid::{Profile, UuidHelper};
@@ -593,7 +593,7 @@ pub struct Bluetooth { 
    ble_scanner_id: Option<u8>,

    ble_scanner_uuid: Option<Uuid>,

    bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,

    bluetooth_media: Arc<Mutex<Box<BluetoothMedia>>>,

    bluetooth_media: Option<Arc<Mutex<Box<BluetoothMedia>>>>,

    callbacks: Callbacks<dyn IBluetoothCallback + Send>,

    connection_callbacks: Callbacks<dyn IBluetoothConnectionCallback + Send>,

    discovering_started: Instant,
@@ -641,7 +641,6 @@ impl Bluetooth { 
        sig_notifier: Arc<SigData>,

        intf: Arc<Mutex<BluetoothInterface>>,

        bluetooth_gatt: Arc<Mutex<Box<BluetoothGatt>>>,

        bluetooth_media: Arc<Mutex<Box<BluetoothMedia>>>,

    ) -> Bluetooth {

        Bluetooth {

            virt_index,
@@ -656,7 +655,7 @@ impl Bluetooth { 
            ble_scanner_id: None,

            ble_scanner_uuid: None,

            bluetooth_gatt,

            bluetooth_media,

            bluetooth_media: None,

            discovering_started: Instant::now(),

            intf,

            is_connectable: false,
@@ -687,6 +686,10 @@ impl Bluetooth { 
        }

    }



    pub(crate) fn set_media(&mut self, bluetooth_media: Arc<Mutex<Box<BluetoothMedia>>>) {

        self.bluetooth_media = Some(bluetooth_media);

    }



    fn update_connectable_mode(&mut self, is_sock_listening: bool) {

        // Set connectable if

        // - there is bredr socket listening, or
@@ -1279,8 +1282,10 @@ impl Bluetooth { 
        };

        let device = context.info.clone();



        let mut connected_profiles =

            self.bluetooth_media.lock().unwrap().get_connected_profiles(&device);

        let mut connected_profiles = self

            .bluetooth_media

            .as_ref()

            .map_or(HashSet::new(), |media| media.lock().unwrap().get_connected_profiles(&device));

        if let Some(profile) = context.connected_hid_profile {

            connected_profiles.insert(profile);

        }
@@ -1470,9 +1475,6 @@ impl BtifBluetoothCallbacks for Bluetooth { 
            }



            BtState::On => {

                // Initialize media

                self.bluetooth_media.lock().unwrap().initialize();



                // Initialize core profiles

                self.init_profiles();
@@ -1532,8 +1534,6 @@ impl BtifBluetoothCallbacks for Bluetooth { 
                });

                tokio::spawn(async move {

                    let _ = api_txl.send(APIMessage::IsReady(BluetoothAPI::Adapter)).await;

                    // TODO(b:300202052) make sure media interface is exposed after initialized

                    let _ = api_txl.send(APIMessage::IsReady(BluetoothAPI::Media)).await;

                });

            }

        }
@@ -2575,12 +2575,18 @@ impl IBluetooth for Bluetooth { 
    fn get_profile_connection_state(&self, profile: Uuid) -> ProfileConnectionState {

        if let Some(known) = UuidHelper::is_known_profile(&profile) {

            match known {

                Profile::A2dpSink | Profile::A2dpSource => {

                    self.bluetooth_media.lock().unwrap().get_a2dp_connection_state()

                }

                Profile::Hfp | Profile::HfpAg => {

                    self.bluetooth_media.lock().unwrap().get_hfp_connection_state()

                }

                Profile::A2dpSink | Profile::A2dpSource => self

                    .bluetooth_media

                    .as_ref()

                    .map_or(ProfileConnectionState::Disconnected, |media| {

                        media.lock().unwrap().get_a2dp_connection_state()

                    }),

                Profile::Hfp | Profile::HfpAg => self

                    .bluetooth_media

                    .as_ref()

                    .map_or(ProfileConnectionState::Disconnected, |media| {

                        media.lock().unwrap().get_hfp_connection_state()

                    }),

                // TODO: (b/223431229) Profile::Hid and Profile::Hogp

                _ => ProfileConnectionState::Disconnected,

            }
@@ -2928,7 +2934,10 @@ impl IBluetooth for Bluetooth { 
                    || uuids.contains(&get_unwrapped_uuid(Profile::Hfp)))

        }



        let media = self.bluetooth_media.lock().unwrap();

        let Some(media) = self.bluetooth_media.as_ref() else {

            return false;

        };

        let media = media.lock().unwrap();

        let group_id = media.get_group_id(device.address);

        if group_id == LEA_UNKNOWN_GROUP_ID {

            return is_dual_mode(self.get_remote_uuids(device));

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

+306 −693

File changed.

Preview size limit exceeded, changes collapsed.

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

+4 −1
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ use crate::bluetooth_gatt::{ 
    dispatch_gatt_client_callbacks, dispatch_gatt_server_callbacks, dispatch_le_scanner_callbacks,

    dispatch_le_scanner_inband_callbacks, BluetoothGatt, GattActions,

};

use crate::bluetooth_media::{BluetoothMedia, MediaActions};

use crate::bluetooth_media::{BluetoothMedia, IBluetoothMedia, MediaActions};

use crate::dis::{DeviceInformation, ServiceCallbacks};

use crate::socket_manager::{BluetoothSocketManager, SocketActions};

use crate::suspend::Suspend;
@@ -304,6 +304,9 @@ impl Stack { 
                    // Initialize objects that need the adapter to be fully

                    // enabled before running.



                    // Init Media and pass it to Bluetooth.

                    bluetooth_media.lock().unwrap().initialize();

                    bluetooth.lock().unwrap().set_media(bluetooth_media.clone());

                    // Register device information service.

                    bluetooth_dis.lock().unwrap().initialize();

                    // Initialize Admin. This toggles the enabled profiles.

system/gd/rust/topshim/src/profiles/csis.rs

+2 −3
Original line number Diff line number Diff line
@@ -143,10 +143,9 @@ impl ToggleableProfile for CsisClient { 


impl CsisClient {

    pub fn new(intf: &BluetoothInterface) -> CsisClient {

        let csis_if: cxx::UniquePtr<ffi::CsisClientIntf>;



        // SAFETY: `intf.as_raw_ptr()` is a valid pointer to a `BluetoothInterface`

        csis_if = unsafe { ffi::GetCsisClientProfile(intf.as_raw_ptr()) };

        let csis_if: cxx::UniquePtr<ffi::CsisClientIntf> =

            unsafe { ffi::GetCsisClientProfile(intf.as_raw_ptr()) };



        CsisClient { internal: csis_if, is_init: false, is_enabled: false }

    }