kevin/meshtastic_firmware
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

Deliver locally-generated replies addressed to us to the phone (#11185)

Browse Source 
* Deliver locally-generated replies addressed to us to the phone

Config get/set from the phone times out on every device: the client sends an
admin request, the node handles it, and the response is silently dropped
before it reaches the phone queue.

#10967 changed Router::sendLocal's isToUs branch from enqueueReceivedMessage()
to handleReceived(p, src), so a local packet keeps its RxSource instead of
being relabeled RX_SRC_RADIO by the queue round-trip. That is the right call
for the new policy gates, but module replies go out through
MeshService::sendToMesh() with the default RX_SRC_LOCAL, and a reply to a
phone-originated request is addressed to our own node (setReplyTo resolves
from == 0 to ourNodeNum). Those replies now re-enter callModules as
RX_SRC_LOCAL, where the loopback gate skips every module whose loopbackOk is
false - including RoutingModule, whose promiscuous sniff is the only path that
moves a received packet into toPhoneQueue. The reply is released, never sent.

Requests still work, because the phone's own packets arrive as RX_SRC_USER and
pass the gate, so a set_config is applied and only its acknowledgement is lost.
That is why a client can connect and download config but times out on every
config screen and every setter.

Deliver the phone's copy from sendToMesh() instead: for a local packet
addressed to us, the loopback gate is doing its job in keeping the packet away
from module re-dispatch, and the phone copy is exactly what is missing. Setting
loopbackOk on RoutingModule would instead echo every locally-generated
broadcast back to the phone, and relabeling replies RX_SRC_RADIO would undo the
origin separation #10967 added.

Also stop reporting ERRNO_SHOULD_RELEASE (35) to the phone in the QueueStatus
for these packets. It means "caller frees", not a send failure, and the same
hunk changed it from the 0 the phone used to see.

* Address review: trim comments, assert the QueueStatus count

Condense the added comments to the one-or-two-line house style; the rationale
lives in the commit message and PR.

The reply test drained QueueStatus records in a while loop, which would have
passed just as happily on an empty queue. Count them and require both the
request's and the reply's.
This commit is contained in:
Ben Meadors
2026-07-24 09:02:51 +00:00
committed by GitHub
co-authored by GitHub
parent 4c17dda3e6
commit 01d1873bfc
2 changed files with 80 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/test_mesh_module/test_main.cpp
+72
View File
@@ -256,6 +256,8 @@ void tearDown(void)
while (auto *status = mockService->getQueueStatusForPhone())
mockService->releaseQueueStatusToPool(status);
while (auto *toPhone = mockService->getForPhone())
mockService->releaseToPool(toPhone);
delete mockService;
mockService = nullptr;
service = nullptr;
@@ -471,6 +473,74 @@ static void test_dispatch_realNeighborInfoCannotShadowTelemetryOwner()
TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
}
// A reply addressed to ourselves reaches the phone queue, with SHOULD_RELEASE reported as success.
static void test_localReplyToSelf_isDeliveredToPhone()
{
meshtastic_MeshPacket reply = meshtastic_MeshPacket_init_zero;
reply.from = LOCAL_NODE;
reply.to = LOCAL_NODE;
reply.id = 0xABCD1234;
reply.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
reply.decoded.portnum = meshtastic_PortNum_ADMIN_APP;
reply.decoded.request_id = 0x12345678;
service->sendToMesh(packetPool.allocCopy(reply)); // default src == RX_SRC_LOCAL, as callModules sends replies
meshtastic_MeshPacket *toPhone = mockService->getForPhone();
TEST_ASSERT_NOT_NULL(toPhone);
TEST_ASSERT_EQUAL_UINT32(0xABCD1234, toPhone->id);
TEST_ASSERT_EQUAL_UINT32(0x12345678, toPhone->decoded.request_id);
mockService->releaseToPool(toPhone);
TEST_ASSERT_NULL(mockService->getForPhone()); // exactly one delivery
meshtastic_QueueStatus *qs = mockService->getQueueStatusForPhone();
TEST_ASSERT_NOT_NULL(qs);
TEST_ASSERT_EQUAL(ERRNO_OK, qs->res);
mockService->releaseQueueStatusToPool(qs);
TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); // nothing went toward the radio
}
// 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()
{
auto *replyOwner = registerDispatchModule(
new SyntheticReplyModule("private-owner", meshtastic_PortNum_PRIVATE_APP, meshtastic_PortNum_PRIVATE_APP));
meshtastic_MeshPacket request = meshtastic_MeshPacket_init_zero;
request.from = 0; // phone-originated, as MeshService::handleToRadio stamps it
request.to = LOCAL_NODE;
request.id = 0x5EED0001;
request.which_payload_variant = meshtastic_MeshPacket_decoded_tag;
request.decoded.portnum = meshtastic_PortNum_PRIVATE_APP;
request.decoded.want_response = true;
service->sendToMesh(packetPool.allocCopy(request), RX_SRC_USER);
TEST_ASSERT_EQUAL_UINT32(1, replyOwner->allocReplyCalls);
meshtastic_MeshPacket *toPhone = mockService->getForPhone();
TEST_ASSERT_NOT_NULL(toPhone);
TEST_ASSERT_EQUAL(meshtastic_PortNum_PRIVATE_APP, toPhone->decoded.portnum);
TEST_ASSERT_EQUAL_UINT32(0x5EED0001, toPhone->decoded.request_id);
TEST_ASSERT_EQUAL_UINT32(LOCAL_NODE, toPhone->to);
mockService->releaseToPool(toPhone);
TEST_ASSERT_NULL(mockService->getForPhone()); // the request itself must not echo back
// One QueueStatus for the request, one for the reply, both reporting success
uint32_t statuses = 0;
while (auto *qs = mockService->getQueueStatusForPhone()) {
TEST_ASSERT_EQUAL(ERRNO_OK, qs->res);
mockService->releaseQueueStatusToPool(qs);
statuses++;
}
TEST_ASSERT_EQUAL_UINT32(2, statuses);
TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size());
TEST_ASSERT_EQUAL_UINT32(0, mockRoutingModule->ackNaks.size());
}
void setup()
{
initializeTestEnvironment();
@@ -495,6 +565,8 @@ void setup()
RUN_TEST(test_dispatch_noResponderSendsNak);
RUN_TEST(test_dispatch_ignoreRequestIsClearedPerPacket);
RUN_TEST(test_dispatch_realNeighborInfoCannotShadowTelemetryOwner);
RUN_TEST(test_localReplyToSelf_isDeliveredToPhone);
RUN_TEST(test_phoneRequest_replyReachesPhone);
exit(UNITY_END());
}