From 83d139a62da215592f3186c0e7b5670a7fd9db53 Mon Sep 17 00:00:00 2001 From: MarekZegare4 Date: Wed, 24 Jun 2026 17:34:53 +0200 Subject: [PATCH] refactor(companion): unify the active target across Locator/Nav entry points Three notions of "set a target" had drifted apart: LocatorScreen's picker hand-wrote the prefs fields and deferred the save, while the Nearby/Waypoints popups wrote the same fields again and saved immediately, and the live>last- advertised position precedence was duplicated between the engine and the picker. - setTarget() is now the single definition of the active target (fields + re-arm). setTargetNow() wraps it for popups (immediate save + confirm); the Locator screen keeps its deferred save so LEFT/RIGHT cycling doesn't thrash flash. applyTarget() delegates instead of re-writing the fields. - resolvePersonPos() is the single live-share / last-advertised precedence, used by both locatorDistance() and the picker. - Consistent action verb "Set as target" in Nearby and Waypoints menus. No behaviour change for the user beyond the relabel; pure consolidation. Co-Authored-By: Claude Opus 4.8 --- .../companion_radio/ui-new/LocatorScreen.h | 27 ++++------ .../companion_radio/ui-new/NearbyScreen.h | 4 +- examples/companion_radio/ui-new/UITask.cpp | 53 +++++++++++++------ examples/companion_radio/ui-new/UITask.h | 24 ++++++--- .../companion_radio/ui-new/WaypointsView.h | 6 +-- 5 files changed, 67 insertions(+), 47 deletions(-) diff --git a/examples/companion_radio/ui-new/LocatorScreen.h b/examples/companion_radio/ui-new/LocatorScreen.h index ddec2ed9..55f75439 100644 --- a/examples/companion_radio/ui-new/LocatorScreen.h +++ b/examples/companion_radio/ui-new/LocatorScreen.h @@ -9,9 +9,9 @@ // (favourites first — offered even with no known position yet, so you can arm // ahead of time — then any other contact with a currently-known position: // live-sharing or just last-advertised, e.g. a repeater; then waypoints). -// LEFT/RIGHT quick-cycles the same set. A target can also be set directly from -// Nearby Nodes' or Waypoints' own menu (UITask::setLocatorTarget()), bypassing -// this picker entirely. +// LEFT/RIGHT quick-cycles the same set. The same active target can also be set +// directly from Nearby Nodes' or Waypoints' own "Set as target" menu item +// (UITask::setTargetNow()), bypassing this picker entirely. // Included by UITask.cpp after LiveShareScreen.h. #include "../NodePrefs.h" @@ -160,14 +160,8 @@ public: if (_targets[j].kind == 1 && memcmp(_targets[j].key, key, 6) == 0) return false; // already added int32_t lat = 0, lon = 0; uint32_t ts = 0; bool live = false; - const LiveTrackStore::Entry* e = _task->liveTrack().activeByKey(key, rtc_clock.getCurrentTime()); - if (e) { - live = true; ts = e->ts; lat = e->lat_1e6; lon = e->lon_1e6; - } else { - ContactInfo* c = the_mesh.lookupContactByPubKey(key, 6); - if (c && (c->gps_lat || c->gps_lon)) { ts = c->lastmod; lat = c->gps_lat; lon = c->gps_lon; } - } - if (require_position && !live && ts == 0) return false; // nothing to navigate to yet + bool has_pos = _task->resolvePersonPos(key, lat, lon, &live, &ts); // same precedence as the engine + if (require_position && !has_pos) return false; // nothing to navigate to yet Target& t = _targets[_target_n++]; t.kind = 1; t.lat = lat; t.lon = lon; t.ts = ts; t.live = live; @@ -222,14 +216,11 @@ public: } void applyTarget(const Target& t) { - _prefs->locator_target_kind = t.kind; - if (t.kind == 1) memcpy(_prefs->locator_key, t.key, 6); - _prefs->locator_lat_1e6 = t.lat; - _prefs->locator_lon_1e6 = t.lon; - snprintf(_prefs->locator_label, sizeof(_prefs->locator_label), "%s", t.name); - _prefs->locator_has_target = 1; + // Same target definition as every other entry point (UITask::setTarget), + // but the save is deferred to screen exit (_dirty) so LEFT/RIGHT cycling + // through candidates doesn't write flash on each step. + _task->setTarget(t.kind, t.kind == 1 ? t.key : nullptr, t.lat, t.lon, t.name); _dirty = true; - _task->resetLocator(); } // LEFT/RIGHT quick-cycle over the same set the picker shows. diff --git a/examples/companion_radio/ui-new/NearbyScreen.h b/examples/companion_radio/ui-new/NearbyScreen.h index 211c2533..57a503ff 100644 --- a/examples/companion_radio/ui-new/NearbyScreen.h +++ b/examples/companion_radio/ui-new/NearbyScreen.h @@ -470,7 +470,7 @@ class NearbyScreen : public UIScreen { if (has_gps) add("Save waypoint", ACT_WAYPOINT); // Needs both a position and a stable identity — a person target is keyed // by pubkey prefix, so a name-only live-scan/channel row can't offer this. - if (has_gps && has_key) add("Set Locator target", ACT_LOCATOR); + if (has_gps && has_key) add("Set as target", ACT_LOCATOR); if (stored) add(_sort_label, ACT_SORT); // sort is meaningless for live-scan rows add(stored ? "Discover scan" : "Rescan", ACT_SCAN); } @@ -494,7 +494,7 @@ class NearbyScreen : public UIScreen { case ACT_LOCATOR: { const Entry* e = selected(); if (e && e->has_key && (e->lat_e6 != 0 || e->lon_e6 != 0)) - _task->setLocatorTarget(1, e->pub_key, e->lat_e6, e->lon_e6, e->name); + _task->setTargetNow(1, e->pub_key, e->lat_e6, e->lon_e6, e->name); break; } case ACT_SORT: break; // adjusted in-place via LEFT/RIGHT, not ENTER diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index 4dccbb67..e6a5a13d 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -2272,23 +2272,37 @@ void UITask::loop() { // configured radius (m). Returns false when no target is set or there's no fix // — the single place the target-distance maths lives, shared by the crossing // evaluator and the proximity beeper. +// One precedence for a person's position — an active [LOC] live share wins, +// else the last-advertised GPS fix. Not everyone keeps live-sharing on, so the +// fallback lets a rarely-updating but stationary node (a repeater, or someone +// who shared a fix once) still work as a target. +bool UITask::resolvePersonPos(const uint8_t* key, int32_t& lat, int32_t& lon, + bool* live, uint32_t* ts) const { + if (live) *live = false; + if (ts) *ts = 0; + if (!key) return false; + const LiveTrackStore::Entry* e = + _livetrack.activeByKey(key, (uint32_t)rtc_clock.getCurrentTime()); + if (e) { + lat = e->lat_1e6; lon = e->lon_1e6; + if (live) *live = true; + if (ts) *ts = e->ts; + return true; + } + ContactInfo* c = the_mesh.lookupContactByPubKey(key, NodePrefs::FAVOURITE_PREFIX_LEN); + if (c && (c->gps_lat != 0 || c->gps_lon != 0)) { + lat = c->gps_lat; lon = c->gps_lon; + if (ts) *ts = c->lastmod; + return true; + } + return false; +} + bool UITask::locatorDistance(float& dist_m, float& radius_m) const { if (!_node_prefs || !_node_prefs->locator_has_target) return false; int32_t tlat, tlon; if (_node_prefs->locator_target_kind == 1) { - // Live contact: prefer the latest [LOC] position. With no active share — - // not everyone keeps live-sharing on — fall back to their last-advertised - // position, so a rarely-updating but stationary node (a repeater, or a - // contact who simply shared a fix once) still works as a target. - const LiveTrackStore::Entry* e = - _livetrack.activeByKey(_node_prefs->locator_key, (uint32_t)rtc_clock.getCurrentTime()); - if (e) { - tlat = e->lat_1e6; tlon = e->lon_1e6; - } else { - ContactInfo* c = the_mesh.lookupContactByPubKey(_node_prefs->locator_key, NodePrefs::FAVOURITE_PREFIX_LEN); - if (!c || (c->gps_lat == 0 && c->gps_lon == 0)) return false; - tlat = c->gps_lat; tlon = c->gps_lon; - } + if (!resolvePersonPos(_node_prefs->locator_key, tlat, tlon)) return false; } else { tlat = _node_prefs->locator_lat_1e6; tlon = _node_prefs->locator_lon_1e6; } @@ -2333,17 +2347,22 @@ void UITask::fireLocator(bool arrived) { playMelody(arrived ? "locarr:d=8,o=6,b=140:c,e,g" : "loclv:d=8,o=6,b=140:g,e,c"); } -void UITask::setLocatorTarget(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name) { +void UITask::setTarget(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name) { if (!_node_prefs) return; _node_prefs->locator_target_kind = kind; - if (kind == 1) memcpy(_node_prefs->locator_key, key, NodePrefs::FAVOURITE_PREFIX_LEN); + if (kind == 1 && key) memcpy(_node_prefs->locator_key, key, NodePrefs::FAVOURITE_PREFIX_LEN); _node_prefs->locator_lat_1e6 = lat; _node_prefs->locator_lon_1e6 = lon; snprintf(_node_prefs->locator_label, sizeof(_node_prefs->locator_label), "%s", name); _node_prefs->locator_has_target = 1; + resetLocator(); // re-seed the crossing engine so the change can't fire on a stale state +} + +void UITask::setTargetNow(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name) { + if (!_node_prefs) return; + setTarget(kind, key, lat, lon, name); the_mesh.savePrefs(); - resetLocator(); - showAlert("Locator target set", 1200); + showAlert("Target set", 1200); } // Homing beeper: while armed with a target and inside the radius, emit a short diff --git a/examples/companion_radio/ui-new/UITask.h b/examples/companion_radio/ui-new/UITask.h index bd8a87f3..b4296959 100644 --- a/examples/companion_radio/ui-new/UITask.h +++ b/examples/companion_radio/ui-new/UITask.h @@ -194,13 +194,23 @@ public: // silently (called by the Locator tool after the target/radius changes, // so re-entering the zone doesn't fire on a stale inside/outside state). void resetLocator() { _locator_known = false; } - // Quick one-shot "make this my Locator target" — used by Nearby Nodes' and - // Waypoints' per-item menus so picking a target doesn't require a detour - // through Tools › Locator. Unlike LocatorScreen's own picker (which batches - // edits and saves on exit), this saves and re-arms immediately, with an - // on-screen confirmation, since there's no screen-exit point to hook here. - // kind 0 = waypoint (key ignored), 1 = person (key required, 6-byte prefix). - void setLocatorTarget(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name); + // The one "active target" the device tracks — shared by the Locator geofence, + // the Nav bearing/ETA view and (future) the map focus, so every entry point + // sets the same thing. kind 0 = waypoint (key ignored), 1 = person (key + // required, 6-byte prefix). setTarget() only *defines* the target (fields + + // re-arm); the caller decides when to persist. Two commit policies, by + // context: a screen with an exit hook (LocatorScreen) batches the save so + // LEFT/RIGHT cycling doesn't thrash flash, while a per-item popup with no + // such hook uses setTargetNow() to save + confirm on the spot. + void setTarget(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name); + void setTargetNow(uint8_t kind, const uint8_t* key, int32_t lat, int32_t lon, const char* name); + // Resolve a person target (6-byte pubkey prefix) to a current position: + // prefers an active [LOC] live share, falls back to their last-advertised + // GPS fix. Returns false when neither is known. Optional live/ts report + // freshness for the picker's age tag. One precedence, used by both the + // Locator engine (locatorDistance) and the target picker. + bool resolvePersonPos(const uint8_t* key, int32_t& lat, int32_t& lon, + bool* live = nullptr, uint32_t* ts = nullptr) const; void gotoTrailScreen(); void gotoMapScreen(); // opens the Trail screen directly in its Map view void gotoCompassScreen(); diff --git a/examples/companion_radio/ui-new/WaypointsView.h b/examples/companion_radio/ui-new/WaypointsView.h index d5ef6666..ab66f71f 100644 --- a/examples/companion_radio/ui-new/WaypointsView.h +++ b/examples/companion_radio/ui-new/WaypointsView.h @@ -283,10 +283,10 @@ public: else snprintf(text, sizeof(text), WAYPOINT_MSG_TAG "%.5f,%.5f", lat, lon); _task->shareToMessage(text); // hands off to the Messages screen } - } else { // Set Locator target + } else { // Set as target if (wi >= 0 && wi < _task->waypoints().count()) { const Waypoint& w = _task->waypoints().at(wi); - _task->setLocatorTarget(0, nullptr, w.lat_1e6, w.lon_1e6, w.label); + _task->setTargetNow(0, nullptr, w.lat_1e6, w.lon_1e6, w.label); } } } @@ -348,7 +348,7 @@ public: _ctx.addItem("Rename"); _ctx.addItem("Delete"); _ctx.addItem("Send"); - _ctx.addItem("Locator target"); + _ctx.addItem("Set as target"); return true; } return true;