diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 0667e0b13e..cfe213ff9b 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -419,27 +419,17 @@ bool MeshService::trySendPosition(NodeNum dest, bool wantReplies) LOG_DEBUG("Skip position ping; no fresh position since boot"); return false; } - // Prefer the node's current channel, but fall back to the first channel with - // position enabled (matching PositionModule::sendOurPosition() behavior). + // Prefer the node's current channel, but fall back to the position channel + // (matching PositionModule::sendOurPosition() behavior). uint8_t sendChan = node->channel; - if (getPositionPrecisionForChannel(sendChan) == 0) { - bool found = false; - for (uint8_t ch = 0; ch < 8; ++ch) { - if (getPositionPrecisionForChannel(ch) != 0) { - sendChan = ch; - found = true; - break; - } - } - if (!found) { - // No channel with position enabled: fall back to sending nodeinfo, as before. - if (nodeInfoModule) { - LOG_INFO("No position-enabled channel; send nodeinfo instead to 0x%08x, wantReplies=%d, channel=%d", dest, - wantReplies, node->channel); - nodeInfoModule->sendOurNodeInfo(dest, wantReplies, node->channel); - } - return false; + if (getPositionPrecisionForChannel(sendChan) == 0 && !findPositionChannel(sendChan)) { + // No channel with position enabled: fall back to sending nodeinfo, as before. + if (nodeInfoModule) { + LOG_INFO("No position-enabled channel; send nodeinfo instead to 0x%08x, wantReplies=%d, channel=%d", dest, + wantReplies, node->channel); + nodeInfoModule->sendOurNodeInfo(dest, wantReplies, node->channel); } + return false; } LOG_INFO("Send position ping to 0x%08x, wantReplies=%d, channel=%d", dest, wantReplies, sendChan); positionModule->sendOurPosition(dest, wantReplies, sendChan); diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index b45783677e..fdffd0c260 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -1826,8 +1826,11 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p) } #endif - // Reject before recording duplicate or per-port cooldown state, so a blocked - // attempt cannot throttle a valid private-channel position retry. + // Coordinates aimed at the event channel go out on the position channel instead (the phone picks the + // channel it last heard the node on, which is the event channel for everyone). Only when there is no + // channel to move them to is the send rejected. Reject before recording duplicate or per-port cooldown + // state, so a blocked attempt cannot throttle a valid private-channel position retry. + coerceCoordinatePacketToPositionChannel(&p); if (isBlockedEventCoordinatePacket(&p)) { LOG_DEBUG("Suppress phone coordinate send on event (everyone) channel"); meshtastic_QueueStatus qs = router->getQueueStatus(); diff --git a/src/mesh/PositionPrecision.cpp b/src/mesh/PositionPrecision.cpp index d34c660861..df846c01c3 100644 --- a/src/mesh/PositionPrecision.cpp +++ b/src/mesh/PositionPrecision.cpp @@ -32,6 +32,17 @@ uint32_t getPositionPrecisionForChannel(uint8_t channelIndex) return precision; } +bool findPositionChannel(uint8_t &channelIndex) +{ + for (uint8_t i = 0; i < channels.getNumChannels(); i++) { + if (getPositionPrecisionForChannel(i) != 0) { + channelIndex = i; + return true; + } + } + return false; +} + int32_t truncateCoordinate(int32_t coordinate, uint32_t precision) { if (precision == 0 || precision >= 32) diff --git a/src/mesh/PositionPrecision.h b/src/mesh/PositionPrecision.h index 0a2dc8ef07..a3565aa564 100644 --- a/src/mesh/PositionPrecision.h +++ b/src/mesh/PositionPrecision.h @@ -16,6 +16,10 @@ uint32_t getPositionPrecisionForChannel(const meshtastic_Channel &channel); // Configured precision, clamped to MAX_POSITION_PRECISION_PUBLIC_KEY when the channel's effective key is publicly decryptable. uint32_t getPositionPrecisionForChannel(uint8_t channelIndex); +// The channel our position goes out on: the lowest index with a non-zero on-wire precision (disabled and event +// channels never qualify). Returns false when position sharing is off on every channel. +bool findPositionChannel(uint8_t &channelIndex); + // Truncate a single latitude_i/longitude_i to `precision` significant bits, centered in the // resulting grid cell (stable under GPS jitter). precision 0 or >=32 returns the value unchanged. // The return is the coordinate (int32_t); the uint8_t overload only narrows the precision arg. diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 750bba7a70..02e85b62d9 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -16,6 +16,9 @@ #include #include #include +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL +#include "modules/PositionModule.h" +#endif #if HAS_TRAFFIC_MANAGEMENT #endif #if HAS_VARIABLE_HOPS @@ -86,6 +89,11 @@ bool isBlockedEventCoordinatePacket(const meshtastic_MeshPacket *p) if (p->pki_encrypted || willUsePki(p)) { return false; } + // From us, to us: never leaves the device (sendLocal delivers it locally). This is how the phone + // hands a GPS-less node its fix and time, so it shares nothing and must not be blocked. + if (isFromUs(p) && isToUs(p)) { + return false; + } if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { return isCoordinatePortnum(p->decoded.portnum) && channels.isEventChannel(getEffectiveChannelIndex(p)); } @@ -96,6 +104,33 @@ bool isBlockedEventCoordinatePacket(const meshtastic_MeshPacket *p) #endif } +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL +// A remote node's unicast position request to us. Only the reply is generated for these; the packet +// itself is still dropped by the caller. +static bool isEventChannelPositionRequestForUs(const meshtastic_MeshPacket *p) +{ + return p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && + p->decoded.portnum == meshtastic_PortNum_POSITION_APP && p->decoded.want_response && isToUs(p) && !isFromUs(p); +} +#endif + +bool coerceCoordinatePacketToPositionChannel(meshtastic_MeshPacket *p) +{ +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL + if (!isBlockedEventCoordinatePacket(p)) + return false; + uint8_t positionChannel; + if (!findPositionChannel(positionChannel)) + return false; + LOG_DEBUG("Coerce coordinate packet 0x%08x from event channel to position channel %u", p->id, positionChannel); + p->channel = positionChannel; + return true; +#else + (void)p; + return false; +#endif +} + bool willUsePki(const meshtastic_MeshPacket *p) { #if !(MESHTASTIC_EXCLUDE_PKI) @@ -396,6 +431,11 @@ ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) return ERRNO_NO_INTERFACES; } else { + // Coordinates never go out on the event channel: any local originator (phone, module, UI) that aimed + // one there is moved onto the position channel instead. Before the loopback below so the local copy + // carries the channel it will actually be sent on. + coerceCoordinatePacketToPositionChannel(p); + // If we are sending a broadcast, we also treat it as if we just received it ourself // this allows local apps (and PCs) to see broadcasts sourced locally. Only the loopback // handleReceived is deferred when nested; send(p) below still transmits immediately. @@ -1029,14 +1069,16 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) return DecodeState::DECODE_POLICY_REJECT; #endif + if (p->decoded.has_bitfield) + p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK; + if (isBlockedEventCoordinatePacket(p)) { + // want_response is already merged above: a position request on the event channel is still + // answered (on the position channel) even though its coordinates are dropped. LOG_DEBUG("Decoded coordinate packet on event channel; suppress payload logging"); return DecodeState::DECODE_SUCCESS; } - if (p->decoded.has_bitfield) - p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK; - /* Not actually ever used. // Decompress if needed. jm if (p->decoded.portnum == meshtastic_PortNum_TEXT_MESSAGE_COMPRESSED_APP) { @@ -1515,6 +1557,12 @@ void Router::dispatchReceived(meshtastic_MeshPacket *p, RxSource src) // Discard coordinate-bearing packets that arrive on the event ("everyone") // channel: don't process, store in NodeDB, or rebroadcast them. if (!skipHandle && isBlockedEventCoordinatePacket(p)) { + // A position request addressed to us is still answered, on our position channel at that + // channel's precision, so "request position" from a node that only shares the event channel + // with us resolves where positions actually live. The requester's own coordinates are + // still dropped: not stored, not forwarded to the phone, not relayed, not published. + if (isEventChannelPositionRequestForUs(p) && positionModule) + positionModule->replyOnPositionChannel(*p); LOG_DEBUG("Drop coordinate packet on event (everyone) channel"); cancelSending(p->from, p->id); skipHandle = true; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 9882a4b9c8..069b4ede0a 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -18,6 +18,10 @@ inline bool isCoordinatePortnum(meshtastic_PortNum portnum) } bool isBlockedEventCoordinatePacket(const meshtastic_MeshPacket *p); +/// Retarget a locally-originated coordinate packet that would be blocked on the event channel onto the +/// position channel (see findPositionChannel). Returns true if p->channel was changed; false when the +/// packet is not a blocked event coordinate packet or no channel carries positions. +bool coerceCoordinatePacketToPositionChannel(meshtastic_MeshPacket *p); bool willUsePki(const meshtastic_MeshPacket *p); /// rx_time/has_rx_time for "now": a real epoch when the clock is trustworthy, else a diff --git a/src/modules/PositionModule.cpp b/src/modules/PositionModule.cpp index 9ee985b156..40ad53d3af 100644 --- a/src/modules/PositionModule.cpp +++ b/src/modules/PositionModule.cpp @@ -289,6 +289,27 @@ meshtastic_MeshPacket *PositionModule::allocReply() return reply; } +void PositionModule::replyOnPositionChannel(const meshtastic_MeshPacket &req) +{ + uint8_t positionChannel; + if (!findPositionChannel(positionChannel)) { + LOG_DEBUG("Skip position reply to 0x%08x: position sharing disabled on all channels", getFrom(&req)); + return; + } + if (!service) + return; + + precision = getPositionPrecisionForChannel(positionChannel); + meshtastic_MeshPacket *reply = allocReply(); // reply throttle + precision-0/no-fix guards live here + if (!reply) + return; + + setReplyTo(reply, req); + reply->channel = positionChannel; // not the channel the request came in on + LOG_INFO("Reply to position request from 0x%08x on position channel %u", getFrom(&req), positionChannel); + service->sendToMesh(reply); +} + meshtastic_MeshPacket *PositionModule::allocAtakPli() { LOG_INFO("Send TAK V2 PLI packet"); @@ -374,12 +395,11 @@ void PositionModule::sendOurPosition() currentGeneration = radioGeneration; // If we changed channels, ask everyone else for their latest info - 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; - } + uint8_t positionChannel; + if (findPositionChannel(positionChannel)) { + LOG_INFO("Send pos@%x:6 to mesh (wantReplies=%d)", localPosition.timestamp, requestReplies); + sendOurPosition(NODENUM_BROADCAST, requestReplies, positionChannel); + return; } LOG_INFO("Skip pos@%x:6 broadcast; position sharing disabled on all channels", localPosition.timestamp); } @@ -467,12 +487,10 @@ bool PositionModule::positionUnchangedSinceLastSend(const meshtastic_PositionLit // precision). Default nodes gauge movement at that on-wire (public-clamped) resolution; // trackers use their own configured (unclamped) precision so finer moves still count. uint32_t precisionBits = 0; - for (uint8_t ch = 0; ch < 8; ch++) { - if (getPositionPrecisionForChannel(ch) == 0) - continue; + uint8_t ch; + if (findPositionChannel(ch)) { precisionBits = useConfiguredPrecision ? getPositionPrecisionForChannel(channels.getByIndex(ch)) : getPositionPrecisionForChannel(ch); - break; } return positionWithinPrecisionCell(selfPos.latitude_i, selfPos.longitude_i, lastGpsLatitude, lastGpsLongitude, precisionBits); diff --git a/src/modules/PositionModule.h b/src/modules/PositionModule.h index 03754c22b2..c5a3d47add 100644 --- a/src/modules/PositionModule.h +++ b/src/modules/PositionModule.h @@ -36,6 +36,13 @@ class PositionModule : public ProtobufModule, private concu void sendOurPosition(NodeNum dest, bool wantReplies = false, uint8_t channel = 0); void sendOurPosition(); + /** + * Answer a position request that arrived on a channel we never share position on (the event channel): + * the reply goes out on the position channel at that channel's precision, tagged as a reply to req. + * Subject to the same reply throttle as allocReply(). No-op when no channel carries positions. + */ + void replyOnPositionChannel(const meshtastic_MeshPacket &req); + void handleNewPosition(); // Pure broadcast-policy helpers, split out so they're unit-testable without the module. diff --git a/test/test_event_channel_phone_api/test_main.cpp b/test/test_event_channel_phone_api/test_main.cpp index f7932b9c3f..57a65915aa 100644 --- a/test/test_event_channel_phone_api/test_main.cpp +++ b/test/test_event_channel_phone_api/test_main.cpp @@ -1,4 +1,5 @@ #include "Channels.h" +#include "MeshModule.h" #include "MeshService.h" #include "NodeDB.h" #include "RadioInterface.h" @@ -15,10 +16,28 @@ namespace { constexpr PacketId BLOCKED_PACKET_ID = 0x10203040; constexpr PacketId FOLLOWUP_PACKET_ID = 0x50607080; +constexpr PacketId WAYPOINT_PACKET_ID = 0x0a0b0c0d; constexpr ChannelIndex EVENT_CHANNEL = 0; constexpr ChannelIndex PRIVATE_CHANNEL = 1; +constexpr NodeNum LOCAL_NODE = 0x87654321; constexpr NodeNum REMOTE_NODE = 0x12345678; +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL && defined(USERPREFS_CHANNEL_0_PSK) +// Where a coordinate packet the phone aimed at the event channel actually goes once a channel carries positions. +constexpr ChannelIndex COERCED_CHANNEL = PRIVATE_CHANNEL; +#else +constexpr ChannelIndex COERCED_CHANNEL = EVENT_CHANNEL; +#endif + +// Router::sendLocal() loops a to-self packet through MeshModule::callModules(), which walks the module +// list; construct one so the list exists in this otherwise module-free binary. +class NoopModule : public MeshModule +{ + public: + NoopModule() : MeshModule("event-phone-api-noop") {} + bool wantPacket(const meshtastic_MeshPacket *) override { return false; } +}; + class MockRadioInterface : public RadioInterface { public: @@ -106,6 +125,7 @@ MockMeshService *mockService; MockRouter *mockRouter; NodeDB *mockNodeDB; TestStreamAPI *streamAPI; +NoopModule *noopModule; void configureChannels() { @@ -135,20 +155,35 @@ void configureChannels() channels.onConfigChanged(); } -meshtastic_ToRadio makePositionToRadio(PacketId id, ChannelIndex channel) +// configureChannels() leaves both channels without module_settings, i.e. position sharing off everywhere +// (getPositionPrecisionForChannel fails closed). Opt the private channel in so it becomes the position channel. +void enablePositionOnPrivateChannel() +{ + auto &privateChannel = channelFile.channels[PRIVATE_CHANNEL]; + privateChannel.settings.has_module_settings = true; + privateChannel.settings.module_settings.position_precision = 32; + channels.onConfigChanged(); +} + +meshtastic_ToRadio makeCoordinateToRadio(PacketId id, ChannelIndex channel, meshtastic_PortNum portnum, NodeNum to) { meshtastic_ToRadio message = meshtastic_ToRadio_init_default; const meshtastic_MeshPacket defaultPacket = meshtastic_MeshPacket_init_default; message.which_payload_variant = meshtastic_ToRadio_packet_tag; message.packet = defaultPacket; - message.packet.to = REMOTE_NODE; + message.packet.to = to; message.packet.id = id; message.packet.channel = channel; message.packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; - message.packet.decoded.portnum = meshtastic_PortNum_POSITION_APP; + message.packet.decoded.portnum = portnum; return message; } +meshtastic_ToRadio makePositionToRadio(PacketId id, ChannelIndex channel) +{ + return makeCoordinateToRadio(id, channel, meshtastic_PortNum_POSITION_APP, REMOTE_NODE); +} + bool sendToRadio(const meshtastic_ToRadio &message) { uint8_t encoded[meshtastic_ToRadio_size] = {}; @@ -160,13 +195,14 @@ bool sendToRadio(const meshtastic_ToRadio &message) return streamAPI->handleToRadio(encoded, encodedSize); } -void assertSentPacket(size_t index, PacketId id, ChannelIndex channel) +void assertSentPacket(size_t index, PacketId id, ChannelIndex channel, + meshtastic_PortNum portnum = meshtastic_PortNum_POSITION_APP) { TEST_ASSERT_GREATER_THAN(index, mockRouter->sentPackets.size()); const auto &packet = mockRouter->sentPackets[index]; TEST_ASSERT_EQUAL_UINT32(id, packet.id); TEST_ASSERT_EQUAL_UINT8(channel, packet.channel); - TEST_ASSERT_EQUAL(meshtastic_PortNum_POSITION_APP, packet.decoded.portnum); + TEST_ASSERT_EQUAL(portnum, packet.decoded.portnum); } } // namespace @@ -177,16 +213,19 @@ void setUp(void) service = mockService = new MockMeshService(); nodeDB = mockNodeDB = new NodeDB(); - myNodeInfo.my_node_num = 0x87654321; + myNodeInfo.my_node_num = LOCAL_NODE; configureChannels(); cryptLock = nullptr; // Router's ctor asserts this is unset before allocating its own. router = mockRouter = new MockRouter(); streamAPI = new TestStreamAPI(); + noopModule = new NoopModule(); testDelay(1); } void tearDown(void) { + delete noopModule; + noopModule = nullptr; delete streamAPI; streamAPI = nullptr; delete mockRouter; @@ -250,12 +289,62 @@ static void test_event_position_ingress_does_not_poison_retry_state() #endif } +// The apps feed the node its phone GPS fix as a POSITION packet addressed to the node itself on channel 0. +// That packet never leaves the device, so it must pass regardless of the event policy and without a +// notification, on any channel configuration (here: no channel carries positions at all). +static void test_phone_position_to_self_is_never_blocked() +{ + const auto toSelf = makeCoordinateToRadio(BLOCKED_PACKET_ID, EVENT_CHANNEL, meshtastic_PortNum_POSITION_APP, LOCAL_NODE); + + TEST_ASSERT_TRUE(sendToRadio(toSelf)); + TEST_ASSERT_EQUAL(0, mockRouter->sentPackets.size()); // delivered locally, never on the air + mockService->assertQueueStatus(BLOCKED_PACKET_ID); + TEST_ASSERT_EQUAL(0, mockService->notifications.size()); +} + +// A coordinate the phone aims at the event channel is moved onto the position channel (the first channel +// with position sharing enabled) instead of being rejected, and the phone is not told anything went wrong. +// Without the event policy the packet stays on the channel the phone chose. +static void test_phone_coordinates_on_event_channel_move_to_position_channel() +{ + enablePositionOnPrivateChannel(); + const auto positionRequest = makePositionToRadio(BLOCKED_PACKET_ID, EVENT_CHANNEL); // DM (e.g. "request position") + const auto waypointBroadcast = + makeCoordinateToRadio(WAYPOINT_PACKET_ID, EVENT_CHANNEL, meshtastic_PortNum_WAYPOINT_APP, NODENUM_BROADCAST); + + TEST_ASSERT_TRUE(sendToRadio(positionRequest)); + TEST_ASSERT_EQUAL(1, mockRouter->sentPackets.size()); + assertSentPacket(0, BLOCKED_PACKET_ID, COERCED_CHANNEL); + mockService->assertQueueStatus(BLOCKED_PACKET_ID); + TEST_ASSERT_EQUAL(0, mockService->notifications.size()); + + TEST_ASSERT_TRUE(sendToRadio(waypointBroadcast)); + TEST_ASSERT_EQUAL(2, mockRouter->sentPackets.size()); + assertSentPacket(1, WAYPOINT_PACKET_ID, COERCED_CHANNEL, meshtastic_PortNum_WAYPOINT_APP); + mockService->assertQueueStatus(WAYPOINT_PACKET_ID); + TEST_ASSERT_EQUAL(0, mockService->notifications.size()); +} + +// A coordinate already on the position channel is left alone. +static void test_phone_coordinates_on_position_channel_are_untouched() +{ + enablePositionOnPrivateChannel(); + + TEST_ASSERT_TRUE(sendToRadio(makePositionToRadio(FOLLOWUP_PACKET_ID, PRIVATE_CHANNEL))); + TEST_ASSERT_EQUAL(1, mockRouter->sentPackets.size()); + assertSentPacket(0, FOLLOWUP_PACKET_ID, PRIVATE_CHANNEL); + TEST_ASSERT_EQUAL(0, mockService->notifications.size()); +} + extern "C" { void setup() { initializeTestEnvironment(); UNITY_BEGIN(); RUN_TEST(test_event_position_ingress_does_not_poison_retry_state); + RUN_TEST(test_phone_position_to_self_is_never_blocked); + RUN_TEST(test_phone_coordinates_on_event_channel_move_to_position_channel); + RUN_TEST(test_phone_coordinates_on_position_channel_are_untouched); exit(UNITY_END()); } diff --git a/test/test_event_channel_router/test_main.cpp b/test/test_event_channel_router/test_main.cpp index 82700a795f..479a0275c3 100644 --- a/test/test_event_channel_router/test_main.cpp +++ b/test/test_event_channel_router/test_main.cpp @@ -9,7 +9,11 @@ #include "mesh/MeshRadio.h" #include "mesh/MeshService.h" #include "mesh/NodeDB.h" +#include "mesh/PositionPrecision.h" #include "mesh/Router.h" +#include "modules/PositionModule.h" +#include "modules/RoutingModule.h" +#include "support/MockMeshService.h" #include #include #include @@ -277,6 +281,151 @@ static void test_opaque_tx_is_not_misclassified_as_coordinates() TEST_ASSERT_EQUAL_UINT32(1, captureRadio->packets.size()); } +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL +static void enablePositionOnPrivateChannel() +{ + meshtastic_Channel &privateChannel = channelFile.channels[kPrivateChannel]; + privateChannel.settings.has_module_settings = true; + privateChannel.settings.module_settings.position_precision = 32; + channels.onConfigChanged(); + uint8_t positionChannel = 0xff; + TEST_ASSERT_TRUE(findPositionChannel(positionChannel)); + TEST_ASSERT_EQUAL_UINT8(kPrivateChannel, positionChannel); +} + +// The reply path needs the module, a service to send through, a routing module for the response hop +// limit, and a fix of our own. Scoped to the one test so the rest of the suite stays module-free. +struct ReplyHarness { + MeshService *savedService = service; + RoutingModule *savedRouting = routingModule; + PositionModule *savedPosition = positionModule; + MockMeshService localService; + RoutingModule localRouting; + PositionModule localPosition; + + ReplyHarness() + { + service = &localService; + routingModule = &localRouting; + positionModule = &localPosition; + testNodeDB->addNode(kLocalNode, kEventChannel); // refreshLocalMeshNode() asserts our own entry exists + meshtastic_Position fix = meshtastic_Position_init_zero; + fix.has_latitude_i = true; + fix.latitude_i = 407825770; + fix.has_longitude_i = true; + fix.longitude_i = -1192084390; + testNodeDB->setLocalPosition(fix); + } + + ~ReplyHarness() + { + // Drain what sendToMesh() queued for the (absent) phone so the pools are clean at exit. + while (auto *status = localService.getQueueStatusForPhone()) + localService.releaseQueueStatusToPool(status); + while (auto *packet = localService.getForPhone()) + localService.releaseToPool(packet); + positionModule = savedPosition; + routingModule = savedRouting; + service = savedService; + } +}; + +// A position request DM'd to us on the event channel is not processed (no module sees it, so nothing is +// stored or forwarded), but it is answered: our position goes out as a reply, on the position channel. +static void test_rx_event_channel_position_request_to_us_is_answered_on_position_channel() +{ + enablePositionOnPrivateChannel(); + ReplyHarness harness; + + meshtastic_MeshPacket request = makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kRemoteNode, kLocalNode, kEventChannel); + request.decoded.want_response = true; + receivePacket(request); + + TEST_ASSERT_EQUAL_UINT32(0, captureModule->packets.size()); + TEST_ASSERT_EQUAL_UINT32(1, captureRadio->packets.size()); + + meshtastic_MeshPacket reply = captureRadio->packets.front(); + TEST_ASSERT_EQUAL_UINT32(kRemoteNode, reply.to); + TEST_ASSERT_EQUAL_UINT32(kLocalNode, reply.from); + TEST_ASSERT_EQUAL(meshtastic_MeshPacket_encrypted_tag, reply.which_payload_variant); // went out under a channel key + TEST_ASSERT_EQUAL(DecodeState::DECODE_SUCCESS, perhapsDecode(&reply)); + TEST_ASSERT_EQUAL_UINT8(kPrivateChannel, reply.channel); // ...the position channel's, not the event channel's + TEST_ASSERT_EQUAL(meshtastic_PortNum_POSITION_APP, reply.decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(request.id, reply.decoded.request_id); +} + +// Without a position channel there is nothing to answer on: the request is simply dropped. +static void test_rx_event_channel_position_request_without_position_channel_is_dropped() +{ + ReplyHarness harness; + + meshtastic_MeshPacket request = makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kRemoteNode, kLocalNode, kEventChannel); + request.decoded.want_response = true; + receivePacket(request); + + TEST_ASSERT_EQUAL_UINT32(0, captureModule->packets.size()); + TEST_ASSERT_EQUAL_UINT32(0, captureRadio->packets.size()); +} + +// A broadcast position on the event channel is dropped outright, want_response or not: only unicast +// requests to us are answered. +static void test_rx_event_channel_position_broadcast_with_want_response_is_not_answered() +{ + enablePositionOnPrivateChannel(); + ReplyHarness harness; + + meshtastic_MeshPacket broadcast = + makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kRemoteNode, NODENUM_BROADCAST, kEventChannel); + broadcast.decoded.want_response = true; + receivePacket(broadcast); + + TEST_ASSERT_EQUAL_UINT32(0, captureModule->packets.size()); + TEST_ASSERT_EQUAL_UINT32(0, captureRadio->packets.size()); +} + +// The phone hands a GPS-less node its fix as a POSITION packet from us to us on channel 0. It never goes on +// the air, so the event policy must let it through to the modules (where PositionModule records it). +static void test_loopback_position_from_us_to_us_on_event_channel_is_not_blocked() +{ + meshtastic_MeshPacket loopback = makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kLocalNode, kLocalNode, kEventChannel); + TEST_ASSERT_FALSE(isBlockedEventCoordinatePacket(&loopback)); + + meshtastic_MeshPacket *packet = packetPool.allocCopy(loopback); + TEST_ASSERT_NOT_NULL(packet); + TEST_ASSERT_EQUAL_INT(ERRNO_SHOULD_RELEASE, testRouter->sendLocal(packet, RX_SRC_USER)); + packetPool.release(packet); + + TEST_ASSERT_EQUAL_UINT32(1, captureModule->packets.size()); + TEST_ASSERT_EQUAL_UINT32(0, captureRadio->packets.size()); +} + +// A local originator (module, UI) that aims a coordinate at the event channel is moved onto the position +// channel by sendLocal(); with no position channel the send is still refused. +static void test_tx_local_coordinate_on_event_channel_is_moved_to_position_channel() +{ + meshtastic_MeshPacket *packet = testRouter->allocForSending(); + TEST_ASSERT_NOT_NULL(packet); + packet->to = NODENUM_BROADCAST; + packet->channel = kEventChannel; + packet->decoded = makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kLocalNode, NODENUM_BROADCAST, kEventChannel).decoded; + TEST_ASSERT_EQUAL_INT(meshtastic_Routing_Error_NOT_AUTHORIZED, testRouter->sendLocal(packet, RX_SRC_LOCAL)); + TEST_ASSERT_EQUAL_UINT32(0, captureRadio->packets.size()); + + enablePositionOnPrivateChannel(); + packet = testRouter->allocForSending(); + TEST_ASSERT_NOT_NULL(packet); + packet->to = NODENUM_BROADCAST; + packet->channel = kEventChannel; + packet->decoded = makeDecodedPacket(meshtastic_PortNum_POSITION_APP, kLocalNode, NODENUM_BROADCAST, kEventChannel).decoded; + TEST_ASSERT_EQUAL_INT(ERRNO_OK, testRouter->sendLocal(packet, RX_SRC_LOCAL)); + TEST_ASSERT_EQUAL_UINT32(1, captureRadio->packets.size()); + + meshtastic_MeshPacket sent = captureRadio->packets.front(); + TEST_ASSERT_EQUAL(DecodeState::DECODE_SUCCESS, perhapsDecode(&sent)); + TEST_ASSERT_EQUAL_UINT8(kPrivateChannel, sent.channel); +} +#endif + static void test_capture_endpoints_release_packet_pool_ownership() { constexpr size_t iterations = 64; @@ -382,6 +531,13 @@ EVENT_ROUTER_TEST_ENTRY void setup() RUN_TEST(test_tx_event_coordinate_that_uses_pki_reaches_radio); #endif RUN_TEST(test_opaque_tx_is_not_misclassified_as_coordinates); +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL + RUN_TEST(test_rx_event_channel_position_request_to_us_is_answered_on_position_channel); + RUN_TEST(test_rx_event_channel_position_request_without_position_channel_is_dropped); + RUN_TEST(test_rx_event_channel_position_broadcast_with_want_response_is_not_answered); + RUN_TEST(test_loopback_position_from_us_to_us_on_event_channel_is_not_blocked); + RUN_TEST(test_tx_local_coordinate_on_event_channel_is_moved_to_position_channel); +#endif RUN_TEST(test_capture_endpoints_release_packet_pool_ownership); exit(UNITY_END()); diff --git a/test/test_position_precision/test_main.cpp b/test/test_position_precision/test_main.cpp index 7497b42f65..034314b1e3 100644 --- a/test/test_position_precision/test_main.cpp +++ b/test/test_position_precision/test_main.cpp @@ -304,6 +304,9 @@ static meshtastic_MeshPacket makeDecodedPacket(meshtastic_PortNum portnum, uint8 packet.which_payload_variant = meshtastic_MeshPacket_decoded_tag; packet.decoded.portnum = portnum; packet.channel = channelIndex; + // A real destination: this suite never sets a node number, so a default to=0 would read as + // "to us" (getNodeNum()==0) and take the from-us-to-us loopback exemption. + packet.to = NODENUM_BROADCAST; return packet; } @@ -373,7 +376,12 @@ static void test_eventCoordinatePolicy_usesResolvedUnicastChannel() configureEventChannels(false, false); meshtastic_NodeInfoLite *node = nodeDB->getNumMeshNodes() > 1 ? nodeDB->getMeshNodeByIndex(1) : nodeDB->getOrCreateMeshNode(0x12345678); + // A persisted DB (unsandboxed host run) can hand back our own entry here; a from-us-to-us packet is + // loopback-exempt, which is not the policy under test. Insist on a remote destination. + if (node && node->num == nodeDB->getNodeNum()) + node = nodeDB->getOrCreateMeshNode(0x12345678); TEST_ASSERT_NOT_NULL(node); + TEST_ASSERT_NOT_EQUAL(nodeDB->getNodeNum(), node->num); const NodeNum destination = node->num; const uint8_t savedChannel = node->channel; @@ -396,6 +404,38 @@ static void test_eventCoordinatePolicy_usesResolvedUnicastChannel() #endif } +static void test_findPositionChannel_skipsEventAndDisabledChannels() +{ + // Both channels store precision 16. Under the block gate the event channel never carries + // positions, so the private one (index 1) is the position channel; otherwise index 0 wins. + configureEventChannels(false, false); + uint8_t positionChannel = 0xff; + TEST_ASSERT_TRUE(findPositionChannel(positionChannel)); +#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL && defined(USERPREFS_CHANNEL_0_PSK) + TEST_ASSERT_EQUAL_UINT8(1, positionChannel); +#else + TEST_ASSERT_EQUAL_UINT8(0, positionChannel); +#endif + + // Reordering follows the effective key, not the index. + configureEventChannels(true, false); + TEST_ASSERT_TRUE(findPositionChannel(positionChannel)); + TEST_ASSERT_EQUAL_UINT8(0, positionChannel); + + // Precision 0 everywhere: nothing to pick. + configureEventChannels(false, false); + channelFile.channels[0].settings.module_settings.position_precision = 0; + channelFile.channels[1].settings.module_settings.position_precision = 0; + channels.onConfigChanged(); + TEST_ASSERT_FALSE(findPositionChannel(positionChannel)); + + // A disabled channel does not count even with a stored precision. + channelFile.channels[1].settings.module_settings.position_precision = 32; + channelFile.channels[1].role = meshtastic_Channel_Role_DISABLED; + channels.onConfigChanged(); + TEST_ASSERT_FALSE(findPositionChannel(positionChannel)); +} + static void test_getPositionPrecisionForChannel_nonEventFullKeyIsHonored() { // A private channel with a full 32-byte key that is not the configured @@ -439,6 +479,7 @@ void setup() RUN_TEST(test_eventCoordinatePolicy_coversPortsAndExcludesPki); RUN_TEST(test_eventCoordinatePolicy_doesNotClassifyOpaquePacketsByHash); RUN_TEST(test_eventCoordinatePolicy_usesResolvedUnicastChannel); + RUN_TEST(test_findPositionChannel_skipsEventAndDisabledChannels); RUN_TEST(test_getPositionPrecisionForChannel_nonEventFullKeyIsHonored); exit(UNITY_END()); }