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

Commit f86647eb authored by Yun-Hao Chung's avatar Yun-Hao Chung
Browse files

Floss: Expose GATT API to DBus after interal API is ready

Make sure the API is ready in Floss layer and topshim api before
exposing the DBus API. Otherwise clients could poke the API too early
and cause crashes or unexpected behavior.

Bug: 300202055
Test: mma -j 32
Test: verify all interfaces are there via gdbus
Tag: #floss
Change-Id: I039c259c1c2cd32b4dbf781b36deff58cc7a9b4c
parent a0c5c54a
Loading
Loading
Loading
Loading
+12 −4
Original line number Diff line number Diff line
@@ -7,10 +7,10 @@ use tokio::sync::mpsc::{channel, Receiver, Sender};

use btstack::{
    battery_manager::BatteryManager, battery_provider_manager::BatteryProviderManager,
    bluetooth::Bluetooth, bluetooth_admin::BluetoothAdmin, bluetooth_gatt::BluetoothGatt,
    bluetooth_logging::BluetoothLogging, bluetooth_media::BluetoothMedia,
    bluetooth_qa::BluetoothQA, socket_manager::BluetoothSocketManager, suspend::Suspend,
    APIMessage, BluetoothAPI,
    battery_service::BatteryService, bluetooth::Bluetooth, bluetooth_admin::BluetoothAdmin,
    bluetooth_gatt::BluetoothGatt, bluetooth_logging::BluetoothLogging,
    bluetooth_media::BluetoothMedia, bluetooth_qa::BluetoothQA,
    socket_manager::BluetoothSocketManager, suspend::Suspend, APIMessage, BluetoothAPI,
};

