-
Notifications
You must be signed in to change notification settings - Fork 0
perf(discovery): VID/PID-filter serial ports + minimal probe (closes #157) #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
cptkoolbeenz
wants to merge
15
commits into
main
Choose a base branch
from
perf/serial-discovery-vid-pid-filter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
416bc93
perf(discovery): VID/PID-filter serial ports + minimal probe (closes …
cptkoolbeenz 6c7cd42
Apply Qodo /improve pass 1: resolve sysfs symlink before traversal
cptkoolbeenz ba62343
Apply Qodo /improve pass 2: validate portName before WMI interpolation
cptkoolbeenz 643a988
Apply Qodo /improve pass 3: invariant culture on hex parse
cptkoolbeenz 97d0959
Apply Qodo /improve pass 4: dispose WMI results, narrow query
cptkoolbeenz d3d869a
Apply Qodo /improve pass 5: guard descriptor provider against throws
cptkoolbeenz 1f24638
Apply Qodo /improve pass 6: explicit null/whitespace guard on portName
cptkoolbeenz 03209c8
Apply Qodo /improve pass 7: System.Management reference unconditional
cptkoolbeenz e0e76ff
Apply Qodo /improve pass 8: tolerate OCE in null/throwing-provider tests
cptkoolbeenz 43d82c2
Apply Qodo /agentic_review pass 9 on PR #201: parallel port probing
cptkoolbeenz 1164d84
Apply Qodo /improve pass 10: rethrow OCE so WhenAll observes cancella…
cptkoolbeenz 4cc0f08
Apply Qodo /improve pass 11: cap parallel probes + preserve completio…
cptkoolbeenz f0f5926
Apply Qodo /agentic_review pass 12: throwing-provider test seam
cptkoolbeenz 5f213f6
Apply Qodo /agentic_review pass 13: invalid mock port name
cptkoolbeenz 8b69f6b
Apply Qodo /improve pass 12: deterministic classifier-rejects test
cptkoolbeenz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 47 additions & 0 deletions
47
src/Daqifi.Core.Tests/Device/Discovery/UsbPortDescriptorTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| using Daqifi.Core.Device.Discovery; | ||
|
|
||
| namespace Daqifi.Core.Tests.Device.Discovery; | ||
|
|
||
| public class UsbPortDescriptorTests | ||
| { | ||
| [Fact] | ||
| public void DaqifiUsbIds_VendorAndCdcProductId_MatchExpectedValues() | ||
| { | ||
| // Locks the published constants. If these change without | ||
| // explicit intent, every consumer's USB filter breaks. | ||
| Assert.Equal(0x04D8, DaqifiUsbIds.VendorId); | ||
| Assert.Equal(0xF794, DaqifiUsbIds.CdcProductId); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DaqifiUsbIds_IsDaqifiCdcDevice_TrueForExactMatch() | ||
| { | ||
| var descriptor = new UsbPortDescriptor(0x04D8, 0xF794); | ||
| Assert.True(DaqifiUsbIds.IsDaqifiCdcDevice(descriptor)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DaqifiUsbIds_IsDaqifiCdcDevice_FalseForBootloaderPid() | ||
| { | ||
| // Bootloader mode PID — not the CDC mode we discover via SerialDeviceFinder. | ||
| var descriptor = new UsbPortDescriptor(0x04D8, 0x003C); | ||
| Assert.False(DaqifiUsbIds.IsDaqifiCdcDevice(descriptor)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DaqifiUsbIds_IsDaqifiCdcDevice_FalseForOtherVendor() | ||
| { | ||
| // CH340 vendor — common USB serial chip; must not be classified as DAQiFi. | ||
| var descriptor = new UsbPortDescriptor(0x1A86, 0x7523); | ||
| Assert.False(DaqifiUsbIds.IsDaqifiCdcDevice(descriptor)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void NullUsbPortDescriptorProvider_AlwaysReturnsNull() | ||
| { | ||
| var provider = NullUsbPortDescriptorProvider.Instance; | ||
| Assert.Null(provider.GetDescriptor("COM9")); | ||
| Assert.Null(provider.GetDescriptor("/dev/ttyACM1")); | ||
| Assert.Null(provider.GetDescriptor("")); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| namespace Daqifi.Core.Device.Discovery; | ||
|
|
||
| /// <summary> | ||
| /// Known USB Vendor / Product identifiers for DAQiFi devices in normal | ||
| /// (non-bootloader) operating mode. Used by <see cref="SerialDeviceFinder"/> | ||
| /// to filter serial ports before probing — only ports matching one of these | ||
| /// IDs are opened, eliminating accidental SCPI traffic to other vendors' | ||
| /// COM ports (Bluetooth radios, GPS receivers, Arduinos, etc.). | ||
| /// </summary> | ||
| public static class DaqifiUsbIds | ||
| { | ||
| /// <summary> | ||
| /// USB vendor ID assigned by Microchip and used by DAQiFi devices | ||
| /// (PIC32-based). Same VID is used in bootloader mode. | ||
| /// </summary> | ||
| public const int VendorId = 0x04D8; | ||
|
|
||
| /// <summary> | ||
| /// USB product ID for DAQiFi devices in normal USB CDC serial mode | ||
| /// (Nyquist1, Nyquist3). Bootloader mode uses a different PID | ||
| /// (<c>0x003C</c>, see <c>FirmwareUpdateServiceOptions.BootloaderProductId</c>). | ||
| /// </summary> | ||
| public const int CdcProductId = 0xF794; | ||
|
|
||
| /// <summary> | ||
| /// Returns true if the supplied descriptor matches a known DAQiFi USB | ||
| /// CDC serial-mode device (matches <see cref="VendorId"/> and one of | ||
| /// the known product IDs). | ||
| /// </summary> | ||
| public static bool IsDaqifiCdcDevice(UsbPortDescriptor descriptor) | ||
| { | ||
| return descriptor.VendorId == VendorId | ||
| && descriptor.ProductId == CdcProductId; | ||
| } | ||
| } |
32 changes: 32 additions & 0 deletions
32
src/Daqifi.Core/Device/Discovery/IUsbPortDescriptorProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| namespace Daqifi.Core.Device.Discovery; | ||
|
|
||
| /// <summary> | ||
| /// Resolves the USB Vendor/Product ID associated with a serial port name | ||
| /// (e.g. <c>COM9</c> on Windows, <c>/dev/ttyACM1</c> on Linux). Used by | ||
| /// <see cref="SerialDeviceFinder"/> to pre-filter ports before opening | ||
| /// them, so non-DAQiFi hardware (Bluetooth radios, GPS receivers, other | ||
| /// vendors' COM ports, etc.) is skipped instantly without sending SCPI | ||
| /// commands or waiting for a probe timeout. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Implementations are platform-specific: Windows uses WMI, Linux reads | ||
| /// <c>/sys/class/tty</c>. Platforms without an implementation fall back to | ||
| /// <see cref="NullUsbPortDescriptorProvider"/> which returns null for every | ||
| /// port, preserving the legacy "probe every port" behavior. | ||
| /// </remarks> | ||
| public interface IUsbPortDescriptorProvider | ||
| { | ||
| /// <summary> | ||
| /// Returns the USB descriptor for <paramref name="portName"/>, or | ||
| /// <c>null</c> if the port is not USB-attached or the descriptor | ||
| /// cannot be resolved. | ||
| /// </summary> | ||
| UsbPortDescriptor? GetDescriptor(string portName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// USB Vendor/Product identification for a serial port. | ||
| /// </summary> | ||
| /// <param name="VendorId">USB vendor ID (e.g. 0x04D8 for DAQiFi/Microchip).</param> | ||
| /// <param name="ProductId">USB product ID (e.g. 0xF794 for DAQiFi USB CDC mode).</param> | ||
| public sealed record UsbPortDescriptor(int VendorId, int ProductId); |
73 changes: 73 additions & 0 deletions
73
src/Daqifi.Core/Device/Discovery/LinuxUsbPortDescriptorProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| using System.Globalization; | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Daqifi.Core.Device.Discovery; | ||
|
|
||
| /// <summary> | ||
| /// Resolves USB VID/PID for serial ports via Linux <c>/sys/class/tty/</c> | ||
| /// sysfs entries. Returns null on non-Linux platforms or for ports whose | ||
| /// sysfs lookup fails (non-USB serial, virtual ttys, etc.). | ||
| /// </summary> | ||
| internal sealed class LinuxUsbPortDescriptorProvider : IUsbPortDescriptorProvider | ||
| { | ||
| public UsbPortDescriptor? GetDescriptor(string portName) | ||
| { | ||
| if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| // portName is typically /dev/ttyACM0 or /dev/ttyUSB0. | ||
| // The corresponding sysfs path is /sys/class/tty/<base>/device/... | ||
| // We walk up the device tree looking for idVendor + idProduct, | ||
| // which sit on the USB device node (a few levels above the tty). | ||
| var baseName = System.IO.Path.GetFileName(portName); | ||
| if (string.IsNullOrEmpty(baseName)) | ||
| return null; | ||
|
|
||
| var sysfsRoot = $"/sys/class/tty/{baseName}/device"; | ||
| if (!System.IO.Directory.Exists(sysfsRoot)) | ||
| return null; | ||
|
|
||
| // Walk up the symlink-resolved path looking for idVendor/idProduct. | ||
| // Bound the depth to keep this defensive against unexpected layouts. | ||
| try | ||
| { | ||
| // /sys/class/tty/<base>/device is a symlink into the actual USB | ||
| // device tree (e.g. /sys/devices/pci.../usb1/.../1-1.2). Walking | ||
| // parents of the unresolved logical path lands back in /sys/class | ||
| // and never reaches the node that holds idVendor/idProduct, so | ||
| // resolve to the physical target before traversal. | ||
| var dirInfo = new System.IO.DirectoryInfo(sysfsRoot); | ||
| var resolved = dirInfo.ResolveLinkTarget(returnFinalTarget: true); | ||
| var current = (resolved ?? dirInfo).FullName; | ||
| for (var i = 0; i < 8; i++) | ||
| { | ||
| var vendorPath = System.IO.Path.Combine(current, "idVendor"); | ||
| var productPath = System.IO.Path.Combine(current, "idProduct"); | ||
| if (System.IO.File.Exists(vendorPath) && System.IO.File.Exists(productPath)) | ||
| { | ||
| var vidText = System.IO.File.ReadAllText(vendorPath).Trim(); | ||
| var pidText = System.IO.File.ReadAllText(productPath).Trim(); | ||
| if (int.TryParse(vidText, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out var vid) && | ||
| int.TryParse(pidText, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out var pid)) | ||
| { | ||
| return new UsbPortDescriptor(vid, pid); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| var parent = System.IO.Directory.GetParent(current); | ||
| if (parent == null || parent.FullName == current) | ||
| break; | ||
| current = parent.FullName; | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| // Permission denied / IO error → fall through to null. | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } |
15 changes: 15 additions & 0 deletions
15
src/Daqifi.Core/Device/Discovery/NullUsbPortDescriptorProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| namespace Daqifi.Core.Device.Discovery; | ||
|
|
||
| /// <summary> | ||
| /// No-op descriptor provider that returns null for every port. Used as the | ||
| /// fallback on platforms where USB descriptor enumeration isn't implemented, | ||
| /// or when callers want to preserve the legacy "probe every port" behavior. | ||
| /// </summary> | ||
| internal sealed class NullUsbPortDescriptorProvider : IUsbPortDescriptorProvider | ||
| { | ||
| public static readonly NullUsbPortDescriptorProvider Instance = new(); | ||
|
|
||
| private NullUsbPortDescriptorProvider() { } | ||
|
|
||
| public UsbPortDescriptor? GetDescriptor(string portName) => null; | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Null descriptor still probes ports
📎 Requirement gap≡ CorrectnessAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools