-
Notifications
You must be signed in to change notification settings - Fork 0
chore: upgrade Daqifi.Core to 0.5.0 (Phase 4 - Device Discovery) #286
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
Conversation
## 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]>
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
PR Code Suggestions ✨Explore these optional code suggestions:
|
This commit completes the actual integration of Core 0.5.0's Phase 4 Device Discovery Framework by replacing all desktop device finder implementations with their Core equivalents. ## Changes ### Files Added - **DeviceInfoConverter.cs** - Bridge layer to convert Core's IDeviceInfo to Desktop device objects - ToWiFiDevice() - Creates DaqifiStreamingDevice from Core WiFi discovery - ToSerialDevice() - Creates SerialStreamingDevice from Core Serial discovery - ToDesktopDeviceInfo() - Converts Core IDeviceInfo to Desktop DeviceInfo datamodel ### Files Deleted (Code Reduction: ~400 lines) - ❌ **DaqifiDeviceFinder.cs** - Replaced by Core's WiFiDeviceFinder - ❌ **SerialDeviceFinder.cs** - Replaced by Core's SerialDeviceFinder - ❌ **HidDeviceFinder.cs** - Replaced by Core's HidDeviceFinder - ❌ **IDeviceFinder.cs** - Replaced by Core's IDeviceFinder interface ### Files Modified **ConnectionDialogViewModel.cs** - Replaced desktop device finders with Core implementations - Added continuous async discovery loops with CancellationToken support - Implemented event handlers to convert Core DeviceDiscovered events to Desktop devices - WiFi discovery now uses Core's UDP broadcast implementation - Serial discovery now uses Core's port enumeration - HID discovery now uses Core's bootloader detection **DaqifiViewModel.cs** - Replaced desktop HidDeviceFinder with Core version - Added async discovery loop for HID devices - Implemented Core event handler conversion ## Benefits ✅ **Code elimination**: Removed ~400 lines of duplicate device discovery code ✅ **Single source of truth**: Device discovery logic now centralized in Core ✅ **Consistent behavior**: Desktop and other apps using Core will discover devices identically ✅ **Better async patterns**: Core uses modern async/await with CancellationToken support ✅ **Reduced warnings**: Build warnings reduced from 758 to 718 ## Testing - [x] Build succeeds with 0 errors - [x] All device discovery types replaced (WiFi, Serial, HID) - [x] Event conversion working (Core IDeviceInfo → Desktop device objects) - [x] Manual IP connection still works with Desktop DeviceInfo ## Migration Path This addresses the feedback from PR review to actually USE Core 0.5.0 features rather than just upgrading the version number. Desktop now leverages Core's Phase 4 Device Discovery Framework as intended. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
… to prevent memory leaks - Explicitly unsubscribe from DeviceDiscovered events before disposing finders - Dispose CancellationTokenSource objects after cancellation - Prevents potential memory leaks from lingering event handlers Per Qodo PR review suggestion.
The duplicate check was comparing SerialPort objects using reference equality (==), which always failed since each discovery creates a new SerialPort instance. This caused the same COM port to be added multiple times to the available devices list. Changed to compare by PortName property (string comparison) to properly detect duplicates. This matches the pattern already used in HandleSerialDeviceRemoved.
Core's SerialDeviceFinder only enumerates ports without probing devices (device info retrieval is Phase 6 work). This caused serial devices in the connection dialog to show minimal info (just port name). Restored the behavior from Desktop's old SerialDeviceFinder: 1. Immediately show device with port name (fast UI response) 2. Probe device in background using TryGetDeviceInfo() 3. Update UI with full device info (name, serial number, firmware version) This matches the original desktop implementation that was working on main.
…cess conflicts The continuous discovery loop (every 2 seconds) was spawning a new background probe task each time it discovered a device, causing multiple overlapping probes: - Probe 1 starts and opens COM4 - Probe 2 starts 2s later and tries to open COM4 -> Access Denied - Probe 3 starts 2s later and tries to open COM4 -> Access Denied - User tries to connect -> Access Denied (port still held by probe 1) Changes: 1. Track probed ports in a HashSet to ensure each port is probed only once 2. Clear the HashSet when stopping discovery so ports can be re-probed next time 3. Remove port from HashSet if probe fails so we can retry 4. Handle ObjectDisposedException in discovery loops (happens when finder is disposed while discovery is in progress during shutdown) This matches the old desktop SerialDeviceFinder behavior of probing once per device.
…serting After successfully probing a serial device for full info, the UI wasn't updating because we were replacing the item with itself using collection[index] = item. This doesn't always trigger the proper CollectionChanged notifications for WPF to rebind. Changed to remove and re-insert at the same index, which: - Triggers CollectionChanged with Remove action - Triggers CollectionChanged with Insert action - Forces WPF to rebind to the item and refresh all property bindings - Maintains the item's position in the list The logs showed the probe was succeeding but UI wasn't updating.
The delay was cargo-culted from the old implementation but serves no purpose: - Device is already added to the collection synchronously before spawning the background task - The probe runs on a background thread, so it doesn't block the UI - The delay just makes the device info appear slower (500ms later) for no benefit Removed the delay entirely - device info now appears as fast as the probe completes.
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 10.5%
Daqifi.Desktop.Bootloader - 20.6%
Daqifi.Desktop.Common - 45.9%
Daqifi.Desktop.DataModel - 100%
Daqifi.Desktop.IO - 23.9%
Coverage report generated by ReportGenerator • View full report in build artifacts |
Updates DAQIFI_CORE_MIGRATION_PLAN.md to reflect current progress: ## Current State Updates - Updated Core version: 0.4.1 → main branch (includes 0.5.0) - Updated Desktop: Using Core 0.5.0 (upgraded from 0.4.1) - Added Phase 4 integration success (-270 lines via PR #286) - Noted Phase 5 is now in progress ## Phase Status Changes ### Phase 3: Connection Management ✅ - Marked as Complete - All transport types implemented (TCP, Serial, UDP) - Connection retry logic with ConnectionRetryOptions - Thread-safe operations - Cross-platform compatible ### Phase 4: Device Discovery Framework ✅ - Marked as Complete (Core 0.5.0) - Desktop successfully integrated (PR #286) - Eliminated ~400 lines of duplicate discovery code - Net reduction of 270 lines - All device finders replaced with Core implementations ### Phase 5: Channel Management 🔄 - Updated status from "Not started" to "In Progress" - Core implementation complete (daqifi-core PR #57) - Ready for desktop integration - Expected to follow Phase 4 pattern ## Benefits - Accurate project status for stakeholders - Clear completion criteria for each phase - Documents successful Phase 4 integration pattern - Prepares for Phase 5 desktop integration ## Related - Core Phase 5 PR: daqifi/daqifi-core#57 - Desktop Phase 4 PR: #286 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Upgrades
Daqifi.Core
from 0.4.1 to 0.5.0 and directly integrates Phase 4: Device Discovery Framework by replacing all desktop device finder implementations with Core equivalents.Key Achievement: ✅ -270 lines of code - Eliminated duplicate device discovery logic from desktop
Changes
Package Updates
Daqifi.Core
0.4.1 → 0.5.0 in IO and Desktop projectsCode Integration (NOT just version upgrade)
Files Added
IDeviceInfo
to Desktop device objectsToWiFiDevice()
- CreatesDaqifiStreamingDevice
from Core WiFi discoveryToSerialDevice()
- CreatesSerialStreamingDevice
from Core Serial discoveryToDesktopDeviceInfo()
- Converts CoreIDeviceInfo
to DesktopDeviceInfo
datamodelFiles Deleted (~400 lines eliminated)
WiFiDeviceFinder
SerialDeviceFinder
HidDeviceFinder
IDeviceFinder
interfaceFiles Modified
ConnectionDialogViewModel.cs - Replaced all desktop device finders with Core implementations
CancellationToken
supportDeviceDiscovered
events to Desktop devicesDaqifiViewModel.cs - Replaced HID device finder with Core version
Documentation
CORE_0.5.0_UPGRADE.md
- Upgrade notes (now optional reference since we already integrated)What's New in Core 0.5.0
Device Discovery Framework (Phase 4)
Desktop now uses Core's complete device discovery system:
IDeviceFinder
interface - Base discovery interfaceWiFiDeviceFinder
- UDP broadcast discovery (port 30303)SerialDeviceFinder
- USB/Serial port enumerationHidDeviceFinder
- HID bootloader detectionIDeviceInfo
- Device metadata interfaceDeviceDiscovered
,DiscoveryCompleted
CancellationToken
andTimeSpan
timeout supportDeviceType Enum
Core 0.5.0 updated its enum to match desktop's existing implementation:
No breaking changes - Desktop already used these exact values.
Integration Benefits
✅ Single source of truth - Device discovery logic centralized in Core
✅ Code elimination - Net reduction of 270 lines (536 deletions, 266 additions)
✅ Consistent behavior - All apps using Core discover devices identically
✅ Better async patterns - Modern async/await with cancellation support
✅ Reduced warnings - Build warnings: 758 → 718 (-40 warnings)
✅ Simplified maintenance - One codebase to fix bugs and add features
Technical Implementation
Before (Desktop Device Finders)
After (Core Device Finders)
Compatibility & Testing
✅ Fully Compatible
Discovery Types Migrated
Migration Status
Per DAQIFI_CORE_MIGRATION_PLAN.md:
Commit Breakdown
References
Testing Checklist
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]