-
Notifications
You must be signed in to change notification settings - Fork 0
fix: resolve UDP discovery and TCP connection issues in multi-NIC environments #173
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
base: main
Are you sure you want to change the base?
Conversation
The WiFi module restarts when ApplyNetworkLan is called, which disables the WiFi. This fix adds logic to re-enable WiFi after the module has restarted, but only when in StreamToApp mode. Changes: - Add using System.Threading for Thread.Sleep - Ensure SD is disabled and WiFi enabled before configuration - Add 2-second delay after ApplyNetworkLan for module restart - Re-enable WiFi after restart ONLY if in StreamToApp mode - Respect hardware limitation that SD and WiFi share same SPI bus This fixes issue #169 where WiFi remains disabled after applying network configuration changes, while preventing conflicts with SD card logging mode. 🤖 Generated with Claude Code (https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…ironments - Add backward-compatible UDP discovery that handles firmware responding to port 30303 - Create multi-interface UDP broadcaster to send discovery from all network adapters - Track which local interface discovered each device - Bind TCP connections to the correct local interface to prevent routing issues - Fix connection timeouts when multiple NICs have routes to same IP (192.168.1.1) This resolves device discovery failures when using WiFi adapters and connection timeouts when both router and DAQiFi device use the same IP on different interfaces. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesFootnotes
|
|
/review |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
/improve |
PR Code Suggestions ✨Latest suggestions up to 70bf53f
Previous suggestions✅ Suggestions up to commit f48448f
Suggestions up to commit 311b056
✅ Suggestions up to commit bcb6549
✅ Suggestions up to commit 300eee2
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…ssion - Restore multi-interface UDP discovery that was accidentally reverted in merge - Simplify architecture by consolidating 3 device finder classes into 1 - Improve robustness with better error handling and connection management - Add proper TCP connection cleanup on timeout - Fix thread safety with collection copying during broadcast - Enhanced logging for debugging multi-NIC scenarios The merge with main had reverted the device finder from the multi-interface version back to single-interface, causing discovery to only work on one NIC. This commit restores full multi-NIC support with backward compatibility for both old firmware (port 30303) and new firmware (source port response). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix receive loop re-arming: Move BeginReceive to finally block to ensure the receive loop continues even if ProcessDiscoveryResponse throws - Improve UDP receiver binding: Explicitly bind to IPv4 Any endpoint for better cross-platform compatibility - Add AsyncWaitHandle disposal: Properly dispose wait handles to prevent resource leaks in TCP connection - Add NoDelay setting: Enable TCP NoDelay for lower latency - Enhanced error handling: Add SocketException handling and null checks These changes address the valid suggestions from the Qodo code review to make the multi-NIC discovery implementation more robust. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix UDP receiver to properly bind to port 30303 by passing endpoint directly to UdpClient constructor - Add 5-minute TTL for discovered devices to handle IP changes and allow rediscovery after network changes - Clean up expired device entries automatically during discovery - Update timestamps for existing devices to keep them alive This addresses remaining Qodo review comments about port binding and device deduplication without TTL. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Set socket options BEFORE binding for maximum compatibility - Create UdpClient with AddressFamily first, then set options, then bind - Add Windows-specific ExclusiveAddressUse=false for better multi-process support - This ensures the UDP receiver works reliably across different platforms This addresses the final Qodo suggestion about legacy UDP bind robustness. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 6.5%
Daqifi.Desktop.Bootloader - 20%
Daqifi.Desktop.Common - 62.9%
Daqifi.Desktop.DataModel - 0%
Daqifi.Desktop.IO - 24.1%
Coverage report generated by ReportGenerator • View full report in build artifacts |
|
Fixed feature regression from merge(s). Tested on multiple NICs. |
Summary
Problem
Solution
DaqifiDeviceFinderMultiInterfaceto broadcast from ALL network interfacesDaqifiDeviceFinderBackwardCompatiblethat:DaqifiStreamingDeviceto bind TCP connections to the correct local interfaceTest plan
Breaking changes
None - maintains full backward compatibility
🤖 Generated with Claude Code