refactor: extract testable MeshTrafficGate; MeshTrafficMonitor singleton - #2218
Open
garthvh wants to merge 1 commit into
Open
refactor: extract testable MeshTrafficGate; MeshTrafficMonitor singleton#2218garthvh wants to merge 1 commit into
garthvh wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe pull request replaces the accessory-local traffic monitor with a shared packet-driven monitor, adds hysteresis/debounce gating and tests, and updates connection lifecycle handling to record inbound packets and reset monitoring during teardown. ChangesMesh traffic monitoring
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AccessoryManager
participant MeshTrafficMonitor
participant MeshTrafficGate
participant DecayTimer
AccessoryManager->>MeshTrafficMonitor: recordInboundPacket()
MeshTrafficMonitor->>MeshTrafficGate: evaluate averaged packet rate
MeshTrafficMonitor->>DecayTimer: schedule rolling-window decay
DecayTimer->>MeshTrafficMonitor: re-evaluate after packets stop
MeshTrafficMonitor-->>AccessoryManager: update isHighTraffic
AccessoryManager->>MeshTrafficMonitor: reset() on closeConnection()
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Docs staleness warningThis PR modifies user-facing Swift source files but does not update any page under Changed source files: What to check:
If this PR does not require a doc update (e.g., internal refactor, bug fix, test change), add the After updating |
Finish an in-progress refactor of the map's trace-route-flyover traffic gate: - Extract the hysteresis + debounce decision into a pure, value-typed MeshTrafficGate state machine (clock-free, unit-tested in MeshTrafficGateTests). - Rework MeshTrafficMonitor as a main-actor singleton (MeshTrafficMonitor.shared) that self-starts its decay timer on the first inbound packet, replacing the earlier EWMA-counter + explicit start()/stop() model. - Relocate the file from Accessory Manager/ to Helpers/ and update the AccessoryManager call sites: record() -> recordInboundPacket(), stop() -> reset(), and drop the explicit start() (it auto-starts now). - Remove the old MeshTrafficMonitorTests, which tested the removed record()/sample()/watermark API; the debounce + hysteresis behaviour it covered now lives in MeshTrafficGateTests against the extracted gate. Two copies of MeshTrafficMonitor.swift had been coexisting — an untracked WIP in Helpers/ plus the committed one in Accessory Manager/ — producing duplicate .stringsdata output that broke every build; this consolidates onto the new version.
garthvh
force-pushed
the
refactor/mesh-traffic-gate
branch
from
July 27, 2026 21:45
ce7d50b to
3f1e227
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the map trace-route flyover traffic gating by extracting a deterministic, testable hysteresis/debounce state machine (MeshTrafficGate) and reworking MeshTrafficMonitor into a main-actor singleton that self-starts on first inbound packet (while removing the duplicated monitor file that was breaking builds).
Changes:
- Added
MeshTrafficGate(pure value-type) and new unit tests covering debounce + hysteresis behavior. - Replaced the prior EWMA/sampling-timer
MeshTrafficMonitorwith a windowed rolling-rate monitor that self-manages its decay timer and exposes a coarseisHighTrafficflag. - Updated
AccessoryManagercall sites to use the singleton and the new APIs (recordInboundPacket(),reset()), and removed the old monitor implementation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| MeshtasticTests/MeshTrafficGateTests.swift | Adds Swift Testing coverage for the new gate’s debounce and hysteresis rules. |
| Meshtastic/Helpers/MeshTrafficMonitor.swift | Introduces the new MeshTrafficGate + singleton MeshTrafficMonitor implementation and timer-driven decay. |
| Meshtastic/Accessory/Accessory Manager/MeshTrafficMonitor.swift | Removes the previous EWMA + explicit start/stop monitor implementation. |
| Meshtastic/Accessory/Accessory Manager/AccessoryManager+Connect.swift | Drops explicit monitor start() call and updates explanatory comments. |
| Meshtastic/Accessory/Accessory Manager/AccessoryManager.swift | Switches to MeshTrafficMonitor.shared, updates packet-recording call, and resets monitor on disconnect. |
Comment on lines
+98
to
+101
| private var decayTimer: Timer? | ||
|
|
||
| private init() {} | ||
|
|
Comment on lines
+132
to
+133
| let rate = Double(timestamps.count) / Self.windowSeconds | ||
| packetsPerSecond = rate |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
Meshtastic/Helpers/MeshTrafficMonitor.swift:154
- The decay timer is scheduled on
RunLoop.main, but the callback currently spawns a newTaskevery tick just to hop back to@MainActor. This adds unnecessary task allocation/latency and can also make timer-driven decay less predictable under load.
Since the timer already fires on the main run loop, use MainActor.assumeIsolated (as the previous implementation did) to call evaluate(now:) directly without creating a new task each time.
let timer = Timer(timeInterval: Self.tickInterval, repeats: true) { [weak self] _ in
// Fires on the main run loop; hop to the main actor to touch isolated state.
Task { @MainActor in self?.evaluate(now: CACurrentMediaTime()) }
}
Comment on lines
+123
to
+130
| private func evaluate(now: TimeInterval) { | ||
| // Age packets out of the trailing window. | ||
| let cutoff = now - Self.windowSeconds | ||
| if let firstFresh = timestamps.firstIndex(where: { $0 >= cutoff }) { | ||
| if firstFresh > 0 { timestamps.removeFirst(firstFresh) } | ||
| } else { | ||
| timestamps.removeAll(keepingCapacity: true) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Finishes an in-progress refactor of the map's trace-route-flyover traffic gate. The branch had been left with two coexisting copies of
MeshTrafficMonitor.swift— an untracked WIP underHelpers/plus the committed one underAccessory Manager/— which produced duplicate.stringsdataoutput and broke the build on every platform.Changes
MeshTrafficGatestate machine. It's clock-free (callers passnow), so it's deterministically unit-testable.MeshTrafficMonitoras a main-actor singleton (MeshTrafficMonitor.shared) that self-starts its decay timer on the first inbound packet, replacing the earlier EWMA-counter + explicitstart()/stop()model.Accessory Manager/toHelpers/and update theAccessoryManagercall sites:record()→recordInboundPacket(),stop()→reset(), and drop the now-unnecessary explicitstart().MeshTrafficGateTestscovering: a brief spike is ignored (debounce), a sustained high rate trips only after the debounce window, and the flag doesn't chatter around the threshold (hysteresis).Testing
MeshTrafficGateTestsrun green on the iOS 26.5 simulator.Summary by CodeRabbit
Bug Fixes
Tests