Skip to content

Conversation

@tylerkron
Copy link
Contributor

@tylerkron tylerkron commented Oct 17, 2025

User description

Summary

Implements Phase 4 of the migration plan: Device Discovery Framework as specified in Issue #49.

This PR adds comprehensive device discovery functionality to daqifi-core, enabling discovery of DAQiFi devices via WiFi/UDP, Serial/USB, and HID (bootloader mode).

New Features

🔍 Device Discovery Interfaces

  • IDeviceFinder - Interface for device discovery mechanisms with async/await and cancellation support
  • IDeviceInfo - Interface for discovered device metadata
  • DeviceInfo - Concrete implementation with support for WiFi, Serial, and HID devices
  • DeviceDiscoveredEventArgs - Event args for real-time discovery notifications
  • Enumerations:
    • DeviceType (Unknown, Daqifi, Nyquist)
    • ConnectionType (Unknown, WiFi, Serial, Hid)

📡 WiFi Device Discovery (WiFiDeviceFinder)

  • UDP broadcast discovery on configurable port (default 30303 matching desktop implementation)
  • Parses protobuf responses from devices extracting:
    • IP address, MAC address, TCP port
    • Hostname, serial number, firmware version
    • Device type (from part number), power status
  • Network interface enumeration for proper subnet-directed broadcasts
  • Duplicate device detection (by MAC address or serial number)
  • Async discovery with timeout and cancellation token support
  • Event-based notifications (DeviceDiscovered, DiscoveryCompleted)

🔌 Serial Device Discovery (SerialDeviceFinder)

  • Enumerates available serial/USB ports using existing SerialStreamTransport
  • Configurable baud rate (default 115200)
  • Async discovery with cancellation support
  • Note: Full device info retrieval via SCPI commands deferred to Phase 6 (Protocol Implementation)

🛠️ HID Device Discovery (HidDeviceFinder)

  • Framework for bootloader mode device discovery (VendorId: 0x4D8, ProductId: 0x03C)
  • Basic structure in place following IDeviceFinder interface
  • Note: Full HID enumeration deferred until HID library dependency is added to core

Testing

Added 23 new unit tests covering all discovery functionality:

Test Coverage

  • WiFiDeviceFinderTests - 7 tests covering async discovery, cancellation, events, disposal
  • SerialDeviceFinderTests - 7 tests for serial enumeration and lifecycle
  • HidDeviceFinderTests - 5 tests for HID discovery framework
  • DeviceInfoTests - 4 tests for device info formatting and defaults

Test Results

Passed!  - Failed: 0, Passed: 23, Skipped: 0, Total: 23
  • All tests passing on .NET 8.0 and .NET 9.0
  • Proper async/await patterns with cancellation token support
  • No test flakiness or port binding conflicts

Implementation Details

Leverages Existing Infrastructure

  • Uses existing UdpTransport from Phase 3 for WiFi discovery
  • Uses existing SerialStreamTransport.GetAvailablePortNames() for serial enumeration
  • Uses existing protobuf message types (DaqifiOutMessage) for device discovery responses
  • Follows code patterns established in Phase 3 (Connection Management)

Cross-Platform Compatible

  • No Windows-specific dependencies
  • Works on Linux, macOS, and Windows
  • Uses standard .NET networking APIs

Code Quality

  • Comprehensive XML documentation on all public APIs
  • Follows existing coding standards and conventions
  • Proper disposal patterns with IDisposable
  • Event-driven architecture for real-time discovery feedback

Desktop Migration Impact

Once this is deployed to NuGet as Core 0.6.0, daqifi-desktop can migrate:

Before (Desktop):

var wifiFinder = new Desktop.DaqifiDeviceFinder(30303);
var serialFinder = new Desktop.SerialDeviceFinder();
var hidFinder = new Desktop.HidDeviceFinder();

After (Core):

using Daqifi.Core.Device.Discovery;

var wifiFinder = new WiFiDeviceFinder(); // Default port 30303
var serialFinder = new SerialDeviceFinder(); // Default 115200 baud
var hidFinder = new HidDeviceFinder();

// All implement IDeviceFinder with consistent async API
foreach (var finder in new IDeviceFinder[] { wifiFinder, serialFinder, hidFinder })
{
    finder.DeviceDiscovered += (s, e) => Console.WriteLine($"Found: {e.DeviceInfo}");
    var devices = await finder.DiscoverAsync(TimeSpan.FromSeconds(5));
}

