openpilot v0.9.7 release

date: 2024-06-11T01:36:39
master commit: f8cb04e4a8

tools/lib/README.md

@@ -34,10 +34,16 @@ for msg in lr:
 
 ### Segment Ranges
 
-We also support a new format called a "segment range", where you can specify which segments from a route to load.
+We also support a new format called a "segment range":
+
+```
+344c5c15b34f2d8a   /   2024-01-03--09-37-12   /     2:6    /       q
+[   dongle id     ] [       timestamp        ] [ selector ]  [ query type]
+```
+
+you can specify which segments from a route to load
 
 ```python
-
 lr = LogReader("a2a0ccea32023010|2023-07-27--13-01-19/4")   # 4th segment
 lr = LogReader("a2a0ccea32023010|2023-07-27--13-01-19/4:6") # 4th and 5th segment
 lr = LogReader("a2a0ccea32023010|2023-07-27--13-01-19/-1")  # last segment

tools/lib/api.py

@@ -2,7 +2,7 @@ import os
 import requests
 API_HOST = os.getenv('API_HOST', 'https://api.commadotai.com')
 
-class CommaApi():
+class CommaApi:
   def __init__(self, token=None):
     self.session = requests.Session()
     self.session.headers['User-agent'] = 'OpenpilotTools'

tools/lib/auth.py

@@ -26,7 +26,7 @@ import sys
 import pprint
 import webbrowser
 from http.server import BaseHTTPRequestHandler, HTTPServer
-from typing import Any, Dict
+from typing import Any
 from urllib.parse import parse_qs, urlencode
 
 from openpilot.tools.lib.api import APIError, CommaApi, UnauthorizedError
@@ -36,7 +36,7 @@ PORT = 3000
 
 
 class ClientRedirectServer(HTTPServer):
-  query_params: Dict[str, Any] = {}
+  query_params: dict[str, Any] = {}
 
 
 class ClientRedirectHandler(BaseHTTPRequestHandler):

tools/lib/azure_container.py

@@ -2,7 +2,7 @@ import os
 from datetime import datetime, timedelta
 from functools import lru_cache
 from pathlib import Path
-from typing import IO, Union
+from typing import IO
 
 
 TOKEN_PATH = Path("/data/azure_token")
@@ -57,18 +57,18 @@ class AzureContainer:
     ext = "hevc" if log_type.endswith('camera') else "bz2"
     return self.BASE_URL + f"{route_name.replace('|', '/')}/{segment_num}/{log_type}.{ext}"
 
-  def upload_bytes(self, data: Union[bytes, IO], blob_name: str) -> str:
+  def upload_bytes(self, data: bytes | IO, blob_name: str, overwrite=False) -> str:
     from azure.storage.blob import BlobClient
     blob = BlobClient(
       account_url=self.ACCOUNT_URL,
       container_name=self.CONTAINER,
       blob_name=blob_name,
       credential=get_azure_credential(),
-      overwrite=False,
+      overwrite=overwrite,
     )
-    blob.upload_blob(data)
+    blob.upload_blob(data, overwrite=overwrite)
     return self.BASE_URL + blob_name
 
-  def upload_file(self, path: Union[str, os.PathLike], blob_name: str) -> str:
+  def upload_file(self, path: str | os.PathLike, blob_name: str, overwrite=False) -> str:
     with open(path, "rb") as f:
-      return self.upload_bytes(f, blob_name)
+      return self.upload_bytes(f, blob_name, overwrite)

tools/lib/bootlog.py

@@ -1,6 +1,5 @@
 import functools
 import re
-from typing import List, Optional
 
 from openpilot.tools.lib.auth_config import get_token
 from openpilot.tools.lib.api import CommaApi
@@ -44,7 +43,7 @@ class Bootlog:
       return False
     return self.id < b.id
 
-def get_bootlog_from_id(bootlog_id: str) -> Optional[Bootlog]:
+def get_bootlog_from_id(bootlog_id: str) -> Bootlog | None:
   # TODO: implement an API endpoint for this
   bl = Bootlog(bootlog_id)
   for b in get_bootlogs(bl.dongle_id):
@@ -52,7 +51,7 @@ def get_bootlog_from_id(bootlog_id: str) -> Optional[Bootlog]:
       return b
   return None
 
-def get_bootlogs(dongle_id: str) -> List[Bootlog]:
+def get_bootlogs(dongle_id: str) -> list[Bootlog]:
   api = CommaApi(get_token())
   r = api.get(f'v1/devices/{dongle_id}/bootlogs')
   return [Bootlog(b) for b in r]

tools/lib/comma_car_segments.py

@@ -1,13 +1,22 @@
 import os
 import requests
 
+
 # Forks with additional car support can fork the commaCarSegments repo on huggingface or host the LFS files themselves
 COMMA_CAR_SEGMENTS_REPO = os.environ.get("COMMA_CAR_SEGMENTS_REPO", "https://huggingface.co/datasets/commaai/commaCarSegments")
 COMMA_CAR_SEGMENTS_BRANCH = os.environ.get("COMMA_CAR_SEGMENTS_BRANCH", "main")
 COMMA_CAR_SEGMENTS_LFS_INSTANCE = os.environ.get("COMMA_CAR_SEGMENTS_LFS_INSTANCE", COMMA_CAR_SEGMENTS_REPO)
 
 def get_comma_car_segments_database():
-  return requests.get(get_repo_raw_url("database.json")).json()
+  from openpilot.selfdrive.car.fingerprints import MIGRATION
+
+  database = requests.get(get_repo_raw_url("database.json")).json()
+
+  ret = {}
+  for platform in database:
+    ret[MIGRATION.get(platform, platform)] = database[platform]
+
+  return ret
 
 
 # Helpers related to interfacing with the commaCarSegments repository, which contains a collection of public segments for users to perform validation on.

tools/lib/filereader.py

@@ -10,10 +10,11 @@ DATA_ENDPOINT = os.getenv("DATA_ENDPOINT", "http://data-raw.comma.internal/")
 def internal_source_available():
   try:
     hostname = urlparse(DATA_ENDPOINT).hostname
-    if hostname:
-      socket.gethostbyname(hostname)
-      return True
-  except socket.gaierror:
+    port = urlparse(DATA_ENDPOINT).port or 80
+    with socket.socket(socket.AF_INET,socket.SOCK_STREAM) as s:
+      s.connect((hostname, port))
+    return True
+  except (socket.gaierror, ConnectionRefusedError):
     pass
   return False
 

tools/lib/helpers.py

@@ -1,7 +1,4 @@
 import bz2
-import datetime
-
-TIME_FMT = "%Y-%m-%d--%H-%M-%S"
 
 
 # regex patterns
@@ -9,25 +6,18 @@ class RE:
   DONGLE_ID = r'(?P<dongle_id>[a-f0-9]{16})'
   TIMESTAMP = r'(?P<timestamp>[0-9]{4}-[0-9]{2}-[0-9]{2}--[0-9]{2}-[0-9]{2}-[0-9]{2})'
   LOG_ID_V2 = r'(?P<count>[a-f0-9]{8})--(?P<uid>[a-z0-9]{10})'
-  LOG_ID = r'(?P<log_id>(?:{}|{}))'.format(TIMESTAMP, LOG_ID_V2)
-  ROUTE_NAME = r'(?P<route_name>{}[|_/]{})'.format(DONGLE_ID, LOG_ID)
-  SEGMENT_NAME = r'{}(?:--|/)(?P<segment_num>[0-9]+)'.format(ROUTE_NAME)
+  LOG_ID = fr'(?P<log_id>(?:{TIMESTAMP}|{LOG_ID_V2}))'
+  ROUTE_NAME = fr'(?P<route_name>{DONGLE_ID}[|_/]{LOG_ID})'
+  SEGMENT_NAME = fr'{ROUTE_NAME}(?:--|/)(?P<segment_num>[0-9]+)'
 
   INDEX = r'-?[0-9]+'
-  SLICE = r'(?P<start>{})?:?(?P<end>{})?:?(?P<step>{})?'.format(INDEX, INDEX, INDEX)
-  SEGMENT_RANGE = r'{}(?:(--|/)(?P<slice>({})))?(?:/(?P<selector>([qras])))?'.format(ROUTE_NAME, SLICE)
+  SLICE = fr'(?P<start>{INDEX})?:?(?P<end>{INDEX})?:?(?P<step>{INDEX})?'
+  SEGMENT_RANGE = fr'{ROUTE_NAME}(?:(--|/)(?P<slice>({SLICE})))?(?:/(?P<selector>([qras])))?'
 
   BOOTLOG_NAME = ROUTE_NAME
 
-  EXPLORER_FILE = r'^(?P<segment_name>{})--(?P<file_name>[a-z]+\.[a-z0-9]+)$'.format(SEGMENT_NAME)
-  OP_SEGMENT_DIR = r'^(?P<segment_name>{})$'.format(SEGMENT_NAME)
-
-
-def timestamp_to_datetime(t: str) -> datetime.datetime:
-  """
-    Convert an openpilot route timestamp to a python datetime
-  """
-  return datetime.datetime.strptime(t, TIME_FMT)
+  EXPLORER_FILE = fr'^(?P<segment_name>{SEGMENT_NAME})--(?P<file_name>[a-z]+\.[a-z0-9]+)$'
+  OP_SEGMENT_DIR = fr'^(?P<segment_name>{SEGMENT_NAME})$'
 
 
 def save_log(dest, log_msgs, compress=True):

tools/lib/live_logreader.py

@@ -1,5 +1,4 @@
 import os
-from typing import List
 from cereal import log as capnp_log, messaging
 from cereal.services import SERVICE_LIST
 
@@ -8,7 +7,7 @@ from openpilot.tools.lib.logreader import LogIterable, RawLogIterable
 
 ALL_SERVICES = list(SERVICE_LIST.keys())
 
-def raw_live_logreader(services: List[str] = ALL_SERVICES, addr: str = '127.0.0.1') -> RawLogIterable:
+def raw_live_logreader(services: list[str] = ALL_SERVICES, addr: str = '127.0.0.1') -> RawLogIterable:
   if addr != "127.0.0.1":
     os.environ["ZMQ"] = "1"
     messaging.context = messaging.Context()
@@ -25,7 +24,7 @@ def raw_live_logreader(services: List[str] = ALL_SERVICES, addr: str = '127.0.0.
       yield msg
 
 
-def live_logreader(services: List[str] = ALL_SERVICES, addr: str = '127.0.0.1') -> LogIterable:
+def live_logreader(services: list[str] = ALL_SERVICES, addr: str = '127.0.0.1') -> LogIterable:
   for m in raw_live_logreader(services, addr):
     with capnp_log.Event.from_bytes(m) as evt:
       yield evt

tools/lib/logreader.py

@@ -11,7 +11,7 @@ import tqdm
 import urllib.parse
 import warnings
 
-from typing import Callable, Dict, Iterable, Iterator, List, Optional, Type
+from collections.abc import Callable, Iterable, Iterator
 from urllib.parse import parse_qs, urlparse
 
 from cereal import log as capnp_log
@@ -21,7 +21,7 @@ from openpilot.tools.lib.openpilotci import get_url
 from openpilot.tools.lib.filereader import FileReader, file_exists, internal_source_available
 from openpilot.tools.lib.route import Route, SegmentRange
 
-LogMessage = Type[capnp._DynamicStructReader]
+LogMessage = type[capnp._DynamicStructReader]
 LogIterable = Iterable[LogMessage]
 RawLogIterable = Iterable[bytes]
 
@@ -76,8 +76,8 @@ class ReadMode(enum.StrEnum):
   AUTO_INTERACTIVE = "i"  # default to rlogs, fallback to qlogs with a prompt from the user
 
 
-LogPath = Optional[str]
-LogPaths = List[LogPath]
+LogPath = str | None
+LogPaths = list[LogPath]
 ValidFileCallable = Callable[[LogPath], bool]
 Source = Callable[[SegmentRange, ReadMode], LogPaths]
 
@@ -89,7 +89,7 @@ def default_valid_file(fn: LogPath) -> bool:
 
 def auto_strategy(rlog_paths: LogPaths, qlog_paths: LogPaths, interactive: bool, valid_file: ValidFileCallable) -> LogPaths:
   # auto select logs based on availability
-  if any(rlog is None or not valid_file(rlog) for rlog in rlog_paths):
+  if any(rlog is None or not valid_file(rlog) for rlog in rlog_paths) and all(qlog is not None and valid_file(qlog) for qlog in qlog_paths):
     if interactive:
       if input("Some rlogs were not found, would you like to fallback to qlogs for those segments? (y/n) ").lower() != "y":
         return rlog_paths
@@ -170,8 +170,17 @@ def auto_source(sr: SegmentRange, mode=ReadMode.RLOG) -> LogPaths:
   if mode == ReadMode.SANITIZED:
     return comma_car_segments_source(sr, mode)
 
-  SOURCES: List[Source] = [internal_source, openpilotci_source, comma_api_source, comma_car_segments_source,]
+  SOURCES: list[Source] = [internal_source, openpilotci_source, comma_api_source, comma_car_segments_source,]
   exceptions = []
+
+  # for automatic fallback modes, auto_source needs to first check if rlogs exist for any source
+  if mode in [ReadMode.AUTO, ReadMode.AUTO_INTERACTIVE]:
+    for source in SOURCES:
+      try:
+        return check_source(source, sr, ReadMode.RLOG)
+      except Exception:
+        pass
+
   # Automatically determine viable source
   for source in SOURCES:
     try:
@@ -212,7 +221,7 @@ def parse_indirect(identifier: str):
 
 
 class LogReader:
-  def _parse_identifiers(self, identifier: str | List[str]):
+  def _parse_identifiers(self, identifier: str | list[str]):
     if isinstance(identifier, list):
       return [i for j in identifier for i in self._parse_identifiers(j)]
 
@@ -234,7 +243,7 @@ class LogReader:
 are uploaded or auto fallback to qlogs with '/a' selector at the end of the route name."
     return identifiers
 
-  def __init__(self, identifier: str | List[str], default_mode: ReadMode = ReadMode.RLOG,
+  def __init__(self, identifier: str | list[str], default_mode: ReadMode = ReadMode.RLOG,
                default_source=auto_source, sort_by_time=False, only_union_types=False):
     self.default_mode = default_mode
     self.default_source = default_source
@@ -243,12 +252,12 @@ are uploaded or auto fallback to qlogs with '/a' selector at the end of the rout
     self.sort_by_time = sort_by_time
     self.only_union_types = only_union_types
 
-    self.__lrs: Dict[int, _LogFileReader] = {}
+    self.__lrs: dict[int, _LogFileReader] = {}
     self.reset()
 
   def _get_lr(self, i):
     if i not in self.__lrs:
-      self.__lrs[i] = _LogFileReader(self.logreader_identifiers[i])
+      self.__lrs[i] = _LogFileReader(self.logreader_identifiers[i], sort_by_time=self.sort_by_time, only_union_types=self.only_union_types)
     return self.__lrs[i]
 
   def __iter__(self):

tools/lib/route.py

@@ -4,7 +4,6 @@ from functools import cache
 from urllib.parse import urlparse
 from collections import defaultdict
 from itertools import chain
-from typing import Optional, cast
 
 from openpilot.tools.lib.auth_config import get_token
 from openpilot.tools.lib.api import CommaApi
@@ -231,7 +230,7 @@ class SegmentName:
   def route_name(self) -> RouteName: return self._route_name
 
   @property
-  def data_dir(self) -> Optional[str]: return self._data_dir
+  def data_dir(self) -> str | None: return self._data_dir
 
   def __str__(self) -> str: return self._canonical_name
 
@@ -240,7 +239,9 @@ class SegmentName:
 def get_max_seg_number_cached(sr: 'SegmentRange') -> int:
   try:
     api = CommaApi(get_token())
-    return cast(int, api.get("/v1/route/" + sr.route_name.replace("/", "|"))["segment_numbers"][-1])
+    max_seg_number = api.get("/v1/route/" + sr.route_name.replace("/", "|"))["maxqlog"]
+    assert isinstance(max_seg_number, int)
+    return max_seg_number
   except Exception as e:
     raise Exception("unable to get max_segment_number. ensure you have access to this route or the route is public.") from e
 
@@ -263,6 +264,10 @@ class SegmentRange:
   def timestamp(self) -> str:
     return self.m.group("timestamp")
 
+  @property
+  def log_id(self) -> str:
+    return self.m.group("log_id")
+
   @property
   def slice(self) -> str:
     return self.m.group("slice") or ""
@@ -291,7 +296,7 @@ class SegmentRange:
       return list(range(end + 1))[s]
 
   def __str__(self) -> str:
-    return f"{self.dongle_id}/{self.timestamp}" + (f"/{self.slice}" if self.slice else "") + (f"/{self.selector}" if self.selector else "")
+    return f"{self.dongle_id}/{self.log_id}" + (f"/{self.slice}" if self.slice else "") + (f"/{self.selector}" if self.selector else "")
 
   def __repr__(self) -> str:
     return self.__str__()

tools/lib/tests/test_caching.py

@@ -1,12 +1,11 @@
-#!/usr/bin/env python3
-from functools import partial
 import http.server
 import os
-import unittest
-
-from parameterized import parameterized
-from openpilot.selfdrive.athena.tests.helpers import with_http_server
+import shutil
+import socket
+import pytest
 
+from openpilot.selfdrive.test.helpers import http_server_context
+from openpilot.system.hardware.hw import Paths
 from openpilot.tools.lib.url_file import URLFile
 
 
@@ -15,7 +14,7 @@ class CachingTestRequestHandler(http.server.BaseHTTPRequestHandler):
 
   def do_GET(self):
     if self.FILE_EXISTS:
-      self.send_response(200, b'1234')
+      self.send_response(206 if "Range" in self.headers else 200, b'1234')
     else:
       self.send_response(404)
     self.end_headers()
@@ -29,10 +28,39 @@ class CachingTestRequestHandler(http.server.BaseHTTPRequestHandler):
     self.end_headers()
 
 
-with_caching_server = partial(with_http_server, handler=CachingTestRequestHandler)
+@pytest.fixture
+def host():
+  with http_server_context(handler=CachingTestRequestHandler) as (host, port):
+    yield f"http://{host}:{port}"
 
+class TestFileDownload:
 
-class TestFileDownload(unittest.TestCase):
+  def test_pipeline_defaults(self, host):
+    # TODO: parameterize the defaults so we don't rely on hard-coded values in xx
+
+    assert URLFile.pool_manager().pools._maxsize == 10# PoolManager num_pools param
+    pool_manager_defaults = {
+      "maxsize": 100,
+      "socket_options": [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),],
+    }
+    for k, v in pool_manager_defaults.items():
+      assert URLFile.pool_manager().connection_pool_kw.get(k) == v
+
+    retry_defaults = {
+      "total": 5,
+      "backoff_factor": 0.5,
+      "status_forcelist": [409, 429, 503, 504],
+    }
+    for k, v in retry_defaults.items():
+      assert getattr(URLFile.pool_manager().connection_pool_kw["retries"], k) == v
+
+    # ensure caching off by default and cache dir doesn't get created
+    os.environ.pop("FILEREADER_CACHE", None)
+    if os.path.exists(Paths.download_cache_root()):
+      shutil.rmtree(Paths.download_cache_root())
+    URLFile(f"{host}/test.txt").get_length()
+    URLFile(f"{host}/test.txt").read()
+    assert not os.path.exists(Paths.download_cache_root())
 
   def compare_loads(self, url, start=0, length=None):
     """Compares range between cached and non cached version"""
@@ -42,21 +70,21 @@ class TestFileDownload(unittest.TestCase):
     file_cached.seek(start)
     file_downloaded.seek(start)
 
-    self.assertEqual(file_cached.get_length(), file_downloaded.get_length())
-    self.assertLessEqual(length + start if length is not None else 0, file_downloaded.get_length())
+    assert file_cached.get_length() == file_downloaded.get_length()
+    assert length + start if length is not None else 0 <= file_downloaded.get_length()
 
     response_cached = file_cached.read(ll=length)
     response_downloaded = file_downloaded.read(ll=length)
 
-    self.assertEqual(response_cached, response_downloaded)
+    assert response_cached == response_downloaded
 
     # Now test with cache in place
     file_cached = URLFile(url, cache=True)
     file_cached.seek(start)
     response_cached = file_cached.read(ll=length)
 
-    self.assertEqual(file_cached.get_length(), file_downloaded.get_length())
-    self.assertEqual(response_cached, response_downloaded)
+    assert file_cached.get_length() == file_downloaded.get_length()
+    assert response_cached == response_downloaded
 
   def test_small_file(self):
     # Make sure we don't force cache
@@ -87,22 +115,16 @@ class TestFileDownload(unittest.TestCase):
     self.compare_loads(large_file_url, length - 100, 100)
     self.compare_loads(large_file_url)
 
-  @parameterized.expand([(True, ), (False, )])
-  @with_caching_server
-  def test_recover_from_missing_file(self, cache_enabled, host):
+  @pytest.mark.parametrize("cache_enabled", [True, False])
+  def test_recover_from_missing_file(self, host, cache_enabled):
     os.environ["FILEREADER_CACHE"] = "1" if cache_enabled else "0"
 
     file_url = f"{host}/test.png"
 
     CachingTestRequestHandler.FILE_EXISTS = False
     length = URLFile(file_url).get_length()
-    self.assertEqual(length, -1)
+    assert length == -1
 
     CachingTestRequestHandler.FILE_EXISTS = True
     length = URLFile(file_url).get_length()
-    self.assertEqual(length, 4)
-
-
-
-if __name__ == "__main__":
-  unittest.main()
+    assert length == 4

tools/lib/tests/test_comma_car_segments.py

@@ -1,14 +1,13 @@
-
-
-import unittest
-
+import pytest
 import requests
+from openpilot.selfdrive.car.fingerprints import MIGRATION
 from openpilot.tools.lib.comma_car_segments import get_comma_car_segments_database, get_url
 from openpilot.tools.lib.logreader import LogReader
 from openpilot.tools.lib.route import SegmentRange
 
 
-class TestCommaCarSegments(unittest.TestCase):
+@pytest.mark.skip(reason="huggingface is flaky, run this test manually to check for issues")
+class TestCommaCarSegments:
   def test_database(self):
     database = get_comma_car_segments_database()
 
@@ -19,7 +18,7 @@ class TestCommaCarSegments(unittest.TestCase):
   def test_download_segment(self):
     database = get_comma_car_segments_database()
 
-    fp = "SUBARU FORESTER 2019"
+    fp = "SUBARU_FORESTER"
 
     segment = database[fp][0]
 
@@ -28,14 +27,8 @@ class TestCommaCarSegments(unittest.TestCase):
     url = get_url(sr.route_name, sr.slice)
 
     resp = requests.get(url)
-    self.assertEqual(resp.status_code, 200)
+    assert resp.status_code == 200
 
     lr = LogReader(url)
-
     CP = lr.first("carParams")
-
-    self.assertEqual(CP.carFingerprint, fp)
-
-
-if __name__ == "__main__":
-  unittest.main()
+    assert MIGRATION.get(CP.carFingerprint, CP.carFingerprint) == fp

tools/lib/tests/test_logreader.py

@@ -1,16 +1,15 @@
-#!/usr/bin/env python3
+import capnp
 import contextlib
 import io
 import shutil
 import tempfile
 import os
-import unittest
 import pytest
 import requests
 
 from parameterized import parameterized
-from unittest import mock
 
+from cereal import log as capnp_log
 from openpilot.tools.lib.logreader import LogIterable, LogReader, comma_api_source, parse_indirect, ReadMode, InternalUnavailableException
 from openpilot.tools.lib.route import SegmentRange
 from openpilot.tools.lib.url_file import URLFileException
@@ -26,24 +25,22 @@ def noop(segment: LogIterable):
 
 
 @contextlib.contextmanager
-def setup_source_scenario(is_internal=False):
-  with (
-    mock.patch("openpilot.tools.lib.logreader.internal_source") as internal_source_mock,
-    mock.patch("openpilot.tools.lib.logreader.openpilotci_source") as openpilotci_source_mock,
-    mock.patch("openpilot.tools.lib.logreader.comma_api_source") as comma_api_source_mock,
-  ):
-    if is_internal:
-      internal_source_mock.return_value = [QLOG_FILE]
-    else:
-      internal_source_mock.side_effect = InternalUnavailableException
+def setup_source_scenario(mocker, is_internal=False):
+  internal_source_mock = mocker.patch("openpilot.tools.lib.logreader.internal_source")
+  openpilotci_source_mock = mocker.patch("openpilot.tools.lib.logreader.openpilotci_source")
+  comma_api_source_mock = mocker.patch("openpilot.tools.lib.logreader.comma_api_source")
+  if is_internal:
+    internal_source_mock.return_value = [QLOG_FILE]
+  else:
+    internal_source_mock.side_effect = InternalUnavailableException
 
-    openpilotci_source_mock.return_value = [None]
-    comma_api_source_mock.return_value = [QLOG_FILE]
+  openpilotci_source_mock.return_value = [None]
+  comma_api_source_mock.return_value = [QLOG_FILE]
 
-    yield
+  yield
 
 
-class TestLogReader(unittest.TestCase):
+class TestLogReader:
   @parameterized.expand([
     (f"{TEST_ROUTE}", ALL_SEGS),
     (f"{TEST_ROUTE.replace('/', '|')}", ALL_SEGS),
@@ -72,7 +69,7 @@ class TestLogReader(unittest.TestCase):
   def test_indirect_parsing(self, identifier, expected):
     parsed, _, _ = parse_indirect(identifier)
     sr = SegmentRange(parsed)
-    self.assertListEqual(list(sr.seg_idxs), expected, identifier)
+    assert list(sr.seg_idxs) == expected, identifier
 
   @parameterized.expand([
     (f"{TEST_ROUTE}", f"{TEST_ROUTE}"),
@@ -84,11 +81,11 @@ class TestLogReader(unittest.TestCase):
   ])
   def test_canonical_name(self, identifier, expected):
     sr = SegmentRange(identifier)
-    self.assertEqual(str(sr), expected)
+    assert str(sr) == expected
 
-  @parameterized.expand([(True,), (False,)])
-  @mock.patch("openpilot.tools.lib.logreader.file_exists")
-  def test_direct_parsing(self, cache_enabled, file_exists_mock):
+  @pytest.mark.parametrize("cache_enabled", [True, False])
+  def test_direct_parsing(self, mocker, cache_enabled):
+    file_exists_mock = mocker.patch("openpilot.tools.lib.logreader.file_exists")
     os.environ["FILEREADER_CACHE"] = "1" if cache_enabled else "0"
     qlog = tempfile.NamedTemporaryFile(mode='wb', delete=False)
 
@@ -98,13 +95,13 @@ class TestLogReader(unittest.TestCase):
 
     for f in [QLOG_FILE, qlog.name]:
       l = len(list(LogReader(f)))
-      self.assertGreater(l, 100)
+      assert l > 100
 
-    with self.assertRaises(URLFileException) if not cache_enabled else self.assertRaises(AssertionError):
+    with pytest.raises(URLFileException) if not cache_enabled else pytest.raises(AssertionError):
       l = len(list(LogReader(QLOG_FILE.replace("/3/", "/200/"))))
 
     # file_exists should not be called for direct files
-    self.assertEqual(file_exists_mock.call_count, 0)
+    assert file_exists_mock.call_count == 0
 
   @parameterized.expand([
     (f"{TEST_ROUTE}///",),
@@ -119,103 +116,136 @@ class TestLogReader(unittest.TestCase):
     (f"{TEST_ROUTE}--3a",),
   ])
   def test_bad_ranges(self, segment_range):
-    with self.assertRaises(AssertionError):
+    with pytest.raises(AssertionError):
       _ = SegmentRange(segment_range).seg_idxs
 
-  @parameterized.expand([
+  @pytest.mark.parametrize("segment_range, api_call", [
     (f"{TEST_ROUTE}/0", False),
     (f"{TEST_ROUTE}/:2", False),
     (f"{TEST_ROUTE}/0:", True),
     (f"{TEST_ROUTE}/-1", True),
     (f"{TEST_ROUTE}", True),
   ])
-  def test_slicing_api_call(self, segment_range, api_call):
-    with mock.patch("openpilot.tools.lib.route.get_max_seg_number_cached") as max_seg_mock:
-      max_seg_mock.return_value = NUM_SEGS
-      _ = SegmentRange(segment_range).seg_idxs
-      self.assertEqual(api_call, max_seg_mock.called)
+  def test_slicing_api_call(self, mocker, segment_range, api_call):
+    max_seg_mock = mocker.patch("openpilot.tools.lib.route.get_max_seg_number_cached")
+    max_seg_mock.return_value = NUM_SEGS
+    _ = SegmentRange(segment_range).seg_idxs
+    assert api_call == max_seg_mock.called
 
   @pytest.mark.slow
   def test_modes(self):
     qlog_len = len(list(LogReader(f"{TEST_ROUTE}/0", ReadMode.QLOG)))
     rlog_len = len(list(LogReader(f"{TEST_ROUTE}/0", ReadMode.RLOG)))
 
-    self.assertLess(qlog_len * 6, rlog_len)
+    assert qlog_len * 6 < rlog_len
 
   @pytest.mark.slow
   def test_modes_from_name(self):
     qlog_len = len(list(LogReader(f"{TEST_ROUTE}/0/q")))
     rlog_len = len(list(LogReader(f"{TEST_ROUTE}/0/r")))
 
-    self.assertLess(qlog_len * 6, rlog_len)
+    assert qlog_len * 6 < rlog_len
 
   @pytest.mark.slow
   def test_list(self):
     qlog_len = len(list(LogReader(f"{TEST_ROUTE}/0/q")))
     qlog_len_2 = len(list(LogReader([f"{TEST_ROUTE}/0/q", f"{TEST_ROUTE}/0/q"])))
 
-    self.assertEqual(qlog_len * 2, qlog_len_2)
+    assert qlog_len * 2 == qlog_len_2
 
   @pytest.mark.slow
-  @mock.patch("openpilot.tools.lib.logreader._LogFileReader")
-  def test_multiple_iterations(self, init_mock):
+  def test_multiple_iterations(self, mocker):
+    init_mock = mocker.patch("openpilot.tools.lib.logreader._LogFileReader")
     lr = LogReader(f"{TEST_ROUTE}/0/q")
     qlog_len1 = len(list(lr))
     qlog_len2 = len(list(lr))
 
     # ensure we don't create multiple instances of _LogFileReader, which means downloading the files twice
-    self.assertEqual(init_mock.call_count, 1)
+    assert init_mock.call_count == 1
 
-    self.assertEqual(qlog_len1, qlog_len2)
+    assert qlog_len1 == qlog_len2
 
   @pytest.mark.slow
   def test_helpers(self):
     lr = LogReader(f"{TEST_ROUTE}/0/q")
-    self.assertEqual(lr.first("carParams").carFingerprint, "SUBARU OUTBACK 6TH GEN")
-    self.assertTrue(0 < len(list(lr.filter("carParams"))) < len(list(lr)))
+    assert lr.first("carParams").carFingerprint == "SUBARU OUTBACK 6TH GEN"
+    assert 0 < len(list(lr.filter("carParams"))) < len(list(lr))
 
   @parameterized.expand([(True,), (False,)])
   @pytest.mark.slow
   def test_run_across_segments(self, cache_enabled):
     os.environ["FILEREADER_CACHE"] = "1" if cache_enabled else "0"
     lr = LogReader(f"{TEST_ROUTE}/0:4")
-    self.assertEqual(len(lr.run_across_segments(4, noop)), len(list(lr)))
+    assert len(lr.run_across_segments(4, noop)) == len(list(lr))
 
   @pytest.mark.slow
-  def test_auto_mode(self):
+  def test_auto_mode(self, subtests, mocker):
     lr = LogReader(f"{TEST_ROUTE}/0/q")
     qlog_len = len(list(lr))
-    with mock.patch("openpilot.tools.lib.route.Route.log_paths") as log_paths_mock:
-      log_paths_mock.return_value = [None] * NUM_SEGS
-      # Should fall back to qlogs since rlogs are not available
+    log_paths_mock = mocker.patch("openpilot.tools.lib.route.Route.log_paths")
+    log_paths_mock.return_value = [None] * NUM_SEGS
+    # Should fall back to qlogs since rlogs are not available
 
-      with self.subTest("interactive_yes"):
-        with mock.patch("sys.stdin", new=io.StringIO("y\n")):
-          lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO_INTERACTIVE, default_source=comma_api_source)
-          log_len = len(list(lr))
-        self.assertEqual(qlog_len, log_len)
+    with subtests.test("interactive_yes"):
+      mocker.patch("sys.stdin", new=io.StringIO("y\n"))
+      lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO_INTERACTIVE, default_source=comma_api_source)
+      log_len = len(list(lr))
+      assert qlog_len == log_len
 
-      with self.subTest("interactive_no"):
-        with mock.patch("sys.stdin", new=io.StringIO("n\n")):
-          with self.assertRaises(AssertionError):
-            lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO_INTERACTIVE, default_source=comma_api_source)
+    with subtests.test("interactive_no"):
+      mocker.patch("sys.stdin", new=io.StringIO("n\n"))
+      with pytest.raises(AssertionError):
+        lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO_INTERACTIVE, default_source=comma_api_source)
 
-      with self.subTest("non_interactive"):
-        lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO, default_source=comma_api_source)
-        log_len = len(list(lr))
-        self.assertEqual(qlog_len, log_len)
+    with subtests.test("non_interactive"):
+      lr = LogReader(f"{TEST_ROUTE}/0", default_mode=ReadMode.AUTO, default_source=comma_api_source)
+      log_len = len(list(lr))
+      assert qlog_len == log_len
 
-  @parameterized.expand([(True,), (False,)])
+  @pytest.mark.parametrize("is_internal", [True, False])
   @pytest.mark.slow
-  def test_auto_source_scenarios(self, is_internal):
+  def test_auto_source_scenarios(self, mocker, is_internal):
     lr = LogReader(QLOG_FILE)
     qlog_len = len(list(lr))
 
-    with setup_source_scenario(is_internal=is_internal):
+    with setup_source_scenario(mocker, is_internal=is_internal):
       lr = LogReader(f"{TEST_ROUTE}/0/q")
       log_len = len(list(lr))
-      self.assertEqual(qlog_len, log_len)
+      assert qlog_len == log_len
 
+  @pytest.mark.slow
+  def test_sort_by_time(self):
+    msgs = list(LogReader(f"{TEST_ROUTE}/0/q"))
+    assert msgs != sorted(msgs, key=lambda m: m.logMonoTime)
 
-if __name__ == "__main__":
-  unittest.main()
+    msgs = list(LogReader(f"{TEST_ROUTE}/0/q", sort_by_time=True))
+    assert msgs == sorted(msgs, key=lambda m: m.logMonoTime)
+
+  def test_only_union_types(self):
+    with tempfile.NamedTemporaryFile() as qlog:
+      # write valid Event messages
+      num_msgs = 100
+      with open(qlog.name, "wb") as f:
+        f.write(b"".join(capnp_log.Event.new_message().to_bytes() for _ in range(num_msgs)))
+
+      msgs = list(LogReader(qlog.name))
+      assert len(msgs) == num_msgs
+      [m.which() for m in msgs]
+
+      # append non-union Event message
+      event_msg = capnp_log.Event.new_message()
+      non_union_bytes = bytearray(event_msg.to_bytes())
+      non_union_bytes[event_msg.total_size.word_count * 8] = 0xff  # set discriminant value out of range using Event word offset
+      with open(qlog.name, "ab") as f:
+        f.write(non_union_bytes)
+
+      # ensure new message is added, but is not a union type
+      msgs = list(LogReader(qlog.name))
+      assert len(msgs) == num_msgs + 1
+      with pytest.raises(capnp.KjException):
+        [m.which() for m in msgs]
+
+      # should not be added when only_union_types=True
+      msgs = list(LogReader(qlog.name, only_union_types=True))
+      assert len(msgs) == num_msgs
+      [m.which() for m in msgs]

tools/lib/tests/test_readers.py

@@ -1,5 +1,4 @@
-#!/usr/bin/env python
-import unittest
+import pytest
 import requests
 import tempfile
 
@@ -9,16 +8,16 @@ from openpilot.tools.lib.framereader import FrameReader
 from openpilot.tools.lib.logreader import LogReader
 
 
-class TestReaders(unittest.TestCase):
-  @unittest.skip("skip for bandwidth reasons")
+class TestReaders:
+  @pytest.mark.skip("skip for bandwidth reasons")
   def test_logreader(self):
     def _check_data(lr):
       hist = defaultdict(int)
       for l in lr:
         hist[l.which()] += 1
 
-      self.assertEqual(hist['carControl'], 6000)
-      self.assertEqual(hist['logMessage'], 6857)
+      assert hist['carControl'] == 6000
+      assert hist['logMessage'] == 6857
 
     with tempfile.NamedTemporaryFile(suffix=".bz2") as fp:
       r = requests.get("https://github.com/commaai/comma2k19/blob/master/Example_1/b0c9d2329ad1606b%7C2018-08-02--08-34-47/40/raw_log.bz2?raw=true", timeout=10)
@@ -31,15 +30,15 @@ class TestReaders(unittest.TestCase):
     lr_url = LogReader("https://github.com/commaai/comma2k19/blob/master/Example_1/b0c9d2329ad1606b%7C2018-08-02--08-34-47/40/raw_log.bz2?raw=true")
     _check_data(lr_url)
 
-  @unittest.skip("skip for bandwidth reasons")
+  @pytest.mark.skip("skip for bandwidth reasons")
   def test_framereader(self):
     def _check_data(f):
-      self.assertEqual(f.frame_count, 1200)
-      self.assertEqual(f.w, 1164)
-      self.assertEqual(f.h, 874)
+      assert f.frame_count == 1200
+      assert f.w == 1164
+      assert f.h == 874
 
       frame_first_30 = f.get(0, 30)
-      self.assertEqual(len(frame_first_30), 30)
+      assert len(frame_first_30) == 30
 
       print(frame_first_30[15])
 
@@ -62,6 +61,3 @@ class TestReaders(unittest.TestCase):
 
     fr_url = FrameReader("https://github.com/commaai/comma2k19/blob/master/Example_1/b0c9d2329ad1606b%7C2018-08-02--08-34-47/40/video.hevc?raw=true")
     _check_data(fr_url)
-
-if __name__ == "__main__":
-  unittest.main()

tools/lib/tests/test_route_library.py

@@ -1,10 +1,8 @@
-#!/usr/bin/env python
-import unittest
 from collections import namedtuple
 
 from openpilot.tools.lib.route import SegmentName
 
-class TestRouteLibrary(unittest.TestCase):
+class TestRouteLibrary:
   def test_segment_name_formats(self):
     Case = namedtuple('Case', ['input', 'expected_route', 'expected_segment_num', 'expected_data_dir'])
 
@@ -21,12 +19,9 @@ class TestRouteLibrary(unittest.TestCase):
 
       s = SegmentName(route_or_segment_name, allow_route_name=True)
 
-      self.assertEqual(str(s.route_name), case.expected_route)
-      self.assertEqual(s.segment_num, case.expected_segment_num)
-      self.assertEqual(s.data_dir, case.expected_data_dir)
+      assert str(s.route_name) == case.expected_route
+      assert s.segment_num == case.expected_segment_num
+      assert s.data_dir == case.expected_data_dir
 
     for case in cases:
       _validate(case)
-
-if __name__ == "__main__":
-  unittest.main()

tools/lib/vidindex.py

@@ -3,7 +3,6 @@ import argparse
 import os
 import struct
 from enum import IntEnum
-from typing import Tuple
 
 from openpilot.tools.lib.filereader import FileReader
 
@@ -120,7 +119,7 @@ HEVC_CODED_SLICE_SEGMENT_NAL_UNITS = (
 class VideoFileInvalid(Exception):
   pass
 
-def get_ue(dat: bytes, start_idx: int, skip_bits: int) -> Tuple[int, int]:
+def get_ue(dat: bytes, start_idx: int, skip_bits: int) -> tuple[int, int]:
   prefix_val = 0
   prefix_len = 0
   suffix_val = 0
@@ -184,7 +183,7 @@ def get_hevc_nal_unit_type(dat: bytes, nal_unit_start: int) -> HevcNalUnitType:
     print("  nal_unit_type:", nal_unit_type.name, f"({nal_unit_type.value})")
   return nal_unit_type
 
-def get_hevc_slice_type(dat: bytes, nal_unit_start: int, nal_unit_type: HevcNalUnitType) -> Tuple[int, bool]:
+def get_hevc_slice_type(dat: bytes, nal_unit_start: int, nal_unit_type: HevcNalUnitType) -> tuple[int, bool]:
   # 7.3.2.9 Slice segment layer RBSP syntax
   # slice_segment_layer_rbsp( ) {
   #   slice_segment_header( )
@@ -259,7 +258,7 @@ def get_hevc_slice_type(dat: bytes, nal_unit_start: int, nal_unit_type: HevcNalU
     raise VideoFileInvalid("slice_type must be 0, 1, or 2")
   return slice_type, is_first_slice
 
-def hevc_index(hevc_file_name: str, allow_corrupt: bool=False) -> Tuple[list, int, bytes]:
+def hevc_index(hevc_file_name: str, allow_corrupt: bool=False) -> tuple[list, int, bytes]:
   with FileReader(hevc_file_name) as f:
     dat = f.read()