Skip to content

feat: Use SerialDeviceFinder probing for --discover-serial - #5

Merged
tylerkron merged 3 commits into
mainfrom
feature/serial-device-probing
Jan 27, 2026
Merged

tylerkron merged 3 commits into
mainfrom
feature/serial-device-probing

Conversation

@tylerkron

@tylerkron tylerkron commented Jan 26, 2026 •

Copy link
Copy Markdown
Contributor

User description

Summary

Updates the --discover-serial option to use Core's enhanced SerialDeviceFinder which now probes ports to identify actual DAQiFi devices instead of just listing available serial ports.

Changes

  • Renamed DiscoverSerialPorts() to DiscoverSerialDevicesAsync()
  • Uses SerialDeviceFinder.DiscoverAsync() to probe and identify devices
  • Shows real-time discovery progress via DeviceDiscovered event
  • Displays device info (name, serial number, firmware) for discovered devices
  • Falls back to listing raw ports if no DAQiFi devices are found

Example Output

$ dotnet run -- --discover-serial
Discovering serial devices (this may take a moment)...
  Found: Nq1 (/dev/cu.usbmodem101) SN:9090539562006014104 FW:3.2.0

Discovered 1 DAQiFi device(s):
  - Nq1 (/dev/cu.usbmodem101) SN:9090539562006014104 FW:3.2.0

Test Plan

  • Tested with physical Nq1 device - correctly identifies the device
  • Tested without device - correctly falls back to listing available ports
  • Verified streaming still works after discovery

Dependencies

🤖 Generated with Claude Code


PR Type

Enhancement


Description

  • Replaced serial port listing with active device probing

  • Uses SerialDeviceFinder to identify actual DAQiFi devices

  • Shows real-time discovery progress and device details

  • Falls back to raw port listing if no devices found

  • Renamed function to DiscoverSerialDevicesAsync for clarity


Diagram Walkthrough

flowchart LR
  A["--discover-serial option"] --> B["DiscoverSerialDevicesAsync"]
  B --> C["SerialDeviceFinder.DiscoverAsync"]
  C --> D["DeviceDiscovered event"]
  D --> E["Display device info"]
  C --> F["Devices found?"]
  F -->|Yes| G["Show discovered devices"]
  F -->|No| H["Fallback to raw ports"]
Loading

File Walkthrough

Relevant files
Enhancement
Program.cs
Replace serial port listing with active device probing     

Program.cs

  • Renamed DiscoverSerialPorts() to DiscoverSerialDevicesAsync() with
    async support
  • Integrated SerialDeviceFinder for active device probing instead of
    passive port listing
  • Added real-time discovery progress via DeviceDiscovered event handler
  • Enhanced output to display device name, port, serial number, and
    firmware version
  • Implemented fallback to raw port listing when no DAQiFi devices are
    discovered
  • Updated discovery timeout handling with 30-second default for serial
    discovery
+34/-8   

Updates the --discover-serial option to use Core's enhanced
SerialDeviceFinder which now probes ports to identify actual
DAQiFi devices.

Changes:
- Renamed DiscoverSerialPorts() to DiscoverSerialDevicesAsync()
- Uses SerialDeviceFinder.DiscoverAsync() instead of just listing ports
- Shows device info (name, serial, firmware) for discovered devices
- Falls back to listing raw ports if no DAQiFi devices found
- Shows real-time discovery progress via DeviceDiscovered event

Depends on: daqifi/daqifi-core#98

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@qodo-code-review

qodo-code-review Bot commented Jan 26, 2026 •

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Sensitive info exposure

Description: The new serial discovery output prints potentially sensitive device identifiers (e.g.,
SerialNumber, PortName, and firmware) to stdout, which could be captured in terminal
logs/CI logs and inadvertently expose device inventory details.
Program.cs [249-287]

Referred Code
Console.WriteLine("Discovering serial devices (this may take a moment)...");

using var finder = new SerialDeviceFinder();
var timeout = TimeSpan.FromSeconds(timeoutSeconds <= 0 ? 30 : timeoutSeconds);

finder.DeviceDiscovered += (_, args) =>
{
    Console.WriteLine($"  Found: {args.DeviceInfo.Name} ({args.DeviceInfo.PortName}) " +
                      $"SN:{args.DeviceInfo.SerialNumber} FW:{args.DeviceInfo.FirmwareVersion}");
};

var devices = await finder.DiscoverAsync(timeout);

