From a8623a60c52e8cacef7521c8733552482f182d40 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 28 Jul 2026 09:17:09 -0500 Subject: [PATCH] Stream our own position to the phone/UI while mesh position sharing is opt-in (#11270) * Position: stream our own position to the phone/UI while mesh sharing is opt-in Position broadcasts became opt-in in 2.8 (#10929): with every public channel at position_precision 0, sendOurPosition() finds no eligible channel and returns without queueing anything, so the connected phone or on-device UI never sees the node's own GPS fix ("GPS looks dead" on standalone MUI devices even though the receiver has a lock). Mirror device telemetry's local delivery: once a minute, when the toPhone queue is idle, stream our own position to the connected client at full precision. The packet is handed straight to sendToPhone() and never touches the mesh, so the per-channel opt-in and the public-channel precision clamp still govern everything on the air. Also stop logging "Send pos ... to mesh" before the channel scan has found an eligible channel; when sharing is disabled everywhere the skip is now logged explicitly instead of pretending a send happened. The cadence gate is a pure static (shouldSendPositionToPhone) alongside the existing broadcast-policy helpers, with unit tests covering the first-send, cadence, gating, and millis() rollover cases. * Review: only advance phone cadence on a queued packet; drop the ms==0 sentinel sendOurPositionToPhone() now reports whether a packet actually reached the phone queue, and runOnce() stamps the cadence only on success, so a guard or allocation failure retries on the next tick instead of waiting out a minute. The never-sent state is a dedicated hasSentPositionToPhone flag rather than lastPhoneSendMs == 0, so a send stamped exactly at millis() == 0 still holds the cadence. New regression test covers that case; existing cases updated to the explicit flag. * Review: align the rollover test fixture with its documented elapsed times lastSent now sits exactly 30,000 ms before the uint32 wrap, so the two cases are precisely 70,000 ms (sends) and 40,000 ms (held) - the previous comments claimed 70s/20s against actual elapsed values of 70,001/40,001 ms. --- src/modules/PositionModule.cpp | 57 +++++++++++++++++++++---- src/modules/PositionModule.h | 15 ++++++- test/test_position_module/test_main.cpp | 57 +++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 9 deletions(-) diff --git a/src/modules/PositionModule.cpp b/src/modules/PositionModule.cpp index 40055006a..cb08503e4 100644 --- a/src/modules/PositionModule.cpp +++ b/src/modules/PositionModule.cpp @@ -167,9 +167,9 @@ bool PositionModule::hasGPS() } // Allocate a packet with our position data if we have one -meshtastic_MeshPacket *PositionModule::allocPositionPacket() +meshtastic_MeshPacket *PositionModule::allocPositionPacket(uint32_t atPrecision) { - if (precision == 0) { + if (atPrecision == 0) { LOG_DEBUG("Skip location send because precision is set to 0!"); return nullptr; } @@ -196,12 +196,12 @@ meshtastic_MeshPacket *PositionModule::allocPositionPacket() } // lat/lon are unconditionally included - IF AVAILABLE! - LOG_DEBUG("Send location with precision %i", precision); + LOG_DEBUG("Send location with precision %i", atPrecision); p.latitude_i = localPosition.latitude_i; p.longitude_i = localPosition.longitude_i; p.has_latitude_i = true; p.has_longitude_i = true; - applyPositionPrecision(p, precision); + applyPositionPrecision(p, atPrecision); // Always use NTP / GPS time if available if (getValidTime(RTCQualityNTP) > 0) { p.time = getValidTime(RTCQualityNTP); @@ -281,7 +281,7 @@ meshtastic_MeshPacket *PositionModule::allocReply() return nullptr; } - meshtastic_MeshPacket *reply = allocPositionPacket(); + meshtastic_MeshPacket *reply = allocPositionPacket(precision); if (reply) { lastSentReply = millis(); // Track when we sent this reply } @@ -373,13 +373,34 @@ void PositionModule::sendOurPosition() currentGeneration = radioGeneration; // If we changed channels, ask everyone else for their latest info - LOG_INFO("Send pos@%x:6 to mesh (wantReplies=%d)", localPosition.timestamp, requestReplies); for (uint8_t channelNum = 0; channelNum < 8; channelNum++) { if (getPositionPrecisionForChannel(channelNum) != 0) { + LOG_INFO("Send pos@%x:6 to mesh (wantReplies=%d)", localPosition.timestamp, requestReplies); sendOurPosition(NODENUM_BROADCAST, requestReplies, channelNum); return; } } + LOG_INFO("Skip pos@%x:6 broadcast; position sharing is opt-in and disabled on all channels", localPosition.timestamp); +} + +// Position broadcasts are opt-in per channel in 2.8, but our own position still plays to the +// connected phone/UI at full precision, like telemetry does. This copy never touches the mesh. +// Returns true only when a packet was actually handed to the phone queue, so the caller can +// hold off the cadence stamp (and retry soon) after a guard or allocation failure. +bool PositionModule::sendOurPositionToPhone() +{ + if (!config.position.fixed_position && !nodeDB->hasLocalPositionSinceBoot()) + return false; // Same stale-restored-position guard as sendOurPosition() + + meshtastic_MeshPacket *p = allocPositionPacket(32); + if (p == nullptr) + return false; + + p->to = NODENUM_BROADCAST; + p->decoded.want_response = false; + p->priority = meshtastic_MeshPacket_Priority_BACKGROUND; + service->sendToPhone(p); + return true; } void PositionModule::sendOurPosition(NodeNum dest, bool wantReplies, uint8_t channel) @@ -396,7 +417,7 @@ void PositionModule::sendOurPosition(NodeNum dest, bool wantReplies, uint8_t cha // Set the class precision value for this particular packet. precision = getPositionPrecisionForChannel(channel); - meshtastic_MeshPacket *p = allocPositionPacket(); + meshtastic_MeshPacket *p = allocPositionPacket(precision); if (p == nullptr) { LOG_DEBUG("allocPositionPacket returned a nullptr"); return; @@ -465,6 +486,14 @@ bool PositionModule::positionWithinPrecisionCell(int32_t aLat, int32_t aLon, int truncateCoordinate(aLon, precision) == truncateCoordinate(bLon, precision); } +bool PositionModule::shouldSendPositionToPhone(bool hasValidPosition, bool phoneQueueEmpty, bool everSentToPhone, uint32_t nowMs, + uint32_t lastSentMs, uint32_t intervalMs) +{ + if (!hasValidPosition || !phoneQueueEmpty) + return false; + return !everSentToPhone || (nowMs - lastSentMs) >= intervalMs; +} + uint32_t PositionModule::effectiveBroadcastIntervalMs(uint32_t configuredIntervalMs, bool stationary, uint32_t stationaryFloorMs) { if (stationary && stationaryFloorMs > configuredIntervalMs) @@ -485,8 +514,20 @@ int32_t PositionModule::runOnce() if (node == nullptr) return RUNONCE_INTERVAL; - // We limit our GPS broadcasts to a max rate uint32_t now = millis(); + + // Local-only delivery, so it runs regardless of mesh opt-in state or channel utilization. + // Only send while the queue is empty (phone assumed connected), like telemetry. The cadence + // stamp only advances when a packet was actually queued, so a guard or allocation failure + // retries on the next tick instead of waiting out a full interval. + if (shouldSendPositionToPhone(nodeDB->hasValidPosition(node), service->isToPhoneQueueEmpty(), hasSentPositionToPhone, now, + lastPhoneSendMs, sendToPhoneIntervalMs) && + sendOurPositionToPhone()) { + hasSentPositionToPhone = true; + lastPhoneSendMs = now; + } + + // We limit our GPS broadcasts to a max rate uint32_t intervalMs = Default::getConfiguredOrDefaultMsScaled( config.position.position_broadcast_secs, default_broadcast_interval_secs, numOnlineNodes, TrafficType::POSITION); uint32_t msSinceLastSend = now - lastGpsSend; diff --git a/src/modules/PositionModule.h b/src/modules/PositionModule.h index bfd48047b..03754c22b 100644 --- a/src/modules/PositionModule.h +++ b/src/modules/PositionModule.h @@ -45,6 +45,12 @@ class PositionModule : public ProtobufModule, private concu // Effective min interval: stationary positions are held to stationaryFloorMs (when that is the // longer of the two); otherwise the normal configured interval. static uint32_t effectiveBroadcastIntervalMs(uint32_t configuredIntervalMs, bool stationary, uint32_t stationaryFloorMs); + // Pure local-play policy: stream our own position to the phone/UI when we have a valid + // position, the phone queue is idle, and the cadence has elapsed. everSentToPhone (rather + // than a lastSentMs sentinel) marks the never-sent state, so a send stamped at millis()==0 + // still honors the cadence. + static bool shouldSendPositionToPhone(bool hasValidPosition, bool phoneQueueEmpty, bool everSentToPhone, uint32_t nowMs, + uint32_t lastSentMs, uint32_t intervalMs); protected: /** Called to handle a particular incoming message @@ -63,7 +69,14 @@ class PositionModule : public ProtobufModule, private concu virtual int32_t runOnce() override; private: - meshtastic_MeshPacket *allocPositionPacket(); + meshtastic_MeshPacket *allocPositionPacket(uint32_t atPrecision); + // Streams our own position to the connected phone/UI at full precision without touching the + // mesh. Keeps the local view alive now that mesh position sharing is opt-in. Returns true + // only when a packet was handed to the phone queue. + bool sendOurPositionToPhone(); + bool hasSentPositionToPhone = false; + uint32_t lastPhoneSendMs = 0; + static constexpr uint32_t sendToPhoneIntervalMs = 60 * 1000; // Matches telemetry's local cadence struct SmartPosition getDistanceTraveledSinceLastSend(meshtastic_PositionLite currentPosition); // True when our position is unchanged since the last broadcast: it truncates to the same // precision grid cell, so re-sending would be a duplicate that traffic management dedups diff --git a/test/test_position_module/test_main.cpp b/test/test_position_module/test_main.cpp index 091eacd48..a5c2c8ea5 100644 --- a/test/test_position_module/test_main.cpp +++ b/test/test_position_module/test_main.cpp @@ -56,6 +56,56 @@ static void test_effectiveInterval_zeroFloorIsNoOp() TEST_ASSERT_EQUAL_UINT32(60000U, PositionModule::effectiveBroadcastIntervalMs(60000U, true, 0U)); } +// Local phone/UI play: positions are opt-in on the mesh, but our own position still streams to +// the connected phone at a fixed cadence (mirroring telemetry's local delivery). + +// A never-sent state streams immediately once a valid position exists, regardless of the clock. +static void test_sendToPhone_firstSendIsImmediate() +{ + TEST_ASSERT_TRUE(PositionModule::shouldSendPositionToPhone(true, true, false, 5000U, 0U, 60000U)); + TEST_ASSERT_TRUE(PositionModule::shouldSendPositionToPhone(true, true, false, 0U, 0U, 60000U)); +} + +static void test_sendToPhone_holdsUntilCadenceElapses() +{ + TEST_ASSERT_FALSE(PositionModule::shouldSendPositionToPhone(true, true, true, 59999U, 1U, 60000U)); +} + +static void test_sendToPhone_sendsWhenCadenceElapses() +{ + TEST_ASSERT_TRUE(PositionModule::shouldSendPositionToPhone(true, true, true, 60001U, 1U, 60000U)); +} + +// No valid local position yet (e.g. GPS has no fix since boot): nothing to stream. +static void test_sendToPhone_requiresValidPosition() +{ + TEST_ASSERT_FALSE(PositionModule::shouldSendPositionToPhone(false, true, true, 60001U, 1U, 60000U)); +} + +// A backed-up toPhone queue means no client is draining it; don't pile on. +static void test_sendToPhone_requiresIdlePhoneQueue() +{ + TEST_ASSERT_FALSE(PositionModule::shouldSendPositionToPhone(true, false, true, 60001U, 1U, 60000U)); +} + +// millis() rollover: unsigned subtraction keeps the elapsed math correct across the wrap. +static void test_sendToPhone_survivesMillisRollover() +{ + constexpr uint32_t lastSent = UINT32_MAX - 29999U; // exactly 30,000 ms before the wrap to 0 + // "now" 40s after the wrap: 70,000 ms elapsed >= the 60s cadence, so it sends. + TEST_ASSERT_TRUE(PositionModule::shouldSendPositionToPhone(true, true, true, 40000U, lastSent, 60000U)); + // "now" 10s after the wrap: only 40,000 ms elapsed, still held. + TEST_ASSERT_FALSE(PositionModule::shouldSendPositionToPhone(true, true, true, 10000U, lastSent, 60000U)); +} + +// Regression: a send stamped at millis()==0 must still hold the cadence - the never-sent state +// is the everSentToPhone flag, not a lastSentMs==0 sentinel. +static void test_sendToPhone_sentAtTimeZeroStillHoldsCadence() +{ + TEST_ASSERT_FALSE(PositionModule::shouldSendPositionToPhone(true, true, true, 30000U, 0U, 60000U)); + TEST_ASSERT_TRUE(PositionModule::shouldSendPositionToPhone(true, true, true, 60000U, 0U, 60000U)); +} + void setUp(void) {} void tearDown(void) {} @@ -74,6 +124,13 @@ void setup() RUN_TEST(test_effectiveInterval_movingKeepsConfigured); RUN_TEST(test_effectiveInterval_longConfiguredWinsOverFloor); RUN_TEST(test_effectiveInterval_zeroFloorIsNoOp); + RUN_TEST(test_sendToPhone_firstSendIsImmediate); + RUN_TEST(test_sendToPhone_holdsUntilCadenceElapses); + RUN_TEST(test_sendToPhone_sendsWhenCadenceElapses); + RUN_TEST(test_sendToPhone_requiresValidPosition); + RUN_TEST(test_sendToPhone_requiresIdlePhoneQueue); + RUN_TEST(test_sendToPhone_survivesMillisRollover); + RUN_TEST(test_sendToPhone_sentAtTimeZeroStillHoldsCadence); exit(UNITY_END()); }