From 180566e5f172186f6b82cb94c7af5e24312ae955 Mon Sep 17 00:00:00 2001 From: Denis Benato Date: Wed, 22 Oct 2025 22:05:17 +0200 Subject: [PATCH] Fix: share a single HID device Avoid opening multiple handles to the same device whenever possible. --- asusd/src/asus_armoury.rs | 2 +- asusd/src/aura_anime/mod.rs | 4 +- asusd/src/aura_anime/trait_impls.rs | 14 +++--- asusd/src/aura_laptop/mod.rs | 2 +- asusd/src/aura_laptop/trait_impls.rs | 4 +- asusd/src/aura_manager.rs | 74 +++++++++++++++++++++++----- asusd/src/aura_scsi/mod.rs | 2 +- asusd/src/aura_scsi/trait_impls.rs | 2 +- asusd/src/aura_slash/mod.rs | 2 +- asusd/src/aura_types.rs | 2 +- asusd/src/ctrl_backlight.rs | 4 +- asusd/src/ctrl_fancurves.rs | 2 +- asusd/src/ctrl_platform.rs | 2 +- asusd/src/daemon.rs | 2 +- 14 files changed, 84 insertions(+), 34 deletions(-) diff --git a/asusd/src/asus_armoury.rs b/asusd/src/asus_armoury.rs index 776bcd42..0862d406 100644 --- a/asusd/src/asus_armoury.rs +++ b/asusd/src/asus_armoury.rs @@ -1,12 +1,12 @@ use std::sync::Arc; use config_traits::StdConfig; -use futures_util::lock::Mutex; use log::{debug, error, info}; use rog_platform::asus_armoury::{AttrValue, Attribute, FirmwareAttribute, FirmwareAttributes}; use rog_platform::platform::{PlatformProfile, RogPlatform}; use rog_platform::power::AsusPower; use serde::{Deserialize, Serialize}; +use tokio::sync::Mutex; use zbus::object_server::SignalEmitter; use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue, Type, Value}; use zbus::{fdo, interface, Connection}; diff --git a/asusd/src/aura_anime/mod.rs b/asusd/src/aura_anime/mod.rs index b0ca07da..6d1d498e 100644 --- a/asusd/src/aura_anime/mod.rs +++ b/asusd/src/aura_anime/mod.rs @@ -8,7 +8,6 @@ use std::sync::Arc; use std::thread::sleep; use config_traits::StdConfig; -use futures_util::lock::Mutex; use log::{debug, error, info, warn}; use rog_anime::usb::{ pkt_flush, pkt_set_brightness, pkt_set_enable_display, pkt_set_enable_powersave_anim, @@ -17,6 +16,7 @@ use rog_anime::usb::{ use rog_anime::{ActionData, AnimeDataBuffer, AnimePacketType}; use rog_platform::hid_raw::HidRaw; use rog_platform::usb_raw::USBRaw; +use tokio::sync::Mutex; use self::config::{AniMeConfig, AniMeConfigCached}; use crate::error::RogError; @@ -51,7 +51,7 @@ impl AniMe { /// Will fail if something is already holding the config lock async fn do_init_cache(&mut self) { - if let Some(mut config) = self.config.try_lock() { + if let Ok(mut config) = self.config.try_lock() { if let Err(e) = self.cache.init_from_config(&config, config.anime_type) { error!( "Trying to cache the Anime Config failed, will reset to default config: {e:?}" diff --git a/asusd/src/aura_anime/trait_impls.rs b/asusd/src/aura_anime/trait_impls.rs index 2e5828c4..c6d4ec0e 100644 --- a/asusd/src/aura_anime/trait_impls.rs +++ b/asusd/src/aura_anime/trait_impls.rs @@ -85,7 +85,7 @@ impl AniMeZbus { /// Set base brightness level #[zbus(property)] async fn brightness(&self) -> Brightness { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.display_brightness; } Brightness::Off @@ -117,7 +117,7 @@ impl AniMeZbus { #[zbus(property)] async fn builtins_enabled(&self) -> bool { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.builtin_anims_enabled; } false @@ -162,7 +162,7 @@ impl AniMeZbus { #[zbus(property)] async fn builtin_animations(&self) -> Animations { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.builtin_anims; } Animations::default() @@ -195,7 +195,7 @@ impl AniMeZbus { #[zbus(property)] async fn enable_display(&self) -> bool { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.display_enabled; } false @@ -218,7 +218,7 @@ impl AniMeZbus { #[zbus(property)] async fn off_when_unplugged(&self) -> bool { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.off_when_unplugged; } false @@ -245,7 +245,7 @@ impl AniMeZbus { #[zbus(property)] async fn off_when_suspended(&self) -> bool { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.off_when_suspended; } false @@ -261,7 +261,7 @@ impl AniMeZbus { #[zbus(property)] async fn off_when_lid_closed(&self) -> bool { - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { return config.off_when_lid_closed; } false diff --git a/asusd/src/aura_laptop/mod.rs b/asusd/src/aura_laptop/mod.rs index 90e55547..7fabceba 100644 --- a/asusd/src/aura_laptop/mod.rs +++ b/asusd/src/aura_laptop/mod.rs @@ -2,13 +2,13 @@ use std::sync::Arc; use config::AuraConfig; use config_traits::StdConfig; -use futures_util::lock::{Mutex, MutexGuard}; use log::info; use rog_aura::keyboard::{AuraLaptopUsbPackets, LedUsbPackets}; use rog_aura::usb::{AURA_LAPTOP_LED_APPLY, AURA_LAPTOP_LED_SET}; use rog_aura::{AuraDeviceType, AuraEffect, LedBrightness, PowerZones, AURA_LAPTOP_LED_MSG_LEN}; use rog_platform::hid_raw::HidRaw; use rog_platform::keyboard_led::KeyboardBacklight; +use tokio::sync::{Mutex, MutexGuard}; use crate::error::RogError; diff --git a/asusd/src/aura_laptop/trait_impls.rs b/asusd/src/aura_laptop/trait_impls.rs index 877fde33..eee5b88f 100644 --- a/asusd/src/aura_laptop/trait_impls.rs +++ b/asusd/src/aura_laptop/trait_impls.rs @@ -112,7 +112,7 @@ impl AuraZbus { // entirely possible to deadlock here, so use try instead of lock() // let ctrl = self.0.lock().await; // Ok(config.current_mode) - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { Ok(config.current_mode) } else { Err(ZbErr::Failed("Aura control couldn't lock self".to_string())) @@ -140,7 +140,7 @@ impl AuraZbus { #[zbus(property)] async fn led_mode_data(&self) -> Result { // entirely possible to deadlock here, so use try instead of lock() - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { let mode = config.current_mode; match config.builtins.get(&mode) { Some(effect) => Ok(effect.clone()), diff --git a/asusd/src/aura_manager.rs b/asusd/src/aura_manager.rs index 38a7c608..e520458c 100644 --- a/asusd/src/aura_manager.rs +++ b/asusd/src/aura_manager.rs @@ -4,15 +4,16 @@ // - Add it to Zbus server // - If udev sees device removed then remove the zbus path +use std::collections::HashMap; use std::sync::Arc; use dmi_id::DMIID; use futures_lite::future::block_on; -use futures_util::lock::Mutex; use log::{debug, error, info, warn}; use mio::{Events, Interest, Poll, Token}; use rog_platform::error::PlatformError; use rog_platform::hid_raw::HidRaw; +use tokio::sync::Mutex; use udev::{Device, MonitorBuilder}; use zbus::zvariant::{ObjectPath, OwnedObjectPath}; use zbus::Connection; @@ -92,16 +93,38 @@ fn dev_prop_matches(dev: &Device, prop: &str, value: &str) -> bool { pub struct AsusDevice { device: DeviceHandle, dbus_path: OwnedObjectPath, + hid_key: Option, } pub struct DeviceManager { _dbus_connection: Connection, + _hid_handles: Arc>>>>, } impl DeviceManager { + async fn get_or_create_hid_handle( + handles: &Arc>>>>, + endpoint: &Device, + ) -> Result<(Arc>, String), RogError> { + let dev_node = endpoint + .devnode() + .ok_or_else(|| RogError::MissingFunction("hidraw devnode missing".to_string()))?; + let key = dev_node.to_string_lossy().to_string(); + + if let Some(existing) = handles.lock().await.get(&key).cloned() { + return Ok((existing, key)); + } + + let hidraw = HidRaw::from_device(endpoint.clone())?; + let handle = Arc::new(Mutex::new(hidraw)); + handles.lock().await.insert(key.clone(), handle.clone()); + Ok((handle, key)) + } + async fn init_hid_devices( connection: &Connection, device: Device, + handles: Arc>>>>, ) -> Result, RogError> { let mut devices = Vec::new(); if let Some(usb_device) = device.parent_with_subsystem_devtype("usb", "usb_device")? { @@ -116,9 +139,10 @@ impl DeviceManager { // 1. Generate an interface path // 2. Create the device // Use the top-level endpoint, not the parent - if let Ok(hidraw) = HidRaw::from_device(device) { + if let Ok((dev, hid_key)) = + Self::get_or_create_hid_handle(&handles, &device).await + { debug!("Testing device {usb_id:?}"); - let dev = Arc::new(Mutex::new(hidraw)); // SLASH DEVICE if let Ok(dev_type) = DeviceHandle::new_slash_hid( dev.clone(), @@ -134,6 +158,7 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: Some(hid_key.clone()), }); } } @@ -152,6 +177,7 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: Some(hid_key.clone()), }); } } @@ -170,9 +196,12 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: Some(hid_key), }); } } + } else { + warn!("Failed to initialise shared hid handle for {usb_id:?}"); } } } @@ -181,7 +210,10 @@ impl DeviceManager { } /// To be called on daemon startup - async fn init_all_hid(connection: &Connection) -> Result, RogError> { + async fn init_all_hid( + connection: &Connection, + handles: Arc>>>>, + ) -> Result, RogError> { // track and ensure we use only one hidraw per prod_id // let mut interfaces = HashSet::new(); let mut devices: Vec = Vec::new(); @@ -200,7 +232,7 @@ impl DeviceManager { .scan_devices() .map_err(|e| PlatformError::IoPath("enumerator".to_owned(), e))? { - devices.append(&mut Self::init_hid_devices(connection, device).await?); + devices.append(&mut Self::init_hid_devices(connection, device, handles.clone()).await?); } Ok(devices) @@ -228,6 +260,7 @@ impl DeviceManager { return Some(AsusDevice { device: dev_type, dbus_path: path, + hid_key: None, }); } } @@ -275,10 +308,13 @@ impl DeviceManager { Ok(devices) } - pub async fn find_all_devices(connection: &Connection) -> Vec { + pub async fn find_all_devices( + connection: &Connection, + handles: Arc>>>>, + ) -> Vec { let mut devices: Vec = Vec::new(); // HID first, always - if let Ok(devs) = &mut Self::init_all_hid(connection).await { + if let Ok(devs) = &mut Self::init_all_hid(connection, handles.clone()).await { devices.append(devs); } // USB after, need to check if HID picked something up and if so, skip it @@ -306,6 +342,7 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: None, }); } } else { @@ -328,6 +365,7 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: None, }); } } @@ -355,6 +393,7 @@ impl DeviceManager { devices.push(AsusDevice { device: dev_type, dbus_path: path, + hid_key: None, }); } } @@ -370,16 +409,19 @@ impl DeviceManager { pub async fn new(connection: Connection) -> Result { let conn_copy = connection.clone(); - let devices = Self::find_all_devices(&conn_copy).await; + let hid_handles = Arc::new(Mutex::new(HashMap::new())); + let devices = Self::find_all_devices(&conn_copy, hid_handles.clone()).await; info!("Found {} valid devices on startup", devices.len()); let devices = Arc::new(Mutex::new(devices)); let manager = Self { _dbus_connection: connection, + _hid_handles: hid_handles.clone(), }; // TODO: The /sysfs/ LEDs don't cause events, so they need to be manually // checked for and added + let hid_handles_thread = hid_handles.clone(); std::thread::spawn(move || { let mut monitor = MonitorBuilder::new()?.listen()?; let mut poll = Poll::new()?; @@ -408,6 +450,7 @@ impl DeviceManager { let devices = devices.clone(); let conn_copy = conn_copy.clone(); + let hid_handles = hid_handles_thread.clone(); block_on(async move { // SCSCI devs if subsys == "block" { @@ -483,6 +526,7 @@ impl DeviceManager { // Iter in reverse so as to not screw up indexing for index in removals.iter().rev() { let dev = devices.lock().await.remove(*index); + let hid_key = dev.hid_key.clone(); let path = path.clone(); let res = match dev.device { DeviceHandle::Aura(_) => { @@ -512,14 +556,20 @@ impl DeviceManager { _ => todo!(), }; info!("AuraManager removed: {path:?}, {res}"); + if let Some(key) = hid_key { + hid_handles.lock().await.remove(&key); + } } } } else if action == "add" { let evdev = event.device(); - if let Ok(mut new_devs) = - Self::init_hid_devices(&conn_copy, evdev) - .await - .map_err(|e| error!("Couldn't add new device: {e:?}")) + if let Ok(mut new_devs) = Self::init_hid_devices( + &conn_copy, + evdev, + hid_handles.clone(), + ) + .await + .map_err(|e| error!("Couldn't add new device: {e:?}")) { devices.lock().await.append(&mut new_devs); } diff --git a/asusd/src/aura_scsi/mod.rs b/asusd/src/aura_scsi/mod.rs index 2fc45b54..c87575de 100644 --- a/asusd/src/aura_scsi/mod.rs +++ b/asusd/src/aura_scsi/mod.rs @@ -1,8 +1,8 @@ use std::sync::Arc; use config::ScsiConfig; -use futures_util::lock::{Mutex, MutexGuard}; use rog_scsi::{AuraEffect, Device, Task}; +use tokio::sync::{Mutex, MutexGuard}; use crate::error::RogError; diff --git a/asusd/src/aura_scsi/trait_impls.rs b/asusd/src/aura_scsi/trait_impls.rs index 564e91d7..7345dc28 100644 --- a/asusd/src/aura_scsi/trait_impls.rs +++ b/asusd/src/aura_scsi/trait_impls.rs @@ -83,7 +83,7 @@ impl ScsiZbus { #[zbus(property)] async fn led_mode_data(&self) -> Result { // entirely possible to deadlock here, so use try instead of lock() - if let Some(config) = self.0.config.try_lock() { + if let Ok(config) = self.0.config.try_lock() { let mode = config.current_mode; match config.modes.get(&mode) { Some(effect) => Ok(effect.clone()), diff --git a/asusd/src/aura_slash/mod.rs b/asusd/src/aura_slash/mod.rs index 42434003..8996f59e 100644 --- a/asusd/src/aura_slash/mod.rs +++ b/asusd/src/aura_slash/mod.rs @@ -1,10 +1,10 @@ use std::sync::Arc; use config::SlashConfig; -use futures_util::lock::{Mutex, MutexGuard}; use rog_platform::hid_raw::HidRaw; use rog_platform::usb_raw::USBRaw; use rog_slash::usb::{slash_pkt_enable, slash_pkt_init, slash_pkt_options, slash_pkt_set_mode}; +use tokio::sync::{Mutex, MutexGuard}; use crate::error::RogError; diff --git a/asusd/src/aura_types.rs b/asusd/src/aura_types.rs index b12a0dcb..3a8cf12d 100644 --- a/asusd/src/aura_types.rs +++ b/asusd/src/aura_types.rs @@ -1,7 +1,6 @@ use std::sync::Arc; use config_traits::{StdConfig, StdConfigLoad}; -use futures_util::lock::Mutex; use log::{debug, error, info}; use rog_anime::error::AnimeError; use rog_anime::usb::get_anime_type; @@ -13,6 +12,7 @@ use rog_platform::usb_raw::USBRaw; use rog_scsi::{open_device, ScsiType}; use rog_slash::error::SlashError; use rog_slash::SlashType; +use tokio::sync::Mutex; use crate::aura_anime::config::AniMeConfig; use crate::aura_anime::AniMe; diff --git a/asusd/src/ctrl_backlight.rs b/asusd/src/ctrl_backlight.rs index 188ca4f7..a6a898ed 100644 --- a/asusd/src/ctrl_backlight.rs +++ b/asusd/src/ctrl_backlight.rs @@ -2,9 +2,9 @@ use std::sync::Arc; use std::time::Duration; use config_traits::StdConfig; -use futures_util::lock::Mutex; use log::{info, warn}; use rog_platform::backlight::{Backlight, BacklightType}; +use tokio::sync::Mutex; use zbus::fdo::Error as FdoErr; use zbus::object_server::SignalEmitter; use zbus::{interface, Connection}; @@ -13,7 +13,7 @@ use crate::config::Config; use crate::error::RogError; use crate::ASUS_ZBUS_PATH; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct CtrlBacklight { backlights: Vec, config: Arc>, diff --git a/asusd/src/ctrl_fancurves.rs b/asusd/src/ctrl_fancurves.rs index 3d335180..9d2c87be 100644 --- a/asusd/src/ctrl_fancurves.rs +++ b/asusd/src/ctrl_fancurves.rs @@ -3,13 +3,13 @@ use std::sync::Arc; use config_traits::{StdConfig, StdConfigLoad}; use futures_lite::StreamExt; -use futures_util::lock::Mutex; use log::{debug, error, info, warn}; use rog_platform::platform::{PlatformProfile, RogPlatform}; use rog_profiles::error::ProfileError; use rog_profiles::fan_curve_set::CurveData; use rog_profiles::{find_fan_curve_node, FanCurvePU, FanCurveProfiles}; use serde::{Deserialize, Serialize}; +use tokio::sync::Mutex; use zbus::object_server::SignalEmitter; use zbus::{interface, Connection}; diff --git a/asusd/src/ctrl_platform.rs b/asusd/src/ctrl_platform.rs index a36328a7..df315e1d 100644 --- a/asusd/src/ctrl_platform.rs +++ b/asusd/src/ctrl_platform.rs @@ -3,12 +3,12 @@ use std::process::Command; use std::sync::Arc; use config_traits::StdConfig; -use futures_util::lock::Mutex; use log::{debug, error, info, warn}; use rog_platform::asus_armoury::{AttrValue, FirmwareAttribute, FirmwareAttributes}; use rog_platform::cpu::{CPUControl, CPUGovernor, CPUEPP}; use rog_platform::platform::{PlatformProfile, Properties, RogPlatform}; use rog_platform::power::AsusPower; +use tokio::sync::Mutex; use zbus::fdo::Error as FdoErr; use zbus::object_server::SignalEmitter; use zbus::{interface, Connection}; diff --git a/asusd/src/daemon.rs b/asusd/src/daemon.rs index 0eae0326..20aebe8a 100644 --- a/asusd/src/daemon.rs +++ b/asusd/src/daemon.rs @@ -11,11 +11,11 @@ use asusd::ctrl_fancurves::CtrlFanCurveZbus; use asusd::ctrl_platform::CtrlPlatform; use asusd::{print_board_info, start_tasks, CtrlTask, ZbusRun, DBUS_NAME}; use config_traits::{StdConfig, StdConfigLoad2}; -use futures_util::lock::Mutex; use log::{error, info}; use rog_platform::asus_armoury::FirmwareAttributes; use rog_platform::platform::RogPlatform; use rog_platform::power::AsusPower; +use tokio::sync::Mutex; use zbus::fdo::ObjectManager; #[tokio::main]