Kaynağa Gözat

fix: request location when speed uses GPS

Turning Show speed on with no OBD adapter (or with OBD speed off) now asks for location permission and retries the GPS fallback. A denial leaves the switch on and explains why; the resume path still never prompts.
Krasimir Kazakov 6 gün önce
ebeveyn
işleme
6b0a9ebe6d

+ 1 - 1
AGENTS.md

@@ -98,7 +98,7 @@ notifyListeners();
 - `FLAG_KEEP_SCREEN_ON` is controlled by `DisplayController` from `display.keepScreenOn` (default on) through `SystemBridge`; do not hard-code it in `MainActivity` or move it to a wake lock.
 - The Music equalizer follows `AudioManager.isMusicActive`. Now-playing title/progress use the notification-listener component and active MediaSession; never read or store notification content.
 - Night mode is a full-screen black overlay with a dim clock. Long-press only on empty background; tap anywhere to dismiss. It does not change brightness or add a wake lock.
-- Speed is shown only when `TelemetrySettingsController.speedEnabled` is on. OBD-II is preferred when connected and fresh; the GPS fallback is used when OBD speed is off or unavailable. GPS starts only with already-granted fine location and GPS; never prompt from that path, and clamp stationary noise below 2 km/h to 0.
+- Speed is shown only when `TelemetrySettingsController.speedEnabled` is on. OBD-II is preferred when connected and fresh; the GPS fallback is used when OBD speed is off or unavailable. Enabling speed with GPS as the source (no adapter selected or OBD speed off) asks for location permission; the normal resume path never prompts. GPS starts only with fine location and GPS, and clamps stationary noise below 2 km/h to 0.
 - Temperature is shown only when `TelemetrySettingsController.temperatureEnabled` is on. OBD ambient is preferred when connected and fresh; Open-Meteo/location weather is the fallback, and the source switch only appears while an OBD-II adapter is selected.
 - Jellyfin lives only in Settings → Jellyfin. Quick Connect is the only sign-in path: the user enters the code in Jellyfin, and CarDash keeps the returned access token in secure storage and reuses it on the next start. A rejected token or server error disables the integration, clears the token and shows **Pair again**; an unreachable server keeps the token and retries with 5–60 s backoff, so never clear the token for a network error. Use `JellyfinController.connected` as the active/inactive state for later features; do not add a username/password form.
 - Updates use a non-blocking public Gogs release check. The Update action downloads the APK to cache and opens Android’s package installer; never install silently. REQUEST_INSTALL_PACKAGES + one-time unknown-apps permission is required. Show the yellow dot on the Settings gear whenever `UpdateController.updateAvailable` is true.

+ 15 - 13
README.md

@@ -69,7 +69,8 @@ live vehicle data.
 - **Speed card with source choice** — Settings → **Speed** shows or hides the
   speed card and, while an adapter is selected, chooses OBD-II or GPS. GPS is
   the fallback when OBD speed is off or unavailable; it reads 0 km/h when
-  stationary and never prompts for location by itself.
+  stationary, and enabling speed with GPS as the source asks for location
+  permission.
 - **Temperature row with source choice** — Settings → **Temperature** shows or
   hides the outside temperature and, while an adapter is selected, chooses
   OBD-II or GPS + weather. Enabling temperature asks for location permission
@@ -278,9 +279,9 @@ the app resumes.
 - One switch shows or hides the speed card on Home.
 - While an OBD-II adapter is selected, **Use OBD-II for speed** selects the
   source. Off means GPS is used.
-- Without OBD (or with OBD speed unavailable), the GPS fallback is used when
-  precise location is already granted. It never prompts from this path and
-  reads 0 km/h when stationary.
+- Without OBD (or with OBD speed off), the GPS fallback is used. Enabling speed
+  or switching the source to GPS asks for location permission; if denied, the
+  switch stays on and a message explains why. It reads 0 km/h when stationary.
 
 **Temperature**
 - One switch shows or hides the outside-temperature row below the clock.
