diff --git a/openpilot/starpilot/system/starport/bootstrap.py b/openpilot/starpilot/system/starport/bootstrap.py index e6419f6fb3..d352255d81 100644 --- a/openpilot/starpilot/system/starport/bootstrap.py +++ b/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})" diff --git a/openpilot/starpilot/system/starport/bugsink_diagnosis.py b/openpilot/starpilot/system/starport/bugsink_diagnosis.py index b7630d0187..2fb83d6f97 100644 --- a/openpilot/starpilot/system/starport/bugsink_diagnosis.py +++ b/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("", 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] + "=", ADDRESS.sub("
", value))[:80] + + def elapsed(start, end): try: if isinstance(start.get("mono_s"), (float, int)) and isinstance(end.get("mono_s"), (float, int)): diff --git a/openpilot/starpilot/system/starport/bugsink_report.py b/openpilot/starpilot/system/starport/bugsink_report.py index b31ac11a9f..ac651edf68 100644 --- a/openpilot/starpilot/system/starport/bugsink_report.py +++ b/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 {}), diff --git a/openpilot/starpilot/system/starport/session.py b/openpilot/starpilot/system/starport/session.py index a5dba99624..9a01146c66 100644 --- a/openpilot/starpilot/system/starport/session.py +++ b/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: diff --git a/openpilot/starpilot/system/starport/supervisor.py b/openpilot/starpilot/system/starport/supervisor.py index e1bafa3e81..5a5f100fb1 100644 --- a/openpilot/starpilot/system/starport/supervisor.py +++ b/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}") diff --git a/openpilot/starpilot/system/starport/tests/test_bugsink_diagnosis.py b/openpilot/starpilot/system/starport/tests/test_bugsink_diagnosis.py new file mode 100644 index 0000000000..f52ac6a3bd --- /dev/null +++ b/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 "} diff --git a/openpilot/starpilot/system/starport/tests/test_bugsink_report.py b/openpilot/starpilot/system/starport/tests/test_bugsink_report.py index c3da61bff8..7d3c784d69 100644 --- a/openpilot/starpilot/system/starport/tests/test_bugsink_report.py +++ b/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", ""] diff --git a/openpilot/starpilot/system/starport/tests/test_observation.py b/openpilot/starpilot/system/starport/tests/test_observation.py new file mode 100644 index 0000000000..77cc486d92 --- /dev/null +++ b/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"