From c436bd42c5961c02eb439fe4ce2584b27f832eff Mon Sep 17 00:00:00 2001 From: Daniel Novak Date: Sat, 28 Feb 2026 16:22:58 +0100 Subject: [PATCH 1/3] Fix countBefore sentinel regression from millis wraparound fix PR #1795 changed PacketQueue::countBefore() to use signed 2's complement arithmetic for millis wraparound safety. However, this broke the 0xFFFFFFFF sentinel pattern used by callers to mean "count all packets regardless of schedule". With the signed comparison, countBefore(0xFFFFFFFF) always returns 0, causing hasPendingWork() to report false and repeaters to sleep with packets still queued. Stats reporting also shows queue_len as 0. Add an early-return for the sentinel value before the loop, and document the sentinel convention on the virtual interface and implementation. --- src/Dispatcher.h | 2 +- src/helpers/StaticPoolPacketManager.cpp | 1 + src/helpers/StaticPoolPacketManager.h | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Dispatcher.h b/src/Dispatcher.h index 0a448c402..7ecee1a43 100644 --- a/src/Dispatcher.h +++ b/src/Dispatcher.h @@ -89,7 +89,7 @@ class PacketManager { virtual void queueOutbound(Packet* packet, uint8_t priority, uint32_t scheduled_for) = 0; virtual Packet* getNextOutbound(uint32_t now) = 0; // by priority - virtual int getOutboundCount(uint32_t now) const = 0; + virtual int getOutboundCount(uint32_t now) const = 0; // pass now=0xFFFFFFFF to count all virtual int getFreeCount() const = 0; virtual Packet* getOutboundByIdx(int i) = 0; virtual Packet* removeOutboundByIdx(int i) = 0; diff --git a/src/helpers/StaticPoolPacketManager.cpp b/src/helpers/StaticPoolPacketManager.cpp index 67d639793..c89d50881 100644 --- a/src/helpers/StaticPoolPacketManager.cpp +++ b/src/helpers/StaticPoolPacketManager.cpp @@ -9,6 +9,7 @@ PacketQueue::PacketQueue(int max_entries) { } int PacketQueue::countBefore(uint32_t now) const { + if (now == 0xFFFFFFFF) return _num; // sentinel: count all entries regardless of schedule int n = 0; for (int j = 0; j < _num; j++) { if ((int32_t)(_schedule_table[j] - now) > 0) continue; // scheduled for future... ignore for now diff --git a/src/helpers/StaticPoolPacketManager.h b/src/helpers/StaticPoolPacketManager.h index 52c299dbc..bcc5deb96 100644 --- a/src/helpers/StaticPoolPacketManager.h +++ b/src/helpers/StaticPoolPacketManager.h @@ -13,7 +13,7 @@ class PacketQueue { mesh::Packet* get(uint32_t now); bool add(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for); int count() const { return _num; } - int countBefore(uint32_t now) const; + int countBefore(uint32_t now) const; // pass now=0xFFFFFFFF to count all mesh::Packet* itemAt(int i) const { return _table[i]; } mesh::Packet* removeByIdx(int i); }; From c7568a8db07194beee71072120ffc1f27389d3e5 Mon Sep 17 00:00:00 2001 From: Daniel Novak Date: Sat, 28 Feb 2026 17:19:04 +0100 Subject: [PATCH 2/3] Replace 0xFFFFFFFF sentinel with explicit getOutboundTotal() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of overloading getOutboundCount() with a magic sentinel value, add a dedicated getOutboundTotal() method to the PacketManager interface that returns the total queue size without time filtering. This eliminates the fragile convention that caused the regression and makes the two operations — time-filtered count vs total count — explicitly separate in the API. --- examples/companion_radio/MyMesh.cpp | 2 +- examples/simple_repeater/MyMesh.cpp | 4 ++-- examples/simple_room_server/MyMesh.cpp | 2 +- src/Dispatcher.h | 3 ++- src/helpers/StaticPoolPacketManager.cpp | 5 ++++- src/helpers/StaticPoolPacketManager.h | 3 ++- src/helpers/StatsFormatHelper.h | 2 +- 7 files changed, 13 insertions(+), 8 deletions(-) diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index c96f7e017..952055d94 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -1712,7 +1712,7 @@ void MyMesh::handleCmdFrame(size_t len) { out_frame[i++] = STATS_TYPE_CORE; uint16_t battery_mv = board.getBattMilliVolts(); uint32_t uptime_secs = _ms->getMillis() / 1000; - uint8_t queue_len = (uint8_t)_mgr->getOutboundCount(0xFFFFFFFF); + uint8_t queue_len = (uint8_t)_mgr->getOutboundTotal(); memcpy(&out_frame[i], &battery_mv, 2); i += 2; memcpy(&out_frame[i], &uptime_secs, 4); i += 4; memcpy(&out_frame[i], &_err_flags, 2); i += 2; diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 81c1dcb42..03ebe0e2d 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -219,7 +219,7 @@ int MyMesh::handleRequest(ClientInfo *sender, uint32_t sender_timestamp, uint8_t if (payload[0] == REQ_TYPE_GET_STATUS) { // guests can also access this now RepeaterStats stats; stats.batt_milli_volts = board.getBattMilliVolts(); - stats.curr_tx_queue_len = _mgr->getOutboundCount(0xFFFFFFFF); + stats.curr_tx_queue_len = _mgr->getOutboundTotal(); stats.noise_floor = (int16_t)_radio->getNoiseFloor(); stats.last_rssi = (int16_t)radio_driver.getLastRSSI(); stats.n_packets_recv = radio_driver.getPacketsRecv(); @@ -1290,5 +1290,5 @@ bool MyMesh::hasPendingWork() const { #if defined(WITH_BRIDGE) if (bridge.isRunning()) return true; // bridge needs WiFi radio, can't sleep #endif - return _mgr->getOutboundCount(0xFFFFFFFF) > 0; + return _mgr->getOutboundTotal() > 0; } diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 5451505a2..8b52f45b0 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -140,7 +140,7 @@ int MyMesh::handleRequest(ClientInfo *sender, uint32_t sender_timestamp, uint8_t if (payload[0] == REQ_TYPE_GET_STATUS) { ServerStats stats; stats.batt_milli_volts = board.getBattMilliVolts(); - stats.curr_tx_queue_len = _mgr->getOutboundCount(0xFFFFFFFF); + stats.curr_tx_queue_len = _mgr->getOutboundTotal(); stats.noise_floor = (int16_t)_radio->getNoiseFloor(); stats.last_rssi = (int16_t)radio_driver.getLastRSSI(); stats.n_packets_recv = radio_driver.getPacketsRecv(); diff --git a/src/Dispatcher.h b/src/Dispatcher.h index 7ecee1a43..7ddac9e9f 100644 --- a/src/Dispatcher.h +++ b/src/Dispatcher.h @@ -89,7 +89,8 @@ class PacketManager { virtual void queueOutbound(Packet* packet, uint8_t priority, uint32_t scheduled_for) = 0; virtual Packet* getNextOutbound(uint32_t now) = 0; // by priority - virtual int getOutboundCount(uint32_t now) const = 0; // pass now=0xFFFFFFFF to count all + virtual int getOutboundCount(uint32_t now) const = 0; + virtual int getOutboundTotal() const = 0; virtual int getFreeCount() const = 0; virtual Packet* getOutboundByIdx(int i) = 0; virtual Packet* removeOutboundByIdx(int i) = 0; diff --git a/src/helpers/StaticPoolPacketManager.cpp b/src/helpers/StaticPoolPacketManager.cpp index c89d50881..7ef2dafa2 100644 --- a/src/helpers/StaticPoolPacketManager.cpp +++ b/src/helpers/StaticPoolPacketManager.cpp @@ -9,7 +9,6 @@ PacketQueue::PacketQueue(int max_entries) { } int PacketQueue::countBefore(uint32_t now) const { - if (now == 0xFFFFFFFF) return _num; // sentinel: count all entries regardless of schedule int n = 0; for (int j = 0; j < _num; j++) { if ((int32_t)(_schedule_table[j] - now) > 0) continue; // scheduled for future... ignore for now @@ -98,6 +97,10 @@ int StaticPoolPacketManager::getOutboundCount(uint32_t now) const { return send_queue.countBefore(now); } +int StaticPoolPacketManager::getOutboundTotal() const { + return send_queue.count(); +} + int StaticPoolPacketManager::getFreeCount() const { return unused.count(); } diff --git a/src/helpers/StaticPoolPacketManager.h b/src/helpers/StaticPoolPacketManager.h index bcc5deb96..59715b4e0 100644 --- a/src/helpers/StaticPoolPacketManager.h +++ b/src/helpers/StaticPoolPacketManager.h @@ -13,7 +13,7 @@ class PacketQueue { mesh::Packet* get(uint32_t now); bool add(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for); int count() const { return _num; } - int countBefore(uint32_t now) const; // pass now=0xFFFFFFFF to count all + int countBefore(uint32_t now) const; mesh::Packet* itemAt(int i) const { return _table[i]; } mesh::Packet* removeByIdx(int i); }; @@ -29,6 +29,7 @@ class StaticPoolPacketManager : public mesh::PacketManager { void queueOutbound(mesh::Packet* packet, uint8_t priority, uint32_t scheduled_for) override; mesh::Packet* getNextOutbound(uint32_t now) override; int getOutboundCount(uint32_t now) const override; + int getOutboundTotal() const override; int getFreeCount() const override; mesh::Packet* getOutboundByIdx(int i) override; mesh::Packet* removeOutboundByIdx(int i) override; diff --git a/src/helpers/StatsFormatHelper.h b/src/helpers/StatsFormatHelper.h index 5aa01da97..bf619133e 100644 --- a/src/helpers/StatsFormatHelper.h +++ b/src/helpers/StatsFormatHelper.h @@ -14,7 +14,7 @@ class StatsFormatHelper { board.getBattMilliVolts(), ms.getMillis() / 1000, err_flags, - mgr->getOutboundCount(0xFFFFFFFF) + mgr->getOutboundTotal() ); } From 0d87dcc989609d570ec06400b4f3f5a40fc8f74d Mon Sep 17 00:00:00 2001 From: Daniel Novak Date: Sun, 1 Mar 2026 07:39:43 +0100 Subject: [PATCH 3/3] Also fix countBefore(0xFFFFFFFF) to return _num The signed comparison in countBefore breaks for the max uint32_t value. Even though callers now use getOutboundTotal(), the function itself should be correct for all inputs. --- src/helpers/StaticPoolPacketManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/helpers/StaticPoolPacketManager.cpp b/src/helpers/StaticPoolPacketManager.cpp index 7ef2dafa2..b8926df0c 100644 --- a/src/helpers/StaticPoolPacketManager.cpp +++ b/src/helpers/StaticPoolPacketManager.cpp @@ -9,6 +9,8 @@ PacketQueue::PacketQueue(int max_entries) { } int PacketQueue::countBefore(uint32_t now) const { + if (now == 0xFFFFFFFF) return _num; // sentinel: count all entries regardless of schedule + int n = 0; for (int j = 0; j < _num; j++) { if ((int32_t)(_schedule_table[j] - now) > 0) continue; // scheduled for future... ignore for now