From 990b30ed25d2ea5abc1187e72be539d008ea5c21 Mon Sep 17 00:00:00 2001 From: Cameron Will Date: Sun, 9 Aug 2026 13:41:50 -0400 Subject: [PATCH] fix: Make the GPS serial port configurable (#82) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GPS was hardcoded to /dev/ttyS0, which is where the AIO V2 board exposes it on CM4-based uConsoles. CM5-based units expose the same UART at /dev/ttyAMA0, so GPS silently never worked there. - Auto-detection now probes /dev/ttyS0 then /dev/ttyAMA0. - The port can be pinned explicitly via Settings > GPS Device or the MESHCORE_GPS_DEVICE env var, which wins over both auto-detection and gpsd — needed when gpsd is running but bound to the wrong device, the case reported in #82. - An explicit device is accepted even if the node is absent at startup (e.g. a USB GPS plugged in later); start() surfaces the open error. - Changing the setting rebuilds the provider in place, so it takes effect without restarting the app. The GPS provider is now built after the settings load rather than before, since it needs the configured path. Based on the patch contributed by @educationalpurposes in #81. Closes #82 Co-Authored-By: Claude Opus 5 --- AGENTS.md | 1 + src/meshcore_console/meshcore/client.py | 16 +++++- src/meshcore_console/meshcore/settings.py | 1 + src/meshcore_console/platform/gps.py | 50 ++++++++++++++----- src/meshcore_console/ui_gtk/views/settings.py | 12 +++++ tests/unit/test_gps.py | 39 +++++++++++++++ 6 files changed, 105 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c44e0ae..72d9d04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -156,6 +156,7 @@ All commands below assume you are inside `nix develop`. | `MESHCORE_GPSD_DISABLE=1` | Skip gpsd detection, use direct serial | | `MESHCORE_GPSD_HOST` | gpsd hostname (default: 127.0.0.1) | | `MESHCORE_GPSD_PORT` | gpsd port (default: 2947) | +| `MESHCORE_GPS_DEVICE` | GPS serial port; overrides auto-detection and gpsd (also in Settings > GPS Device) | ### Initial Setup (macOS) diff --git a/src/meshcore_console/meshcore/client.py b/src/meshcore_console/meshcore/client.py index ccbb3a7..88de88d 100644 --- a/src/meshcore_console/meshcore/client.py +++ b/src/meshcore_console/meshcore/client.py @@ -69,7 +69,6 @@ def __init__( self._message_store = message_store or MessageStore(self._db) self._peer_store = peer_store or PeerStore(self._db) self._channel_store = channel_store or UIChannelStore(self._db) - self._gps_provider = gps_provider or create_gps_provider() self._repeater_password_store = RepeaterPasswordStore(self._db) self._repeater_sessions: dict[str, RepeaterLoginState] = {} # Load persisted state @@ -80,6 +79,11 @@ def __init__( self._settings = self._settings_store.load() if node_name != "uconsole-node": self._settings.node_name = node_name + # Built after the settings load so the configured gps_device takes effect. + self._gps_provider_injected = gps_provider is not None + self._gps_provider = gps_provider or create_gps_provider( + serial_port=self._settings.gps_device + ) self._session = session if session is not None else self._new_session() self._config = runtime_config_from_settings(self._settings) @@ -794,10 +798,20 @@ def get_settings(self) -> MeshcoreSettings: def update_settings(self, settings: MeshcoreSettings) -> None: updated = settings.clone() + gps_device_changed = updated.gps_device != self._settings.gps_device self._settings = updated self._settings_store.save(updated) self._config = runtime_config_from_settings(self._settings) + # Rebuild the GPS provider if the device path changed, so the setting + # takes effect without an app restart. A provider injected by a test or + # mock is left alone. + if gps_device_changed and not self._gps_provider_injected: + self._gps_provider.stop() + self._gps_provider = create_gps_provider(serial_port=updated.gps_device) + if self._connected: + self._gps_provider.start() + # Prepare a fresh session so the next connect() picks up new config, # but do NOT restart the radio here — the user must restart the app # for radio-parameter changes to take effect. diff --git a/src/meshcore_console/meshcore/settings.py b/src/meshcore_console/meshcore/settings.py index 6eddf2a..de547e4 100644 --- a/src/meshcore_console/meshcore/settings.py +++ b/src/meshcore_console/meshcore/settings.py @@ -13,6 +13,7 @@ class MeshcoreSettings: allow_telemetry: bool = True # Allow telemetry requests from other nodes telemetry_favorites_only: bool = False autoconnect: bool = False # Automatically connect to radio on app startup + gps_device: str = "" # GPS serial port path; empty = auto-detect (#82) suppress_service_dialog: bool = False # Don't prompt to stop conflicting services log_level: str = "INFO" # stderr log level (DEBUG, INFO, WARNING, ERROR, CRITICAL) diff --git a/src/meshcore_console/platform/gps.py b/src/meshcore_console/platform/gps.py index ff7c352..914532b 100644 --- a/src/meshcore_console/platform/gps.py +++ b/src/meshcore_console/platform/gps.py @@ -67,15 +67,19 @@ def has_fix(self) -> bool: class UConsoleGps: """GPS provider for uConsole AIO board. - The AIO V2 board provides GPS via the Pi's UART at /dev/ttyS0. + The AIO V2 board provides GPS via the Pi's UART, /dev/ttyS0 on older + CM4-based units and /dev/ttyAMA0 on CM5-based units. The port can be + overridden via the constructor (Settings > GPS Device) or the + MESHCORE_GPS_DEVICE environment variable. GPIO 27 is used to enable/disable the GPS module. """ GPIO_ENABLE_PIN = 27 - SERIAL_PORT = "/dev/ttyS0" + SERIAL_PORT = "/dev/ttyS0" # Default; see GPS_SERIAL_CANDIDATES BAUD_RATE = 9600 - def __init__(self) -> None: + def __init__(self, serial_port: str | None = None) -> None: + self._serial_port = serial_port or self.SERIAL_PORT self._callback: Callable[[float, float], None] | None = None self._error_callback: Callable[[str], None] | None = None self._running = False @@ -114,17 +118,17 @@ def start(self) -> None: import serial # type: ignore[import-not-found] self._serial = serial.Serial( - self.SERIAL_PORT, + self._serial_port, self.BAUD_RATE, timeout=1.0, ) self._running = True - logger.debug("GPS: opened %s at %d baud", self.SERIAL_PORT, self.BAUD_RATE) + logger.debug("GPS: opened %s at %d baud", self._serial_port, self.BAUD_RATE) except ImportError: self._report_error("pyserial not installed - GPS unavailable") except PermissionError: self._report_error( - f"Permission denied on {self.SERIAL_PORT} - add user to dialout group" + f"Permission denied on {self._serial_port} - add user to dialout group" ) except OSError as e: self._report_error(f"Serial port error: {e}") @@ -538,20 +542,38 @@ def has_fix(self) -> bool: return False -def create_gps_provider() -> GpsProvider: +# Serial ports probed when no explicit device is configured. /dev/ttyS0 is +# the GPS UART on CM4-based uConsoles; CM5-based units expose it at +# /dev/ttyAMA0 instead (#82). +GPS_SERIAL_CANDIDATES: tuple[str, ...] = ("/dev/ttyS0", "/dev/ttyAMA0") + + +def create_gps_provider(serial_port: str | None = None) -> GpsProvider: """Create the appropriate GPS provider for the current environment. Priority: 1. MESHCORE_MOCK=1 → MockGps - 2. gpsd reachable (unless MESHCORE_GPSD_DISABLE=1) → GpsdProvider - 3. /dev/ttyS0 exists → UConsoleGps - 4. Fallback → NullGps (returns None; callers use settings fixed position) + 2. Explicit device (``serial_port`` arg, e.g. from Settings, or the + MESHCORE_GPS_DEVICE env var) → UConsoleGps on that port + 3. gpsd reachable (unless MESHCORE_GPSD_DISABLE=1) → GpsdProvider + 4. First existing candidate port (/dev/ttyS0, /dev/ttyAMA0) → UConsoleGps + 5. Fallback → NullGps (returns None; callers use settings fixed position) """ if os.environ.get("MESHCORE_MOCK", "0") == "1": from meshcore_console.mock import MockGps return MockGps() + # An explicitly configured device wins over auto-detection. Use it even + # if it doesn't exist yet (e.g. USB GPS plugged in later); start() will + # surface a visible error if the port can't be opened. + configured = (serial_port or os.environ.get("MESHCORE_GPS_DEVICE", "")).strip() + if configured: + if not Path(configured).exists(): + logger.warning("GPS: configured device %s not present (yet)", configured) + logger.info("GPS: using configured device %s", configured) + return UConsoleGps(serial_port=configured) + # Check for gpsd if os.environ.get("MESHCORE_GPSD_DISABLE", "0") != "1": host = os.environ.get("MESHCORE_GPSD_HOST", "127.0.0.1") @@ -560,9 +582,11 @@ def create_gps_provider() -> GpsProvider: logger.info("GPS: gpsd detected at %s:%d, using GpsdProvider", host, port) return GpsdProvider(host=host, port=port) - # Check if we're on a Pi with GPS hardware - if Path("/dev/ttyS0").exists(): - return UConsoleGps() + # Check if we're on a Pi with GPS hardware (CM4: ttyS0, CM5: ttyAMA0) + for candidate in GPS_SERIAL_CANDIDATES: + if Path(candidate).exists(): + logger.info("GPS: auto-detected serial device %s", candidate) + return UConsoleGps(serial_port=candidate) logger.debug("GPS: no hardware detected, using NullGps") return NullGps() diff --git a/src/meshcore_console/ui_gtk/views/settings.py b/src/meshcore_console/ui_gtk/views/settings.py index bbaafcc..675a0f9 100644 --- a/src/meshcore_console/ui_gtk/views/settings.py +++ b/src/meshcore_console/ui_gtk/views/settings.py @@ -152,6 +152,16 @@ def _sync_fav_sensitivity(*_args: object) -> None: grid.attach(self._grid_label("Autoconnect"), 0, 7, 1, 1) grid.attach(self._grid_switch("autoconnect"), 1, 7, 1, 1) + # Row 8: GPS serial device (empty = auto-detect ttyS0/ttyAMA0) + grid.attach(self._grid_label("GPS Device"), 0, 8, 1, 1) + gps_entry = self._grid_entry("gps_device", 18) + gps_entry.set_placeholder_text("auto (/dev/ttyS0, /dev/ttyAMA0)") + gps_entry.set_tooltip_text( + "Serial port for the GPS module. Leave empty to auto-detect. " + "CM4 units use /dev/ttyS0, CM5 units use /dev/ttyAMA0." + ) + grid.attach(gps_entry, 1, 8, 2, 1) + panel.append(grid) return panel @@ -490,6 +500,7 @@ def _load_from_service(self) -> None: self._set_switch("allow_telemetry", settings.allow_telemetry) self._set_switch("telemetry_favorites_only", settings.telemetry_favorites_only) self._set_switch("autoconnect", settings.autoconnect) + self._set_entry("gps_device", settings.gps_device) # Update public key display public_key = self._service.get_self_public_key() @@ -539,6 +550,7 @@ def _collect_settings(self, allow_partial: bool = False) -> MeshcoreSettings: out.allow_telemetry = self._switches["allow_telemetry"].get_active() out.telemetry_favorites_only = self._switches["telemetry_favorites_only"].get_active() out.autoconnect = self._switches["autoconnect"].get_active() + out.gps_device = self._entries["gps_device"].get_text().strip() # Radio out.radio_preset = self._preset.get_active_id() or "custom" diff --git a/tests/unit/test_gps.py b/tests/unit/test_gps.py index 3680ae0..a2968f7 100644 --- a/tests/unit/test_gps.py +++ b/tests/unit/test_gps.py @@ -148,3 +148,42 @@ def test_create_gps_provider_falls_back_to_serial() -> None: mock_path.return_value.exists.return_value = True provider = create_gps_provider() assert isinstance(provider, UConsoleGps) + + +def test_create_gps_provider_uses_configured_device() -> None: + """Issue #82: an explicitly configured port must be used, even over gpsd.""" + env = {"MESHCORE_MOCK": "0"} + with ( + patch.dict(os.environ, env, clear=False), + patch("meshcore_console.platform.gps._gpsd_available", return_value=True), + ): + provider = create_gps_provider(serial_port="/dev/ttyAMA0") + assert isinstance(provider, UConsoleGps) + assert provider._serial_port == "/dev/ttyAMA0" + + +def test_create_gps_provider_env_var_device() -> None: + env = {"MESHCORE_MOCK": "0", "MESHCORE_GPS_DEVICE": "/dev/ttyUSB0"} + with ( + patch.dict(os.environ, env, clear=False), + patch("meshcore_console.platform.gps._gpsd_available", return_value=False), + ): + provider = create_gps_provider() + assert isinstance(provider, UConsoleGps) + assert provider._serial_port == "/dev/ttyUSB0" + + +def test_create_gps_provider_autodetects_cm5_port() -> None: + """Issue #82: fall back to /dev/ttyAMA0 (CM5) when /dev/ttyS0 is absent.""" + env = {"MESHCORE_MOCK": "0", "MESHCORE_GPS_DEVICE": ""} + with ( + patch.dict(os.environ, env, clear=False), + patch("meshcore_console.platform.gps._gpsd_available", return_value=False), + patch("meshcore_console.platform.gps.Path") as mock_path, + ): + mock_path.side_effect = lambda p: type( + "P", (), {"exists": staticmethod(lambda p=p: p == "/dev/ttyAMA0")} + )() + provider = create_gps_provider() + assert isinstance(provider, UConsoleGps) + assert provider._serial_port == "/dev/ttyAMA0"