diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aca99db..498d4c7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,21 @@ # Changelog ## [Unreleased] +### Changed +- Core: Refactored connection monitoring from polling to event-driven D-Bus signals for faster response times and lower CPU usage ([#46](https://github.com/cachebag/nmrs/issues/46)) +- Core: Replaced `tokio` with `futures-timer` for runtime-agnostic async support (fixes GTK/glib compatibility) + +### Added +- Core: `ActiveConnectionState` and `ConnectionStateReason` enums for detailed connection status tracking ([#46](https://github.com/cachebag/nmrs/issues/46)) +- Core: `monitor_network_changes()` API for real-time network list updates via D-Bus signals +- Core: `NetworkManager` is now `Clone` + +### Fixed +- Core: `forget()` now verifies device is disconnected before deleting saved connections ([#124](https://github.com/cachebag/nmrs/issues/124)) +- Core: `list_networks()` preserves security flags when deduplicating APs ([#123](https://github.com/cachebag/nmrs/issues/123)) +- Core: Fixed race condition in signal subscription where rapid state changes could be missed +- GUI: Fixed UI freeze when connecting/forgetting networks + ## [0.4.0-beta] - 2025-12-11 ### **Breaking Changes** - **nmrs**: Expanded `ConnectionError` enum with new variants (`AuthFailed`, `SupplicantConfigFailed`, `SupplicantTimeout`, `DhcpFailed`, `Timeout`, `Stuck`, `NoWifiDevice`, `WifiNotReady`, `NoSavedConnection`, `Failed(StateReason)`) - exhaustive matches will need a wildcard ([#82](https://github.com/cachebag/nmrs/issues/82)) diff --git a/Cargo.lock b/Cargo.lock index fde8b54c..d4c844c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -449,6 +449,21 @@ dependencies = [ "winapi", ] +[[package]] +name = "futures" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + [[package]] name = "futures-channel" version = "0.3.31" @@ -456,6 +471,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2dff15bf788c671c1934e366d07e30c1814a8ef514e1af724a602e8a2fbe1b10" dependencies = [ "futures-core", + "futures-sink", ] [[package]] @@ -505,6 +521,12 @@ dependencies = [ "syn", ] +[[package]] +name = "futures-sink" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" + [[package]] name = "futures-task" version = "0.3.31" @@ -523,9 +545,13 @@ version = "0.3.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" dependencies = [ + "futures-channel", "futures-core", + "futures-io", "futures-macro", + "futures-sink", "futures-task", + "memchr", "pin-project-lite", "pin-utils", "slab", @@ -933,6 +959,7 @@ dependencies = [ name = "nmrs" version = "0.4.0" dependencies = [ + "futures", "futures-timer", "log", "serde", @@ -1479,13 +1506,13 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.18.1" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f87b8aa10b915a06587d0dec516c282ff295b475d94abf425d62b57710070a2" +checksum = "e2e054861b4bd027cd373e18e8d8d8e6548085000e41290d95ce0c373a654b4a" dependencies = [ "getrandom 0.3.3", "js-sys", - "serde", + "serde_core", "wasm-bindgen", ] diff --git a/nmrs-gui/Cargo.toml b/nmrs-gui/Cargo.toml index 5b59fe35..68f07ec8 100644 --- a/nmrs-gui/Cargo.toml +++ b/nmrs-gui/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nmrs-gui" -version = "0.4.0" +version = "0.5.0" edition = "2024" [dependencies] diff --git a/nmrs-gui/src/ui/header.rs b/nmrs-gui/src/ui/header.rs index 57b3dd23..a2dbab46 100644 --- a/nmrs-gui/src/ui/header.rs +++ b/nmrs-gui/src/ui/header.rs @@ -312,7 +312,7 @@ pub async fn refresh_networks( is_scanning.set(false); } -fn clear_children(container: >k::Box) { +pub fn clear_children(container: >k::Box) { let mut child = container.first_child(); while let Some(widget) = child { child = widget.next_sibling(); diff --git a/nmrs-gui/src/ui/mod.rs b/nmrs-gui/src/ui/mod.rs index 7cc09ad7..ced4f584 100644 --- a/nmrs-gui/src/ui/mod.rs +++ b/nmrs-gui/src/ui/mod.rs @@ -115,7 +115,7 @@ pub fn build_ui(app: &Application) { }; let ctx = Rc::new(networks::NetworksContext { - nm, + nm: nm.clone(), on_success, status: status_clone.clone(), stack: stack_clone.clone(), @@ -123,9 +123,37 @@ pub fn build_ui(app: &Application) { details_page, }); - let header = - header::build_header(ctx, &list_container_clone, is_scanning_clone, &win_clone); + let header = header::build_header( + ctx.clone(), + &list_container_clone, + is_scanning_clone.clone(), + &win_clone, + ); vbox_clone.prepend(&header); + + // TODO: Re-enable network monitoring with proper UI integration + // Currently disabled because it needs access to full context for row creation + /* + // Start background network monitoring for live updates + let nm_monitor = nm.clone(); + let list_container_monitor = list_container_clone.clone(); + let is_scanning_monitor = is_scanning_clone; + let ctx_monitor = ctx.clone(); + + glib::MainContext::default().spawn_local(async move { + let _ = nm_monitor.monitor_network_changes(move || { + let ctx = ctx_monitor.clone(); + let list_container = list_container_monitor.clone(); + let is_scanning = is_scanning_monitor.clone(); + + glib::MainContext::default().spawn_local(async move { + if !is_scanning.get() { + header::refresh_networks(ctx, &list_container, &is_scanning).await; + } + }); + }).await; + }); + */ } Err(err) => { status_clone.set_text(&format!("Failed to initialize: {err}")); diff --git a/nmrs/Cargo.toml b/nmrs/Cargo.toml index 5c5c5fb9..5d0dec61 100644 --- a/nmrs/Cargo.toml +++ b/nmrs/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nmrs" -version = "0.4.0" +version = "0.5.0" edition = "2024" description = "A Rust library for NetworkManager over D-Bus" license = "MIT" @@ -13,10 +13,11 @@ categories = ["api-bindings", "asynchronous"] log = "0.4.29" zbus = "5.11.0" zvariant = "5.7.0" -serde = { version = "1", features = ["derive"] } -thiserror = "2.0.16" -futures-timer = "3.0.3" -uuid = { version = "1.18.1", features = ["v4"] } +serde = { version = "1.0.228", features = ["derive"] } +thiserror = "2.0.17" +uuid = { version = "1.19.0", features = ["v4"] } +futures = "0.3.31" +futures-timer = "3.0" [dev-dependencies] tokio = { version = "1.48.0", features = ["rt-multi-thread", "macros", "sync", "time"] } \ No newline at end of file diff --git a/nmrs/src/connection.rs b/nmrs/src/connection.rs index b0c894b3..d3551c1d 100644 --- a/nmrs/src/connection.rs +++ b/nmrs/src/connection.rs @@ -6,11 +6,11 @@ use zvariant::OwnedObjectPath; use crate::Result; use crate::connection_settings::{delete_connection, get_saved_connection_path}; -use crate::constants::{device_state, device_type, retries, timeouts}; +use crate::constants::{device_state, device_type, timeouts}; use crate::models::{ConnectionError, ConnectionOptions, WifiSecurity}; use crate::network_info::current_ssid; use crate::proxies::{NMAccessPointProxy, NMDeviceProxy, NMProxy, NMWirelessProxy}; -use crate::state_wait::wait_for_connection_state; +use crate::state_wait::{wait_for_connection_activation, wait_for_device_disconnect}; use crate::utils::decode_ssid_or_empty; use crate::wifi_builders::build_wifi_connection; @@ -76,14 +76,10 @@ pub(crate) async fn connect(conn: &Connection, ssid: &str, creds: WifiSecurity) build_and_activate_new(conn, &nm, &wifi_device, &specific_object, ssid, creds).await?; } } - let dev_proxy = NMDeviceProxy::builder(conn) - .path(wifi_device.clone())? - .build() - .await?; - debug!("Waiting for connection to complete..."); - wait_for_connection_state(&dev_proxy).await?; - info!("Connection request for '{ssid}' submitted successfully"); + // Connection activation is now handled within connect_via_saved() and + // build_and_activate_new() using signal-based monitoring + info!("Successfully connected to '{ssid}'"); Ok(()) } @@ -128,38 +124,22 @@ pub(crate) async fn forget(conn: &Connection, ssid: &str) -> Result<()> { && decode_ssid_or_empty(&bytes) == ssid { debug!("Disconnecting from active network: {ssid}"); - let dev_proxy: zbus::Proxy<'_> = zbus::proxy::Builder::new(conn) - .destination("org.freedesktop.NetworkManager")? - .path(dev_path.clone())? - .interface("org.freedesktop.NetworkManager.Device")? - .build() - .await?; - - match dev_proxy.call_method("Disconnect", &()).await { - Ok(_) => debug!("Disconnect call succeeded"), - Err(e) => warn!("Disconnect call failed: {e}"), - } - - debug!("About to enter wait loop..."); - for i in 0..retries::FORGET_MAX_RETRIES { - Delay::new(timeouts::forget_poll_interval()).await; - match dev.state().await { - Ok(current_state) => { - debug!("Wait loop {i}: device state = {current_state}"); - if current_state == device_state::DISCONNECTED - || current_state == device_state::UNAVAILABLE - { - debug!("Device reached disconnected state"); - break; - } - } - Err(e) => { - warn!("Failed to get device state in wait loop {i}: {e}"); - break; - } + if let Err(e) = disconnect_wifi_and_wait(conn, dev_path).await { + warn!("Disconnect wait failed: {e}"); + let final_state = dev.state().await?; + if final_state != device_state::DISCONNECTED + && final_state != device_state::UNAVAILABLE + { + error!( + "Device still connected (state: {final_state}), cannot safely delete" + ); + return Err(ConnectionError::Stuck(format!( + "disconnect failed, device in state {final_state}" + ))); } + debug!("Device confirmed disconnected, proceeding with deletion"); } - debug!("Wait loop completed"); + debug!("Disconnect phase completed"); } } } @@ -249,9 +229,11 @@ pub(crate) async fn forget(conn: &Connection, ssid: &str) -> Result<()> { /// Disconnects a Wi-Fi device and waits for it to reach disconnected state. /// -/// Calls the Disconnect method on the device and polls until the device -/// state becomes Disconnected or Unavailable, or the retry limit is reached. -pub(crate) async fn disconnect_wifi_device( +/// Calls the Disconnect method on the device and waits for the `StateChanged` +/// signal to indicate the device has reached Disconnected or Unavailable state. +/// This is more efficient than polling and responds immediately when the +/// device disconnects. +pub(crate) async fn disconnect_wifi_and_wait( conn: &Connection, dev_path: &OwnedObjectPath, ) -> Result<()> { @@ -260,6 +242,13 @@ pub(crate) async fn disconnect_wifi_device( .build() .await?; + // Check if already disconnected + let current_state = dev.state().await?; + if current_state == device_state::DISCONNECTED || current_state == device_state::UNAVAILABLE { + debug!("Device already disconnected"); + return Ok(()); + } + let raw: zbus::proxy::Proxy = zbus::proxy::Builder::new(conn) .destination("org.freedesktop.NetworkManager")? .path(dev_path.clone())? @@ -267,26 +256,16 @@ pub(crate) async fn disconnect_wifi_device( .build() .await?; + debug!("Sending disconnect request"); let _ = raw.call_method("Disconnect", &()).await; - for _ in 0..retries::DISCONNECT_MAX_RETRIES { - Delay::new(timeouts::disconnect_poll_interval()).await; - match dev.state().await { - Ok(s) if s == device_state::DISCONNECTED || s == device_state::UNAVAILABLE => { - break; - } - Ok(_) => continue, - Err(e) => return Err(e.into()), - } - } + // Wait for disconnect using signal-based monitoring + wait_for_device_disconnect(&dev).await?; - Delay::new(timeouts::disconnect_final_delay()).await; + // Brief stabilization delay + Delay::new(timeouts::stabilization_delay()).await; - match dev.state().await { - Ok(s) if s == device_state::DISCONNECTED || s == device_state::UNAVAILABLE => Ok(()), - Ok(s) => Err(ConnectionError::Stuck(format!("{s}"))), - Err(e) => Err(e.into()), - } + Ok(()) } /// Finds the first Wi-Fi device on the system. @@ -355,7 +334,7 @@ async fn ensure_disconnected( } } - disconnect_wifi_device(conn, wifi_device).await?; + disconnect_wifi_and_wait(conn, wifi_device).await?; } Ok(()) @@ -363,10 +342,10 @@ async fn ensure_disconnected( /// Attempts to connect using a saved connection profile. /// -/// Activates the saved connection. If activation succeeds but the device -/// ends up in a disconnected state (indicating invalid saved settings), -/// deletes the saved connection and creates a fresh one with the provided -/// credentials. +/// Activates the saved connection and monitors the activation state using +/// D-Bus signals. If activation fails (device disconnects or enters failed +/// state), deletes the saved connection and creates a fresh one with the +/// provided credentials. /// /// This handles cases where saved passwords are outdated or corrupted. async fn connect_via_saved( @@ -389,38 +368,38 @@ async fn connect_via_saved( active_conn.as_str() ); - Delay::new(timeouts::disconnect_final_delay()).await; - - let dev_check = NMDeviceProxy::builder(conn) - .path(wifi_device.clone())? - .build() - .await?; - - let check_state = dev_check.state().await?; - - if check_state == device_state::DISCONNECTED { - warn!("Connection activated but device stuck in Disconnected state"); - warn!("Saved connection has invalid settings, deleting and retrying"); - - let _ = nm.deactivate_connection(active_conn).await; - - let _ = delete_connection(conn, saved.clone()).await; - - let opts = ConnectionOptions { - autoconnect: true, - autoconnect_priority: None, - autoconnect_retries: None, - }; - - let settings = build_wifi_connection(ap.as_str(), creds, &opts); - - debug!("Creating fresh connection with corrected settings"); - nm.add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) - .await - .map_err(|e| { - error!("Fresh connection also failed: {e}"); - e - })?; + // Wait for connection activation using signal-based monitoring + match wait_for_connection_activation(conn, &active_conn).await { + Ok(()) => { + debug!("Saved connection activated successfully"); + } + Err(e) => { + warn!("Saved connection activation failed: {e}"); + warn!("Deleting saved connection and retrying with fresh credentials"); + + let _ = nm.deactivate_connection(active_conn).await; + let _ = delete_connection(conn, saved.clone()).await; + + let opts = ConnectionOptions { + autoconnect: true, + autoconnect_priority: None, + autoconnect_retries: None, + }; + + let settings = build_wifi_connection(ap.as_str(), creds, &opts); + + debug!("Creating fresh connection with corrected settings"); + let (_, new_active_conn) = nm + .add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) + .await + .map_err(|e| { + error!("Fresh connection also failed: {e}"); + e + })?; + + // Wait for the fresh connection to activate + wait_for_connection_activation(conn, &new_active_conn).await?; + } } } @@ -438,12 +417,16 @@ async fn connect_via_saved( let settings = build_wifi_connection(ap.as_str(), creds, &opts); - nm.add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) + let (_, active_conn) = nm + .add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) .await .map_err(|e| { error!("Fresh connection also failed: {e}"); e })?; + + // Wait for the fresh connection to activate + wait_for_connection_activation(conn, &active_conn).await?; } } @@ -454,7 +437,8 @@ async fn connect_via_saved( /// /// Builds connection settings from the provided credentials, ensures the /// device is disconnected, then calls AddAndActivateConnection to create -/// and activate the connection in one step. +/// and activate the connection in one step. Monitors activation using +/// D-Bus signals for immediate feedback on success or failure. async fn build_and_activate_new( conn: &Connection, nm: &NMProxy<'_>, @@ -475,38 +459,39 @@ async fn build_and_activate_new( ensure_disconnected(conn, nm, wifi_device).await?; - match nm + let (_, active_conn) = match nm .add_and_activate_connection(settings, wifi_device.clone(), ap.clone()) .await { - Ok(_) => debug!("add_and_activate_connection() succeeded"), + Ok(paths) => { + debug!( + "add_and_activate_connection() succeeded, active connection: {}", + paths.1.as_str() + ); + paths + } Err(e) => { error!("add_and_activate_connection() failed: {e}"); return Err(e.into()); } - } - - Delay::new(timeouts::disconnect_poll_interval()).await; + }; - let dev_proxy = NMDeviceProxy::builder(conn) - .path(wifi_device.clone())? - .build() - .await?; + debug!("Waiting for connection activation using signal monitoring..."); - let initial_state = dev_proxy.state().await?; - debug!("Dev state after build_and_activate_new(): {initial_state:?}"); - debug!("Waiting for connection to complete..."); - wait_for_connection_state(&dev_proxy).await?; + // Wait for connection activation using the ActiveConnection signals + wait_for_connection_activation(conn, &active_conn).await?; - info!("Connection request for '{ssid}' submitted successfully"); + info!("Connection to '{ssid}' activated successfully"); Ok(()) } /// Triggers a Wi-Fi scan and finds the target access point. /// -/// Requests a scan, waits for it to complete, then searches for an -/// access point matching the target SSID. +/// Requests a scan, waits briefly for results, then searches for an +/// access point matching the target SSID. The wait time is shorter than +/// polling-based approaches since we just need the scan to populate +/// initial results. async fn scan_and_resolve_ap( conn: &Connection, wifi: &NMWirelessProxy<'_>, @@ -517,6 +502,7 @@ async fn scan_and_resolve_ap( Err(e) => warn!("Scan request failed: {e}"), } + // Brief wait for scan results to populate Delay::new(timeouts::scan_wait()).await; debug!("Scan wait complete"); diff --git a/nmrs/src/constants.rs b/nmrs/src/constants.rs index 15992e78..1bc06df6 100644 --- a/nmrs/src/constants.rs +++ b/nmrs/src/constants.rs @@ -13,14 +13,9 @@ pub mod device_type { /// NetworkManager device state constants pub mod device_state { - // pub const UNMANAGED: u32 = 10; pub const UNAVAILABLE: u32 = 20; pub const DISCONNECTED: u32 = 30; - // pub const PREPARE: u32 = 40; - pub const CONFIG: u32 = 50; pub const ACTIVATED: u32 = 100; - // pub const DEACTIVATING: u32 = 110; - pub const FAILED: u32 = 120; } /// WiFi security flag constants @@ -37,47 +32,43 @@ pub mod wifi_mode { pub const AP: u32 = 3; } -/// Timeout and delay constants (in milliseconds) +/// Timeout constants for signal-based waiting. +/// +/// These timeouts are used with D-Bus signal monitoring instead of polling. +/// They define how long to wait for state transitions before giving up. pub mod timeouts { use std::time::Duration; - pub const DISCONNECT_POLL_INTERVAL_MS: u64 = 300; - pub const DISCONNECT_FINAL_DELAY_MS: u64 = 500; - pub const CONNECTION_POLL_INTERVAL_MS: u64 = 500; - pub const SCAN_WAIT_SECONDS: u64 = 3; - pub const FORGET_POLL_INTERVAL_MS: u64 = 200; + /// Maximum time to wait for Wi-Fi device to become ready (60 seconds). + /// + /// Used after enabling Wi-Fi to wait for the hardware to initialize. + const WIFI_READY_TIMEOUT_SECS: u64 = 60; - pub fn disconnect_poll_interval() -> Duration { - Duration::from_millis(DISCONNECT_POLL_INTERVAL_MS) - } + /// Time to wait after requesting a scan before checking results (2 seconds). + /// + /// While we could use signals for scan completion, a short delay is + /// sufficient for now, and simpler for most use cases. + const SCAN_WAIT_SECS: u64 = 2; - pub fn disconnect_final_delay() -> Duration { - Duration::from_millis(DISCONNECT_FINAL_DELAY_MS) - } + /// Brief delay after state transitions to allow NetworkManager to stabilize. + const STABILIZATION_DELAY_MS: u64 = 100; - pub fn connection_poll_interval() -> Duration { - Duration::from_millis(CONNECTION_POLL_INTERVAL_MS) + /// Returns the Wi-Fi ready timeout duration. + pub fn wifi_ready_timeout() -> Duration { + Duration::from_secs(WIFI_READY_TIMEOUT_SECS) } + /// Returns the scan wait duration. pub fn scan_wait() -> Duration { - Duration::from_secs(SCAN_WAIT_SECONDS) + Duration::from_secs(SCAN_WAIT_SECS) } - pub fn forget_poll_interval() -> Duration { - Duration::from_millis(FORGET_POLL_INTERVAL_MS) + /// Returns a brief stabilization delay. + pub fn stabilization_delay() -> Duration { + Duration::from_millis(STABILIZATION_DELAY_MS) } } -/// Retry count constants -pub mod retries { - pub const DISCONNECT_MAX_RETRIES: u32 = 10; - pub const CONNECTION_MAX_RETRIES: u32 = 40; - pub const CONNECTION_CONFIG_STUCK_THRESHOLD: u32 = 15; - pub const CONNECTION_STUCK_CHECK_START: u32 = 10; - pub const FORGET_MAX_RETRIES: u32 = 20; - pub const WIFI_READY_MAX_RETRIES: u32 = 20; -} - /// Signal strength thresholds for bar display pub mod signal_strength { pub const BAR_1_MAX: u8 = 24; diff --git a/nmrs/src/device.rs b/nmrs/src/device.rs index 8b7f1e5a..0fe92e2a 100644 --- a/nmrs/src/device.rs +++ b/nmrs/src/device.rs @@ -1,14 +1,17 @@ //! Network device enumeration and control. //! //! Provides functions for listing network devices, checking Wi-Fi state, -//! and enabling/disabling Wi-Fi. +//! and enabling/disabling Wi-Fi. Uses D-Bus signals for efficient state +//! monitoring instead of polling. +use log::debug; use zbus::Connection; use crate::Result; -use crate::constants::{retries, timeouts}; -use crate::models::{ConnectionError, Device, DeviceState, DeviceType}; +use crate::constants::device_type; +use crate::models::{ConnectionError, Device, DeviceState}; use crate::proxies::{NMDeviceProxy, NMProxy}; +use crate::state_wait::wait_for_wifi_device_ready; /// Lists all network devices managed by NetworkManager. /// @@ -47,25 +50,43 @@ pub(crate) async fn list_devices(conn: &Connection) -> Result> { /// Waits for a Wi-Fi device to become ready for operations. /// -/// Polls until a Wi-Fi device reaches either Disconnected or Activated state, -/// indicating it's ready for scanning or connection operations. This is useful -/// after enabling Wi-Fi, as the device may take time to initialize. +/// Uses D-Bus signals to efficiently wait until a Wi-Fi device reaches +/// either Disconnected or Activated state, indicating it's ready for +/// scanning or connection operations. This is useful after enabling Wi-Fi, +/// as the device may take time to initialize. /// /// Returns `WifiNotReady` if no Wi-Fi device becomes ready within the timeout. pub(crate) async fn wait_for_wifi_ready(conn: &Connection) -> Result<()> { - for _ in 0..retries::WIFI_READY_MAX_RETRIES { - let devices = list_devices(conn).await?; - for dev in devices { - if dev.device_type == DeviceType::Wifi - && (dev.state == DeviceState::Disconnected || dev.state == DeviceState::Activated) - { - return Ok(()); - } + let nm = NMProxy::new(conn).await?; + let devices = nm.get_devices().await?; + + // Find the Wi-Fi device + for dev_path in devices { + let dev = NMDeviceProxy::builder(conn) + .path(dev_path.clone())? + .build() + .await?; + + if dev.device_type().await? != device_type::WIFI { + continue; } - futures_timer::Delay::new(timeouts::scan_wait()).await; + + debug!("Found Wi-Fi device, waiting for it to become ready"); + + // Check current state first + let current_state = dev.state().await?; + let state = DeviceState::from(current_state); + + if state == DeviceState::Disconnected || state == DeviceState::Activated { + debug!("Wi-Fi device already ready"); + return Ok(()); + } + + // Wait for device to become ready using signal-based monitoring + return wait_for_wifi_device_ready(&dev).await; } - Err(ConnectionError::WifiNotReady) + Err(ConnectionError::NoWifiDevice) } /// Enables or disables Wi-Fi globally. diff --git a/nmrs/src/lib.rs b/nmrs/src/lib.rs index ada0659e..3adc1c83 100644 --- a/nmrs/src/lib.rs +++ b/nmrs/src/lib.rs @@ -35,6 +35,17 @@ //! specific variants for common failures like authentication errors, timeouts, //! and missing devices. //! +//! # Signal-Based State Monitoring +//! +//! This crate uses D-Bus signals for efficient state monitoring instead of polling. +//! When connecting to a network, the library subscribes to NetworkManager's +//! `StateChanged` signals to detect connection success or failure immediately, +//! rather than polling device state in a loop. This provides: +//! +//! - Faster response times (immediate notification vs polling delay) +//! - Lower CPU usage (no spinning loops) +//! - Better error messages with specific failure reasons +//! //! # Logging //! //! This crate uses the [`log`](https://docs.rs/log) facade for logging. To see @@ -51,6 +62,7 @@ mod connection_settings; mod constants; mod device; mod network_info; +mod network_monitor; mod proxies; mod scan; mod state_wait; @@ -63,8 +75,9 @@ pub mod wifi_builders; // Re-exported public API pub use models::{ - ConnectionError, ConnectionOptions, Device, DeviceState, DeviceType, EapMethod, EapOptions, - Network, NetworkInfo, Phase2, StateReason, WifiSecurity, reason_to_error, + ActiveConnectionState, ConnectionError, ConnectionOptions, ConnectionStateReason, Device, + DeviceState, DeviceType, EapMethod, EapOptions, Network, NetworkInfo, Phase2, StateReason, + WifiSecurity, connection_state_reason_to_error, reason_to_error, }; pub use network_manager::NetworkManager; diff --git a/nmrs/src/models.rs b/nmrs/src/models.rs index 3ce203a0..a0f266b3 100644 --- a/nmrs/src/models.rs +++ b/nmrs/src/models.rs @@ -2,6 +2,164 @@ use serde::{Deserialize, Serialize}; use std::fmt::{Display, Formatter}; use thiserror::Error; +/// NetworkManager active connection state. +/// +/// These values represent the lifecycle states of an active connection +/// as reported by the NM D-Bus API. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ActiveConnectionState { + /// Connection state is unknown. + Unknown, + /// Connection is activating (connecting). + Activating, + /// Connection is fully activated (connected). + Activated, + /// Connection is deactivating (disconnecting). + Deactivating, + /// Connection is fully deactivated (disconnected). + Deactivated, + /// Unknown state code not mapped to a specific variant. + Other(u32), +} + +impl From for ActiveConnectionState { + fn from(code: u32) -> Self { + match code { + 0 => Self::Unknown, + 1 => Self::Activating, + 2 => Self::Activated, + 3 => Self::Deactivating, + 4 => Self::Deactivated, + v => Self::Other(v), + } + } +} + +impl Display for ActiveConnectionState { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::Unknown => write!(f, "unknown"), + Self::Activating => write!(f, "activating"), + Self::Activated => write!(f, "activated"), + Self::Deactivating => write!(f, "deactivating"), + Self::Deactivated => write!(f, "deactivated"), + Self::Other(v) => write!(f, "unknown state ({v})"), + } + } +} + +/// NetworkManager active connection state reason codes. +/// +/// These values indicate why an active connection transitioned to its +/// current state. Use `ConnectionStateReason::from(code)` to convert +/// from the raw u32 values returned by NetworkManager signals. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConnectionStateReason { + /// The reason is unknown. + Unknown, + /// No specific reason. + None, + /// User disconnected. + UserDisconnected, + /// Device disconnected. + DeviceDisconnected, + /// The NetworkManager service stopped. + ServiceStopped, + /// IP configuration was invalid. + IpConfigInvalid, + /// Connection timed out while activating. + ConnectTimeout, + /// Service start timed out. + ServiceStartTimeout, + /// Service failed to start. + ServiceStartFailed, + /// No secrets (password) were provided. + NoSecrets, + /// Login/authentication failed. + LoginFailed, + /// The connection was removed. + ConnectionRemoved, + /// A dependency failed. + DependencyFailed, + /// Device realization failed. + DeviceRealizeFailed, + /// Device was removed. + DeviceRemoved, + /// Unknown reason code not mapped to a specific variant. + Other(u32), +} + +impl From for ConnectionStateReason { + fn from(code: u32) -> Self { + match code { + 0 => Self::Unknown, + 1 => Self::None, + 2 => Self::UserDisconnected, + 3 => Self::DeviceDisconnected, + 4 => Self::ServiceStopped, + 5 => Self::IpConfigInvalid, + 6 => Self::ConnectTimeout, + 7 => Self::ServiceStartTimeout, + 8 => Self::ServiceStartFailed, + 9 => Self::NoSecrets, + 10 => Self::LoginFailed, + 11 => Self::ConnectionRemoved, + 12 => Self::DependencyFailed, + 13 => Self::DeviceRealizeFailed, + 14 => Self::DeviceRemoved, + v => Self::Other(v), + } + } +} + +impl Display for ConnectionStateReason { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::Unknown => write!(f, "unknown"), + Self::None => write!(f, "none"), + Self::UserDisconnected => write!(f, "user disconnected"), + Self::DeviceDisconnected => write!(f, "device disconnected"), + Self::ServiceStopped => write!(f, "service stopped"), + Self::IpConfigInvalid => write!(f, "IP configuration invalid"), + Self::ConnectTimeout => write!(f, "connection timed out"), + Self::ServiceStartTimeout => write!(f, "service start timed out"), + Self::ServiceStartFailed => write!(f, "service start failed"), + Self::NoSecrets => write!(f, "no secrets (password) provided"), + Self::LoginFailed => write!(f, "login/authentication failed"), + Self::ConnectionRemoved => write!(f, "connection was removed"), + Self::DependencyFailed => write!(f, "dependency failed"), + Self::DeviceRealizeFailed => write!(f, "device realization failed"), + Self::DeviceRemoved => write!(f, "device was removed"), + Self::Other(v) => write!(f, "unknown reason ({v})"), + } + } +} + +/// Converts a connection state reason code to a specific `ConnectionError`. +/// +/// Maps authentication-related failures to `AuthFailed`, timeout issues to `Timeout`, +/// and other failures to the appropriate variant. +pub fn connection_state_reason_to_error(code: u32) -> ConnectionError { + let reason = ConnectionStateReason::from(code); + match reason { + // Authentication failures + ConnectionStateReason::NoSecrets | ConnectionStateReason::LoginFailed => { + ConnectionError::AuthFailed + } + + // Timeout failures + ConnectionStateReason::ConnectTimeout | ConnectionStateReason::ServiceStartTimeout => { + ConnectionError::Timeout + } + + // IP configuration failures (often DHCP) + ConnectionStateReason::IpConfigInvalid => ConnectionError::DhcpFailed, + + // All other failures + _ => ConnectionError::ConnectionFailed(reason), + } +} + /// NetworkManager device state reason codes. /// /// These values come from the NM D-Bus API and indicate why a device @@ -200,10 +358,14 @@ pub enum ConnectionError { #[error("no saved connection for network")] NoSavedConnection, - /// A general connection failure with a reason code from NetworkManager. + /// A general connection failure with a device state reason code. #[error("connection failed: {0}")] Failed(StateReason), + /// A connection activation failure with a connection state reason. + #[error("connection activation failed: {0}")] + ConnectionFailed(ConnectionStateReason), + /// Invalid UTF-8 encountered in SSID. #[error("invalid UTF-8 in SSID: {0}")] InvalidUtf8(#[from] std::str::Utf8Error), @@ -425,6 +587,20 @@ impl WifiSecurity { } } +impl Network { + pub fn merge_ap(&mut self, other: &Network) { + if other.strength.unwrap_or(0) > self.strength.unwrap_or(0) { + self.strength = other.strength; + self.frequency = other.frequency; + self.bssid = other.bssid.clone(); + } + + self.secured |= other.secured; + self.is_psk |= other.is_psk; + self.is_eap |= other.is_eap; + } +} + #[cfg(test)] mod tests { use super::*; @@ -623,4 +799,165 @@ mod tests { "connection failed: carrier changed" ); } + + #[test] + fn active_connection_state_from_u32() { + assert_eq!( + ActiveConnectionState::from(0), + ActiveConnectionState::Unknown + ); + assert_eq!( + ActiveConnectionState::from(1), + ActiveConnectionState::Activating + ); + assert_eq!( + ActiveConnectionState::from(2), + ActiveConnectionState::Activated + ); + assert_eq!( + ActiveConnectionState::from(3), + ActiveConnectionState::Deactivating + ); + assert_eq!( + ActiveConnectionState::from(4), + ActiveConnectionState::Deactivated + ); + assert_eq!( + ActiveConnectionState::from(99), + ActiveConnectionState::Other(99) + ); + } + + #[test] + fn active_connection_state_display() { + assert_eq!(format!("{}", ActiveConnectionState::Unknown), "unknown"); + assert_eq!( + format!("{}", ActiveConnectionState::Activating), + "activating" + ); + assert_eq!(format!("{}", ActiveConnectionState::Activated), "activated"); + assert_eq!( + format!("{}", ActiveConnectionState::Deactivating), + "deactivating" + ); + assert_eq!( + format!("{}", ActiveConnectionState::Deactivated), + "deactivated" + ); + assert_eq!( + format!("{}", ActiveConnectionState::Other(42)), + "unknown state (42)" + ); + } + + #[test] + fn connection_state_reason_from_u32() { + assert_eq!( + ConnectionStateReason::from(0), + ConnectionStateReason::Unknown + ); + assert_eq!(ConnectionStateReason::from(1), ConnectionStateReason::None); + assert_eq!( + ConnectionStateReason::from(2), + ConnectionStateReason::UserDisconnected + ); + assert_eq!( + ConnectionStateReason::from(3), + ConnectionStateReason::DeviceDisconnected + ); + assert_eq!( + ConnectionStateReason::from(6), + ConnectionStateReason::ConnectTimeout + ); + assert_eq!( + ConnectionStateReason::from(9), + ConnectionStateReason::NoSecrets + ); + assert_eq!( + ConnectionStateReason::from(10), + ConnectionStateReason::LoginFailed + ); + assert_eq!( + ConnectionStateReason::from(99), + ConnectionStateReason::Other(99) + ); + } + + #[test] + fn connection_state_reason_display() { + assert_eq!(format!("{}", ConnectionStateReason::Unknown), "unknown"); + assert_eq!( + format!("{}", ConnectionStateReason::NoSecrets), + "no secrets (password) provided" + ); + assert_eq!( + format!("{}", ConnectionStateReason::LoginFailed), + "login/authentication failed" + ); + assert_eq!( + format!("{}", ConnectionStateReason::ConnectTimeout), + "connection timed out" + ); + assert_eq!( + format!("{}", ConnectionStateReason::Other(123)), + "unknown reason (123)" + ); + } + + #[test] + fn connection_state_reason_to_error_auth_failures() { + // NoSecrets and LoginFailed map to AuthFailed + assert!(matches!( + connection_state_reason_to_error(9), + ConnectionError::AuthFailed + )); + assert!(matches!( + connection_state_reason_to_error(10), + ConnectionError::AuthFailed + )); + } + + #[test] + fn connection_state_reason_to_error_timeout() { + // ConnectTimeout and ServiceStartTimeout map to Timeout + assert!(matches!( + connection_state_reason_to_error(6), + ConnectionError::Timeout + )); + assert!(matches!( + connection_state_reason_to_error(7), + ConnectionError::Timeout + )); + } + + #[test] + fn connection_state_reason_to_error_dhcp() { + // IpConfigInvalid maps to DhcpFailed + assert!(matches!( + connection_state_reason_to_error(5), + ConnectionError::DhcpFailed + )); + } + + #[test] + fn connection_state_reason_to_error_generic() { + // Other reasons map to ConnectionFailed + match connection_state_reason_to_error(2) { + ConnectionError::ConnectionFailed(reason) => { + assert_eq!(reason, ConnectionStateReason::UserDisconnected); + } + _ => panic!("expected ConnectionError::ConnectionFailed"), + } + } + + #[test] + fn connection_failed_error_display() { + assert_eq!( + format!( + "{}", + ConnectionError::ConnectionFailed(ConnectionStateReason::NoSecrets) + ), + "connection activation failed: no secrets (password) provided" + ); + } } diff --git a/nmrs/src/network_manager.rs b/nmrs/src/network_manager.rs index 60e0a8c1..68e27809 100644 --- a/nmrs/src/network_manager.rs +++ b/nmrs/src/network_manager.rs @@ -6,12 +6,14 @@ use crate::connection_settings::{get_saved_connection_path, has_saved_connection use crate::device::{list_devices, set_wifi_enabled, wait_for_wifi_ready, wifi_enabled}; use crate::models::{Device, Network, NetworkInfo, WifiSecurity}; use crate::network_info::{current_connection_info, current_ssid, show_details}; +use crate::network_monitor; use crate::scan::{list_networks, scan_networks}; /// High-level interface to NetworkManager over D-Bus. /// /// Provides methods for listing devices, scanning networks, connecting, /// and managing saved connections. +#[derive(Clone)] pub struct NetworkManager { conn: Connection, } @@ -98,4 +100,39 @@ impl NetworkManager { pub async fn forget(&self, ssid: &str) -> Result<()> { forget(&self.conn, ssid).await } + + /// Monitors Wi-Fi network changes in real-time. + /// + /// Subscribes to D-Bus signals for access point additions and removals + /// on all Wi-Fi devices. Invokes the callback whenever the network list + /// changes, enabling live UI updates without polling. + /// + /// This function runs indefinitely until an error occurs. Run it in a + /// background task. + /// + /// # Example + /// + /// ```no_run + /// # use nmrs::NetworkManager; + /// # async fn example() -> nmrs::Result<()> { + /// let nm = NetworkManager::new().await?; + /// + /// // Spawn monitoring task + /// glib::MainContext::default().spawn_local({ + /// let nm = nm.clone(); + /// async move { + /// nm.monitor_network_changes(|| { + /// println!("Networks changed!"); + /// }).await + /// } + /// }); + /// # Ok(()) + /// # } + /// ``` + pub async fn monitor_network_changes(&self, callback: F) -> Result<()> + where + F: Fn() + 'static, + { + network_monitor::monitor_network_changes(&self.conn, callback).await + } } diff --git a/nmrs/src/network_monitor.rs b/nmrs/src/network_monitor.rs new file mode 100644 index 00000000..c856f9a5 --- /dev/null +++ b/nmrs/src/network_monitor.rs @@ -0,0 +1,89 @@ +//! Real-time network monitoring using D-Bus signals. +//! +//! Provides functionality to monitor access point changes (additions/removals) +//! in real-time without needing to poll. This enables live UI updates. + +use futures::stream::{Stream, StreamExt}; +use log::{debug, warn}; +use std::pin::Pin; +use zbus::Connection; + +use crate::Result; +use crate::constants::device_type; +use crate::models::ConnectionError; +use crate::proxies::{NMDeviceProxy, NMProxy, NMWirelessProxy}; + +/// Monitors access point changes on all Wi-Fi devices. +/// +/// Subscribes to `AccessPointAdded` and `AccessPointRemoved` signals on all +/// wireless devices. When any signal is received, invokes the callback to +/// notify the caller that the network list has changed. +/// +/// This function runs indefinitely until an error occurs or the connection +/// is lost. Run it in a background task. +/// +/// # Example +/// +/// ```ignore +/// let nm = NetworkManager::new().await?; +/// nm.monitor_network_changes(|| { +/// println!("Network list changed, refresh UI!"); +/// }).await?; +/// ``` +pub async fn monitor_network_changes(conn: &Connection, callback: F) -> Result<()> +where + F: Fn() + 'static, +{ + let nm = NMProxy::new(conn).await?; + let devices = nm.get_devices().await?; + + // Use dynamic dispatch to handle different signal stream types + let mut streams: Vec>>> = Vec::new(); + + // Subscribe to signals on all Wi-Fi devices + for dev_path in devices { + let dev = NMDeviceProxy::builder(conn) + .path(dev_path.clone())? + .build() + .await?; + + if dev.device_type().await? != device_type::WIFI { + continue; + } + + let wifi = NMWirelessProxy::builder(conn) + .path(dev_path.clone())? + .build() + .await?; + + let added_stream = wifi.receive_access_point_added().await?; + let removed_stream = wifi.receive_access_point_removed().await?; + + // Box both streams as trait objects + streams.push(Box::pin(added_stream.map(|_| ()))); + streams.push(Box::pin(removed_stream.map(|_| ()))); + + debug!("Subscribed to AP signals on device: {dev_path}"); + } + + if streams.is_empty() { + warn!("No Wi-Fi devices found to monitor"); + return Err(ConnectionError::NoWifiDevice); + } + + debug!( + "Monitoring {} signal streams for network changes", + streams.len() + ); + + // Merge all streams and listen for any signal + let mut merged = futures::stream::select_all(streams); + + while let Some(_signal) = merged.next().await { + debug!("Network change detected"); + callback(); + } + + warn!("Network monitoring stream ended unexpectedly"); + Err(ConnectionError::Stuck("monitoring stream ended".into())) +} diff --git a/nmrs/src/proxies.rs b/nmrs/src/proxies.rs deleted file mode 100644 index ee731532..00000000 --- a/nmrs/src/proxies.rs +++ /dev/null @@ -1,175 +0,0 @@ -//! D-Bus proxy traits for NetworkManager interfaces. -//! -//! These traits define the NetworkManager D-Bus API surface used by this crate. -//! The `zbus::proxy` macro generates proxy implementations that handle -//! D-Bus communication automatically. -//! -//! # NetworkManager D-Bus Structure -//! -//! - `/org/freedesktop/NetworkManager` - Main NM object -//! - `/org/freedesktop/NetworkManager/Devices/*` - Device objects -//! - `/org/freedesktop/NetworkManager/AccessPoint/*` - Access point objects -//! - `/org/freedesktop/NetworkManager/Settings` - Connection settings - -use std::collections::HashMap; -use zbus::{Result, proxy}; -use zvariant::OwnedObjectPath; - -/// Proxy for NetworkManager device interface. -/// -/// Provides access to device properties like interface name, type, state, -/// and the reason for state transitions. -#[proxy( - interface = "org.freedesktop.NetworkManager.Device", - default_service = "org.freedesktop.NetworkManager" -)] -pub trait NMDevice { - /// The network interface name (e.g., "wlan0"). - #[zbus(property)] - fn interface(&self) -> Result; - - /// Device type as a numeric code (2 = Wi-Fi). - #[zbus(property)] - fn device_type(&self) -> Result; - - /// Current device state (100 = activated, 120 = failed). - #[zbus(property)] - fn state(&self) -> Result; - - /// Whether NetworkManager manages this device. - #[zbus(property)] - fn managed(&self) -> Result; - - /// The kernel driver in use. - #[zbus(property)] - fn driver(&self) -> Result; - - /// Current state and reason code for the last state change. - #[zbus(property)] - fn state_reason(&self) -> Result<(u32, u32)>; -} - -/// Proxy for the main NetworkManager interface. -/// -/// Provides methods for listing devices, managing connections, -/// and controlling Wi-Fi state. -#[proxy( - interface = "org.freedesktop.NetworkManager", - default_service = "org.freedesktop.NetworkManager", - default_path = "/org/freedesktop/NetworkManager" -)] -pub trait NM { - /// Returns paths to all network devices. - fn get_devices(&self) -> zbus::Result>; - - /// Whether Wi-Fi is globally enabled. - #[zbus(property)] - fn wireless_enabled(&self) -> zbus::Result; - - /// Enable or disable Wi-Fi globally. - #[zbus(property)] - fn set_wireless_enabled(&self, value: bool) -> zbus::Result<()>; - - /// Paths to all active connections. - #[zbus(property)] - fn active_connections(&self) -> zbus::Result>; - - /// Creates a new connection and activates it simultaneously. - /// - /// Returns paths to both the new connection settings and active connection. - fn add_and_activate_connection( - &self, - connection: HashMap<&str, HashMap<&str, zvariant::Value<'_>>>, - device: OwnedObjectPath, - specific_object: OwnedObjectPath, - ) -> zbus::Result<(OwnedObjectPath, OwnedObjectPath)>; - - /// Activates an existing saved connection. - fn activate_connection( - &self, - connection: OwnedObjectPath, - device: OwnedObjectPath, - specific_object: OwnedObjectPath, - ) -> zbus::Result; - - /// Deactivates an active connection. - fn deactivate_connection(&self, active_connection: OwnedObjectPath) -> zbus::Result<()>; -} - -/// Proxy for wireless device interface. -/// -/// Extends the base device interface with Wi-Fi specific functionality -/// like scanning and access point enumeration. -#[proxy( - interface = "org.freedesktop.NetworkManager.Device.Wireless", - default_service = "org.freedesktop.NetworkManager" -)] -pub trait NMWireless { - /// Returns paths to all visible access points. - fn get_all_access_points(&self) -> Result>; - - /// Requests a Wi-Fi scan. Options are usually empty. - fn request_scan(&self, options: HashMap>) -> Result<()>; - - /// Signal emitted when a new access point is discovered. - #[zbus(signal)] - fn access_point_added(&self, path: OwnedObjectPath); - - /// Signal emitted when an access point is no longer visible. - #[zbus(signal)] - fn access_point_removed(&self, path: OwnedObjectPath); - - /// Path to the currently connected access point ("/" if none). - #[zbus(property)] - fn active_access_point(&self) -> Result; - - /// Current connection bitrate in Kbit/s. - #[zbus(property)] - fn bitrate(&self) -> Result; -} - -/// Proxy for access point interface. -/// -/// Provides information about a visible Wi-Fi network including -/// SSID, signal strength, security capabilities, and frequency. -#[proxy( - interface = "org.freedesktop.NetworkManager.AccessPoint", - default_service = "org.freedesktop.NetworkManager" -)] -pub trait NMAccessPoint { - /// SSID as raw bytes (may not be valid UTF-8). - #[zbus(property)] - fn ssid(&self) -> Result>; - - /// Signal strength as percentage (0-100). - #[zbus(property)] - fn strength(&self) -> Result; - - /// BSSID (MAC address) of the access point. - #[zbus(property)] - fn hw_address(&self) -> Result; - - /// General capability flags (bit 0 = privacy/WEP). - #[zbus(property)] - fn flags(&self) -> Result; - - /// WPA security flags (PSK, EAP, etc.). - #[zbus(property)] - fn wpa_flags(&self) -> Result; - - /// RSN/WPA2 security flags. - #[zbus(property)] - fn rsn_flags(&self) -> Result; - - /// Operating frequency in MHz. - #[zbus(property)] - fn frequency(&self) -> Result; - - /// Maximum supported bitrate in Kbit/s. - #[zbus(property)] - fn max_bitrate(&self) -> Result; - - /// Wi-Fi mode (1 = adhoc, 2 = infrastructure, 3 = AP). - #[zbus(property)] - fn mode(&self) -> Result; -} diff --git a/nmrs/src/proxies/access_point.rs b/nmrs/src/proxies/access_point.rs new file mode 100644 index 00000000..b17580b8 --- /dev/null +++ b/nmrs/src/proxies/access_point.rs @@ -0,0 +1,49 @@ +//! NetworkManager Access Point proxy. + +use zbus::{Result, proxy}; + +/// Proxy for access point interface. +/// +/// Provides information about a visible Wi-Fi network including +/// SSID, signal strength, security capabilities, and frequency. +#[proxy( + interface = "org.freedesktop.NetworkManager.AccessPoint", + default_service = "org.freedesktop.NetworkManager" +)] +pub trait NMAccessPoint { + /// SSID as raw bytes (may not be valid UTF-8). + #[zbus(property)] + fn ssid(&self) -> Result>; + + /// Signal strength as percentage (0-100). + #[zbus(property)] + fn strength(&self) -> Result; + + /// BSSID (MAC address) of the access point. + #[zbus(property)] + fn hw_address(&self) -> Result; + + /// General capability flags (bit 0 = privacy/WEP). + #[zbus(property)] + fn flags(&self) -> Result; + + /// WPA security flags (PSK, EAP, etc.). + #[zbus(property)] + fn wpa_flags(&self) -> Result; + + /// RSN/WPA2 security flags. + #[zbus(property)] + fn rsn_flags(&self) -> Result; + + /// Operating frequency in MHz. + #[zbus(property)] + fn frequency(&self) -> Result; + + /// Maximum supported bitrate in Kbit/s. + #[zbus(property)] + fn max_bitrate(&self) -> Result; + + /// Wi-Fi mode (1 = adhoc, 2 = infrastructure, 3 = AP). + #[zbus(property)] + fn mode(&self) -> Result; +} diff --git a/nmrs/src/proxies/active_connection.rs b/nmrs/src/proxies/active_connection.rs new file mode 100644 index 00000000..a98d3bab --- /dev/null +++ b/nmrs/src/proxies/active_connection.rs @@ -0,0 +1,75 @@ +//! NetworkManager Active Connection proxy. + +use zbus::{Result, proxy}; +use zvariant::OwnedObjectPath; + +/// Proxy for active connection interface. +/// +/// Provides access to the state of an active (in-progress or established) +/// network connection. Use this to monitor connection activation progress +/// and detect failures with specific reason codes. +/// +/// # Signals +/// +/// The `StateChanged` signal is emitted when the connection activation state +/// changes. Use `receive_activation_state_changed()` to get a stream of state changes: +/// +/// ```ignore +/// let mut stream = active_conn_proxy.receive_activation_state_changed().await?; +/// while let Some(signal) = stream.next().await { +/// let args = signal.args()?; +/// match args.state { +/// 2 => println!("Connection activated!"), +/// 4 => println!("Connection failed: reason {}", args.reason), +/// _ => {} +/// } +/// } +/// ``` +#[proxy( + interface = "org.freedesktop.NetworkManager.Connection.Active", + default_service = "org.freedesktop.NetworkManager" +)] +pub trait NMActiveConnection { + /// Current state of the active connection. + /// + /// Values: + /// - 0: Unknown + /// - 1: Activating + /// - 2: Activated + /// - 3: Deactivating + /// - 4: Deactivated + #[zbus(property)] + fn state(&self) -> Result; + + /// Path to the connection settings used for this connection. + #[zbus(property)] + fn connection(&self) -> Result; + + /// Path to the specific object (e.g., access point) used for this connection. + #[zbus(property)] + fn specific_object(&self) -> Result; + + /// Connection identifier (usually the SSID for Wi-Fi). + #[zbus(property)] + fn id(&self) -> Result; + + /// Connection UUID. + #[zbus(property)] + fn uuid(&self) -> Result; + + /// Paths to devices using this connection. + #[zbus(property)] + fn devices(&self) -> Result>; + + /// Signal emitted when the connection activation state changes. + /// + /// The method is named `activation_state_changed` to avoid conflicts with + /// the `state` property's change stream. Use `receive_activation_state_changed()` + /// to subscribe to this signal. + /// + /// Arguments: + /// - `state`: The new connection state (see `ActiveConnectionState`) + /// - `reason`: The reason for the state change (see `ConnectionStateReason`) + #[zbus(signal, name = "StateChanged")] + fn activation_state_changed(&self, state: u32, reason: u32); +} diff --git a/nmrs/src/proxies/device.rs b/nmrs/src/proxies/device.rs new file mode 100644 index 00000000..dab79cc5 --- /dev/null +++ b/nmrs/src/proxies/device.rs @@ -0,0 +1,64 @@ +//! NetworkManager Device proxy. + +use zbus::{Result, proxy}; + +/// Proxy for NetworkManager device interface. +/// +/// Provides access to device properties like interface name, type, state, +/// and the reason for state transitions. +/// +/// # Signals +/// +/// The `StateChanged` signal is emitted whenever the device state changes. +/// Use `receive_device_state_changed()` to get a stream of state change events: +/// +/// ```ignore +/// let mut stream = device_proxy.receive_device_state_changed().await?; +/// while let Some(signal) = stream.next().await { +/// let args = signal.args()?; +/// println!("New state: {}, Old state: {}, Reason: {}", +/// args.new_state, args.old_state, args.reason); +/// } +/// ``` +#[proxy( + interface = "org.freedesktop.NetworkManager.Device", + default_service = "org.freedesktop.NetworkManager" +)] +pub trait NMDevice { + /// The network interface name (e.g., "wlan0"). + #[zbus(property)] + fn interface(&self) -> Result; + + /// Device type as a numeric code (2 = Wi-Fi). + #[zbus(property)] + fn device_type(&self) -> Result; + + /// Current device state (100 = activated, 120 = failed). + #[zbus(property)] + fn state(&self) -> Result; + + /// Whether NetworkManager manages this device. + #[zbus(property)] + fn managed(&self) -> Result; + + /// The kernel driver in use. + #[zbus(property)] + fn driver(&self) -> Result; + + /// Current state and reason code for the last state change. + #[zbus(property)] + fn state_reason(&self) -> Result<(u32, u32)>; + + /// Signal emitted when device state changes. + /// + /// The method is named `device_state_changed` to avoid conflicts with the + /// `state` property's change stream. Use `receive_device_state_changed()` + /// to subscribe to this signal. + /// + /// Arguments: + /// - `new_state`: The new device state code + /// - `old_state`: The previous device state code + /// - `reason`: The reason code for the state change + #[zbus(signal, name = "StateChanged")] + fn device_state_changed(&self, new_state: u32, old_state: u32, reason: u32); +} diff --git a/nmrs/src/proxies/main_nm.rs b/nmrs/src/proxies/main_nm.rs new file mode 100644 index 00000000..677a79b1 --- /dev/null +++ b/nmrs/src/proxies/main_nm.rs @@ -0,0 +1,52 @@ +//! Main NetworkManager proxy. + +use std::collections::HashMap; +use zbus::proxy; +use zvariant::OwnedObjectPath; + +/// Proxy for the main NetworkManager interface. +/// +/// Provides methods for listing devices, managing connections, +/// and controlling Wi-Fi state. +#[proxy( + interface = "org.freedesktop.NetworkManager", + default_service = "org.freedesktop.NetworkManager", + default_path = "/org/freedesktop/NetworkManager" +)] +pub trait NM { + /// Returns paths to all network devices. + fn get_devices(&self) -> zbus::Result>; + + /// Whether Wi-Fi is globally enabled. + #[zbus(property)] + fn wireless_enabled(&self) -> zbus::Result; + + /// Enable or disable Wi-Fi globally. + #[zbus(property)] + fn set_wireless_enabled(&self, value: bool) -> zbus::Result<()>; + + /// Paths to all active connections. + #[zbus(property)] + fn active_connections(&self) -> zbus::Result>; + + /// Creates a new connection and activates it simultaneously. + /// + /// Returns paths to both the new connection settings and active connection. + fn add_and_activate_connection( + &self, + connection: HashMap<&str, HashMap<&str, zvariant::Value<'_>>>, + device: OwnedObjectPath, + specific_object: OwnedObjectPath, + ) -> zbus::Result<(OwnedObjectPath, OwnedObjectPath)>; + + /// Activates an existing saved connection. + fn activate_connection( + &self, + connection: OwnedObjectPath, + device: OwnedObjectPath, + specific_object: OwnedObjectPath, + ) -> zbus::Result; + + /// Deactivates an active connection. + fn deactivate_connection(&self, active_connection: OwnedObjectPath) -> zbus::Result<()>; +} diff --git a/nmrs/src/proxies/mod.rs b/nmrs/src/proxies/mod.rs new file mode 100644 index 00000000..0a526b2b --- /dev/null +++ b/nmrs/src/proxies/mod.rs @@ -0,0 +1,34 @@ +//! D-Bus proxy traits for NetworkManager interfaces. +//! +//! These traits define the NetworkManager D-Bus API surface used by this crate. +//! The `zbus::proxy` macro generates proxy implementations that handle +//! D-Bus communication automatically. +//! +//! # NetworkManager D-Bus Structure +//! +//! - `/org/freedesktop/NetworkManager` - Main NM object +//! - `/org/freedesktop/NetworkManager/Devices/*` - Device objects +//! - `/org/freedesktop/NetworkManager/AccessPoint/*` - Access point objects +//! - `/org/freedesktop/NetworkManager/ActiveConnection/*` - Active connection objects +//! - `/org/freedesktop/NetworkManager/Settings` - Connection settings +//! +//! # Signal-based State Monitoring +//! +//! This crate uses D-Bus signals for efficient state monitoring instead of polling: +//! - `NMDevice::StateChanged` - Emitted when device state changes +//! - `NMActiveConnection::StateChanged` - Emitted when connection activation state changes +//! +//! Use the generated `receive_device_state_changed()` and `receive_activation_state_changed()` +//! methods to get signal streams. + +mod access_point; +mod active_connection; +mod device; +mod main_nm; +mod wireless; + +pub use access_point::NMAccessPointProxy; +pub use active_connection::NMActiveConnectionProxy; +pub use device::NMDeviceProxy; +pub use main_nm::NMProxy; +pub use wireless::NMWirelessProxy; diff --git a/nmrs/src/proxies/wireless.rs b/nmrs/src/proxies/wireless.rs new file mode 100644 index 00000000..f3d43c3c --- /dev/null +++ b/nmrs/src/proxies/wireless.rs @@ -0,0 +1,37 @@ +//! NetworkManager Wireless Device proxy. + +use std::collections::HashMap; +use zbus::{Result, proxy}; +use zvariant::OwnedObjectPath; + +/// Proxy for wireless device interface. +/// +/// Extends the base device interface with Wi-Fi specific functionality +/// like scanning and access point enumeration. +#[proxy( + interface = "org.freedesktop.NetworkManager.Device.Wireless", + default_service = "org.freedesktop.NetworkManager" +)] +pub trait NMWireless { + /// Returns paths to all visible access points. + fn get_all_access_points(&self) -> Result>; + + /// Requests a Wi-Fi scan. Options are usually empty. + fn request_scan(&self, options: HashMap>) -> Result<()>; + + /// Signal emitted when a new access point is discovered. + #[zbus(signal)] + fn access_point_added(&self, path: OwnedObjectPath); + + /// Signal emitted when an access point is no longer visible. + #[zbus(signal)] + fn access_point_removed(&self, path: OwnedObjectPath); + + /// Path to the currently connected access point ("/" if none). + #[zbus(property)] + fn active_access_point(&self) -> Result; + + /// Current connection bitrate in Kbit/s. + #[zbus(property)] + fn bitrate(&self) -> Result; +} diff --git a/nmrs/src/scan.rs b/nmrs/src/scan.rs index 7d8d33c4..409fe91e 100644 --- a/nmrs/src/scan.rs +++ b/nmrs/src/scan.rs @@ -86,17 +86,9 @@ pub(crate) async fn list_networks(conn: &Connection) -> Result> { // Deduplicate: use (SSID, frequency) as key, keep strongest signal for (ssid, frequency, new_net) in all_networks { - let strength = new_net.strength.unwrap_or(0); networks .entry((ssid, frequency)) - .and_modify(|n| { - if strength > n.strength.unwrap_or(0) { - *n = new_net.clone(); - } - if new_net.secured { - n.secured = true; - } - }) + .and_modify(|n| n.merge_ap(&new_net)) .or_insert(new_net); } diff --git a/nmrs/src/state_wait.rs b/nmrs/src/state_wait.rs index e277a7e0..d9af1c65 100644 --- a/nmrs/src/state_wait.rs +++ b/nmrs/src/state_wait.rs @@ -1,51 +1,236 @@ -//! Connection state monitoring. +//! Connection state monitoring using D-Bus signals. //! -//! Provides functions to wait for device state transitions during -//! connection establishment, with proper error mapping for failures. +//! Provides functions to wait for device and connection state transitions +//! using NetworkManager's signal-based API instead of polling. This approach +//! is more efficient and provides faster response times. +//! +//! # Signal-Based Monitoring +//! +//! Instead of polling device state in a loop, these functions subscribe to +//! D-Bus signals that NetworkManager emits when state changes occur: +//! +//! - `NMDevice.StateChanged` - Emitted when device state changes +//! - `NMActiveConnection.StateChanged` - Emitted when connection activation state changes +//! +//! This provides a few benefits: +//! - Immediate response to state changes (no polling delay) +//! - Lower CPU usage (no spinning loops) +//! - More reliable; at least in the sense that we won't miss rapid state transitions. +//! - Better error messages with specific failure reasons +use futures::{FutureExt, StreamExt, select}; use futures_timer::Delay; +use log::{debug, warn}; +use std::pin::pin; +use std::time::Duration; +use zbus::Connection; use crate::Result; -use crate::constants::{device_state, retries, timeouts}; -use crate::models::{ConnectionError, StateReason, reason_to_error}; -use crate::proxies::NMDeviceProxy; +use crate::constants::{device_state, timeouts}; +use crate::models::{ + ActiveConnectionState, ConnectionError, ConnectionStateReason, connection_state_reason_to_error, +}; +use crate::proxies::{NMActiveConnectionProxy, NMDeviceProxy}; -/// Waits for a device to reach the activated state or fail. +/// Default timeout for connection activation (30 seconds). +const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30); + +/// Default timeout for device disconnection (10 seconds). +const DISCONNECT_TIMEOUT: Duration = Duration::from_secs(10); + +/// Waits for an active connection to reach the activated state. /// -/// Polls the device state until activation succeeds, fails, or times out. -/// Returns a structured error indicating the specific failure reason. -pub(crate) async fn wait_for_connection_state(dev: &NMDeviceProxy<'_>) -> Result<()> { - let mut config_stuck = 0; +/// Monitors the connection activation process by subscribing to the +/// `StateChanged` signal on the active connection object. This provides +/// more detailed error information than device-level monitoring. +pub(crate) async fn wait_for_connection_activation( + conn: &Connection, + active_conn_path: &zvariant::OwnedObjectPath, +) -> Result<()> { + let active_conn = NMActiveConnectionProxy::builder(conn) + .path(active_conn_path.clone())? + .build() + .await?; - for i in 0..retries::CONNECTION_MAX_RETRIES { - Delay::new(timeouts::connection_poll_interval()).await; + // Subscribe to signals FIRST to avoid race condition + let mut stream = active_conn.receive_activation_state_changed().await?; + debug!("Subscribed to ActiveConnection StateChanged signal"); - let raw = dev.state().await?; + // Check current state - if already terminal, return immediately + let current_state = active_conn.state().await?; + let state = ActiveConnectionState::from(current_state); + debug!("Current active connection state: {state}"); - if raw == device_state::ACTIVATED { + match state { + ActiveConnectionState::Activated => { + debug!("Connection already activated"); return Ok(()); } - - if raw == device_state::FAILED { - return Err(match dev.state_reason().await { - Ok((_, code)) => reason_to_error(code), - Err(_) => ConnectionError::Failed(StateReason::Unknown), - }); + ActiveConnectionState::Deactivated => { + warn!("Connection already deactivated"); + return Err(ConnectionError::ConnectionFailed( + ConnectionStateReason::Unknown, + )); } + _ => {} + } - if raw == device_state::CONFIG { - config_stuck += 1; - if config_stuck > retries::CONNECTION_CONFIG_STUCK_THRESHOLD { - return Err(ConnectionError::Stuck("config".into())); + // Wait for state change with timeout (runtime-agnostic) + let mut timeout_delay = pin!(Delay::new(CONNECTION_TIMEOUT).fuse()); + + loop { + select! { + _ = timeout_delay => { + warn!("Connection activation timed out after {:?}", CONNECTION_TIMEOUT); + return Err(ConnectionError::Timeout); + } + signal_opt = stream.next() => { + match signal_opt { + Some(signal) => { + match signal.args() { + Ok(args) => { + let new_state = ActiveConnectionState::from(args.state); + let reason = ConnectionStateReason::from(args.reason); + debug!("Active connection state changed to: {new_state} (reason: {reason})"); + + match new_state { + ActiveConnectionState::Activated => { + debug!("Connection activation successful"); + return Ok(()); + } + ActiveConnectionState::Deactivated => { + debug!("Connection activation failed: {reason}"); + return Err(connection_state_reason_to_error(args.reason)); + } + _ => {} + } + } + Err(e) => { + warn!("Failed to parse StateChanged signal args: {e}"); + } + } + } + None => { + return Err(ConnectionError::Stuck("signal stream ended".into())); + } + } } - } else { - config_stuck = 0; } + } +} + +/// Waits for a device to reach the disconnected state using D-Bus signals. +pub(crate) async fn wait_for_device_disconnect(dev: &NMDeviceProxy<'_>) -> Result<()> { + // Subscribe to signals FIRST to avoid race condition + let mut stream = dev.receive_device_state_changed().await?; + debug!("Subscribed to device StateChanged signal for disconnect"); + + let current_state = dev.state().await?; + debug!("Current device state for disconnect: {current_state}"); + + if current_state == device_state::DISCONNECTED || current_state == device_state::UNAVAILABLE { + debug!("Device already disconnected"); + return Ok(()); + } + + // Wait for disconnect with timeout (runtime-agnostic) + let mut timeout_delay = pin!(Delay::new(DISCONNECT_TIMEOUT).fuse()); + + loop { + select! { + _ = timeout_delay => { + // Check final state - might have reached target during the last moments + let final_state = dev.state().await?; + if final_state == device_state::DISCONNECTED || final_state == device_state::UNAVAILABLE { + return Ok(()); + } else { + warn!("Disconnect timed out, device still in state: {final_state}"); + return Err(ConnectionError::Stuck(format!("state {final_state}"))); + } + } + signal_opt = stream.next() => { + match signal_opt { + Some(signal) => { + match signal.args() { + Ok(args) => { + let new_state = args.new_state; + debug!("Device state during disconnect: {new_state}"); - if i > retries::CONNECTION_STUCK_CHECK_START && raw == device_state::DISCONNECTED { - return Err(ConnectionError::Stuck("disconnected".into())); + if new_state == device_state::DISCONNECTED + || new_state == device_state::UNAVAILABLE + { + debug!("Device reached disconnected state"); + return Ok(()); + } + } + Err(e) => { + warn!("Failed to parse StateChanged signal args: {e}"); + } + } + } + None => { + return Err(ConnectionError::Stuck("signal stream ended".into())); + } + } + } } } +} + +/// Waits for a Wi-Fi device to be ready (Disconnected or Activated state). +pub(crate) async fn wait_for_wifi_device_ready(dev: &NMDeviceProxy<'_>) -> Result<()> { + // Subscribe to signals FIRST to avoid race condition + let mut stream = dev.receive_device_state_changed().await?; + debug!("Subscribed to device StateChanged signal for ready check"); + + let current_state = dev.state().await?; + debug!("Current device state for ready check: {current_state}"); + + if current_state == device_state::DISCONNECTED || current_state == device_state::ACTIVATED { + debug!("Device already ready"); + return Ok(()); + } + + let ready_timeout = timeouts::wifi_ready_timeout(); + let mut timeout_delay = pin!(Delay::new(ready_timeout).fuse()); + + loop { + select! { + _ = timeout_delay => { + // Check final state + let final_state = dev.state().await?; + if final_state == device_state::DISCONNECTED || final_state == device_state::ACTIVATED { + return Ok(()); + } else { + warn!("Wi-Fi device not ready after timeout, state: {final_state}"); + return Err(ConnectionError::WifiNotReady); + } + } + signal_opt = stream.next() => { + match signal_opt { + Some(signal) => { + match signal.args() { + Ok(args) => { + let new_state = args.new_state; + debug!("Device state during ready wait: {new_state}"); - Err(ConnectionError::Timeout) + if new_state == device_state::DISCONNECTED + || new_state == device_state::ACTIVATED + { + debug!("Device is now ready"); + return Ok(()); + } + } + Err(e) => { + warn!("Failed to parse StateChanged signal args: {e}"); + } + } + } + None => { + return Err(ConnectionError::WifiNotReady); + } + } + } + } + } } diff --git a/package.nix b/package.nix index 76e6c730..fb354e3b 100644 --- a/package.nix +++ b/package.nix @@ -19,7 +19,7 @@ rustPlatform.buildRustPackage { src = ./.; - cargoHash = "sha256-Z538+q/Af7nshS8G8mPV7aGfTd1XiGKjYljNf9FW3HA="; + cargoHash = "sha256-vTNK0VmmrpJRqQ50OvYyrhH7ygbTrS1d6u/228pglvE="; nativeBuildInputs = [ pkg-config