diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9574594..4dc1632 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,7 +58,7 @@ jobs: - name: Verify stats and endpoint queries against PostgreSQL 16 env: BEACON_TEST_POSTGRES_DSN: postgres://postgres:backup-ci-only@127.0.0.1:5432/postgres?sslmode=disable - run: go test ./db -run '^Test(Signal|Paths|PacketSummaries|PacketEndpointsResolveLive|ObserverMetrics|RouteEvidence|RouteEvidenceIndex|AnalyticsRetention|AnalyticsRetentionConcurrent|DeleteOldPacketsBatches|MeshMapperCatalogue|ChannelMessageScopes|Reconfirm[A-Za-z]*)Postgres$' -count=1 -v + run: go test ./db -run '^Test(Signal|Paths|PacketSummaries|PacketEndpointsResolveLive|ObserverMetrics|RouteEvidence|RouteEvidenceIndex|AnalyticsRetention|AnalyticsRetentionConcurrent|DeleteOldPacketsBatches|MeshMapperCatalogue|ChannelMessageScopes|NodeLocationReset|Reconfirm[A-Za-z]*)Postgres$' -count=1 -v - name: Verify backup command against PostgreSQL 16 run: | diff --git a/README.md b/README.md index cae5c80..dfcec34 100644 --- a/README.md +++ b/README.md @@ -313,15 +313,18 @@ invalid geometry fails startup. Border changes require a restart. Inside any polygon (including its edges) means `false`; outside the entire union means `true`. Hole interiors are outside; hole edges are local. Other node roles, -missing/invalid positions and the 0/0 location reset have no classification. +missing or invalid positions have no classification (an explicit 0/0 advert +clears the stored position). This is a hint based on reported position, not proof of a repeater's origin. Packet ingestion, heard-in IATAs and route matching remain unchanged. Node list/detail reads apply the current geometry after cache reads, so existing historical nodes need no backfill. A `nodeUpdate` includes `possiblyForeign` when its advert provides a position: a boolean for known positions, `null` to -clear an unknown/reset position. Omission retains the previous value when a -repeater's advert omits its position. A change to another role also sends `null`. +clear it when the advert reports 0/0 or an invalid position. Omission retains the +previous value when a repeater's advert omits its position. A change to another +role also sends `null`. `lat`/`lng` follow the same rule: omitted keeps the +client's position, explicit `null` clears it. The field is omitted everywhere when the feature is disabled (the default). Use longitude/latitude coordinate order and split antimeridian-crossing borders diff --git a/db/migrations/043_clear_zero_locations.sql b/db/migrations/043_clear_zero_locations.sql new file mode 100644 index 0000000..b4a83b4 --- /dev/null +++ b/db/migrations/043_clear_zero_locations.sql @@ -0,0 +1,6 @@ +-- Copyright 2026 Beacon Contributors +-- SPDX-License-Identifier: AGPL-3.0-or-later + +-- An explicit 0/0 advert now clears the stored position; drop old 0/0 rows and stale sources. +UPDATE nodes SET latitude = NULL, longitude = NULL, location_source = NULL +WHERE location_source = 'advert' AND (latitude IS NULL OR (latitude = 0 AND longitude = 0)); diff --git a/db/node_location_integration_test.go b/db/node_location_integration_test.go index dd06001..51f700c 100644 --- a/db/node_location_integration_test.go +++ b/db/node_location_integration_test.go @@ -41,31 +41,58 @@ func TestNodeLocationResetPostgres(t *testing.T) { if err != nil { t.Fatal(err) } + f := func(v float64) *float64 { return &v } + advert := "advert" for _, tc := range []struct { - name string - present bool - lat, lon float64 + name string + lat, lon *float64 + clear bool + wantLat *float64 + wantLon *float64 + wantSource *string }{ - {"omission preserves location", false, 45, -75}, - {"explicit reset replaces location", true, 0, 0}, - {"omission preserves reset", false, 0, 0}, + {"omission preserves location", nil, nil, false, f(45), f(-75), &advert}, + {"explicit reset clears location", nil, nil, true, nil, nil, nil}, + {"omission preserves cleared location", nil, nil, false, nil, nil, nil}, + {"new coordinates restore location", f(46), f(-76), false, f(46), f(-76), &advert}, } { t.Run(tc.name, func(t *testing.T) { - params.Latitude, params.Longitude = nil, nil - if tc.present { - params.Latitude, params.Longitude = &tc.lat, &tc.lon - } + params.Latitude, params.Longitude, params.ClearLocation = tc.lat, tc.lon, tc.clear updatedID, err := store.UpsertNode(ctx, params, ingest.RadioSettings{}) if err != nil || updatedID != id { t.Fatalf("node identity changed: %v", err) } - var gotLat, gotLon float64 - if err := tx.QueryRow(ctx, "SELECT latitude,longitude FROM nodes WHERE id=$1", id).Scan(&gotLat, &gotLon); err != nil { - t.Fatal(err) - } - if gotLat != tc.lat || gotLon != tc.lon { - t.Fatalf("location = (%g,%g), want (%g,%g)", gotLat, gotLon, tc.lat, tc.lon) - } + assertNodeLocation(t, ctx, tx, []byte{0x97}, tc.wantLat, tc.wantLon, tc.wantSource) }) } + t.Run("new node with cleared location has no source", func(t *testing.T) { + fresh := ingest.UpsertNodeParams{PublicKey: []byte{0x98}, NodeType: 2, Name: "cleared fixture", ClearLocation: true} + if _, err := store.UpsertNode(ctx, fresh, ingest.RadioSettings{}); err != nil { + t.Fatal(err) + } + assertNodeLocation(t, ctx, tx, []byte{0x98}, nil, nil, nil) + }) +} + +func assertNodeLocation(t *testing.T, ctx context.Context, tx pgx.Tx, pubkey []byte, wantLat, wantLon *float64, wantSource *string) { + t.Helper() + var gotLat, gotLon *float64 + var gotSource *string + if err := tx.QueryRow(ctx, "SELECT latitude, longitude, location_source FROM nodes WHERE public_key=$1", pubkey).Scan(&gotLat, &gotLon, &gotSource); err != nil { + t.Fatal(err) + } + if !equalPtr(gotLat, wantLat) || !equalPtr(gotLon, wantLon) || !equalPtr(gotSource, wantSource) { + t.Fatalf("location = (%v,%v,%v), want (%v,%v,%v)", deref(gotLat), deref(gotLon), deref(gotSource), deref(wantLat), deref(wantLon), deref(wantSource)) + } +} + +func equalPtr[T comparable](a, b *T) bool { + return a == nil && b == nil || a != nil && b != nil && *a == *b +} + +func deref[T any](p *T) any { + if p == nil { + return nil + } + return *p } diff --git a/db/nodes.go b/db/nodes.go index 9cc53e3..a602cb3 100644 --- a/db/nodes.go +++ b/db/nodes.go @@ -34,6 +34,7 @@ func (s *Store) UpsertNode(ctx context.Context, n ingest.UpsertNodeParams, radio Name: &n.Name, Latitude: n.Latitude, Longitude: n.Longitude, + ClearLocation: n.ClearLocation, DeviceClockDriftSeconds: driftSeconds, } if radio.FreqMHz != 0 { diff --git a/db/queries/queries.sql b/db/queries/queries.sql index 77f1c51..c206a2f 100644 --- a/db/queries/queries.sql +++ b/db/queries/queries.sql @@ -745,13 +745,13 @@ ORDER BY po.heard_at ASC; -- name: UpsertNode :one INSERT INTO nodes (public_key, node_type, name, latitude, longitude, location_source, last_advert_at, last_seen, radio_freq_mhz, radio_sf, radio_bw_khz, device_clock_drift_seconds) -VALUES ($1, $2, $3, $4, $5, 'advert', NOW(), NOW(), $6, $7, $8, $9) +VALUES (@public_key, @node_type, @name, @latitude, @longitude, CASE WHEN @latitude::double precision IS NOT NULL THEN 'advert' END, NOW(), NOW(), @radio_freq_mhz, @radio_sf, @radio_bw_khz, @device_clock_drift_seconds) ON CONFLICT (public_key) DO UPDATE SET node_type = EXCLUDED.node_type, name = COALESCE(EXCLUDED.name, nodes.name), - latitude = COALESCE(EXCLUDED.latitude, nodes.latitude), - longitude = COALESCE(EXCLUDED.longitude, nodes.longitude), - location_source = CASE WHEN EXCLUDED.latitude IS NOT NULL THEN 'advert' ELSE nodes.location_source END, + latitude = CASE WHEN @clear_location::bool THEN NULL ELSE COALESCE(EXCLUDED.latitude, nodes.latitude) END, + longitude = CASE WHEN @clear_location::bool THEN NULL ELSE COALESCE(EXCLUDED.longitude, nodes.longitude) END, + location_source = CASE WHEN @clear_location::bool THEN NULL WHEN EXCLUDED.latitude IS NOT NULL THEN 'advert' ELSE nodes.location_source END, last_advert_at = NOW(), last_seen = NOW(), radio_freq_mhz = EXCLUDED.radio_freq_mhz, diff --git a/db/sqlc/queries.sql.go b/db/sqlc/queries.sql.go index dcd52c2..0a5aa36 100644 --- a/db/sqlc/queries.sql.go +++ b/db/sqlc/queries.sql.go @@ -4961,13 +4961,13 @@ func (q *Queries) UpsertKnownRoute(ctx context.Context, arg UpsertKnownRoutePara const upsertNode = `-- name: UpsertNode :one INSERT INTO nodes (public_key, node_type, name, latitude, longitude, location_source, last_advert_at, last_seen, radio_freq_mhz, radio_sf, radio_bw_khz, device_clock_drift_seconds) -VALUES ($1, $2, $3, $4, $5, 'advert', NOW(), NOW(), $6, $7, $8, $9) +VALUES ($1, $2, $3, $4, $5, CASE WHEN $4::double precision IS NOT NULL THEN 'advert' END, NOW(), NOW(), $6, $7, $8, $9) ON CONFLICT (public_key) DO UPDATE SET node_type = EXCLUDED.node_type, name = COALESCE(EXCLUDED.name, nodes.name), - latitude = COALESCE(EXCLUDED.latitude, nodes.latitude), - longitude = COALESCE(EXCLUDED.longitude, nodes.longitude), - location_source = CASE WHEN EXCLUDED.latitude IS NOT NULL THEN 'advert' ELSE nodes.location_source END, + latitude = CASE WHEN $10::bool THEN NULL ELSE COALESCE(EXCLUDED.latitude, nodes.latitude) END, + longitude = CASE WHEN $10::bool THEN NULL ELSE COALESCE(EXCLUDED.longitude, nodes.longitude) END, + location_source = CASE WHEN $10::bool THEN NULL WHEN EXCLUDED.latitude IS NOT NULL THEN 'advert' ELSE nodes.location_source END, last_advert_at = NOW(), last_seen = NOW(), radio_freq_mhz = EXCLUDED.radio_freq_mhz, @@ -4987,6 +4987,7 @@ type UpsertNodeParams struct { RadioSf *int16 `json:"radio_sf"` RadioBwKhz *float32 `json:"radio_bw_khz"` DeviceClockDriftSeconds *int32 `json:"device_clock_drift_seconds"` + ClearLocation bool `json:"clear_location"` } // ============================================================ @@ -5003,6 +5004,7 @@ func (q *Queries) UpsertNode(ctx context.Context, arg UpsertNodeParams) (Node, e arg.RadioSf, arg.RadioBwKhz, arg.DeviceClockDriftSeconds, + arg.ClearLocation, ) var i Node err := row.Scan( diff --git a/internal/borders/borders.go b/internal/borders/borders.go index 1ab1501..ad2c560 100644 --- a/internal/borders/borders.go +++ b/internal/borders/borders.go @@ -61,11 +61,11 @@ func validPosition(lat, lng float64) bool { } // PossiblyForeign is nil when disabled, the role is not repeater, or position -// is unknown/invalid (including the protocol's 0/0 location reset). +// is unknown/invalid. // All polygon boundaries, including hole edges, count as local; hole interiors // remain outside. Location is self-reported, so this is an indication only. func (local *Local) PossiblyForeign(role int16, lat, lng *float64) *bool { - if local == nil || role != int16(meshcore.AdvertTypeRepeater) || lat == nil || lng == nil || !validPosition(*lat, *lng) || *lat == 0 && *lng == 0 { + if local == nil || role != int16(meshcore.AdvertTypeRepeater) || lat == nil || lng == nil || !validPosition(*lat, *lng) { return nil } point := orb.Point{*lng, *lat} diff --git a/internal/borders/borders_test.go b/internal/borders/borders_test.go index a90aaf9..c5d4fdf 100644 --- a/internal/borders/borders_test.go +++ b/internal/borders/borders_test.go @@ -34,7 +34,7 @@ func TestPossiblyForeign(t *testing.T) { {"companion", 1, 25, 25, nil}, {"room", 3, 25, 25, nil}, {"sensor", 4, 25, 25, nil}, - {"zero reset", 2, 0, 0, nil}, + {"null island is a position", 2, 0, 0, new(true)}, {"equator is valid", 2, 0, 25, new(true)}, {"prime meridian is valid", 2, 25, 0, new(true)}, {"invalid latitude", 2, 91, 25, nil}, diff --git a/internal/ingest/side_effects.go b/internal/ingest/side_effects.go index 0913242..46fa8fe 100644 --- a/internal/ingest/side_effects.go +++ b/internal/ingest/side_effects.go @@ -22,6 +22,8 @@ type UpsertNodeParams struct { NodeType uint8 // 1=companion, 2=repeater, 3=room server Latitude *float64 Longitude *float64 + // ClearLocation drops the stored position; set by an explicit 0/0 advert. + ClearLocation bool // AdvertTimestamp is the device's self-reported wall-clock time (epoch seconds) from the // signed advert body. Used to derive clock drift for repeaters/room servers; see // api.Node.ClockDriftSeconds. @@ -65,8 +67,8 @@ type nodeUpdateEvent struct { NodeType uint8 `json:"nodeType"` NodeTypeName string `json:"nodeTypeName"` IATA string `json:"iata"` - Lat *float64 `json:"lat,omitempty"` - Lng *float64 `json:"lng,omitempty"` + Lat **float64 `json:"lat,omitempty"` // Omitted keeps the client's position; explicit null clears it. + Lng **float64 `json:"lng,omitempty"` IsObserver bool `json:"isObserver"` IATAs []api.NodeIATA `json:"iatas"` DefaultScope *string `json:"defaultScope,omitempty"` @@ -92,8 +94,10 @@ func (w *Worker) handlePayloadTypeSideEffects(ctx context.Context, packet *meshc return } var lat, lon *float64 - // Presence, not value: an explicit 0/0 advert resets the stored location. - if advert.Flags()&meshcore.AdvertLatLonMask != 0 { + present := advert.Flags()&meshcore.AdvertLatLonMask != 0 + // Presence decides: omitted keeps the stored position, an explicit 0/0 clears it. + cleared := present && advert.AppData().Lat == 0 && advert.AppData().Lon == 0 + if present && !cleared { la := float64(advert.AppData().Lat) / 1e6 lo := float64(advert.AppData().Lon) / 1e6 lat = &la @@ -105,6 +109,7 @@ func (w *Worker) handlePayloadTypeSideEffects(ctx context.Context, packet *meshc NodeType: advert.Type(), Latitude: lat, Longitude: lon, + ClearLocation: cleared, AdvertTimestamp: advert.Timestamp, } var nodeRadio RadioSettings @@ -179,14 +184,15 @@ func (w *Worker) handlePayloadTypeSideEffects(ctx context.Context, packet *meshc NodeType: advert.Type(), NodeTypeName: api.NodeTypeName(int16(advert.Type())), IATA: iata, - Lat: lat, - Lng: lon, IsObserver: isObserver, IATAs: []api.NodeIATA{{IATA: iata, LastHeard: time.Now().UnixMilli()}}, DefaultScope: defaultScope, Radio: radioStr, } - if w.cfg.LocalBorders != nil && (lat != nil || advert.Type() != meshcore.AdvertTypeRepeater) { + if present { + evt.Lat, evt.Lng = &lat, &lon + } + if w.cfg.LocalBorders != nil && (present || advert.Type() != meshcore.AdvertTypeRepeater) { foreign := w.cfg.LocalBorders.PossiblyForeign(int16(advert.Type()), lat, lon) evt.PossiblyForeign = &foreign } diff --git a/internal/ingest/side_effects_test.go b/internal/ingest/side_effects_test.go index 782b79f..93c5a4e 100644 --- a/internal/ingest/side_effects_test.go +++ b/internal/ingest/side_effects_test.go @@ -9,9 +9,11 @@ import ( "encoding/binary" "encoding/hex" "encoding/json" + "strings" "testing" "time" + "github.com/MeshCore-Beacon/beacon-server/internal/hub" "github.com/MeshCore-Beacon/beacon-server/internal/keystore" "github.com/meshcore-go/meshcore-go" ) @@ -68,16 +70,16 @@ func buildAdvertPacketWithData(t *testing.T, data []byte, tamper bool) *meshcore func TestAdvertLocationPresence(t *testing.T) { for _, tc := range []struct { - name string - present, tamper bool - lat, lon int32 + name string + present, tamper, clear bool + lat, lon int32 }{ - {"ordinary location", true, false, 45000000, -75000000}, - {"explicit reset", true, false, 0, 0}, - {"zero latitude", true, false, 0, -75000000}, - {"zero longitude", true, false, 45000000, 0}, - {"no location", false, false, 0, 0}, - {"tampered reset", true, true, 0, 0}, + {"ordinary location", true, false, false, 45000000, -75000000}, + {"explicit reset", true, false, true, 0, 0}, + {"zero latitude", true, false, false, 0, -75000000}, + {"zero longitude", true, false, false, 45000000, 0}, + {"no location", false, false, false, 0, 0}, + {"tampered reset", true, true, false, 0, 0}, } { t.Run(tc.name, func(t *testing.T) { data := []byte{meshcore.AdvertTypeRepeater} @@ -101,7 +103,10 @@ func TestAdvertLocationPresence(t *testing.T) { t.Fatal("signed advert did not update the node") } got := store.upsertNodeParams - if !tc.present { + if got.ClearLocation != tc.clear { + t.Fatalf("ClearLocation = %v, want %v", got.ClearLocation, tc.clear) + } + if !tc.present || tc.clear { if got.Latitude != nil || got.Longitude != nil { t.Fatal("absent location became an update") } @@ -114,6 +119,58 @@ func TestAdvertLocationPresence(t *testing.T) { } } +func TestAdvertClearEmitsNullLocation(t *testing.T) { + w, _ := newTestWorker() + go w.hub.Run() + client := w.hub.NewClient() + defer w.hub.Remove(client) + w.hub.AddScope(client, "clear", hub.Scope{Events: []hub.EventType{hub.EventNodeUpdate, hub.EventObserverStatus}}) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + waitForSummarySubscriber(t, ctx, w.hub, client) + for _, tc := range []struct { + name string + present, want bool + lat, lon int32 + wantLat string + wantLng string + }{ + {"explicit reset sends null", true, true, 0, 0, "null", "null"}, + {"omission sends nothing", false, false, 0, 0, "", ""}, + {"real location sends coordinates", true, true, 45000000, -75000000, "45", "-75"}, + } { + t.Run(tc.name, func(t *testing.T) { + data := []byte{meshcore.AdvertTypeRepeater} + if tc.present { + data[0] |= meshcore.AdvertLatLonMask + data = binary.LittleEndian.AppendUint32(data, uint32(tc.lat)) + data = binary.LittleEndian.AppendUint32(data, uint32(tc.lon)) + } + w.handlePayloadTypeSideEffects(ctx, buildAdvertPacketWithData(t, data, false), "AAA", []byte{1}, RadioSettings{}, nil, nil, nil, 0) + for { + select { + case event := <-client.Send: + if event.Type != hub.EventNodeUpdate { + continue + } + payload := string(event.Payload) + for field, want := range map[string]string{`"lat"`: tc.wantLat, `"lng"`: tc.wantLng} { + if tc.want && !strings.Contains(payload, field+":"+want+",") { + t.Fatalf("%s != %s: %s", field, want, payload) + } + if !tc.want && strings.Contains(payload, field) { + t.Fatalf("%s present on omission: %s", field, payload) + } + } + return + case <-ctx.Done(): + t.Fatal("node update not received") + } + } + }) + } +} + func TestHandlePayloadTypeSideEffects_Advert_ValidSignature_UpsertsNode(t *testing.T) { w, db := newTestWorker() packet := buildAdvertPacket(t, false)