feat: add --via-discovery for canonical "find then connect" flow

[tylerkron]

Summary

Adds a new --via-discovery flag that routes the WiFi streaming session through WiFiDeviceFinder.DiscoverAsync + DaqifiDeviceFactory.ConnectFromDeviceInfoAsync instead of the direct ConnectTcpAsync call.

This serves two purposes:

1. Pedagogical — ConnectFromDeviceInfoAsync is the canonical WiFi entry point in Daqifi.Core (find the device, then connect to it). The current --ip example only demonstrates the "I already know the IP" path. Real consumers should generally discover first; this example now shows them how.
2. Hardware-in-the-loop validation — this is the only path that honors IDeviceInfo.LocalInterfaceAddress, which controls which NIC the outbound socket binds to. Required on multi-homed hosts (see daqifi-core#187). Without an example-app path that exercises this, regressions to that behavior would not be caught by hardware tests.

UX

Flags	Behavior
--via-discovery (alone)	Discover; connect to the first device found.
--via-discovery --ip <addr>	Discover; filter to the device matching <addr>.
--via-discovery --serial <port>	Rejected — WiFi only.
--ip <addr> (no --via-discovery)	Unchanged — direct ConnectTcpAsync.
--serial <port>	Unchanged.

The change is purely additive: existing --ip and --serial paths and their callers are untouched.

Example output

$ dotnet run -- --via-discovery --discover-timeout 15 --rate 10 --channels 7 --duration 5 --min-samples 10
Discovering WiFi devices (timeout 15s)...
Discovered NYQUIST1 at 192.168.1.39:9760 (LocalInterfaceAddress=192.168.1.29)
Connected to NYQUIST1 at 192.168.1.39:9760 via discovery, bound to local interface 192.168.1.29
Streaming at 10 Hz...
...

Test plan

• dotnet build — clean (against daqifi-core PR #187 sources)
• --via-discovery (no --ip) — discovers and connects to single device on network, prints LocalInterfaceAddress
• --via-discovery --ip 192.168.1.39 — discovers, filters to that IP, connects
• --via-discovery --serial /dev/cu.usbmodem101 — rejected with a clear error
• Existing --ip path — unchanged, still works
• Existing --serial path — unchanged, still works (29 samples in 3s)

Note: under hardware testing, the device's WiFi side stopped emitting stream messages partway through (USB serial kept working). This affected the legacy --ip path identically, so it's a device-state issue and not a regression from this change. The successful run above (hundreds of samples) was captured prior to the WiFi hang.

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Add --via-discovery flag for canonical WiFi find-then-connect flow

✨ Enhancement

Walkthroughs

Description

• Add --via-discovery flag for canonical WiFi device discovery and connection flow
• Route streaming through WiFiDeviceFinder.DiscoverAsync +
  DaqifiDeviceFactory.ConnectFromDeviceInfoAsync
• Support filtering discovered devices by IP address with --via-discovery --ip <addr>
• Validate that --via-discovery cannot be combined with --serial (WiFi-only)
• Update help documentation and README with discovery usage examples

Diagram

flowchart LR
  A["CLI Arguments"] --> B{"--via-discovery?"}
  B -->|Yes| C["WiFiDeviceFinder.DiscoverAsync"]
  B -->|No| D["Direct ConnectTcpAsync or ConnectSerialAsync"]
  C --> E{"--ip specified?"}
  E -->|Yes| F["Filter to matching IP"]
  E -->|No| G["Use first device"]
  F --> H["ConnectFromDeviceInfoAsync"]
  G --> H
  H --> I["Bind to LocalInterfaceAddress"]
  D --> J["Stream Session"]
  I --> J

Loading

File Changes

1. Program.cs ✨ Enhancement +87/-3

Add --via-discovery flag and discovery connection flow

• Add ViaDiscovery boolean property to CliOptions class and parse --via-discovery flag
• Update validation logic to accept --via-discovery as a valid connection target and reject
 combination with --serial
• Implement new ConnectViaDiscoveryAsync method that discovers WiFi devices, optionally filters by
 IP, and connects via ConnectFromDeviceInfoAsync
• Integrate discovery path into RunStreamingSessionAsync to display device name, IP, port, and
 LocalInterfaceAddress
• Expand help text to document --via-discovery flag with detailed explanation of multi-homed host
 requirements

Program.cs

---

2. README.md 📝 Documentation +11/-0

Document --via-discovery usage and examples

• Add new example showing --via-discovery usage for canonical find-then-connect pattern
• Document --via-discovery option with explanation of multi-homed host requirements
• Clarify that --via-discovery with --ip filters to specific address, without --ip connects to
 first device

README.md

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0)

1. Discovery allowed for non-stream commands 🐞 Bug ≡ Correctness

Description

Main() now accepts --via-discovery as a valid connection target globally, but SD ops /
capture-and-parse / LAN chip info / firmware update paths still connect via
ConnectTcpAsync(options.IpAddress!) when --serial is absent. Invocations like `--via-discovery
--sd-list` can dereference a null IpAddress and throw before any try/catch, crashing the CLI.

Code

Program.cs[R72-86]

+        // Check if we have a connection target (IP, serial, or via-discovery)
        var hasIpTarget = !string.IsNullOrWhiteSpace(options.IpAddress);
        var hasSerialTarget = !string.IsNullOrWhiteSpace(options.SerialPort);

-        if (!hasIpTarget && !hasSerialTarget)
+        if (!hasIpTarget && !hasSerialTarget && !options.ViaDiscovery)
        {
            if (options.Discover || options.DiscoverSerial)
            {
                return 0;
            }

-            Console.Error.WriteLine("Missing required option: --ip or --serial");
+            Console.Error.WriteLine("Missing required option: --ip, --serial, or --via-discovery");
            Console.Error.WriteLine("Use --help to see available options.");
            return 1;
        }

Evidence

Main() now allows proceeding without --ip/--serial when --via-discovery is present, but routing
still invokes command handlers that unconditionally call ConnectTcpAsync(options.IpAddress!) when
SerialPort is empty; these TCP connects occur before the handlers’ try/catch blocks, so a null
IpAddress can crash the program.

Program.cs[72-86]
Program.cs[110-139]
Program.cs[713-744]
Program.cs[1122-1152]
Program.cs[1234-1264]
Program.cs[400-439]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`--via-discovery` is treated as a valid "connection target" for the entire CLI, but only `RunStreamingSessionAsync` implements the discovery connection branch. Other command paths still call `ConnectTcpAsync(options.IpAddress!)` (or similar) and will crash when `IpAddress` is null (e.g., `--via-discovery --sd-list`).

### Issue Context
The connection target validation happens before routing to firmware update, capture-and-parse, SD operations, and LAN chip info. Those commands currently require `--ip` or `--serial` (and some are serial-only by design), but `--via-discovery` now bypasses that requirement.

### Fix Focus Areas
- Program.cs[72-139]
- Program.cs[713-744]
- Program.cs[1122-1152]
- Program.cs[1234-1264]
- Program.cs[400-439]

### What to change
Choose one of these approaches:
1) **Restrict `--via-discovery` to streaming only (recommended for minimal change):**
  - Keep the global requirement as `--ip` or `--serial` for non-streaming commands.
  - Add an explicit validation in `Main()` such as:
    - If `options.ViaDiscovery` is set AND any non-streaming command flag is selected (SD ops, capture-and-parse, firmware update, LAN chip info), then print a clear error and exit 1.
  - Only allow `--via-discovery` to satisfy the missing-target gate when the program will route to `RunStreamingSessionAsync`.

