diff --git a/.changeset/http-driver-mac-identity.md b/.changeset/http-driver-mac-identity.md new file mode 100644 index 00000000..bff256c6 --- /dev/null +++ b/.changeset/http-driver-mac-identity.md @@ -0,0 +1,17 @@ +--- +"ftw": patch +--- + +Give HTTP drivers a hardware-stable device identity. An HTTP driver's MAC was +never resolved, so a LAN device that had not yet reported a serial fell back to +no identity at all and its persistent state (battery model, RLS twin, +calibration history) was orphaned whenever its address changed. The registry now +ARP-resolves the driver's own configured address, exactly as it already does for +MQTT and Modbus. + +Only the driver's own `host`/`url` is probed, never the merged HTTP allowlist, +so vendor cloud endpoints are never treated as identity candidates. ARP lookups +are also now skipped for addresses that provably cannot appear in an ARP table +(loopback, carrier-grade-NAT/tunnel, multicast and broadcast), saving 150 ms of +pointless dialing. Public addresses stay eligible, so devices on a segment with +an ISP-routed block keep the MAC-derived identity they have today. diff --git a/go/internal/arp/arp.go b/go/internal/arp/arp.go index c651ecb9..b0fb1398 100644 --- a/go/internal/arp/arp.go +++ b/go/internal/arp/arp.go @@ -23,6 +23,9 @@ import ( // the host before reading. The probe is cheap (50 ms timeout) and silent // on failure — what matters is the SYN packet itself triggering ARP. // +// Only a bare IPv4 literal is accepted; hostnames are not resolved, so a +// driver configured with a DNS name gets no MAC. +// // Returns (mac, true) on success or ("", false) if the host can't be // reached on L2 or isn't on this segment at all. func Lookup(ipStr string) (string, bool) { @@ -30,6 +33,12 @@ func Lookup(ipStr string) (string, bool) { if ip == nil { return "", false } ip4 := ip.To4() if ip4 == nil { return "", false } + // Skip addresses that provably cannot produce an ARP entry, rather + // than spending 150 ms of dialing to learn nothing. Deliberately NOT + // limited to RFC1918: a public address can legitimately sit on the + // local segment (an ISP routing a small block onto the LAN), and + // excluding it would silently re-key those devices off their MAC. + if !canHaveARPEntry(ip4) { return "", false } // Nudge the ARP cache. We don't care if connect succeeds — the kernel // resolves ARP regardless when it tries to send the SYN. for _, port := range []string{"80", "502", "1883"} { @@ -45,6 +54,26 @@ func Lookup(ipStr string) (string, bool) { return "", false } +// canHaveARPEntry reports whether an IPv4 address could ever appear in the +// kernel's ARP table. Only the provably-impossible cases are excluded, so +// that a device on an unusual-but-real local segment keeps its MAC-derived +// identity. ip4 must be the 4-byte form from net.IP.To4. +func canHaveARPEntry(ip4 net.IP) bool { + switch { + case ip4[0] == 0: // "this network", including 0.0.0.0 + return false + case ip4[0] == 127: // loopback never resolves to a hardware address + return false + case ip4[0] == 100 && ip4[1] >= 64 && ip4[1] <= 127: + // Carrier-grade NAT space, where Tailscale and similar tunnels + // live. Reached over an L3 tun device, which has no L2 to ARP on. + return false + case ip4[0] >= 224: // multicast, reserved space, broadcast + return false + } + return true +} + // lookupLinux parses /proc/net/arp. // // IP address HW type Flags HW address Mask Device @@ -69,22 +98,30 @@ func lookupLinux(ipStr string) (string, bool) { func lookupDarwin(ipStr string) (string, bool) { out, err := exec.Command("/usr/sbin/arp", "-n", ipStr).CombinedOutput() if err != nil { return "", false } - // Output: "? (192.168.1.42) at aa:bb:cc:dd:ee:ff on en0 ifscope [ethernet]" - s := string(out) + return parseDarwinARP(string(out)) +} + +// parseDarwinARP extracts the MAC from `arp -n ` output: +// +// ? (192.168.1.42) at aa:bb:cc:dd:ee:ff on en0 ifscope [ethernet] +// +// A miss prints "(incomplete)" or "no entry" in the same slot; both fail +// the six-octet check. macOS drops leading zeros ("1:2:3:4:5:6"), so +// octets are padded back to the canonical form callers compare against. +func parseDarwinARP(s string) (string, bool) { idx := strings.Index(s, " at ") if idx < 0 { return "", false } - rest := s[idx+4:] - end := strings.Index(rest, " ") - if end < 0 { return "", false } - mac := strings.ToLower(rest[:end]) - if mac == "(incomplete)" || mac == "no" { return "", false } - // Normalize single-digit octets (macOS prints "1:2:3:4:5:6") - parts := strings.Split(mac, ":") + fields := strings.Fields(s[idx+len(" at "):]) + if len(fields) == 0 { return "", false } + parts := strings.Split(strings.ToLower(fields[0]), ":") if len(parts) != 6 { return "", false } for i, p := range parts { + if len(p) == 0 || len(p) > 2 { return "", false } if len(p) == 1 { parts[i] = "0" + p } } - return strings.Join(parts, ":"), true + mac := strings.Join(parts, ":") + if mac == "00:00:00:00:00:00" { return "", false } + return mac, true } // readFile is a tiny indirection so tests can stub /proc/net/arp. diff --git a/go/internal/arp/arp_test.go b/go/internal/arp/arp_test.go new file mode 100644 index 00000000..91c50dc4 --- /dev/null +++ b/go/internal/arp/arp_test.go @@ -0,0 +1,180 @@ +package arp + +import ( + "net" + "os" + "testing" +) + +// stubARPTable replaces the /proc/net/arp reader for one test. +func stubARPTable(t *testing.T, contents string) { + t.Helper() + prev := readFile + readFile = func(string) ([]byte, error) { return []byte(contents), nil } + t.Cleanup(func() { readFile = prev }) +} + +// Every case here must bail before the ARP-cache nudge, which is what +// keeps this test hermetic: no case may touch the network. A hostname is +// never resolved, IPv6 has no ARP at all, and the rest are addresses that +// provably cannot appear in an ARP table. +func TestLookupRejectsAddressesThatCannotHaveAnARPEntry(t *testing.T) { + cases := []struct{ name, host string }{ + {"empty", ""}, + {"hostname", "inverter.local"}, + {"host:port", "192.168.1.5:502"}, + {"ipv6 loopback", "::1"}, + {"ipv6 link-local", "fe80::1"}, + {"ipv6 ula", "fd00::5"}, + {"ipv4 loopback", "127.0.0.1"}, + {"unspecified", "0.0.0.0"}, + {"cgnat tunnel", "100.96.146.117"}, + {"multicast", "224.0.0.251"}, + {"broadcast", "255.255.255.255"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if mac, ok := Lookup(tc.host); ok { + t.Fatalf("Lookup(%q) = (%q, true), want no match", tc.host, mac) + } + }) + } +} + +// The gate excludes only what provably cannot resolve. A public address +// stays eligible on purpose: an ISP can route a small block straight onto +// the LAN, and excluding it would silently re-key those devices off their +// MAC and orphan their persistent state. +func TestCanHaveARPEntry(t *testing.T) { + cases := []struct { + ip string + want bool + }{ + {"10.0.0.1", true}, + {"172.16.0.1", true}, + {"192.168.1.42", true}, + {"169.254.10.1", true}, // link-local: no DHCP, still on our segment + {"8.8.8.8", true}, // public, but could be on our segment + {"172.32.0.1", true}, // outside RFC1918, still a routable unicast host + {"0.0.0.0", false}, + {"127.0.0.1", false}, + {"100.64.0.1", false}, + {"100.127.255.254", false}, + {"100.128.0.1", true}, // just past CGNAT, ordinary unicast again + {"224.0.0.251", false}, + {"239.255.255.250", false}, + {"255.255.255.255", false}, + } + for _, tc := range cases { + t.Run(tc.ip, func(t *testing.T) { + ip4 := net.ParseIP(tc.ip).To4() + if ip4 == nil { + t.Fatalf("ParseIP(%q).To4() = nil", tc.ip) + } + if got := canHaveARPEntry(ip4); got != tc.want { + t.Errorf("canHaveARPEntry(%s) = %v, want %v", tc.ip, got, tc.want) + } + }) + } +} + +const procNetARP = `IP address HW type Flags HW address Mask Device +192.168.1.42 0x1 0x2 AA:BB:CC:DD:EE:FF * eth0 +192.168.1.43 0x1 0x0 00:00:00:00:00:00 * eth0 +192.168.1.44 0x1 0x2 aa:bb:cc:dd:ee:01 * wlan0 +` + +func TestLookupLinuxParsesProcNetARP(t *testing.T) { + stubARPTable(t, procNetARP) + cases := []struct { + name string + ip string + wantMAC string + wantOK bool + }{ + // The kernel prints uppercase; device_id comparisons downstream + // assume lowercase, so normalization here is load-bearing. + {"match is lowercased", "192.168.1.42", "aa:bb:cc:dd:ee:ff", true}, + {"match on second interface", "192.168.1.44", "aa:bb:cc:dd:ee:01", true}, + {"incomplete entry is not a match", "192.168.1.43", "", false}, + {"absent host", "192.168.1.99", "", false}, + {"header row is not matched", "IP", "", false}, + {"prefix of a listed ip", "192.168.1.4", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mac, ok := lookupLinux(tc.ip) + if mac != tc.wantMAC || ok != tc.wantOK { + t.Errorf("lookupLinux(%q) = (%q, %v), want (%q, %v)", + tc.ip, mac, ok, tc.wantMAC, tc.wantOK) + } + }) + } +} + +func TestLookupLinuxHandlesUnreadableTable(t *testing.T) { + prev := readFile + readFile = func(string) ([]byte, error) { return nil, os.ErrNotExist } + t.Cleanup(func() { readFile = prev }) + + if mac, ok := lookupLinux("192.168.1.42"); ok { + t.Fatalf("lookupLinux with unreadable table = (%q, true), want no match", mac) + } +} + +func TestParseDarwinARP(t *testing.T) { + cases := []struct { + name string + out string + wantMAC string + wantOK bool + }{ + { + "typical entry", + "? (192.168.1.42) at aa:bb:cc:dd:ee:ff on en0 ifscope [ethernet]\n", + "aa:bb:cc:dd:ee:ff", true, + }, + { + // macOS drops leading zeros; the padded form is what the + // devices table and every comparison downstream expect. + "single-digit octets are padded", + "? (192.168.1.42) at 1:2:3:4:5:6 on en0 ifscope [ethernet]\n", + "01:02:03:04:05:06", true, + }, + { + "uppercase is normalized", + "? (192.168.1.42) at AA:BB:CC:DD:EE:FF on en0\n", + "aa:bb:cc:dd:ee:ff", true, + }, + { + "mac at end of output without trailing field", + "? (192.168.1.42) at aa:bb:cc:dd:ee:ff", + "aa:bb:cc:dd:ee:ff", true, + }, + { + "incomplete entry", + "? (192.168.1.42) at (incomplete) on en0 ifscope [ethernet]\n", + "", false, + }, + { + "no entry at all", + "192.168.1.99 (192.168.1.99) -- no entry\n", + "", false, + }, + {"all-zero mac", "? (192.168.1.42) at 00:00:00:00:00:00 on en0\n", "", false}, + {"too few octets", "? (192.168.1.42) at aa:bb:cc on en0\n", "", false}, + {"too many octets", "? (192.168.1.42) at aa:bb:cc:dd:ee:ff:11 on en0\n", "", false}, + {"oversized octet", "? (192.168.1.42) at aaa:bb:cc:dd:ee:ff on en0\n", "", false}, + {"empty octet", "? (192.168.1.42) at aa::cc:dd:ee:ff on en0\n", "", false}, + {"empty output", "", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mac, ok := parseDarwinARP(tc.out) + if mac != tc.wantMAC || ok != tc.wantOK { + t.Errorf("parseDarwinARP(%q) = (%q, %v), want (%q, %v)", + tc.out, mac, ok, tc.wantMAC, tc.wantOK) + } + }) + } +} diff --git a/go/internal/drivers/registry.go b/go/internal/drivers/registry.go index 29f99c31..f969eef1 100644 --- a/go/internal/drivers/registry.go +++ b/go/internal/drivers/registry.go @@ -31,7 +31,8 @@ type Registry struct { ModbusFactory func(name string, c *config.ModbusConfig) (ModbusCap, error) // SerialFactory opens one read-only local serial device. SerialFactory func(name string, c *config.SerialConfig) (SerialCap, error) - // ARPLookup resolves a hostname/IP to a MAC for L2-stable identity. + // ARPLookup resolves an on-LAN IPv4 literal to a MAC for L2-stable + // identity; hostnames and off-segment addresses yield ("", false). // Optional — when nil, devices fall back to endpoint-hash IDs. ARPLookup func(host string) (mac string, ok bool) // SecretPersister, when set, durably stores a driver secret (keyed by @@ -234,6 +235,19 @@ func (r *Registry) Add(ctx context.Context, cfg config.Driver) error { if cfg.Capabilities.HTTP.AllowWrite { env.WithHTTPAllowWrite() } + // A LAN HTTP device often has no serial until it answers, and the + // HTTP branch sets no endpoint — so without this it has no stable + // identity at all and its persistent state is orphaned on a DHCP + // lease change. Probe only the driver's OWN address, never the + // merged allowlist: that list also names vendor cloud endpoints, + // which must not be identity candidates. + if r.ARPLookup != nil { + if host := httpDeviceHost(cfg.Config); host != "" { + if mac, ok := r.ARPLookup(host); ok { + env.SetMAC(mac) + } + } + } } if cfg.Capabilities.WebSocket != nil { env.WithWS(NewGorillaWS(cfg.Name)) @@ -868,6 +882,29 @@ func mergeAllowedHosts(explicit []string, drvCfg map[string]any) []string { return out } +// httpDeviceHost returns the address of the device an HTTP driver talks +// to, taken from its own config block: the `host` key, else the host part +// of `url`. Any port is stripped — ARP keys on the L3 address alone. +// Returns "" for a driver with neither (a cloud driver whose base URL +// lives in the Lua source, not in config). +func httpDeviceHost(drvCfg map[string]any) string { + if drvCfg == nil { + return "" + } + if v, ok := drvCfg["host"].(string); ok { + if h := strings.TrimSpace(v); h != "" { + host, _, _ := splitHostPortLower(h) + return host + } + } + if v, ok := drvCfg["url"].(string); ok { + if u, err := url.Parse(strings.TrimSpace(v)); err == nil && u.Hostname() != "" { + return strings.ToLower(u.Hostname()) + } + } + return "" +} + func findDriver(list []config.Driver, name string) (config.Driver, bool) { for _, d := range list { if d.Name == name { diff --git a/go/internal/drivers/registry_arp_test.go b/go/internal/drivers/registry_arp_test.go new file mode 100644 index 00000000..a463973d --- /dev/null +++ b/go/internal/drivers/registry_arp_test.go @@ -0,0 +1,146 @@ +package drivers + +import ( + "context" + "sync" + "testing" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/telemetry" +) + +const arpTestDriver = ` +function driver_init(config) end +function driver_poll() return 1000 end +function driver_command(action, w, cmd) end +` + +// recordingARP stands in for arp.Lookup and remembers every host it was +// asked about, so a test can assert on what was NOT probed. +type recordingARP struct { + mu sync.Mutex + hosts []string + macs map[string]string +} + +func (r *recordingARP) lookup(host string) (string, bool) { + r.mu.Lock() + defer r.mu.Unlock() + r.hosts = append(r.hosts, host) + mac, ok := r.macs[host] + return mac, ok +} + +func (r *recordingARP) asked() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.hosts...) +} + +func addARPTestDriver(t *testing.T, arp *recordingARP, cfg config.Driver) *Registry { + t.Helper() + r := NewRegistry(telemetry.NewStore()) + r.ARPLookup = arp.lookup + cfg.Lua = writeTestDriver(t, arpTestDriver) + if err := r.Add(context.Background(), cfg); err != nil { + t.Fatalf("Add: %v", err) + } + t.Cleanup(func() { r.remove(cfg.Name, true) }) + return r +} + +// An HTTP driver sets no endpoint, so without an ARP-derived MAC a LAN +// device that has not yet reported a serial has no stable identity at all. +func TestAddResolvesMACForHTTPDriver(t *testing.T) { + arp := &recordingARP{macs: map[string]string{"192.168.1.50": "aa:bb:cc:dd:ee:ff"}} + r := addARPTestDriver(t, arp, config.Driver{ + Name: "heatpump", + Capabilities: config.Capabilities{HTTP: &config.HTTPCapability{}}, + Config: map[string]any{"host": "192.168.1.50"}, + }) + + env := r.Env("heatpump") + if env == nil { + t.Fatal("Env = nil after Add") + } + if _, _, mac, _ := env.FullIdentity(); mac != "aa:bb:cc:dd:ee:ff" { + t.Errorf("mac = %q, want aa:bb:cc:dd:ee:ff", mac) + } +} + +// The merged allowlist also names vendor cloud endpoints. Probing those +// would be both useless (never on our L2) and an unsolicited connection +// to a third party, so only the driver's own address may be asked about. +func TestHTTPDriverARPProbesOnlyItsOwnHost(t *testing.T) { + arp := &recordingARP{macs: map[string]string{"192.168.1.50": "aa:bb:cc:dd:ee:ff"}} + addARPTestDriver(t, arp, config.Driver{ + Name: "heatpump", + Capabilities: config.Capabilities{HTTP: &config.HTTPCapability{ + AllowedHosts: []string{"api.vendor.example", "telemetry.vendor.example"}, + }}, + Config: map[string]any{"host": "192.168.1.50"}, + }) + + asked := arp.asked() + if len(asked) != 1 || asked[0] != "192.168.1.50" { + t.Errorf("ARP probed %v, want exactly [192.168.1.50]", asked) + } +} + +func TestCloudOnlyHTTPDriverIsNotProbed(t *testing.T) { + arp := &recordingARP{} + addARPTestDriver(t, arp, config.Driver{ + Name: "easee", + Capabilities: config.Capabilities{HTTP: &config.HTTPCapability{ + AllowedHosts: []string{"api.easee.cloud"}, + }}, + }) + + if asked := arp.asked(); len(asked) != 0 { + t.Errorf("ARP probed %v for a driver with no device address, want none", asked) + } +} + +func TestHTTPDriverSurvivesUnresolvableMAC(t *testing.T) { + arp := &recordingARP{} // every lookup misses + r := addARPTestDriver(t, arp, config.Driver{ + Name: "heatpump", + Capabilities: config.Capabilities{HTTP: &config.HTTPCapability{}}, + Config: map[string]any{"host": "192.168.1.50"}, + }) + + if _, _, mac, _ := r.Env("heatpump").FullIdentity(); mac != "" { + t.Errorf("mac = %q, want empty when ARP misses", mac) + } +} + +func TestHTTPDeviceHost(t *testing.T) { + cases := []struct { + name string + cfg map[string]any + want string + }{ + {"nil config", nil, ""}, + {"empty config", map[string]any{}, ""}, + {"bare host", map[string]any{"host": "192.168.1.50"}, "192.168.1.50"}, + {"host with port", map[string]any{"host": "192.168.1.50:8080"}, "192.168.1.50"}, + {"host is trimmed and lowercased", map[string]any{"host": " Inverter.Local "}, "inverter.local"}, + {"blank host", map[string]any{"host": " "}, ""}, + {"host wins over url", map[string]any{ + "host": "192.168.1.50", "url": "https://api.vendor.example", + }, "192.168.1.50"}, + {"url fallback", map[string]any{"url": "http://192.168.1.60/api"}, "192.168.1.60"}, + {"url port is stripped", map[string]any{"url": "http://192.168.1.60:8080/api"}, "192.168.1.60"}, + {"url with credentials", map[string]any{"url": "http://user:pw@192.168.1.60/api"}, "192.168.1.60"}, + {"bracketed ipv6 url is unwrapped", map[string]any{"url": "http://[fd00::5]:8080/"}, "fd00::5"}, + {"url without host", map[string]any{"url": "/local/path"}, ""}, + {"non-string host is ignored", map[string]any{"host": 42}, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := httpDeviceHost(tc.cfg); got != tc.want { + t.Errorf("httpDeviceHost(%v) = %q, want %q", tc.cfg, got, tc.want) + } + }) + } +} diff --git a/go/internal/state/devices_test.go b/go/internal/state/devices_test.go new file mode 100644 index 00000000..b4ec4f7e --- /dev/null +++ b/go/internal/state/devices_test.go @@ -0,0 +1,192 @@ +package state + +import "testing" + +// device_id is the key every piece of persistent per-device state hangs +// off (battery models, the RLS twin, calibration history), so the +// priority order below is a compatibility surface: changing it silently +// orphans that state on the next boot. +func TestResolveDeviceIDPriority(t *testing.T) { + cases := []struct { + name string + make, serial, mac, endpoint string + want string + }{ + { + name: "hardware serial outranks mac and endpoint", + make: "Sungrow", serial: "A2340", mac: "aa:bb:cc:dd:ee:ff", + endpoint: "modbus://192.168.1.5:502", + want: "sungrow:A2340", + }, + { + name: "mac outranks endpoint", + mac: "AA:BB:CC:DD:EE:FF", endpoint: "modbus://192.168.1.5:502", + want: "mac:aabbccddeeff", + }, + { + name: "endpoint is the last resort", + endpoint: "modbus://192.168.1.5:502", + want: "ep:modbus://192.168.1.5:502", + }, + { + name: "nothing known yields no id", + want: "", + }, + { + // Half a hardware identity is not an identity — a make shared + // by every unit of a model would collide across devices. + name: "make without serial falls through to mac", + make: "Sungrow", mac: "aa:bb:cc:dd:ee:ff", + want: "mac:aabbccddeeff", + }, + { + name: "serial without make falls through to mac", + serial: "A2340", mac: "aa:bb:cc:dd:ee:ff", + want: "mac:aabbccddeeff", + }, + { + // Make is normalized but serial is not: vendor serials are + // case-significant and two units may differ only in case. + name: "make is lowercased and serial case is preserved", + make: "SunGrow", serial: "A2340b", + want: "sungrow:A2340b", + }, + { + name: "surrounding whitespace is trimmed", + make: " sungrow ", serial: " A2340 ", + want: "sungrow:A2340", + }, + { + name: "mac separators are stripped and case normalized", + mac: " AA:BB:CC:DD:EE:FF ", + want: "mac:aabbccddeeff", + }, + { + name: "whitespace-only fields count as absent", + make: " ", serial: " ", mac: " ", endpoint: "ep-fallback", + want: "ep:ep-fallback", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := ResolveDeviceID(tc.make, tc.serial, tc.mac, tc.endpoint) + if got != tc.want { + t.Errorf("ResolveDeviceID(%q, %q, %q, %q) = %q, want %q", + tc.make, tc.serial, tc.mac, tc.endpoint, got, tc.want) + } + }) + } +} + +func TestRegisterDeviceRejectsAnUnidentifiableDevice(t *testing.T) { + s := freshStore(t) + if id, err := s.RegisterDevice(Device{DriverName: "inv"}); err == nil { + t.Fatalf("RegisterDevice with no identity bits = (%q, nil), want an error", id) + } + devs, err := s.AllDevices() + if err != nil { + t.Fatal(err) + } + if len(devs) != 0 { + t.Fatalf("AllDevices = %d rows, want 0 — a nameless device must not be stored", len(devs)) + } +} + +func TestRegisterDeviceIsIdempotentAndPreservesKnownIdentity(t *testing.T) { + s := freshStore(t) + const mac = "aa:bb:cc:dd:ee:ff" + + id, err := s.RegisterDevice(Device{ + DriverName: "inv", MAC: mac, Endpoint: "http://192.168.1.5", FirstSeenMs: 1000, + }) + if err != nil { + t.Fatal(err) + } + if id != "mac:aabbccddeeff" { + t.Fatalf("device_id = %q, want mac:aabbccddeeff", id) + } + + // Re-register the same row after a cold ARP cache: the MAC is no + // longer resolvable and the serial has since arrived. The stored MAC + // must survive the empty field, otherwise the row loses the very + // identity it is keyed on. + if _, err := s.RegisterDevice(Device{ + DeviceID: id, DriverName: "inv", Make: "sungrow", Serial: "A2340", FirstSeenMs: 9999, + }); err != nil { + t.Fatal(err) + } + + devs, err := s.AllDevices() + if err != nil { + t.Fatal(err) + } + if len(devs) != 1 { + t.Fatalf("AllDevices = %d rows, want 1 — re-registering must upsert, not insert", len(devs)) + } + got := devs[0] + if got.MAC != mac { + t.Errorf("mac = %q, want %q — an empty field must not erase a known one", got.MAC, mac) + } + if got.Endpoint != "http://192.168.1.5" { + t.Errorf("endpoint = %q, want it preserved", got.Endpoint) + } + if got.Make != "sungrow" || got.Serial != "A2340" { + t.Errorf("make/serial = %q/%q, want the newly-learned values", got.Make, got.Serial) + } + if got.FirstSeenMs != 1000 { + t.Errorf("first_seen_ms = %d, want 1000 — first sighting is immutable", got.FirstSeenMs) + } +} + +// Documents a real consequence of the priority order rather than +// asserting it is desirable: registerAllDevices resolves the id afresh on +// every pass, so a device first seen by MAC is re-keyed the moment its +// driver reports a serial, leaving the mac: row behind. Any state still +// keyed on the old id is orphaned at that point. +func TestRegisterDeviceRekeysWhenSerialArrivesAfterMAC(t *testing.T) { + s := freshStore(t) + + first, err := s.RegisterDevice(Device{DriverName: "inv", MAC: "aa:bb:cc:dd:ee:ff"}) + if err != nil { + t.Fatal(err) + } + second, err := s.RegisterDevice(Device{ + DriverName: "inv", Make: "sungrow", Serial: "A2340", MAC: "aa:bb:cc:dd:ee:ff", + }) + if err != nil { + t.Fatal(err) + } + + if first == second { + t.Fatalf("device_id stayed %q after the serial arrived; this test is stale", first) + } + devs, err := s.AllDevices() + if err != nil { + t.Fatal(err) + } + if len(devs) != 2 { + t.Fatalf("AllDevices = %d rows, want 2 (the orphaned %q plus %q)", len(devs), first, second) + } +} + +func TestLookupDeviceByDriverName(t *testing.T) { + s := freshStore(t) + if got := s.LookupDeviceByDriverName("inv"); got != nil { + t.Fatalf("cold lookup = %+v, want nil", got) + } + + id, err := s.RegisterDevice(Device{DriverName: "inv", Make: "sungrow", Serial: "A2340"}) + if err != nil { + t.Fatal(err) + } + got := s.LookupDeviceByDriverName("inv") + if got == nil { + t.Fatal("lookup after register = nil, want the registered device") + } + if got.DeviceID != id { + t.Errorf("device_id = %q, want %q", got.DeviceID, id) + } + if s.LookupDeviceByDriverName("other") != nil { + t.Error("lookup of an unregistered driver name returned a device") + } +}