feat: optimize serial device discovery performance

[tylerkron]

Summary

• Filter COM ports by USB VID/PID (Microchip 0x04D8) before probing, using platform-specific detection (Windows registry, macOS ioreg, Linux sysfs) to skip non-DAQiFi devices without opening serial ports
• Reduce probe commands to only GetDeviceInfo (SYSTem:SYSInfoPB?), deferring setup commands to the connection phase
• Shorten discovery timeouts: wake-up 200ms (was 1s), response 1s (was 4s), retry interval 300ms (was 1s), max retries 2 (was 3)
• Probe filtered candidate ports in parallel via Task.WhenAll

Closes #157

Test plan

• All 835 existing tests pass on net8.0 and net9.0
• New unit tests for VID/PID filtering logic (DAQiFi VID kept, non-DAQiFi rejected, unknown ports kept as fallback)
• New unit tests for port name filtering (Bluetooth, debug, wlan exclusion)
• New unit tests for SerialPortUsbDetector (VID matching, constant consistency with HidDeviceFinder)
• Hardware test with --discover-serial on real device at /dev/cu.usbmodem101

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Optimize serial device discovery with USB VID/PID filtering and parallel probing

✨ Enhancement

Walkthroughs

Description

• Filter serial ports by USB VID/PID before probing to skip non-DAQiFi devices
  - Windows registry, macOS ioreg, Linux sysfs platform-specific detection
  - Keeps ports with unknown USB info as fallback candidates
• Reduce discovery timeouts and remove setup commands from probe phase
  - Wake-up 200ms (was 1s), response 1s (was 4s), retry 300ms (was 1s), max retries 2 (was 3)
  - Defer DisableEcho, StopStreaming, TurnDeviceOn, SetProtobufStreamFormat to connection phase
• Probe filtered candidate ports in parallel using Task.WhenAll
• Add comprehensive unit tests for VID/PID filtering and USB detection logic

Diagram

flowchart LR
  A["Get Available Ports"] --> B["Filter Port Names<br/>Exclude Bluetooth/Debug/WLAN"]
  B --> C["Detect USB VID/PID<br/>Windows/macOS/Linux"]
  C --> D["Filter by DAQiFi VID<br/>0x04D8"]
  D --> E["Probe Candidates<br/>in Parallel"]
  E --> F["Discovered Devices"]

Loading

File Changes

1. src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs ✨ Enhancement +256/-0

Platform-specific USB VID/PID detection for serial ports

• New platform-specific USB VID/PID detection for Windows (registry), macOS (ioreg), Linux (sysfs)
• Implements GetPortUsbInfo() to map serial ports to USB identifiers
• Provides IsDaqifiVendor() helper to check if VID matches Microchip 0x04D8
• Defines UsbId record for storing vendor and product IDs
• Gracefully handles detection failures by returning empty dictionary

src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs

---

2. src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs ✨ Enhancement +84/-52

Parallel probing with USB VID/PID pre-filtering and reduced timeouts

• Refactor discovery to filter ports by USB VID/PID before probing
• Replace sequential port probing with parallel Task.WhenAll for candidates
• Reduce discovery timeouts: wake-up 200ms, response 1s, retry 300ms, max retries 2
• Remove setup commands (DisableEcho, StopStreaming, TurnDeviceOn, SetProtobufStreamFormat) from
 probe phase
• Add FilterByUsbVidPid() internal methods to filter ports by USB vendor ID
• Change FilterProbableDaqifiPorts() from private to internal for testability

src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs

---

3. src/Daqifi.Core.Tests/Device/Discovery/SerialDeviceFinderFilterTests.cs 🧪 Tests +144/-0

Unit tests for port name and USB VID/PID filtering logic

• New unit tests for FilterProbableDaqifiPorts() covering Bluetooth, debug, WLAN exclusion
• New unit tests for FilterByUsbVidPid() covering DAQiFi VID matching, non-DAQiFi rejection,
 unknown port fallback
• Test mixed scenarios with multiple port types and USB info combinations
• Verify empty input handling for both filter methods

src/Daqifi.Core.Tests/Device/Discovery/SerialDeviceFinderFilterTests.cs

---

View more (1)

4. src/Daqifi.Core.Tests/Device/Discovery/SerialPortUsbDetectorTests.cs 🧪 Tests +46/-0

Unit tests for USB vendor ID detection and record equality

• New unit tests for IsDaqifiVendor() with DAQiFi and non-DAQiFi VIDs
• Test DaqifiVendorId constant consistency with HidDeviceFinder.DefaultVendorId
• Verify GetPortUsbInfo() returns non-null dictionary and never throws
• Test UsbId record equality semantics

src/Daqifi.Core.Tests/Device/Discovery/SerialPortUsbDetectorTests.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1) 🎨 UX Issues (0)

1. FilterByUsbVidPid keeps unknown ports 📎 Requirement gap ⛨ Security

Description

Discovery can still open/probe ports that are not confirmed DAQiFi because unknown ports are
retained and an empty USB-info result causes all ports to be probed. This can send SCPI commands to
non-DAQiFi devices, violating the requirement to only probe VID/PID-matched ports.

