fix(hidpp): retry lost feature-table reads during enumeration

crates/openlogi-hid/src/inventory.rs

@@ -38,13 +38,18 @@ const MAX_BOLT_SLOTS: u8 = 6;
 /// device wedges the whole enumeration — and the GUI runs `enumerate` on a
 /// polling watcher, so a permanent hang would stall every later refresh.
 ///
-/// Kept short so a snapshot settles quickly: a timed-out node is skipped and
-/// re-probed on the next watcher tick (~2 s), and the first probe usually wakes
-/// the device so the retry succeeds fast. Slots are probed concurrently on both
-/// receiver paths, so a healthy receiver's worst case is the 1.5 s arrival drain
-/// plus a single slot's [`BOLT_SLOT_PROBE`] / [`UNIFYING_SLOT_PROBE`] — not their
-/// sum — which this stays comfortably above, so awake devices never trip it.
-const PROBE_BUDGET: Duration = Duration::from_secs(6);
+/// A timed-out node is skipped and re-probed on the next watcher tick (~2 s),
+/// and the first probe usually wakes the device so the retry succeeds fast.
+/// Slots are probed concurrently on both receiver paths, so a healthy
+/// receiver's worst case is the 1.5 s arrival drain plus a single slot's
+/// [`BOLT_SLOT_PROBE`] / [`UNIFYING_SLOT_PROBE`] — not their sum — which this
+/// stays comfortably above, so awake devices never trip it.
+///
+/// Sized for the Bluetooth-direct feature walk, the long pole: a ~35-entry
+/// table over a link that drops individual reports, which `hidpp::device`
+/// re-asks for per entry. At 6 s one lost report consumed the whole budget and
+/// the walk was abandoned mid-table, surfacing as a mouse that never appeared.
+const PROBE_BUDGET: Duration = Duration::from_secs(25);
 
 /// Per-slot budget for the HID++ 2.0 feature walk on a Unifying paired device.
 ///

crates/openlogi-hid/src/inventory/probe.rs

