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

Commit 9475247b authored by Abhishek Pandit-Subedi's avatar Abhishek Pandit-Subedi Committed by Sonny Sasaka
Browse files

floss: Refactor client for proper adapter mgmt

The adapter dbus proxy should only be created after the adapter is
enabled. This refactors the code to create the proxy object on the
OnHciEnabled callback.

As part of the refactor, a deadlock was discovered when trying to call
dbus apis from the callback. Thus, we added a new ForegroundActions
message loop so that we can trigger actions from callbacks.

Bug: 199213563
Tag: #floss
Test: Run btclient on chromeos
Change-Id: Ia7ca8af72cbed20474d8592d13e98b24dd6ce98b
parent 67ab63c6
Loading
Loading
Loading
Loading
+52 −49
Original line number Diff line number Diff line
use btstack::bluetooth::{BluetoothDevice, BluetoothTransport, IBluetooth};

use manager_service::iface_bluetooth_manager::IBluetoothManager;

use num_traits::cast::FromPrimitive;

use std::collections::HashMap;
use std::sync::{Arc, Mutex};

use crate::console_yellow;
use crate::print_info;
use crate::{BluetoothDBus, BluetoothManagerDBus, BtCallback};
use num_traits::cast::FromPrimitive;

use crate::ClientContext;
use crate::{console_red, console_yellow, print_error, print_info};
use btstack::bluetooth::{BluetoothDevice, BluetoothTransport, IBluetooth};
use manager_service::iface_bluetooth_manager::IBluetoothManager;