Desktop Code Reduction

  • DaqifiDeviceFinder → replaced with WiFiDeviceFinder (remove ~255 lines)
  • SerialDeviceFinder → replaced with core implementation (remove ~154 lines)
  • HidDeviceFinder → replaced with core implementation (remove ~83 lines)
  • Total: ~492 lines removed from desktop

Migration Plan Progress

  • ✅ Phase 1: Foundation (Core 0.3.0)
  • ✅ Phase 2: Message System (Core 0.4.0)
  • ✅ Phase 3: Connection Management (Core 0.5.0)
  • Phase 4: Device Discovery (Core 0.6.0)This PR
  • ⏳ Phase 5: Channel Management
  • ⏳ Phase 6: Protocol Implementation
  • ⏳ Phase 7: Advanced Features
  • 🚫 Phase 8: Desktop Integration (deferred until Phases 3-7 complete)

Breaking Changes

None - This is purely additive functionality.

Dependencies

  • Depends on: Phase 3 (Connection Management) - UDP transport ✅
  • Blocks: Phase 5 (Channel Management) and Phase 6 (Protocol Implementation)

Related Issues

Checklist

  • Code builds successfully
  • All tests passing (23/23 pass)
  • No breaking changes to existing APIs
  • XML documentation on all public APIs
  • Cross-platform compatible (no Windows-specific code)
  • Follows existing code patterns and conventions
  • Migration plan updated with completion status

🤖 Generated with Claude Code


PR Type

Enhancement, Tests


Description

  • Implements Phase 4 device discovery framework with WiFi, Serial, and HID support

  • Adds IDeviceFinder interface and concrete implementations for three connection types

  • Includes DeviceInfo class and enumerations for device/connection types

  • Provides 23 comprehensive unit tests covering all discovery mechanisms


Diagram Walkthrough

flowchart LR
  IDeviceFinder["IDeviceFinder<br/>Interface"]
  WiFi["WiFiDeviceFinder<br/>UDP Broadcast"]
  Serial["SerialDeviceFinder<br/>Port Enumeration"]
  HID["HidDeviceFinder<br/>Bootloader Mode"]
  DeviceInfo["DeviceInfo<br/>Metadata"]
  Events["DeviceDiscoveredEventArgs<br/>Event Notifications"]
  
  IDeviceFinder -- "implemented by" --> WiFi
  IDeviceFinder -- "implemented by" --> Serial
  IDeviceFinder -- "implemented by" --> HID
  WiFi -- "returns" --> DeviceInfo
  Serial -- "returns" --> DeviceInfo
  HID -- "returns" --> DeviceInfo
  WiFi -- "raises" --> Events
  Serial -- "raises" --> Events
  HID -- "raises" --> Events
Loading

File Walkthrough

Relevant files
Enhancement
7 files
IDeviceFinder.cs
Device discovery interface with async support                       
+36/-0   
IDeviceInfo.cs
Device metadata interface and type enumerations                   
+113/-0 
DeviceInfo.cs
Concrete device information implementation                             
+81/-0   
DeviceDiscoveredEventArgs.cs
Event arguments for device discovery notifications             
+23/-0   
WiFiDeviceFinder.cs
UDP broadcast WiFi device discovery implementation             
+359/-0 
SerialDeviceFinder.cs
Serial port enumeration device discovery                                 
+189/-0 
HidDeviceFinder.cs
HID bootloader mode discovery framework                                   
+125/-0 
Tests
4 files
WiFiDeviceFinderTests.cs
WiFi discovery async and event tests                                         
+102/-0 
SerialDeviceFinderTests.cs
Serial discovery lifecycle and baud rate tests                     
+97/-0   
HidDeviceFinderTests.cs
HID discovery framework and disposal tests                             
+80/-0   
DeviceInfoTests.cs
Device info formatting and default value tests                     
+86/-0   

Implements device discovery functionality as specified in Issue #49.

## New Features

### Device Discovery Interfaces
- `IDeviceFinder` - Interface for device discovery mechanisms with async support
- `IDeviceInfo` - Interface for discovered device metadata
- `DeviceInfo` - Implementation class for device information
- `DeviceDiscoveredEventArgs` - Event args for device discovered events
- `DeviceType` enum (Unknown, Daqifi, Nyquist)
- `ConnectionType` enum (Unknown, WiFi, Serial, Hid)