Code

src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[R315-337]

+        if (usbInfo.Count == 0)
+        {
+            // VID/PID detection unavailable — probe all ports
+            return portList;
+        }
+
+        var candidates = new List<string>();
+        foreach (var port in portList)
+        {
+            if (usbInfo.TryGetValue(port, out var id))
+            {
+                // USB info available — only include if VID matches DAQiFi
+                if (SerialPortUsbDetector.IsDaqifiVendor(id.VendorId))
+                {
+                    candidates.Add(port);
+                }
+            }
+            else
+            {
+                // No USB info for this port — include to be safe
+                // (could be a non-USB serial port or detection missed it)
+                candidates.Add(port);
+            }

Evidence

PR Compliance ID 1 requires discovery to only open/probe ports whose USB descriptor matches DAQiFi
VID/PID and to avoid sending commands to other devices. In FilterByUsbVidPid, when `usbInfo.Count
== 0` the code returns all ports (so discovery will probe everything), and when a port has no USB
info it is still included as a candidate (so discovery may probe non-DAQiFi ports).

Filter serial ports by DAQiFi USB VID/PID before probing
src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[315-337]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FilterByUsbVidPid` currently allows discovery to probe ports that are not positively identified as DAQiFi by USB VID/PID (and even probes all ports when VID/PID detection returns an empty dictionary). This violates the requirement that discovery must not open/probe non-DAQiFi devices.

## Issue Context
The compliance requirement expects discovery to only probe ports whose USB descriptor matches known DAQiFi VID/PID values.

## Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[315-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. macOS ioreg can hang ☑ 🐞 Bug ☼ Reliability

Description

SerialPortUsbDetector.RunProcess calls StandardOutput.ReadToEnd() before any effective timeout,
so if ioreg stalls, serial discovery can block indefinitely. The subsequent WaitForExit(5000)
cannot prevent the hang because it runs after the blocking read.

Code

src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[R232-247]

+            using var process = new Process
+            {
+                StartInfo = new ProcessStartInfo
+                {
+                    FileName = fileName,
+                    Arguments = arguments,
+                    RedirectStandardOutput = true,
+                    UseShellExecute = false,
+                    CreateNoWindow = true
+                }
+            };
+
+            process.Start();
+            var output = process.StandardOutput.ReadToEnd();
+            process.WaitForExit(5000);
+            return output;

Evidence

The helper intended to bound process execution uses ReadToEnd() (blocking until process exit/EOF)
and only then calls WaitForExit(5000), meaning the 5s limit is not enforced during the blocking
read; any stuck ioreg run will stall discovery.

src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[228-253]
src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[104-123]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
On macOS, USB detection runs `ioreg` via `RunProcess()`. The current implementation can block indefinitely because it performs a synchronous `StandardOutput.ReadToEnd()` before enforcing any timeout; the later `WaitForExit(5000)` cannot interrupt that read.

### Issue Context
This can hang `SerialDeviceFinder.DiscoverAsync()` before any port probing begins.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[228-253]

### Suggested fix
- Implement a real timeout that covers both process runtime and output reading:
 - Prefer `await process.WaitForExitAsync(ct)` with a `CancellationTokenSource(TimeSpan.FromSeconds(…))`.
 - Read stdout asynchronously (`ReadToEndAsync`) and race it with the timeout.
 - If timeout triggers, kill the process (`process.Kill(entireProcessTree: true)`), then return empty output.
- Consider redirecting stderr as well and include it in diagnostics (optional).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. No probe concurrency limit 🐞 Bug ☼ Reliability

Description

SerialDeviceFinder.DiscoverAsync creates one probe task per candidate port and awaits them all at
once, so in fallback cases where USB VID/PID detection yields no filtering, discovery may open many
serial ports concurrently and cause resource pressure or transient port-open failures. This is
amplified by FilterByUsbVidPid explicitly returning all ports when usbInfo is empty.

Code

src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[R99-106]

+            var candidatePorts = FilterByUsbVidPid(allPorts).ToList();

-                try
-                {
-                    var deviceInfo = await TryGetDeviceInfoAsync(portName, cancellationToken);
-                    if (deviceInfo != null)
-                    {
-                        discoveredDevices.Add(deviceInfo);
-                        OnDeviceDiscovered(deviceInfo);
-                    }
-                }
-                catch (OperationCanceledException)
-                {
-                    break;
-                }
-                catch (Exception)
-                {
-                    // Skip ports that fail to open or respond
-                    // This is normal as not all serial ports are DAQiFi devices
-                }
+            // Probe candidate ports in parallel
+            var probeTasks = candidatePorts.Select(portName =>
+                TryGetDeviceInfoAsync(portName, cancellationToken));
+
+            var results = await Task.WhenAll(probeTasks);
+            var discoveredDevices = results.Where(d => d != null).Cast<IDeviceInfo>().ToList();

Evidence

Discovery probes are launched for every candidate port via Task.WhenAll with no throttling.
Separately, the filtering logic explicitly falls back to returning the full port list when USB
detection returns an empty dictionary, which can result in probing all ports concurrently on systems
where VID/PID detection is unavailable/fails.

src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[96-106]
src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[313-340]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DiscoverAsync()` probes candidate ports using `Task.WhenAll` without limiting concurrency. If USB VID/PID detection is unavailable (empty dictionary) the code intentionally probes all ports, which can trigger a large burst of simultaneous `SerialPort.Open()` calls.

### Issue Context
This can cause resource contention, transient failures, or long pauses on machines with many enumerated serial ports.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[96-116]
- src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[313-340]

### Suggested fix
- Introduce a probe concurrency cap (e.g., `const int MaxConcurrentProbes = 4/8`) and use a `SemaphoreSlim` or `Parallel.ForEachAsync` to throttle.
- Keep behavior the same (still parallel), but bounded.
- Optionally make the cap configurable for power users/tests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Linux sysfs symlink traversal 🐞 Bug ➹ Performance

Description

On Linux, USB VID/PID detection walks upward from /sys/class/tty/<name>/device using
Path.GetFullPath and parent directories; if device is a symlink (common in sysfs), this walk may
never reach the real USB device directory containing idVendor/idProduct. When that happens,
VID/PID filtering is missed and discovery falls back to probing more ports than intended.

Code

src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[R172-222]

+            foreach (var ttyPath in Directory.GetDirectories(ttyDir))
+            {
+                var ttyName = Path.GetFileName(ttyPath);
+                var deviceLink = Path.Combine(ttyPath, "device");
+                if (!Directory.Exists(deviceLink)) continue;
+
+                // Walk up the sysfs tree to find the USB device with idVendor/idProduct
+                var usbDevicePath = FindUsbParent(deviceLink);
+                if (usbDevicePath == null) continue;
+
+                var vidFile = Path.Combine(usbDevicePath, "idVendor");
+                var pidFile = Path.Combine(usbDevicePath, "idProduct");
+
+                if (!File.Exists(vidFile) || !File.Exists(pidFile)) continue;
+
+                var vidStr = File.ReadAllText(vidFile).Trim();
+                var pidStr = File.ReadAllText(pidFile).Trim();
+
+                if (int.TryParse(vidStr, System.Globalization.NumberStyles.HexNumber, null, out var vid) &&
+                    int.TryParse(pidStr, System.Globalization.NumberStyles.HexNumber, null, out var pid))
+                {
+                    result[$"/dev/{ttyName}"] = new UsbId(vid, pid);
+                }
+            }
+        }
+        catch
+        {
+            // sysfs access failed — return what we have
+        }
+
+        return result;
+    }
+
+    private static string? FindUsbParent(string devicePath)
+    {
+        var current = Path.GetFullPath(devicePath);
+
+        for (var i = 0; i < 10; i++)
+        {
+            if (File.Exists(Path.Combine(current, "idVendor")))
+                return current;
+
+            var parent = Path.GetDirectoryName(current);
+            if (parent == null || parent == current)
+                break;
+
+            current = parent;
+        }
+
+        return null;
+    }