2) **Implement via-discovery connection for each command:**
  - Add `else if (options.ViaDiscovery)` branches to `RunSdCardOperationAsync`, `RunCaptureAndParseAsync`, `RunLanChipInfoAsync`, `RunFirmwareUpdateAsync` (and any other command using TCP) similar to the streaming implementation, or explicitly reject in serial-only scenarios.

Either way, ensure no command can reach `ConnectTcpAsync(options.IpAddress!)` when `IpAddress` is null.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Untrimmed IP reparsed 🐞 Bug ☼ Reliability

Description

Main() validates a trimmed --ip value, but ConnectViaDiscoveryAsync re-parses the untrimmed string
and can reject inputs that already passed validation (e.g., leading/trailing whitespace). This
creates inconsistent behavior and a confusing error path for --via-discovery --ip users.

Code

Program.cs[R346-353]

+        IPAddress? targetIp = null;
+        if (!string.IsNullOrWhiteSpace(options.IpAddress))
+        {
+            if (!IPAddress.TryParse(options.IpAddress, out targetIp))
+            {
+                throw new InvalidOperationException($"Invalid --ip value: {options.IpAddress}");
+            }
+        }

Evidence

Main() trims before IPAddress.TryParse, but ConnectViaDiscoveryAsync uses options.IpAddress directly
when parsing, creating inconsistent acceptance of the same user input string.

