diff --git a/cmd/server/db.go b/cmd/server/db.go index b53744ab..4d075b52 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -1704,6 +1704,62 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { } } + // Fallback for observers whose own `observers.id` isn't its mesh + // pubkey at all -- e.g. an MQTT-bridge-type observer (observed: + // openHop-Repeater firmware) that publishes its status keyed by + // device name rather than pubkey, so the lookup above never matches + // even though the same physical device has a real, positioned nodes + // row under its actual pubkey. Matched by exact display name; only + // queried for observers the pubkey lookup left unpositioned, and only + // applied if the pubkey lookup didn't already find something. + nameFallbackNeeded := map[string]bool{} + for _, b := range best { + if b.observerName == "" { + continue + } + if ni, ok := nodeByPK[b.observerPubkey]; ok && ni.lat != nil && ni.lon != nil { + continue + } + nameFallbackNeeded[b.observerName] = true + } + nodeByName := make(map[string]nodeInfo, len(nameFallbackNeeded)) + if len(nameFallbackNeeded) > 0 { + names := make([]string, 0, len(nameFallbackNeeded)) + for n := range nameFallbackNeeded { + names = append(names, n) + } + placeholders := make([]byte, 0, len(names)*2) + args := make([]interface{}, len(names)) + for i, n := range names { + if i > 0 { + placeholders = append(placeholders, ',') + } + placeholders = append(placeholders, '?') + args[i] = n + } + nameRows, err := db.conn.Query( + "SELECT name, role, lat, lon FROM nodes WHERE name IN ("+string(placeholders)+") AND lat IS NOT NULL AND lon IS NOT NULL", args...) + if err == nil { + ambiguous := map[string]bool{} + for nameRows.Next() { + var name, role sql.NullString + var lat, lon sql.NullFloat64 + if nameRows.Scan(&name, &role, &lat, &lon) == nil { + if _, exists := nodeByName[name.String]; exists { + ambiguous[name.String] = true // >1 positioned node shares this name -- don't guess which one + continue + } + v1, v2 := lat.Float64, lon.Float64 + nodeByName[name.String] = nodeInfo{role: role.String, lat: &v1, lon: &v2} + } + } + nameRows.Close() + for n := range ambiguous { + delete(nodeByName, n) + } + } + } + for _, b := range best { branch := PacketPathBranch{Hops: b.hops, Points: []PacketPathPoint{}} for _, pk := range b.resolvedPath { @@ -1725,13 +1781,20 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { obs.IATA = strings.ToUpper(strings.TrimSpace(b.observerIATA.String)) } // Prefer the observer's own self-advertised GPS (same source as - // /api/observers and the Wardriving tab) over its configured - // IATA code -- the IATA table only covers a fixed list of real - // airports plus a handful of hand-added local codes, so a - // custom/regional code an operator typed in (or a typo) falls - // through it even when the node itself knows exactly where it is. + // /api/observers and the Wardriving tab), falling back to a + // name match against `nodes` (some bridge-type observers -- + // seen on openHop-Repeater firmware -- publish their MQTT + // status keyed by device name rather than mesh pubkey, so the + // pubkey lookup above never finds their real, positioned node + // row), and only then the configured IATA code -- which only + // covers a fixed list of real airports plus a handful of + // hand-added local codes, so a custom/regional code an + // operator typed in (or a typo) falls through it even when + // the node itself knows exactly where it is. if ni, ok := nodeByPK[b.observerPubkey]; ok && ni.lat != nil && ni.lon != nil { obs.Lat, obs.Lon = ni.lat, ni.lon + } else if ni, ok := nodeByName[b.observerName]; ok { + obs.Lat, obs.Lon = ni.lat, ni.lon } else if obs.IATA != "" { if coord, ok := iataCoords[obs.IATA]; ok { lat, lon := coord.Lat, coord.Lon diff --git a/cmd/server/db_test.go b/cmd/server/db_test.go index 82185328..6f9285b1 100644 --- a/cmd/server/db_test.go +++ b/cmd/server/db_test.go @@ -716,6 +716,78 @@ func TestGetPacketPath_ObserverPositionPrefersOwnGPS(t *testing.T) { } } +// TestGetPacketPath_ObserverPositionFallsBackToNameMatch covers a +// bridge-type observer (seen in the wild on openHop-Repeater firmware) +// whose `observers.id` is a human-readable device name rather than its +// mesh pubkey -- so the pubkey lookup can never find its real, positioned +// node row. The position should still be found by matching display name +// against `nodes` instead of leaving the observer unplaced. +func TestGetPacketPath_ObserverPositionFallsBackToNameMatch(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + // observers.id is the device's NAME, not a pubkey -- the real pubkey + // only exists as nodes.public_key on a separate row. + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('DK_FRØRUP_5871_R0001', 'DK_FRØRUP_5871_R0001', 'REPEATER')`) + db.conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon) VALUES ('226e9df0...real-pubkey', 'DK_FRØRUP_5871_R0001', 'repeater', 55.237344, 10.710082)`) + + db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('AA', 'pathtest00000005', '2026-01-15T10:00:00Z', 1, 5, + '{"type":"CHAN","channel":"#ping","text":"ping","sender":"Eve"}', '#ping')`) + db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) + VALUES (1, 1, 9.0, -88, '[]', 1736935200)`) + + resp, err := db.GetPacketPath("pathtest00000005") + if err != nil { + t.Fatal(err) + } + if len(resp.Branches) != 1 { + t.Fatalf("Branches = %+v, want 1", resp.Branches) + } + obs := resp.Branches[0].Observer + if obs == nil { + t.Fatalf("Observer = nil, want a populated observer") + } + if obs.Lat == nil || *obs.Lat != 55.237344 || obs.Lon == nil || *obs.Lon != 10.710082 { + t.Errorf("Observer.Lat/Lon = %v/%v, want the name-matched node's GPS (55.237344, 10.710082)", obs.Lat, obs.Lon) + } +} + +// TestGetPacketPath_ObserverPositionSkipsAmbiguousNameMatch covers two +// unrelated positioned nodes that happen to share a display name: the +// name-match fallback must not guess which one is the real observer, so +// the observer should stay unplaced (falling through to the IATA table, +// or nil if that has nothing either) rather than silently picking one. +func TestGetPacketPath_ObserverPositionSkipsAmbiguousNameMatch(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('bridge-id-not-a-pubkey', 'Duplicate Name', NULL)`) + db.conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon) VALUES ('pkOne', 'Duplicate Name', 'repeater', 10.0, 20.0)`) + db.conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon) VALUES ('pkTwo', 'Duplicate Name', 'repeater', 30.0, 40.0)`) + + db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('AA', 'pathtest00000006', '2026-01-15T10:00:00Z', 1, 5, + '{"type":"CHAN","channel":"#ping","text":"ping","sender":"Eve"}', '#ping')`) + db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) + VALUES (1, 1, 9.0, -88, '[]', 1736935200)`) + + resp, err := db.GetPacketPath("pathtest00000006") + if err != nil { + t.Fatal(err) + } + if len(resp.Branches) != 1 { + t.Fatalf("Branches = %+v, want 1", resp.Branches) + } + obs := resp.Branches[0].Observer + if obs == nil { + t.Fatalf("Observer = nil, want a populated observer (just without a position)") + } + if obs.Lat != nil || obs.Lon != nil { + t.Errorf("Observer.Lat/Lon = %v/%v, want nil -- must not guess between two same-named nodes", obs.Lat, obs.Lon) + } +} + // TestGetPacketPath_NoResolvedPath covers a station whose observation // never resolved: it still contributes a branch (hop count from // path_json, so reach is never silently dropped), just with no points.