diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index d25b08e7..1589d7d1 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] +### Fixed + +- Resolve Bluetooth devices through the BlueZ adapter that owns their address + instead of assuming the adapter is `hci0`. ([#501](https://github.com/freedesktop-rs/nmrs/pull/501)) + ## [3.4.1] - 2026-07-19 ### Added diff --git a/nmrs/src/core/bluetooth.rs b/nmrs/src/core/bluetooth.rs index f39ab4f4..9d06befb 100644 --- a/nmrs/src/core/bluetooth.rs +++ b/nmrs/src/core/bluetooth.rs @@ -8,6 +8,7 @@ use log::{debug, trace}; use zbus::Connection; +use zbus::fdo::{ManagedObjects, ObjectManagerProxy}; use zvariant::OwnedObjectPath; // use futures_timer::Delay; @@ -20,7 +21,6 @@ use crate::monitoring::bluetooth::Bluetooth; use crate::monitoring::transport::ActiveTransport; use crate::types::constants::device_state; use crate::types::constants::device_type; -use crate::util::utils::bluez_device_path; use crate::util::validation::validate_bluetooth_address; use crate::{ Result, @@ -28,6 +28,56 @@ use crate::{ models::{BluetoothIdentity, TimeoutConfig}, }; +const BLUEZ_DEVICE_INTERFACE: &str = "org.bluez.Device1"; + +fn bluez_device_path_for_adapter(bdaddr: &str, adapter: &str) -> Result { + OwnedObjectPath::try_from(format!( + "/org/bluez/{adapter}/dev_{}", + bdaddr.replace(':', "_") + )) + .map_err(|error| ConnectionError::InvalidAddress(format!("Invalid BlueZ device path: {error}"))) +} + +fn find_bluez_device_path(objects: &ManagedObjects, bdaddr: &str) -> Option { + objects.iter().find_map(|(path, interfaces)| { + let properties = interfaces.get(BLUEZ_DEVICE_INTERFACE)?; + let address = <&str>::try_from(properties.get("Address")?).ok()?; + address.eq_ignore_ascii_case(bdaddr).then(|| path.clone()) + }) +} + +async fn bluez_managed_objects(conn: &Connection) -> Result { + let manager = ObjectManagerProxy::builder(conn) + .destination("org.bluez") + .map_err(|error| ConnectionError::BluezUnavailable(error.to_string()))? + .path("/") + .map_err(|error| ConnectionError::BluezUnavailable(error.to_string()))? + .build() + .await + .map_err(|error| { + ConnectionError::BluezUnavailable(format!("failed to connect to BlueZ: {error}")) + })?; + + manager.get_managed_objects().await.map_err(|error| { + ConnectionError::BluezUnavailable(format!("failed to enumerate BlueZ objects: {error}")) + }) +} + +pub(crate) async fn resolve_bluez_device_path( + conn: &Connection, + bdaddr: &str, + adapter: Option<&str>, +) -> Result { + validate_bluetooth_address(bdaddr)?; + + if let Some(adapter) = adapter { + return bluez_device_path_for_adapter(bdaddr, adapter); + } + + let objects = bluez_managed_objects(conn).await?; + find_bluez_device_path(&objects, bdaddr).ok_or(ConnectionError::NoBluetoothDevice) +} + /// Populated Bluetooth device information via BlueZ. /// /// Given a Bluetooth device address (BDADDR), this function queries BlueZ @@ -47,7 +97,16 @@ pub(crate) async fn populate_bluez_info( ) -> Result<(Option, Option)> { validate_bluetooth_address(bdaddr)?; - let bluez_path = bluez_device_path(bdaddr, adapter); + let bluez_path = match resolve_bluez_device_path(conn, bdaddr, adapter).await { + Ok(path) => path, + Err( + error @ (ConnectionError::NoBluetoothDevice | ConnectionError::BluezUnavailable(_)), + ) => { + trace!("Could not resolve BlueZ metadata path for {bdaddr}: {error}"); + return Ok((None, None)); + } + Err(error) => return Err(error), + }; match BluezDeviceExtProxy::builder(conn) .path(bluez_path)? @@ -143,11 +202,8 @@ pub(crate) async fn connect_bluetooth( // Check for saved connection let saved = get_saved_connection_path(conn, name).await?; - let specific_object = OwnedObjectPath::try_from(bluez_device_path( - &settings.bdaddr, - settings.adapter.as_deref(), - )) - .map_err(|e| ConnectionError::InvalidAddress(format!("Invalid BlueZ path: {e}")))?; + let specific_object = + resolve_bluez_device_path(conn, &settings.bdaddr, settings.adapter.as_deref()).await?; match saved { Some(saved_path) => { @@ -235,3 +291,63 @@ pub(crate) async fn disconnect_bluetooth_and_wait( Ok(()) } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use zbus::names::OwnedInterfaceName; + use zvariant::{OwnedValue, Str}; + + use super::*; + + fn managed_device( + path: &str, + address: &str, + ) -> ( + OwnedObjectPath, + HashMap>, + ) { + let properties = + HashMap::from([("Address".to_string(), OwnedValue::from(Str::from(address)))]); + let interfaces = HashMap::from([( + OwnedInterfaceName::try_from(BLUEZ_DEVICE_INTERFACE).expect("valid interface name"), + properties, + )]); + ( + OwnedObjectPath::try_from(path).expect("valid object path"), + interfaces, + ) + } + + #[test] + fn formats_path_for_explicit_adapter() { + let path = + bluez_device_path_for_adapter("00:1A:7D:DA:71:13", "hci1").expect("valid BlueZ path"); + + assert_eq!(path.as_str(), "/org/bluez/hci1/dev_00_1A_7D_DA_71_13"); + } + + #[test] + fn finds_device_path_on_matching_adapter_case_insensitively() { + let objects = HashMap::from([ + managed_device("/org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF", "AA:BB:CC:DD:EE:FF"), + managed_device("/org/bluez/hci1/dev_00_1A_7D_DA_71_13", "00:1A:7D:DA:71:13"), + ]); + + let path = + find_bluez_device_path(&objects, "00:1a:7d:da:71:13").expect("matching BlueZ device"); + + assert_eq!(path.as_str(), "/org/bluez/hci1/dev_00_1A_7D_DA_71_13"); + } + + #[test] + fn returns_none_when_bluez_device_is_absent() { + let objects = HashMap::from([managed_device( + "/org/bluez/hci0/dev_AA_BB_CC_DD_EE_FF", + "AA:BB:CC:DD:EE:FF", + )]); + + assert!(find_bluez_device_path(&objects, "00:1A:7D:DA:71:13").is_none()); + } +} diff --git a/nmrs/src/core/custom_connection.rs b/nmrs/src/core/custom_connection.rs index 4122a90d..4f99a938 100644 --- a/nmrs/src/core/custom_connection.rs +++ b/nmrs/src/core/custom_connection.rs @@ -12,11 +12,12 @@ use zvariant::{OwnedObjectPath, Value}; use crate::Result; use crate::api::models::{ConnectionError, TimeoutConfig}; +use crate::core::bluetooth::resolve_bluez_device_path; use crate::core::connection::{disconnect_wifi_and_wait, get_device_by_interface}; use crate::core::state_wait::wait_for_connection_activation; use crate::dbus::{NMDeviceProxy, NMProxy}; use crate::types::constants::device_type; -use crate::util::utils::{bluez_device_path, settings_proxy}; +use crate::util::utils::settings_proxy; fn connection_type_from_settings<'a>( settings: &'a HashMap<&str, HashMap<&str, Value<'_>>>, @@ -98,25 +99,29 @@ fn bluetooth_bdaddr_from_settings( }) } -fn resolve_specific_object( +fn explicit_specific_object(specific_object: Option<&str>) -> Result> { + specific_object + .map(|path| { + OwnedObjectPath::try_from(path).map_err(|error| ConnectionError::InvalidInput { + field: "specific_object".into(), + reason: error.to_string(), + }) + }) + .transpose() +} + +async fn resolve_specific_object( + conn: &Connection, settings: &HashMap<&str, HashMap<&str, Value<'_>>>, specific_object: Option<&str>, ) -> Result { - if let Some(path) = specific_object { - return OwnedObjectPath::try_from(path).map_err(|e| ConnectionError::InvalidInput { - field: "specific_object".into(), - reason: e.to_string(), - }); + if let Some(path) = explicit_specific_object(specific_object)? { + return Ok(path); } if connection_type_from_settings(settings)? == "bluetooth" { let bdaddr = bluetooth_bdaddr_from_settings(settings)?; - return OwnedObjectPath::try_from(bluez_device_path(&bdaddr, None)).map_err(|e| { - ConnectionError::InvalidInput { - field: "specific_object".into(), - reason: e.to_string(), - } - }); + return resolve_bluez_device_path(conn, &bdaddr, None).await; } Ok(OwnedObjectPath::default()) @@ -154,7 +159,7 @@ pub(crate) async fn add_and_activate_connection( timeout_config: TimeoutConfig, ) -> Result<(OwnedObjectPath, OwnedObjectPath)> { let device = resolve_device_path(conn, &settings, interface).await?; - let specific_object = resolve_specific_object(&settings, specific_object)?; + let specific_object = resolve_specific_object(conn, &settings, specific_object).await?; if device.as_str() != "/" { disconnect_wifi_and_wait(conn, &device, Some(timeout_config)).await?; @@ -301,25 +306,21 @@ mod tests { } #[test] - fn resolve_specific_object_defaults_to_root_path() { - let settings = sample_wifi_settings(); - let path = resolve_specific_object(&settings, None).unwrap(); - assert_eq!(path.as_str(), "/"); + fn explicit_specific_object_returns_none_when_omitted() { + assert!(explicit_specific_object(None).unwrap().is_none()); } #[test] - fn resolve_specific_object_parses_explicit_path() { - let settings = sample_wifi_settings(); - let path = - resolve_specific_object(&settings, Some("/org/freedesktop/NetworkManager/Devices/3")) - .unwrap(); + fn explicit_specific_object_parses_path() { + let path = explicit_specific_object(Some("/org/freedesktop/NetworkManager/Devices/3")) + .unwrap() + .expect("explicit path"); assert_eq!(path.as_str(), "/org/freedesktop/NetworkManager/Devices/3"); } #[test] - fn resolve_specific_object_rejects_invalid_explicit_path() { - let settings = sample_wifi_settings(); - let error = resolve_specific_object(&settings, Some("not/an/object/path")).unwrap_err(); + fn explicit_specific_object_rejects_invalid_path() { + let error = explicit_specific_object(Some("not/an/object/path")).unwrap_err(); match error { ConnectionError::InvalidInput { field, reason } => { @@ -332,22 +333,4 @@ mod tests { other => panic!("expected InvalidInput, got {other:?}"), } } - - #[test] - fn resolve_specific_object_derives_bluez_device_path() { - let settings = sample_bluetooth_settings(Some(Value::from("00:1A:7D:DA:71:13"))); - let path = resolve_specific_object(&settings, None).unwrap(); - - assert_eq!(path.as_str(), "/org/bluez/hci0/dev_00_1A_7D_DA_71_13"); - } - - #[test] - fn resolve_specific_object_requires_bluetooth_address() { - let settings = sample_bluetooth_settings(None); - assert_invalid_input( - resolve_specific_object(&settings, None).unwrap_err(), - "bluetooth.bdaddr", - "bluetooth settings are missing bdaddr", - ); - } } diff --git a/nmrs/src/util/utils.rs b/nmrs/src/util/utils.rs index d86c48fa..dc8c56a8 100644 --- a/nmrs/src/util/utils.rs +++ b/nmrs/src/util/utils.rs @@ -269,25 +269,6 @@ pub(crate) async fn extract_connection_state_reason( } } -/// Constructs a BlueZ D-Bus object path from a Bluetooth device address. -/// -/// Uses the given adapter name (e.g. `"hci0"`) or defaults to `"hci0"` -/// when `None` is provided. -/// -/// # Example -/// -/// ```ignore -/// bluez_device_path("00:1A:7D:DA:71:13", None) -/// // => "/org/bluez/hci0/dev_00_1A_7D_DA_71_13" -/// -/// bluez_device_path("00:1A:7D:DA:71:13", Some("hci1")) -/// // => "/org/bluez/hci1/dev_00_1A_7D_DA_71_13" -/// ``` -pub(crate) fn bluez_device_path(bdaddr: &str, adapter: Option<&str>) -> String { - let adapter = adapter.unwrap_or("hci0"); - format!("/org/bluez/{adapter}/dev_{}", bdaddr.replace(':', "_")) -} - /// Macro to convert Result to Option with error logging. /// Usage: `try_log!(result, "context message")?` #[macro_export] @@ -442,16 +423,4 @@ mod tests { assert_eq!(strength_or_zero(Some(100)), 100); assert_eq!(strength_or_zero(None), 0); } - - #[test] - fn test_bluez_device_path() { - assert_eq!( - bluez_device_path("00:1A:7D:DA:71:13", None), - "/org/bluez/hci0/dev_00_1A_7D_DA_71_13" - ); - assert_eq!( - bluez_device_path("00:1A:7D:DA:71:13", Some("hci1")), - "/org/bluez/hci1/dev_00_1A_7D_DA_71_13" - ) - } }