Program.cs[100-108]
Program.cs[346-353]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Main()` trims `options.IpAddress` before validating it, but `ConnectViaDiscoveryAsync()` re-parses `options.IpAddress` without trimming. As a result, the same CLI input can be accepted by `Main()` and then rejected later during discovery connect.

### Issue Context
This only affects the `--via-discovery --ip <addr>` path. The exception is caught higher up, but it is still an avoidable inconsistency.

### Fix Focus Areas
- Program.cs[100-108]
- Program.cs[346-353]

### What to change
In `ConnectViaDiscoveryAsync`, normalize the IP string the same way as `Main()`:
- Use `var ipStr = options.IpAddress.Trim();`
- Then `IPAddress.TryParse(ipStr, out targetIp)`
Optionally, centralize IP normalization/validation so it happens exactly once and the normalized value is reused.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Discovery timeout log mismatch 🐞 Bug ◔ Observability

Description

ConnectViaDiscoveryAsync logs the raw DiscoveryTimeoutSeconds value, but the effective timeout is
forced to 5s when DiscoveryTimeoutSeconds <= 0. This can mislead debugging when users pass
0/negative values (log says 0s, code uses 5s).

Code

Program.cs[R355-360]

+        var filterDescription = targetIp != null ? $" matching {targetIp}" : string.Empty;
+        Console.WriteLine($"Discovering WiFi devices{filterDescription} (timeout {options.DiscoveryTimeoutSeconds}s)...");
+
+        using var finder = new WiFiDeviceFinder();
+        var timeout = TimeSpan.FromSeconds(options.DiscoveryTimeoutSeconds <= 0 ? 5 : options.DiscoveryTimeoutSeconds);
+        var discovered = (await finder.DiscoverAsync(timeout)).ToList();

Evidence

The console message prints options.DiscoveryTimeoutSeconds, but the code immediately uses a
different effective timeout when the value is <= 0.

Program.cs[355-360]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The discovery connect path prints the configured timeout seconds, but then computes an effective timeout that differs when `DiscoveryTimeoutSeconds <= 0` (fallback to 5 seconds). The log output can therefore be wrong.

### Issue Context
This affects the console output for `--via-discovery` and makes troubleshooting discovery behavior harder.

### Fix Focus Areas
- Program.cs[355-360]

### What to change
Compute `timeout` first, then log using the effective value, e.g.:
- `var timeoutSeconds = options.DiscoveryTimeoutSeconds <= 0 ? 5 : options.DiscoveryTimeoutSeconds;`
- `var timeout = TimeSpan.FromSeconds(timeoutSeconds);`
- Print `timeoutSeconds` (or `timeout.TotalSeconds`) in the message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

1. Discovery allowed for non-stream commands 🐞 Bug ≡ Correctness

Main() now accepts --via-discovery as a valid connection target globally, but SD ops /
capture-and-parse / LAN chip info / firmware update paths still connect via
ConnectTcpAsync(options.IpAddress!) when --serial is absent. Invocations like `--via-discovery
--sd-list` can dereference a null IpAddress and throw before any try/catch, crashing the CLI.

Agent Prompt

### Issue description
`--via-discovery` is treated as a valid "connection target" for the entire CLI, but only `RunStreamingSessionAsync` implements the discovery connection branch. Other command paths still call `ConnectTcpAsync(options.IpAddress!)` (or similar) and will crash when `IpAddress` is null (e.g., `--via-discovery --sd-list`).

### Issue Context
The connection target validation happens before routing to firmware update, capture-and-parse, SD operations, and LAN chip info. Those commands currently require `--ip` or `--serial` (and some are serial-only by design), but `--via-discovery` now bypasses that requirement.

### Fix Focus Areas
- Program.cs[72-139]
- Program.cs[713-744]
- Program.cs[1122-1152]
- Program.cs[1234-1264]
- Program.cs[400-439]

### What to change
Choose one of these approaches:
1) **Restrict `--via-discovery` to streaming only (recommended for minimal change):**
   - Keep the global requirement as `--ip` or `--serial` for non-streaming commands.
   - Add an explicit validation in `Main()` such as:
     - If `options.ViaDiscovery` is set AND any non-streaming command flag is selected (SD ops, capture-and-parse, firmware update, LAN chip info), then print a clear error and exit 1.
   - Only allow `--via-discovery` to satisfy the missing-target gate when the program will route to `RunStreamingSessionAsync`.

2) **Implement via-discovery connection for each command:**
   - Add `else if (options.ViaDiscovery)` branches to `RunSdCardOperationAsync`, `RunCaptureAndParseAsync`, `RunLanChipInfoAsync`, `RunFirmwareUpdateAsync` (and any other command using TCP) similar to the streaming implementation, or explicitly reject in serial-only scenarios.

Either way, ensure no command can reach `ConnectTcpAsync(options.IpAddress!)` when `IpAddress` is null.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools