mirror of
https://github.com/MeshCore-Beacon/beacon-server.git
synced 2026-10-23 01:51:27 +00:00
fix(ingest): clear a node's location on an explicit 0/0 advert
A repeater that advertises 0/0 is hiding its position. It was stored as a literal 0/0 that every consumer had to special-case; now the stored latitude, longitude and location source are cleared, an omitted location still preserves the stored one, and the nodeUpdate event sends explicit null lat/lng so clients can clear too. Migration 043 drops rows stored as 0/0 before this change.
This commit is contained in:
1 parent
f6fe177a4e
commit
689bc2326a
11 files changed
+151
-49
No files matched your search
@@ -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: |
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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));
|
||||
@@ -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
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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},
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user