StarPort: Report fix outcomes, focus milestones and head-unit versions

openpilot/starpilot/system/starport/bootstrap.py

@@ -489,8 +489,12 @@ class WirelessBootstrap:
         # Some receivers send the network before the endpoint; keep it so it is not asked for again.
         # One that cannot be read is ignored as before, and the network is asked for after the endpoint.
         try:
+          earlier = credentials
           credentials = parse_info_response(payload)
           self._reply_accepted = True
+          if earlier is not None:
+            self.log("bootstrap_info_repeat", same_network=(earlier.ssid, earlier.bssid, earlier.security) ==
+                     (credentials.ssid, credentials.bssid, credentials.security), phase="before_join")
         except ValueError as error:
           self.log("bootstrap_early_info_unreadable", error=str(error))
       else:
@@ -608,5 +612,12 @@ class WirelessBootstrap:
         return
 
 
+STATUS_LABELS = {0: "success", -1: "no compatible version", -3: "incorrect credentials", -6: "not yet started", -11: "network unavailable"}
+
+
 def describe_status(status: int | None) -> str:
-  return "success" if signed(status) == 0 else f"status {signed(status)}"
+  value = signed(status)
+  label = STATUS_LABELS.get(value)
+  if label is None:
+    return f"status {value}"
+  return label if value == 0 else f"status {value} ({label})"

openpilot/starpilot/system/starport/bugsink_diagnosis.py

@@ -18,9 +18,13 @@ EVIDENCE_EVENTS = {"bootstrap_start_response", "wifi_joined", "tls_established",
                    "usb_recover_failed", "encoder_failed", "encoder_recovered", "encoder_reopen_failed", "headunitpad_rejected"}
 
 
+VERSION_KEYS = {"head_unit_software_version", "head_unit_software_build"}
+
+
 def sanitize(value, names=()):
   if isinstance(value, dict):
-    return {key: sanitize(item, names) for key, item in value.items() if str(key).lower() not in PRIVATE_KEYS}
+    return {key: _sanitize_version(item) if key in VERSION_KEYS and isinstance(item, str) else sanitize(item, names)
+            for key, item in value.items() if str(key).lower() not in PRIVATE_KEYS}
   if isinstance(value, (tuple, list)):
     return [sanitize(item, names) for item in value]
   if isinstance(value, float) and not math.isfinite(value):
@@ -42,6 +46,11 @@ def sanitize(value, names=()):
   return IPV4.sub("<ip>", value)[:8000]
 
 
+def _sanitize_version(value: str) -> str:
+  """A head unit's software version is public build text; a dotted version is not an address."""
+  return SECRET.sub(lambda m: m[1] + "=<redacted>", ADDRESS.sub("<address>", value))[:80]
+
+
 def elapsed(start, end):
   try:
     if isinstance(start.get("mono_s"), (float, int)) and isinstance(end.get("mono_s"), (float, int)):

openpilot/starpilot/system/starport/bugsink_report.py

@@ -219,6 +219,40 @@ def session_setup(events: list[dict]) -> dict:
   ) if value}
 
 
+HANGUP_BUCKETS = ((10, "0-10s"), (30, "10-30s"), (120, "30-120s"))
+MILESTONES = ("discovery_requested", "discovered", "focus_requested", "focus_granted", "video_acknowledged")
+
+
+def session_fixes(events: list[dict]) -> dict:
+  """Car-specific fixes this session ran, how it ended for them, and how far the car got before it ended the session."""
+  fixes: dict = {}
+  reached = set()
+  for event in events:
+    name = event["event"]
+    if name == "fix_applied":
+      fixes["rule"], fixes["level"] = str(event.get("rule", ""))[:40], event.get("level")
+    elif name == "fix_outcome":
+      fixes["verdict"], fixes["state"] = str(event.get("verdict", ""))[:16], str(event.get("state", ""))[:16]
+    elif name == "peer_requested_shutdown":
+      fixes["shutdown"] = str(event.get("reason_label", ""))[:24]
+      if isinstance(event.get("after_focus_s"), (int, float)):
+        fixes["after_focus_s"] = event["after_focus_s"]
+    elif name == "video_focus" and event.get("focused"):
+      reached.add("focus_granted")
+      if isinstance(event.get("wait_s"), (int, float)):
+        fixes["focus_wait_s"] = event["wait_s"]
+    elif name in MILESTONES:
+      reached.add(name)
+  furthest = next((m for m in reversed(MILESTONES) if m in reached), "")
+  return {**fixes, **({"milestone": furthest} if furthest else {})}
+
+
+def hangup_bucket(seconds) -> str:
+  if not isinstance(seconds, (int, float)):
+    return ""
+  return next((label for limit, label in HANGUP_BUCKETS if seconds < limit), ">120s")
+
+
 def session_ladder(events: list[dict]) -> list[dict]:
   """Summarize attempted handshakes, outcomes, failure reasons and busy waits."""
   tried: list[dict] = []
