Harden GPS mode updates with callback reattach and status polling fallback

routes/gps.py

@@ -65,14 +65,17 @@ def auto_connect_gps():
     If gpsd is not running, attempts to detect GPS devices and start gpsd.
     Returns current status if already connected.
     """
-    # Check if already running
-    reader = get_gps_reader()
-    if reader and reader.is_running:
-        position = reader.position
-        sky = reader.sky
-        return jsonify({
-            'status': 'connected',
-            'source': 'gpsd',
+    # Check if already running
+    reader = get_gps_reader()
+    if reader and reader.is_running:
+        # Ensure stream callbacks are attached for this process.
+        reader.add_callback(_position_callback)
+        reader.add_sky_callback(_sky_callback)
+        position = reader.position
+        sky = reader.sky
+        return jsonify({
+            'status': 'connected',
+            'source': 'gpsd',
             'has_fix': position is not None,
             'position': position.to_dict() if position else None,
             'sky': sky.to_dict() if sky else None,

static/js/modes/gps.js

@@ -9,6 +9,7 @@ const GPS = (function() {
     let lastPosition = null;
     let lastSky = null;
     let skyPollTimer = null;
+    let statusPollTimer = null;
     let themeObserver = null;
     let skyRenderer = null;
     let skyRendererInitAttempted = false;
@@ -413,10 +414,10 @@ const GPS = (function() {
     }
 
     function connect() {
-        updateConnectionUI(false, false, 'connecting');
-        fetch('/gps/auto-connect', { method: 'POST' })
-            .then(r => r.json())
-            .then(data => {
+        updateConnectionUI(false, false, 'connecting');
+        fetch('/gps/auto-connect', { method: 'POST' })
+            .then(r => r.json())
+            .then(data => {
                 if (data.status === 'connected') {
                     connected = true;
                     updateConnectionUI(true, data.has_fix);
@@ -427,16 +428,18 @@ const GPS = (function() {
                     if (data.sky) {
                         lastSky = data.sky;
                         updateSkyUI(data.sky);
-                    }
-                    subscribeToStream();
-                    startSkyPolling();
-                    // Ensure the global GPS stream is running
-                    if (typeof startGpsStream === 'function' && !gpsEventSource) {
-                        startGpsStream();
-                    }
-                } else {
-                    connected = false;
-                    updateConnectionUI(false, false, 'error', data.message || 'gpsd not available');
+                    }
+                    subscribeToStream();
+                    startSkyPolling();
+                    startStatusPolling();
+                    // Ensure the global GPS stream is running
+                    const hasGlobalGpsStream = typeof gpsEventSource !== 'undefined' && !!gpsEventSource;
+                    if (typeof startGpsStream === 'function' && !hasGlobalGpsStream) {
+                        startGpsStream();
+                    }
+                } else {
+                    connected = false;
+                    updateConnectionUI(false, false, 'error', data.message || 'gpsd not available');
                 }
             })
             .catch(() => {
@@ -445,14 +448,15 @@ const GPS = (function() {
             });
     }
 
-    function disconnect() {
-        unsubscribeFromStream();
-        stopSkyPolling();
-        fetch('/gps/stop', { method: 'POST' })
-            .then(() => {
-                connected = false;
-                updateConnectionUI(false);
-            });
+    function disconnect() {
+        unsubscribeFromStream();
+        stopSkyPolling();
+        stopStatusPolling();
+        fetch('/gps/stop', { method: 'POST' })
+            .then(() => {
+                connected = false;
+                updateConnectionUI(false);
+            });
     }
 
     function onGpsStreamData(data) {
@@ -470,14 +474,14 @@ const GPS = (function() {
         }
     }
 
-    function startSkyPolling() {
-        stopSkyPolling();
-        // Poll satellite data every 5 seconds as a reliable fallback
-        // SSE stream may miss sky updates due to queue contention with position messages
-        pollSatellites();
-        skyPollTimer = setInterval(pollSatellites, 5000);
-    }
-
+    function startSkyPolling() {
+        stopSkyPolling();
+        // Poll satellite data every 5 seconds as a reliable fallback
+        // SSE stream may miss sky updates due to queue contention with position messages
+        pollSatellites();
+        skyPollTimer = setInterval(pollSatellites, 5000);
+    }
+
     function stopSkyPolling() {
         if (skyPollTimer) {
             clearInterval(skyPollTimer);
@@ -485,18 +489,55 @@ const GPS = (function() {
         }
     }
 
-    function pollSatellites() {
-        if (!connected) return;
-        fetch('/gps/satellites')
-            .then(r => r.json())
-            .then(data => {
+    function pollSatellites() {
+        if (!connected) return;
+        fetch('/gps/satellites')
+            .then(r => r.json())
+            .then(data => {
                 if (data.status === 'ok' && data.sky) {
                     lastSky = data.sky;
                     updateSkyUI(data.sky);
                 }
-            })
-            .catch(() => {});
-    }
+            })
+            .catch(() => {});
+    }
+
+    function startStatusPolling() {
+        stopStatusPolling();
+        // Poll full status as a fallback when SSE is unavailable or blocked.
+        pollStatus();
+        statusPollTimer = setInterval(pollStatus, 2000);
+    }
+
+    function stopStatusPolling() {
+        if (statusPollTimer) {
+            clearInterval(statusPollTimer);
+            statusPollTimer = null;
+        }
+    }
+
+    function pollStatus() {
+        if (!connected) return;
+        fetch('/gps/status')
+            .then(r => r.json())
+            .then(data => {
+                if (!connected || !data || data.running !== true) return;
+
+                if (data.position) {
+                    lastPosition = data.position;
+                    updatePositionUI(data.position);
+                    updateConnectionUI(true, true);
+                } else {
+                    updateConnectionUI(true, false);
+                }
+
+                if (data.sky) {
+                    lastSky = data.sky;
+                    updateSkyUI(data.sky);
+                }
+            })
+            .catch(() => {});
+    }
 
     function subscribeToStream() {
         // Subscribe to the global GPS stream instead of opening a separate SSE connection
@@ -1443,6 +1484,7 @@ const GPS = (function() {
     function destroy() {
         unsubscribeFromStream();
         stopSkyPolling();
+        stopStatusPolling();
         if (themeObserver) {
             themeObserver.disconnect();
             themeObserver = null;

tests/test_gps_routes.py

@@ -0,0 +1,63 @@
+"""Tests for GPS route behavior and gps client callback management."""
+
+from routes import gps as gps_routes
+from utils.gps import GPSDClient
+
+
+def test_gpsd_client_add_callback_deduplicates():
+    """Adding the same position callback twice should only register once."""
+    client = GPSDClient()
+
+    def callback(_position):
+        return None
+
+    client.add_callback(callback)
+    client.add_callback(callback)
+
+    assert client._callbacks.count(callback) == 1
+
+
+def test_gpsd_client_add_sky_callback_deduplicates():
+    """Adding the same sky callback twice should only register once."""
+    client = GPSDClient()
+
+    def callback(_sky):
+        return None
+
+    client.add_sky_callback(callback)
+    client.add_sky_callback(callback)
+
+    assert client._sky_callbacks.count(callback) == 1
+
+
+def test_auto_connect_attaches_callbacks_when_reader_already_running(client, monkeypatch):
+    """Auto-connect should re-attach stream callbacks for an already-running reader."""
+
+    class FakeReader:
+        is_running = True
+        position = None
+        sky = None
+
+        def __init__(self):
+            self.position_callbacks = []
+            self.sky_callbacks = []
+
+        def add_callback(self, callback):
+            self.position_callbacks.append(callback)
+
+        def add_sky_callback(self, callback):
+            self.sky_callbacks.append(callback)
+
+    reader = FakeReader()
+    monkeypatch.setattr(gps_routes, 'get_gps_reader', lambda: reader)
+
+    with client.session_transaction() as sess:
+        sess['logged_in'] = True
+
+    response = client.post('/gps/auto-connect')
+    payload = response.get_json()
+
+    assert response.status_code == 200
+    assert payload['status'] == 'connected'
+    assert reader.position_callbacks == [gps_routes._position_callback]
+    assert reader.sky_callbacks == [gps_routes._sky_callback]

utils/gps.py

@@ -194,18 +194,20 @@ class GPSDClient:
         """Return gpsd connection info."""
         return f"gpsd://{self.host}:{self.port}"
 
-    def add_callback(self, callback: Callable[[GPSPosition], None]) -> None:
-        """Add a callback to be called on position updates."""
-        self._callbacks.append(callback)
+    def add_callback(self, callback: Callable[[GPSPosition], None]) -> None:
+        """Add a callback to be called on position updates."""
+        if callback not in self._callbacks:
+            self._callbacks.append(callback)
 
     def remove_callback(self, callback: Callable[[GPSPosition], None]) -> None:
         """Remove a position update callback."""
         if callback in self._callbacks:
             self._callbacks.remove(callback)
 
-    def add_sky_callback(self, callback: Callable[[GPSSkyData], None]) -> None:
-        """Add a callback to be called on sky data updates."""
-        self._sky_callbacks.append(callback)
+    def add_sky_callback(self, callback: Callable[[GPSSkyData], None]) -> None:
+        """Add a callback to be called on sky data updates."""
+        if callback not in self._sky_callbacks:
+            self._sky_callbacks.append(callback)
 
     def remove_sky_callback(self, callback: Callable[[GPSSkyData], None]) -> None:
         """Remove a sky data update callback."""