Add tvOS app: TCP mesh map with Bonjour discovery - #2186
Open
garthvh wants to merge 19 commits into
Open
Conversation
New 'Meshtastic TV' target — a thin, independent Apple TV client that connects to a node over TCP and shows a live mesh map operable with the Siri Remote. No Bluetooth (sidesteps BLE pairing on a remote); a plugged-in TV on Wi-Fi is a natural TCP client. - Reuses the portable transport core (TCPConnection / Connection) by file membership plus the MeshtasticProtobufs package; does not link the iOS AccessoryManager / SwiftData / BLE / CoreLocation stack. AccessoryError, the want-config nonces and TransportType are small tvOS-local shims. - MeshClient runs a minimal want-config / want-database handshake and keeps an in-memory node store from the FromRadio stream (NodeInfo + Position); no persistence in v1. - MeshTVMapView is a fresh MKMapView wrapper (Apple basemap) with annotation diffing, centroid framing and focus / side-list selection. No user-location features (no GPS on tvOS). - Bonjour discovery (NodeDiscovery) browses _meshtastic._tcp so nodes appear in the connect list without typing an IP; manual host/port entry remains. - tvOS Brand Assets app icon. iOS target is unchanged (additive project regeneration only).
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds a tvOS Meshtastic application with a configured Xcode target, TCP client, Bonjour discovery, connection flow, live node map, node details, and tvOS asset catalogs. ChangestvOS Mesh Application
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
- Bring the Meshtastic brand into the TV app: m-logo imagesets and the LightIndigo green from the widgets, plus the iOS app's indigo AccentColor. Logo hero on the connect screen, logo + wordmark header on the map sidebar, brand-green tint throughout. - Regenerate the app icon and top shelf images as the brand-green M on dark, matching the iOS icon rather than a guessed palette. - Replace default marker pins with iOS-parity node pins: a circle colored by UIColor(hex: nodeNum) with the short name inside (CircleText + Color.swift reused by target membership), focus-scaled for the Siri Remote; clusters render as accent circles with a member count. Cluster select zooms in. - Node list rows and node detail reuse the same circle badges; battery reads in brand green. Fix the discovered-node port rendering with a thousands separator (4,403 -> 4403).
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new standalone tvOS application target (Meshtastic TV) that connects to a Meshtastic node over TCP, discovers nodes via Bonjour (_meshtastic._tcp), and renders a live mesh map with Siri Remote-friendly selection and a node detail view—while intentionally avoiding the iOS BLE/AccessoryManager/SwiftData stack.
Changes:
- Added a new tvOS app target + scheme and wired it into the Xcode project.
- Implemented a minimal TCP mesh client (handshake + in-memory node store) and Bonjour discovery for connect flows.
- Added a tvOS MapKit-backed map UI with focus/selection syncing, plus node list + node detail views.
Reviewed changes
Copilot reviewed 33 out of 49 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| project.yml | Adds the Meshtastic TV tvOS target, reusing portable TCP transport primitives and Logger. |
| Meshtastic.xcodeproj/xcshareddata/xcschemes/Meshtastic TV.xcscheme | Adds a shared Xcode scheme for running/building the new tvOS app. |
| Meshtastic.xcodeproj/project.pbxproj | Project regeneration to include the new tvOS target, scheme integration, and file memberships. |
| Meshtastic TV/Views/NodeDetailView.swift | Minimal tvOS node detail view (identity/status/position). |
| Meshtastic TV/Models/MeshNode.swift | Lightweight in-memory node model used by the tvOS client/UI. |
| Meshtastic TV/Map/MeshTVMapView.swift | MKMapView wrapper with annotation diffing, initial framing, and selection sync. |
| Meshtastic TV/Info.plist | tvOS Info.plist including Local Network + Bonjour service declarations. |
| Meshtastic TV/Client/TransportType.swift | tvOS-local transport discriminator shim for reused transport code. |
| Meshtastic TV/Client/NodeDiscovery.swift | Bonjour (NetServiceBrowser) discovery for _meshtastic._tcp nodes. |
| Meshtastic TV/Client/MeshClient.swift | Minimal TCP client with want-config handshake and node/position ingestion. |
| Meshtastic TV/Client/AccessoryError.swift | tvOS-local shim to satisfy reused transport error references. |
| Meshtastic TV/Assets.xcassets/Contents.json | Adds the tvOS asset catalog container. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.json | Top Shelf image asset metadata for tvOS. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.json | Wide Top Shelf image asset metadata for tvOS. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.json | Brand assets catalog metadata (app icon + top shelf images). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.json | App icon imagestack layer metadata (front). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.json | App icon front layer image set metadata. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.json | App icon imagestack metadata (front/back layers). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.json | App icon imagestack layer metadata (back). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.json | App icon back layer image set metadata. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Front.imagestacklayer/Contents.json | App Store icon imagestack layer metadata (front). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Front.imagestacklayer/Content.imageset/Contents.json | App Store icon front layer image set metadata. |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.json | App Store icon imagestack metadata (front/back layers). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.json | App Store icon imagestack layer metadata (back). |
| Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.json | App Store icon back layer image set metadata. |
| Meshtastic TV/Assets.xcassets/AccentColor.colorset/Contents.json | Adds tvOS accent color definition. |
| Meshtastic TV/App/RootView.swift | Root state switcher between connect/connecting/map screens. |
| Meshtastic TV/App/MeshtasticTVApp.swift | tvOS app entry point (@main) wiring the client into the root view. |
| Meshtastic TV/App/MapScreen.swift | Main tvOS map screen: side list + map + node detail navigation + disconnect action. |
| Meshtastic TV/App/ConnectView.swift | Connect UI: Bonjour-discovered nodes + manual host/port entry + error display. |
Comment on lines
+87
to
+89
| private var port: Int { | ||
| Int(portText.trimmingCharacters(in: .whitespaces)) ?? 4403 | ||
| } |
Comment on lines
+46
to
+50
| func stop() { | ||
| browser.stop() | ||
| resolving.removeAll() | ||
| isBrowsing = false | ||
| } |
Comment on lines
+60
to
+62
| func netServiceBrowser(_ browser: NetServiceBrowser, didRemove service: NetService, moreComing: Bool) { | ||
| discovered.removeAll { $0.id == Self.identity(for: service) || $0.name == service.name } | ||
| } |
Comment on lines
+188
to
+192
| // Remove annotations for nodes that are gone. | ||
| for (num, annotation) in annotationsByNum where !incoming.contains(num) { | ||
| mapView.removeAnnotation(annotation) | ||
| annotationsByNum[num] = nil | ||
| } |
Comment on lines
+84
to
+87
| } catch { | ||
| Logger.transport.error("📺 [MeshClient] connect failed: \(error.localizedDescription, privacy: .public)") | ||
| self.state = .failed(error.localizedDescription) | ||
| } |
Comment on lines
+668
to
+670
| # Node-pin styling parity with the iOS map (CircleText + Color(hex:)/isLight). | ||
| - path: "Meshtastic/Views/Helpers/CircleText.swift" | ||
| - path: "Meshtastic/Extensions/Color.swift" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Meshtastic TV/Client/NodeDiscovery.swift (1)
26-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
NWBrowserinstead of the deprecatedNetServiceBrowser/NetService.This is new code built on Foundation's legacy Bonjour APIs, while the rest of the client (
TCPConnection) already usesNetwork.framework. Apple DTS has confirmed NetServiceBrowser has been deprecated in favour of NWBrowser. Consolidating discovery ontoNWBrowserwould keep the whole client on one modern, async-friendly networking stack.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic` TV/Client/NodeDiscovery.swift around lines 26 - 95, Replace the legacy NetServiceBrowser/NetService implementation in NodeDiscovery with Network.framework’s NWBrowser, matching the existing TCPConnection networking stack. Preserve the current published discovery state, start/stop lifecycle, Meshtastic Bonjour service type, TXT-derived node identity/name, resolved host and port, deduplication, and removal behavior while adapting callbacks to NWBrowser’s async result updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic` TV/Client/MeshClient.swift:
- Around line 40-41: Track the outer asynchronous connection attempt in a
dedicated property alongside connection and consumeTask, and cancel it from
disconnect() and before starting a new connect(). In connect(), guard completion
and subsequent assignment of connection/consumeTask so a stale attempt cannot
overwrite state created by a newer attempt; ensure any stale TCPConnection is
closed. Clear the tracked connect task when the current attempt finishes.
---
Nitpick comments:
In `@Meshtastic` TV/Client/NodeDiscovery.swift:
- Around line 26-95: Replace the legacy NetServiceBrowser/NetService
implementation in NodeDiscovery with Network.framework’s NWBrowser, matching the
existing TCPConnection networking stack. Preserve the current published
discovery state, start/stop lifecycle, Meshtastic Bonjour service type,
TXT-derived node identity/name, resolved host and port, deduplication, and
removal behavior while adapting callbacks to NWBrowser’s async result updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3637ed8f-a375-4e19-8bf1-12703b51120b
⛔ Files ignored due to path filters (10)
Meshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/back_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Front.imagestacklayer/Content.imageset/front_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/back_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/back_2x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/front_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/front_2x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/tsw_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/tsw_2x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/ts_1x.pngis excluded by!**/*.pngMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/ts_2x.pngis excluded by!**/*.png
📒 Files selected for processing (30)
Meshtastic TV/App/ConnectView.swiftMeshtastic TV/App/MapScreen.swiftMeshtastic TV/App/MeshtasticTVApp.swiftMeshtastic TV/App/RootView.swiftMeshtastic TV/Assets.xcassets/AccentColor.colorset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Front.imagestacklayer/Content.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Front.imagestacklayer/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.jsonMeshtastic TV/Assets.xcassets/Contents.jsonMeshtastic TV/Client/AccessoryError.swiftMeshtastic TV/Client/MeshClient.swiftMeshtastic TV/Client/NodeDiscovery.swiftMeshtastic TV/Client/TransportType.swiftMeshtastic TV/Info.plistMeshtastic TV/Map/MeshTVMapView.swiftMeshtastic TV/Models/MeshNode.swiftMeshtastic TV/Views/NodeDetailView.swiftMeshtastic.xcodeproj/project.pbxprojMeshtastic.xcodeproj/xcshareddata/xcschemes/Meshtastic TV.xcschemeproject.yml
The 'crazy static' while moving between nodes was tvOS focus/selection sounds machine-gunning, from three compounding churn sources: - Every data tick reconfigured every annotation view, and reassigning clusteringIdentifier makes MapKit tear down and rebuild views under the focus engine. Now the identifier is set only when an annotation is (re)assigned, content updates are cheap and idempotent, and the sync diff mutates nothing when nothing changed. - tvOS List selection follows focus, so gliding across the side list fired an animated selectAnnotation (callout + selection sound) per row. Now browsing centers the map only, only when the selection actually changed; callouts appear only when a pin itself is clicked. - MeshClient republished the node store for every packet even when nothing visible changed. NodeInfo upserts now skip no-op writes, and position packets only publish when the fix moved or lastHeard is >60s stale. Also adds a Re-center button next to Disconnect that re-frames the camera on the whole mesh (clearing any list selection), with a neutral tint so the label stays readable against the brand-green tint.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
Meshtastic TV/Map/MeshTVMapView.swift:205
annotationsByNumis mutated (annotationsByNum[num] = nil) while iterating over it, which can trap at runtime ("Dictionary mutated while being enumerated") when nodes disappear. Collect keys to remove first, then mutate the dictionary in a second step.
project.yml:670- This target pulls in
Meshtastic/Views/Helpers/CircleText.swift, which currently importsSwiftData. SwiftData is not available on tvOS, so this will fail to compile the new tvOS target unlessCircleText.swiftdrops that import (it appears unused) or gates it with#if canImport(SwiftData)/ a tvOS-specific implementation.
Comment on lines
+92
to
+98
| let color = UIColor(hex: node.num) | ||
| let text = node.shortName.isEmpty ? String(format: "%04x", node.num & 0xffff) : node.shortName | ||
| if circle.backgroundColor != color { circle.backgroundColor = color } | ||
| if label.text != text { | ||
| label.text = text | ||
| label.textColor = color.isLight() ? .black : .white | ||
| } |
Comment on lines
+60
to
+70
| func connect(host: String, port: Int) { | ||
| disconnect() | ||
| self.host = host | ||
| state = .connecting | ||
| nodes = [:] | ||
| myNodeNum = nil | ||
|
|
||
| Task { | ||
| do { | ||
| let conn = try await TCPConnection(host: host, port: port) | ||
| self.connection = conn |
Centering alone at mesh-wide zoom left a list-selected node swallowed by its cluster badge. Selection now zooms to city scale (4 km) centered on the node so it breaks out of the cluster — and only ever zooms IN: when the user is already closer than that, keep their zoom and just center.
…on fly The remaining static on list transitions came from the zoom animation itself: every animated region change re-evaluates clustering, and each cluster/pin view added or removed during the flight was a focusable element, so the focus engine re-evaluated (with its click sound) all the way through the animation. - Annotation views now opt out of focus entirely (canBecomeFocused false). The side list is the selection path, so pins never needed focus; the map itself still pans with the remote. The dead pin/cluster didSelect path is removed with it. - Selection flights are debounced (0.4s): gliding across list rows no longer launches an overlapping animated fly per row — the map flies once, to the row the user settles on.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (8)
Meshtastic TV/Client/MeshClient.swift:42
- MeshClient launches an untracked connection Task but there’s no stored handle to cancel it during
disconnect(), which can lead to state/connection mutations happening after the user cancels. Add aconnectTaskproperty so the connection attempt can be cancelled cleanly.
Meshtastic TV/Client/MeshClient.swift:71 connect(host:port:)uses a detachedTask { ... }that is not cancelled ondisconnect(). If the user cancels while the awaits are in-flight, the task can still setconnection/statelater (including flipping back to.failed). Store the task and short-circuit on cancellation.
Meshtastic TV/Client/MeshClient.swift:95disconnect()cancels the consume task but not the in-flight connection attempt. Ifconnect()is still awaitingTCPConnection(...)/connect(), it can complete after disconnect and re-mutate state. Cancel and nil outconnectTaskhere as well.
Meshtastic TV/Client/NodeDiscovery.swift:23- Using TXT-advertised node IDs as
Identifiable.idmakes add/update/remove inconsistent:didRemoveonly receives theNetService(service name), so entries whoseidcame from the TXT record won’t be removed, leaving stale discovered nodes. Useservice.nameas the stable identity (likeTCPTransportdoes), and keep the advertised node ID as a separate field if needed.
Meshtastic TV/Client/NodeDiscovery.swift:50 stop()clearsresolvingbut doesn’t actually cancel outstandingNetService.resolve(...)operations. Per Apple’s pattern (and this repo’sTCPTransport.cancelPendingResolutionsAndStopBrowsing()), a latenetServiceDidResolveAddresscallback can still arrive after stop and repopulatediscovered. Explicitly stop each pending service and clear its delegate before stopping the browser.
Meshtastic TV/Client/NodeDiscovery.swift:62didRemovecurrently tries to match by TXT-advertisedid/shortname, but those values may not equalNetService.name. WithDiscoveredNode.idstabilized toservice.name, removal can be deterministic; also remove the service from theresolvingset when it disappears.
Meshtastic TV/Client/NodeDiscovery.swift:78- After making
DiscoveredNode.ida stableservice.name,netServiceDidResolveAddressshould populate bothidandadvertisedIDconsistently and keep the update-keyed-by-id behavior intact (so a later resolve doesn’t create duplicates).
Meshtastic TV/Map/MeshTVMapView.swift:41 - The NodeCircleAnnotationView doc comment says the pin “is focus-engine selectable”, but
canBecomeFocusedis overridden tofalse. Update the comment so it matches the (non-focusable) behavior and rationale.
Comment on lines
+7
to
+11
| // Live mesh map with a focusable node side-list for Siri Remote operation. | ||
| // The list is the primary, reliable selection path on tvOS; clicking a pin on | ||
| // the map (focus engine) selects too and keeps the list in sync. Rows carry the | ||
| // same node-color circle badges as the map pins (CircleText, shared with iOS). | ||
| // |
Comment on lines
+7
to
+12
| // Fresh, minimal MKMapView wrapper for tvOS. Deliberately does NOT port the iOS | ||
| // `ClusterMapView` (welded to touch gestures, MKUserTrackingButton and SwiftData). | ||
| // No device GPS on tvOS, so `showsUserLocation` is off and we only plot the | ||
| // reported positions of other nodes. Selection is driven by the focus engine | ||
| // (clicking a pin) and by the side list via `selectedNodeNum`. | ||
| // |
The 'static' was visual: long animated region changes re-evaluate clustering every frame (pins and cluster badges popping in and out) while map tiles stream in behind — plus every pin paying an offscreen shadow pass per frame. Compounding it, annotation views were focusable, so swiping near the map walked focus into pins and fired flights the list never asked for. - setRegionSmart: CUT instead of animating when the jump is big (>80 km or >4x span change); short hops still animate. Applies to selection, re-center and initial framing. - Pre-computed shadowPath on node pins: no per-frame offscreen shadow passes during map animation. - Track list-row focus explicitly with @focusstate and mirror it into the map selection: List(selection:) does not follow focus for NavigationLink rows, and with pins now non-focusable this is the browse signal that drives the (debounced) map preview.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic` TV/Map/MeshTVMapView.swift:
- Around line 308-338: Update the selection handling around lastAppliedSelection
and the delayed pendingSelectionFly so a selection is not marked applied before
annotationsByNum contains the node. Track the selected number as pending,
reattempt it when the annotation becomes available, and assign
lastAppliedSelection only after setRegionSmart is invoked to center the
annotation; preserve the existing debounce and zoom behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eef489bc-bf41-4a1a-b383-e3f3e94fbe54
📒 Files selected for processing (2)
Meshtastic TV/App/MapScreen.swiftMeshtastic TV/Map/MeshTVMapView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Meshtastic TV/App/MapScreen.swift
MKMapView on tvOS captures directional input for panning and never releases focus on its own, so once focus entered the map there was no way back to the node list — and after opening a node detail, focus fell into the map, leaving Menu unable to pop the detail either. Claim the Menu press with a gesture recognizer on the map view: it pops any open node detail (NavigationStack path is now state-driven) and hands focus back to the node list via the tracked row FocusState. Menu/back now always returns to the list from anywhere on the map screen.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (8)
Meshtastic TV/Client/MeshClient.swift:1
connect()creates an untracked Task that can complete afterdisconnect()is called (e.g., when the user hits Cancel), reassignself.connection, startconsumeTask, and/or overwritestateafter the app believes it disconnected. Track the connect task (e.g.,private var connectTask: Task<Void, Never>?), cancel it indisconnect(), and add cancellation checks after eachawait(or use a monotonically increasing connection token /withTaskCancellationHandler) before mutatingconnection,consumeTask, orstate.
Meshtastic TV/Client/NodeDiscovery.swift:1DiscoveredNode.idis sometimes set from the TXT record"id"(line 75), but removal only matches againstservice.name(viaidentity(for:)) orname == service.name. This can leave stale discovered entries when a service disappears. Prefer makingDiscoveredNode.idconsistently equal toservice.name(and store the advertised node id separately), or maintain a mapping fromNetService(orservice.name) to the chosenDiscoveredNode.idsodidRemovecan remove the correct entry deterministically.
Meshtastic TV/Client/NodeDiscovery.swift:1DiscoveredNode.idis sometimes set from the TXT record"id"(line 75), but removal only matches againstservice.name(viaidentity(for:)) orname == service.name. This can leave stale discovered entries when a service disappears. Prefer makingDiscoveredNode.idconsistently equal toservice.name(and store the advertised node id separately), or maintain a mapping fromNetService(orservice.name) to the chosenDiscoveredNode.idsodidRemovecan remove the correct entry deterministically.
Meshtastic TV/Map/MeshTVMapView.swift:1- The map view currently can’t drive selection back into SwiftUI:
selectedNodeNumis only used to apply selection/centering, but there’s no delegate callback (e.g.,mapView(_:didSelect:)) updating the@Binding, and the annotation views are explicitly non-focusable. This conflicts with the stated behavior (“clicking a pin … selects too”) and makes pin-based selection effectively impossible on tvOS. Either (mandatory) implement a map-to-binding selection path (delegatedidSelect, or a custom selection gesture that identifies the annotation and setsparent.selectedNodeNum), or (optional) if list-only is intended, remove the pin-click selection claim from the surrounding comments/PR narrative and UI expectations.
Meshtastic.xcodeproj/project.pbxproj:1 - This introduces duplicate top-level groups with identical names (
Accessory,Extensions,Views) under theMeshtasticgroup, which will make project navigation and future file moves/error triage confusing. Consider placing the tvOS-reused shared files under theMeshtastic TVroot group (or a clearly named single “Shared” group) instead of adding parallel duplicate groups with the same display name.
Meshtastic.xcodeproj/project.pbxproj:1 - This introduces duplicate top-level groups with identical names (
Accessory,Extensions,Views) under theMeshtasticgroup, which will make project navigation and future file moves/error triage confusing. Consider placing the tvOS-reused shared files under theMeshtastic TVroot group (or a clearly named single “Shared” group) instead of adding parallel duplicate groups with the same display name.
Meshtastic.xcodeproj/project.pbxproj:1 - This introduces duplicate top-level groups with identical names (
Accessory,Extensions,Views) under theMeshtasticgroup, which will make project navigation and future file moves/error triage confusing. Consider placing the tvOS-reused shared files under theMeshtastic TVroot group (or a clearly named single “Shared” group) instead of adding parallel duplicate groups with the same display name.
Meshtastic TV/Client/MeshClient.swift:1 - The new in-memory ingestion and “no-op skip” logic is core behavior for the tvOS app (e.g., coalescing frequent position packets, node upserts, and connection cancellation behavior) but currently has no visible tests. Add unit tests around
MeshClientingestion (e.g., moved vs. stale update, correct coordinate scaling, node sorting) and (optionally)NodeDiscoveryTXT parsing/removal behavior to prevent regressions as protobuf/admin fields evolve.
The buttons floated in an overlay on the map's top-right corner — but with the map capturing focus for panning (and pins non-focusable), there was no focus path to reach them with the remote. They now live as the first section of the left list column, reachable like any row; the map surface is clean.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (10)
Meshtastic TV/Client/MeshClient.swift:1
@Observablerequires the Observation module; this file doesn’t import it, which will fail to compile. Addimport Observation(or another appropriate import that provides@Observable) alongside the existing imports.
Meshtastic TV/Client/MeshClient.swift:1@Observablerequires the Observation module; this file doesn’t import it, which will fail to compile. Addimport Observation(or another appropriate import that provides@Observable) alongside the existing imports.
Meshtastic TV/Views/NodeDetailView.swift:1UIColoris referenced but UIKit isn’t imported in this file, which will fail to compile. Addimport UIKit(or switch to a non-UIKit color constructor if you have one available).
Meshtastic TV/Views/NodeDetailView.swift:1UIColoris referenced but UIKit isn’t imported in this file, which will fail to compile. Addimport UIKit(or switch to a non-UIKit color constructor if you have one available).
Meshtastic TV/Client/MeshClient.swift:1- You’re marking the client
.connectedon anyconfigCompleteID, butconnect()sends two nonces (config + DB). This can transition to.connectedearly (after config completes) before the DB dump is finished. Track which nonce completed (bind the associated value from.configCompleteID(let nonce)if available) and only switch to.connectedafter the DB nonce (or after both nonces have completed).
Meshtastic TV/Client/MeshClient.swift:1 - If an error is thrown after
self.connection = conn(e.g., duringconnect()or eithersend), the failure path sets.failedbut doesn’t clean up the partially-established connection/stream. Consider clearingself.connectionand disconnecting theconnin the catch to avoid leaking an in-flight network connection and to ensure subsequent connects start from a clean state.
Meshtastic TV/Client/NodeDiscovery.swift:1 - When the TXT record provides an
id(soDiscoveredNode.id != service.name) andnameis a shortname (sonode.name != service.name),didRemovewon’t match and the stale discovered entry can remain. Fix by keying removal (and the list identity) on a value that is guaranteed to matchdidRemove—e.g., always useservice.name(or a composite ofname+type+domain) as the stable identifier and keep the Meshtastic node id as a separate field.
Meshtastic TV/Client/NodeDiscovery.swift:1 - When the TXT record provides an
id(soDiscoveredNode.id != service.name) andnameis a shortname (sonode.name != service.name),didRemovewon’t match and the stale discovered entry can remain. Fix by keying removal (and the list identity) on a value that is guaranteed to matchdidRemove—e.g., always useservice.name(or a composite ofname+type+domain) as the stable identifier and keep the Meshtastic node id as a separate field.
Meshtastic TV/Client/NodeDiscovery.swift:1 - When the TXT record provides an
id(soDiscoveredNode.id != service.name) andnameis a shortname (sonode.name != service.name),didRemovewon’t match and the stale discovered entry can remain. Fix by keying removal (and the list identity) on a value that is guaranteed to matchdidRemove—e.g., always useservice.name(or a composite ofname+type+domain) as the stable identifier and keep the Meshtastic node id as a separate field.
Meshtastic TV/Client/MeshClient.swift:1 - These top-level globals leak into the module namespace and make it harder to reason about ownership. Prefer moving them into
MeshClient(e.g., asprivate static letconstants or a private nested enum) so they’re scoped to the only consumer.
| // same node-color circle badges as the map pins (CircleText, shared with iOS). | ||
| // | ||
|
|
||
| import SwiftUI |
Comment on lines
+126
to
+130
| let node: MeshNode | ||
|
|
||
| var body: some View { | ||
| HStack(spacing: 16) { | ||
| // Same circle badge as the map pin (node color + short name). |
Comment on lines
+110
to
+117
| } | ||
| .padding(.horizontal, 32) | ||
| .padding(.vertical, 16) | ||
| } | ||
|
|
||
| private var disconnectButton: some View { | ||
| Button(role: .destructive) { | ||
| client.disconnect() |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
Meshtastic TV/Client/NodeDiscovery.swift:1
- The removal predicate is inconsistent with how
DiscoveredNodeitems are keyed/constructed. When TXT records provide an"id"and/or"shortname",DiscoveredNode.idmay not equalservice.name, andDiscoveredNode.namemay not equalservice.nameeither—so removed services can remain “stuck” indiscovered. Consider storing a stable service key (e.g.,service.name+service.type+service.domain) inDiscoveredNodeand using that for insert/update/remove, while keeping the advertised"id"as a separate display field.
Meshtastic TV/Client/NodeDiscovery.swift:1 isBrowsingis set totrueonstart(), but there’s no delegate handling for browse failures (e.g.,netServiceBrowser(_:didNotSearch:)), so the UI can remain in a perpetual “searching” state if the browse fails due to local network permission denial or other errors. AdddidNotSearch(and optionallynetServiceBrowserDidStopSearch) to clearisBrowsingand optionally publish an error state for the UI.
Meshtastic TV/Map/MeshTVMapView.swift:1- Region fitting via min/max longitude breaks for meshes spanning the antimeridian (e.g., nodes at +179° and -179°), producing an overly large span and incorrect center. Using
MKMapRectunioning viaMKMapPoint(and callingsetVisibleMapRect(_:edgePadding:animated:)) avoids dateline issues and tends to fit points more robustly.
Meshtastic.xcodeproj/project.pbxproj:1 - There are now two sibling PBXGroups both named “Accessory” under
Meshtastic, which makes navigation and future maintenance error-prone (it’s easy to add files to the wrong group). Consider renaming the tvOS-specific subset group (e.g., “Accessory (Transport Only)” / “AccessoryTransport”), or nesting it explicitly under a tvOS-related group to keep intent clear.
Comment on lines
+110
to
+117
| private var disconnectButton: some View { | ||
| Button(role: .destructive) { | ||
| client.disconnect() | ||
| } label: { | ||
| Label("Disconnect", systemImage: "xmark.circle.fill") | ||
| } | ||
| .buttonStyle(.bordered) | ||
| } |
Comment on lines
+8
to
+10
| // The list is the primary, reliable selection path on tvOS; clicking a pin on | ||
| // the map (focus engine) selects too and keeps the list in sync. Rows carry the | ||
| // same node-color circle badges as the map pins (CircleText, shared with iOS). |
… exists Resolve two CodeRabbit findings on the tvOS app. MeshClient: overlapping connect() calls raced — the outer Task was never stored, so disconnect() could not cancel it and a stale attempt awaiting the TCP handshake could clobber the connection/consumeTask a newer attempt set up, orphaning a TCPConnection. Track the attempt in connectTask, cancel it from disconnect() (and implicitly from connect()'s leading disconnect()), and guard the post-handshake assignment on !Task.isCancelled, tearing down the orphaned connection. Also guard the catch so a superseded attempt cancelled during connect()/send() can't clobber the newer attempt's state to .failed. MeshTVMapView: the list can select a node with no location, but the map only annotates located nodes. Marking lastAppliedSelection immediately meant that once the node gained a location its re-applied selection was skipped and never centered. Defer lastAppliedSelection until centering succeeds, track a separate pendingSelectionNum, and only schedule the fly once the annotation exists — while still cancelling a stale pending fly when focus moves to an unlocated node.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
Meshtastic TV/App/MapScreen.swift:13
- This file uses
UIColor(hex:)inNodeRowbut doesn’t import UIKit, which will fail to compile becauseUIColoris defined in UIKit (not SwiftUI).
import SwiftUI
Meshtastic TV/App/MapScreen.swift:10
- The header comment says clicking a map pin “keeps the list in sync”, but the current implementation only drives
selectedNodeNumfrom list focus (focusedNodeNum);MeshTVMapViewnever writes back toselectedNodeNum, andMapScreendoesn’t update focus from map selection. Update the comment or implement two-way selection syncing.
// Live mesh map with a focusable node side-list for Siri Remote operation.
// The list is the primary, reliable selection path on tvOS; clicking a pin on
// the map (focus engine) selects too and keeps the list in sync. Rows carry the
// same node-color circle badges as the map pins (CircleText, shared with iOS).
project.yml:675
- The tvOS target pulls in
Meshtastic/Views/Helpers/CircleText.swiftfor pin styling, but that file currently imports SwiftData (import SwiftData). This contradicts the PR description (“does not link … SwiftData”) and may break tvOS builds if SwiftData isn’t available/desired. Consider either (a) removing/conditionalizing the SwiftData import inCircleText.swift, or (b) adding a tvOS-localCircleTextimplementation instead of sharing the iOS file.
| // Minimal node detail. No distance-to-me / bearing (no device GPS on tvOS). | ||
| // | ||
|
|
||
| import SwiftUI |
Comment on lines
+24
to
+25
| // Meshtastic brand green (the Live Activity / widget tint). | ||
| .tint(Color("LightIndigo")) |
Replace the SF Rounded 'Meshtastic' text (Connect hero + map header) with the official outlined wordmark from meshtastic/design (typelogo), recoloured white via a root fill so every letter inherits it. The rounded system font read as too playful next to the brand mark.
Make cluster badges focusable with a focus highlight; selecting one zooms to fit its members so it breaks apart into individual pins. Selecting a node from the side list now zooms down tight (~800m) and animates, so a node inside a dense cluster (e.g. the 900-node sim) is actually visible rather than swallowed.
This app is a live wall display, so set isIdleTimerDisabled on appear and re-assert it on every active scene transition — the tvOS screensaver must not interrupt the mesh map.
Promote MeshNode to a @model and add a tvOS-local ModelContainer. MeshClient fetch-or-creates nodes into that context (change-guarded to avoid @query churn) instead of an in-memory dict; the map/list read via @query. The store is kept across relaunch, so the map is populated from the last session before the radio's node-DB dump completes. Deliberately NOT the iOS entities — NodeInfoEntity's relationship graph is one connected component of ~39 tables, so it can't be subsetted for a map.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 51 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
Meshtastic TV/Views/NodeDetailView.swift:10
NodeDetailViewreferencesUIColor(viaColor(UIColor(hex: ...))) but the file only importsSwiftUI.UIColorisn’t available without importing UIKit, which will cause a compile error for this target.
Meshtastic TV/App/MapScreen.swift:14- This file uses
UIColor(e.g.Color(UIColor(hex: node.num))) but doesn’t import UIKit, which will fail to compile.
import SwiftUI
import SwiftData
Meshtastic TV/Client/NodeDiscovery.swift:79
didRemovetries to remove discovered entries usingservice.name, butDiscoveredNode.idis currently set to the TXT-recordidwhen present. Whenidis present andshortnamediffers fromservice.name, the removed service may never be removed fromdiscovered(and may also duplicate on re-resolve). Usingservice.nameas the stable identifier fixes removal/deduping.
Meshtastic TV/Client/NodeDiscovery.swift:50stop()clears theresolvingset but does not cancel in-flightNetService.resolve(withTimeout:)operations. LatenetServiceDidResolveAddresscallbacks can still arrive after browsing stops / the view disappears and mutatediscoveredunexpectedly. The iOSTCPTransportexplicitly cancels pending resolutions by callingservice.stop()and clearing delegates.
Comment on lines
+84
to
+86
| Button { | ||
| client.connect(host: node.host, port: node.port) | ||
| } label: { |
Comment on lines
+19
to
+25
| init() { | ||
| // Slim tvOS-local store — just `MeshNode` (see MeshNode.swift). Persists the | ||
| // node database so the map is populated on relaunch before the radio re-dumps. | ||
| let container = try! ModelContainer(for: MeshNode.self) | ||
| self.container = container | ||
| _client = State(initialValue: MeshClient(context: container.mainContext)) | ||
| } |
Comment on lines
+20
to
+22
| // Slim tvOS-local store — just `MeshNode` (see MeshNode.swift). Persists the | ||
| // node database so the map is populated on relaunch before the radio re-dumps. | ||
| let container = try! ModelContainer(for: MeshNode.self) |
Add a Settings detail reachable from both the connect screen and the map's side list. Two options for now: - Clear Node Database: wipes the persisted MeshNode store; footer shows the live saved-node count. Nodes repopulate from the radio on next connect. - Map Type: Standard / Hybrid / Satellite, stored in the shared tv.mapType AppStorage key the map reads. Default-focuses the destructive Clear button so a Menu press can't drift onto the map behind it.
…t-zoom Several map-interaction fixes so the Siri Remote can always get back to the node list and stacked pins actually separate: - Make the map non-interactive (isUserInteractionEnabled = false) and the cluster views non-focusable, so the focus engine never traps the remote on the MKMapView — the list stays the reliable selection path. - Spread (near-)coincident nodes onto a small ring (ported from the iOS map's pre-disambiguation-sheet behaviour, sized up for the TV's larger pins) so a stubborn cluster breaks into individual pins once you zoom in. - Only ever zoom in on selection (selectionZoomMeters), never back out. - Apply the tv.mapType preference (Standard / Hybrid / Satellite) live.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 52 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (8)
Meshtastic TV/Views/NodeDetailView.swift:11
UIColor(hex:)is used here, but this file doesn't import UIKit, so it won't compile on tvOS. Addimport UIKit(or switch to a pure-SwiftUI color initializer if you prefer).
Meshtastic TV/App/MapScreen.swift:15UIColor(hex:)is referenced below, but UIKit isn't imported in this file, which will cause a compile error. Addimport UIKitalongside the other imports.
import SwiftUI
import SwiftData
Meshtastic TV/Map/MeshTVMapView.swift:11
- The header comment says map selection is driven by the focus engine (clicking a pin), but the implementation disables map interaction (
isUserInteractionEnabled = false) and pins/clusters are not focusable. This comment is misleading; either update the comment to match the current behavior or re-enable map interaction/focus if pin selection is intended.
Meshtastic TV/Map/MeshTVMapView.swift:41 - This comment says the annotation view is “focus-engine selectable”, but
canBecomeFocusedis overridden to returnfalsebelow, so pins are explicitly non-focusable. Consider updating the comment to avoid implying pin selection via focus is supported.
Meshtastic TV/App/MeshtasticTVApp.swift:25 - Using
try!for the SwiftData container will crash the app at launch if the container cannot be created (e.g. file-system or migration issues). Prefer handling the thrown error explicitly and failing with a clearer message (or showing an in-app error state).
init() {
// Slim tvOS-local store — just `MeshNode` (see MeshNode.swift). Persists the
// node database so the map is populated on relaunch before the radio re-dumps.
let container = try! ModelContainer(for: MeshNode.self)
self.container = container
_client = State(initialValue: MeshClient(context: container.mainContext))
}
Meshtastic TV/App/MapScreen.swift:10
- This file’s header comment says “clicking a pin” selects nodes, but MeshTVMapView disables map interaction and pins/clusters are non-focusable. Update this comment to match the current interaction model (side list drives selection), or re-enable pin selection if that’s still a goal.
// Live mesh map with a focusable node side-list for Siri Remote operation.
// The list is the primary, reliable selection path on tvOS; clicking a pin on
// the map (focus engine) selects too and keeps the list in sync. Rows carry the
// same node-color circle badges as the map pins (CircleText, shared with iOS).
Meshtastic.xcodeproj/project.pbxproj:645
- The
Meshtasticgroup also contains duplicateExtensionsgroups (both namedExtensions). This duplication can cause ambiguous file locations and makes future edits harder; consider consolidating to a single group and keeping only one set of file references.
Meshtastic.xcodeproj/project.pbxproj:662 - There are duplicate
Viewsgroups underMeshtastic(the newly addedA51EAC...and the existingD98377...). Consolidating these avoids a confusing project navigator structure and reduces the chance of accidentally adding files to the wrong group later.
Comment on lines
+167
to
+172
| /// Called on a Menu press while the map has focus. MKMapView captures the | ||
| /// directional input for panning and never releases focus on its own, so | ||
| /// without this the map is a focus trap (and an unhandled Menu press can | ||
| /// suspend the app instead of going back). MapScreen uses it to hand focus | ||
| /// back to the node list. | ||
| var onMenuExit: (() -> Void)? |
Comment on lines
+7
to
+17
| // Persistent, tvOS-local model for a mesh node. Deliberately NOT the iOS | ||
| // `NodeInfoEntity` — that entity's relationship graph is a single connected | ||
| // component of ~39 tables (messages, channels, traceroutes, every config), so it | ||
| // can't be subsetted for a map. This is a slim, map-scoped SwiftData store: the | ||
| // map is populated on relaunch from the last session, before the radio's node-DB | ||
| // dump completes. | ||
| // | ||
|
|
||
| import CoreLocation | ||
| import Foundation | ||
| import SwiftData |
Comment on lines
631
to
635
| children = ( | ||
| FB5CB2FD9C793A43DE2DB818 /* Accessory */, | ||
| 46D9EC9A9870D40B64CBD339 /* Accessory */, | ||
| 1B104A61866EBBAB39A96CD6 /* API */, | ||
| 91DE2B8079DF4453A2B7A512 /* AppIntents */, |
Bring a few things over from the iOS app: - Generate a default user for packet-discovered nodes: a node first seen via a position packet (no NodeInfo yet) now gets "Meshtastic <last4>" / "<last4>" (last 4 hex of the node number), matching the firmware/app default, instead of rendering as "?". A real NodeInfo frame overwrites it; an empty one won't. - Compact list rows modelled on NodeListItemCompact: node-color circle, name, then last-heard (green when online, heard <2 h) and role (mapped icon + name). - Scrollable node detail. The old read-only Form had no focusable rows, so tvOS couldn't scroll to fields below the fold; rows are now individually focusable (with the usual highlight), and the whole panel is resized down for the 520pt side column. Role is now stored as its raw protobuf value and mapped for display by a slim, tvOS-local NodeRole (name + SF Symbol), so this target doesn't need the app's full DeviceRoles enum. The store open is now wipe-and-retry resilient so the role field's type change can't brick launch on an un-migratable store.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 52 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
Meshtastic TV/Map/MeshTVMapView.swift:239
annotationsByNumis mutated while being iterated (annotationsByNum[num] = nilinside afor ... in annotationsByNumloop). In Swift this can trap at runtime with “Dictionary mutated while being enumerated”. Collect removals first, then apply them.
Meshtastic TV/Views/NodeDetailView.swift:13- This file uses
UIColor(hex:)but only importsSwiftUI. Addimport UIKit(or refactor to avoidUIColor) to ensure it compiles on tvOS.
Meshtastic TV/App/MapScreen.swift:14 - This file uses
UIColor(hex:)inNodeRowbut doesn’t import UIKit. Addimport UIKitsoUIColoris available to this target.
import SwiftUI
import SwiftData
Meshtastic TV/App/MapScreen.swift:135
disconnectButtonis a private computed view that isn’t used anywhere in this file. Keeping it around risks an “unused” warning and adds dead code to maintain.
private var disconnectButton: some View {
Button(role: .destructive) {
client.disconnect()
} label: {
Label("Disconnect", systemImage: "xmark.circle.fill")
Comment on lines
+40
to
+42
| /// The tvOS-local SwiftData store the map and list read via `@Query`. Every | ||
| /// upsert lands here; nothing is kept in memory, so the map survives relaunch. | ||
| private let context: ModelContext |
- Frost the map's wordmark header (a bottom-fading .ultraThinMaterial scrim) so scrolling node rows no longer collide with the logo / host text. - Align to design-standards v1.4: port the semantic palette (MeshtasticSuccess / Warning / Error) into the tvOS asset catalog and use it for online / offline / connect-error states instead of ad-hoc .green / .orange / .red. - Drop the off-palette "LightIndigo" green tint. Interactive buttons and icons now follow the app AccentColor (Blue 700), matching the iOS app — the brand green fails WCAG contrast as a UI tint, so it stays reserved for the logo and semantic success only. The Bonjour hint reverts to .secondary (accent-blue text would fail contrast on the dark background).
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 54 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
Meshtastic TV/App/MapScreen.swift:14
- This file references
UIColor(viaColor(UIColor(hex: ...))) but doesn’t import UIKit, which will fail to compile.
import SwiftUI
import SwiftData
Meshtastic TV/Views/NodeDetailView.swift:13
- This file references
UIColor(viaColor(UIColor(hex: ...))) but doesn’t import UIKit, which will fail to compile.
Meshtastic.xcodeproj/project.pbxproj:643 - This adds a second
AccessoryandExtensionsgroup under the mainMeshtasticgroup (duplicate names at the same hierarchy level), which makes the Xcode navigator ambiguous/confusing. Consider nesting these tvOS-shared file references under theMeshtastic TVgroup (or renaming toAccessory (Shared)/Extensions (Shared)) to avoid duplicates.
Meshtastic TV/App/MeshtasticTVApp.swift:14 - The PR description says the tvOS target does not link SwiftData and that there is 'No persistence in v1', but this target imports SwiftData and creates a persistent
ModelContainerforMeshNode. Either update the PR description to reflect the persistence, or adjust the implementation to be in-memory only.
import OSLog
import SwiftUI
import SwiftData
Comment on lines
+18
to
+19
| import MapKit | ||
| import SwiftUI |
Comment on lines
+176
to
+177
| @AppStorage("tv.mapType") private var mapTypeRaw: Int = Int(MKMapType.standard.rawValue) | ||
| private var mkMapType: MKMapType { MKMapType(rawValue: UInt(mapTypeRaw)) ?? .standard } |
Comment on lines
+145
to
+148
| case .configCompleteID: | ||
| // Initial dump finished — we have the node database; go live. | ||
| if state != .connected { state = .connected } | ||
|
|
Address the two actionable findings from the design-standards /audit-ui pass: - Typography: NodeDetailView's fixed .system(size:) fonts are now @ScaledMetric (mirroring the iOS NodeListItemCompact) so they track Dynamic Type rather than being hardcoded. Default sizes are unchanged. - Magic numbers: extract the recurring design-decision sizes — side-list width, node-avatar diameters, screen padding, section spacing, row corner radius, wordmark height — into a shared TVTheme, replacing scattered literals across the map, detail, and connect views. Values are identical, so no visual change.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 55 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
Meshtastic TV/Client/MeshClient.swift:10
- The PR description (and this header comment) describe MeshClient as “in-memory” and explicitly “no SwiftData persistence”, but the implementation uses SwiftData (
ModelContext,MeshNode, persisted node store). Please either update the PR description to reflect the intentional persistence, or remove persistence if that’s not intended. At minimum, the header comment should match the actual behavior.
Meshtastic TV/App/MapScreen.swift:10 - This file header says “clicking a pin on the map (focus engine) selects too”, but MeshTVMapView currently sets
mapView.isUserInteractionEnabled = false, making the map non-interactive. Update this comment to reflect the actual interaction model (side-list-driven selection), or re-enable map interaction if pin selection is intended.
// Live mesh map with a focusable node side-list for Siri Remote operation.
// The list is the primary, reliable selection path on tvOS; clicking a pin on
// the map (focus engine) selects too and keeps the list in sync. Rows carry the
// same node-color circle badges as the map pins (CircleText, shared with iOS).
Meshtastic TV/Map/MeshTVMapView.swift:171
- The doc comment for
onMenuExitdescribes handling Menu presses when the map has focus, butmakeUIViewdisables user interaction (isUserInteractionEnabled = false), so the map shouldn’t receive focus/presses andmenuPressed()is effectively unused. Either wire up Menu handling + re-enable interaction, or adjust/remove this comment/callback to match the current non-interactive map behavior.
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.
What
A new Meshtastic TV target — a thin, independent Apple TV client that connects to a node over TCP and shows a live mesh map operable with the Siri Remote. No Bluetooth (sidesteps BLE pairing on a remote); a plugged-in TV on Wi-Fi is a natural TCP client.
Approach
Mirrors the Watch App precedent (self-contained target), reusing only what's genuinely portable:
TCPConnection/Connection, Network.framework) by file membership, plus theMeshtasticProtobufspackage. It does not link the iOSAccessoryManager/MeshPackets/ SwiftData / BLE / CoreLocation stack.AccessoryError, the want-config nonces andTransportTypeare small tvOS-local shims.FromRadiostream (NodeInfo + Position). No persistence in v1.MKMapViewwrapper (Apple basemap) with annotation diffing, centroid framing, and focus / side-list selection. No user-location features (no GPS on tvOS)._meshtastic._tcp(Bonjour) so nodes appear in the connect list without typing an IP; manual host/port entry remains as a fallback.iOS impact
None. No existing iOS source is modified — the shared transport files are added by target membership only. The project regeneration is additive (new target + scheme).
Testing
Deferred (post-v1)
Offline PMTiles basemap, SwiftData persistence, messaging/channels, waypoint placement, traceroute flyover, user-location features, and MQTT/TAK/WatchConnectivity side effects.
Summary by CodeRabbit