@@ -343,18 +344,19 @@ flutter test
 ./tool/check.sh
 ```
 
-The 66 tests cover PIN hashing/persistence, settings persistence, theme/accent,
+The 89 tests cover PIN hashing/persistence, settings persistence, theme/accent,
 dark accent tinting, battery visibility, first-run state, the ten-minute
 onboarding hint, slot visibility, pinned apps, custom labels/icons, the
 16-button cap at 1280×800, 1280×720 and 1024×600, the dense shortcut grid, the
 equalizer animation, OBD economy, the OBD card, media-key playback, media
 metadata/progress, the marquee, the play/pause ring, fixed-volume handling,
 volume-write diagnostics, the keep-awake preference, the night overlay, GPS
-speed fallback, speed/temperature source persistence, weather fallback, the
-update version/availability logic and the Settings update dot. Native package
-enumeration, the HOME role, volume, app launching and PIN flows were verified
-on API 35 and API 29 emulators. Location permission, reverse geocoding and the
-Open-Meteo fallback were verified live on the API 35 emulator. Now-playing
+speed fallback and its permission prompt, speed/temperature source persistence,
+weather fallback, the update version/availability logic and the Settings update
+dot. Native package enumeration, the HOME role, volume, app launching and PIN
+flows were verified on API 35 and API 29 emulators. Location permission,
+reverse geocoding and the Open-Meteo fallback were verified live on the API 35
+emulator. Now-playing
 title, artist, progress and pause/resume were verified against a live Chrome
 MediaSession. The OBD hardware path compiles and is covered by the
 parser/economy design, but no ELM327 adapter is available on this machine;
@@ -432,9 +434,9 @@ Android 15 AVD is left running headlessly after development; use
   not expose MediaSession metadata to a normal launcher; the card still controls
   playback and shows Nothing playing or Playing.
 - **Speed source** is selectable only while an OBD adapter is selected. GPS
-  fallback requires precise location already granted and the GPS provider
-  enabled; CarDash does not request permission from this path. Stale/no-fix GPS
-  values are hidden.
+  fallback requires fine location and the GPS provider enabled; enabling speed
+  with GPS as the source asks for location permission. Stale/no-fix GPS values
+  are hidden.
 - **OBD-II is a subset**: Bluetooth Classic SPP only, paired devices only,
   read-only. Speed, throttle, MAF economy and ambient temperature are implemented;
   RPM, coolant, voltage, MIL/DTCs, fuel level and BLE/Wi-Fi adapters are not.

+ 7 - 3
docs/IMPLEMENTATION_PLAN.md

@@ -79,7 +79,7 @@ compatibility:
 | OBD values | Speed, throttle (`49`/`11`), MAF economy, ambient (`46`) | The requested dashboard subset; unsupported PIDs stay hidden. |
 | Economy | 5 km MAF rolling window, petrol constants, fuel-cut guard | Matches the design note; show `—` until 0.5 km. |
 | Weather fallback | Open-Meteo + one-shot device location via the Temperature card | No API key/account; hide the row on any failure. |
-| GPS speed fallback | Poll GPS when speed source is GPS, OBD is unavailable, or the Speed card is off | No permission prompt from this path; 0 below 2 km/h to suppress stationary drift. |
+| GPS speed fallback | Poll GPS when speed source is GPS, OBD is unavailable, or the Speed card is off | Asks for location when speed is enabled with GPS as the source; 0 below 2 km/h to suppress stationary drift. |
 | Speed source | OBD-II preferred while connected; GPS fallback, switch in Settings | The user can force GPS even with an adapter connected. |
 | Update check | Public Gogs releases API, non-blocking startup check, Settings card | Update downloads the APK and opens Android’s installer; installation stays user-confirmed. Yellow dot on Settings gear when available. |
 | Temperature priority | OBD ambient while its source switch is on, else location weather | OBD wins when fresh; weather is the fallback. |
@@ -105,7 +105,7 @@ Flutter
     LauncherController  -> apps, slots, pinned apps, 16-button cap, default-home state
     SystemController    -> battery/Wi-Fi/volume/media state + media keys
     DisplayController   -> persisted keep-awake window flag
-    GpsSpeedController  -> GPS speed fallback (permission already granted)
+    GpsSpeedController  -> GPS speed fallback (permission requested from Speed settings)
     TelemetrySettingsController -> speed/temperature visibility + sources
     UpdateController    -> public Gogs release check + release-page link
     JellyfinController  -> Quick Connect pairing, token reuse, retry/offline state
@@ -214,7 +214,7 @@ Android (Kotlin)
   both the ring and scroll position, resume continues.
 - [x] Settings → Media grants notification access and shows Granted.
 - [x] Settings gear is next to Wi-Fi; the grid ends with All apps.
-- [x] GPS speed shows 0 km/h when stationary with no OBD adapter; no permission prompt.
+- [x] GPS speed shows 0 km/h when stationary with no OBD adapter; enabling speed asks for location permission.
 - [x] 1.0.0 updater build installed on emulator/tablet; it reported up to date before 1.0.1.
 - [x] 1.0.1 universal APK built and added to `releases/`; release published for update testing.
 - [x] Emulator completed the full in-app 1.0.0 → 1.0.1 download/installer flow.
@@ -358,3 +358,7 @@ Android (Kotlin)
   `releases/cardash-android-1.0.0.apk` with checksum and release notes.
 - 2026-10-29: Centered the now-playing title across the whole media card instead of
   the right-hand control column. Verified 0.9.1 on emulator/tablet.
+- 2026-10-31: Enabling **Show speed** with GPS as the source (no OBD adapter
+  selected or OBD speed off) now requests location permission and retries the
+  GPS fallback; a denied request leaves the switch on and shows an explanation.
+  The normal resume path still never prompts. 89 tests pass.

+ 18 - 0
lib/features/home/gps_speed_controller.dart

@@ -32,6 +32,7 @@ class GpsSpeedController extends ChangeNotifier {
     _active = true;
     final started = await _platform.startGpsSpeed();
     if (!started) {
+      _active = false;
       _notify();
       return;
     }
@@ -39,6 +40,23 @@ class GpsSpeedController extends ChangeNotifier {
     await _poll();
   }
 
+  /// Asks for precise location access and starts the fallback when granted.
+  ///
+  /// Used only by the Settings switches that explicitly choose GPS speed; the
+  /// normal resume path still never prompts.
+  Future<bool> requestPermission() async {
+    final granted = await _platform.ensureLocationPermission();
+    if (!granted) {
+      _notify();
+      return false;
+    }
+    _timer?.cancel();
+    _timer = null;
+    _active = false;
+    await start();
+    return true;
+  }
+
   void stop() {
     _active = false;
     _timer?.cancel();

+ 21 - 7
lib/features/home/telemetry_settings_controller.dart

@@ -1,18 +1,21 @@
 import 'package:flutter/foundation.dart';
 
 import '../../data/repositories/settings_repository.dart';
+import 'gps_speed_controller.dart';
 import 'weather_controller.dart';
 
 /// Persisted Home visibility and source choices for speed and temperature.
 ///
 /// OBD-II is preferred when it is connected and fresh; GPS and the weather
-/// provider are the fallbacks. Turning temperature off also stops the weather
-/// poller so CarDash keeps working quietly when the row is hidden.
+/// provider are the fallbacks. Turning either row on with a GPS source asks for
+/// location permission, and turning temperature off stops the weather poller so
+/// CarDash keeps working quietly when the row is hidden.
 class TelemetrySettingsController extends ChangeNotifier {
-  TelemetrySettingsController(this._settings, this._weather);
+  TelemetrySettingsController(this._settings, this._weather, this._gps);
 
   final SettingsRepository _settings;
   final WeatherController _weather;
+  final GpsSpeedController _gps;
 
   bool _speedEnabled = true;
   bool _speedUseObd = true;
@@ -42,20 +45,31 @@ class TelemetrySettingsController extends ChangeNotifier {
     );
   }
 
-  Future<void> setSpeedEnabled(bool value) async {
-    if (value == _speedEnabled) return;
+  /// Returns false when the speed switch is on but GPS fallback permission was
+  /// denied and OBD-II is not configured as the source.
+  Future<bool> setSpeedEnabled(bool value) async {
+    if (value == _speedEnabled) return true;
     _speedEnabled = value;
     _notify();
     await _settings.setSpeedEnabled(value);
+    if (!value || !_needsGpsSpeed) return true;
+    return _gps.requestPermission();
   }
 
-  Future<void> setSpeedUseObd(bool value) async {
-    if (value == _speedUseObd) return;
+  Future<bool> setSpeedUseObd(bool value) async {
+    if (value == _speedUseObd) return true;
     _speedUseObd = value;
     _notify();
     await _settings.setSpeedUseObd(value);
+    if (value || !_speedEnabled || !_needsGpsSpeed) return true;
+    return _gps.requestPermission();
   }
 
+  /// GPS is the source whenever the adapter is not configured or OBD speed is
+  /// explicitly off. A configured-but-disconnected adapter keeps OBD selected.
+  bool get _needsGpsSpeed =>
+      !_speedUseObd || _settings.obdAdapterAddress == null;
+
   /// Returns false when the temperature switch is on but location permission
   /// for the weather fallback was denied. OBD-II temperature can still work.
   Future<bool> setTemperatureEnabled(bool value) async {

+ 24 - 2
lib/features/settings/speed_settings_card.dart

@@ -59,7 +59,15 @@ class SpeedSettingsCard extends StatelessWidget {
               semanticsLabel: telemetry.speedEnabled
                   ? 'Hide the speed card'
                   : 'Show the speed card',
-              onChanged: telemetry.setSpeedEnabled,
+              onChanged: (value) async {
+                final ok = await telemetry.setSpeedEnabled(value);
+                if (!ok && context.mounted) {
+                  _message(
+                    context,
+                    'Location permission is required for GPS speed.',
+                  );
+                }
+              },
             ),
           ],
         ),
@@ -91,7 +99,15 @@ class SpeedSettingsCard extends StatelessWidget {
                 semanticsLabel: telemetry.speedUseObd
                     ? 'Use GPS for speed'
                     : 'Use OBD-II for speed',
-                onChanged: telemetry.setSpeedUseObd,
+                onChanged: (value) async {
+                  final ok = await telemetry.setSpeedUseObd(value);
+                  if (!ok && context.mounted) {
+                    _message(
+                      context,
+                      'Location permission is required for GPS speed.',
+                    );
+                  }
+                },
               ),
             ],
           ),
@@ -99,4 +115,10 @@ class SpeedSettingsCard extends StatelessWidget {
       ],
     );
   }
+
+  void _message(BuildContext context, String message) {
+    final messenger = ScaffoldMessenger.of(context);
+    messenger.hideCurrentSnackBar();
+    messenger.showSnackBar(SnackBar(content: Text(message)));
+  }
 }

+ 1 - 0
lib/main.dart

@@ -60,6 +60,7 @@ Future<void> main() async {
   final telemetrySettingsController = TelemetrySettingsController(
     settings,
     weatherController,
+    gpsSpeedController,
   )..initialize();
   final lockController = LockController(PinService(FlutterSecureStore()));
   await lockController.initialize();

+ 29 - 2
test/gps_speed_test.dart

@@ -13,13 +13,16 @@ import 'package:flutter_test/flutter_test.dart';
 class _GpsPlatform implements TelemetryPlatform {
   _GpsPlatform({this.permission = true, this.speedKmh = 0});
 
-  final bool permission;
+  bool permission;
   final double speedKmh;
   var started = false;
+  var startCalls = 0;
+  var permissionRequests = 0;
 
   @override
   Future<bool> startGpsSpeed() async {
     started = true;
+    startCalls++;
     return permission;
   }
 
@@ -50,7 +53,10 @@ class _GpsPlatform implements TelemetryPlatform {
   Future<bool> openBluetoothSettings() async => true;
 
   @override
-  Future<bool> ensureLocationPermission() async => permission;
+  Future<bool> ensureLocationPermission() async {
+    permissionRequests++;
+    return permission;
+  }
 
   @override
   Future<LocationReading> getLocation() async =>
@@ -93,6 +99,27 @@ void main() {
     controller.stop();
   });
 
+  test('permission grant retries the GPS fallback', () async {
+    final platform = _GpsPlatform(permission: false);
+    final controller = GpsSpeedController(
+      platform,
+      clock: () => DateTime(2026),
+    );
+    addTearDown(controller.dispose);
+    await controller.start();
+    expect(platform.startCalls, 1);
+    expect(controller.hasPermission, isFalse);
+
+    platform.permission = true;
+    final granted = await controller.requestPermission();
+
+    expect(granted, isTrue);
+    expect(platform.permissionRequests, 1);
+    expect(platform.startCalls, 2);
+    expect(controller.hasPermission, isTrue);
+    controller.stop();
+  });
+
   testWidgets('GPS card displays 0 km/h', (tester) async {
     await tester.pumpWidget(
       MaterialApp(

+ 8 - 2
test/settings_screen_test.dart

@@ -18,6 +18,7 @@ import 'package:cardash_launcher/data/services/native_bridge.dart';
 import 'package:cardash_launcher/data/services/pin_service.dart';
 import 'package:cardash_launcher/data/services/secure_store.dart';
 import 'package:cardash_launcher/data/services/telemetry_bridge.dart';
+import 'package:cardash_launcher/features/home/gps_speed_controller.dart';
 import 'package:cardash_launcher/features/home/launcher_controller.dart';
 import 'package:cardash_launcher/features/home/obd_controller.dart';
 import 'package:cardash_launcher/features/home/system_controller.dart';
@@ -194,9 +195,13 @@ void main() {
       final launcher = LauncherController(platform, settings);
       final system = SystemController(platform);
       final obd = ObdController(telemetry, settings)..initialize();
+      final gps = GpsSpeedController(telemetry);
       final weather = WeatherController(telemetry);
-      final telemetrySettings = TelemetrySettingsController(settings, weather)
-        ..initialize();
+      final telemetrySettings = TelemetrySettingsController(
+        settings,
+        weather,
+        gps,
+      )..initialize();
       final update = UpdateController(platform);
       final lock = LockController(PinService(_SecureStore(), iterations: 10));
       await lock.initialize();
@@ -239,6 +244,7 @@ void main() {
       expect(tester.takeException(), isNull);
 
       obd.dispose();
+      gps.dispose();
       weather.dispose();
       telemetrySettings.dispose();
     },

+ 106 - 5
test/telemetry_settings_controller_test.dart

@@ -6,6 +6,7 @@ import 'package:cardash_launcher/data/models/obd_state.dart';
 import 'package:cardash_launcher/data/repositories/settings_repository.dart';
 import 'package:cardash_launcher/data/services/telemetry_bridge.dart';
 import 'package:cardash_launcher/data/services/weather_service.dart';
+import 'package:cardash_launcher/features/home/gps_speed_controller.dart';
 import 'package:cardash_launcher/features/home/telemetry_settings_controller.dart';
 import 'package:cardash_launcher/features/home/weather_controller.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -14,10 +15,14 @@ import 'package:shared_preferences/shared_preferences.dart';
 class _TelemetryPlatform implements TelemetryPlatform {
   _TelemetryPlatform({this.locationGranted = true});
 
-  final bool locationGranted;
+  bool locationGranted;
+  int locationRequests = 0;
 
   @override
-  Future<bool> ensureLocationPermission() async => locationGranted;
+  Future<bool> ensureLocationPermission() async {
+    locationRequests++;
+    return locationGranted;
+  }
 
   @override
   Future<LocationReading> getLocation() async =>
@@ -74,9 +79,11 @@ void main() {
       _TelemetryPlatform(),
       weatherService: _WeatherService(),
     );
-    final controller = TelemetrySettingsController(settings, weather)
+    final gps = GpsSpeedController(_TelemetryPlatform());
+    final controller = TelemetrySettingsController(settings, weather, gps)
       ..initialize();
     addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
     addTearDown(weather.dispose);
 
     expect(controller.speedEnabled, isTrue);
@@ -105,9 +112,11 @@ void main() {
       _TelemetryPlatform(),
       weatherService: _WeatherService(),
     );
-    final controller = TelemetrySettingsController(settings, weather)
+    final gps = GpsSpeedController(_TelemetryPlatform());
+    final controller = TelemetrySettingsController(settings, weather, gps)
       ..initialize();
     addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
     addTearDown(weather.dispose);
 
     await controller.setTemperatureUseObd(false);
@@ -133,9 +142,11 @@ void main() {
         _TelemetryPlatform(locationGranted: false),
         weatherService: _WeatherService(),
       );
-      final controller = TelemetrySettingsController(settings, weather)
+      final gps = GpsSpeedController(_TelemetryPlatform());
+      final controller = TelemetrySettingsController(settings, weather, gps)
         ..initialize();
       addTearDown(controller.dispose);
+      addTearDown(gps.dispose);
       addTearDown(weather.dispose);
 
       final granted = await controller.setTemperatureUseObd(false);
@@ -146,4 +157,94 @@ void main() {
       expect(controller.weatherError, isNotNull);
     },
   );
+
+  test('enabling speed asks for location without an OBD adapter', () async {
+    SharedPreferences.setMockInitialValues({});
+    final prefs = await SharedPreferences.getInstance();
+    final settings = SettingsRepository(prefs);
+    await settings.setSpeedEnabled(false);
+    final platform = _TelemetryPlatform();
+    final weather = WeatherController(platform);
+    final gps = GpsSpeedController(platform);
+    final controller = TelemetrySettingsController(settings, weather, gps)
+      ..initialize();
+    addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
+    addTearDown(weather.dispose);
+
+    final granted = await controller.setSpeedEnabled(true);
+
+    expect(granted, isTrue);
+    expect(platform.locationRequests, 1);
+    expect(controller.speedEnabled, isTrue);
+    expect(settings.speedEnabled, isTrue);
+  });
+
+  test('enabling speed does not ask when OBD speed is selected', () async {
+    SharedPreferences.setMockInitialValues({});
+    final prefs = await SharedPreferences.getInstance();
+    final settings = SettingsRepository(prefs);
+    await settings.setSpeedEnabled(false);
+    await settings.setObdAdapterAddress('AA:BB:CC:DD:EE:FF');
+    final platform = _TelemetryPlatform();
+    final weather = WeatherController(platform);
+    final gps = GpsSpeedController(platform);
+    final controller = TelemetrySettingsController(settings, weather, gps)
+      ..initialize();
+    addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
+    addTearDown(weather.dispose);
+
+    final granted = await controller.setSpeedEnabled(true);
+
+    expect(granted, isTrue);
+    expect(platform.locationRequests, 0);
+    expect(controller.speedEnabled, isTrue);
+  });
+
+  test('denied location keeps speed on and reports the failure', () async {
+    SharedPreferences.setMockInitialValues({});
+    final prefs = await SharedPreferences.getInstance();
+    final settings = SettingsRepository(prefs);
+    await settings.setSpeedEnabled(false);
+    final platform = _TelemetryPlatform(locationGranted: false);
+    final weather = WeatherController(platform);
+    final gps = GpsSpeedController(platform);
+    final controller = TelemetrySettingsController(settings, weather, gps)
+      ..initialize();
+    addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
+    addTearDown(weather.dispose);
+
+    final granted = await controller.setSpeedEnabled(true);
+
+    expect(granted, isFalse);
+    expect(platform.locationRequests, 1);
+    expect(controller.speedEnabled, isTrue);
+    expect(settings.speedEnabled, isTrue);
+  });
+
+  test('choosing GPS speed asks for location while speed is on', () async {
+    SharedPreferences.setMockInitialValues({});
+    final prefs = await SharedPreferences.getInstance();
+    final settings = SettingsRepository(prefs);
+    await settings.setObdAdapterAddress('AA:BB:CC:DD:EE:FF');
+    final platform = _TelemetryPlatform();
+    final weather = WeatherController(platform);
+    final gps = GpsSpeedController(platform);
+    final controller = TelemetrySettingsController(settings, weather, gps)
+      ..initialize();
+    addTearDown(controller.dispose);
+    addTearDown(gps.dispose);
+    addTearDown(weather.dispose);
+
+    final granted = await controller.setSpeedUseObd(false);
+
+    expect(granted, isTrue);
+    expect(platform.locationRequests, 1);
+    expect(controller.speedUseObd, isFalse);
+
+    expect(await controller.setSpeedUseObd(true), isTrue);
+    expect(platform.locationRequests, 1);
+  });
 }