use crate::iface_battery_manager;
@@ -43,6 +43,7 @@ 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>>>,
@@ -197,6 +198,13 @@ 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(
+7 −10
Original line number Diff line number Diff line
@@ -8,7 +8,6 @@ use std::error::Error;
use std::sync::{Arc, Condvar, Mutex};
use std::time::Duration;
use tokio::sync::mpsc::Sender;
use tokio::time;

// Necessary to link right entries.
#[allow(unused_imports)]
@@ -149,6 +148,7 @@ fn main() -> Result<(), Box<dyn Error>> {
        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(),
@@ -232,6 +232,7 @@ 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(),
@@ -252,15 +253,11 @@ fn main() -> Result<(), Box<dyn Error>> {
            bluetooth.init(init_flags);
            bluetooth.enable();

            bluetooth_gatt.lock().unwrap().init_profiles(tx.clone(), adapter.clone());
            // TODO(b/247093293): Gatt topshim api is only usable some
            // time after init. Investigate why this delay is needed
            // and make it a blocking part of init before removing
            // this.
            tokio::spawn(async move {
                time::sleep(Duration::from_millis(500)).await;
                battery_service.lock().unwrap().init();
            });
            bluetooth_gatt.lock().unwrap().init_profiles(
                tx.clone(),
                api_tx.clone(),
                adapter.clone(),
            );
            bt_sock_mgr.lock().unwrap().initialize(intf.clone());

            // Install SIGTERM handler so that we can properly shutdown
+11 −1
Original line number Diff line number Diff line
@@ -7,10 +7,10 @@ use crate::bluetooth_gatt::{
    BluetoothGatt, BluetoothGattService, IBluetoothGatt, IBluetoothGattCallback,
};
use crate::callbacks::Callbacks;
use crate::uuid;
use crate::uuid::UuidHelper;
use crate::Message;
use crate::RPCProxy;
use crate::{uuid, APIMessage, BluetoothAPI};
use bt_topshim::btif::BtTransport;
use bt_topshim::profiles::gatt::{GattStatus, LePhy};
use log::debug;
@@ -31,6 +31,8 @@ pub struct BatteryService {
    battery_provider_id: u32,
    /// Sender for callback communication with the main thread.
    tx: Sender<Message>,
    /// Sender for callback communication with the api message thread.
    api_tx: Sender<APIMessage>,
    callbacks: Callbacks<dyn IBatteryServiceCallback + Send>,
    /// The GATT client ID needed for GATT calls.
    client_id: Option<i32>,
@@ -98,6 +100,7 @@ impl BatteryService {
        gatt: Arc<Mutex<Box<BluetoothGatt>>>,
        battery_provider_manager: Arc<Mutex<Box<BatteryProviderManager>>>,
        tx: Sender<Message>,
        api_tx: Sender<APIMessage>,
    ) -> BatteryService {
        let tx = tx.clone();
        let callbacks = Callbacks::new(tx.clone(), Message::BatteryServiceCallbackDisconnected);
@@ -113,6 +116,7 @@ impl BatteryService {
            battery_provider_manager,
            battery_provider_id,
            tx,
            api_tx,
            callbacks,
            client_id,
            battery_sets,
@@ -129,6 +133,12 @@ impl BatteryService {
            Box::new(GattCallback::new(self.tx.clone())),
            false,
        );

        // TODO(b:300202503) make sure battery interface is exposed after initialized
        let api_tx = self.api_tx.clone();
        tokio::spawn(async move {
            let _ = api_tx.send(APIMessage::IsReady(BluetoothAPI::Battery)).await;
        });
    }

    /// Handles all callback messages in a central location to avoid deadlocks.
+0 −4
Original line number Diff line number Diff line
@@ -1347,10 +1347,6 @@ impl BtifBluetoothCallbacks for Bluetooth {
                    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;
                    // TODO(b:300202055) make sure GATT interface is exposed after initialized
                    let _ = api_txl.send(APIMessage::IsReady(BluetoothAPI::Gatt)).await;
                    // TODO(b:300202503) make sure battery interface is exposed after initialized
                    let _ = api_txl.send(APIMessage::IsReady(BluetoothAPI::Battery)).await;
                });
            }
        }
+14 −2
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ use crate::bluetooth_adv::{
};
use crate::callbacks::Callbacks;
use crate::uuid::UuidHelper;
use crate::{Message, RPCProxy, SuspendMode};
use crate::{APIMessage, BluetoothAPI, Message, RPCProxy, SuspendMode};
use log::{debug, warn};
use num_derive::{FromPrimitive, ToPrimitive};
use num_traits::cast::{FromPrimitive, ToPrimitive};
@@ -36,6 +36,7 @@ use std::collections::{HashMap, HashSet};
use std::convert::TryInto;
use std::sync::{Arc, Mutex, MutexGuard};
use tokio::sync::mpsc::Sender;
use tokio::time;

struct Client {
    id: Option<i32>,
@@ -1389,7 +1390,12 @@ impl BluetoothGatt {
        }
    }

    pub fn init_profiles(&mut self, tx: Sender<Message>, adapter: Arc<Mutex<Box<Bluetooth>>>) {
    pub fn init_profiles(
        &mut self,
        tx: Sender<Message>,
        api_tx: Sender<APIMessage>,
        adapter: Arc<Mutex<Box<Bluetooth>>>,
    ) {
        self.gatt = Gatt::new(&self.intf.lock().unwrap()).map(|gatt| Arc::new(Mutex::new(gatt)));
        self.adapter = Some(adapter);

@@ -1464,8 +1470,14 @@ impl BluetoothGatt {

        let gatt = self.gatt.clone();
        let gatt_async = self.gatt_async.clone();
        let api_tx_clone = api_tx.clone();
        tokio::spawn(async move {
            gatt_async.lock().await.gatt = gatt;
            // TODO(b/247093293): Gatt topshim api is only usable some
            // time after init. Investigate why this delay is needed
            // and make it a blocking part before removing this.
            time::sleep(time::Duration::from_millis(500)).await;
            let _ = api_tx_clone.send(APIMessage::IsReady(BluetoothAPI::Gatt)).await;
        });
    }