From c20e57385a6b9912663553040153732fe31b5703 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Thu, 7 May 2026 16:51:49 -0700 Subject: [PATCH] Fix direct retry diagnostics --- docs/cli_commands.md | 10 ++- examples/simple_repeater/MyMesh.cpp | 133 ++++++++++++++++++++-------- src/Mesh.cpp | 80 +++++++++++++---- src/Mesh.h | 1 + 4 files changed, 162 insertions(+), 62 deletions(-) diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 78f8626d..12f49de2 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -111,10 +111,12 @@ - `page`: 1-based page number **Notes:** -- `set` is rejected when the prefix already exists in neighbors. -- Rows are shown newest-first. -- Serial CLI prints all rows (no paging). -- Over LoRa remote CLI, page size is fixed at `4` rows; choose page with `get recent.repeater `. +- `set` stores or updates the prefix in the recent repeater table. +- Rows are sorted by prefix width (3-byte, 2-byte, 1-byte), then SNR descending. +- A full direct retry failure lowers the stored SNR by `0.25 dB`. +- If a full failure has no row yet, it first seeds the row at the active retry cutoff + `2.5 dB`, then applies the `0.25 dB` penalty. +- Serial CLI page size is fixed at `128` rows; choose page with `get recent.repeater `. +- Over LoRa remote CLI, page size is fixed at `7` rows; choose page with `get recent.repeater `. --- diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 10be2212..34b9bb57 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -1,5 +1,6 @@ #include "MyMesh.h" #include +#include /* ------------------------------ Config -------------------------------- */ @@ -124,6 +125,45 @@ static void formatRecentRepeaterSnrX4(int8_t snr_x4, char* out, size_t out_len) } } +static int buildSortedRecentRepeaterView(SimpleMeshTables* tables, + const SimpleMeshTables::RecentRepeaterInfo** out, + int out_cap) { + if (tables == NULL || out == NULL || out_cap <= 0) { + return 0; + } + + int total = tables->getRecentRepeaterCount(); + if (total > out_cap) { + total = out_cap; + } + + int count = 0; + for (int i = 0; i < total; i++) { + const auto* info = tables->getRecentRepeaterNewestByIdx(i); + if (info != NULL) { + out[count++] = info; + } + } + + std::stable_sort(out, out + count, [](const SimpleMeshTables::RecentRepeaterInfo* a, + const SimpleMeshTables::RecentRepeaterInfo* b) { + uint8_t a_len = a->prefix_len; + uint8_t b_len = b->prefix_len; + if (a_len > MAX_ROUTE_HASH_BYTES) a_len = MAX_ROUTE_HASH_BYTES; + if (b_len > MAX_ROUTE_HASH_BYTES) b_len = MAX_ROUTE_HASH_BYTES; + + if (a_len != b_len) { + return a_len > b_len; + } + if (a->snr_x4 != b->snr_x4) { + return a->snr_x4 > b->snr_x4; + } + return false; + }); + + return count; +} + static uint8_t decodeTraceHashSize(uint8_t flags, uint8_t route_bytes) { uint8_t code = flags & 0x03; uint8_t size_pow2 = (uint8_t)(1U << code); // legacy TRACE interpretation @@ -605,6 +645,9 @@ void MyMesh::onDirectRetryEvent(const char* event, const mesh::Packet* packet, u if (packet == NULL) { return; } + if (strcmp(event, "failure") == 0) { + return; + } uint8_t prefix[MAX_ROUTE_HASH_BYTES] = {0}; uint8_t prefix_len = 0; @@ -1561,15 +1604,58 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply } } } else { - int total = tables->getRecentRepeaterCount(); + const long page_size = sender_timestamp == 0 ? 128 : 7; + long page_num = 1; + const char* arg = sub; + + if (strncmp(arg, "page ", 5) == 0) { + arg += 5; + while (*arg == ' ') arg++; + } + + if (*arg != 0) { + char* end_ptr = NULL; + page_num = strtol(arg, &end_ptr, 10); + while (end_ptr != NULL && *end_ptr == ' ') end_ptr++; + if (end_ptr == NULL || page_num <= 0 || (end_ptr != NULL && *end_ptr != 0)) { + strcpy(reply, "Err - usage: get recent.repeater [page]"); + return; + } + } + + size_t sorted_size = sizeof(SimpleMeshTables::RecentRepeaterInfo*) * MAX_RECENT_REPEATERS; + const SimpleMeshTables::RecentRepeaterInfo** sorted_recent = + (const SimpleMeshTables::RecentRepeaterInfo**)malloc(sorted_size); + if (sorted_recent == NULL) { + strcpy(reply, "Err - unable to allocate recent repeater view"); + return; + } + + int total = buildSortedRecentRepeaterView(tables, sorted_recent, MAX_RECENT_REPEATERS); if (total <= 0) { strcpy(reply, "> none"); } else { + int total_pages = (total + (int)page_size - 1) / (int)page_size; + if (page_num > total_pages) { + sprintf(reply, "> none (page=%ld/%d)", page_num, total_pages); + free(sorted_recent); + return; + } + + int offset = ((int)page_num - 1) * (int)page_size; + int limit = total - offset; + if (limit > (int)page_size) { + limit = (int)page_size; + } + if (sender_timestamp == 0) { - // Serial CLI: print all entries (no paging). - Serial.printf("Recent repeater table (newest first, total=%d):\n", total); - for (int i = 0; i < total; i++) { - const auto* info = tables->getRecentRepeaterNewestByIdx(i); + Serial.printf("Recent repeater table (3-byte,2-byte,1-byte; SNR desc, page=%ld/%d, n=%d/%d):\n", + page_num, + total_pages, + limit, + total); + for (int i = 0; i < limit; i++) { + const auto* info = sorted_recent[offset + i]; if (info == NULL) { continue; } @@ -1592,40 +1678,8 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply (uint32_t)info->fail_count, info->snr_locked ? ",l" : ""); } - sprintf(reply, "> n=%d/%d", total, total); + sprintf(reply, "> page=%ld/%d n=%d/%d", page_num, total_pages, limit, total); } else { - // Remote CLI: page by fixed size to fit packet-limited reply payload. - long page_num = 1; - const long page_size = 4; - const char* arg = sub; - - if (strncmp(arg, "page ", 5) == 0) { - arg += 5; - while (*arg == ' ') arg++; - } - - if (*arg != 0) { - char* end_ptr = NULL; - page_num = strtol(arg, &end_ptr, 10); - while (end_ptr != NULL && *end_ptr == ' ') end_ptr++; - if (end_ptr == NULL || page_num <= 0 || (end_ptr != NULL && *end_ptr != 0)) { - strcpy(reply, "Err - usage: get recent.repeater [page]"); - return; - } - } - - int total_pages = (total + (int)page_size - 1) / (int)page_size; - if (page_num > total_pages) { - sprintf(reply, "> none (page=%ld/%d)", page_num, total_pages); - return; - } - - int offset = ((int)page_num - 1) * (int)page_size; - int limit = total - offset; - if (limit > (int)page_size) { - limit = (int)page_size; - } - int written = snprintf(reply, 160, "> page=%ld/%d n=%d/%d", page_num, total_pages, limit, total); bool truncated = false; if (written < 0) { @@ -1635,7 +1689,7 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply for (int i = 0; i < limit; i++) { int idx = offset + i; - const auto* info = tables->getRecentRepeaterNewestByIdx(idx); + const auto* info = sorted_recent[idx]; if (info == NULL) { continue; } @@ -1668,6 +1722,7 @@ void MyMesh::handleCommand(uint32_t sender_timestamp, char *command, char *reply } } } + free(sorted_recent); } } else{ _cli.handleCommand(sender_timestamp, command, reply); // common CLI commands diff --git a/src/Mesh.cpp b/src/Mesh.cpp index bc34c5ee..a84a1fda 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -16,6 +16,7 @@ void Mesh::begin() { _direct_retries[i].priority = 0; _direct_retries[i].progress_marker = 0; _direct_retries[i].expect_path_growth = false; + _direct_retries[i].waiting_final_echo = false; _direct_retries[i].queued = false; _direct_retries[i].active = false; } @@ -26,7 +27,25 @@ void Mesh::loop() { Dispatcher::loop(); for (int i = 0; i < MAX_DIRECT_RETRY_SLOTS; i++) { - if (!_direct_retries[i].active || !_direct_retries[i].queued || !millisHasNowPassed(_direct_retries[i].retry_at)) { + if (!_direct_retries[i].active) { + continue; + } + + if (_direct_retries[i].waiting_final_echo) { + if (!millisHasNowPassed(_direct_retries[i].retry_at)) { + continue; + } + + uint32_t elapsed_millis = _direct_retries[i].retry_started_at == 0 + ? 0 + : (uint32_t)(_ms->getMillis() - _direct_retries[i].retry_started_at); + onDirectRetryEvent("failed_all_tries", _direct_retries[i].packet, elapsed_millis, _direct_retries[i].retry_attempts_sent); + onDirectRetryEvent("failure", _direct_retries[i].packet, elapsed_millis, _direct_retries[i].retry_attempts_sent); + clearDirectRetrySlot(i); + continue; + } + + if (!_direct_retries[i].queued || !millisHasNowPassed(_direct_retries[i].retry_at)) { continue; } @@ -454,6 +473,9 @@ void Mesh::routeDirectRecvAcks(Packet* packet, uint32_t delay_millis) { } void Mesh::clearDirectRetrySlot(int idx) { + if (_direct_retries[idx].waiting_final_echo && _direct_retries[idx].packet != NULL) { + releasePacket(_direct_retries[idx].packet); + } _direct_retries[idx].packet = NULL; _direct_retries[idx].trigger_packet = NULL; _direct_retries[idx].retry_at = 0; @@ -462,6 +484,7 @@ void Mesh::clearDirectRetrySlot(int idx) { _direct_retries[idx].priority = 0; _direct_retries[idx].progress_marker = 0; _direct_retries[idx].expect_path_growth = false; + _direct_retries[idx].waiting_final_echo = false; _direct_retries[idx].queued = false; _direct_retries[idx].active = false; } @@ -497,24 +520,30 @@ bool Mesh::cancelDirectRetryOnEcho(const Packet* packet) { continue; } - if (_direct_retries[i].queued) { - if (_direct_retries[i].expect_path_growth - && _direct_retries[i].packet != NULL - && _direct_retries[i].progress_marker < packet->path_len) { - // For retry-good quality, use the received echo packet SNR (return-link quality). - _direct_retries[i].packet->_snr = packet->_snr; + int8_t echo_snr_x4 = packet->_snr; + if (_direct_retries[i].queued || _direct_retries[i].waiting_final_echo) { + if (_direct_retries[i].packet != NULL) { + // Success quality comes from the received downstream echo, not the original upstream RX. + _direct_retries[i].packet->_snr = echo_snr_x4; } - for (int j = 0; j < _mgr->getOutboundTotal(); j++) { - if (_mgr->getOutboundByIdx(j) == _direct_retries[i].packet) { - Packet* pending = _mgr->removeOutboundByIdx(j); - if (pending) { - releasePacket(pending); + uint32_t echo_millis = _direct_retries[i].echo_wait_started_at == 0 + ? 0 + : (uint32_t)(_ms->getMillis() - _direct_retries[i].echo_wait_started_at); + uint8_t retry_attempt = _direct_retries[i].waiting_final_echo + ? _direct_retries[i].retry_attempts_sent + : _direct_retries[i].retry_attempts_sent + 1; + onDirectRetryEvent("good", _direct_retries[i].packet, echo_millis, retry_attempt); + if (_direct_retries[i].queued) { + for (int j = 0; j < _mgr->getOutboundTotal(); j++) { + if (_mgr->getOutboundByIdx(j) == _direct_retries[i].packet) { + Packet* pending = _mgr->removeOutboundByIdx(j); + if (pending) { + releasePacket(pending); + } + break; } - break; } } - onDirectRetryEvent("canceled_echo", _direct_retries[i].packet, 0); - onDirectRetryEvent("good", _direct_retries[i].packet, 0); clearDirectRetrySlot(i); } else { if (_direct_retries[i].expect_path_growth @@ -551,9 +580,19 @@ void Mesh::armDirectRetryOnSendComplete(const Packet* packet) { max_attempts = DIRECT_RETRY_MAX_ATTEMPTS_HARD_MAX; } if (_direct_retries[i].retry_attempts_sent >= max_attempts) { - onDirectRetryEvent("failed_all_tries", packet, 0); - onDirectRetryEvent("failure", packet, 0); - clearDirectRetrySlot(i); + Packet* final_wait = obtainNewPacket(); + if (final_wait == NULL) { + onDirectRetryEvent("dropped_no_packet", packet, elapsed_millis, _direct_retries[i].retry_attempts_sent); + onDirectRetryEvent("failure", packet, elapsed_millis, _direct_retries[i].retry_attempts_sent); + clearDirectRetrySlot(i); + continue; + } + + *final_wait = *packet; + _direct_retries[i].packet = final_wait; + _direct_retries[i].retry_at = futureMillis(_direct_retries[i].retry_delay); + _direct_retries[i].waiting_final_echo = true; + _direct_retries[i].queued = false; continue; } @@ -572,7 +611,8 @@ void Mesh::armDirectRetryOnSendComplete(const Packet* packet) { _direct_retries[i].packet = retry; _direct_retries[i].retry_delay = retry_delay; _direct_retries[i].retry_at = futureMillis(retry_delay); - onDirectRetryEvent("queued", retry, retry_delay); + _direct_retries[i].waiting_final_echo = false; + onDirectRetryEvent("queued", retry, retry_delay, _direct_retries[i].retry_attempts_sent + 1); } else { onDirectRetryEvent("dropped_queue_full", retry, retry_delay); onDirectRetryEvent("failure", retry, 0); @@ -603,6 +643,7 @@ void Mesh::armDirectRetryOnSendComplete(const Packet* packet) { _direct_retries[i].packet = retry; _direct_retries[i].trigger_packet = NULL; _direct_retries[i].queued = true; + _direct_retries[i].waiting_final_echo = false; _direct_retries[i].retry_at = futureMillis(_direct_retries[i].retry_delay); onDirectRetryEvent("queued", retry, _direct_retries[i].retry_delay); } else { @@ -726,6 +767,7 @@ void Mesh::maybeScheduleDirectRetry(const Packet* packet, uint8_t priority) { _direct_retries[slot_idx].priority = priority; _direct_retries[slot_idx].progress_marker = progress_marker; _direct_retries[slot_idx].expect_path_growth = expect_path_growth; + _direct_retries[slot_idx].waiting_final_echo = false; _direct_retries[slot_idx].queued = false; _direct_retries[slot_idx].active = true; onDirectRetryEvent("armed", packet, retry_delay); diff --git a/src/Mesh.h b/src/Mesh.h index dd58d754..85848875 100644 --- a/src/Mesh.h +++ b/src/Mesh.h @@ -39,6 +39,7 @@ class Mesh : public Dispatcher { uint8_t priority; uint8_t progress_marker; bool expect_path_growth; + bool waiting_final_echo; bool queued; bool active; };