Distrust a >120s-off ECU RTC and skip Redis on car-mode boot

The car's battery-backed ECU DS3231 RTC can be set wrong — most often an
exactly-3600s EST/EDT DST error — and the car LAN has no NTP to ever
correct it. _handle_ecu_timestamp used to trust the ECU unconditionally,
so every frame would be stamped an hour off. Trust the local system clock
instead when it disagrees with the ECU by more than
_MAX_ECU_SYSTEM_DISAGREEMENT_S (120s), and log a warning naming the likely
DS3231 cause so future regressions are visible.

Separately, the car mode used to call redis_utils.get_sync_client() during
TelemetryNode.__init__, which blocks ~25s on connection-refused retries to
127.0.0.1:6379 (the car LAN has no Redis), preventing the WebSocket on
:9080 from ever binding. main.py:start_car_services now passes
redis_client=None and __init__ honors that on role=car without attempting
a connection; base role still auto-connects on None as before.

Adds TestCarModeClockAndRedis (5 cases) covering both the disagreement
guard and the car-skip-Redis path. Full telemetry-ci test set: 81/81 pass.

Author: haoruizhou

universal-telemetry-software/main.py

@@ -87,7 +87,10 @@ async def _run():
         # sets a default loop; asyncio.gather() then returns a Future instead of a
         # coroutine, causing asyncio.run() to raise ValueError.
         direct_queue: asyncio.Queue = asyncio.Queue(maxsize=2000)
