setup, discover --save: save the LAN address or usb, not the USB link

README.md

@@ -667,6 +667,9 @@ one that is already done:
    none or more than one.
 2. **Network**: a pod with no address gets one. Choose Ethernet (setup waits up to
    two minutes for its DHCP lease) or Wi-Fi (SSID and password, like `set-wifi`).
+   A pod this computer reaches over its USB network link (firmware 3.9.0+) already
+   works from here, so setup goes on and says how to add Ethernet or Wi-Fi for other
+   machines and CI.
 3. **Board I/O voltage**: 1.8 V or 3.3 V, the DUT's logic level (like `la voltage`).
    One already set is kept unless `--la-voltage` is given. The pod forgets it on a
    restart, so when this machine is signed in and the pod is registered to that
@@ -679,7 +682,13 @@ one that is already done:
    voltage. A pod registered to your account is named; one registered to another
    account is explained (sign in with the address your activation email was sent to,
    `benchpod login --force`) and left alone.
-5. **Save the connection**: the pod's address becomes the default (like `discover --save`).
+5. **Save the connection** (like `discover --save`): the pod's Ethernet or Wi-Fi
+   address becomes the default when it has one, so the default keeps working from other
+   machines and after the pod moves to another desk or lab. A pod that only has its USB
+   network link (firmware 3.9.0+, no Ethernet or Wi-Fi) is saved as `usb`, not as its
+   `10.237.x.y` address, which only works while it is plugged into this computer. A
+   `--connection` given to setup is saved as it is. The `pytest` line in the next steps
+   uses the saved connection.
 
 It ends with the next steps: SWD wiring (SWCLK on LA11, SWDIO on LA12, GND), a
 `benchpod flash` line, the `pytest` line and the getting-started guide.
@@ -731,12 +740,15 @@ a registered pod is also checked against that account: discover names it when it
 yours, and says so when it is registered to a different account (usually a second
 account; a pod set up for you is on the account your activation email was sent to).
 A pod advertises itself on each of its interfaces; when this computer hears it on the
-pod's USB network link (firmware 3.9.0+), discover shows and uses that address rather
-than the LAN one, and `--save` stores `usb` for it.
+pod's USB network link (firmware 3.9.0+), discover shows and uses that address and
+lists the LAN one next to it (`LAN address 192.168.1.220:8080`). `--save` stores the
+LAN address when the pod has one, and `usb` when it was only heard on its USB link: a
+`10.237.x.y` address only works while the pod is plugged into this computer, so it is
+never saved; `usb` finds the pod again on any port.
 
 | Flag              | Default | Purpose                                                         |
 |-------------------|---------|-----------------------------------------------------------------|
-| `--save`          | off     | Save the pod as the default connection when exactly one is found (the network address first; `usb` for a pod on its USB link). |
+| `--save`          | off     | Save the pod as the default connection when exactly one is found: its Ethernet or Wi-Fi address first, `usb` for a pod only on its USB link, else its USB console device. |
 | `--no-usb`        | off     | Skip probing USB ports.                                          |
 | `--no-network`    | off     | Skip mDNS browsing of the LAN.                                   |
 | `--wait`          | `3s`    | How long to browse the LAN for mDNS replies.                     |

cmd/benchpod-cli/commands_discover.go