@@ -437,6 +437,24 @@ async fn probe_direct(
     let walk_succeeded = capabilities.is_some();
     let caps = capabilities.unwrap_or_default();
     let is_peripheral = probe.battery.is_some() || caps.buttons || caps.pointer || caps.lighting;
+    // A walk that never completed says nothing about what this node is: the
+    // discriminator below would read "no battery, no config feature" off an
+    // empty probe and reject a real mouse as a receiver's secondary interface.
+    // Settle it as a transient failure and keep the node's cache entry, so the
+    // last-good inventory is replayed while the link recovers.
+    if !walk_succeeded {
+        debug!(
+            vid = format_args!("{:04x}", info.vendor_id),
+            pid = format_args!("{:04x}", info.product_id),
+            "feature walk did not complete — transient probe failure, keeping last-known identity"
+        );
+        return NodeProbe {
+            inventory: None,
+            healthy: false,
+            complete: false,
+            outcomes: vec![seen(Some(CacheKey::Direct(info.id.clone())))],
+        };
+    }
     if !is_peripheral {
         debug!(
             vid = format_args!("{:04x}", info.vendor_id),

crates/openlogi-hidpp/src/channel.rs

@@ -688,7 +688,7 @@ fn short_payload_as_long(payload: &[u8; SHORT_REPORT_LENGTH - 1]) -> [u8; LONG_R
 }
 
 #[cfg(test)]
-mod tests {
+pub(crate) mod tests {
     use super::*;
     use std::{
         io,
@@ -1107,14 +1107,14 @@ mod tests {
     }
 
     #[derive(Clone)]
-    struct MockRawHidHandle {
+    pub(crate) struct MockRawHidHandle {
         incoming_tx: async_channel::Sender<Vec<u8>>,
         written_reports: Arc<Mutex<Vec<Vec<u8>>>>,
         responses_on_write: Arc<Mutex<VecDeque<Vec<u8>>>>,
     }
 
     impl MockRawHidHandle {
-        fn queue_response(&self, msg: HidppMessage) {
+        pub(crate) fn queue_response(&self, msg: HidppMessage) {
             self.responses_on_write
                 .lock()
                 .unwrap()
@@ -1125,20 +1125,20 @@ mod tests {
             self.incoming_tx.send(raw_report(msg)).await.unwrap();
         }
 
-        fn written_reports(&self) -> Vec<Vec<u8>> {
+        pub(crate) fn written_reports(&self) -> Vec<Vec<u8>> {
             self.written_reports.lock().unwrap().clone()
         }
     }
 
-    struct MockRawHidChannel {
+    pub(crate) struct MockRawHidChannel {
         incoming_tx: async_channel::Sender<Vec<u8>>,
         incoming_rx: async_channel::Receiver<Vec<u8>>,
         written_reports: Arc<Mutex<Vec<Vec<u8>>>>,
         responses_on_write: Arc<Mutex<VecDeque<Vec<u8>>>>,
     }
 
     impl MockRawHidChannel {
-        fn new() -> (Self, MockRawHidHandle) {
+        pub(crate) fn new() -> (Self, MockRawHidHandle) {
             let (incoming_tx, incoming_rx) = async_channel::unbounded();
             let written_reports = Arc::new(Mutex::new(Vec::new()));
             let responses_on_write = Arc::new(Mutex::new(VecDeque::new()));

crates/openlogi-hidpp/src/device.rs

@@ -1,7 +1,8 @@
 //! Implements peripheral devices connected to HID++ channels.
 
-use std::{any::TypeId, collections::HashMap, sync::Arc};
+use std::{any::TypeId, collections::HashMap, sync::Arc, time::Duration};
 
+use futures::{FutureExt, select};
 use thiserror::Error;
 use tracing::trace;
 
@@ -145,7 +146,7 @@ impl Device {
         );
         let mut features = Vec::with_capacity(count as usize);
         for i in 1..=count {
-            let info = feature_set_feature.get_feature(i).await?;
+            let info = read_feature_entry(&feature_set_feature, self.device_index, i).await?;
             trace!(
                 index = self.device_index,
                 slot = i,
@@ -175,6 +176,101 @@ impl Device {
     }
 }
 
+/// Per-attempt deadline for one feature-table read during enumeration.
+///
+/// The channel's default [`crate::channel::SEND_RESPONSE_TIMEOUT`] (5s) is
+/// longer than the budget most callers give the whole walk, so one dropped
+/// report used to consume the caller's entire probe budget and abort
+/// enumeration. A HID++ round trip that is going to answer answers in tens of
+/// milliseconds; past this the report is lost, and re-asking beats waiting.
+const FEATURE_READ_ATTEMPT: Duration = Duration::from_millis(700);
+
+/// Attempts per feature-table entry before enumeration gives up on it.
+///
+/// Bluetooth-direct links drop individual reports while the table itself stays
+/// stable, so a lost entry is worth re-asking for rather than discarding a walk
+/// that may already be thirty entries deep.
+const FEATURE_READ_ATTEMPTS: u8 = 4;
+
+/// Pause between attempts, letting the link drain before re-asking.
+const FEATURE_READ_BACKOFF: Duration = Duration::from_millis(120);
+
+/// Reads one feature-table entry, re-asking under a short per-attempt deadline
+/// when the link drops the report.
+///
+/// A feature-level refusal ([`Hidpp20Error::Feature`]) or an unsupported
+/// response returns immediately: the device answered, so re-asking cannot
+/// change the answer. Only transport failures are retried.
+async fn read_feature_entry(
+    feature_set: &FeatureSetFeature,
+    device_index: u8,
+    index: u8,
+) -> Result<FeatureInformation, Hidpp20Error> {
+    let mut last_error = None;
+    for attempt in 1..=FEATURE_READ_ATTEMPTS {
+        let mut read = std::pin::pin!(feature_set.get_feature(index).fuse());
+        let outcome = select! {
+            result = read => Some(result),
+            _ = futures_timer::Delay::new(FEATURE_READ_ATTEMPT).fuse() => None,
+        };
+        match outcome {
+            Some(Ok(info)) => return Ok(info),
+            Some(Err(e @ (Hidpp20Error::Feature(_) | Hidpp20Error::UnsupportedResponse))) => {
+                return Err(e);
+            }
+            Some(Err(e)) => last_error = Some(e),
+            None => trace!(
+                index = device_index,
+                slot = index,
+                attempt,
+                "feature-table read timed out — re-asking"
+            ),
+        }
+        if attempt < FEATURE_READ_ATTEMPTS {
+            futures_timer::Delay::new(FEATURE_READ_BACKOFF).await;
+        }
+    }
+    Err(last_error.unwrap_or(Hidpp20Error::Channel(ChannelError::Timeout)))
+}
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use crate::{
+        channel::{HidppChannel, tests::MockRawHidChannel},
+        feature::{CreatableFeature as _, feature_set::FeatureSetFeature},
+        protocol::v20::Hidpp20Error,
+    };
+
+    use super::{FEATURE_READ_ATTEMPTS, read_feature_entry};
+
+    /// An entry whose report is lost is re-asked rather than abandoned. Aborting
+    /// on the first lost report is what made Bluetooth-direct enumeration give
+    /// up mid-table, which callers then misread as "not a peripheral".
+    #[test]
+    fn lost_feature_entry_is_retried_before_giving_up() {
+        futures::executor::block_on(async {
+            let (raw, handle) = MockRawHidChannel::new();
+            let channel = Arc::new(HidppChannel::from_raw_channel(raw).await.unwrap());
+            // The mock answers nothing, so every attempt runs to its deadline.
+            let feature_set = FeatureSetFeature::new(Arc::clone(&channel), 0xff, 0x01);
+
+            let err = read_feature_entry(&feature_set, 0xff, 1).await.unwrap_err();
+
+            assert!(
+                matches!(err, Hidpp20Error::Channel(_)),
+                "an unanswered entry surfaces as a transport failure, got {err:?}"
+            );
+            assert_eq!(
+                handle.written_reports().len(),
+                usize::from(FEATURE_READ_ATTEMPTS),
+                "every attempt should reach the wire"
+            );
+        });
+    }
+}
+
 /// Represents a device-specific error.
 #[derive(Debug, Error)]
 #[non_exhaustive]