Sentry: Tidy battery warning and status checks

openpilot/starpilot/galaxy/sentry_events.py

@@ -5,7 +5,7 @@ import re
 from collections.abc import Callable
 from typing import Protocol, cast
 
-from openpilot.starpilot.sentry_mode.storage import EventStore, StorageUnavailable
+from openpilot.starpilot.sentry_mode.storage import BATTERY_FIELDS, EventStore, StorageUnavailable, valid_tenths
 
 
 MAX_RESPONSE_BYTES = 128 * 1024
@@ -40,13 +40,13 @@ class SentryEvents:
         if (type(value) is not dict or type(value.get("eventId")) is not str or
             ID.fullmatch(value["eventId"]) is None or type(value.get("kind")) is not str or
             value["kind"] not in ("warning", "alarm", "selfie", "battery") or
-            (value["kind"] == "battery" and not all(type(value.get(k)) is int for k in ("voltage", "cutoff"))) or
+            (value["kind"] == "battery" and not all(valid_tenths(value.get(k)) for k in BATTERY_FIELDS)) or
             type(value.get("wallTimeNs")) is not int or not 0 < value["wallTimeNs"] < 2**63):
           raise ValueError("Invalid motion event")
         events.append({"eventId": value["eventId"], "kind": value["kind"],
                        "systemTimeMs": value["wallTimeNs"] // 1_000_000,
                        **({"simulated": True} if value.get("simulated") else {}),
-                       **({k: value[k] for k in ("voltage", "cutoff")} if value["kind"] == "battery" else {}),
+                       **({k: value[k] for k in BATTERY_FIELDS} if value["kind"] == "battery" else {}),
                        "images": cast(Callable[[str], list], self.store.images)(value["eventId"]) if hasattr(self.store, "images") else []})
       result = {"schemaVersion": 1, "source": "local", "scanIncomplete": source["incomplete"],
                 "capacity": source["capacity"], "events": events,

openpilot/starpilot/galaxy/tests/test_settings.mjs

@@ -424,11 +424,13 @@ for (const subtitle of ["Off: Motion monitoring is disabled.", "Not armed: Batte
   assert.equal(sentryMonitorStatus({ subtitle }).label, "Disarmed")
   assert.equal(sentryMonitorStatus({ subtitle }).detail, subtitle)
 }
-assert.equal(sentryMonitorStatus({ subtitle: "Arming · 82s: Monitoring begins after 90 seconds parked." }).tone, "warning")
-assert.equal(sentryMonitorStatus({ subtitle: "Arming · 82s: Monitoring begins after 90 seconds parked." }).label, "Arming")
+const armingStatus = sentryMonitorStatus({ subtitle: "Arming · 82s: Monitoring begins after 90 seconds parked." })
+assert.equal(armingStatus.tone, "warning")
+assert.equal(armingStatus.label, "Arming")
 const armedStatus = { subtitle: "Monitoring motion: Watching for movement while parked." }
 assert.equal(sentryMonitorStatus(armedStatus).label, "Armed")
 assert.equal(sentryMonitorStatus(armedStatus, "Connection lost").label, "Status unavailable")
 assert.equal(sentryMonitorStatus({ subtitle: "Waiting for monitor: The monitor has not responded recently." }).label, "Status unavailable")
 assert.equal(sentryMonitorStatus({ subtitle: "Motion monitor status is unavailable. Reconnecting automatically…" }).label, "Status unavailable")
 assert.equal(sentryMonitorStatus(null).label, "Status unavailable")
+assert.equal(sentryMonitorStatus({ subtitle: 42 }).label, "Status unavailable")

openpilot/starpilot/galaxy/web/css/settings.css

@@ -742,8 +742,7 @@ html.gx-maker-open, html.gx-maker-open body { touch-action: manipulation; }
 }
 
 .gx-sentry-status { flex: 1 1 100%; order: 1; display: grid; gap: var(--sp-1); color: var(--on-surface-variant); font-size: var(--fs-sm); }
-.gx-sentry-status__label { display: inline-flex; align-items: center; gap: var(--sp-2); width: fit-content; font-weight: 500; }
-.gx-sentry-status__label::before { content: ""; width: 6px; height: 6px; flex: 0 0 6px; border-radius: 50%; background: var(--sentry-status-dot, currentColor); }
-.gx-sentry-status--armed { --sentry-status-dot: var(--success); }
-.gx-sentry-status--warning { --sentry-status-dot: var(--warning); }
-.gx-sentry-status__detail { color: var(--on-surface-variant); font-size: var(--fs-sm); }
+.gx-sentry-status__label { display: inline-flex; align-items: center; gap: var(--sp-2); width: fit-content; font-weight: var(--fw-medium); }
+.gx-sentry-status .gx-status-dot { flex: none; background: currentColor; }
+.gx-sentry-status--armed .gx-status-dot { background: var(--success); }
+.gx-sentry-status--warning .gx-status-dot { background: var(--warning); }

openpilot/starpilot/galaxy/web/js/settings.js

@@ -44,7 +44,7 @@ const activeSettings = new Set()
 export const settingsBusy = state => ["updating", "saving"].includes(state.status) || !!state.pending || !!state.reviewing
 
 export function sentryMonitorStatus(data, error = "") {
-  const subtitle = data?.subtitle || ""
+  const subtitle = typeof data?.subtitle === "string" ? data.subtitle : ""
   const label = subtitle.split(":", 1)[0]
   if (error || !subtitle || label.startsWith("Waiting for monitor") || label === "Unavailable" || subtitle.includes("status is unavailable"))
     return { label: "Status unavailable", tone: "warning", detail: error ? "Reconnecting to the motion monitor…" : subtitle || "Checking motion monitor…" }
@@ -465,7 +465,6 @@ export const SettingsPage = {
       <p class="gx-page-description" v-else-if="state.data?.page === 'wheel'">Assign what the steering-wheel controls do.</p>
       <p class="gx-page-description" v-else-if="state.data?.page === 'data'">Choose what uploads when the vehicle uses mobile data.</p>
       <p class="gx-page-description" v-else-if="['aggressive','standard','relaxed'].includes(state.data?.page)">Adjust acceleration, braking and following for this driving style.</p>
-
       <GxState v-if="mode !== 'local'">Local settings are unavailable in preview.</GxState>
       <template v-else>
         <GxNotice v-if="state.data && state.data.vehicleIdentified === false" tone="info">Vehicle not identified yet. Turn on the car and wait for fingerprinting; vehicle-specific settings will then appear here.</GxNotice>
@@ -485,7 +484,7 @@ export const SettingsPage = {
               <div v-if="cardIndex" class="gx-section__header"><i class="bi bi-car-front" aria-hidden="true"></i><span class="gx-section__title">{{ card.title }}</span></div>
               <div v-else class="gx-section__header"><i class="bi" :class="activeSection.icon" aria-hidden="true"></i><span class="gx-section__title">{{ state.data.page === 'hub' ? activeSection.label : state.data.title }}</span>
                 <div v-if="state.data.page === 'sentry'" class="gx-sentry-status" :class="'gx-sentry-status--' + sentryStatus.tone" role="status">
-                  <strong class="gx-sentry-status__label">{{ sentryStatus.label }}</strong>
+                  <strong class="gx-sentry-status__label"><span class="gx-status-dot" aria-hidden="true"></span>{{ sentryStatus.label }}</strong>
                   <span class="gx-sentry-status__detail">{{ sentryStatus.detail }}</span>
                 </div>
                 <button v-if="state.data.page !== 'hub' && state.data.rows.length" type="button" class="gx-icon-btn" :aria-label="state.searchOpen ? 'Close page search' : 'Search within this page'" :aria-pressed="state.searchOpen" @click="togglePageSearch"><i class="bi" :class="state.searchOpen ? 'bi-x-lg' : 'bi-search'" aria-hidden="true"></i></button>

openpilot/starpilot/sentry_mode/README.md

@@ -31,8 +31,7 @@ threshold and warning-count behavior. An alarm requires more than 25 qualifying
 deltas and at least 30 seconds within a 60-second motion window. A stale or
 repeated sample, sensor loss, owner-loop gap, system resume, settings change,
 ignition or shutdown request disarms the policy. Recovery requires rearming.
-Polling without a new sensor message preserves arming while the last accepted
-sample remains fresh (at most 0.5 seconds); it never counts as another motion sample.
+A poll with no new sensor message keeps arming while the last sample is fresh.
 Hardwared retains battery accounting and shutdown authority; Sentry neither
 extends parked runtime nor clears shutdown requests.
 

openpilot/starpilot/sentry_mode/notifications.py

@@ -21,7 +21,7 @@ import uuid
 import requests
 
 from openpilot.starpilot.storage import starpilot_storage_root
-from openpilot.starpilot.sentry_mode.storage import BATTERY_FIELDS, EventStore
+from openpilot.starpilot.sentry_mode.storage import BATTERY_FIELDS, EventStore, valid_tenths, volts
 from openpilot.starpilot.sentry_mode import web_push, relay_enrollment
 from openpilot.starpilot.sentry_mode.relay_config import DEFAULT_RELAY_URL
 
@@ -41,7 +41,7 @@ def message_text(job: dict) -> str:
   if job['kind'] == 'test':
     return 'This is a test StarPilot Sentry notification.'
   if job['kind'] == 'battery':
-    return f"Car battery is low ({job['voltage'] / 10:.1f} V). The device and Sentry turn off at {job['cutoff'] / 10:.1f} V."
+    return f"Car battery is low ({volts(job['voltage'])}). The device and Sentry turn off at {volts(job['cutoff'])}."
   return f"Parked motion detected ({job['kind']})."
 
 
@@ -204,7 +204,7 @@ class NotificationOwner:
         raise ValueError('Invalid subscription time')
     for key, job in state['jobs'].items():
       if (type(job) is not dict or set(job) != JOB_FIELDS | (BATTERY_FIELDS if job.get('kind') == 'battery' else set()) or
-          (job['kind'] == 'battery' and not all(type(job[k]) is int and 0 < job[k] < 1000 for k in BATTERY_FIELDS)) or
+          (job['kind'] == 'battery' and not all(valid_tenths(job[k]) for k in BATTERY_FIELDS)) or
           type(key) is not str or len(key) != 64 or type(job['eventId']) is not str or ID.fullmatch(job['eventId']) is None or
           job['kind'] not in ('warning', 'alarm', 'battery', 'test') or job['channel'] not in CHANNELS or
           type(job['generation']) is not str or ID.fullmatch(job['generation']) is None or

openpilot/starpilot/sentry_mode/policy.py

@@ -124,8 +124,7 @@ class SentryPolicy:
       return Decision("unavailable")
 
     sample = inputs.sample
-    # Polling without a new message is not sensor loss. Preserve continuity only
-    # while the last accepted sample is fresh, without counting it as motion.
+    # A poll with no new message keeps arming while the last sample is fresh.
     if sample is None and inputs.sample_pending and self.arm_started is not None and \
        self.last_sample_time is not None and 0 <= now - self.last_sample_time <= MAX_SAMPLE_AGE_SECONDS:
       remaining = ARM_DELAY_SECONDS - (now - self.arm_started)

openpilot/starpilot/sentry_mode/runtime.py

@@ -15,6 +15,7 @@ from openpilot.starpilot.saved_source import read_saved
 from openpilot.starpilot.sentry_mode.policy import Inputs, MotionSample, SentryPolicy
 from openpilot.starpilot.sentry_mode.preferences import read_preferences
 from openpilot.starpilot.sentry_mode.status import RuntimeStatus
+from openpilot.starpilot.sentry_mode.storage import StorageUnavailable
 
 
 PERIPHERAL_TTL_NS = 1_500_000_000  # C++ 2 Hz publisher; one missed frame.
@@ -27,6 +28,10 @@ BATTERY_CRITICAL_HOLD_NS = 10_000_000_000
 BATTERY_REPEAT_NS = 3600 * 1_000_000_000
 
 
+def recent_battery_warning(events: list[dict], now_ns: int) -> bool:
+  return any(event["kind"] == "battery" and 0 <= now_ns - event["wallTimeNs"] <= BATTERY_REPEAT_NS for event in events)
+
+
 def boot_time_ns() -> int:
   return time.clock_gettime_ns(getattr(time, "CLOCK_BOOTTIME", time.CLOCK_MONOTONIC))
 
@@ -193,7 +198,7 @@ class SentryRuntime:
       self.battery_low_since = None
       return
     if voltage >= cutoff + BATTERY_RECOVERED_TENTHS:
-      self.battery_low_since, self.battery_warned = None, False
+      self.battery_warned = False
     if voltage > cutoff + BATTERY_WARNING_TENTHS or self.battery_warned:
       self.battery_low_since = None
       return
@@ -205,7 +210,7 @@ class SentryRuntime:
     if self.battery is not None:
       try:
         self.battery(voltage, cutoff)
-      except Exception:
+      except StorageUnavailable:
         cloudlog.exception("sentry motion battery warning failed")
 
   def close(self) -> None:
@@ -237,8 +242,7 @@ def main() -> None:
           raise
     return store.record(kind, stamp, permitted=permitted, images=images)
   def battery(voltage, cutoff):
-    recent = time.time_ns() - BATTERY_REPEAT_NS
-    if any(event["kind"] == "battery" and event["wallTimeNs"] > recent for event in store.snapshot()["events"]):
+    if recent_battery_warning(store.snapshot()["events"], time.time_ns()):
       return
     store.record("battery", time.monotonic_ns(), permitted=lambda: True, details={"voltage": voltage, "cutoff": cutoff})
   runtime = SentryRuntime(Params(), messages, record=record, status=RuntimeStatus(), battery=battery)

openpilot/starpilot/sentry_mode/status.py

@@ -10,7 +10,7 @@ import uuid
 from openpilot.common.hardware.hw import Paths
 from openpilot.starpilot.parked_evidence import RESUME_SKEW_NS
 from openpilot.starpilot.sentry_mode.preferences import SavedPreferences
-from openpilot.starpilot.sentry_mode.storage import EventStore, FILE_FLAGS, StorageUnavailable, _private
+from openpilot.starpilot.sentry_mode.storage import EventStore, FILE_FLAGS, StorageUnavailable, _private, valid_tenths, volts
 from openpilot.starpilot.storage import starpilot_storage_root
 
 
@@ -39,15 +39,13 @@ def boot_id() -> str:
 
 
 def low_voltage_reason(voltage, cutoff, fallback: str) -> str:
-  def valid(tenths):
-    return type(tenths) is int and 0 < tenths < 1000
-  if not valid(voltage):
+  if not valid_tenths(voltage):
     return "Battery voltage cannot be read"
-  if not valid(cutoff):
-    return f"Battery is {voltage / 10:.1f} V; the low-battery cutoff cannot be read"
+  if not valid_tenths(cutoff):
+    return f"Battery is {volts(voltage)}; the low-battery cutoff cannot be read"
   if voltage <= cutoff:
-    return f"Battery is {voltage / 10:.1f} V; needs above {cutoff / 10:.1f} V"
-  return f"{fallback} (battery {voltage / 10:.1f} V)"
+    return f"Battery is {volts(voltage)}; needs above {volts(cutoff)}"
+  return f"{fallback} (battery {volts(voltage)})"
 
 
 class RuntimeStatus(EventStore):

openpilot/starpilot/sentry_mode/storage.py

@@ -25,6 +25,14 @@ DIR_FLAGS = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW
 FILE_FLAGS = os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK
 
 
+def valid_tenths(value: object) -> bool:
+  return type(value) is int and 0 < value < 1000
+
+
+def volts(tenths: int) -> str:
+  return f"{tenths / 10:.1f} V"
+
+
 class StorageUnavailable(Exception):
   """No event was published by this call; recording cannot continue."""
 
@@ -51,7 +59,7 @@ def _valid(value: object, event_id: str) -> bool:
   base = FIELDS | BATTERY_FIELDS if battery else FIELDS
   if set(fields) not in (base, base | {"simulated"}) or ("simulated" in fields and fields["simulated"] is not True) or ID.fullmatch(event_id) is None:
     return False
-  if battery and not all(type(fields[key]) is int and 0 < fields[key] < 1000 for key in BATTERY_FIELDS):
+  if battery and not all(valid_tenths(fields[key]) for key in BATTERY_FIELDS):
     return False
   version = fields["version"]
   session_id = fields["sessionId"]

openpilot/starpilot/sentry_mode/tests/test_runtime.py

@@ -70,6 +70,7 @@ class TestRuntime(unittest.TestCase):
     self.addCleanup(self.environment.stop)
     self.params = Params()
     self.params.put_bool("IsOffroad", True, block=True)
+    self.cutoff = effective_power(read_power(self.params)).cutoff_tenths
     Path(self.params.get_param_path(KEY)).write_bytes(encode(Preferences(True)))
     self.clock = Clock()
     self.messages = Messages(self.clock)
@@ -89,7 +90,7 @@ class TestRuntime(unittest.TestCase):
     status = Mock()
     self.runtime.status = status
     decision = self.tick()
-    status.publish.assert_called_once_with(decision, encode(Preferences(True)), "idle", power=(125, effective_power(read_power(self.params)).cutoff_tenths))
+    status.publish.assert_called_once_with(decision, encode(Preferences(True)), "idle", power=(125, self.cutoff))
     self.assertEqual(decision.state, "arming")
     self.messages.updated["accelerometer"] = False
     self.messages.valid["accelerometer"] = False
@@ -115,7 +116,7 @@ class TestRuntime(unittest.TestCase):
   def test_low_battery_warns_once_after_holding_near_cutoff_and_rearms_after_recovery(self):
     warnings = []
     self.runtime.battery = lambda voltage, cutoff: warnings.append((voltage, cutoff))
-    cutoff = effective_power(read_power(self.params)).cutoff_tenths
+    cutoff = self.cutoff
     self.messages.voltage = (cutoff + 2) * 100
     for _ in range(599):
       self.tick()
@@ -137,7 +138,7 @@ class TestRuntime(unittest.TestCase):
   def test_brief_voltage_dip_does_not_warn(self):
     warnings = []
     self.runtime.battery = lambda voltage, cutoff: warnings.append((voltage, cutoff))
-    cutoff = effective_power(read_power(self.params)).cutoff_tenths
+    cutoff = self.cutoff
     for _ in range(20):
       self.messages.voltage = (cutoff + 1) * 100
       for _ in range(300):
@@ -146,6 +147,26 @@ class TestRuntime(unittest.TestCase):
       self.tick()
     self.assertEqual(warnings, [])
 
+  def test_battery_warning_storage_failure_is_logged_but_bugs_propagate(self):
+    self.messages.voltage = (self.cutoff - 1) * 100
+    self.runtime.battery = Mock(side_effect=runtime_module.StorageUnavailable("Event storage is full"))
+    for _ in range(101):
+      self.tick()
+    self.runtime.battery.assert_called_once()
+    self.runtime.battery_warned = False
+    self.runtime.battery = Mock(side_effect=ValueError("Invalid Sentry event"))
+    with self.assertRaises(ValueError):
+      for _ in range(101):
+        self.tick()
+
+  def test_recent_battery_warning_ignores_old_other_and_future_events(self):
+    hour = runtime_module.BATTERY_REPEAT_NS
+    now = 10 * hour
+    self.assertTrue(runtime_module.recent_battery_warning([{"kind": "battery", "wallTimeNs": now - hour}], now))
+    self.assertFalse(runtime_module.recent_battery_warning([{"kind": "battery", "wallTimeNs": now - hour - 1}], now))
+    self.assertFalse(runtime_module.recent_battery_warning([{"kind": "warning", "wallTimeNs": now}], now))
+    self.assertFalse(runtime_module.recent_battery_warning([{"kind": "battery", "wallTimeNs": now + 1}], now))
+
   def test_arming_completes_with_brief_gaps_between_sensor_messages(self):
     for _ in range(451):
       self.tick()
@@ -156,7 +177,6 @@ class TestRuntime(unittest.TestCase):
     self.assertEqual(decision.state, "armed")
     self.assertEqual(self.events, [])
 
-
   def test_serialized_device_panda_peripheral_use_actual_clock_domains(self):
     self.clock.mono += 100_000_000
     sensor_raw = self.messages.set_sources()