diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 42f76ef0..9cf5efe6 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -1267,7 +1267,7 @@ another qualifying repeater. **Parameters:** - `count`: Base retry attempts after the original send, from `0` to `15`. `0` disables flood retry. -**Note:** Actual attempts are capped at `15`. Hop 1 flood retries use `count * 2`; hop 2 flood retries use `count * 1.5`, rounded up. +**Note:** Actual attempts are capped at `15`. Path count 0 flood retries use `count * 2`; path count 1 retries use `count * 1.5`, rounded up; path count 2 and higher use the configured base count. **Defaults:** - `infra`: `1` @@ -1374,7 +1374,9 @@ set flood.retry.ignore none - `get flood.retry.bridge` - `set flood.retry.bridge ` -**Note:** Bridge mode retries until each configured fresh bucket, plus the non-source `other` bucket, has been heard or the retry count is exhausted. +**Note:** Bridge mode retries until each configured fresh bucket, plus the non-source `other` bucket, has been heard or the retry count is exhausted. If prefixes in different buckets share their first byte, configuration commands return a warning because a 1-byte path cannot distinguish those buckets. The configuration remains valid: an ambiguous source is treated as belonging to every matching source bucket, and an ambiguous echo credits every matching target bucket so it cannot force retry exhaustion. + +Flood retry timing retains its fixed maximum-frame plus 20 packet-airtime wait, then adds a random `0-200%` of one additional packet airtime on every attempt. This de-synchronizes repeaters that may have missed the same echo while capping the added wait at two frames. **Examples:** ``` diff --git a/docs/halo_keymind_settings.md b/docs/halo_keymind_settings.md index 9ef40190..07dea6fd 100644 --- a/docs/halo_keymind_settings.md +++ b/docs/halo_keymind_settings.md @@ -236,7 +236,7 @@ the bucket rules below instead. | Setting | What it does | How to use | Example | | --- | --- | --- | --- | -| `flood.retry.count` | Base flood retry attempts after initial TX. Hop 1 doubles it, hop 2 uses 1.5x rounded up, and actual attempts cap at `15`; `0` disables flood retry. | `get flood.retry.count`, `set flood.retry.count <0-15>` | `set flood.retry.count 7` | +| `flood.retry.count` | Base flood retry attempts after initial TX. Path count 0 doubles it, path count 1 uses 1.5x rounded up, path count 2+ uses the base, and actual attempts cap at `15`; `0` disables flood retry. | `get flood.retry.count`, `set flood.retry.count <0-15>` | `set flood.retry.count 7` | | `flood.retry.path` | Maximum path hash count eligible for flood retry, or `off` to disable the gate. | `get flood.retry.path`, `set flood.retry.path <0-63/off>` | `set flood.retry.path 1` | | `flood.retry.advert` | Allows or blocks retry for node advert packets (`type=4`). Default is `off`. | `get flood.retry.advert`, `set flood.retry.advert on/off` | `set flood.retry.advert off` | | `flood.retry.prefixes` | Target prefixes. If set, only same-packet echoes from matching last-hop prefixes cancel a retry. | `get flood.retry.prefixes`, `set flood.retry.prefixes ` | `set flood.retry.prefixes BEEBB0,425E5C` | @@ -338,6 +338,18 @@ Bridge retry stays eligible until every target bucket has been heard or least one of its prefixes is fresh in `recent.repeater`. Prefixes in `flood.retry.ignore` never count as bucket hits. +Configuration reports a warning when prefixes in different buckets, including +bucket 7 (`flood.retry.prefixes`), share the same first byte. A 1-byte path cannot +distinguish those buckets. Bridge mode therefore excludes every matching bucket +when that short prefix is the source, and credits every matching target bucket +when it is heard as an echo. This prevents an ambiguous short prefix from keeping +an impossible target outstanding through every retry. + +Each flood retry wait retains the fixed maximum-frame plus 20 packet-airtime +delay, then adds random jitter from zero to 200 percent of one additional packet +airtime. This keeps nearby repeaters from repeating a collision on fixed timing +while capping the added wait at two frames. + ## Troubleshooting If advert packets are still retrying: diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 09851141..09bbbf36 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -1342,56 +1342,60 @@ static uint8_t floodRetryBucketMask(uint8_t bucket) { return (uint8_t)(1U << bucket); } -int MyMesh::floodRetryBucketForPrefix(const uint8_t* prefix, uint8_t prefix_len, bool require_fresh) const { +uint8_t MyMesh::floodRetryBucketMaskForPrefix(const uint8_t* prefix, uint8_t prefix_len, bool require_fresh) const { if (prefix == NULL || prefix_len == 0 || prefix_len > MAX_ROUTE_HASH_BYTES) { - return -1; + return 0; } if (floodRetryPrefixIgnored(prefix, prefix_len)) { - return -1; + return 0; } if (require_fresh && !floodRetryPrefixFresh(prefix, prefix_len)) { - return -1; + return 0; } + uint8_t mask = 0; for (int bucket = 0; bucket < FLOOD_RETRY_BRIDGE_BUCKETS; bucket++) { for (int i = 0; i < FLOOD_RETRY_BUCKET_PREFIXES; i++) { const uint8_t* configured = _prefs.flood_retry_bridge_buckets[bucket][i]; if (configuredFloodRetryPrefixMatches(configured, prefix, prefix_len)) { - return bucket; + mask |= floodRetryBucketMask((uint8_t)bucket); + break; } } } for (int i = 0; i < FLOOD_RETRY_PREFIX_SLOTS; i++) { if (configuredFloodRetryPrefixMatches(_prefs.flood_retry_prefixes[i], prefix, prefix_len)) { - return FLOOD_RETRY_BRIDGE_OTHER_BUCKET; + mask |= floodRetryBucketMask(FLOOD_RETRY_BRIDGE_OTHER_BUCKET); + break; } } - return -1; + return mask; } -int MyMesh::floodRetryBucketForPathHop(const uint8_t* prefix, uint8_t prefix_len, uint8_t hop, - uint8_t progress_marker) const { - return floodRetryBucketForPrefix(prefix, prefix_len, hop < progress_marker); +uint8_t MyMesh::floodRetryBucketMaskForPathHop(const uint8_t* prefix, uint8_t prefix_len, uint8_t hop, + uint8_t progress_marker) const { + return floodRetryBucketMaskForPrefix(prefix, prefix_len, hop < progress_marker); } -int MyMesh::floodRetrySourceBucket(const mesh::Packet* packet) const { +uint8_t MyMesh::floodRetrySourceMask(const mesh::Packet* packet) const { if (packet == NULL) { - return -1; + return 0; } uint8_t hash_size = packet->getPathHashSize(); if (hash_size == 0 || hash_size > MAX_ROUTE_HASH_BYTES) { - return -1; + return 0; } if (packet->getPathHashCount() < 2) { - return FLOOD_RETRY_BRIDGE_OTHER_BUCKET; + return floodRetryBucketMask(FLOOD_RETRY_BRIDGE_OTHER_BUCKET); } const uint8_t* source_prefix = &packet->path[(packet->getPathHashCount() - 2) * hash_size]; - return floodRetryBucketForPrefix(source_prefix, hash_size, true); + return floodRetryBucketMaskForPrefix(source_prefix, hash_size, true); } -uint8_t MyMesh::floodRetryBridgeTargetMask(uint8_t source_bucket) const { +uint8_t MyMesh::floodRetryBridgeTargetMask(uint8_t source_mask) const { uint8_t mask = 0; for (int bucket = 0; bucket < FLOOD_RETRY_BRIDGE_BUCKETS; bucket++) { - if (bucket == source_bucket) { + uint8_t bucket_mask = floodRetryBucketMask((uint8_t)bucket); + if ((source_mask & bucket_mask) != 0) { continue; } for (int i = 0; i < FLOOD_RETRY_BUCKET_PREFIXES; i++) { @@ -1399,18 +1403,19 @@ uint8_t MyMesh::floodRetryBridgeTargetMask(uint8_t source_bucket) const { if ((configured[0] != 0 || configured[1] != 0 || configured[2] != 0) && !floodRetryPrefixIgnored(configured, FLOOD_RETRY_PREFIX_LEN) && floodRetryPrefixFresh(configured, FLOOD_RETRY_PREFIX_LEN)) { - mask |= floodRetryBucketMask((uint8_t)bucket); + mask |= bucket_mask; break; } } } - if (source_bucket != FLOOD_RETRY_BRIDGE_OTHER_BUCKET) { + uint8_t other_mask = floodRetryBucketMask(FLOOD_RETRY_BRIDGE_OTHER_BUCKET); + if ((source_mask & other_mask) == 0) { for (int i = 0; i < FLOOD_RETRY_PREFIX_SLOTS; i++) { const uint8_t* configured = _prefs.flood_retry_prefixes[i]; if ((configured[0] != 0 || configured[1] != 0 || configured[2] != 0) && !floodRetryPrefixIgnored(configured, FLOOD_RETRY_PREFIX_LEN) && floodRetryPrefixFresh(configured, FLOOD_RETRY_PREFIX_LEN)) { - mask |= floodRetryBucketMask(FLOOD_RETRY_BRIDGE_OTHER_BUCKET); + mask |= other_mask; break; } } @@ -1418,7 +1423,7 @@ uint8_t MyMesh::floodRetryBridgeTargetMask(uint8_t source_bucket) const { return mask; } -uint8_t MyMesh::floodRetryBridgeHeardMask(const mesh::Packet* packet, uint8_t source_bucket, +uint8_t MyMesh::floodRetryBridgeHeardMask(const mesh::Packet* packet, uint8_t source_mask, uint8_t progress_marker) const { if (packet == NULL || packet->getPathHashCount() == 0) { return 0; @@ -1435,10 +1440,8 @@ uint8_t MyMesh::floodRetryBridgeHeardMask(const mesh::Packet* packet, uint8_t so path += hash_size; continue; } - int bucket = floodRetryBucketForPathHop(path, hash_size, (uint8_t)hop, progress_marker); - if (bucket >= 0 && bucket != source_bucket) { - mask |= floodRetryBucketMask((uint8_t)bucket); - } + uint8_t bucket_mask = floodRetryBucketMaskForPathHop(path, hash_size, (uint8_t)hop, progress_marker); + mask |= bucket_mask & (uint8_t)~source_mask; path += hash_size; } return mask; @@ -1465,25 +1468,25 @@ MyMesh::FloodRetryBridgeState* MyMesh::floodRetryBridgeStateFor(const mesh::Pack return NULL; } - int source_bucket = floodRetrySourceBucket(packet); - if (source_bucket < 0) { + uint8_t source_mask = floodRetrySourceMask(packet); + if (source_mask == 0) { return NULL; } - uint8_t target_mask = floodRetryBridgeTargetMask((uint8_t)source_bucket); + uint8_t target_mask = floodRetryBridgeTargetMask(source_mask); if (target_mask == 0) { return NULL; } uint8_t progress_marker = packet->getPathHashCount(); - uint8_t heard_mask = floodRetryBridgeHeardMask(packet, (uint8_t)source_bucket, progress_marker) & target_mask; + uint8_t heard_mask = floodRetryBridgeHeardMask(packet, source_mask, progress_marker) & target_mask; if ((heard_mask & target_mask) == target_mask) { return NULL; } memset(free_slot, 0, sizeof(*free_slot)); memcpy(free_slot->key, key, sizeof(free_slot->key)); - free_slot->source_bucket = (uint8_t)source_bucket; + free_slot->source_mask = source_mask; free_slot->target_mask = target_mask; free_slot->heard_mask = heard_mask; free_slot->progress_marker = progress_marker; @@ -1542,9 +1545,10 @@ void MyMesh::refreshFloodRetryHeardRecent(const mesh::Packet* packet) { path += hash_size; continue; } - int bucket = floodRetryBucketForPathHop(path, hash_size, (uint8_t)hop, state->progress_marker); - uint8_t bucket_mask = bucket >= 0 ? floodRetryBucketMask((uint8_t)bucket) : 0; - if (bucket >= 0 && bucket != state->source_bucket && (state->target_mask & bucket_mask)) { + uint8_t bucket_mask = floodRetryBucketMaskForPathHop(path, hash_size, (uint8_t)hop, + state->progress_marker); + bucket_mask &= state->target_mask & (uint8_t)~state->source_mask; + if (bucket_mask != 0) { tables->setRecentRepeater(path, hash_size, packet->_snr, false, true); } path += hash_size; @@ -1622,11 +1626,16 @@ bool MyMesh::formatFloodRetryHeard(char* dest, size_t dest_len, const mesh::Pack path += hash_size; continue; } - int bucket = floodRetryBucketForPathHop(path, hash_size, (uint8_t)hop, state->progress_marker); - uint8_t bucket_mask = bucket >= 0 ? floodRetryBucketMask((uint8_t)bucket) : 0; - if (bucket >= 0 && bucket != state->source_bucket && (state->target_mask & bucket_mask)) { + uint8_t matching_mask = floodRetryBucketMaskForPathHop(path, hash_size, (uint8_t)hop, + state->progress_marker); + matching_mask &= state->target_mask & (uint8_t)~state->source_mask; + for (uint8_t bucket = 0; bucket <= FLOOD_RETRY_BRIDGE_OTHER_BUCKET; bucket++) { + uint8_t bucket_mask = floodRetryBucketMask(bucket); + if ((matching_mask & bucket_mask) == 0) { + continue; + } char bucket_label[8]; - if ((uint8_t)bucket == FLOOD_RETRY_BRIDGE_OTHER_BUCKET) { + if (bucket == FLOOD_RETRY_BRIDGE_OTHER_BUCKET) { strcpy(bucket_label, "other"); } else { snprintf(bucket_label, sizeof(bucket_label), "b%d", bucket + 1); @@ -1776,9 +1785,9 @@ uint8_t MyMesh::getFloodRetryMaxAttempts(const mesh::Packet* packet) const { uint8_t attempts = constrain(_prefs.flood_retry_attempts, 0, 15); uint16_t scaled_attempts = attempts; uint8_t hops = packet != NULL ? packet->getPathHashCount() : 0; - if (hops == 1) { + if (hops == 0) { scaled_attempts = (uint16_t)attempts * 2U; - } else if (hops == 2) { + } else if (hops == 1) { scaled_attempts = (((uint16_t)attempts * 3U) + 1U) / 2U; } return scaled_attempts > 15 ? 15 : (uint8_t)scaled_attempts; @@ -1804,7 +1813,7 @@ bool MyMesh::isFloodRetryEchoTarget(const mesh::Packet* packet, uint8_t progress if (state == NULL) { return false; } - state->heard_mask |= floodRetryBridgeHeardMask(packet, state->source_bucket, state->progress_marker) & state->target_mask; + state->heard_mask |= floodRetryBridgeHeardMask(packet, state->source_mask, state->progress_marker) & state->target_mask; return (state->heard_mask & state->target_mask) == state->target_mask; } if (hasFloodRetryPrefixes()) { diff --git a/examples/simple_repeater/MyMesh.h b/examples/simple_repeater/MyMesh.h index 69468844..e2d9bbdc 100644 --- a/examples/simple_repeater/MyMesh.h +++ b/examples/simple_repeater/MyMesh.h @@ -137,7 +137,7 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks { RateLimiter discover_limiter, anon_limiter; struct FloodRetryBridgeState { uint8_t key[MAX_HASH_SIZE]; - uint8_t source_bucket; + uint8_t source_mask; uint8_t target_mask; uint8_t heard_mask; uint8_t progress_marker; @@ -208,12 +208,12 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks { bool floodRetryPrefixIgnored(const uint8_t* prefix, uint8_t prefix_len) const; uint8_t floodRetryEffectivePathLength(const mesh::Packet* packet, uint8_t max_hops = 0xFF) const; bool floodRetryPrefixFresh(const uint8_t* prefix, uint8_t prefix_len) const; - int floodRetryBucketForPrefix(const uint8_t* prefix, uint8_t prefix_len, bool require_fresh) const; - int floodRetryBucketForPathHop(const uint8_t* prefix, uint8_t prefix_len, uint8_t hop, - uint8_t progress_marker) const; - int floodRetrySourceBucket(const mesh::Packet* packet) const; - uint8_t floodRetryBridgeTargetMask(uint8_t source_bucket) const; - uint8_t floodRetryBridgeHeardMask(const mesh::Packet* packet, uint8_t source_bucket, + uint8_t floodRetryBucketMaskForPrefix(const uint8_t* prefix, uint8_t prefix_len, bool require_fresh) const; + uint8_t floodRetryBucketMaskForPathHop(const uint8_t* prefix, uint8_t prefix_len, uint8_t hop, + uint8_t progress_marker) const; + uint8_t floodRetrySourceMask(const mesh::Packet* packet) const; + uint8_t floodRetryBridgeTargetMask(uint8_t source_mask) const; + uint8_t floodRetryBridgeHeardMask(const mesh::Packet* packet, uint8_t source_mask, uint8_t progress_marker) const; FloodRetryBridgeState* floodRetryBridgeStateFor(const mesh::Packet* packet, bool create) const; void clearFloodRetryBridgeState(const mesh::Packet* packet); diff --git a/platformio.ini b/platformio.ini index 960e3ba5..e8ac393e 100644 --- a/platformio.ini +++ b/platformio.ini @@ -198,6 +198,7 @@ build_src_filter = -<*> +<../src/Utils.cpp> +<../src/Packet.cpp> + +<../src/helpers/StaticPoolPacketManager.cpp> +<../src/helpers/ota/MerkleTree.cpp> +<../src/helpers/ota/MotaContainer.cpp> +<../src/helpers/ota/FirmwareInfo.cpp> diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 2288df25..e312db68 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -402,13 +402,16 @@ uint8_t Mesh::getFloodRetryMaxAttempts(const Packet* packet) const { return FLOOD_RETRY_MAX_ATTEMPTS_DEFAULT; } uint32_t Mesh::getFloodRetryAttemptDelay(const Packet* packet, uint8_t attempt_idx) { + (void)attempt_idx; if (packet == NULL) { return _radio->getEstAirtimeFor(MAX_TRANS_UNIT); } uint32_t max_packet_airtime = _radio->getEstAirtimeFor(MAX_TRANS_UNIT); uint32_t packet_airtime = _radio->getEstAirtimeFor(packet->getRawLength()); - return max_packet_airtime + (20UL * packet_airtime); + uint32_t jitter_percent = _rng->nextInt(0, 201); + uint32_t jitter = (packet_airtime * jitter_percent) / 100UL; + return max_packet_airtime + (20UL * packet_airtime) + jitter; } uint8_t Mesh::getExtraAckTransmitCount() const { return 0; diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 9239bbf9..5e10e254 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -571,6 +571,61 @@ static bool parseFloodRetryPrefixList(uint8_t dest[][FLOOD_RETRY_PREFIX_LEN], ui return true; } +static const uint8_t* floodRetryBucketPrefixAt(const NodePrefs* prefs, uint8_t bucket, uint8_t index) { + if (bucket < FLOOD_RETRY_BRIDGE_BUCKETS && index < FLOOD_RETRY_BUCKET_PREFIXES) { + return prefs->flood_retry_bridge_buckets[bucket][index]; + } + if (bucket == FLOOD_RETRY_BRIDGE_BUCKETS && index < FLOOD_RETRY_PREFIX_SLOTS) { + return prefs->flood_retry_prefixes[index]; + } + return NULL; +} + +static uint8_t floodRetryBucketPrefixCount(uint8_t bucket) { + return bucket < FLOOD_RETRY_BRIDGE_BUCKETS ? FLOOD_RETRY_BUCKET_PREFIXES : FLOOD_RETRY_PREFIX_SLOTS; +} + +static bool findFloodRetryFirstByteCollision(const NodePrefs* prefs, uint8_t& first_byte, + uint8_t& first_bucket, uint8_t& second_bucket) { + for (uint8_t bucket_a = 0; bucket_a <= FLOOD_RETRY_BRIDGE_BUCKETS; bucket_a++) { + for (uint8_t bucket_b = bucket_a + 1; bucket_b <= FLOOD_RETRY_BRIDGE_BUCKETS; bucket_b++) { + for (uint8_t a = 0; a < floodRetryBucketPrefixCount(bucket_a); a++) { + const uint8_t* prefix_a = floodRetryBucketPrefixAt(prefs, bucket_a, a); + if (prefix_a == NULL || (prefix_a[0] == 0 && prefix_a[1] == 0 && prefix_a[2] == 0)) { + continue; + } + for (uint8_t b = 0; b < floodRetryBucketPrefixCount(bucket_b); b++) { + const uint8_t* prefix_b = floodRetryBucketPrefixAt(prefs, bucket_b, b); + if (prefix_b != NULL && (prefix_b[0] != 0 || prefix_b[1] != 0 || prefix_b[2] != 0) + && prefix_a[0] == prefix_b[0]) { + first_byte = prefix_a[0]; + first_bucket = bucket_a; + second_bucket = bucket_b; + return true; + } + } + } + } + } + return false; +} + +static bool formatFloodRetryBucketCollisionWarning(char* reply, const NodePrefs* prefs, const char* prefix) { + uint8_t first_byte, first_bucket, second_bucket; + if (!findFloodRetryFirstByteCollision(prefs, first_byte, first_bucket, second_bucket)) { + return false; + } + if (second_bucket == FLOOD_RETRY_BRIDGE_BUCKETS) { + snprintf(reply, 160, "%sWARNING: 1-byte %02X matches buckets %u and 7 (other)", prefix, + (unsigned int)first_byte, (unsigned int)first_bucket + 1U); + } else { + snprintf(reply, 160, "%sWARNING: 1-byte %02X matches buckets %u and %u", prefix, + (unsigned int)first_byte, (unsigned int)first_bucket + 1U, + (unsigned int)second_bucket + 1U); + } + return true; +} + static bool parseFloodChannelBlockKey(const char* text, uint8_t secret[PUB_KEY_SIZE], uint8_t& key_len) { if (text == NULL || text[0] == 0) { return false; @@ -2509,7 +2564,9 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep } else if (memcmp(config, "flood.retry.prefixes ", 21) == 0) { if (parseFloodRetryPrefixList(_prefs->flood_retry_prefixes, FLOOD_RETRY_PREFIX_SLOTS, &config[21])) { savePrefs(); - strcpy(reply, "OK"); + if (!formatFloodRetryBucketCollisionWarning(reply, _prefs, "OK - ")) { + strcpy(reply, "OK"); + } } else { sprintf(reply, "Error, use up to %u comma-separated 3-byte hex prefixes", (unsigned int)FLOOD_RETRY_PREFIX_SLOTS); @@ -2539,7 +2596,9 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep if (strcmp(&config[19], "on") == 0) { _prefs->flood_retry_bridge_enabled = 1; savePrefs(); - strcpy(reply, "OK - flood.retry.prefixes acts as bucket 7 (other); multi-try bridge routing enabled"); + if (!formatFloodRetryBucketCollisionWarning(reply, _prefs, "OK - bucket 7=prefixes; ")) { + strcpy(reply, "OK - flood.retry.prefixes acts as bucket 7 (other); multi-try bridge routing enabled"); + } } else if (strcmp(&config[19], "off") == 0) { _prefs->flood_retry_bridge_enabled = 0; savePrefs(); @@ -2556,7 +2615,9 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, char* command, char* rep } else if (parseFloodRetryPrefixList(_prefs->flood_retry_bridge_buckets[bucket - 1], FLOOD_RETRY_BUCKET_PREFIXES, list + 1)) { savePrefs(); - strcpy(reply, "OK"); + if (!formatFloodRetryBucketCollisionWarning(reply, _prefs, "OK - ")) { + strcpy(reply, "OK"); + } } else { sprintf(reply, "Error, use up to %u comma-separated 3-byte hex prefixes", (unsigned int)FLOOD_RETRY_BUCKET_PREFIXES); diff --git a/src/helpers/RxReservePacketManager.h b/src/helpers/RxReservePacketManager.h index 23c8f046..5efaeae6 100644 --- a/src/helpers/RxReservePacketManager.h +++ b/src/helpers/RxReservePacketManager.h @@ -64,7 +64,8 @@ public: if (free_count < _emergency_floor || (free_count < _rx_reserve && priority > MAX_PROTECTED_PRI)) { MESH_DEBUG_PRINTLN("RxReservePacketManager: pool below RX reserve, shedding outbound (pri %d)", (int)priority); - free(packet); + // queueOutbound() follows the PacketManager ownership contract: a false + // return leaves the packet with the caller, which will release it once. return false; } recordAge(packet, scheduled_for); diff --git a/test/test_packet_manager/test_rx_reserve_packet_manager.cpp b/test/test_packet_manager/test_rx_reserve_packet_manager.cpp new file mode 100644 index 00000000..1941d150 --- /dev/null +++ b/test/test_packet_manager/test_rx_reserve_packet_manager.cpp @@ -0,0 +1,28 @@ +#include + +#include + +TEST(RxReservePacketManager, RejectedOutboundRemainsOwnedByCaller) { + RxReservePacketManager manager(8, 4); + mesh::Packet* held[6]; + for (int i = 0; i < 6; i++) { + held[i] = manager.allocNew(); + ASSERT_NE(held[i], nullptr); + } + ASSERT_EQ(manager.getFreeCount(), 2); + + EXPECT_FALSE(manager.queueOutbound(held[5], 2, 0)); + EXPECT_EQ(manager.getFreeCount(), 2); + + manager.free(held[5]); + EXPECT_EQ(manager.getFreeCount(), 3); + for (int i = 0; i < 5; i++) { + manager.free(held[i]); + } + EXPECT_EQ(manager.getFreeCount(), 8); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}