-        node = TelemetryNode(can_event=can_event, telemetry_event=telemetry_event)
+        # Pass redis_client=None so the car never blocks trying to connect to a
+        # Redis that isn't there — Redis only runs on the base. All Redis paths
+        # in TelemetryNode (safe_publish, _redis_ok, health reads) handle None.
+        node = TelemetryNode(can_event=can_event, telemetry_event=telemetry_event, redis_client=None)
         node.direct_queue = direct_queue
         await asyncio.gather(
             node.start(),

universal-telemetry-software/src/data.py

@@ -52,21 +52,37 @@ def unpack(cls, binary_data):
 # NOTE: 2026-03-22 is today's date at time of writing.
 _SYSTEM_CLOCK_TRUST_EPOCH = 1742601600.0  # 2026-03-22 00:00:00 UTC
 
+# When the system clock is itself trustworthy (past _SYSTEM_CLOCK_TRUST_EPOCH and kept
+# correct by the base station's /set-time injection or NTP), distrust an ECU RTC that
+# disagrees with it by more than this many seconds. The ECU's battery-backed DS3231 can
+# be set wrong — most often an exactly-3600s DST (EST vs EDT) error — and the car LAN has
+# no NTP to ever correct it, so without this guard every frame would be stamped an hour
+# off. 120s is far above normal RTC-vs-clock jitter but well below the 1h failure mode.
+_MAX_ECU_SYSTEM_DISAGREEMENT_S = 120.0
+
 class TelemetryNode:
-    def __init__(self, can_event=None, telemetry_event=None):
+    def __init__(self, can_event=None, telemetry_event=None, redis_client=None):
+        # redis_client: pass an existing connected sync client (base), or None
+        # to skip Redis entirely (car — no Redis on the car LAN, and blocking
+        # here on a missing 127.0.0.1:6379 would prevent the WebSocket from ever
+        # binding). When None, all publish/health reads degrade gracefully.
         self.buffer = deque()
         self.seq_num = 0
         self.received_messages = {} # seq_num -> message_batch
         self.role = os.getenv("ROLE", "auto")
         self.has_can = False
         self.can_event = can_event  # multiprocessing.Event signalled on each CAN RX
         self.telemetry_event = telemetry_event  # heartbeat for LED status
-        self.redis_client = redis_utils.get_sync_client(REDIS_URL)
-        self.direct_queue: asyncio.Queue | None = None  # set by main.py for car mode (no Redis)
+        # On the car (no Redis available), pass redis_client=None explicitly to skip
+        # the connect attempt — see main.py:start_car_services. On the base, the
+        # explicit None is treated as "auto-connect" (the normal path).
+        if redis_client is None and self.role != "car":
+            redis_client = redis_utils.get_sync_client(REDIS_URL)
+        self.redis_client = redis_client  # None on the car, possibly None on a Redis-less base
         # ECU clock sync — offset between ECU RTC and local monotonic clock
         self._clock_offset: float | None = None   # epoch_sec - monotonic at last sync
         self._last_ecu_sync: float = 0.0          # monotonic time of last valid ECU 1999
-        self._sync_source: str = "none"           # "ecu_rtc" | "system_clock" | "override"
+        self._sync_source: str = "none"           # "ecu_rtc" | "system_clock" | "system_clock_override"
         self._base_to_car_offset: float | None = None # Offset between base station time and car's 1970 time
         self._last_raw_car_time: float | None = None  # Raw timestamp of last message
         self._car_internal_jump: float | None = None  # Cumulative jump from 1970 to 2026+ internal car time
@@ -90,23 +106,50 @@ def publish(self, channel, data):
                 pass  # drop under backpressure rather than block the CAN reader
 
     def _handle_ecu_timestamp(self, data: bytes) -> None:
-        """Extract epoch_ms from ECU VCU_Timestamp message and update clock offset."""
+        """Extract epoch_ms from the ECU VCU_Timestamp message and update the clock offset.
+
+        The ECU's battery-backed DS3231 RTC is normally authoritative. But it can be set
+        wrong (most often an exactly-1h DST / EST-vs-EDT error) and the car LAN has no NTP
+        to ever correct it, so a bad ECU clock would silently stamp every frame an hour off.
+        Guard against that: when the local system clock is itself trustworthy (kept correct
+        by the base station's /set-time injection or NTP) and the ECU disagrees with it by
+        more than _MAX_ECU_SYSTEM_DISAGREEMENT_S, distrust the ECU and use the system clock.
+        """
         if len(data) < 8:
             return
         epoch_ms = struct.unpack_from("<q", data)[0]  # int64 little-endian
         if epoch_ms <= 0:
             return
+        epoch_s = epoch_ms / 1000.0
         mono = time.monotonic()
-        self._clock_offset = epoch_ms / 1000.0 - mono
+        sys_time = time.time()
+
+        # Distrust an ECU clock that is far from a trustworthy system clock.
+        if (sys_time > _SYSTEM_CLOCK_TRUST_EPOCH
+                and abs(epoch_s - sys_time) > _MAX_ECU_SYSTEM_DISAGREEMENT_S):
+            if self._sync_source != "system_clock_override":
+                logger.warning(
+                    f"ECU time ({epoch_s:.0f}) disagrees with system clock ({sys_time:.0f}) "
+                    f"by {epoch_s - sys_time:+.0f}s — distrusting ECU, using system clock. "
+                    "Fix the ECU DS3231 RTC (likely a 1h DST/EST-vs-EDT error)."
+                )
+            self._clock_offset = sys_time - mono
+            self._last_ecu_sync = mono
+            self._sync_source = "system_clock_override"
+            return
+
+        self._clock_offset = epoch_s - mono
         self._last_ecu_sync = mono
         self._sync_source = "ecu_rtc"
-        logger.info(f"ECU time sync: epoch={epoch_ms/1000:.3f}  offset={self._clock_offset:+.3f}s")
+        logger.info(f"ECU time sync: epoch={epoch_s:.3f}  offset={self._clock_offset:+.3f}s")
 
     def _try_auto_sync(self) -> None:
         """Check fallback time sources when no ECU sync has arrived yet.
 
         Priority:
-          1. ECU RTC (handled by _handle_ecu_timestamp, always wins)
+          1. ECU RTC (handled by _handle_ecu_timestamp) — wins unless it disagrees with a
+             trustworthy system clock by more than _MAX_ECU_SYSTEM_DISAGREEMENT_S, in which
+             case _handle_ecu_timestamp falls back to the system clock.
           2. RPi system clock past _SYSTEM_CLOCK_TRUST_EPOCH — covers NTP-synced RPis
              and RPis whose clock was manually set via POST /set-time on the status page.
         """

universal-telemetry-software/tests/test_car_mode.py

@@ -29,6 +29,9 @@
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
 import src.websocket_bridge as wb
+import struct
+import time as _time
+from src import data as _data_mod
 from .test_helpers import WebSocketHelper, RedisHelper
 
 logging.basicConfig(level=logging.INFO)
@@ -497,3 +500,85 @@ async def test_hex_string_data_rejected(self, car_ws_server):
 
 if __name__ == "__main__":
     pytest.main([__file__, "-v", "-s"])
+
+
+# ---------------------------------------------------------------------------
+# Car-mode boot path + ECU clock guard
+#
+# Regression coverage for two car-specific failure modes that previously
+# could only be caught on the real car:
+#   1. TelemetryNode.__init__ must accept redis_client=None and not attempt
+#      to connect to 127.0.0.1:6379 — the car LAN has no Redis and blocking
+#      on retries prevents the WebSocket on :9080 from ever binding.
+#   2. _handle_ecu_timestamp must distrust an ECU RTC that disagrees with a
+#      trustworthy system clock by more than _MAX_ECU_SYSTEM_DISAGREEMENT_S,
+#      to catch a stuck/wrong ECU DS3231 (the classic exactly-1h DST error).
+# ---------------------------------------------------------------------------
+
+class TestCarModeClockAndRedis:
+    THRESHOLD = _data_mod._MAX_ECU_SYSTEM_DISAGREEMENT_S
+
+    def _make_node(self):
+        """Build a TelemetryNode without going through __init__ (skips Redis connect)."""
+        n = object.__new__(_data_mod.TelemetryNode)
+        n._clock_offset = None
+        n._last_ecu_sync = 0.0
+        n._sync_source = "none"
+        return n
+
+    def _ecu_bytes(self, epoch_s: float) -> bytes:
+        return struct.pack("<q", int(epoch_s * 1000))
+
+    def test_init_does_not_connect_to_redis_when_redis_client_none(self, monkeypatch):
+        """TelemetryNode(redis_client=None) must not call get_sync_client.
+
+        On the car, Redis is unreachable; if __init__ tries to connect, it blocks
+        ~25s on retries and the WebSocket never binds on :9080.
+        """
+        called = {"n": 0}
+        def _spy(url):
+            called["n"] += 1
+            return None
+        monkeypatch.setattr(_data_mod.redis_utils, "get_sync_client", _spy)
+
+        # role=car should not even attempt a Redis connection
+        with monkeypatch.context() as m:
+            m.setenv("ROLE", "car")
+            _data_mod.TelemetryNode(redis_client=None)
+        assert called["n"] == 0, "TelemetryNode(car) called redis get_sync_client — car will hang on boot"
+
+        # role=base WITHOUT an explicit override still attempts a connection
+        # (this is the normal base path; we just confirm the car differs from base).
+        with monkeypatch.context() as m:
+            m.setenv("ROLE", "base")
+            _data_mod.TelemetryNode(redis_client=None)
+        assert called["n"] == 1, "TelemetryNode(base) should still try Redis"
+
+    def test_ecu_within_tolerance_is_trusted(self):
+        n = self._make_node()
+        now = _time.time()
+        n._handle_ecu_timestamp(self._ecu_bytes(now - 60))   # 60s slow, within 120s tol
+        assert n._sync_source == "ecu_rtc"
+        # Corrected time tracks the actual ECU time, not the system clock
+        assert abs(n._corrected_time() - (now - 60)) < 5
+
+    def test_ecu_off_by_one_hour_is_overridden(self):
+        n = self._make_node()
+        now = _time.time()
+        n._handle_ecu_timestamp(self._ecu_bytes(now - 3600))  # the classic DST/EST error
+        assert n._sync_source == "system_clock_override"
+        # Corrected time snaps to real system time
+        assert abs(n._corrected_time() - now) < 5
+
+    def test_ecu_just_outside_tolerance_is_overridden(self):
+        n = self._make_node()
+        now = _time.time()
+        n._handle_ecu_timestamp(self._ecu_bytes(now - (self.THRESHOLD + 10)))
+        assert n._sync_source == "system_clock_override"
+        assert abs(n._corrected_time() - now) < 5
+
+    def test_ecu_agreeing_with_system_clock_is_trusted(self):
+        n = self._make_node()
+        now = _time.time()
+        n._handle_ecu_timestamp(self._ecu_bytes(now))
+        assert n._sync_source == "ecu_rtc"