From 49feb3a56aef8213265022e8a5fb9ded40a82871 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 27 May 2026 22:40:28 -0400 Subject: [PATCH 1/5] feat(location): add one-time current location detection --- CHANGELOG.md | 1 + installer/build_nuitka.py | 4 + pyproject.toml | 2 + src/accessiweather/current_location.py | 288 ++++++++++++++++++ .../ui/dialogs/location_dialog.py | 120 ++++++++ tests/test_current_location.py | 144 +++++++++ tests/test_nuitka_build.py | 4 + 7 files changed, 563 insertions(+) create mode 100644 src/accessiweather/current_location.py create mode 100644 tests/test_current_location.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c72419d8f..03ba5f4fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] ### Added +- You can now choose "Use my current location" when adding or updating a saved location. AccessiWeather asks the OS once, only after you press the button, and keeps manual search available if location access is denied or unavailable. - Alert updates can now use their own mappable sound in sound packs. - Settings > Audio can now enable specific-alert sounds per sound pack, so packs with sounds like `tornado_watch` and `tornado_warning` keep working while severity-only packs stay simple. - First-run setup can now import existing settings and encrypted API keys from the wizard, or exit with Escape at any wizard step when you want to configure everything yourself. diff --git a/installer/build_nuitka.py b/installer/build_nuitka.py index ae05c1e35..4565c3b49 100644 --- a/installer/build_nuitka.py +++ b/installer/build_nuitka.py @@ -242,6 +242,10 @@ def build_nuitka_command( [ f"--macos-app-name={APP_NAME}", f"--macos-app-icon={_repo_path(icon)}", + "--macos-app-protected-resource=" + "NSLocationWhenInUseUsageDescription:" + "AccessiWeather uses your location only when you choose Use my current location " + "to add or edit a saved weather location.", ] ) elif system == "Linux": diff --git a/pyproject.toml b/pyproject.toml index 705a50520..ad48a3148 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,6 +27,8 @@ dependencies = [ "tzdata", "cryptography>=42.0.0", "Unidecode>=1.3.8", + "winsdk; sys_platform == 'win32'", + "pyobjc-framework-CoreLocation; sys_platform == 'darwin'", ] readme = "README.md" classifiers = [ diff --git a/src/accessiweather/current_location.py b/src/accessiweather/current_location.py new file mode 100644 index 000000000..bdcb838f1 --- /dev/null +++ b/src/accessiweather/current_location.py @@ -0,0 +1,288 @@ +"""One-time native current-location detection helpers.""" + +from __future__ import annotations + +import asyncio +import importlib +import logging +import sys +import threading +from dataclasses import dataclass +from enum import Enum +from typing import Any, Protocol + +from .models import Location + +logger = logging.getLogger(__name__) + + +class LocationDetectionStatus(Enum): + """Outcome of a one-time current-location request.""" + + SUCCESS = "success" + DENIED = "denied" + TIMEOUT = "timeout" + UNAVAILABLE = "unavailable" + UNSUPPORTED = "unsupported" + + +@dataclass(frozen=True) +class CurrentCoordinates: + """Coordinates returned by a native location provider.""" + + latitude: float + longitude: float + accuracy_meters: float | None = None + + +@dataclass(frozen=True) +class CurrentLocationResult: + """Normalized result for the UI.""" + + status: LocationDetectionStatus + message: str + coordinates: CurrentCoordinates | None = None + location: Location | None = None + + +class CurrentLocationError(Exception): + """Expected provider failure normalized for UI handling.""" + + def __init__(self, status: LocationDetectionStatus, message: str) -> None: + """Store the normalized status alongside the user-facing message.""" + super().__init__(message) + self.status = status + self.message = message + + +class CurrentLocationProvider(Protocol): + """Protocol implemented by native one-shot providers.""" + + async def detect(self, timeout_seconds: float = 15.0) -> CurrentCoordinates: + """Return current coordinates or raise ``CurrentLocationError``.""" + raise NotImplementedError + + +def location_from_coordinates( + coordinates: CurrentCoordinates, + *, + name: str | None = None, +) -> Location: + """Create the same editable Location object used by manual location saves.""" + label = name or f"Current Location ({coordinates.latitude:.4f}, {coordinates.longitude:.4f})" + return Location(name=label, latitude=coordinates.latitude, longitude=coordinates.longitude) + + +class CurrentLocationService: + """Facade that normalizes native provider outcomes for the UI.""" + + def __init__(self, provider: CurrentLocationProvider | None = None) -> None: + """Initialize with an injected provider or the native provider for this platform.""" + self._provider = provider or get_native_provider() + + async def detect_once(self, timeout_seconds: float = 15.0) -> CurrentLocationResult: + """Run one user-initiated location request.""" + try: + coordinates = await self._provider.detect(timeout_seconds=timeout_seconds) + except CurrentLocationError as exc: + return CurrentLocationResult(status=exc.status, message=exc.message) + except TimeoutError: + return CurrentLocationResult( + status=LocationDetectionStatus.TIMEOUT, + message="Current location detection timed out. You can still search manually.", + ) + except Exception as exc: # noqa: BLE001 - native providers can fail many ways + logger.debug("Current location detection failed unexpectedly: %s", exc, exc_info=True) + return CurrentLocationResult( + status=LocationDetectionStatus.UNAVAILABLE, + message="Current location is unavailable. You can still search manually.", + ) + + location = location_from_coordinates(coordinates) + return CurrentLocationResult( + status=LocationDetectionStatus.SUCCESS, + message="Current location detected. Review the editable name before saving.", + coordinates=coordinates, + location=location, + ) + + +class UnsupportedLocationProvider: + """Provider used where native location detection is not implemented.""" + + async def detect(self, timeout_seconds: float = 15.0) -> CurrentCoordinates: + raise CurrentLocationError( + LocationDetectionStatus.UNSUPPORTED, + "Current location detection is not supported on this platform. You can still search manually.", + ) + + +class WindowsLocationProvider: + """One-shot Windows Location Services provider through WinRT when available.""" + + async def detect(self, timeout_seconds: float = 15.0) -> CurrentCoordinates: + try: + geolocation = importlib.import_module("winsdk.windows.devices.geolocation") + except Exception as exc: # noqa: BLE001 + raise CurrentLocationError( + LocationDetectionStatus.UNAVAILABLE, + "Windows Location Services support is not installed. You can still search manually.", + ) from exc + + try: + access = await asyncio.wait_for( + geolocation.Geolocator.request_access_async(), + timeout=timeout_seconds, + ) + except TimeoutError as exc: + raise CurrentLocationError( + LocationDetectionStatus.TIMEOUT, + "Windows Location Services did not respond. You can still search manually.", + ) from exc + + access_name = str(getattr(access, "name", access)).lower() + if "allowed" not in access_name: + raise CurrentLocationError( + LocationDetectionStatus.DENIED, + "Location permission was denied. You can still search manually.", + ) + + locator = geolocation.Geolocator() + if hasattr(geolocation, "PositionAccuracy"): + locator.desired_accuracy = geolocation.PositionAccuracy.DEFAULT + + try: + position = await asyncio.wait_for( + locator.get_geoposition_async(), + timeout=timeout_seconds, + ) + except TimeoutError as exc: + raise CurrentLocationError( + LocationDetectionStatus.TIMEOUT, + "Windows Location Services timed out. You can still search manually.", + ) from exc + + return _coordinates_from_windows_position(position) + + +def _coordinates_from_windows_position(position: Any) -> CurrentCoordinates: + """Extract latitude/longitude from common WinRT Python projections.""" + coordinate = getattr(position, "coordinate", None) + point = getattr(coordinate, "point", None) + point_position = getattr(point, "position", None) + + latitude = getattr(point_position, "latitude", None) + longitude = getattr(point_position, "longitude", None) + if latitude is None or longitude is None: + latitude = getattr(coordinate, "latitude", None) + longitude = getattr(coordinate, "longitude", None) + + if latitude is None or longitude is None: + raise CurrentLocationError( + LocationDetectionStatus.UNAVAILABLE, + "Windows Location Services returned no coordinates. You can still search manually.", + ) + + accuracy = getattr(coordinate, "accuracy", None) + return CurrentCoordinates( + latitude=float(latitude), + longitude=float(longitude), + accuracy_meters=float(accuracy) if accuracy is not None else None, + ) + + +class MacOSLocationProvider: + """One-shot CoreLocation provider through PyObjC when available.""" + + async def detect(self, timeout_seconds: float = 15.0) -> CurrentCoordinates: + try: + CoreLocation = importlib.import_module("CoreLocation") + Foundation = importlib.import_module("Foundation") + except Exception as exc: # noqa: BLE001 + raise CurrentLocationError( + LocationDetectionStatus.UNAVAILABLE, + "macOS CoreLocation support is not installed. You can still search manually.", + ) from exc + + loop = asyncio.get_running_loop() + future: asyncio.Future[CurrentCoordinates] = loop.create_future() + + class _LocationDelegate(Foundation.NSObject): + def locationManager_didUpdateLocations_(self, manager, locations): # noqa: N802 + latest = locations.lastObject() + coordinate = latest.coordinate() + accuracy = latest.horizontalAccuracy() + if not future.done(): + loop.call_soon_threadsafe( + future.set_result, + CurrentCoordinates( + latitude=float(coordinate.latitude), + longitude=float(coordinate.longitude), + accuracy_meters=float(accuracy) if accuracy >= 0 else None, + ), + ) + manager.stopUpdatingLocation() + + def locationManager_didFailWithError_(self, manager, error): # noqa: N802 + if not future.done(): + loop.call_soon_threadsafe( + future.set_exception, + CurrentLocationError( + LocationDetectionStatus.UNAVAILABLE, + "macOS could not detect your current location. You can still search manually.", + ), + ) + manager.stopUpdatingLocation() + + def locationManagerDidChangeAuthorization_(self, manager): # noqa: N802 + status = manager.authorizationStatus() + denied_statuses = { + getattr(CoreLocation, "kCLAuthorizationStatusDenied", object()), + getattr(CoreLocation, "kCLAuthorizationStatusRestricted", object()), + } + if status in denied_statuses and not future.done(): + loop.call_soon_threadsafe( + future.set_exception, + CurrentLocationError( + LocationDetectionStatus.DENIED, + "Location permission was denied. You can still search manually.", + ), + ) + + manager = CoreLocation.CLLocationManager.alloc().init() + delegate = _LocationDelegate.alloc().init() + manager.setDelegate_(delegate) + + def _run_core_location() -> None: + manager.requestWhenInUseAuthorization() + if hasattr(manager, "requestLocation"): + manager.requestLocation() + else: + manager.startUpdatingLocation() + Foundation.NSRunLoop.currentRunLoop().runUntilDate_( + Foundation.NSDate.dateWithTimeIntervalSinceNow_(timeout_seconds) + ) + + thread = threading.Thread(target=_run_core_location, daemon=True) + thread.start() + + try: + return await asyncio.wait_for(future, timeout=timeout_seconds + 0.5) + except TimeoutError as exc: + manager.stopUpdatingLocation() + raise CurrentLocationError( + LocationDetectionStatus.TIMEOUT, + "macOS location detection timed out. You can still search manually.", + ) from exc + finally: + # Keep delegate alive until the future completes; then break the reference cycle. + manager.setDelegate_(None) + + +def get_native_provider() -> CurrentLocationProvider: + """Return the provider for the current platform.""" + if sys.platform == "win32": + return WindowsLocationProvider() + if sys.platform == "darwin": + return MacOSLocationProvider() + return UnsupportedLocationProvider() diff --git a/src/accessiweather/ui/dialogs/location_dialog.py b/src/accessiweather/ui/dialogs/location_dialog.py index dc4454773..ef2a1c02f 100644 --- a/src/accessiweather/ui/dialogs/location_dialog.py +++ b/src/accessiweather/ui/dialogs/location_dialog.py @@ -8,6 +8,8 @@ import wx +from ...current_location import CurrentLocationService, LocationDetectionStatus + if TYPE_CHECKING: from ...app import AccessiWeatherApp from ...models import Location @@ -132,10 +134,12 @@ def __init__(self, parent, app: AccessiWeatherApp, location: Location): self._selected_location: Location | None = None self._search_results: list[Location] = [] self._is_searching = False + self._is_detecting_current_location = False from ...location_manager import LocationManager self.location_manager = LocationManager() + self.current_location_service = CurrentLocationService() panel = wx.Panel(self) sizer = wx.BoxSizer(wx.VERTICAL) @@ -185,6 +189,18 @@ def __init__(self, parent, app: AccessiWeatherApp, location: Location): search_row.Add(self.address_search_button, 0) address_sizer.Add(search_row, 0, wx.EXPAND | wx.ALL, 5) + self.current_location_button = wx.Button( + address_parent, + label="Use my current location", + ) + self.current_location_button.SetToolTip( + "Ask the operating system once for your current coordinates. " + "The existing coordinates stay available if this is unavailable or denied." + ) + self.current_location_button.SetName("Use my current location for this saved location") + self.current_location_button.Bind(wx.EVT_BUTTON, self._on_use_current_location) + address_sizer.Add(self.current_location_button, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) + self.address_results_list = wx.ListCtrl( address_parent, style=wx.LC_REPORT | wx.LC_SINGLE_SEL | wx.BORDER_SUNKEN, @@ -262,6 +278,51 @@ def _on_address_search(self, event) -> None: self.app.run_async(self._do_address_search(query)) + def _on_use_current_location(self, event) -> None: + """Handle one-time current-location detection for an existing location.""" + if self._is_detecting_current_location: + self._set_coordinate_comparison( + "Current location detection is already in progress.", + is_error=True, + ) + return + + self._is_detecting_current_location = True + self.current_location_button.Disable() + self._set_coordinate_comparison( + "Requesting current location. Your system may ask for permission now..." + ) + self.app.run_async(self._do_current_location_detection()) + + async def _do_current_location_detection(self) -> None: + """Run native location detection once.""" + result = await self.current_location_service.detect_once() + if result.status is LocationDetectionStatus.SUCCESS and result.location is not None: + wx.CallAfter(self._on_current_location_detected, result.location) + return + wx.CallAfter(self._on_current_location_error, result.message) + + def _on_current_location_detected(self, location: Location) -> None: + """Apply detected coordinates as the pending coordinate update.""" + self._is_detecting_current_location = False + self.current_location_button.Enable() + self._selected_location = location + self._search_results = [location] + self.address_results_list.DeleteAllItems() + coords = self.location_manager.format_coordinates(location.latitude, location.longitude) + index = self.address_results_list.InsertItem(0, location.name) + self.address_results_list.SetItem(index, 1, coords) + distance = self.location_manager.calculate_distance(self._location, location) + self._set_coordinate_comparison( + f"Detected current location: {coords}. Difference from saved coordinates: {distance:.2f} miles." + ) + + def _on_current_location_error(self, message: str) -> None: + """Handle unavailable, denied, unsupported, or timed-out detection.""" + self._is_detecting_current_location = False + self.current_location_button.Enable() + self._set_coordinate_comparison(message, is_error=True) + async def _do_address_search(self, query: str) -> None: """Perform the address lookup.""" try: @@ -372,11 +433,13 @@ def __init__(self, parent, app: AccessiWeatherApp): self._search_results = [] self._selected_location = None self._is_searching = False + self._is_detecting_current_location = False # Import LocationManager from ...location_manager import LocationManager self.location_manager = LocationManager() + self.current_location_service = CurrentLocationService() self._create_ui() self._setup_accessibility() @@ -436,6 +499,14 @@ def _create_ui(self): search_sizer.Add(search_row, 0, wx.EXPAND) + self.current_location_button = wx.Button(panel, label="Use my current location") + self.current_location_button.SetToolTip( + "Ask the operating system once for your current coordinates. " + "Manual search stays available if this is unavailable or denied." + ) + self.current_location_button.Bind(wx.EVT_BUTTON, self._on_use_current_location) + search_sizer.Add(self.current_location_button, 0, wx.TOP, 8) + search_help = wx.StaticText( panel, label="Examples: 'London', 'New York', '10001', or '123 Main St, Carrollton, TX'", @@ -488,6 +559,7 @@ def _setup_accessibility(self): self.search_input.SetName("Search for location") self.results_list.SetName("Search results") self.marine_mode_checkbox.SetName("Enable Marine Mode for this location") + self.current_location_button.SetName("Use my current location") def _on_key(self, event: wx.KeyEvent) -> None: """Handle key events.""" @@ -531,6 +603,54 @@ def _on_search(self, event): # Run async search self.app.run_async(self._do_search(query)) + def _on_use_current_location(self, event) -> None: + """Handle one-time current-location detection button press.""" + if self._is_detecting_current_location: + self._update_status("Current location detection is already in progress.", is_error=True) + return + + self._is_detecting_current_location = True + self.current_location_button.Disable() + self._update_status( + "Requesting current location. Your system may ask for permission now..." + ) + self.app.run_async(self._do_current_location_detection()) + + async def _do_current_location_detection(self) -> None: + """Run native location detection once.""" + result = await self.current_location_service.detect_once() + if result.status is LocationDetectionStatus.SUCCESS and result.location is not None: + wx.CallAfter(self._on_current_location_detected, result.location) + return + wx.CallAfter(self._on_current_location_error, result.message) + + def _on_current_location_detected(self, location: Location) -> None: + """Handle successful current-location detection.""" + self._is_detecting_current_location = False + self.current_location_button.Enable() + self._apply_detected_location(location) + + def _apply_detected_location(self, location: Location) -> None: + """Populate the normal add-location fields from detected coordinates.""" + self._selected_location = location + self._search_results = [location] + self.results_list.DeleteAllItems() + coords_str = self.location_manager.format_coordinates( + location.latitude, + location.longitude, + ) + index = self.results_list.InsertItem(0, location.name) + self.results_list.SetItem(index, 1, coords_str) + if not self.name_input.GetValue().strip(): + self.name_input.SetValue(location.name) + self._update_status("Detected current location. Review the editable name, then save it.") + + def _on_current_location_error(self, message: str) -> None: + """Handle unavailable, denied, unsupported, or timed-out detection.""" + self._is_detecting_current_location = False + self.current_location_button.Enable() + self._update_status(message, is_error=True) + async def _do_search(self, query: str): """Perform the location search.""" try: diff --git a/tests/test_current_location.py b/tests/test_current_location.py new file mode 100644 index 000000000..7526452a1 --- /dev/null +++ b/tests/test_current_location.py @@ -0,0 +1,144 @@ +"""Tests for one-time current-location detection.""" + +from __future__ import annotations + +import logging +from typing import cast +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from accessiweather.config.config_manager import ConfigManager +from accessiweather.config.locations import LocationOperations +from accessiweather.models import AppConfig, AppSettings + + +class _FakeConfigManager: + """Minimal stand-in for ``ConfigManager`` used by location save tests.""" + + def __init__(self) -> None: + self.save_calls = 0 + self._config = AppConfig(settings=AppSettings(), locations=[], current_location=None) + + def get_config(self) -> AppConfig: + return self._config + + def save_config(self) -> bool: + self.save_calls += 1 + return True + + def _get_logger(self) -> logging.Logger: + return logging.getLogger("accessiweather.config.test") + + +class TestCurrentLocationProviders: + async def test_factory_returns_unsupported_provider_on_linux(self, monkeypatch): + from accessiweather.current_location import UnsupportedLocationProvider, get_native_provider + + monkeypatch.setattr("accessiweather.current_location.sys.platform", "linux") + + provider = get_native_provider() + + assert isinstance(provider, UnsupportedLocationProvider) + + async def test_service_returns_unavailable_result_when_provider_fails(self): + from accessiweather.current_location import ( + CurrentLocationError, + CurrentLocationService, + LocationDetectionStatus, + ) + + provider = MagicMock() + provider.detect = AsyncMock( + side_effect=CurrentLocationError( + LocationDetectionStatus.DENIED, + "Location permission was denied.", + ) + ) + service = CurrentLocationService(provider=provider) + + result = await service.detect_once() + + assert result.status is LocationDetectionStatus.DENIED + assert result.coordinates is None + assert "denied" in result.message.lower() + provider.detect.assert_awaited_once() + + async def test_service_converts_coordinates_to_editable_location(self): + from accessiweather.current_location import ( + CurrentCoordinates, + CurrentLocationService, + LocationDetectionStatus, + ) + + provider = MagicMock() + provider.detect = AsyncMock( + return_value=CurrentCoordinates( + latitude=39.9526, longitude=-75.1652, accuracy_meters=25 + ) + ) + service = CurrentLocationService(provider=provider) + + result = await service.detect_once() + + assert result.status is LocationDetectionStatus.SUCCESS + assert result.location is not None + assert result.location.name == "Current Location (39.9526, -75.1652)" + assert result.location.latitude == pytest.approx(39.9526) + assert result.location.longitude == pytest.approx(-75.1652) + + +class TestCurrentLocationSaveFlow: + async def test_detected_location_saves_like_manual_location_with_enrichment(self): + from accessiweather.current_location import CurrentCoordinates, location_from_coordinates + + detected = location_from_coordinates( + CurrentCoordinates(latitude=39.9526, longitude=-75.1652), + name="Home", + ) + mock_service = MagicMock() + mock_service.enrich_location = AsyncMock(side_effect=lambda loc: loc) + manager = _FakeConfigManager() + ops = LocationOperations(cast(ConfigManager, manager), zone_enrichment_service=mock_service) + + ok = await ops.add_location_with_enrichment( + detected.name, + detected.latitude, + detected.longitude, + country_code=detected.country_code, + ) + + assert ok is True + saved = manager.get_config().locations[0] + assert saved.name == "Home" + assert saved.latitude == pytest.approx(39.9526) + assert saved.longitude == pytest.approx(-75.1652) + assert manager.get_config().current_location is saved + + +class TestAddLocationDialogCurrentLocation: + def test_detected_location_prefills_editable_name_and_selection(self): + from accessiweather.current_location import CurrentCoordinates, location_from_coordinates + from accessiweather.ui.dialogs.location_dialog import AddLocationDialog + + dialog = AddLocationDialog.__new__(AddLocationDialog) + dialog.name_input = MagicMock() + dialog.name_input.GetValue.return_value = "" + dialog.results_list = MagicMock() + dialog.location_manager = MagicMock() + dialog.location_manager.format_coordinates.return_value = "39.9526N, 75.1652W" + dialog._update_status = MagicMock() + dialog._selected_location = None + dialog._search_results = [] + + detected = location_from_coordinates( + CurrentCoordinates(latitude=39.9526, longitude=-75.1652) + ) + + dialog._apply_detected_location(detected) + + assert dialog._selected_location is detected + dialog.name_input.SetValue.assert_called_once_with(detected.name) + dialog._update_status.assert_called_once_with( + "Detected current location. Review the editable name, then save it." + ) diff --git a/tests/test_nuitka_build.py b/tests/test_nuitka_build.py index a3ad52bfa..f55bc8d3a 100644 --- a/tests/test_nuitka_build.py +++ b/tests/test_nuitka_build.py @@ -70,6 +70,10 @@ def test_macos_nuitka_command_uses_app_mode() -> None: assert "--mode=app" in command assert "--macos-create-app-bundle" not in command assert "--macos-app-name=AccessiWeather" in command + assert any( + item.startswith("--macos-app-protected-resource=NSLocationWhenInUseUsageDescription:") + for item in command + ) assert "--include-data-dir=src/accessiweather/resources=accessiweather/resources" not in command From 4a02f6e8a947edcaadcf36869d249a7a63d6071f Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 27 May 2026 22:40:36 -0400 Subject: [PATCH 2/5] test(single-instance): isolate fallback handoff from live IPC --- tests/test_single_instance.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_single_instance.py b/tests/test_single_instance.py index e2f794afe..b88557207 100644 --- a/tests/test_single_instance.py +++ b/tests/test_single_instance.py @@ -204,6 +204,7 @@ def test_second_windows_launch_writes_generic_handoff_when_window_lookup_fails( monkeypatch.setattr(single_instance.sys, "platform", "win32") monkeypatch.setattr(single_instance, "ctypes", _FakeCtypes(kernel32, user32)) + monkeypatch.setattr(single_instance, "_send_activation_request_ipc", lambda request: False) manager = SingleInstanceManager(app, runtime_paths=runtime_paths) From ecbf42d8c6176d4089ebc9e60b1bd2a7c6208309 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 27 May 2026 22:49:08 -0400 Subject: [PATCH 3/5] test(location): cover native current location providers --- tests/test_current_location.py | 460 +++++++++++++++++++++++++++++++++ 1 file changed, 460 insertions(+) diff --git a/tests/test_current_location.py b/tests/test_current_location.py index 7526452a1..cfa1b65d3 100644 --- a/tests/test_current_location.py +++ b/tests/test_current_location.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +from types import SimpleNamespace from typing import cast from unittest.mock import AsyncMock, MagicMock @@ -31,7 +32,123 @@ def _get_logger(self) -> logging.Logger: return logging.getLogger("accessiweather.config.test") +class _ImmediateThread: + """Thread stand-in that runs the CoreLocation target synchronously.""" + + def __init__(self, target, daemon: bool) -> None: + self._target = target + self.daemon = daemon + + def start(self) -> None: + self._target() + + +class _FakeNSObject: + @classmethod + def alloc(cls): + return cls() + + def init(self): + return self + + +class _FakeRunLoop: + @classmethod + def currentRunLoop(cls): + return cls() + + def runUntilDate_(self, date) -> None: + return None + + +class _FakeDate: + @staticmethod + def dateWithTimeIntervalSinceNow_(timeout_seconds): + return timeout_seconds + + +class _FakeLocation: + def __init__(self, *, accuracy: float) -> None: + self._accuracy = accuracy + + def coordinate(self): + return SimpleNamespace(latitude=39.9526, longitude=-75.1652) + + def horizontalAccuracy(self): + return self._accuracy + + +class _FakeLocations: + def __init__(self, *, accuracy: float) -> None: + self._accuracy = accuracy + + def lastObject(self): + return _FakeLocation(accuracy=self._accuracy) + + +def _fake_core_location_modules(mode: str): + class FakeManager(_FakeNSObject): + def __init__(self) -> None: + self.delegate = None + self.stopped = False + + def setDelegate_(self, delegate) -> None: + self.delegate = delegate + + def requestWhenInUseAuthorization(self) -> None: + return None + + def authorizationStatus(self): + return "denied" + + def stopUpdatingLocation(self) -> None: + self.stopped = True + + if mode != "success-start-updating": + + def requestLocation(self) -> None: + if mode == "success-request-location": + self.delegate.locationManager_didUpdateLocations_( + self, _FakeLocations(accuracy=15) + ) + elif mode == "failure": + self.delegate.locationManager_didFailWithError_(self, object()) + elif mode == "denied": + self.delegate.locationManagerDidChangeAuthorization_(self) + + def startUpdatingLocation(self) -> None: + if mode == "success-start-updating": + self.delegate.locationManager_didUpdateLocations_(self, _FakeLocations(accuracy=-1)) + + fake_foundation = SimpleNamespace( + NSObject=_FakeNSObject, + NSRunLoop=_FakeRunLoop, + NSDate=_FakeDate, + ) + fake_core_location = SimpleNamespace( + CLLocationManager=FakeManager, + kCLAuthorizationStatusDenied="denied", + kCLAuthorizationStatusRestricted="restricted", + ) + return fake_foundation, fake_core_location + + class TestCurrentLocationProviders: + async def test_unsupported_provider_reports_manual_fallback(self): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + UnsupportedLocationProvider, + ) + + provider = UnsupportedLocationProvider() + + with pytest.raises(CurrentLocationError) as exc_info: + await provider.detect() + + assert exc_info.value.status is LocationDetectionStatus.UNSUPPORTED + assert "search manually" in exc_info.value.message + async def test_factory_returns_unsupported_provider_on_linux(self, monkeypatch): from accessiweather.current_location import UnsupportedLocationProvider, get_native_provider @@ -41,6 +158,24 @@ async def test_factory_returns_unsupported_provider_on_linux(self, monkeypatch): assert isinstance(provider, UnsupportedLocationProvider) + async def test_factory_returns_windows_provider_on_windows(self, monkeypatch): + from accessiweather.current_location import WindowsLocationProvider, get_native_provider + + monkeypatch.setattr("accessiweather.current_location.sys.platform", "win32") + + provider = get_native_provider() + + assert isinstance(provider, WindowsLocationProvider) + + async def test_factory_returns_macos_provider_on_macos(self, monkeypatch): + from accessiweather.current_location import MacOSLocationProvider, get_native_provider + + monkeypatch.setattr("accessiweather.current_location.sys.platform", "darwin") + + provider = get_native_provider() + + assert isinstance(provider, MacOSLocationProvider) + async def test_service_returns_unavailable_result_when_provider_fails(self): from accessiweather.current_location import ( CurrentLocationError, @@ -64,6 +199,30 @@ async def test_service_returns_unavailable_result_when_provider_fails(self): assert "denied" in result.message.lower() provider.detect.assert_awaited_once() + async def test_service_returns_timeout_result_for_plain_timeout(self): + from accessiweather.current_location import CurrentLocationService, LocationDetectionStatus + + provider = MagicMock() + provider.detect = AsyncMock(side_effect=TimeoutError) + service = CurrentLocationService(provider=provider) + + result = await service.detect_once() + + assert result.status is LocationDetectionStatus.TIMEOUT + assert "timed out" in result.message + + async def test_service_returns_unavailable_result_for_unexpected_failure(self): + from accessiweather.current_location import CurrentLocationService, LocationDetectionStatus + + provider = MagicMock() + provider.detect = AsyncMock(side_effect=RuntimeError("native backend exploded")) + service = CurrentLocationService(provider=provider) + + result = await service.detect_once() + + assert result.status is LocationDetectionStatus.UNAVAILABLE + assert "unavailable" in result.message + async def test_service_converts_coordinates_to_editable_location(self): from accessiweather.current_location import ( CurrentCoordinates, @@ -88,6 +247,307 @@ async def test_service_converts_coordinates_to_editable_location(self): assert result.location.longitude == pytest.approx(-75.1652) +class TestWindowsCurrentLocationProvider: + def test_coordinates_from_windows_position_uses_point_position(self): + from accessiweather.current_location import _coordinates_from_windows_position + + position = SimpleNamespace( + coordinate=SimpleNamespace( + point=SimpleNamespace( + position=SimpleNamespace(latitude=39.9526, longitude=-75.1652) + ), + accuracy=12.5, + ) + ) + + coordinates = _coordinates_from_windows_position(position) + + assert coordinates.latitude == pytest.approx(39.9526) + assert coordinates.longitude == pytest.approx(-75.1652) + assert coordinates.accuracy_meters == pytest.approx(12.5) + + def test_coordinates_from_windows_position_uses_projection_fallback(self): + from accessiweather.current_location import _coordinates_from_windows_position + + position = SimpleNamespace( + coordinate=SimpleNamespace(latitude=40.0, longitude=-76.0, accuracy=None) + ) + + coordinates = _coordinates_from_windows_position(position) + + assert coordinates.latitude == pytest.approx(40.0) + assert coordinates.longitude == pytest.approx(-76.0) + assert coordinates.accuracy_meters is None + + def test_coordinates_from_windows_position_rejects_missing_coordinates(self): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + _coordinates_from_windows_position, + ) + + with pytest.raises(CurrentLocationError) as exc_info: + _coordinates_from_windows_position(SimpleNamespace(coordinate=SimpleNamespace())) + + assert exc_info.value.status is LocationDetectionStatus.UNAVAILABLE + + async def test_windows_provider_reports_unavailable_when_winsdk_missing(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + WindowsLocationProvider, + ) + + def fake_import(name: str): + if name == "winsdk.windows.devices.geolocation": + raise ModuleNotFoundError(name) + raise AssertionError(name) + + monkeypatch.setattr("accessiweather.current_location.importlib.import_module", fake_import) + + with pytest.raises(CurrentLocationError) as exc_info: + await WindowsLocationProvider().detect() + + assert exc_info.value.status is LocationDetectionStatus.UNAVAILABLE + + async def test_windows_provider_reports_denied_permission(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + WindowsLocationProvider, + ) + + class FakeGeolocator: + @staticmethod + async def request_access_async(): + return SimpleNamespace(name="Denied") + + fake_geolocation = SimpleNamespace(Geolocator=FakeGeolocator) + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_geolocation, + ) + + with pytest.raises(CurrentLocationError) as exc_info: + await WindowsLocationProvider().detect() + + assert exc_info.value.status is LocationDetectionStatus.DENIED + + async def test_windows_provider_reports_access_timeout(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + WindowsLocationProvider, + ) + + class FakeGeolocator: + @staticmethod + async def request_access_async(): + return SimpleNamespace(name="Allowed") + + async def fake_wait_for(awaitable, timeout): + awaitable.close() + raise TimeoutError + + fake_geolocation = SimpleNamespace(Geolocator=FakeGeolocator) + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_geolocation, + ) + monkeypatch.setattr("accessiweather.current_location.asyncio.wait_for", fake_wait_for) + + with pytest.raises(CurrentLocationError) as exc_info: + await WindowsLocationProvider().detect() + + assert exc_info.value.status is LocationDetectionStatus.TIMEOUT + assert "did not respond" in exc_info.value.message + + async def test_windows_provider_reports_position_timeout(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + WindowsLocationProvider, + ) + + class FakeGeolocator: + @staticmethod + async def request_access_async(): + return SimpleNamespace(name="Allowed") + + async def get_geoposition_async(self): + return SimpleNamespace() + + calls = 0 + + async def fake_wait_for(awaitable, timeout): + nonlocal calls + calls += 1 + if calls == 1: + return await awaitable + awaitable.close() + raise TimeoutError + + fake_geolocation = SimpleNamespace(Geolocator=FakeGeolocator) + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_geolocation, + ) + monkeypatch.setattr("accessiweather.current_location.asyncio.wait_for", fake_wait_for) + + with pytest.raises(CurrentLocationError) as exc_info: + await WindowsLocationProvider().detect() + + assert exc_info.value.status is LocationDetectionStatus.TIMEOUT + assert "timed out" in exc_info.value.message + + async def test_windows_provider_returns_coordinates_when_allowed(self, monkeypatch): + from accessiweather.current_location import WindowsLocationProvider + + class FakeGeolocator: + desired_accuracy = None + + @staticmethod + async def request_access_async(): + return SimpleNamespace(name="Allowed") + + async def get_geoposition_async(self): + return SimpleNamespace( + coordinate=SimpleNamespace(latitude=39.9526, longitude=-75.1652, accuracy=8) + ) + + fake_geolocation = SimpleNamespace( + Geolocator=FakeGeolocator, + PositionAccuracy=SimpleNamespace(DEFAULT="default"), + ) + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_geolocation, + ) + + coordinates = await WindowsLocationProvider().detect() + + assert coordinates.latitude == pytest.approx(39.9526) + assert coordinates.longitude == pytest.approx(-75.1652) + assert coordinates.accuracy_meters == pytest.approx(8) + + +class TestMacOSCurrentLocationProvider: + async def test_macos_provider_reports_unavailable_when_pyobjc_missing(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + MacOSLocationProvider, + ) + + def fake_import(name: str): + if name == "CoreLocation": + raise ModuleNotFoundError(name) + raise AssertionError(name) + + monkeypatch.setattr("accessiweather.current_location.importlib.import_module", fake_import) + + with pytest.raises(CurrentLocationError) as exc_info: + await MacOSLocationProvider().detect() + + assert exc_info.value.status is LocationDetectionStatus.UNAVAILABLE + + async def test_macos_provider_returns_coordinates_from_request_location(self, monkeypatch): + from accessiweather.current_location import MacOSLocationProvider + + fake_foundation, fake_core_location = _fake_core_location_modules( + "success-request-location" + ) + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_core_location if name == "CoreLocation" else fake_foundation, + ) + monkeypatch.setattr("accessiweather.current_location.threading.Thread", _ImmediateThread) + + coordinates = await MacOSLocationProvider().detect(timeout_seconds=0.1) + + assert coordinates.latitude == pytest.approx(39.9526) + assert coordinates.longitude == pytest.approx(-75.1652) + assert coordinates.accuracy_meters == pytest.approx(15) + + async def test_macos_provider_uses_start_updating_location_fallback(self, monkeypatch): + from accessiweather.current_location import MacOSLocationProvider + + fake_foundation, fake_core_location = _fake_core_location_modules("success-start-updating") + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_core_location if name == "CoreLocation" else fake_foundation, + ) + monkeypatch.setattr("accessiweather.current_location.threading.Thread", _ImmediateThread) + + coordinates = await MacOSLocationProvider().detect(timeout_seconds=0.1) + + assert coordinates.latitude == pytest.approx(39.9526) + assert coordinates.longitude == pytest.approx(-75.1652) + assert coordinates.accuracy_meters is None + + async def test_macos_provider_reports_native_failure(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + MacOSLocationProvider, + ) + + fake_foundation, fake_core_location = _fake_core_location_modules("failure") + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_core_location if name == "CoreLocation" else fake_foundation, + ) + monkeypatch.setattr("accessiweather.current_location.threading.Thread", _ImmediateThread) + + with pytest.raises(CurrentLocationError) as exc_info: + await MacOSLocationProvider().detect(timeout_seconds=0.1) + + assert exc_info.value.status is LocationDetectionStatus.UNAVAILABLE + + async def test_macos_provider_reports_denied_authorization(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + MacOSLocationProvider, + ) + + fake_foundation, fake_core_location = _fake_core_location_modules("denied") + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_core_location if name == "CoreLocation" else fake_foundation, + ) + monkeypatch.setattr("accessiweather.current_location.threading.Thread", _ImmediateThread) + + with pytest.raises(CurrentLocationError) as exc_info: + await MacOSLocationProvider().detect(timeout_seconds=0.1) + + assert exc_info.value.status is LocationDetectionStatus.DENIED + + async def test_macos_provider_reports_timeout(self, monkeypatch): + from accessiweather.current_location import ( + CurrentLocationError, + LocationDetectionStatus, + MacOSLocationProvider, + ) + + fake_foundation, fake_core_location = _fake_core_location_modules("timeout") + + async def fake_wait_for(future, timeout): + raise TimeoutError + + monkeypatch.setattr( + "accessiweather.current_location.importlib.import_module", + lambda name: fake_core_location if name == "CoreLocation" else fake_foundation, + ) + monkeypatch.setattr("accessiweather.current_location.threading.Thread", _ImmediateThread) + monkeypatch.setattr("accessiweather.current_location.asyncio.wait_for", fake_wait_for) + + with pytest.raises(CurrentLocationError) as exc_info: + await MacOSLocationProvider().detect(timeout_seconds=0.1) + + assert exc_info.value.status is LocationDetectionStatus.TIMEOUT + + class TestCurrentLocationSaveFlow: async def test_detected_location_saves_like_manual_location_with_enrichment(self): from accessiweather.current_location import CurrentCoordinates, location_from_coordinates From 52d5abf6d70c297d06b771fc9361831787941200 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 27 May 2026 23:04:43 -0400 Subject: [PATCH 4/5] fix(location): refresh metadata when editing current location --- CHANGELOG.md | 1 + src/accessiweather/config/config_manager.py | 2 + src/accessiweather/config/locations.py | 27 +++++-- src/accessiweather/location_manager.py | 50 ++++++++++++ .../ui/dialogs/location_dialog.py | 57 +++++++++++--- .../ui/main_window_locations.py | 10 ++- tests/test_all_locations_view.py | 5 ++ tests/test_config_manager.py | 25 +++++- tests/test_current_location.py | 77 +++++++++++++++++++ ...test_location_manager_address_geocoding.py | 52 +++++++++++++ 10 files changed, 288 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03ba5f4fb..9f0c9943a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ All notable changes to this project will be documented in this file. - Alert sounds and sound-pack creation now focus on alert severity instead of a long list of specific alert names, while existing packs can keep their older mappings. ### Fixed +- Editing a location after using "Use my current location" now refreshes the editable name and saved US metadata for the detected point, so locations don't quietly fall back to metric defaults. - Notification sounds now recover after Windows sleep or hibernation, so update notifications don't fall back to only the generic Windows toast sound. - Plain Language Summary now shows and speaks which OpenRouter model it is trying, when it falls back from a busy free model, and why the final model was used. diff --git a/src/accessiweather/config/config_manager.py b/src/accessiweather/config/config_manager.py index f3e41b484..0b821d74c 100644 --- a/src/accessiweather/config/config_manager.py +++ b/src/accessiweather/config/config_manager.py @@ -303,6 +303,7 @@ def update_location_details( longitude: float, country_code: str | None, marine_mode: bool, + display_name: str | None = None, ) -> bool: """Update editable details on an existing location.""" return self._locations.update_location_details( @@ -311,6 +312,7 @@ def update_location_details( longitude=longitude, country_code=country_code, marine_mode=marine_mode, + display_name=display_name, ) def remove_location(self, name: str) -> bool: diff --git a/src/accessiweather/config/locations.py b/src/accessiweather/config/locations.py index 2b2b27ea6..1b2165eda 100644 --- a/src/accessiweather/config/locations.py +++ b/src/accessiweather/config/locations.py @@ -207,15 +207,24 @@ def update_location_details( longitude: float, country_code: str | None, marine_mode: bool, + display_name: str | None = None, ) -> bool: """ Update editable details on an existing location and persist them. - The location name is intentionally stable. When coordinates change, - cached NWS metadata is cleared so the next refresh resolves zones for - the new point instead of reusing the old city/ZIP point. + When coordinates change, cached NWS metadata is cleared so the next + refresh resolves zones for the new point instead of reusing the old + city/ZIP point. """ config = self._manager.get_config() + new_name = (display_name or name).strip() + if not new_name: + self.logger.warning("Location %s update rejected: empty display name", name) + return False + + if new_name != name and any(location.name == new_name for location in config.locations): + self.logger.warning("Location %s update rejected: %s already exists", name, new_name) + return False for location in config.locations: if location.name != name: @@ -225,7 +234,10 @@ def update_location_details( isclose(location.latitude, latitude, abs_tol=1e-6) and isclose(location.longitude, longitude, abs_tol=1e-6) ) + current = config.current_location + current_matches_location = current is not None and current.name == name + location.name = new_name location.latitude = latitude location.longitude = longitude location.country_code = country_code @@ -239,13 +251,16 @@ def update_location_details( location.fire_zone_id = None location.radar_station = None - current = config.current_location - if current is not None and current.name == name: + if current_matches_location: config.current_location = location + if new_name != name: + self._sort_locations(config) + self.logger.info( - "Updated location details for %s%s", + "Updated location details for %s%s%s", name, + f" as {new_name}" if new_name != name else "", " and cleared zone metadata" if coordinates_changed else "", ) return self._manager.save_config() diff --git a/src/accessiweather/location_manager.py b/src/accessiweather/location_manager.py index ca339ea27..f375cfc0d 100644 --- a/src/accessiweather/location_manager.py +++ b/src/accessiweather/location_manager.py @@ -168,6 +168,56 @@ async def _search_us_street_address(self, query: str, limit: int = 5) -> list[Lo return locations + async def reverse_geocode_coordinates( + self, + latitude: float, + longitude: float, + ) -> Location | None: + """Resolve coordinates to a friendly location label when a native source can name them.""" + url = f"https://api.weather.gov/points/{latitude},{longitude}" + headers = { + "User-Agent": "AccessiWeather/1.0 (AccessiWeather)", + "Accept": "application/geo+json", + } + + try: + async with httpx.AsyncClient(timeout=self.timeout, follow_redirects=True) as client: + response = await client.get(url, headers=headers) + except Exception as exc: # noqa: BLE001 - reverse lookup should never block manual save + logger.debug("Reverse geocoding failed for (%s, %s): %s", latitude, longitude, exc) + return None + + if response.status_code != 200: + logger.debug( + "Reverse geocoding returned status %s for (%s, %s)", + response.status_code, + latitude, + longitude, + ) + return None + + try: + properties = response.json().get("properties", {}) + except (AttributeError, ValueError) as exc: + logger.debug("Reverse geocoding returned invalid JSON: %s", exc) + return None + + relative = properties.get("relativeLocation", {}) + relative_properties = relative.get("properties", {}) if isinstance(relative, dict) else {} + city = str(relative_properties.get("city") or "").strip() + state = str(relative_properties.get("state") or "").strip() + if not city: + return None + + display_name = f"{city}, {state}" if state else city + return Location( + name=display_name, + latitude=latitude, + longitude=longitude, + timezone=properties.get("timeZone") or None, + country_code="US", + ) + def _parse_census_address_match(self, data: dict) -> Location | None: """Parse a Census Geocoder address match into a Location.""" try: diff --git a/src/accessiweather/ui/dialogs/location_dialog.py b/src/accessiweather/ui/dialogs/location_dialog.py index ef2a1c02f..e8cda48c3 100644 --- a/src/accessiweather/ui/dialogs/location_dialog.py +++ b/src/accessiweather/ui/dialogs/location_dialog.py @@ -21,6 +21,7 @@ class EditLocationResult: """Editable values returned by the edit location dialog.""" + display_name: str latitude: float longitude: float country_code: str | None @@ -144,8 +145,14 @@ def __init__(self, parent, app: AccessiWeatherApp, location: Location): panel = wx.Panel(self) sizer = wx.BoxSizer(wx.VERTICAL) - name_label = wx.StaticText(panel, label=f"Location: {location.name}") - sizer.Add(name_label, 0, wx.ALL, 10) + name_sizer = wx.BoxSizer(wx.VERTICAL) + name_label = wx.StaticText(panel, label="Location Name:") + name_sizer.Add(name_label, 0, wx.BOTTOM, 5) + self.name_input = wx.TextCtrl(panel, value=location.name, size=wx.Size(400, -1)) + self.name_input.SetHint("Enter a descriptive name for this location") + self.name_input.SetName("Location name input") + name_sizer.Add(self.name_input, 0, wx.EXPAND) + sizer.Add(name_sizer, 0, wx.ALL | wx.EXPAND, 10) self.current_coordinates_label = wx.StaticText( panel, @@ -298,10 +305,23 @@ async def _do_current_location_detection(self) -> None: """Run native location detection once.""" result = await self.current_location_service.detect_once() if result.status is LocationDetectionStatus.SUCCESS and result.location is not None: - wx.CallAfter(self._on_current_location_detected, result.location) + location = await self._resolve_detected_location(result.location) + wx.CallAfter(self._on_current_location_detected, location) return wx.CallAfter(self._on_current_location_error, result.message) + async def _resolve_detected_location(self, location: Location) -> Location: + """Prefer a reverse-geocoded label and metadata for detected coordinates.""" + try: + resolved = await self.location_manager.reverse_geocode_coordinates( + location.latitude, + location.longitude, + ) + except Exception as exc: # noqa: BLE001 - fallback label remains editable + logger.debug("Detected-location reverse geocoding failed: %s", exc) + return location + return resolved or location + def _on_current_location_detected(self, location: Location) -> None: """Apply detected coordinates as the pending coordinate update.""" self._is_detecting_current_location = False @@ -312,9 +332,12 @@ def _on_current_location_detected(self, location: Location) -> None: coords = self.location_manager.format_coordinates(location.latitude, location.longitude) index = self.address_results_list.InsertItem(0, location.name) self.address_results_list.SetItem(index, 1, coords) + self.name_input.SetValue(location.name) distance = self.location_manager.calculate_distance(self._location, location) self._set_coordinate_comparison( - f"Detected current location: {coords}. Difference from saved coordinates: {distance:.2f} miles." + f"Detected current location as {location.name}: {coords}. " + f"Difference from saved coordinates: {distance:.2f} miles. " + "Review the editable name, then save it." ) def _on_current_location_error(self, message: str) -> None: @@ -369,19 +392,21 @@ def _on_address_selected(self, event) -> None: if not 0 <= index < len(self._search_results): return - self._selected_location = self._search_results[index] + selected_location = self._search_results[index] + self._selected_location = selected_location distance = self.location_manager.calculate_distance( self._location, - self._selected_location, + selected_location, ) current_coords = self.location_manager.format_coordinates( self._location.latitude, self._location.longitude, ) new_coords = self.location_manager.format_coordinates( - self._selected_location.latitude, - self._selected_location.longitude, + selected_location.latitude, + selected_location.longitude, ) + self.name_input.SetValue(selected_location.name) self._set_coordinate_comparison( f"Current: {current_coords}. New: {new_coords}. Difference: {distance:.2f} miles." ) @@ -401,6 +426,7 @@ def get_result(self) -> EditLocationResult: """Return the editable values chosen in the dialog.""" selected = self._selected_location return EditLocationResult( + display_name=self.name_input.GetValue().strip(), latitude=selected.latitude if selected else self._location.latitude, longitude=selected.longitude if selected else self._location.longitude, country_code=(selected.country_code if selected else self._location.country_code), @@ -620,10 +646,23 @@ async def _do_current_location_detection(self) -> None: """Run native location detection once.""" result = await self.current_location_service.detect_once() if result.status is LocationDetectionStatus.SUCCESS and result.location is not None: - wx.CallAfter(self._on_current_location_detected, result.location) + location = await self._resolve_detected_location(result.location) + wx.CallAfter(self._on_current_location_detected, location) return wx.CallAfter(self._on_current_location_error, result.message) + async def _resolve_detected_location(self, location: Location) -> Location: + """Prefer a reverse-geocoded label and metadata for detected coordinates.""" + try: + resolved = await self.location_manager.reverse_geocode_coordinates( + location.latitude, + location.longitude, + ) + except Exception as exc: # noqa: BLE001 - fallback label remains editable + logger.debug("Detected-location reverse geocoding failed: %s", exc) + return location + return resolved or location + def _on_current_location_detected(self, location: Location) -> None: """Handle successful current-location detection.""" self._is_detecting_current_location = False diff --git a/src/accessiweather/ui/main_window_locations.py b/src/accessiweather/ui/main_window_locations.py index 75cd425e8..7fab22b77 100644 --- a/src/accessiweather/ui/main_window_locations.py +++ b/src/accessiweather/ui/main_window_locations.py @@ -119,13 +119,21 @@ def on_edit_location(self) -> None: if isinstance(edit_result, bool): self.app.config_manager.update_location_marine_mode(selected, edit_result) else: - self.app.config_manager.update_location_details( + updated = self.app.config_manager.update_location_details( selected, latitude=edit_result.latitude, longitude=edit_result.longitude, country_code=edit_result.country_code, marine_mode=edit_result.marine_mode, + display_name=getattr(edit_result, "display_name", selected), ) + if updated: + self._populate_locations() + new_name = getattr(edit_result, "display_name", selected) + idx = self.location_dropdown.FindString(new_name) + if idx != getattr(wx, "NOT_FOUND", -1): + self.location_dropdown.SetSelection(idx) + self._set_current_location(new_name) self.refresh_weather_async(force_refresh=True) def on_remove_location(self) -> None: diff --git a/tests/test_all_locations_view.py b/tests/test_all_locations_view.py index 64cd6816d..283e192c0 100644 --- a/tests/test_all_locations_view.py +++ b/tests/test_all_locations_view.py @@ -58,6 +58,7 @@ class _Stub: win._fetch_weather_data = MagicMock(return_value=MagicMock()) win._set_current_location = MagicMock() win._update_title_for_location = MagicMock() + win._populate_locations = MagicMock() # Delegate the real implementations we want to test. win.set_status = MainWindow.set_status.__get__(win, MainWindow) @@ -499,6 +500,7 @@ def test_edit_location_applies_coordinate_result_from_dialog(self): longitude=-76.4922, country_code="US", marine_mode=True, + display_name="Annapolis Harbor", ) with patch.object(mw_module, "show_edit_location_dialog", return_value=edit_result): @@ -510,8 +512,11 @@ def test_edit_location_applies_coordinate_result_from_dialog(self): longitude=-76.4922, country_code="US", marine_mode=True, + display_name="Annapolis Harbor", ) win.app.config_manager.update_location_marine_mode.assert_not_called() + win._populate_locations.assert_called_once() + win.location_dropdown.FindString.assert_called_once_with("Annapolis Harbor") def test_edit_location_does_nothing_on_cancel(self): import accessiweather.ui.main_window as mw_module diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index a2a4daffc..9b91362dc 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -238,7 +238,7 @@ def test_update_location_marine_mode_nonexistent_returns_false(self, manager): assert manager.update_location_marine_mode("Nowhere", True) is False def test_update_location_details_changes_coordinates_and_clears_zone_metadata(self, manager): - """Updating saved coordinates keeps the name and clears stale zone fields.""" + """Updating saved coordinates can also update the editable display name.""" manager.add_location("Lumberton", 39.965, -74.805, country_code="US") location = manager.get_all_locations()[0] location.timezone = "America/New_York" @@ -255,11 +255,12 @@ def test_update_location_details_changes_coordinates_and_clears_zone_metadata(se longitude=-74.8069, country_code="US", marine_mode=True, + display_name="Mount Holly, New Jersey", ) assert result is True updated = manager.get_all_locations()[0] - assert updated.name == "Lumberton" + assert updated.name == "Mount Holly, New Jersey" assert updated.latitude == pytest.approx(39.9571) assert updated.longitude == pytest.approx(-74.8069) assert updated.country_code == "US" @@ -272,6 +273,7 @@ def test_update_location_details_changes_coordinates_and_clears_zone_metadata(se assert updated.radar_station is None assert manager.get_current_location() is not None assert manager.get_current_location().latitude == pytest.approx(39.9571) + assert manager.get_current_location().name == "Mount Holly, New Jersey" def test_update_location_details_keeps_zone_metadata_when_coordinates_match(self, manager): """Updating only marine mode/country preserves resolved zone metadata.""" @@ -287,6 +289,7 @@ def test_update_location_details_keeps_zone_metadata_when_coordinates_match(self longitude=-74.805, country_code="US", marine_mode=True, + display_name="Lumberton", ) assert result is True @@ -303,10 +306,28 @@ def test_update_location_details_nonexistent_returns_false(self, manager): longitude=2.0, country_code="US", marine_mode=False, + display_name="Nowhere", ) is False ) + def test_update_location_details_rejects_duplicate_display_name(self, manager): + """Renaming a location cannot collide with another saved location.""" + manager.add_location("Lumberton", 39.965, -74.805, country_code="US") + manager.add_location("Mount Holly", 39.9934, -74.7879, country_code="US") + + result = manager.update_location_details( + "Lumberton", + latitude=39.9571, + longitude=-74.8069, + country_code="US", + marine_mode=False, + display_name="Mount Holly", + ) + + assert result is False + assert sorted(manager.get_location_names()) == ["Lumberton", "Mount Holly"] + def test_update_settings(self, manager): """Test updating settings.""" manager.load_config() diff --git a/tests/test_current_location.py b/tests/test_current_location.py index cfa1b65d3..d12097008 100644 --- a/tests/test_current_location.py +++ b/tests/test_current_location.py @@ -602,3 +602,80 @@ def test_detected_location_prefills_editable_name_and_selection(self): dialog._update_status.assert_called_once_with( "Detected current location. Review the editable name, then save it." ) + + +class TestEditLocationDialogCurrentLocation: + async def test_detection_uses_reverse_geocoded_location_when_available(self, monkeypatch): + from accessiweather.current_location import ( + CurrentCoordinates, + CurrentLocationResult, + LocationDetectionStatus, + location_from_coordinates, + ) + from accessiweather.models import Location + from accessiweather.ui.dialogs.location_dialog import EditLocationDialog + + detected = location_from_coordinates( + CurrentCoordinates(latitude=39.9571, longitude=-74.8069) + ) + resolved = Location( + name="Mount Holly, NJ", + latitude=39.9571, + longitude=-74.8069, + country_code="US", + timezone="America/New_York", + ) + applied: list[Location] = [] + dialog = EditLocationDialog.__new__(EditLocationDialog) + dialog.current_location_service = MagicMock() + dialog.current_location_service.detect_once = AsyncMock( + return_value=CurrentLocationResult( + status=LocationDetectionStatus.SUCCESS, + message="Detected", + coordinates=CurrentCoordinates(latitude=39.9571, longitude=-74.8069), + location=detected, + ) + ) + dialog.location_manager = MagicMock() + dialog.location_manager.reverse_geocode_coordinates = AsyncMock(return_value=resolved) + + monkeypatch.setattr( + "accessiweather.ui.dialogs.location_dialog.wx.CallAfter", + lambda func, *args: func(*args), + ) + dialog._on_current_location_detected = applied.append + + await dialog._do_current_location_detection() + + assert applied == [resolved] + dialog.location_manager.reverse_geocode_coordinates.assert_awaited_once_with( + 39.9571, + -74.8069, + ) + + def test_edit_result_includes_editable_display_name(self): + from accessiweather.models import Location + from accessiweather.ui.dialogs.location_dialog import EditLocationDialog + + original = Location(name="Lumberton", latitude=39.965, longitude=-74.805) + selected = Location( + name="Mount Holly, NJ", + latitude=39.9571, + longitude=-74.8069, + country_code="US", + ) + dialog = EditLocationDialog.__new__(EditLocationDialog) + dialog._location = original + dialog._selected_location = selected + dialog.name_input = MagicMock() + dialog.name_input.GetValue.return_value = "Mount Holly, NJ" + dialog.marine_checkbox = MagicMock() + dialog.marine_checkbox.GetValue.return_value = True + + result = dialog.get_result() + + assert result.display_name == "Mount Holly, NJ" + assert result.latitude == pytest.approx(39.9571) + assert result.longitude == pytest.approx(-74.8069) + assert result.country_code == "US" + assert result.marine_mode is True diff --git a/tests/test_location_manager_address_geocoding.py b/tests/test_location_manager_address_geocoding.py index 3df57dbb4..460e6c5fe 100644 --- a/tests/test_location_manager_address_geocoding.py +++ b/tests/test_location_manager_address_geocoding.py @@ -16,6 +16,13 @@ def _make_response(payload: dict) -> MagicMock: return response +def _make_nws_response(payload: dict, status_code: int = 200) -> MagicMock: + response = MagicMock() + response.status_code = status_code + response.json.return_value = payload + return response + + @pytest.fixture def census_payload() -> dict: return { @@ -121,3 +128,48 @@ async def test_search_locations_falls_back_to_openmeteo_when_address_has_no_matc assert locations[0].latitude == pytest.approx(32.95373) assert locations[0].longitude == pytest.approx(-96.89028) assert mock_client.get.call_count == 2 + + +@pytest.mark.asyncio +async def test_reverse_geocode_coordinates_uses_nws_relative_location() -> None: + manager = LocationManager() + + with patch("accessiweather.location_manager.httpx.AsyncClient") as mock_client_class: + mock_client = AsyncMock() + mock_client_class.return_value.__aenter__.return_value = mock_client + mock_client.get.return_value = _make_nws_response( + { + "properties": { + "relativeLocation": { + "properties": { + "city": "Mount Holly", + "state": "NJ", + } + }, + "timeZone": "America/New_York", + } + } + ) + + location = await manager.reverse_geocode_coordinates(39.9571, -74.8069) + + assert location is not None + assert location.name == "Mount Holly, NJ" + assert location.latitude == pytest.approx(39.9571) + assert location.longitude == pytest.approx(-74.8069) + assert location.country_code == "US" + assert location.timezone == "America/New_York" + + +@pytest.mark.asyncio +async def test_reverse_geocode_coordinates_returns_none_when_nws_cannot_name_point() -> None: + manager = LocationManager() + + with patch("accessiweather.location_manager.httpx.AsyncClient") as mock_client_class: + mock_client = AsyncMock() + mock_client_class.return_value.__aenter__.return_value = mock_client + mock_client.get.return_value = _make_nws_response({"properties": {}}, status_code=404) + + location = await manager.reverse_geocode_coordinates(51.5074, -0.1278) + + assert location is None From 8339cb6a136130c08db9280f4eb6dad7916f071a Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 27 May 2026 23:11:40 -0400 Subject: [PATCH 5/5] fix(location): infer US metadata for detected coordinates --- src/accessiweather/current_location.py | 6 +++++- tests/test_current_location.py | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/accessiweather/current_location.py b/src/accessiweather/current_location.py index bdcb838f1..c457d7fc3 100644 --- a/src/accessiweather/current_location.py +++ b/src/accessiweather/current_location.py @@ -11,6 +11,7 @@ from enum import Enum from typing import Any, Protocol +from .location_classification import is_us_location from .models import Location logger = logging.getLogger(__name__) @@ -70,7 +71,10 @@ def location_from_coordinates( ) -> Location: """Create the same editable Location object used by manual location saves.""" label = name or f"Current Location ({coordinates.latitude:.4f}, {coordinates.longitude:.4f})" - return Location(name=label, latitude=coordinates.latitude, longitude=coordinates.longitude) + location = Location(name=label, latitude=coordinates.latitude, longitude=coordinates.longitude) + if is_us_location(location): + location.country_code = "US" + return location class CurrentLocationService: diff --git a/tests/test_current_location.py b/tests/test_current_location.py index d12097008..159e3550b 100644 --- a/tests/test_current_location.py +++ b/tests/test_current_location.py @@ -245,6 +245,26 @@ async def test_service_converts_coordinates_to_editable_location(self): assert result.location.name == "Current Location (39.9526, -75.1652)" assert result.location.latitude == pytest.approx(39.9526) assert result.location.longitude == pytest.approx(-75.1652) + assert result.location.country_code == "US" + + async def test_service_leaves_unclear_international_coordinates_without_country_code(self): + from accessiweather.current_location import ( + CurrentCoordinates, + CurrentLocationService, + LocationDetectionStatus, + ) + + provider = MagicMock() + provider.detect = AsyncMock( + return_value=CurrentCoordinates(latitude=51.5074, longitude=-0.1278, accuracy_meters=25) + ) + service = CurrentLocationService(provider=provider) + + result = await service.detect_once() + + assert result.status is LocationDetectionStatus.SUCCESS + assert result.location is not None + assert result.location.country_code is None class TestWindowsCurrentLocationProvider: