From f244f27a6316abb27e9c08596e376d60ec89e097 Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 24 Jul 2026 16:43:53 +0200 Subject: [PATCH] fix: View Path treats (0,0) node coordinates as no fix, not the Gulf of Guinea GetPacketPath's node-position lookups (relay-hop points and observer positioning) read raw lat/lon straight off the nodes row, so a node with (0,0) stored literally instead of NULL -- MeshCore's "never actually reported a GPS position" case in practice -- rendered as a real point off the coast of Ghana instead of no position at all. Excludes (0,0) the same way GetNodesForScopeAdoption and geofilter.PassesFilter already do; the node's name is kept, just not the bogus position. Co-Authored-By: Claude Sonnet 5 --- cmd/server/db.go | 18 ++++++++++-------- cmd/server/db_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/cmd/server/db.go b/cmd/server/db.go index 7f03a77f..7de32d85 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -1714,13 +1714,15 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { var lat, lon sql.NullFloat64 if nodeRows.Scan(&pk, &name, &role, &lat, &lon) == nil { ni := nodeInfo{name: name.String, role: role.String} - if lat.Valid { - v := lat.Float64 - ni.lat = &v - } - if lon.Valid { - v := lon.Float64 - ni.lon = &v + // (0,0) is the ocean off Ghana, not a real fix -- some + // nodes have it stored literally instead of NULL when + // they've never actually reported a GPS position. + // Excluded the same way GetNodesForScopeAdoption and + // geofilter.PassesFilter already do; the node's own + // name/role are kept, just not its bogus position. + if lat.Valid && lon.Valid && !(lat.Float64 == 0 && lon.Float64 == 0) { + v1, v2 := lat.Float64, lon.Float64 + ni.lat, ni.lon = &v1, &v2 } nodeByPK[pk] = ni } @@ -1767,7 +1769,7 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { 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...) + "SELECT name, role, lat, lon FROM nodes WHERE name IN ("+string(placeholders)+") AND lat IS NOT NULL AND lon IS NOT NULL AND lat != 0 AND lon != 0", args...) if err == nil { ambiguous := map[string]bool{} for nameRows.Next() { diff --git a/cmd/server/db_test.go b/cmd/server/db_test.go index 849fd861..ee7643e1 100644 --- a/cmd/server/db_test.go +++ b/cmd/server/db_test.go @@ -721,6 +721,49 @@ func TestGetPacketPath_First(t *testing.T) { } } +// TestGetPacketPath_ExcludesNullIsland covers a node whose nodes.lat/lon +// are stored as literal (0,0) rather than NULL -- MeshCore's "never +// actually reported a GPS position" sentinel in practice, not a real fix +// off the coast of Ghana. Both the relay-hop-point lookup and the +// observer-position lookup must treat it as unpositioned (matching the +// same convention GetNodesForScopeAdoption and geofilter.PassesFilter +// already use), keeping the node's name but not its bogus position. +func TestGetPacketPath_ExcludesNullIsland(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('obsZero', 'Zero Observer', NULL)`) + db.conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon) VALUES ('pkZero', 'ZeroRepeater', 'repeater', 0, 0)`) + db.conn.Exec(`INSERT INTO nodes (public_key, name, role, lat, lon) VALUES ('obsZero', 'Zero Observer', 'repeater', 0, 0)`) + + db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('AA', 'pathtest00000008', '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, resolved_path, timestamp) + VALUES (1, 1, 9.0, -88, '["aa"]', '["pkZero"]', 1736935200)`) + + resp, err := db.GetPacketPath("pathtest00000008") + if err != nil { + t.Fatal(err) + } + if len(resp.Branches) != 1 { + t.Fatalf("Branches = %+v, want 1", resp.Branches) + } + b := resp.Branches[0] + if len(b.Points) != 1 || b.Points[0].Name != "ZeroRepeater" { + t.Fatalf("Points = %+v, want ZeroRepeater still named", b.Points) + } + if b.Points[0].Lat != nil || b.Points[0].Lon != nil { + t.Errorf("Points[0].Lat/Lon = %v/%v, want nil -- (0,0) is a no-fix sentinel, not a real position", b.Points[0].Lat, b.Points[0].Lon) + } + if b.Observer == nil || b.Observer.Name != "Zero Observer" { + t.Fatalf("Observer = %+v, want Zero Observer still named", b.Observer) + } + if b.Observer.Lat != nil || b.Observer.Lon != nil { + t.Errorf("Observer.Lat/Lon = %v/%v, want nil -- the observer's own node row is also (0,0)", b.Observer.Lat, b.Observer.Lon) + } +} + // TestGetPacketPath_ObserverPositionPrefersOwnGPS covers an observer whose // configured IATA code isn't a real airport (a custom/regional code an // operator typed in, or a typo) and so isn't in the hardcoded iataCoords