diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 591245db9..0667e0b13 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -114,6 +114,14 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp) } } + // Our own packet heard back off the mesh, which the duplicate cache only suppresses best-effort. + // Clients can't tell an echo from genuine ingress, so it surfaces as an incoming message. Packets + // addressed to us are locally-generated feedback (implicit ACK, NAK, routing error), not an echo. + if (isFromUs(mp) && !isToUs(mp)) { + LOG_DEBUG("Skip phone echo of our own packet 0x%08x", mp->id); + return 0; + } + printPacket("Forwarding to phone", mp); if (auto *toPhone = packetPool.allocCopy(*mp)) sendToPhone(toPhone); diff --git a/src/mesh/MeshService.h b/src/mesh/MeshService.h index 8ddc6e434..7adcdb7c6 100644 --- a/src/mesh/MeshService.h +++ b/src/mesh/MeshService.h @@ -222,6 +222,9 @@ class MeshService /// needs to keep the packet around it makes a copy int handleFromRadio(const meshtastic_MeshPacket *p); friend class RoutingModule; +#ifdef PIO_UNIT_TESTING + friend class MeshServicePhoneDeliveryTest; +#endif }; extern MeshService *service; diff --git a/test/test_mesh_module/test_main.cpp b/test/test_mesh_module/test_main.cpp index a39988064..9cc0d1811 100644 --- a/test/test_mesh_module/test_main.cpp +++ b/test/test_mesh_module/test_main.cpp @@ -265,6 +265,7 @@ static MockMeshService *mockService; static MockRouter *mockRouter; static MockRoutingModule *mockRoutingModule; static NeighborInfoModule *realNeighborInfoModule; +static RoutingModule *realRoutingModule; static std::vector dispatchModules; template static T *registerDispatchModule(T *module) @@ -273,6 +274,14 @@ template static T *registerDispatchModule(T *module) return module; } +// Swap the mocked RoutingModule for a real one. tearDown() owns the cleanup because a failed +// assertion longjmps out of the test, which would otherwise leave it registered in MeshModule::modules. +static void installRealRoutingModule() +{ + realRoutingModule = new RoutingModule(); + routingModule = realRoutingModule; +} + static meshtastic_MeshPacket makeRequest(meshtastic_PortNum port) { meshtastic_MeshPacket packet = meshtastic_MeshPacket_init_zero; @@ -335,6 +344,7 @@ void setUp(void) mockRoutingModule = new MockRoutingModule(); routingModule = mockRoutingModule; + realRoutingModule = nullptr; testModule = new TestModule(); memset(&testPacket, 0, sizeof(testPacket)); @@ -355,6 +365,9 @@ void tearDown(void) delete testModule; testModule = nullptr; + delete realRoutingModule; + realRoutingModule = nullptr; + delete mockRoutingModule; mockRoutingModule = nullptr; routingModule = nullptr; @@ -606,6 +619,108 @@ static void test_localReplyToSelf_isDeliveredToPhone() TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); // nothing went toward the radio } +// handleFromRadio() is private to MeshService, which befriends RoutingModule and, under +// PIO_UNIT_TESTING, this seam. +class MeshServicePhoneDeliveryTest +{ + public: + static void deliver(const meshtastic_MeshPacket &p) { service->handleFromRadio(&p); } +}; + +static void test_handleFromRadio_remotePacketReachesPhone() +{ + meshtastic_MeshPacket rx = meshtastic_MeshPacket_init_zero; + rx.from = REMOTE_NODE; + rx.to = NODENUM_BROADCAST; + rx.id = 0x0BADF00D; + rx.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + rx.decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; + + MeshServicePhoneDeliveryTest::deliver(rx); + + meshtastic_MeshPacket *toPhone = mockService->getForPhone(); + TEST_ASSERT_NOT_NULL(toPhone); + TEST_ASSERT_EQUAL_UINT32(0x0BADF00D, toPhone->id); + mockService->releaseToPool(toPhone); + TEST_ASSERT_NULL(mockService->getForPhone()); +} + +// A packet we originated, coming back around, must not be echoed to the client that sent it. +static void test_handleFromRadio_ownPacketIsNotEchoedToPhone() +{ + meshtastic_MeshPacket ours = meshtastic_MeshPacket_init_zero; + ours.from = LOCAL_NODE; + ours.to = NODENUM_BROADCAST; + ours.id = 0x5E1F0001; + ours.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + ours.decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; + + MeshServicePhoneDeliveryTest::deliver(ours); + TEST_ASSERT_NULL(mockService->getForPhone()); + + // Same for the from==0 spelling handleToRadio stamps on phone-originated packets. + ours.from = 0; + ours.id = 0x5E1F0002; + MeshServicePhoneDeliveryTest::deliver(ours); + TEST_ASSERT_NULL(mockService->getForPhone()); +} + +// A packet from us *addressed to us* is locally-generated feedback, not an echo, and must still be +// delivered - suppressing it would silently drop every ACK/NAK the client relies on. +static void test_handleFromRadio_ownPacketAddressedToUsReachesPhone() +{ + meshtastic_MeshPacket ack = meshtastic_MeshPacket_init_zero; + ack.from = LOCAL_NODE; + ack.to = LOCAL_NODE; + ack.id = 0x5E1F0003; + ack.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + ack.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + ack.decoded.request_id = 0x0C0FFEE0; + + MeshServicePhoneDeliveryTest::deliver(ack); + + meshtastic_MeshPacket *toPhone = mockService->getForPhone(); + TEST_ASSERT_NOT_NULL(toPhone); + TEST_ASSERT_EQUAL_UINT32(0x0C0FFEE0, toPhone->decoded.request_id); + mockService->releaseToPool(toPhone); + TEST_ASSERT_NULL(mockService->getForPhone()); +} + +// sendAckNak stamps from == our nodenum and to == us, and sendLocal defaults to RX_SRC_RADIO, so the +// loopback gate never applies and only handleFromRadio's filter gates the implicit ACK / NAK path. +static void test_localAckNak_reachesPhoneViaRealRoutingModule() +{ + installRealRoutingModule(); + + realRoutingModule->sendAckNak(meshtastic_Routing_Error_NONE, LOCAL_NODE, 0xFEEDBEEF, 0); + + meshtastic_MeshPacket *toPhone = mockService->getForPhone(); + TEST_ASSERT_NOT_NULL(toPhone); + TEST_ASSERT_EQUAL(meshtastic_PortNum_ROUTING_APP, toPhone->decoded.portnum); + TEST_ASSERT_EQUAL_UINT32(0xFEEDBEEF, toPhone->decoded.request_id); + TEST_ASSERT_EQUAL_UINT32(LOCAL_NODE, toPhone->to); + TEST_ASSERT_EQUAL_UINT32(LOCAL_NODE, toPhone->from); + mockService->releaseToPool(toPhone); +} + +// The mirror of the above: a broadcast we originated, heard back off the mesh, must not reach the +// phone even though it travels the same RoutingModule path. +static void test_ownBroadcastEcho_isDroppedByRealRoutingModule() +{ + installRealRoutingModule(); + + meshtastic_MeshPacket echo = meshtastic_MeshPacket_init_zero; + echo.from = LOCAL_NODE; + echo.to = NODENUM_BROADCAST; + echo.id = 0x5E1F0004; + echo.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + echo.decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; + + MeshModule::callModules(echo, RX_SRC_RADIO); + + TEST_ASSERT_NULL(mockService->getForPhone()); +} + // Full loop: a phone-originated want_response request (from == 0, RX_SRC_USER) dispatched // through the real router must produce a module reply that reaches the phone queue. static void test_phoneRequest_replyReachesPhone() @@ -736,6 +851,11 @@ void setup() RUN_TEST(test_dispatch_ignoreRequestIsClearedPerPacket); RUN_TEST(test_dispatch_realNeighborInfoCannotShadowTelemetryOwner); RUN_TEST(test_localReplyToSelf_isDeliveredToPhone); + RUN_TEST(test_handleFromRadio_remotePacketReachesPhone); + RUN_TEST(test_handleFromRadio_ownPacketIsNotEchoedToPhone); + RUN_TEST(test_handleFromRadio_ownPacketAddressedToUsReachesPhone); + RUN_TEST(test_localAckNak_reachesPhoneViaRealRoutingModule); + RUN_TEST(test_ownBroadcastEcho_isDroppedByRealRoutingModule); RUN_TEST(test_phoneRequest_replyReachesPhone); RUN_TEST(test_nestedLocalSend_isDeferred_notReentrant); RUN_TEST(test_deferredChain_drainsBreadthFirst);