Skip to content

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

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.

Sign up for GitHub

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

Open
tylerkron wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 87 additions & 3 deletions Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,18 @@ private static async Task<int> Main(string[] args)
return await RunFirmwareDownloadByTagAsync(options);
}

// Check if we have a connection target (IP or serial)
// 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;
}
Comment on lines +72 to 86
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Expand All @@ -91,6 +91,12 @@ private static async Task<int> Main(string[] args)
return 1;
}

if (options.ViaDiscovery && hasSerialTarget)
{
Console.Error.WriteLine("--via-discovery is only valid for WiFi devices and cannot be combined with --serial.");
return 1;
}

if (hasIpTarget)
{
var ipAddress = options.IpAddress!.Trim();
Expand Down Expand Up @@ -157,6 +163,15 @@ private static async Task<int> RunStreamingSessionAsync(CliOptions options)
connectionOptions);
connectionDescription = $"{options.SerialPort} @ {options.BaudRate} baud";
}
else if (options.ViaDiscovery)
{
var (discoveredDevice, deviceInfo) = await ConnectViaDiscoveryAsync(options, connectionOptions);
device = discoveredDevice;
var ifaceStr = deviceInfo.LocalInterfaceAddress?.ToString() ?? "(none — OS-chosen)";
connectionDescription =
$"{deviceInfo.Name} at {deviceInfo.IPAddress}:{deviceInfo.Port} via discovery, " +
$"bound to local interface {ifaceStr}";
}
else
{
device = await DaqifiDeviceFactory.ConnectTcpAsync(
Expand Down Expand Up @@ -324,6 +339,64 @@ private static async Task<int> RunStreamingSessionAsync(CliOptions options)
}
}

private static async Task<(DaqifiDevice Device, IDeviceInfo DeviceInfo)> ConnectViaDiscoveryAsync(
CliOptions options,
DeviceConnectionOptions connectionOptions)
{
IPAddress? targetIp = null;
if (!string.IsNullOrWhiteSpace(options.IpAddress))
{
if (!IPAddress.TryParse(options.IpAddress, out targetIp))
{
throw new InvalidOperationException($"Invalid --ip value: {options.IpAddress}");
}
}

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();

IDeviceInfo? match;
if (targetIp != null)
{
match = discovered.FirstOrDefault(d => Equals(d.IPAddress, targetIp));
if (match == null)
{
var seen = string.Join(", ", discovered.Select(d => d.IPAddress?.ToString() ?? "?"));
throw new InvalidOperationException(
$"Target device {targetIp} was not discovered. Discovered: [{seen}]");
}
}
else
{
if (discovered.Count == 0)
{
throw new InvalidOperationException(
"No WiFi devices discovered. Pass --ip to filter to a specific device or increase --discover-timeout.");
}

if (discovered.Count > 1)
{
var summary = string.Join(", ", discovered.Select(d => $"{d.Name}@{d.IPAddress}"));
Console.WriteLine(
$"Discovered {discovered.Count} devices ({summary}); connecting to the first. " +
"Pass --ip to choose a specific device.");
}

match = discovered[0];
}

Console.WriteLine(
$"Discovered {match.Name} at {match.IPAddress}:{match.Port} " +
$"(LocalInterfaceAddress={match.LocalInterfaceAddress?.ToString() ?? "null"})");

var device = await DaqifiDeviceFactory.ConnectFromDeviceInfoAsync(match, connectionOptions);
return (device, match);
}

private static async Task<int> RunFirmwareUpdateAsync(
CliOptions options,
string? firmwareHexPathOverride = null)
Expand Down Expand Up @@ -1390,12 +1463,19 @@ private static void PrintHelp()
Console.WriteLine("Usage:");
Console.WriteLine(" dotnet run -- --ip <address> [options]");
Console.WriteLine(" dotnet run -- --serial <port> [options]");
Console.WriteLine(" dotnet run -- --via-discovery [--ip <address>] [options]");
Console.WriteLine();
Console.WriteLine("Connection Options:");
Console.WriteLine(" --ip <address> Device IP address (for TCP/WiFi connection).");
Console.WriteLine($" --port <number> TCP port (default: {DefaultPort}).");
Console.WriteLine(" --serial <port> Serial port name (e.g., COM3, /dev/ttyUSB0, /dev/cu.usbmodem101).");
Console.WriteLine($" --baud <rate> Baud rate for serial connection (default: {DefaultBaudRate}).");
Console.WriteLine(" --via-discovery Discover WiFi devices over UDP, then connect via");
Console.WriteLine(" DaqifiDeviceFactory.ConnectFromDeviceInfoAsync. This");
Console.WriteLine(" binds the outbound socket to the NIC that received the");
Console.WriteLine(" discovery reply (required on multi-homed hosts). If --ip");
Console.WriteLine(" is set, filter discovery to that address; otherwise");
Console.WriteLine(" connect to the first discovered device.");
Console.WriteLine();
Console.WriteLine("Discovery Options:");
Console.WriteLine(" -d, --discover Discover WiFi devices over UDP.");
Expand Down Expand Up @@ -1502,6 +1582,7 @@ private sealed class CliOptions
public string? FirmwareHexPath { get; private set; }
public string? FirmwareUpdateLatestDirectory { get; private set; }
public bool LanChipInfo { get; private set; }
public bool ViaDiscovery { get; private set; }
public List<string> Errors { get; } = new();

public static CliOptions Parse(string[] args)
Expand Down Expand Up @@ -1620,6 +1701,9 @@ public static CliOptions Parse(string[] args)
case "--lan-chip-info":
options.LanChipInfo = true;
break;
case "--via-discovery":
options.ViaDiscovery = true;
break;
case "-h":
case "--help":
options.ShowHelp = true;
Expand Down
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,22 @@ dotnet run -- \
--duration 10
```

Discover and stream (canonical "find then connect" pattern; required on multi-homed hosts):

```bash
dotnet run -- \
--via-discovery \
--rate 10 \
--channels 0000000011 \
--duration 10
```

## Options

- `--discover` discover devices over UDP
- `--ip <address>` device IP address
- `--port <number>` TCP port (default 9760)
- `--via-discovery` discover first, then connect via `DaqifiDeviceFactory.ConnectFromDeviceInfoAsync`. Required on multi-homed hosts so the outbound socket binds to the NIC that received the discovery reply. With `--ip`, filters to that address; without `--ip`, connects to the first discovered device.
- `--rate <hz>` sampling rate in Hz (default 100)
- `--duration <seconds>` streaming duration (default 10)
- `--channels <mask>` ADC channel enable mask (0/1 string)
Expand Down