Console.WriteLine();
Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
if (!devices.Any())
{
    Console.WriteLine("  (no DAQiFi devices found)");
    Console.WriteLine();
    Console.WriteLine("Available serial ports (not verified as DAQiFi devices):");
    var ports = SerialStreamTransport.GetAvailablePortNames();


 ... (clipped 18 lines)
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing exception handling: Serial discovery and port enumeration calls are not wrapped with error handling, so
failures (e.g., permission/IO errors) may terminate the command without a clear,
actionable message or graceful fallback.

Referred Code
private static async Task DiscoverSerialDevicesAsync(int timeoutSeconds)
{
    Console.WriteLine("Discovering serial devices (this may take a moment)...");

    using var finder = new SerialDeviceFinder();
    var timeout = TimeSpan.FromSeconds(timeoutSeconds <= 0 ? 30 : timeoutSeconds);

    finder.DeviceDiscovered += (_, args) =>
    {
        Console.WriteLine($"  Found: {args.DeviceInfo.Name} ({args.DeviceInfo.PortName}) " +
                          $"SN:{args.DeviceInfo.SerialNumber} FW:{args.DeviceInfo.FirmwareVersion}");
    };

    var devices = await finder.DiscoverAsync(timeout);

    Console.WriteLine();
    Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
    if (!devices.Any())
    {
        Console.WriteLine("  (no DAQiFi devices found)");
        Console.WriteLine();


 ... (clipped 13 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Potential sensitive output: The command prints device SerialNumber (and other identifiers) to console output, which
may be considered sensitive in some environments and should be confirmed as acceptable or
redacted/configurable.

Referred Code
    Console.WriteLine($"  Found: {args.DeviceInfo.Name} ({args.DeviceInfo.PortName}) " +
                      $"SN:{args.DeviceInfo.SerialNumber} FW:{args.DeviceInfo.FirmwareVersion}");
};

var devices = await finder.DiscoverAsync(timeout);

Console.WriteLine();
Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
if (!devices.Any())
{
    Console.WriteLine("  (no DAQiFi devices found)");
    Console.WriteLine();
    Console.WriteLine("Available serial ports (not verified as DAQiFi devices):");
    var ports = SerialStreamTransport.GetAvailablePortNames();
    if (ports.Length == 0)
    {
        Console.WriteLine("  (none)");
    }
    else
    {
        foreach (var port in ports)


 ... (clipped 11 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

qodo-code-review Bot commented Jan 26, 2026 •

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
✅ Handle discovery exceptions
Suggestion Impact:The commit wrapped finder.DiscoverAsync(timeout) in a try/catch, logged an error message on exception, and defaulted to an empty device list. It also adjusted counting/emptiness checks to use the concrete list (Count) after converting the discovery result to a List.

code diff:

-        var devices = await finder.DiscoverAsync(timeout);
+        List<IDeviceInfo> devices;
+        try
+        {
+            devices = (await finder.DiscoverAsync(timeout)).ToList();
+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"Error during serial discovery: {ex.Message}");
+            devices = new List<IDeviceInfo>();
+        }
 
         Console.WriteLine();
-        Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
-        if (!devices.Any())
+        Console.WriteLine($"Discovered {devices.Count} DAQiFi device(s):");
+        if (devices.Count == 0)
         {

Add a try-catch block around the finder.DiscoverAsync call to gracefully handle
potential exceptions during device discovery and prevent the application from
crashing.

Program.cs [260]

-var devices = await finder.DiscoverAsync(timeout);
+List<SerialDeviceInfo> devices;
+try
+{
+    devices = await finder.DiscoverAsync(timeout);
+}
+catch (Exception ex)
+{
+    Console.WriteLine($"Error discovering serial devices: {ex.Message}");
+    devices = new List<SerialDeviceInfo>();
+}

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out a lack of error handling around an I/O-bound asynchronous operation. Adding a try-catch block improves the application's robustness by preventing a potential crash if the device discovery fails.

Medium
General
✅ Avoid printing discovered devices twice
Suggestion Impact:The commit modifies the same post-discovery output block by changing how the device count and emptiness check are handled (Count property / Count == 0) and slightly adjusts the summary line formatting, but it does not remove the final printing loop of discovered devices as suggested (so the main duplication issue is not addressed in this diff).

code diff:

         Console.WriteLine();
-        Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
-        if (!devices.Any())
+        Console.WriteLine($"Discovered {devices.Count} DAQiFi device(s):");
+        if (devices.Count == 0)
         {
             Console.WriteLine("  (no DAQiFi devices found)");
             Console.WriteLine();

Remove the final loop that prints discovered devices to avoid displaying the
same information twice, as it's already shown by the DeviceDiscovered event
handler.

Program.cs [254-288]

     finder.DeviceDiscovered += (_, args) =>
     {
-        Console.WriteLine($"  Found: {args.DeviceInfo.Name} ({args.DeviceInfo.PortName}) " +
+        Console.WriteLine($"  - {args.DeviceInfo.Name} ({args.DeviceInfo.PortName}) " +
                           $"SN:{args.DeviceInfo.SerialNumber} FW:{args.DeviceInfo.FirmwareVersion}");
     };
 
     var devices = await finder.DiscoverAsync(timeout);
 
     Console.WriteLine();
-    Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
-    if (!devices.Any())
+    if (devices.Any())
     {
-...
+        Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s).");
     }
     else
     {
-        foreach (var device in devices)
+        Console.WriteLine("  (no DAQiFi devices found)");
+        Console.WriteLine();
+        Console.WriteLine("Available serial ports (not verified as DAQiFi devices):");
+        var ports = SerialStreamTransport.GetAvailablePortNames();
+        if (ports.Length == 0)
         {
-            Console.WriteLine($"  - {device.Name} ({device.PortName}) SN:{device.SerialNumber} FW:{device.FirmwareVersion}");
+            Console.WriteLine("  (none)");
+        }
+        else
+        {
+            foreach (var port in ports)
+            {
+                Console.WriteLine($"  - {port}");
+            }
         }
     }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that device information is printed twice, leading to redundant output. Removing the final loop cleans up the console output while retaining the real-time discovery feedback, which improves the user experience.

Low
✅ Cache enumeration result
Suggestion Impact:The commit materializes the discovery result into a List via ToList(), then uses the List.Count property and checks emptiness via Count == 0, preventing repeated enumeration. It also adds try/catch error handling around discovery.

code diff:

-        var devices = await finder.DiscoverAsync(timeout);
+        List<IDeviceInfo> devices;
+        try
+        {
+            devices = (await finder.DiscoverAsync(timeout)).ToList();
+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"Error during serial discovery: {ex.Message}");
+            devices = new List<IDeviceInfo>();
+        }
 
         Console.WriteLine();
-        Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
-        if (!devices.Any())
+        Console.WriteLine($"Discovered {devices.Count} DAQiFi device(s):");
+        if (devices.Count == 0)
         {

Convert the IEnumerable of discovered devices to a List to avoid potential
multiple enumerations when calling Count() and Any().

Program.cs [260-264]

-var devices = await finder.DiscoverAsync(timeout);
+var devicesList = (await finder.DiscoverAsync(timeout)).ToList();
 
-Console.WriteLine($"Discovered {devices.Count()} DAQiFi device(s):");
-if (!devices.Any())
+Console.WriteLine($"Discovered {devicesList.Count} DAQiFi device(s):");
+if (!devicesList.Any())

[Suggestion processed]

Suggestion importance[1-10]: 4

__

Why: This is a valid micro-optimization to prevent multiple enumerations of the devices collection by calling .ToList(). While good practice, the performance impact is likely negligible given the expected small number of serial devices.

Low
  • Update

tylerkron and others added 2 commits January 26, 2026 14:54
- Add try-catch around discovery to handle exceptions gracefully
- Use .ToList() to avoid multiple enumeration of devices

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@tylerkron
tylerkron merged commit 8aaf5e2 into main Jan 27, 2026
@tylerkron tylerkron linked an issue Jan 27, 2026 that may be closed by this pull request
@tylerkron
tylerkron deleted the feature/serial-device-probing branch March 20, 2026 19:55
tylerkron added a commit that referenced this pull request Apr 25, 2026
- Remove the unrelated --mimic-desktop / DesktopFlowTest WIP that was
  accidentally carried over from an earlier branch via git stash. The
  PR's stated scope is SD-export-csv only; DesktopFlowTest doesn't exist
  in this repo so the call site broke compilation (Qodo #1).
- Validate --sd-export-csv requires --sd-parse; previously, passing
  --sd-export-csv alone fell through to the connection logic and
  emitted a misleading "Missing required option: --ip or --serial"
  error (Qodo #2).
- Emit a one-time stderr warning when an SD log entry carries more
  analog values than the configured analog port count, so silent
  truncation is observable (Qodo #3).
- Rename progress display from "samples written" / "samples/s" to
  "rows written" / "rows/s". The counter increments per SampleRow
  (channel-value pair), not per SD entry — matches what's emitted
  to the CSV (Qodo #4).
- Update README ".NET 8 SDK" → ".NET 9 SDK" to match the project
  TFM bump (Qodo #5).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Update --discover-serial to use enhanced SerialDeviceFinder

1 participant