Evidence

Linux detection calls FindUsbParent(deviceLink) where deviceLink is /sys/class/tty/.../device.
FindUsbParent normalizes the path and walks up directories looking for idVendor, but does not
resolve the symlink target first, so it can traverse the symlink’s location hierarchy instead of the
real device hierarchy.

src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[162-195]
src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[205-222]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Linux USB VID/PID detection may miss devices because `FindUsbParent()` walks up from the symlink path `/sys/class/tty/<tty>/device` without resolving the symlink to its target, so it may never reach the directory that contains `idVendor`/`idProduct`.

### Issue Context
When VID/PID detection misses ports, serial discovery’s intended filtering is reduced and more ports get probed.

### Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[172-195]
- src/Daqifi.Core/Device/Discovery/SerialPortUsbDetector.cs[205-222]

### Suggested fix
- Resolve the `device` symlink before walking parents:
 - Use `new DirectoryInfo(devicePath).ResolveLinkTarget(returnFinalTarget: true)` (or equivalent) when available.
 - Fall back to leaving behavior unchanged if link resolution fails.
- Then run the existing parent-walk on the resolved target path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. filterbyusbvidpid keeps unknown ports 📎 Requirement gap ⛨ Security

Discovery can still open/probe ports that are not confirmed DAQiFi because unknown ports are
retained and an empty USB-info result causes all ports to be probed. This can send SCPI commands to
non-DAQiFi devices, violating the requirement to only probe VID/PID-matched ports.

Agent Prompt

## Issue description
`FilterByUsbVidPid` currently allows discovery to probe ports that are not positively identified as DAQiFi by USB VID/PID (and even probes all ports when VID/PID detection returns an empty dictionary). This violates the requirement that discovery must not open/probe non-DAQiFi devices.

## Issue Context
The compliance requirement expects discovery to only probe ports whose USB descriptor matches known DAQiFi VID/PID values.

## Fix Focus Areas
- src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs[315-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools