diff --git a/crates/openlogi-hid/src/transport/windows.rs b/crates/openlogi-hid/src/transport/windows.rs index 1dac8b47..75fcca84 100644 --- a/crates/openlogi-hid/src/transport/windows.rs +++ b/crates/openlogi-hid/src/transport/windows.rs @@ -1,4 +1,6 @@ #[cfg(target_os = "windows")] +use std::sync::atomic::{AtomicBool, Ordering}; +#[cfg(target_os = "windows")] use std::{error::Error, io}; #[cfg(target_os = "windows")] @@ -58,6 +60,14 @@ impl HidEndpoint { async fn write_report(&self, src: &[u8]) -> Result> { let mut writer = self.writer.lock().await; if let Err(e) = writer.write_output_report(src).await { + // The native fallback works around async-hid write quirks on a + // *live* device; reopening a device that is gone can only fail, and + // its `NativeWriteError` would replace the typed `Disconnected` + // that `is_permanent_disconnect` needs to retire the channel. + if matches!(e, async_hid::HidError::Disconnected) { + return Err(Box::new(e)); + } + if let Some(native_writer) = &self.native_writer { debug!( error = %e, @@ -80,6 +90,11 @@ pub(super) struct WindowsHidppChannel { info: DeviceInfo, short: Option, long: Option, + /// Cleared the first time either endpoint reports a permanently dead + /// handle. Read by [`RawHidChannel::is_connected`], which is what lets + /// `inventory::ledger` evict this channel — the node-vanish path never + /// fires for a cabled device whose receiver keeps the HID node enumerated. + connected: AtomicBool, } #[cfg(target_os = "windows")] @@ -130,8 +145,36 @@ impl WindowsHidppChannel { info: long_info, short, long, + connected: AtomicBool::new(true), }) } + + fn mark_disconnected(&self) { + if self.connected.swap(false, Ordering::AcqRel) { + debug!(name = %self.info.name, "HID channel disconnected"); + } + } + + /// Honour the permanent-failure half of the [`RawHidChannel::read_report`] + /// contract: an `Err` is retried by the `hidpp` read loop, so a condition + /// that will never clear has to park instead of surfacing — otherwise the + /// loop busy-spins a core until something evicts the channel. Sound because + /// the read loop always races this future against the channel's close + /// signal in a `select!`. + async fn park_disconnected(&self) -> T { + self.mark_disconnected(); + std::future::pending().await + } +} + +/// Whether an already-boxed transport error means the handle is permanently +/// dead. [`HidEndpoint::write_report`] boxes the `async_hid` error before the +/// channel sees it, so that path can only recognise the variant by downcast. +#[cfg(target_os = "windows")] +fn is_permanent_disconnect(error: &(dyn Error + Send + Sync + 'static)) -> bool { + error + .downcast_ref::() + .is_some_and(|e| matches!(e, async_hid::HidError::Disconnected)) } #[cfg(target_os = "windows")] @@ -219,7 +262,13 @@ impl RawHidChannel for WindowsHidppChannel { ) })?; - endpoint.write_report(src).await + let result = endpoint.write_report(src).await; + if let Err(e) = &result + && is_permanent_disconnect(e.as_ref()) + { + self.mark_disconnected(); + } + result } async fn read_report(&self, buf: &mut [u8]) -> Result> { @@ -236,17 +285,25 @@ impl RawHidChannel for WindowsHidppChannel { // resumes it and retrieves the report. This relies on reusing the // per-endpoint reader across calls; do not reopen readers per read. tokio::select! { - res = short_reader.read_input_report(&mut short_buf) => { - copy_report(&short_buf, res?, buf) - } - res = long_reader.read_input_report(&mut long_buf) => { - copy_report(&long_buf, res?, buf) - } + res = short_reader.read_input_report(&mut short_buf) => match res { + Ok(len) => copy_report(&short_buf, len, buf), + Err(async_hid::HidError::Disconnected) => self.park_disconnected().await, + Err(e) => Err(e.into()), + }, + res = long_reader.read_input_report(&mut long_buf) => match res { + Ok(len) => copy_report(&long_buf, len, buf), + Err(async_hid::HidError::Disconnected) => self.park_disconnected().await, + Err(e) => Err(e.into()), + }, } } (Some(endpoint), None) | (None, Some(endpoint)) => { let mut reader = endpoint.reader.lock().await; - Ok(reader.read_input_report(buf).await?) + match reader.read_input_report(buf).await { + Ok(len) => Ok(len), + Err(async_hid::HidError::Disconnected) => self.park_disconnected().await, + Err(e) => Err(e.into()), + } } (None, None) => Err(Box::new(io::Error::new( io::ErrorKind::NotConnected, @@ -255,6 +312,10 @@ impl RawHidChannel for WindowsHidppChannel { } } + fn is_connected(&self) -> bool { + self.connected.load(Ordering::Acquire) + } + fn supports_short_long_hidpp(&self) -> Option<(bool, bool)> { Some((self.short.is_some(), self.long.is_some())) } @@ -303,4 +364,25 @@ mod tests { ); assert_eq!(endpoint_for_report_id(0x13), None); } + + /// `HidEndpoint::write_report` boxes the `async_hid` error before the + /// channel sees it, so the permanent-disconnect check on that path only + /// works through a downcast — a plain `matches!` silently never fires. + #[cfg(target_os = "windows")] + #[test] + fn a_boxed_disconnect_is_recognised_as_permanent() { + let disconnected: Box = + Box::new(async_hid::HidError::Disconnected); + assert!(is_permanent_disconnect(disconnected.as_ref())); + + // `NotConnected` is the *open* failure, not a dead live handle: it is + // retryable, so it must keep flowing to the read loop as an error. + let not_connected: Box = + Box::new(async_hid::HidError::NotConnected); + assert!(!is_permanent_disconnect(not_connected.as_ref())); + + let unrelated: Box = + Box::new(io::Error::other("write failed for some other reason")); + assert!(!is_permanent_disconnect(unrelated.as_ref())); + } }