Repository navigation
feat(notifications): Live Update for mesh status or a tracked node - #7664
jamesarich wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Tests Prove The Path, Not The End State |  |
Two added tests still do not prove the production path. LiveUpdateGuardTest.the firmware flow deselecting the radio keeps the live update seeds FakeLiveUpdatePrefs with MeshStatus, runs the guar… |
Update the firmware guard test to retain the assertion that firmware deselection and reselection preserve MeshStatus, then release the firmware lock, perform a real user radio switch, and assert that the mode becomes Off. Update the pre… |
| Regression Coverage For Changed Behavior | |
Regression coverage remains incomplete. The changed production paths have only partial happy-path coverage: - createTrackedNodeLiveUpdate adds the <60s chip suppression in lines 956-985. `MeshNoti… |
Add the missing common, Android host, and Compose tests. In particular, add the pre-fix-failing sub-minute chip boundary test; notification formatting, disconnected-state, locale, and deep-link assertions; virtual-time throttle tests for me… |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding Live Update support for mesh status and tracked nodes. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Sibling Call Sites And Presence Semantics PASS. The PR does not change a physical metric field or its sentinel representation. It adds a new tracked-node notification call site that uses the existing nullable Node.snrOrNulland `Node.rssiOr…Moved Code Diffed Against Its Original PASS: The PR does not move a type or function to another file or module. The only extraction is in MeshNotificationManagerImpl: the old state-rendering block from base lines 430-443 is now `renderSe…Full details: Sibling Call Sites And Presence Semantics
-
Explanation
PASS. The PR does not change a physical metric field or its sentinel representation. It adds a new tracked-node notification call site that uses the existing nullable
Node.snrOrNullandNode.rssiOrNullaccessors and checks presence before rendering.NodeItem.ktandNodeItemCompact.ktare unchanged, and no new RSSI, temperature, current, voltage, or particulate field defaults to 0.
Full details: Moved Code Diffed Against Its Original
-
Explanation
PASS: The PR does not move a type or function to another file or module. The only extraction is in
MeshNotificationManagerImpl: the old state-rendering block from base lines 430-443 is nowrenderServiceStateNotificationat head lines 535-553. The block retains the same stats selection, fallback, notification construction, return type, and exception behavior; onlytitlebecomes a helper parameter.renderServiceNotificationremains the same declaration, and its existing caller remains unchanged. The other deletions only update a test placeholder, a baseline ID, and theManagementActionssignature/call. No moved-code override, validation, default, nullability, annotation, scope, dispatcher, or sharing-policy change is present.
-
Full details: Tests Prove The Path, Not The End State
-
Explanation
Two added tests still do not prove the production path.
LiveUpdateGuardTest.the firmware flow deselecting the radio keeps the live updateseedsFakeLiveUpdatePrefswithMeshStatus, runs the guard, and only checks that the value remains unchanged at lines 100-110. Removingguard().run()makes it pass.LiveUpdatePrefsTest.each mode round tripschecks the sameStateFlowimmediately aftersetModeat lines 67-76. A setter that updates only memory and does not persist to DataStore would pass. The candidate tests assert exact surviving node IDs, and no added test asserts emission order underUnconfinedTestDispatcher.Resolution
Update the firmware guard test to retain the assertion that firmware deselection and reselection preserve
MeshStatus, then release the firmware lock, perform a real user radio switch, and assert that the mode becomesOff. Update the preference test to await the DataStore write, construct a newLiveUpdatePrefsImplover the same DataStore file, and assert that the new instance decodes the stored mode. Cover all supported modes or inspect the persisted DataStore entry directly.
Full details: Regression Coverage For Changed Behavior
-
Explanation
Regression coverage remains incomplete. The changed production paths have only partial happy-path coverage: -
createTrackedNodeLiveUpdateadds the<60schip suppression in lines 956-985.MeshNotificationManagerImplTestcovers only a named node with a battery and deletion. It lacks a pre-fix-failing boundary test for no battery withlastHeard < 60sandlastHeard >= 60s. It also lacks assertions for signal, heard text, direct/hops, distance and locale units, powered battery, title fallback, disconnected mesh-status text, and both node and Connections deep-link intents. -liveUpdateReadingsappliesthrottleLatestto mesh and tracked-node flows at lines 386-410, andthrottleLatestemits immediately then coalesces for two seconds at lines 1512-1516. No test advances virtual time or verifies immediate delivery, latest-value delivery, and suppression of intermediate posts for either mode. -LiveUpdateGuard.watchSelectionuses valid-address-to-null and valid-address-to-new-radio transitions, and checks both firmware operations in lines 60-69 ofLiveUpdateGuard.kt. The tests cover only a helper for switching and an integration case forFirmwareUpdate; they do not cover valid-to-null, valid-to-new-radio throughrun(),FirmwareMaintenance, or the dropped-link preservation case across those transitions. -LiveUpdatePrefsImplpersistsoff,mesh, andnode:<num>through DataStore at lines 36-46.LiveUpdatePrefsTestround-trips one instance and tests decoder inputs, but does not re-instantiate the implementation against the same store or assert the raw persisted encodings. -rememberLiveUpdateSupportgates API 36+, refreshes promotion permission on resume, and falls back between promotion and app notification settings at lines 33-62 ofLiveUpdateSupport.android.kt. The diff adds no tests for API 35/36, resume refresh, or either settings fallback branch. -LiveUpdateSectionandLiveUpdateChooserSheetadd connected/disconnected visibility, Off/active state, support gating, Start/Switch/Stop, search, selection, and settings behavior at lines 80-265 ofLiveUpdateCard.kt. No Compose interaction tests cover these paths.NodeDetailScreenandManagementActionsadd the tracking switch wiring, but no UI test covers it. -LiveUpdateViewModelandNodeLiveUpdateViewModeladd state-flow and mode-transition behavior at the cited new files. No tests cover candidate/tracked-node updates, tracking, stopping only the matching node, or switching to another node. The existing candidate-ordering, preference-decoder, guard-helper, basic notification, and basic node deletion tests do not close these gaps.Resolution
Add the missing common, Android host, and Compose tests. In particular, add the pre-fix-failing sub-minute chip boundary test; notification formatting, disconnected-state, locale, and deep-link assertions; virtual-time throttle tests for mesh and tracked-node readings; full
LiveUpdateGuard.run()integration coverage for null, radio-switch,FirmwareUpdate, andFirmwareMaintenancetransitions; DataStore re-instantiation and raw encoding assertions; Robolectric tests for API gates, resume refresh, and settings fallback; Compose tests for Connections and node-detail interactions; and ViewModel tests for state updates and mode transitions.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
99a13f2 to
0e458fa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: meshtastic/Meshtastic-Android/.coderabbit.yaml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5d06896e-d459-4597-a695-15b43c546f84
📒 Files selected for processing: 29
.skills/compose-ui/strings-index.txtcore/prefs/src/commonMain/kotlin/org/meshtastic/core/prefs/notification/LiveUpdatePrefsImpl.ktcore/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/LiveUpdatePrefsTest.ktcore/repository/src/commonMain/kotlin/org/meshtastic/core/repository/LiveUpdatePrefs.ktcore/resources/src/commonMain/composeResources/values/strings.xmlcore/service/src/androidHostTest/kotlin/org/meshtastic/core/service/MeshNotificationManagerImplConversationTest.ktcore/service/src/androidHostTest/kotlin/org/meshtastic/core/service/MeshNotificationManagerImplRoutingTest.ktcore/service/src/androidHostTest/kotlin/org/meshtastic/core/service/MeshNotificationManagerImplTest.ktcore/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshNotificationManagerImpl.ktcore/service/src/commonMain/kotlin/org/meshtastic/core/service/LiveUpdateGuard.ktcore/service/src/commonTest/kotlin/org/meshtastic/core/service/LiveUpdateGuardTest.ktcore/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeLiveUpdatePrefs.ktcore/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/LiveUpdateSupport.android.ktcore/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/LiveUpdateSupport.ktcore/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/LiveUpdateSupport.ios.ktcore/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/LiveUpdateSupport.jvm.ktdocs/en/user/connections.mddocs/en/user/notifications.mdfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/LiveUpdateViewModel.ktfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.ktfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/components/LiveUpdateCard.ktfeature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/LiveUpdateCandidatesTest.ktfeature/node/detekt-baseline.xmlfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/DeviceActions.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/NodeDetailContent.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreens.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/NodeLiveUpdateViewModel.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/model/NodeDetailAction.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/navigation/NodesNavigation.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| is LiveUpdateMode.TrackNode -> { | ||
| val nodes = nodeRepository.value | ||
| combine(nodes.nodeDBbyNum.map { it[mode.nodeNum] }.distinctUntilChanged(), nodes.ourNodeInfo) { | ||
| node, | ||
| ourNode, | ||
| -> | ||
| node?.let { LiveUpdateReading.TrackNode(it, ourNode) } | ||
| } | ||
| .throttleLatest(LIVE_UPDATE_MIN_INTERVAL) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The tracked-node "last heard" text and chip stop aging after the link drops.
createTrackedNodeLiveUpdate computes heardSecondsAgo and the heardText string once per render. A render happens only when the TrackNode flow emits a new reading. That flow emits only when the tracked node's nodeDBbyNum row or ourNodeInfo changes. After a dropped link, neither flow changes. The PR keeps the Live Update running in that state.
- Trigger: Track a node that was heard seconds ago. Then lose the link, for example by walking out of BLE range.
- Consequence: The promoted notification keeps "Heard just now" indefinitely. If the node reports no battery, the status-bar chip also keeps its first elapsed time, such as "5m". The text tells the user that a node is fresh when it may be hours old. This contradicts the PR's stated behavior for a dropped link: keep the update and show the connection state.
- No other refresh source:
setShowWhen(false)disables the platform timestamp, so the system does not refresh the time either. The PR description lists timer-based aging after a dropped link as not observed.
Add a periodic tick to the TrackNode reading so that the reading re-emits at least once per minute. An alternative is to carry a render timestamp in the reading.
🐛 Proposed fix: re-render the tracked node once per minute
--- "a/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshNotificationManagerImpl.kt"
+++ "b/core/service/src/androidMain/kotlin/org/meshtastic/core/service/MeshNotificationManagerImpl.kt"
@@ -399,16 +399,21 @@
.throttleLatest(LIVE_UPDATE_MIN_INTERVAL)
}
is LiveUpdateMode.TrackNode -> {
val nodes = nodeRepository.value
- combine(nodes.nodeDBbyNum.map { it[mode.nodeNum] }.distinctUntilChanged(), nodes.ourNodeInfo) {
+ combine(
+ nodes.nodeDBbyNum.map { it[mode.nodeNum] }.distinctUntilChanged(),
+ nodes.ourNodeInfo,
+ minuteTicks(),
+ ) {
node,
ourNode,
+ tick,
->
- node?.let { LiveUpdateReading.TrackNode(it, ourNode) }
+ node?.let { LiveUpdateReading.TrackNode(it, ourNode, tick) }
}
.throttleLatest(LIVE_UPDATE_MIN_INTERVAL)
}
}
/**// LiveUpdateReading.TrackNode gains `val tick: Long` so distinctUntilChanged() lets each tick through.
private fun minuteTicks(): Flow<Long> = flow {
var n = 0L
while (true) {
emit(n++)
delay(1.minutes)
}
}
A user-started Android 16 Live Update on the service notification, the counterpart of the Apple Live Activity. Off by default; Off leaves the service notification as it was. - Mesh status: connection, nodes online, unread messages and the radio's battery, with nodes online in the status bar chip. - Track a node: its battery, signal, last heard, hops and distance, with its battery (or time since heard) in the chip. - The mode is stored in DataStore (`LiveUpdatePrefs`). A user disconnect, a radio switch, or the tracked node being deleted or ignored turns it off; a dropped link keeps it and shows the connection state. - Connections gets a Live Update row and chooser (favorites first, then most recently heard, searchable); node detail gets a Track in Live Update switch. Both are Android 16+ only.
…a minute ago It read `0m`.
0e458fa to
5aa96d9
Compare
This branch has not been deployed
1 participant
iOS now shows a Live Activity the user picks (Meshtastic-Apple#2670); Android had only the fixed service notification. This is the Android 16 counterpart: a Live Update the user starts, switches or stops, showing either the mesh at a glance or one node. It's off by default, and Off leaves the service notification as it was.
🌟 New Features
online/totalwhile connected.🛠️ Refactoring & Architecture
LiveUpdatePrefs(commonMain),LiveUpdateGuardincore:servicecommonMain, and arememberLiveUpdateSupport()expect incore:ui.renderServiceNotificationmoved unchanged intorenderServiceStateNotification.feature/node/detekt-baseline.xml: the ID of the existingViewModelForwardingentry changed with the call's arguments.📝 Documentation
Testing Performed
MeshNotificationManagerImplTest: not promoted while Off; Mesh status promoted with chip and text, and reverts on Off; firmware progress outranks it; a tracked node renders, then turns Off when deleted.LiveUpdateGuardTest,LiveUpdatePrefsTest,LiveUpdateCandidatesTest.kmpSmokeCompileand-PwarningsAsErrors=truegreen locally.PROMOTED_ONGOING; the status bar chip shows41/41. Stopping from node detail restores the stock notification. Dropping the link keeps the Live Update, with "Disconnected" as subtext.nodes/<num>for a tracked node.Constitution Check
stringResource,MeshtasticIcons,MetricFormatter; platform gate through an expect.features/directory and no Live Activity spec, so there is none to reference.connections.mdandnotifications.mdupdated,last_updatedbumped.Summary by CodeRabbit
New Features
Documentation