@@ -39,7 +39,11 @@ type discoveredPod struct {
 	instance string // DNS-SD instance, e.g. "BenchPod a1b2c3"
 	hostname string // "benchpod-a1b2c3.local."
 	addr     string // "host:port", a numeric IPv4 when one was resolved
-	id       string // full base64url Ed25519 pubkey (TXT id=)
+	// lanAddr is the pod's "host:port" on Ethernet or Wi-Fi, "" when it was only heard on its USB
+	// network link. addr prefers the link (it is how this computer reaches the pod fastest);
+	// lanAddr is what gets saved, since it works from other machines and after the pod is unplugged.
+	lanAddr string
+	id      string // full base64url Ed25519 pubkey (TXT id=)
 
 	// Filled in by the health check.
 	reachable bool
@@ -78,8 +82,11 @@ func newDiscoverCmd(g *globalFlags) *cobra.Command {
 			"is reachable both ways the two entries are matched up as one physical pod.\n" +
 			"When this machine is signed in (`benchpod login`), a registered pod is also\n" +
 			"checked against that account, so a pod on another account is called out.\n\n" +
-			"With --save and exactly one pod present, its address is stored as the default\n" +
-			"connection (like set-connection), preferring the network address.\n\n" +
+			"With --save and exactly one pod present, it is stored as the default connection\n" +
+			"(like set-connection): its Ethernet or Wi-Fi address when it has one, else `usb`\n" +
+			"for a pod only reached over its USB network link (that 10.237.x.y address only\n" +
+			"works while the pod is plugged into this computer; `usb` finds it again), else\n" +
+			"its USB console device.\n\n" +
 			"mDNS is link-local: it works on a flat bench/office subnet but does not cross\n" +
 			"routers/VLANs and is often blocked on CI runners. USB probing needs no network\n" +
 			"at all, so a pod that is plugged in always shows up even when mDNS is blocked.",
@@ -243,6 +250,9 @@ func discoverPods(wait time.Duration) ([]discoveredPod, error) {
 				host = ip.String()
 			}
 			p.addr = benchpodconfig.EnsurePort(fmt.Sprintf("%s:%d", host, e.Port))
+			if ip := lanPodIP(ips[e.Instance]); ip != nil {
+				p.lanAddr = benchpodconfig.EnsurePort(fmt.Sprintf("%s:%d", ip, e.Port))
+			}
 			byKey[e.Instance] = p
 		}
 	}()
@@ -296,6 +306,46 @@ func preferredPodIP(ips []net.IP) net.IP {
 	return nil
 }
 
+// lanPodIP is the first address mDNS returned for a pod that is not on its USB link: its
+// Ethernet or Wi-Fi address. nil when the pod was only heard over USB.
+func lanPodIP(ips []net.IP) net.IP {
+	for _, ip := range ips {
+		if !usbLinkNet.Contains(ip) {
+			return ip
+		}
+	}
+	return nil
+}
+
+// isUSBLinkAddr reports whether a "host[:port]" is on a pod's USB network link, which only the
+// computer the pod is plugged into reaches.
+func isUSBLinkAddr(addr string) bool {
+	host, _, err := net.SplitHostPort(addr)
+	if err != nil {
+		host = addr
+	}
+	ip := net.ParseIP(host)
+	return ip != nil && usbLinkNet.Contains(ip)
+}
+
+// savedConnection is the connection to store as the default for a pod reached at addr, with
+// lanAddr its Ethernet or Wi-Fi address ("" when none is known). The saved default has to keep
+// working when the pod moves (set up at a desk over USB, then cabled into a lab network), so:
+//   - the pod's LAN address when it has one: it works from any machine on that network and from CI;
+//   - else `usb` for a pod reached on its USB network link (10.237.x.y): that address only works
+//     while the pod is plugged into this computer, and `usb` finds the pod again on any port (and
+//     falls back to its console when Wi-Fi turns USB networking off);
+//   - else addr as it is.
+func savedConnection(addr, lanAddr string) string {
+	switch {
+	case lanAddr != "":
+		return lanAddr
+	case isUSBLinkAddr(addr):
+		return "usb"
+	}
+	return addr
+}
+
 // multicastIfaces lists the interfaces worth browsing for mDNS: up, multicast
 // capable, not loopback, and carrying an IPv4 address. Loopback is excluded
 // because a pod is never on it and including it is one of the ways the default
@@ -677,6 +727,9 @@ func describeHealth(p discoveredPod) string {
 			parts = append(parts, fmt.Sprintf("registered, cloud %s", strings.ToLower(strings.TrimSpace(p.cloud.State))))
 		}
 	}
+	if p.lanAddr != "" && p.lanAddr != p.addr {
+		parts = append(parts, "LAN address "+p.lanAddr)
+	}
 	if p.sameAs != "" {
 		parts = append(parts, "same pod as "+p.sameAs)
 	}
@@ -711,19 +764,14 @@ func plural(n int, one, many string) string {
 	return fmt.Sprintf("%d %s", n, many)
 }
 