@@ -289,6 +323,7 @@ def session_record(path: Path) -> dict:
     "phone_address_sent": bool(opened.get("phone_address")) if opened is not None else None,
     "strategy": session_strategy(events, setup),
     "ladder": session_ladder(events),
+    "fixes": session_fixes(events),
     "sdp": session_sdp(events),
     "software": {key: str(software.get(key) or "unknown") for key in ("commit", "branch")} if isinstance(software, dict) else {},
     "settings": settings if isinstance(settings, dict) else {},
@@ -510,6 +545,9 @@ def drive_event(sessions: list[dict]) -> dict:
   sent = next((s["phone_address_sent"] for s in reversed(sessions) if s.get("phone_address_sent") is not None), None)
   ladder = next((s["ladder"][-1] for s in reversed(sessions) if s.get("ladder")), {})
   strategy = next((s["strategy"] for s in reversed(sessions) if s.get("strategy")), {})
+  fixes = next((s["fixes"] for s in reversed(sessions) if s.get("fixes", {}).get("rule")), {})
+  last_fixes = next((s["fixes"] for s in reversed(sessions) if s.get("fixes")), {})
+  ended_after = next((s["streamed_s"] for s in reversed(sessions) if s.get("ended_by_car") and s.get("streamed_s") is not None), None)
   failures = [failure_detail(failure) for failure in drive_failures(sessions)]
   failed = [session for session in sessions if session["seq"] in {failure["session"] for failure in failures}]
   heading = "failures" if failures else "sessions"
@@ -547,6 +585,10 @@ def drive_event(sessions: list[dict]) -> dict:
              **({"aa.phone_address_sent": "yes" if sent else "no"} if sent is not None else {}),
              **({"aa.reply_style": strategy["reply_style"]} if strategy.get("reply_style") else {}),
              **({"aa.wifi_channel_type": str(strategy["channel_type"])} if strategy.get("channel_type") else {}),
+             **({"aa.fixes": f"{fixes['rule']}:{fixes.get('level')}"[:60]} if fixes else {}),
+             **({"aa.fix_verdict": fixes["verdict"]} if fixes.get("verdict") else {}),
+             **({"aa.milestone": last_fixes["milestone"]} if last_fixes.get("milestone") else {}),
+             **({"aa.hangup_after": hangup_bucket(ended_after)} if ended_after is not None else {}),
              **({"aa.ladder_rung": ladder["rung"], "aa.ladder_learned": "yes" if ladder.get("learned") else "no"} if ladder else {}),
              **({"aa.attention": ",".join(reasons)[:190]} if reasons else {}),
              **({"aa.cause": latest["cause"], "aa.stage": latest["stage"], "aa.error_kind": latest["error_kind"]} if latest else {}),

openpilot/starpilot/system/starport/session.py

@@ -526,6 +526,7 @@ class Session:
     self.event("authenticated", version=self.tls.version(), cipher=cipher[0])
 
   def discover(self, device_name: str, device_brand: str) -> list[dict]:
+    self.event("discovery_requested")
     self.send(0, MSG_SERVICE_DISCOVERY_REQUEST, field(4, device_name) + field(5, device_brand))
     raw = self.wait_for(0, MSG_SERVICE_DISCOVERY_RESPONSE, timeout=self.DISCOVERY_TIMEOUT, patient=True)
     response = parse_fields(raw)
@@ -852,6 +853,7 @@ class ProjectionSession(Session):
     assert self.mode is not None
     self.allow_projection = True
     self.focus_requested_at = time.monotonic()
+    self.event("focus_requested")
     self.send(self.mode.channel, VIDEO_FOCUS_REQUEST, field(2, FOCUS_PROJECTED) + field(3, FOCUS_REASON_USER_SELECTION))
 
   def request_native(self) -> bool:

openpilot/starpilot/system/starport/supervisor.py

@@ -81,6 +81,16 @@ REPORTED_SETTINGS = ("connection", "usb_mode", "view", "encoder", "fps", "bitrat
                      "head_unit_experiment_rules")
 
 
+def _tcp_failure_kind(error: OSError) -> str:
+  if isinstance(error, (TimeoutError, socket.timeout)):
+    return "timeout"
+  if isinstance(error, ConnectionRefusedError):
+    return "refused"
+  if getattr(error, "errno", None) in (errno.ENETUNREACH, errno.EHOSTUNREACH, errno.ENETDOWN, errno.EHOSTDOWN):
+    return "unreachable"
+  return "other"
+
+
 def encoder_preference(configured: str) -> str:
   return 'hardware' if COMMA_HARDWARE else configured
 
