Claude/meshtastic offline map brtxh5 - #6584
Closed
marmotte5 wants to merge 4 commits into
Closed
Conversation
An imported MBTiles tile provider and its selected layer were lost on every cold start, so the file had to be re-imported and the layer re-enabled by hand. Two independent causes, each sufficient on its own: - CustomTileProviderRepositoryImpl read MapTileProviderPrefs.customTileProviders synchronously in its constructor. That StateFlow is seeded with null while the real value arrives asynchronously from DataStore, so the constructor always read the placeholder and cached an empty list permanently. The next edit then persisted a list built on that empty baseline, destroying the stored providers rather than merely failing to show them, and orphaning their copied files. - MapViewModel.loadPersistedMapType() matched a saved selection with `urlTemplate == saved && isValidTileUrlTemplate(saved)`. A local provider has an empty urlTemplate and is persisted by its file:// URI, so both conditions were always false and the fallback branch actively cleared the stored preference. The renderer resolved the same selection correctly, so the two paths disagreed. Stored values are now plain Flows rather than StateFlows: a StateFlow has to invent an initial value, and callers could not tell that placeholder apart from a genuine "nothing saved". Start-up awaits a real value from both stores before deciding, and no write is built on a list that has not been loaded. Selection matching now lives in CustomTileProviderConfig and is shared by the renderer and the restore path so they cannot drift apart again. Also fixed: editing a provider left the persisted selection pointing at a stale key, dropping the layer on the following start. Adds one deliberate fork behaviour, documented in FORK.md: when no valid saved selection applies, an imported local provider is selected on start-up so the app opens on usable offline tiles with no network and no user action. Trade-off: while an MBTiles provider exists, an explicit switch to a Google base map does not survive a restart. Removes feature/map/src/androidUnitTestGoogle/, a source set no Gradle task builds, whose tests targeted a package and a constructor signature that no longer exist. Replacement tests live in androidApp/src/testGoogle/. Claude-Session: https://claude.ai/code/session_015A8oGSLDYu8BW5WknB1qzx Co-authored-by: Claude <noreply@anthropic.com>
Hides the transport selector and pins the Connections pane to BLE, so the app presents one way to connect instead of three. Pinning the pane matters beyond hiding the control. activeTransport was resolved from a stored preference and from the selected device's address, so a phone that had TCP or USB stored — from an earlier build or a stray tap — would open on a pane that is now unreachable, with an empty device list and no visible way back to Bluetooth. Pinning removes that trap. Consequences, both intended: network auto-scan can no longer start, and BLE auto-scan is never gated off. selectTransport is left in place; its scan-stopping side effects still apply and keeping it limits the divergence from upstream. Updates the ScannerViewModel tests that encoded upstream's multi-transport contract, and adds one covering the stored-preference trap. Fork-specific, not upstreamable: upstream deliberately supports three transports. Documented in FORK.md. Claude-Session: https://claude.ai/code/session_015A8oGSLDYu8BW5WknB1qzx Co-authored-by: Claude <noreply@anthropic.com>
The compass already existed and already does the hard part: it reads the phone's magnetometer, so the arrow points where to walk rather than showing a bearing relative to north, and it reports distance, alignment, and the degraded cases. What it lacked was reach — node list, find the node, open it, tap the compass. Adds a map toolbar button that lands straight on it: - NodesRoute.NodeDetail gains openCompass, and NodeDetailScreen opens the overlay once the node has loaded when it is set. Keyed on the node so dismissing the overlay does not reopen it on the next recomposition. - MapViewProvider.MapView and LocalMapMainScreenProvider gain navigateToNodeCompass, defaulting to plain node details so callers that do not offer the shortcut are unaffected. The osmdroid provider accepts it and ignores it. - The button renders only when a favourite node has a known position, so it is never a control that does nothing, and only on the main map. Targeting is by favourite node rather than a hard-coded node number: mark the device once and it stays changeable in the field without a rebuild. The route flag is generic and could go upstream; the map button is opinionated. Documented in FORK.md. Claude-Session: https://claude.ai/code/session_015A8oGSLDYu8BW5WknB1qzx Co-authored-by: Claude <noreply@anthropic.com>
MapButton inherited Material's FilledIconButton container size, which left the map controls below the 44dp minimum in .skills/design-standards. The size is now set explicitly, and the glyph goes from Material's 24dp default to 26dp so the symbol stays readable at arm's length outdoors. Sized to 44dp rather than the roomier 48dp deliberately: HorizontalFloatingToolbar does not scroll, and a fully-populated map toolbar — compass, find-favourite, filter, map type, layers, site planner, location — at 48dp overflows a 360dp-wide screen and clips its last buttons. 44dp meets the standard and still fits. If the toolbar ever gains horizontal scrolling, 48dp becomes the better value. Only one caller passes a modifier (the compass, a rotation), so no existing sizing is overridden. Deliberately not changed: the icon glyphs. Which symbols read as unclear needs eyes on a real screen, and swapping artwork blind trades a known set for an unverified one. Claude-Session: https://claude.ai/code/session_015A8oGSLDYu8BW5WknB1qzx Co-authored-by: Claude <noreply@anthropic.com>
|
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (31)
📝 WalkthroughWalkthroughThe pull request adds favorite-node compass navigation, fixes asynchronous offline tile-provider persistence and selection restoration, pins the connections pane to Bluetooth, and documents fork-specific behavior and map-control sizing. ChangesFavorite-node compass navigation
Offline map persistence
BLE-only connections
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MapControlsOverlay
participant MapView
participant MapNavigation
participant NodeDetailScaffold
MapControlsOverlay->>MapView: invoke favorite shortcut
MapView->>MapNavigation: navigate to favorite node
MapNavigation->>NodeDetailScaffold: open node with openCompass=true
NodeDetailScaffold->>NodeDetailScaffold: start and show compass once
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 Warning |
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
2 participants
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.
Thank you for sending in a pull request, here's some tips to get started!
(Please delete all these tips and replace with your text)
Summary by CodeRabbit