const INDENT_CHAR: &str = " ";
const BAR1_CHAR: &str = "=";
@@ -30,11 +27,7 @@ pub struct CommandOption {

/// Handles string command entered from command line.
pub(crate) struct CommandHandler {
    bluetooth_manager: Arc<Mutex<Box<BluetoothManagerDBus>>>,
    bluetooth: Arc<Mutex<Box<BluetoothDBus>>>,

    is_bluetooth_callback_registered: bool,

    context: Arc<Mutex<ClientContext>>,
    command_options: HashMap<String, CommandOption>,
}

@@ -118,16 +111,8 @@ fn build_commands() -> HashMap<String, CommandOption> {

impl CommandHandler {
    /// Creates a new CommandHandler.
    pub fn new(
        bluetooth_manager: Arc<Mutex<Box<BluetoothManagerDBus>>>,
        bluetooth: Arc<Mutex<Box<BluetoothDBus>>>,
    ) -> CommandHandler {
        CommandHandler {
            bluetooth_manager,
            bluetooth,
            is_bluetooth_callback_registered: false,
            command_options: build_commands(),
        }
    pub fn new(context: Arc<Mutex<ClientContext>>) -> CommandHandler {
        CommandHandler { context, command_options: build_commands() }
    }

    /// Entry point for command and arguments
@@ -145,6 +130,15 @@ impl CommandHandler {
        };
    }

    //  Common message for when the adapter isn't ready
    fn adapter_not_ready(&self) {
        let adapter_idx = self.context.lock().unwrap().default_adapter;
        print_error!(
            "Default adapter {} is not enabled. Enable the adapter before using this command.",
            adapter_idx
        );
    }

    fn cmd_help(&mut self, args: &Vec<String>) {
        if args.len() > 0 {
            match self.command_options.get(&args[0]) {
@@ -195,12 +189,13 @@ impl CommandHandler {
    }

    fn cmd_adapter(&mut self, args: &Vec<String>) {
        let default_adapter = self.context.lock().unwrap().default_adapter;
        enforce_arg_len(args, 1, "adapter <enable|disable>", || match &args[0][0..] {
            "enable" => {
                self.bluetooth_manager.lock().unwrap().start(0);
                self.context.lock().unwrap().manager_dbus.start(default_adapter);
            }
            "disable" => {
                self.bluetooth_manager.lock().unwrap().stop(0);
                self.context.lock().unwrap().manager_dbus.stop(default_adapter);
            }
            _ => {
                println!("Invalid argument '{}'", args[0]);
@@ -209,44 +204,52 @@ impl CommandHandler {
    }

    fn cmd_get_address(&mut self, _args: &Vec<String>) {
        let addr = self.bluetooth.lock().unwrap().get_address();
        print_info!("Local address = {}", addr);
        if !self.context.lock().unwrap().adapter_ready {
            self.adapter_not_ready();
            return;
        }

        let address = self.context.lock().unwrap().adapter_dbus.as_ref().unwrap().get_address();
        print_info!("Local address = {}", &address);
    }

    fn cmd_discovery(&mut self, args: &Vec<String>) {
        enforce_arg_len(args, 1, "discovery <start|stop>", || {
            match &args[0][0..] {
                "start" => {
                    // TODO: Register the BtCallback when getting a OnStateChangedCallback from btmanagerd.
                    if !self.is_bluetooth_callback_registered {
                        self.bluetooth.lock().unwrap().register_callback(Box::new(BtCallback {
                            objpath: String::from(
                                "/org/chromium/bluetooth/client/bluetooth_callback",
                            ),
                        }));
                        self.is_bluetooth_callback_registered = true;
        if !self.context.lock().unwrap().adapter_ready {
            self.adapter_not_ready();
            return;
        }
                    self.bluetooth.lock().unwrap().start_discovery();

        enforce_arg_len(args, 1, "discovery <start|stop>", || match &args[0][0..] {
            "start" => {
                self.context.lock().unwrap().adapter_dbus.as_ref().unwrap().start_discovery();
            }
            "stop" => {
                    self.bluetooth.lock().unwrap().cancel_discovery();
                self.context.lock().unwrap().adapter_dbus.as_ref().unwrap().cancel_discovery();
            }
            _ => {
                println!("Invalid argument '{}'", args[0]);
            }
            }
        });
    }

    fn cmd_bond(&mut self, args: &Vec<String>) {
        if !self.context.lock().unwrap().adapter_ready {
            self.adapter_not_ready();
            return;
        }

        enforce_arg_len(args, 1, "bond <address>", || {
            let device = BluetoothDevice {
                address: String::from(&args[0]),
                name: String::from("Classic Device"),
            };
            self.bluetooth

            self.context
                .lock()
                .unwrap()
                .adapter_dbus
                .as_ref()
                .unwrap()
                .create_bond(device, BluetoothTransport::from_i32(0).unwrap());
        });
    }
+179 −49
Original line number Diff line number Diff line
use bt_topshim::btif::BtSspVariant;
use bt_topshim::topstack;
use btstack::bluetooth::{BluetoothDevice, IBluetoothCallback};
use btstack::RPCProxy;
use std::collections::HashMap;
use std::sync::{Arc, Mutex};

use dbus::channel::MatchingReceiver;

use dbus::message::MatchRule;

use dbus::nonblock::SyncConnection;

use manager_service::iface_bluetooth_manager::{IBluetoothManager, IBluetoothManagerCallback};

use std::sync::{Arc, Mutex};
use dbus_crossroads::Crossroads;
use tokio::sync::mpsc;

use crate::command_handler::CommandHandler;
use crate::dbus_iface::{BluetoothDBus, BluetoothManagerDBus};
use crate::editor::AsyncEditor;

use dbus_crossroads::Crossroads;
use bt_topshim::btif::BtSspVariant;
use bt_topshim::topstack;
use btstack::bluetooth::{BluetoothDevice, IBluetooth, IBluetoothCallback};
use btstack::RPCProxy;
use manager_service::iface_bluetooth_manager::{IBluetoothManager, IBluetoothManagerCallback};

mod command_handler;
mod console;
@@ -25,17 +22,127 @@ mod dbus_arg;
mod dbus_iface;
mod editor;

/// Context structure for the client. Used to keep track details about the active adapter and its
/// state.
pub(crate) struct ClientContext {
    /// List of adapters and whether they are enabled.
    pub(crate) adapters: HashMap<i32, bool>,

    // TODO(abps) - Change once we have multi-adapter support.
    /// The default adapter is also the active adapter. Defaults to 0.
    pub(crate) default_adapter: i32,

    /// Current adapter is enabled?
    pub(crate) enabled: bool,

    /// Current adapter is ready to be used?
    pub(crate) adapter_ready: bool,

    /// Proxy for manager interface.
    pub(crate) manager_dbus: BluetoothManagerDBus,

    /// Proxy for adapter interface. Only exists when the default adapter is enabled.
    pub(crate) adapter_dbus: Option<BluetoothDBus>,

    /// Channel to send actions to take in the foreground
    fg: mpsc::Sender<ForegroundActions>,

    /// Internal DBus connection object.
    dbus_connection: Arc<SyncConnection>,

    /// Internal DBus crossroads object.
    dbus_crossroads: Arc<Mutex<Crossroads>>,
}

impl ClientContext {
    pub fn new(
        dbus_connection: Arc<SyncConnection>,
        dbus_crossroads: Arc<Mutex<Crossroads>>,
        tx: mpsc::Sender<ForegroundActions>,
    ) -> ClientContext {
        // Manager interface is always available but adapter interface requires
        // that the specific adapter is enabled.
        let manager_dbus =
            BluetoothManagerDBus::new(dbus_connection.clone(), dbus_crossroads.clone());

        ClientContext {
            adapters: HashMap::new(),
            default_adapter: 0,
            enabled: false,
            adapter_ready: false,
            manager_dbus,
            adapter_dbus: None,
            fg: tx,
            dbus_connection,
            dbus_crossroads,
        }
    }

    // Creates adapter proxy, registers callbacks and initializes address.
    fn create_adapter_proxy(context: Arc<Mutex<ClientContext>>, idx: &i32) {
        let conn = context.lock().unwrap().dbus_connection.clone();
        let cr = context.lock().unwrap().dbus_crossroads.clone();

        let dbus = BluetoothDBus::new(conn, cr, *idx);
        context.lock().unwrap().adapter_dbus = Some(dbus);

        // Trigger callback registration in the foreground
        let fg = context.lock().unwrap().fg.clone();
        tokio::spawn(async move {
            let objpath = String::from("/org/chromium/bluetooth/client/bluetooth_callback");
            let _ = fg.send(ForegroundActions::RegisterAdapterCallback(objpath)).await;
        });
    }
}

/// Actions to take on the foreground loop. This allows us to queue actions in
/// callbacks that get run in the foreground context.
enum ForegroundActions {
    RegisterAdapterCallback(String), // Register callbacks with this objpath
    Readline(rustyline::Result<String>), // Readline result from rustyline
}

/// Callback context for manager interface callbacks.
struct BtManagerCallback {
    objpath: String,
    context: Arc<Mutex<ClientContext>>,
}

impl IBluetoothManagerCallback for BtManagerCallback {
    fn on_hci_device_changed(&self, hci_interface: i32, present: bool) {
        print_info!("hci{} present = {}", hci_interface, present);

        if present {
            self.context.lock().unwrap().adapters.entry(hci_interface).or_insert(false);
        } else {
            self.context.lock().unwrap().adapters.remove(&hci_interface);
        }
    }

    fn on_hci_enabled_changed(&self, hci_interface: i32, enabled: bool) {
        print_info!("hci{} enabled = {}", hci_interface, enabled);

        self.context
            .lock()
            .unwrap()
            .adapters
            .entry(hci_interface)
            .and_modify(|v| *v = enabled)
            .or_insert(enabled);

        // When the default adapter's state is updated, we need to modify a few more things.
        // Only do this if we're not repeating the previous state.
        let prev_enabled = self.context.lock().unwrap().enabled;
        let default_adapter = self.context.lock().unwrap().default_adapter;
        if hci_interface == default_adapter && prev_enabled != enabled {
            self.context.lock().unwrap().enabled = enabled;
            self.context.lock().unwrap().adapter_ready = false;
            if enabled {
                ClientContext::create_adapter_proxy(self.context.clone(), &hci_interface);
            } else {
                self.context.lock().unwrap().adapter_dbus = None;
            }
        }
    }
}

@@ -47,6 +154,7 @@ impl manager_service::RPCProxy for BtManagerCallback {
    }
}

/// Callback container for adapter interface callbacks.
struct BtCallback {
    objpath: String,
}
@@ -94,22 +202,6 @@ impl RPCProxy for BtCallback {
    }
}

struct API {
    bluetooth_manager: Arc<Mutex<Box<BluetoothManagerDBus>>>,
    bluetooth: Arc<Mutex<Box<BluetoothDBus>>>,
}

// This creates the API implementations over D-Bus.
fn create_api_dbus(conn: Arc<SyncConnection>, cr: Arc<Mutex<Crossroads>>, idx: i32) -> API {
    let bluetooth_manager =
        Arc::new(Mutex::new(Box::new(BluetoothManagerDBus::new(conn.clone(), cr.clone()))));

    let bluetooth =
        Arc::new(Mutex::new(Box::new(BluetoothDBus::new(conn.clone(), cr.clone(), idx))));

    API { bluetooth_manager, bluetooth }
}

/// Runs a command line program that interacts with a Bluetooth stack.
fn main() -> Result<(), Box<dyn std::error::Error>> {
    // TODO: Process command line arguments.
@@ -142,17 +234,20 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
            }),
        );

        // We only need hci index 0 for now.
        // TODO: Have a mechanism (e.g. CLI argument or btclient command) to select the hci index.
        let api = create_api_dbus(conn, cr, 0);
        // Accept foreground actions with mpsc
        let (tx, rx) = mpsc::channel::<ForegroundActions>(10);

        // Create the context needed for handling commands
        let context = Arc::new(Mutex::new(ClientContext::new(conn, cr, tx.clone())));

        // TODO: Registering the callback should be done when btmanagerd is ready (detect with
        // ObjectManager).
        api.bluetooth_manager.lock().unwrap().register_callback(Box::new(BtManagerCallback {
        context.lock().unwrap().manager_dbus.register_callback(Box::new(BtManagerCallback {
            objpath: String::from("/org/chromium/bluetooth/client/bluetooth_manager_callback"),
            context: context.clone(),
        }));

        let mut handler = CommandHandler::new(api.bluetooth_manager.clone(), api.bluetooth.clone());
        let mut handler = CommandHandler::new(context.clone());

        let args: Vec<String> = std::env::args().collect();

@@ -160,21 +255,56 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
        if args.len() > 1 {
            handler.process_cmd_line(&args[1], &args[2..].to_vec());
        } else {
            start_interactive_shell(handler).await;
            start_interactive_shell(handler, tx, rx, context).await;
        }
        return Result::Ok(());
    })
}

async fn start_interactive_shell(mut handler: CommandHandler) {
    let editor = AsyncEditor::new(handler.get_command_list().clone());
async fn start_interactive_shell(
    mut handler: CommandHandler,
    tx: mpsc::Sender<ForegroundActions>,
    mut rx: mpsc::Receiver<ForegroundActions>,
    context: Arc<Mutex<ClientContext>>,
) {
    let command_list = handler.get_command_list().clone();

    // Async task to keep reading new lines from user
    tokio::spawn(async move {
        let editor = AsyncEditor::new(command_list);

        loop {
            let result = editor.readline().await;
        match result {
            Err(_err) => break,
            let _ = tx.send(ForegroundActions::Readline(result)).await;
        }
    });

    loop {
        let m = rx.recv().await;

        if m.is_none() {
            break;
        }

        match m.unwrap() {
            // Once adapter is ready, register callbacks, get the address and mark it as ready
            ForegroundActions::RegisterAdapterCallback(objpath) => {
                context
                    .lock()
                    .unwrap()
                    .adapter_dbus
                    .as_mut()
                    .unwrap()
                    .register_callback(Box::new(BtCallback { objpath }));
                context.lock().unwrap().adapter_ready = true;
            }
            ForegroundActions::Readline(result) => match result {
                Err(_err) => {
                    break;
                }
                Ok(line) => {
                let command_vec = line.split(" ").map(|s| String::from(s)).collect::<Vec<String>>();
                    let command_vec =
                        line.split(" ").map(|s| String::from(s)).collect::<Vec<String>>();
                    let cmd = &command_vec[0];
                    if cmd.eq("quit") {
                        break;
@@ -184,8 +314,8 @@ async fn start_interactive_shell(mut handler: CommandHandler) {
                        &command_vec[1..command_vec.len()].to_vec(),
                    );
                }
            },
        }
    }

    print_info!("Client exiting");
}