-// saveTarget is the connection string --save would store: the network address
-// of the single pod found (preferred — it works from other machines and from
-// CI), else the serial device of the single pod found over USB. It returns ""
-// when the choice is ambiguous, so --save never guesses between two pods. A pod
-// heard on its USB network link is saved as `usb`: its link address is only
-// reachable from this computer, and `usb` keeps working (over the console) when
-// Wi-Fi turns the pod's USB networking off.
+// saveTarget is the connection string --save would store: for the single pod found on the
+// network, its LAN address (preferred: it works from other machines and from CI), or `usb` when
+// it was only heard on its USB network link (see savedConnection); else the serial device of the
+// single pod found over USB. It returns "" when the choice is ambiguous, so --save never guesses
+// between two pods.
 func saveTarget(netPods []discoveredPod, serialPods []serialconsole.SerialPod) string {
 	if len(netPods) == 1 {
-		if host, _, err := net.SplitHostPort(netPods[0].addr); err == nil && usbLinkNet.Contains(net.ParseIP(host)) {
-			return "usb"
-		}
-		return netPods[0].addr
+		return savedConnection(netPods[0].addr, netPods[0].lanAddr)
 	}
 	if len(netPods) == 0 && len(serialPods) == 1 {
 		return serialPods[0].Device

cmd/benchpod-cli/commands_discover_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"net"
 	"os"
 	"strings"
 	"testing"
@@ -340,3 +341,43 @@ func TestSaveTargetSavesUSBForAPodOnItsUSBLink(t *testing.T) {
 		t.Fatalf("saveTarget = %q, want usb for a pod found on its USB network link", got)
 	}
 }
+
+func TestSaveTargetPrefersTheLANAddressOverTheUSBLink(t *testing.T) {
+	// One pod plugged in over USB and on the LAN: mDNS hears it on both, discover drives it over
+	// the link, but the saved default must survive the pod being unplugged and moved.
+	pod := discoveredPod{addr: "10.237.8.85:8080", lanAddr: "192.168.1.220:8080"}
+	if got := saveTarget([]discoveredPod{pod}, nil); got != "192.168.1.220:8080" {
+		t.Fatalf("saveTarget = %q, want the LAN address", got)
+	}
+}
+
+func TestSavedConnection(t *testing.T) {
+	for _, c := range []struct{ addr, lan, want string }{
+		{"10.237.8.85:8080", "192.168.1.220:8080", "192.168.1.220:8080"},
+		{"10.237.8.85:8080", "", "usb"},
+		{"10.237.8.85", "", "usb"},
+		{"192.168.1.220:8080", "", "192.168.1.220:8080"},
+		{"benchpod-baea06.local:8080", "", "benchpod-baea06.local:8080"},
+	} {
+		if got := savedConnection(c.addr, c.lan); got != c.want {
+			t.Errorf("savedConnection(%q, %q) = %q, want %q", c.addr, c.lan, got, c.want)
+		}
+	}
+}
+
+func TestLANPodIPSkipsTheUSBLink(t *testing.T) {
+	lan, usb := net.ParseIP("192.168.1.220"), net.ParseIP("10.237.8.85")
+	if got := lanPodIP([]net.IP{usb, lan}); !got.Equal(lan) {
+		t.Fatalf("got %v, want the LAN address", got)
+	}
+	if got := lanPodIP([]net.IP{usb}); got != nil {
+		t.Fatalf("got %v, want nil for a pod only heard on its USB link", got)
+	}
+}
+
+func TestDescribeHealthShowsTheLANAddress(t *testing.T) {
+	got := describeHealth(discoveredPod{reachable: true, addr: "10.237.8.85:8080", lanAddr: "192.168.1.220:8080"})
+	if !strings.Contains(got, "LAN address 192.168.1.220:8080") {
+		t.Fatalf("got %q", got)
+	}
+}

cmd/benchpod-cli/setup.go

@@ -48,7 +48,11 @@ func newSetupCmd(g *globalFlags) *cobra.Command {
 			"  4. embeddedci.com: an unregistered pod is registered to your account (signing\n" +
 			"     in first when needed), and its wiring profile gets the I/O voltage. A pod\n" +
 			"     on another account is explained, not touched.\n" +
-			"  5. Save the pod's address as the default connection (like `discover --save`).\n\n" +
+			"  5. Save the default connection (like `discover --save`): the pod's Ethernet\n" +
+			"     or Wi-Fi address when it has one, else `usb` for a pod reached only over\n" +
+			"     its USB network link (that 10.237.x.y address only works while the pod is\n" +
+			"     plugged into this computer; `usb` finds it again). A --connection given to\n" +
+			"     setup is saved as it is.\n\n" +
 			"It then prints the next steps: wiring SWD, flashing and running tests.\n\n" +
 			"Pass --connection to set up a pod at a known address or USB device instead of\n" +
 			"discovering it. With --yes setup never prompts: it fails with a message naming\n" +
@@ -92,7 +96,10 @@ type setupOpts struct {
 // setupPod is one physical pod as setup sees it.
 type setupPod struct {
 	device   string // USB console device, "" when not seen over USB
-	addr     string // network host:port, "" when it has no address
+	addr     string // network host:port setup drives it at, "" when it has no address
+	lanAddr  string // its Ethernet or Wi-Fi host:port, "" when it only has its USB network link
+	saveAs   string // the connection step 5 saves when --connection named it; "" = savedConnection
+	usbConn  string // what step 5 saves instead of `usb`: "usb:<name>" when --connection named the pod
 	firmware string
 	cloud    cloudState // known=false when the pod could not say
 }
@@ -223,9 +230,18 @@ func runSetup(env setupEnv, ui *setupUI, o setupOpts) error {
 
 	// 2. Network.
 	step(2, "Network")
-	if p.addr != "" {
+	switch {
+	case p.lanAddr != "":
+		fmt.Fprintf(out, "     Done: the pod is on the network at %s.\n", p.lanAddr)
+	case p.addr != "" && isUSBLinkAddr(p.addr):
+		// Enough for this computer: the whole API runs over the USB cable. Other machines and CI
+		// need Ethernet or Wi-Fi, which setup does not force on a pod that already works.
+		fmt.Fprintf(out, "     Done: this computer reaches the pod over its USB network link (%s).\n", p.addr)
+		fmt.Fprintln(out, "     No Ethernet or Wi-Fi address was found. To use it from other machines or CI, plug in")
+		fmt.Fprintln(out, "     Ethernet (it takes a DHCP lease on its own) or join Wi-Fi with `benchpod set-wifi`.")
+	case p.addr != "":
 		fmt.Fprintf(out, "     Done: the pod is on the network at %s.\n", p.addr)
-	} else {
+	default:
 		if p.device == "" {
 			return errors.New("the pod has no network address and is not on USB; connect it over USB and run setup again")
 		}
@@ -234,6 +250,9 @@ func runSetup(env setupEnv, ui *setupUI, o setupOpts) error {
 			return err
 		}
 		p.addr = addr
+		if !isUSBLinkAddr(addr) {
+			p.lanAddr = addr
+		}
 		fmt.Fprintf(out, "     The pod is on the network at %s.\n", p.addr)
 	}
 
@@ -256,11 +275,22 @@ func runSetup(env setupEnv, ui *setupUI, o setupOpts) error {
 
 	// 5. Save the connection.
 	step(5, "Save the connection")
-	if err := env.save(p.addr); err != nil {
+	target := p.saveAs
+	if target == "" {
+		target = savedConnection(p.addr, p.lanAddr)
+	}
+	if target == "usb" && p.usbConn != "" {
+		target = p.usbConn
+	}
+	if strings.HasPrefix(target, "usb") {
+		fmt.Fprintf(out, "     Saving `%s` rather than %s: that address only works while the pod is plugged into\n", target, p.addr)
+		fmt.Fprintln(out, "     this computer, and `usb` finds the pod again on any USB port.")
+	}
+	if err := env.save(target); err != nil {
 		return err
 	}
 
-	printSetupNextSteps(out, p.addr, name)
+	printSetupNextSteps(out, target, name)
 	return nil
 }
 
@@ -269,9 +299,15 @@ func describeSetupPod(p setupPod) string {
 	if p.device != "" {
 		parts = append(parts, "USB "+p.device)
 	}
-	if p.addr != "" {
+	switch {
+	case p.addr != "" && isUSBLinkAddr(p.addr):
+		parts = append(parts, "USB network "+p.addr)
+		if p.lanAddr != "" {
+			parts = append(parts, "network "+p.lanAddr)
+		}
+	case p.addr != "":
 		parts = append(parts, "network "+p.addr)
-	} else {
+	default:
 		parts = append(parts, "no network address yet")
 	}
 	if p.firmware != "" {
@@ -488,8 +524,9 @@ func setupCloud(env setupEnv, ui *setupUI, o setupOpts, p setupPod, mv int) (str
 	return "", nil
 }
 
-func printSetupNextSteps(out io.Writer, addr, name string) {
-	addr = strings.TrimSuffix(addr, ":8080") // the SDK's default port
+// printSetupNextSteps ends setup; conn is the saved connection (an address or `usb`).
+func printSetupNextSteps(out io.Writer, conn, name string) {
+	addr := strings.TrimSuffix(conn, ":8080") // the SDK's default port
 	fmt.Fprintln(out, "\nThe pod is set up. Next steps:")
 	fmt.Fprintln(out, "  • Wire SWD to your target: SWCLK on LA11, SWDIO on LA12, and GND. The DUT's reset")
 	fmt.Fprintln(out, "    line goes to the pod's reset pin.")
@@ -531,7 +568,11 @@ func (e *realSetupEnv) find() ([]setupPod, error) {
 			if !dp.reachable {
 				return nil, fmt.Errorf("no BenchPod answers at %s: %s", spec.Addr, firstLine(dp.healthErr))
 			}
-			return []setupPod{{addr: spec.Addr, cloud: dp.cloud}}, nil
+			p := setupPod{addr: spec.Addr, cloud: dp.cloud, saveAs: spec.Addr}
+			if !isUSBLinkAddr(spec.Addr) {
+				p.lanAddr = spec.Addr
+			}
+			return []setupPod{p}, nil
 		}
 		serialPods, _, err := serialconsole.ProbeSerial(spec.Device, 2*time.Second)
 		if err != nil {
@@ -540,7 +581,11 @@ func (e *realSetupEnv) find() ([]setupPod, error) {
 		var pods []setupPod
 		for _, sp := range serialPods {
 			if spec.Device == "" || sp.Device == spec.Device {
-				pods = append(pods, setupPodFromSerial(sp))
+				p := setupPodFromSerial(sp)
+				if spec.Pod != "" {
+					p.usbConn = raw
+				}
+				pods = append(pods, p)
 			}
 		}
 		return pods, nil
@@ -568,6 +613,9 @@ func mergeSetupPods(serialPods []serialconsole.SerialPod, netPods []discoveredPo
 		for _, np := range netPods {
 			if np.sameAs == sp.Device && np.reachable {
 				p.addr = np.addr
+				if np.lanAddr != "" {
+					p.lanAddr = np.lanAddr
+				}
 				if np.cloud.known {
 					p.cloud = np.cloud
 				}
@@ -577,7 +625,7 @@ func mergeSetupPods(serialPods []serialconsole.SerialPod, netPods []discoveredPo
 	}
 	for _, np := range netPods {
 		if np.sameAs == "" && np.reachable {
-			pods = append(pods, setupPod{addr: np.addr, cloud: np.cloud})
+			pods = append(pods, setupPod{addr: np.addr, lanAddr: np.lanAddr, cloud: np.cloud})
 		}
 	}
 	return pods
@@ -587,6 +635,9 @@ func setupPodFromSerial(sp serialconsole.SerialPod) setupPod {
 	p := setupPod{device: sp.Device, firmware: sp.Firmware}
 	if sp.Addressed() {
 		p.addr = benchpodconfig.EnsurePort(strings.TrimSpace(sp.IP))
+		if !isUSBLinkAddr(p.addr) {
+			p.lanAddr = p.addr
+		}
 	}
 	if sp.CloudKnown {
 		p.cloud = cloudState{known: true, Configured: sp.Registered, State: sp.CloudState, DeviceID: sp.DeviceID}