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

Commit 08abb3e4 authored by Abhishek Pandit-Subedi's avatar Abhishek Pandit-Subedi
Browse files

floss: Use devpath to determine virtual hci index 

While we depended directly on hci index for tracking adapters, we found
races with the kernel that could sometimes affect the ordering. This
refactor separates the hci index into |VirtualHciIndex| and
|RealHciIndex| so that we can keep the same virtual index even when the
real hci index changes.

Bug: 244457509
Tag: #floss
Test: Manual testing on Realtek Zork
Change-Id: Ia601e917328728350bacf5cf3600790fb707e1c1
parent 228d64bf

system/gd/rust/linux/mgmt/src/bluetooth_manager.rs

+21 −16
Original line number Diff line number Diff line
@@ -7,7 +7,9 @@ use crate::iface_bluetooth_experimental::IBluetoothExperimental; 
use crate::iface_bluetooth_manager::{

    AdapterWithEnabled, IBluetoothManager, IBluetoothManagerCallback,

};

use crate::state_machine::{state_to_enabled, AdapterState, Message, StateMachineProxy};

use crate::state_machine::{

    state_to_enabled, AdapterState, Message, StateMachineProxy, VirtualHciIndex,

};

use crate::{config_util, migrate};



const BLUEZ_INIT_TARGET: &str = "bluetoothd";
@@ -23,11 +25,11 @@ impl BluetoothManager { 
        BluetoothManager { proxy, callbacks: HashMap::new() }

    }



    fn is_adapter_enabled(&self, hci_device: i32) -> bool {

    fn is_adapter_enabled(&self, hci_device: VirtualHciIndex) -> bool {

        state_to_enabled(self.proxy.get_process_state(hci_device))

    }



    fn is_adapter_present(&self, hci_device: i32) -> bool {

    fn is_adapter_present(&self, hci_device: VirtualHciIndex) -> bool {

        self.proxy.get_state(hci_device, move |a| Some(a.present)).unwrap_or(false)

    }
@@ -68,15 +70,17 @@ impl IBluetoothManager for BluetoothManager { 
            error!("Config is not successfully modified");

        }



        let virt_hci = VirtualHciIndex(hci_interface);



        // Store that this adapter is meant to be started in state machine.

        self.proxy.modify_state(hci_interface, move |a: &mut AdapterState| a.config_enabled = true);

        self.proxy.modify_state(virt_hci, move |a: &mut AdapterState| a.config_enabled = true);



        // Ignore the request if adapter is already enabled or not present.

        if self.is_adapter_enabled(hci_interface) || !self.is_adapter_present(hci_interface) {

        if self.is_adapter_enabled(virt_hci) || !self.is_adapter_present(virt_hci) {

            return;

        }



        self.proxy.start_bluetooth(hci_interface);

        self.proxy.start_bluetooth(virt_hci);

    }



    fn stop(&mut self, hci_interface: i32) {
@@ -85,20 +89,21 @@ impl IBluetoothManager for BluetoothManager { 
            error!("Config is not successfully modified");

        }



        let virt_hci = VirtualHciIndex(hci_interface);



        // Store that this adapter is meant to be stopped in state machine.

        self.proxy

            .modify_state(hci_interface, move |a: &mut AdapterState| a.config_enabled = false);

        self.proxy.modify_state(virt_hci, move |a: &mut AdapterState| a.config_enabled = false);



        // Ignore the request if adapter is already disabled.

        if !self.is_adapter_enabled(hci_interface) {

        if !self.is_adapter_enabled(virt_hci) {

            return;

        }



        self.proxy.stop_bluetooth(hci_interface);

        self.proxy.stop_bluetooth(virt_hci);

    }



    fn get_adapter_enabled(&mut self, hci_interface: i32) -> bool {

        self.is_adapter_enabled(hci_interface)

        self.is_adapter_enabled(VirtualHciIndex(hci_interface))

    }



    fn register_callback(&mut self, mut callback: Box<dyn IBluetoothManagerCallback + Send>) {
@@ -129,13 +134,13 @@ impl IBluetoothManager for BluetoothManager { 
            migrate::migrate_bluez_devices();

            for hci in config_util::list_hci_devices() {

                if config_util::is_hci_n_enabled(hci) {

                    let _ = self.proxy.start_bluetooth(hci);

                    let _ = self.proxy.start_bluetooth(VirtualHciIndex(hci));

                }

            }

        } else if prev != enabled {

            for hci in config_util::list_hci_devices() {

                if config_util::is_hci_n_enabled(hci) {

                    let _ = self.proxy.stop_bluetooth(hci);

                    let _ = self.proxy.stop_bluetooth(VirtualHciIndex(hci));

                }

            }

            migrate::migrate_floss_devices();
@@ -150,18 +155,18 @@ impl IBluetoothManager for BluetoothManager { 
            .get_valid_adapters()

            .iter()

            .map(|a| AdapterWithEnabled {

                hci_interface: a.hci,

                hci_interface: a.virt_hci.to_i32(),

                enabled: state_to_enabled(a.state),

            })

            .collect::<Vec<AdapterWithEnabled>>()

    }



    fn get_default_adapter(&mut self) -> i32 {

        self.proxy.get_default_adapter()

        self.proxy.get_default_adapter().to_i32()

    }



    fn set_desired_default_adapter(&mut self, adapter_index: i32) {

        self.proxy.set_desired_default_adapter(adapter_index);

        self.proxy.set_desired_default_adapter(VirtualHciIndex(adapter_index));

    }

}

system/gd/rust/linux/mgmt/src/config_util.rs

+12 −0
Original line number Diff line number Diff line
@@ -159,6 +159,18 @@ pub fn check_hci_device_exists(hci: i32) -> bool { 
    Path::new(format!("{}/hci{}", HCI_DEVICES_DIR, hci).as_str()).exists()

}



/// Get the devpath for a given hci index. This gives a stable path that can be

/// used to identify a device even as the hci index fluctuates.

pub fn get_devpath_for_hci(hci: i32) -> Option<String> {

    match std::fs::canonicalize(format!("{}/hci{}/device", HCI_DEVICES_DIR, hci).as_str()) {

        Ok(p) => Some(p.into_os_string().into_string().ok()?),

        Err(e) => {

            log::debug!("Failed to get devpath for hci{} with error: {}", hci, e);

            None

        }

    }

}



fn hci_devices_string_to_int(devices: Vec<String>) -> Vec<i32> {

    devices

        .into_iter()

system/gd/rust/linux/mgmt/src/state_machine.rs

+605 −228

File changed.

Preview size limit exceeded, changes collapsed.

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

+10 −5
Original line number Diff line number Diff line
@@ -58,6 +58,13 @@ fn main() -> Result<(), Box<dyn Error>> { 
                .takes_value(true)

                .help("The HCI index"),

        )

        .arg(

            Arg::with_name("index")

                .long("index")

                .value_name("INDEX")

                .takes_value(true)

                .help("The Virtual index"),

        )

        .arg(Arg::with_name("debug").long("debug").short("d").help("Enables debug level logs"))

        .arg(Arg::from_usage("[init-flags] 'Fluoride INIT_ flags'").multiple(true))

        .arg(
@@ -73,10 +80,8 @@ fn main() -> Result<(), Box<dyn Error>> { 
    let is_debug = matches.is_present("debug");

    let log_output = matches.value_of("log-output").unwrap_or("syslog");



    let adapter_index = match matches.value_of("hci") {

        Some(idx) => idx.parse::<i32>().unwrap_or(0),

        None => 0,

    };

    let adapter_index = matches.value_of("index").map_or(0, |idx| idx.parse::<i32>().unwrap_or(0));

    let hci_index = matches.value_of("hci").map_or(0, |idx| idx.parse::<i32>().unwrap_or(0));



    // The remaining flags are passed down to Fluoride as is.

    let mut init_flags: Vec<String> = match matches.values_of("init-flags") {
@@ -90,7 +95,7 @@ fn main() -> Result<(), Box<dyn Error>> { 
    }



    // Forward --hci to Fluoride.

    init_flags.push(format!("--hci={}", adapter_index));

    init_flags.push(format!("--hci={}", hci_index));



    let level_filter = if is_debug { LevelFilter::Debug } else { LevelFilter::Info };