Fix countBefore regression: replace sentinel with getOutboundTotal()

[DanielNovak]

Summary

PR #1795 introduced a millis wraparound fix in PacketQueue::countBefore(), changing the comparison from unsigned to signed 2's complement arithmetic:

if ((int32_t)(_schedule_table[j] - now) > 0) continue;

This correctly handles millis wraparound (~49.7 day rollover), but 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.

As noted in #1795 and #1687, this causes:

• hasPendingWork() to always return false — repeaters sleep with packets still queued
• Stats reporting to always show queue_len = 0

Fix

This PR adds a dedicated getOutboundTotal() method to the PacketManager interface that returns the total queue size via PacketQueue::count(), without any time filtering. This makes the two operations explicit in the API:

• getOutboundCount(now) — count packets scheduled at or before time now
• getOutboundTotal() — count all queued packets

All 5 call sites that previously passed 0xFFFFFFFF now use the new method:

• examples/simple_repeater/MyMesh.cpp — hasPendingWork() + repeater stats
• examples/simple_room_server/MyMesh.cpp — server status
• examples/companion_radio/MyMesh.cpp — companion stats
• src/helpers/StatsFormatHelper.h — core stats JSON

Additionally, countBefore(0xFFFFFFFF) itself is fixed to return _num, so the function is correct for all inputs even if called directly with the max value.

Impact

• Performance: None — getOutboundTotal() returns _num directly, avoiding the loop
• Backwards compatibility: Adds a new pure virtual method to PacketManager. Out-of-tree implementations will get a compile error guiding them to add it. StaticPoolPacketManager is the only implementation in-tree.

[DanielNovak]

Sorry for breaking this, I should have checked the callers of that function for any special constants :-/.

[fschrempf]

Instead of continuing to use the error prone magic number 0xFFFFFFFF, I suggest to introduce:

int PacketQueue::count() const {
  return _num;
}

And then replacing countBefore(0xFFFFFFFF) with count() everywhere. This is much more readable and less likely to cause problems in the future.

[DanielNovak]

Definitely better. I just didn't want to make the surface of the change bigger - but I guess it makes sense to do it now because someone else will step on this again :). I will push the commit in few minutes.

[fschrempf]

Even better: count() already exists.

@DanielNovak Thanks! Looks good to me now!

[IoTThinks]

I agree we should use the new getOutboundTotal() as it is easier to understand.

However, countBefore(0xFFFFFFFF) should return the same value as getOutboundTotal();
Using another method does not fix the bug in the method countBefore itself.

I suggest to use getOutboundTotal() and to fix the countBefore(0xFFFFFFFF) to return _num too.

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
    n++;
  }
  return n;
}

Have a nice day.

[DanielNovak]

@IoTThinks I added the handling of 0xFFFFFFFF to the original countBefore.

Btw. these functions are great candidates for unit tests - this would prevent these regression as we would have tests documenting and testing some of the specific behaviour.

[IoTThinks]

Thanks a lot for quick fix.

I hope this PR to get merged soon.

Have a nice weekend.