### WiFi Device Discovery
- `WiFiDeviceFinder` - Discovers DAQiFi devices on network using UDP broadcast
- UDP broadcast on configurable port (default 30303)
- Parses protobuf responses from devices (IP, MAC, port, hostname, serial, firmware)
- Network interface enumeration for subnet-directed broadcasts
- Timeout and cancellation token support
- Event-based discovery notifications (DeviceDiscovered, DiscoveryCompleted)
- Duplicate device detection based on MAC address or serial number

### Serial Device Discovery
- `SerialDeviceFinder` - Discovers DAQiFi devices connected via USB/Serial ports
- Enumerates available serial ports
- Configurable baud rate (default 115200)
- Async discovery with cancellation support
- Note: Full device info retrieval deferred to Phase 6 (Protocol Implementation)

### HID Device Discovery
- `HidDeviceFinder` - Framework for bootloader mode device discovery
- VendorId: 0x4D8, ProductId: 0x03C
- Basic structure in place (full implementation requires HID library dependency)
- Note: HID enumeration deferred until HID library is added to core

## Testing
- 23 new unit tests covering all discovery functionality
- xUnit test framework (matching existing test structure)
- Tests for WiFi, Serial, and HID device finders
- Tests for DeviceInfo formatting and default values
- All tests passing on .NET 8 and .NET 9
- Proper async/await patterns with cancellation token support

## Implementation Notes
- Leverages existing `UdpTransport` for WiFi discovery
- Leverages existing `SerialStreamTransport.GetAvailablePortNames()` for serial enumeration
- Uses existing protobuf message types (`DaqifiOutMessage`) for device discovery responses
- Cross-platform compatible (no Windows-specific dependencies)
- Follows existing code patterns and architecture from Phase 3

## Desktop Migration Impact
Once deployed, daqifi-desktop can:
- Replace `DaqifiDeviceFinder` with `WiFiDeviceFinder` from core
- Replace `SerialDeviceFinder` with core implementation
- Replace `HidDeviceFinder` with core implementation (when HID support added)
- Remove device enumeration logic from desktop

## Related Issues
- Closes #49 (Phase 4: Device Discovery Framework)
- Part of migration plan documented in DAQIFI_CORE_MIGRATION_PLAN.md
- Depends on Phase 3 (Connection Management - UDP transport)
- Blocks Phase 5 (Channel Management)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@tylerkron tylerkron requested a review from a team as a code owner October 17, 2025 21:47
@qodo-code-review
Copy link

qodo-code-review bot commented Oct 17, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

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

@qodo-code-review
Copy link

qodo-code-review bot commented Oct 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Prevent concurrent discovery operations

To prevent resource conflicts from simultaneous discovery operations, add a
SemaphoreSlim to each finder class. This will ensure that only one DiscoverAsync
call can execute at a time per instance, improving the robustness of the
classes.

Examples:

src/Daqifi.Core/Device/Discovery/WiFiDeviceFinder.cs [81-177]
    public async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(CancellationToken cancellationToken = default)
    {
        return await DiscoverAsync(TimeSpan.FromSeconds(DefaultTimeoutSeconds), cancellationToken);
    }

    /// <summary>
    /// Discovers devices asynchronously with a timeout.
    /// </summary>
    /// <param name="timeout">The timeout for discovery.</param>
    /// <returns>A task containing the collection of discovered devices.</returns>

 ... (clipped 87 lines)