@@ -1965,7 +1975,7 @@ class Supervisor:
         return self._track(sock)
       except OSError as error:
         last_error = error
-        self.log("tcp_retry", attempt=attempt + 1, error=str(error))
+        self.log("tcp_retry", attempt=attempt + 1, error=str(error), kind=_tcp_failure_kind(error))
         self._wait(1.0 + attempt * 0.5)
     raise RuntimeError(f"Car did not accept the projection connection: {last_error}")
 

openpilot/starpilot/system/starport/tests/test_bugsink_diagnosis.py

@@ -0,0 +1,6 @@
+
+
+def test_head_unit_software_versions_are_not_redacted_as_addresses():
+  from openpilot.starpilot.system.starport.bugsink_diagnosis import sanitize
+  clean = sanitize({"head_unit_software_version": "1.2.3.4", "head_unit_software_build": "W28E-V155.3.8", "detail": "peer 1.2.3.4"})
+  assert clean == {"head_unit_software_version": "1.2.3.4", "head_unit_software_build": "W28E-V155.3.8", "detail": "peer <ip>"}

openpilot/starpilot/system/starport/tests/test_bugsink_report.py

@@ -539,3 +539,26 @@ def test_trace_keeps_how_the_session_started_as_well_as_how_it_ended(tmp_path):
   assert names[:2] == ["session_start", "hfp_initiator"] and names[-2:] == ["attempt_failed", "session_stop"]
   assert trace["omitted"] > 0 and 0 < trace["head"] < len(names)
   assert len(json.dumps(trace)) <= br.TRACE_SESSION_BYTES + 1024
+
+
+def test_fix_milestones_and_hangup_bucket_are_reported_without_changing_other_tags(tmp_path):
+  events = STREAMED[:10] + [
+    (0, 10, "fix_applied", {"rule": "gm_mychevrolet_w28e", "level": 1, "actions": ["open_bluetooth"]}),
+    (0, 11, "focus_requested", {}),
+    (0, 12, "video_focus", {"focused": True, "wait_s": 0.4}),
+    (0, 12, "video_acknowledged", {}),
+    (0, 40, "peer_requested_shutdown", {"reason": 1, "reason_label": "user_selection", "after_focus_s": 28.0}),
+    (0, 41, "fix_outcome", {"rule": "gm_mychevrolet_w28e", "level": 1, "verdict": "failed", "state": "trying"}),
+  ]
+  record = br.session_record(write_session(tmp_path, 1, events))
+  assert record["fixes"] == {"rule": "gm_mychevrolet_w28e", "level": 1, "focus_wait_s": 0.4, "shutdown": "user_selection",
+                             "after_focus_s": 28.0, "verdict": "failed", "state": "trying", "milestone": "video_acknowledged"}
+  tags = br.drive_event([record])["tags"]
+  assert tags["aa.fixes"] == "gm_mychevrolet_w28e:1" and tags["aa.fix_verdict"] == "failed"
+  assert tags["aa.milestone"] == "video_acknowledged"
+  plain = br.drive_event([br.session_record(write_session(tmp_path, 2, STREAMED))])["tags"]
+  assert not any(key in plain for key in ("aa.fixes", "aa.fix_verdict", "aa.hangup_after"))
+
+
+def test_hangup_bucket_edges():
+  assert [br.hangup_bucket(v) for v in (0, 9.9, 10, 29, 30, 119, 120, None)] == ["0-10s", "0-10s", "10-30s", "10-30s", "30-120s", "30-120s", ">120s", ""]

openpilot/starpilot/system/starport/tests/test_observation.py

@@ -0,0 +1,21 @@
+import errno
+import socket
+
+from openpilot.starpilot.system.starport import bootstrap as bs
+from openpilot.starpilot.system.starport.supervisor import _tcp_failure_kind
+
+
+def test_wifi_status_labels_name_the_known_codes():
+  assert bs.describe_status(0) == "success"
+  assert bs.describe_status(-1) == "status -1 (no compatible version)"
+  assert bs.describe_status(-11) == "status -11 (network unavailable)"
+  assert bs.describe_status(-3) == "status -3 (incorrect credentials)"
+  assert bs.describe_status(-99) == "status -99"
+  assert bs.describe_status(2**64 - 1) == "status -1 (no compatible version)"
+
+
+def test_tcp_failure_kinds():
+  assert _tcp_failure_kind(socket.timeout()) == "timeout"
+  assert _tcp_failure_kind(ConnectionRefusedError()) == "refused"
+  assert _tcp_failure_kind(OSError(errno.EHOSTUNREACH, "no route")) == "unreachable"
+  assert _tcp_failure_kind(OSError(errno.EPERM, "denied")) == "other"