Add configurable GPS location provider for GrapheneOS#44
Draft
dougborg wants to merge 9 commits intoFoggedLens:mainfrom
Draft
Add configurable GPS location provider for GrapheneOS#44dougborg wants to merge 9 commits intoFoggedLens:mainfrom
dougborg wants to merge 9 commits intoFoggedLens:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an Android-specific, user-configurable GPS provider selection aimed at improving compatibility on GrapheneOS/devices without Google Play Services, plus UI feedback while a GPS fix is pending.
Changes:
- Introduces a persisted
forceLocationManagersetting (defaulttrue) and exposes it throughAppState, with a new Advanced Settings section to toggle Google Location Services on Android. - Updates GPS tracking to use Android
forceLocationManagerand supports restarting the position stream immediately when the setting changes. - Adds “searching for GPS signal” UI feedback (orange
gps_not_fixedicon + tooltip) and includes a broad set of refactors/cleanup plus new state tests.
Reviewed changes
Copilot reviewed 92 out of 94 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/state/upload_queue_state_test.dart | New unit tests covering UploadQueueState queue behavior and persistence. |
| test/state/session_state_test.dart | New unit tests for SessionState lifecycle, dirty checking, and direction handling. |
| test/state/app_state_integration_test.dart | New integration-style tests replicating AppState commit flows without full AppState init. |
| test/models/tile_provider_test.dart | Minor test variable renames/cleanup. |
| pubspec.yaml | Adds dependencies/dev-dependencies used by changes (e.g., path_provider, collection, flutter_lints). |
| pubspec.lock | Locks newly added dependencies. |
| lib/widgets/welcome_dialog.dart | Replaces withOpacity usage with withValues. |
| lib/widgets/suspected_location_sheet.dart | Removes unused AppState watch; updates color alpha API usage. |
| lib/widgets/submission_guide_dialog.dart | Updates color alpha API usage. |
| lib/widgets/search_bar.dart | Uses spread for results list and updates color alpha API usage. |
| lib/widgets/refine_tags_sheet.dart | Refactors radio selection UI to use a RadioGroup wrapper. |
| lib/widgets/reauth_messages_dialog.dart | Updates color alpha API usage. |
| lib/widgets/proximity_warning_dialog.dart | Removes unused import. |
| lib/widgets/proximity_alert_banner.dart | Updates color alpha API usage. |
| lib/widgets/provisional_pin.dart | Updates color alpha API usage. |
| lib/widgets/positioning_tutorial_overlay.dart | Updates color alpha API usage. |
| lib/widgets/nuclear_reset_dialog.dart | Migrates WillPopScope to PopScope and adds mounted check before dialog. |
| lib/widgets/node_tag_sheet.dart | Local function renames and color alpha API update. |
| lib/widgets/node_provider_with_cache.dart | Removes unused imports. |
| lib/widgets/navigation_sheet.dart | Removes unused import and updates color alpha API usage. |
| lib/widgets/map_view.dart | Passes getForceLocationManager callback; restarts GPS stream on setting changes. |
| lib/widgets/map/tile_layer_manager.dart | Minor cleanup and maxZoom conversion update. |
| lib/widgets/map/suspected_location_markers.dart | Modernizes constructor to super.key. |
| lib/widgets/map/overlay_layer_builder.dart | Removes unused import and updates color alpha API usage. |
| lib/widgets/map/node_refresh_controller.dart | Removes unused import. |
| lib/widgets/map/node_markers.dart | Modernizes constructor to super.key. |
| lib/widgets/map/marker_layer_builder.dart | Removes unused import. |
| lib/widgets/map/map_overlays.dart | Removes unused imports and updates color alpha API usage. |
| lib/widgets/map/map_data_manager.dart | Removes unused import. |
| lib/widgets/map/layer_selector_button.dart | Uses surfaceContainerHighest for Material color scheme consistency. |
| lib/widgets/map/gps_controller.dart | Adds Android forceLocationManager support and restart method for stream changes. |
| lib/widgets/map/direction_cones.dart | Removes unused code and updates color alpha API usage. |
| lib/widgets/edit_node_sheet.dart | Refactors commit flow, adds mounted checks, and reduces parameter threading. |
| lib/widgets/download_area_dialog.dart | Refactors download start flow to avoid context-after-pop and improves async safety. |
| lib/widgets/compass_indicator.dart | Removes unused import and updates color alpha API usage. |
| lib/widgets/changelog_dialog.dart | Removes unused import. |
| lib/widgets/camera_icon.dart | Updates color alpha API usage. |
| lib/widgets/advanced_edit_options_sheet.dart | Adds context.mounted guard before redirect. |
| lib/widgets/add_node_sheet.dart | Refactors commit flow, adds mounted checks, and removes unused tutorial hooks. |
| lib/state/upload_queue_state.dart | Adds injectable cache/provider for testability; switches prints to debugPrint. |
| lib/state/settings_state.dart | Adds persisted forceLocationManager setting and getter/setter. |
| lib/state/search_state.dart | Removes unused import. |
| lib/state/auth_state.dart | Replaces print with debugPrint. |
| lib/services/uploader.dart | Improves error message string interpolation and removes unused _post. |
| lib/services/tile_preview_service.dart | Minor formatting cleanup. |
| lib/services/suspected_location_service.dart | Removes unused imports and simplifies catch block. |
| lib/services/suspected_location_database.dart | Removes unused counters. |
| lib/services/suspected_location_cache.dart | Removes unused import. |
| lib/services/routing_service.dart | Cleans up variable naming and removes unnecessary await/string interpolation. |
| lib/services/offline_area_service.dart | Adds path_provider usage and removes unused imports. |
| lib/services/node_data_manager.dart | Removes unused imports. |
| lib/services/node_cache.dart | Adds foundation import and replaces prints with debugPrint. |
| lib/services/network_status.dart | Minor whitespace cleanup. |
| lib/services/map_data_submodules/tiles_from_local.dart | Removes unused import. |
| lib/services/map_data_submodules/nodes_from_osm_api.dart | Uses rethrow and removes unused import. |
| lib/services/map_data_provider.dart | Removes unused import. |
| lib/services/deflock_tile_provider.dart | Import ordering cleanup and uses rethrow. |
| lib/services/deep_link_service.dart | Removes unused import and improves doc comment formatting. |
| lib/services/changelog_service.dart | Removes unused import and expands while loops to block form. |
| lib/services/auth_service.dart | Removes unused import, replaces prints with debugPrint, and small null-safety cleanup. |
| lib/screens/upload_queue_screen.dart | Removes unused import and updates color alpha API usage + string building cleanup. |
| lib/screens/tile_provider_editor_screen.dart | Removes unused import. |
| lib/screens/settings_screen.dart | Updates color alpha API usage. |
| lib/screens/settings/sections/upload_mode_section.dart | Updates color alpha API usage and removes redundant default branch. |
| lib/screens/settings/sections/tile_provider_section.dart | Removes unnecessary toList() and updates Material surface color. |
| lib/screens/settings/sections/queue_section.dart | Removes unused import and cleans up string building. |
| lib/screens/settings/sections/proximity_alerts_section.dart | Updates color alpha API usage. |
| lib/screens/settings/sections/offline_areas_section.dart | Cleans up string building and removes unnecessary toList(). |
| lib/screens/settings/sections/node_profiles_section.dart | Adds context.mounted guard before navigation. |
| lib/screens/settings/sections/language_section.dart | Adds mounted checks for async loads and refactors radio selection via RadioGroup. |
| lib/screens/settings/sections/gps_provider_section.dart | New Android-only Advanced Settings section to toggle Google vs raw GPS provider. |
| lib/screens/release_notes_screen.dart | Updates color alpha API usage. |
| lib/screens/profile_editor.dart | Removes unused controller variable. |
| lib/screens/osm_account_screen.dart | Updates Material surface color and color alpha API usage. |
| lib/screens/operator_profile_editor.dart | Removes unused controller variable. |
| lib/screens/navigation_settings_screen.dart | Updates color alpha API usage. |
| lib/screens/home_screen.dart | Shows orange GPS-searching icon/tooltip when no fix; adds mounted checks. |
| lib/screens/coordinators/sheet_coordinator.dart | Minor cleanup and debug string interpolation change. |
| lib/screens/advanced_settings_screen.dart | Adds GPS provider section to Advanced Settings. |
| lib/models/suspected_location.dart | Replaces prints with debugPrint and adds foundation import. |
| lib/models/operator_profile.dart | Removes unused uuid import. |
| lib/models/node_profile.dart | Removes unused uuid import. |
| lib/models/direction_fov.dart | Simplifies string interpolation. |
| lib/migrations.dart | Adds context.mounted guard before showing NuclearResetDialog. |
| lib/localizations/zh.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/pt.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/it.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/fr.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/es.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/en.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/localizations/de.json | Adds GPS provider strings + “searching GPS” follow-me tooltip translation. |
| lib/app_state.dart | Exposes forceLocationManager and delegates setter to SettingsState; adds mounted guard in reauth prompt. |
| android/app/build.gradle.kts | Changes minSdk wiring (requires adjustment—see PR comment). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
android/app/build.gradle.kts
Outdated
| // oauth2_client 4.x & flutter_web_auth_2 5.x require minSdk 23 | ||
| // ──────────────────────────────────────────────────────────── | ||
| minSdk = 23 | ||
| minSdk = flutter.minSdkVersion |
There was a problem hiding this comment.
minSdk was changed from 23 to flutter.minSdkVersion, but this file’s comment indicates the app requires minSdk 23 (oauth2_client 4.x / flutter_web_auth_2 5.x). Flutter’s default minSdkVersion is commonly 21, which would make the Android build fail or crash at runtime due to plugin minSdk requirements. Consider restoring minSdk = 23 (or using max(flutter.minSdkVersion, 23) if you need to keep it configurable).
Suggested change
| minSdk = flutter.minSdkVersion | |
| minSdk = maxOf(flutter.minSdkVersion, 23) |
Collaborator
Author
There was a problem hiding this comment.
This is from the parent PR (#36 / pr/04-state-context) — the stacked diff shows its commits here because the PR base is set to main rather than the parent branch. The minSdk is already correctly handled with maxOf(flutter.minSdkVersion, 23) in that parent PR, which is exactly what Copilot suggested.
Comment on lines
33
to
48
| shared_preferences: ^2.2.2 | ||
| sqflite: ^2.4.1 | ||
| path: ^1.8.3 | ||
| path_provider: ^2.1.0 | ||
| uuid: ^4.0.0 | ||
| package_info_plus: ^8.0.0 | ||
| csv: ^6.0.0 | ||
| collection: ^1.18.0 | ||
|
|
||
| dev_dependencies: | ||
| flutter_test: | ||
| sdk: flutter | ||
| mocktail: ^1.0.4 | ||
| flutter_launcher_icons: ^0.14.4 | ||
| flutter_lints: ^6.0.0 | ||
| flutter_native_splash: ^2.4.6 |
There was a problem hiding this comment.
The PR description focuses on adding a configurable GPS provider, but this change set also introduces broader refactors/dependency updates (e.g., new dependencies like path_provider, collection, flutter_lints, plus large-scale widget/style refactors and new test suites). Please either update the PR description to reflect these additional changes or split unrelated refactors into separate PRs to reduce review/rollback risk.
Collaborator
Author
There was a problem hiding this comment.
These additional changes are from the parent PR (#36 / pr/04-state-context) appearing in the diff because the PR base is set to main. This PR's own changes are limited to the GPS provider fix. Ideally the base would be set to the parent branch to show only this PR's commits, but that requires push access to the upstream repo.
Collaborator
Author
|
I'm not sure this is necessary yet, but I do think there should be a better indication when we have no live location data available. |
dougborg
force-pushed
the
fix/gps-grapheneos
branch
4 times, most recently
from
2596679 to
bd33511
Compare
Private State methods were accepting BuildContext as a parameter, shadowing this.context and forcing the less-idiomatic context.mounted guard. Per dart.dev linter guidance, State.mounted is the correct guard when using the State's own context property. - add_node_sheet/edit_node_sheet: Drop context/appState/locService params from _checkProximityAndCommit, _checkSubmissionGuideAndProceed, _checkProximityOnly, _commitWithoutCheck - download_area_dialog: Extract inline async lambda to _startDownload() State method with mounted guards Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move context.read<AppState>() before the await on getOfflineAreaDir() to avoid reading stale state if the dialog is disposed during the I/O operation. The mounted checks were already added in an earlier commit; this fixes the remaining state-capture ordering issue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
57 tests covering the ChangeNotifier state layer that widgets depend on: - Session lifecycle: start/clear add vs edit, operator profile detection, direction initialization from nodes with and without directions - Dirty checking: updateSession only notifies on actual changes, profile change regenerates changeset comment, defensive copy of refinedTags - Edit session recalculation: profile change recalculates additionalExistingTags/refinedTags/changesetComment, extractFromWay snap-back, explicit tags override auto-calculation - Direction management: add/remove/cycle with correct min enforcement (min=1 for add, min=0 for edit when original had no directions) - Commit guards: returns null unless target+profile set (add) or profile set (edit), double commit returns null safely - Cancel: clears session and detected operator profile - Changeset comment generation for all operation types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Small constructor change: MapDataProvider and NodeProviderWithCache are now injectable via optional constructor parameters with defaults to the existing singletons. Production code unchanged. 27 tests covering: - addFromSession: creates PendingUpload with correct operation, adds temp node with negative ID and _pending_upload tag to cache - addFromEditSession: modify marks original with _pending_edit + creates temp node; extract creates only temp node; constrained modify uses original coordinates - addFromNodeDeletion: marks node with _pending_deletion - clearQueue/removeFromQueue: correct cache cleanup dispatch (create removes temp, edit removes temp + pending_edit marker, delete removes pending_deletion marker, extract removes temp only) - Direction formatting: single as double, multiple as semicolon-separated, FOV range notation, 360 FOV, wrapping ranges - Queue persistence: save/load round-trip via SharedPreferences Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
13 tests verifying the coordination between SessionState and UploadQueueState that AppState.commitSession() performs: - Full add flow: startAddSession -> set target + profile -> commitSession -> addFromSession -> queue has 1 item, session null - Full edit flow: both modify and extract paths - Commit guards: incomplete session doesn't add to queue, double commit is safe (second returns null) - Profile deletion callback: deleting profile used in active add/edit session cancels that session; unrelated profile deletion doesn't affect session - Notification propagation: sub-module notifyListeners fires on all state-changing operations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Force Android LocationManager by default (raw GPS) so the app works on GrapheneOS and devices without Google Play Services. Users on standard Android can toggle "Use Google Location Services" in Advanced Settings to get Fused Location Provider benefits (Wi-Fi/cell triangulation, better battery). - Add forceLocationManager persisted setting (default true) - Wire setting through AppState -> GpsController via callback - Restart GPS stream immediately when the setting changes - New GpsProviderSection in Advanced Settings (Android only) - Show orange gps_not_fixed icon + tooltip when GPS has no fix - Localization strings for all 7 languages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The GPS issue was likely caused by being indoors with no satellite signal, not a GrapheneOS compatibility problem. Default to Google Fused Location (Wi-Fi/cell triangulation) for the majority of users; those on GrapheneOS or without Play Services can toggle to raw GPS in Advanced Settings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test default value, SharedPreferences persistence, listener notification on change, no-op on same value, and round-trip through save and reload. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dougborg
force-pushed
the
fix/gps-grapheneos
branch
from
bd33511 to
09b1677
Compare
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
Changes
SettingsState: newforceLocationManagerpersisted setting (defaultfalse= use Google Fused)AppState: delegated getter/setterGpsController: reads setting via callback, usesAndroidSettings(forceLocationManager:)on Android, newrestartLocationProvider()method for immediate effect on toggleMapView: passes callback and detects setting changes to restart GPS streamGpsProviderSectionwidget in Advanced Settings (hidden on non-Android)HomeScreen: follow-me button showsgps_not_fixedwith orange tint when no GPS fixTest plan
flutter analyze— no new issuesflutter test— all 129 tests pass🤖 Generated with Claude Code