src/Daqifi.Core/Device/Discovery/SerialDeviceFinder.cs [70-104]
    public async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(CancellationToken cancellationToken = default)
    {
        ThrowIfDisposed();

        var discoveredDevices = new List<IDeviceInfo>();
        var availablePorts = SerialStreamTransport.GetAvailablePortNames();

        foreach (var portName in availablePorts)
        {
            if (cancellationToken.IsCancellationRequested)

 ... (clipped 25 lines)

Solution Walkthrough:

Before:

// In WiFiDeviceFinder.cs
public class WiFiDeviceFinder : IDeviceFinder, IDisposable
{
    // ...
    private async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(TimeSpan timeout, CancellationToken cancellationToken)
    {
        ThrowIfDisposed();

        using var udpTransport = new UdpTransport(_discoveryPort);
        await udpTransport.OpenAsync();

        // ... send broadcast and receive responses

        await udpTransport.CloseAsync();
        OnDiscoveryCompleted();
        return discoveredDevices;
    }
}

After:

// In WiFiDeviceFinder.cs
public class WiFiDeviceFinder : IDeviceFinder, IDisposable
{
    private readonly SemaphoreSlim _discoverySemaphore = new SemaphoreSlim(1, 1);
    // ...
    private async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(TimeSpan timeout, CancellationToken cancellationToken)
    {
        await _discoverySemaphore.WaitAsync(cancellationToken);
        try
        {
            ThrowIfDisposed();
            // ... (rest of the discovery logic)
        }
        finally
        {
            _discoverySemaphore.Release();
        }
    }
}
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a significant concurrency issue in the finder classes that could lead to runtime exceptions from resource conflicts, and proposes a standard, effective solution.

Medium
Possible issue
Fix incorrect timeout in async method
Suggestion Impact:The commit updated DiscoverAsync(CancellationToken) to call the overload with Timeout.InfiniteTimeSpan instead of a 5-second timeout, aligning behavior with the suggestion.

code diff:

     public async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(CancellationToken cancellationToken = default)
     {
-        return await DiscoverAsync(TimeSpan.FromSeconds(DefaultTimeoutSeconds), cancellationToken);
+        return await DiscoverAsync(Timeout.InfiniteTimeSpan, cancellationToken);
     }

In DiscoverAsync(CancellationToken), remove the fixed 5-second timeout by
calling the overload with Timeout.InfiniteTimeSpan to let the cancellationToken
be the sole cancellation trigger.

src/Daqifi.Core/Device/Discovery/WiFiDeviceFinder.cs [81-84]

 public async Task<IEnumerable<IDeviceInfo>> DiscoverAsync(CancellationToken cancellationToken = default)
 {
-    return await DiscoverAsync(TimeSpan.FromSeconds(DefaultTimeoutSeconds), cancellationToken);
+    return await DiscoverAsync(Timeout.InfiniteTimeSpan, cancellationToken);
 }

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the method imposes an unexpected 5-second timeout, which violates the expected behavior of a method overload that only takes a CancellationToken.

Low
  • Update

tylerkron and others added 4 commits October 17, 2025 15:53
…imeout behavior

## Changes

### Concurrency Protection (Importance: 8/10)
- Added `SemaphoreSlim` to all three device finders (WiFi, Serial, HID)
- Prevents race conditions when multiple threads call `DiscoverAsync()` simultaneously
- Ensures only one discovery operation runs at a time per finder instance
- Added proper disposal of semaphore in `Dispose()` methods

### Timeout Behavior Fix (Importance: 6/10)
- Fixed `WiFiDeviceFinder.DiscoverAsync(CancellationToken)` to use `Timeout.InfiniteTimeSpan`
- Previously hardcoded 5-second timeout, now properly respects cancellation token only
- Consolidated timeout logic in private `DiscoverAsync(TimeSpan, CancellationToken)` method
- Added conditional timeout application: only calls `CancelAfter()` when timeout is not infinite

## Testing
- All 23 tests passing on .NET 8.0 and .NET 9.0
- Verified builds successfully

## Qodo PR Review
- ✅ Suggestion #1: Add SemaphoreSlim (importance 8/10)
- ✅ Suggestion #2: Fix timeout behavior (importance 6/10)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
## Changes

### Feature Status Section
- Split features into "✅ Available Now" and "🚧 In Development"
- Moved Device Discovery from roadmap to available features
- Added Transport Layer and Protocol Buffers to available features
- Clarified cross-platform support (.NET 8.0 and 9.0)

### Quick Start Guide
- Added practical device discovery code examples
- Demonstrated WiFi discovery with event handling
- Showed serial device enumeration
- Included real output examples for developers

### Advanced Usage
- Cancellation token examples for fine-grained control
- Custom UDP port configuration
- Error handling patterns

### Documentation Additions
- Supported device types (Nyquist, DAQiFi, Unknown)
- Connection types (WiFi, Serial, HID)
- Real-world usage section highlighting daqifi-desktop
- Requirements section with firewall/port notes

## Motivation

The README was outdated - it listed device discovery as a "roadmap" feature when Phase 4 implementation is now complete. External developers building apps with DAQiFi need clear, accurate documentation showing:
- What's available today vs. planned
- How to actually use the library
- Real code examples they can copy/paste

This update provides the practical guidance developers need to integrate DAQiFi devices into their applications.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Removed misleading "DAQiFi vs Nyquist" distinction. There are only two
DAQiFi hardware products: Nyquist 1 and Nyquist 3. Both are identified
by their part number prefix during discovery.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
…Nyquist3)

## Changes

### DeviceType Enum Update
- Changed from generic `Nyquist` and non-existent `Daqifi` types
- Updated to specific `Nyquist1` and `Nyquist3` to match actual DAQiFi hardware
- Aligns with daqifi-desktop implementation for consistency

### Device Type Detection Logic
- Updated `WiFiDeviceFinder.GetDeviceType()` to use exact part number matching
- "Nq1" → DeviceType.Nyquist1
- "Nq3" → DeviceType.Nyquist3
- Case-insensitive matching using pattern matching switch
- Removed unused "Daq" prefix detection (no such devices exist)

## Motivation

The previous enum was inaccurate:
1. **DeviceType.Daqifi** - No devices with "Daq" part number prefix exist
2. **DeviceType.Nyquist** - Too generic, can't distinguish Nyquist 1 vs Nyquist 3

DAQiFi only manufactures two devices: Nyquist 1 and Nyquist 3. Applications
may need to handle them differently based on their capabilities, so the enum
should reflect the actual hardware models.

This matches the proven implementation in daqifi-desktop where DeviceType
has been Nyquist1 and Nyquist3 since September 2025.

## Testing
- All 215 tests passing on .NET 8.0 and .NET 9.0
- No test changes required (no tests directly referenced enum values)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@tylerkron tylerkron merged commit 82fa4a0 into main Oct 17, 2025
1 check passed
@tylerkron tylerkron deleted the feature/phase-4-device-discovery branch October 17, 2025 22:18
tylerkron added a commit to daqifi/daqifi-desktop that referenced this pull request Oct 17, 2025
## Changes

### Package Updates
- Upgraded `Daqifi.Core` from 0.4.1 to 0.5.0 in:
  - Daqifi.Desktop.IO/Daqifi.Desktop.IO.csproj
  - Daqifi.Desktop/Daqifi.Desktop.csproj

### Documentation
- Added `CORE_0.5.0_UPGRADE.md` with:
  - Overview of Phase 4 device discovery features
  - DeviceType enum compatibility notes
  - Optional integration examples
  - Migration guidance

## What's New in Core 0.5.0

### Device Discovery Framework (Phase 4)
- `IDeviceFinder` interface with async discovery
- `WiFiDeviceFinder` - UDP broadcast discovery (port 30303)
- `SerialDeviceFinder` - USB/Serial port enumeration
- `HidDeviceFinder` - HID bootloader detection (stub)
- `IDeviceInfo` interface for discovered device metadata

### DeviceType Enum Update
Core updated enum to match desktop's existing implementation:
- `Unknown` (0)
- `Nyquist1` (1) - was "Daqifi" in 0.4.1
- `Nyquist3` (2) - was "Nyquist" in 0.4.1

**No breaking changes** - Desktop already uses Nyquist1/Nyquist3.

## Compatibility

✅ **Build successful** - 0 errors related to core upgrade
✅ **No code changes required** - DeviceType enum already compatible
✅ **All desktop functionality preserved** - Fully backward compatible

## Testing

- Desktop builds successfully with Core 0.5.0
- Core 0.5.0 has 215/215 tests passing on .NET 8.0 and 9.0
- DeviceType enum values match between core and desktop

## Migration Status

Per [DAQIFI_CORE_MIGRATION_PLAN.md](DAQIFI_CORE_MIGRATION_PLAN.md):
- ✅ Phase 1: Foundation (Complete)
- ✅ Phase 2: Message System (Complete)
- ✅ Phase 3: Connection Management (Complete)
- ✅ Phase 4: Device Discovery (Complete in core, optional for desktop)
- ⏳ Phase 5-7: Pending
- ⏳ Phase 8: Full desktop integration (future)

## Optional Integration

Desktop can optionally start using Core's device discovery, but this is NOT required. Current desktop device finders continue to work. Full integration is planned for Phase 8.

See `CORE_0.5.0_UPGRADE.md` for examples of using Core device discovery.

## References

- Core 0.5.0 Release: https://www.nuget.org/packages/Daqifi.Core/0.5.0
- Core Phase 4 PR: daqifi/daqifi-core#54
- Migration Plan: DAQIFI_CORE_MIGRATION_PLAN.md

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 4: Device Discovery Framework

2 participants