params: auto type cast on put (#35810)

* start

* fix

* fix

* more

* more

* more

* fix

* fix

* []

* f

* f

* fix

* lint

* back

* fix

* yep

* better msg

* fix

* fix

* fix

* fix

* more

* more

common/params_keys.h

@@ -75,7 +75,7 @@ inline static std::unordered_map<std::string, ParamKeyAttributes> keys = {
     {"LastUpdateException", {CLEAR_ON_MANAGER_START, STRING}},
     {"LastUpdateTime", {PERSISTENT, TIME}},
     {"LiveDelay", {PERSISTENT, BYTES}},
-    {"LiveParameters", {PERSISTENT, BYTES}},
+    {"LiveParameters", {PERSISTENT, JSON}},
     {"LiveParametersV2", {PERSISTENT, BYTES}},
     {"LiveTorqueParameters", {PERSISTENT | DONT_LOG, BYTES}},
     {"LocationFilterInitialState", {PERSISTENT, BYTES}},
@@ -116,10 +116,10 @@ inline static std::unordered_map<std::string, ParamKeyAttributes> keys = {
     {"UpdateFailedCount", {CLEAR_ON_MANAGER_START, INT}},
     {"UpdaterAvailableBranches", {PERSISTENT, STRING}},
     {"UpdaterCurrentDescription", {CLEAR_ON_MANAGER_START, STRING}},
-    {"UpdaterCurrentReleaseNotes", {CLEAR_ON_MANAGER_START, STRING}},
+    {"UpdaterCurrentReleaseNotes", {CLEAR_ON_MANAGER_START, BYTES}},
     {"UpdaterFetchAvailable", {CLEAR_ON_MANAGER_START, BOOL}},
     {"UpdaterNewDescription", {CLEAR_ON_MANAGER_START, STRING}},
-    {"UpdaterNewReleaseNotes", {CLEAR_ON_MANAGER_START, STRING}},
+    {"UpdaterNewReleaseNotes", {CLEAR_ON_MANAGER_START, BYTES}},
     {"UpdaterState", {CLEAR_ON_MANAGER_START, STRING}},
     {"UpdaterTargetBranch", {CLEAR_ON_MANAGER_START, STRING}},
     {"UpdaterLastFetchTime", {PERSISTENT, TIME}},

common/params_pyx.pyx

@@ -1,5 +1,6 @@
 # distutils: language = c++
 # cython: language_level = 3
+import builtins
 import datetime
 import json
 from libcpp cimport bool
@@ -7,6 +8,8 @@ from libcpp.string cimport string
 from libcpp.vector cimport vector
 from libcpp.optional cimport optional
 
+from openpilot.common.swaglog import cloudlog
+
 cdef extern from "common/params.h":
   cpdef enum ParamKeyFlag:
     PERSISTENT
@@ -42,6 +45,25 @@ cdef extern from "common/params.h":
     void clearAll(ParamKeyFlag)
     vector[string] allKeys()
 
+PYTHON_2_CPP = {
+  (str, STRING): lambda v: v,
+  (builtins.bool, BOOL): lambda v: "1" if v else "0",
+  (int, INT): str,
+  (float, FLOAT): str,
+  (datetime.datetime, TIME): lambda v: v.isoformat(),
+  (dict, JSON): json.dumps,
+  (list, JSON): json.dumps,
+  (bytes, BYTES): lambda v: v,
+}
+CPP_2_PYTHON = {
+  STRING: lambda v: v.decode("utf-8"),
+  BOOL: lambda v: v == b"1",
+  INT: int,
+  FLOAT: float,
+  TIME: lambda v: datetime.datetime.fromisoformat(v.decode("utf-8")),
+  JSON: json.loads,
+  BYTES: lambda v: v,
+}
 
 def ensure_bytes(v):
   return v.encode() if isinstance(v, str) else v
@@ -74,45 +96,38 @@ cdef class Params:
       raise UnknownKeyName(key)
     return key
 
-  def cast(self, t, value, default):
+  def python2cpp(self, proposed_type, expected_type, value, key):
+    cast = PYTHON_2_CPP.get((proposed_type, expected_type))
+    if cast:
+      return cast(value)
+    raise TypeError(f"Type mismatch while writing param {key}: {proposed_type=} {expected_type=} {value=}")
+
+  def cpp2python(self, t, value, default, key):
     if value is None:
       return None
     try:
-      if t == STRING:
+      return CPP_2_PYTHON[t](value)
-        return value.decode("utf-8")
+    except (KeyError, TypeError, ValueError):
-      elif t == BOOL:
+      cloudlog.warning(f"Failed to cast param {key} with {value=} from type {t=}")
-        return value == b"1"
+      return self.cpp2python(t, default, None, key)
-      elif t == INT:
-        return int(value)
-      elif t == FLOAT:
-        return float(value)
-      elif t == TIME:
-        return datetime.datetime.fromisoformat(value.decode("utf-8"))
-      elif t == JSON:
-        return json.loads(value)
-      elif t == BYTES:
-        return value
-      else:
-        raise TypeError()
-    except (TypeError, ValueError):
-      return self.cast(t, default, None)
 
   def get(self, key, bool block=False, bool return_default=False):
     cdef string k = self.check_key(key)
     cdef ParamKeyType t = self.p.getKeyType(k)
+    cdef optional[string] default = self.p.getKeyDefaultValue(k)
     cdef string val
     with nogil:
       val = self.p.get(k, block)
 
-    default_val = self.get_default_value(k) if return_default else None
+    default_val = (default.value() if default.has_value() else None) if return_default else None
     if val == b"":
       if block:
         # If we got no value while running in blocked mode
         # it means we got an interrupt while waiting
         raise KeyboardInterrupt
       else:
-        return self.cast(t, default_val, None)
+        return self.cpp2python(t, default_val, None, key)
-    return self.cast(t, val, default_val)
+    return self.cpp2python(t, val, default_val, key)
 
   def get_bool(self, key, bool block=False):
     cdef string k = self.check_key(key)
@@ -121,6 +136,11 @@ cdef class Params:
       r = self.p.getBool(k, block)
     return r
 
+  def _put_cast(self, key, dat):
+    cdef string k = self.check_key(key)
+    cdef ParamKeyType t = self.p.getKeyType(k)
+    return ensure_bytes(self.python2cpp(type(dat), t, dat, key))
+
   def put(self, key, dat):
     """
     Warning: This function blocks until the param is written to disk!
@@ -129,7 +149,7 @@ cdef class Params:
     in general try to avoid writing params as much as possible.
     """
     cdef string k = self.check_key(key)
-    cdef string dat_bytes = ensure_bytes(dat)
+    cdef string dat_bytes = self._put_cast(key, dat)
     with nogil:
       self.p.put(k, dat_bytes)
 
@@ -140,7 +160,7 @@ cdef class Params:
 
   def put_nonblocking(self, key, dat):
     cdef string k = self.check_key(key)
-    cdef string dat_bytes = ensure_bytes(dat)
+    cdef string dat_bytes = self._put_cast(key, dat)
     with nogil:
       self.p.putNonBlocking(k, dat_bytes)
 
@@ -165,5 +185,7 @@ cdef class Params:
     return self.p.allKeys()
 
   def get_default_value(self, key):
-    cdef optional[string] default = self.p.getKeyDefaultValue(self.check_key(key))
+    cdef string k = self.check_key(key)
-    return default.value() if default.has_value() else None
+    cdef ParamKeyType t = self.p.getKeyType(k)
+    cdef optional[string] default = self.p.getKeyDefaultValue(k)
+    return self.cpp2python(t, default.value(), None, key) if default.has_value() else None

common/tests/test_params.py

@@ -1,6 +1,5 @@
 import pytest
 import datetime
-import json
 import os
 import threading
 import time
@@ -22,7 +21,7 @@ class TestParams:
     assert self.params.get("CarParams") == st
 
   def test_params_get_cleared_manager_start(self):
-    self.params.put("CarParams", "test")
+    self.params.put("CarParams", b"test")
     self.params.put("DongleId", "cb38263377b873ee")
     assert self.params.get("CarParams") == b"test"
 
@@ -45,10 +44,10 @@ class TestParams:
   def test_params_get_block(self):
     def _delayed_writer():
       time.sleep(0.1)
-      self.params.put("CarParams", "test")
+      self.params.put("CarParams", b"test")
     threading.Thread(target=_delayed_writer).start()
     assert self.params.get("CarParams") is None
-    assert self.params.get("CarParams", True) == b"test"
+    assert self.params.get("CarParams", block=True) == b"test"
 
   def test_params_unknown_key_fails(self):
     with pytest.raises(UnknownKeyName):
@@ -78,17 +77,17 @@ class TestParams:
     self.params.put_bool("IsMetric", False)
     assert not self.params.get_bool("IsMetric")
 
-    self.params.put("IsMetric", "1")
+    self.params.put("IsMetric", True)
     assert self.params.get_bool("IsMetric")
 
-    self.params.put("IsMetric", "0")
+    self.params.put("IsMetric", False)
     assert not self.params.get_bool("IsMetric")
 
   def test_put_non_blocking_with_get_block(self):
     q = Params()
     def _delayed_writer():
       time.sleep(0.1)
-      Params().put_nonblocking("CarParams", "test")
+      Params().put_nonblocking("CarParams", b"test")
     threading.Thread(target=_delayed_writer).start()
     assert q.get("CarParams") is None
     assert q.get("CarParams", True) == b"test"
@@ -124,19 +123,19 @@ class TestParams:
 
   def test_params_get_type(self):
     # json
-    self.params.put("ApiCache_FirehoseStats", json.dumps({"a": 0}))
+    self.params.put("ApiCache_FirehoseStats", {"a": 0})
     assert self.params.get("ApiCache_FirehoseStats") == {"a": 0}
 
     # int
-    self.params.put("BootCount", str(1441))
+    self.params.put("BootCount", 1441)
     assert self.params.get("BootCount") == 1441
 
     # bool
-    self.params.put("AdbEnabled", "1")
+    self.params.put("AdbEnabled", True)
     assert self.params.get("AdbEnabled")
     assert isinstance(self.params.get("AdbEnabled"), bool)
 
     # time
     now = datetime.datetime.now(datetime.UTC)
-    self.params.put("InstallDate", str(now))
+    self.params.put("InstallDate", now)
     assert self.params.get("InstallDate") == now

selfdrive/locationd/paramsd.py

@@ -1,6 +1,5 @@
 #!/usr/bin/env python3
 import os
-import json
 import numpy as np
 import capnp
 
@@ -207,12 +206,11 @@ def migrate_cached_vehicle_params_if_needed(params: Params):
     return
 
   try:
-    last_parameters_dict = json.loads(last_parameters_data_old)
     last_parameters_msg = messaging.new_message('liveParameters')
     last_parameters_msg.liveParameters.valid = True
-    last_parameters_msg.liveParameters.steerRatio = last_parameters_dict['steerRatio']
+    last_parameters_msg.liveParameters.steerRatio = last_parameters_data_old['steerRatio']
-    last_parameters_msg.liveParameters.stiffnessFactor = last_parameters_dict['stiffnessFactor']
+    last_parameters_msg.liveParameters.stiffnessFactor = last_parameters_data_old['stiffnessFactor']
-    last_parameters_msg.liveParameters.angleOffsetAverageDeg = last_parameters_dict['angleOffsetAverageDeg']
+    last_parameters_msg.liveParameters.angleOffsetAverageDeg = last_parameters_data_old['angleOffsetAverageDeg']
     params.put("LiveParametersV2", last_parameters_msg.to_bytes())
   except Exception as e:
     cloudlog.error(f"Failed to perform parameter migration: {e}")

selfdrive/locationd/test/test_paramsd.py

@@ -1,6 +1,5 @@
 import random
 import numpy as np
-import json
 
 from cereal import messaging
 from openpilot.selfdrive.locationd.paramsd import retrieve_initial_vehicle_params, migrate_cached_vehicle_params_if_needed
@@ -47,7 +46,7 @@ class TestParamsd:
     CP = next(m for m in lr if m.which() == "carParams").carParams
 
     msg = get_random_live_parameters(CP)
-    params.put("LiveParameters", json.dumps(msg.liveParameters.to_dict()))
+    params.put("LiveParameters", msg.liveParameters.to_dict())
     params.put("CarParamsPrevRoute", CP.as_builder().to_bytes())
     params.remove("LiveParametersV2")
 
@@ -60,7 +59,7 @@ class TestParamsd:
 
   def test_read_saved_params_corrupted_old_format(self):
     params = Params()
-    params.put("LiveParameters", b'\x00\x00\x02\x00\x01\x00:F\xde\xed\xae;')
+    params.put("LiveParameters", {})
     params.remove("LiveParametersV2")
 
     migrate_cached_vehicle_params_if_needed(params)

selfdrive/selfdrived/alertmanager.py

@@ -17,7 +17,7 @@ def set_offroad_alert(alert: str, show_alert: bool, extra_text: str = None) -> N
   if show_alert:
     a = copy.copy(OFFROAD_ALERTS[alert])
     a['extra'] = extra_text or ''
-    Params().put(alert, json.dumps(a))
+    Params().put(alert, a)
   else:
     Params().remove(alert)
 

selfdrive/selfdrived/selfdrived.py

@@ -407,7 +407,7 @@ class SelfdriveD:
     if self.CP.openpilotLongitudinalControl:
       if any(not be.pressed and be.type == ButtonType.gapAdjustCruise for be in CS.buttonEvents):
         self.personality = (self.personality - 1) % 3
-        self.params.put_nonblocking('LongitudinalPersonality', str(self.personality))
+        self.params.put_nonblocking('LongitudinalPersonality', self.personality)
         self.events.add(EventName.personalityChanged)
 
   def data_sample(self):

selfdrive/ui/layouts/settings/firehose.py

@@ -1,5 +1,4 @@
 import pyray as rl
-import json
 import time
 import threading
 
@@ -169,7 +168,7 @@ class FirehoseLayout(Widget):
       if response.status_code == 200:
         data = response.json()
         self.segment_count = data.get("firehose", 0)
-        self.params.put(self.PARAM_KEY, json.dumps(data))
+        self.params.put(self.PARAM_KEY, data)
     except Exception as e:
       cloudlog.error(f"Failed to fetch firehose stats: {e}")
 

selfdrive/ui/layouts/settings/toggles.py

@@ -92,4 +92,4 @@ class TogglesLayout(Widget):
     self._scroller.render(rect)
 
   def _set_longitudinal_personality(self, button_index: int):
-    self._params.put("LongitudinalPersonality", str(button_index))
+    self._params.put("LongitudinalPersonality", button_index)

selfdrive/ui/lib/prime_state.py

@@ -63,7 +63,7 @@ class PrimeState:
     with self._lock:
       if prime_type != self.prime_type:
         self.prime_type = prime_type
-        self._params.put("PrimeType", str(int(prime_type)))
+        self._params.put("PrimeType", int(prime_type))
         cloudlog.info(f"Prime type updated to {prime_type}")
 
   def _worker_thread(self) -> None:

selfdrive/ui/tests/test_ui/run.py

@@ -299,9 +299,9 @@ def create_screenshots():
       params = Params()
       params.put("DongleId", "123456789012345")
       if name == 'prime':
-        params.put('PrimeType', '1')
+        params.put('PrimeType', 1)
       elif name == 'pair_device':
-        params.put('ApiCache_Device', '{"is_paired":0, "prime_type":-1}')
+        params.put('ApiCache_Device', {"is_paired":0, "prime_type":-1})
 
       t.test_ui(name, setup)
 

system/athena/athenad.py

@@ -161,7 +161,7 @@ class UploadQueueCache:
     try:
       queue: list[UploadItem | None] = list(upload_queue.queue)
       items = [asdict(i) for i in queue if i is not None and (i.id not in cancelled_uploads)]
-      Params().put("AthenadUploadQueue", json.dumps(items))
+      Params().put("AthenadUploadQueue", items)
     except Exception:
       cloudlog.exception("athena.UploadQueueCache.cache.exception")
 
@@ -748,7 +748,7 @@ def ws_recv(ws: WebSocket, end_event: threading.Event) -> None:
         recv_queue.put_nowait(data)
       elif opcode == ABNF.OPCODE_PING:
         last_ping = int(time.monotonic() * 1e9)
-        Params().put("LastAthenaPingTime", str(last_ping))
+        Params().put("LastAthenaPingTime", last_ping)
     except WebSocketTimeoutException:
       ns_since_last_ping = int(time.monotonic() * 1e9) - last_ping
       if ns_since_last_ping > RECONNECT_TIMEOUT_S * 1e9:

system/athena/tests/test_athenad.py

@@ -66,9 +66,9 @@ class TestAthenadMethods:
   def setup_method(self):
     self.default_params = {
       "DongleId": "0000000000000000",
-      "GithubSshKeys": b"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC307aE+nuHzTAgaJhzSf5v7ZZQW9gaperjhCmyPyl4PzY7T1mDGenTlVTN7yoVFZ9UfO9oMQqo0n1OwDIiqbIFxqnhrHU0cYfj88rI85m5BEKlNu5RdaVTj1tcbaPpQc5kZEolaI1nDDjzV0lwS7jo5VYDHseiJHlik3HH1SgtdtsuamGR2T80q1SyW+5rHoMOJG73IH2553NnWuikKiuikGHUYBd00K1ilVAK2xSiMWJp55tQfZ0ecr9QjEsJ+J/efL4HqGNXhffxvypCXvbUYAFSddOwXUPo5BTKevpxMtH+2YrkpSjocWA04VnTYFiPG6U4ItKmbLOTFZtPzoez private", # noqa: E501
+      "GithubSshKeys": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC307aE+nuHzTAgaJhzSf5v7ZZQW9gaperjhCmyPyl4PzY7T1mDGenTlVTN7yoVFZ9UfO9oMQqo0n1OwDIiqbIFxqnhrHU0cYfj88rI85m5BEKlNu5RdaVTj1tcbaPpQc5kZEolaI1nDDjzV0lwS7jo5VYDHseiJHlik3HH1SgtdtsuamGR2T80q1SyW+5rHoMOJG73IH2553NnWuikKiuikGHUYBd00K1ilVAK2xSiMWJp55tQfZ0ecr9QjEsJ+J/efL4HqGNXhffxvypCXvbUYAFSddOwXUPo5BTKevpxMtH+2YrkpSjocWA04VnTYFiPG6U4ItKmbLOTFZtPzoez private", # noqa: E501
-      "GithubUsername": b"commaci",
+      "GithubUsername": "commaci",
-      "AthenadUploadQueue": '[]',
+      "AthenadUploadQueue": [],
     }
 
     self.params = Params()
@@ -400,11 +400,11 @@ class TestAthenadMethods:
 
   def test_get_ssh_authorized_keys(self):
     keys = dispatcher["getSshAuthorizedKeys"]()
-    assert keys == self.default_params["GithubSshKeys"].decode('utf-8')
+    assert keys == self.default_params["GithubSshKeys"]
 
   def test_get_github_username(self):
     keys = dispatcher["getGithubUsername"]()
-    assert keys == self.default_params["GithubUsername"].decode('utf-8')
+    assert keys == self.default_params["GithubUsername"]
 
   def test_get_version(self):
     resp = dispatcher["getVersion"]()

system/hardware/hardwared.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 import fcntl
 import os
-import json
 import queue
 import struct
 import threading
@@ -440,7 +439,7 @@ def hardware_thread(end_event, hw_queue) -> None:
       # save last one before going onroad
       if rising_edge_started:
         try:
-          params.put("LastOffroadStatusPacket", json.dumps(dat))
+          params.put("LastOffroadStatusPacket", dat)
         except Exception:
           cloudlog.exception("failed to save offroad status")
 

system/hardware/power_monitoring.py

@@ -56,7 +56,7 @@ class PowerMonitoring:
       self.car_battery_capacity_uWh = max(self.car_battery_capacity_uWh, 0)
       self.car_battery_capacity_uWh = min(self.car_battery_capacity_uWh, CAR_BATTERY_CAPACITY_uWh)
       if now - self.last_save_time >= 10:
-        self.params.put_nonblocking("CarBatteryCapacity", str(int(self.car_battery_capacity_uWh)))
+        self.params.put_nonblocking("CarBatteryCapacity", int(self.car_battery_capacity_uWh))
         self.last_save_time = now
 
       # First measurement, set integration time

system/loggerd/tests/loggerd_tests_common.py

@@ -76,7 +76,7 @@ class UploaderTestCase:
     self.seg_dir = self.seg_format.format(self.seg_num)
 
     self.params = Params()
-    self.params.put("IsOffroad", "1")
+    self.params.put("IsOffroad", True)
     self.params.put("DongleId", "0000000000000000")
 
   def make_file_with_data(self, f_dir: str, fn: str, size_mb: float = .1, lock: bool = False,

system/loggerd/tests/test_loggerd.py

@@ -182,7 +182,7 @@ class TestLoggerd:
 
   @pytest.mark.xdist_group("camera_encoder_tests")  # setting xdist group ensures tests are run in same worker, prevents encoderd from crashing
   def test_rotation(self):
-    Params().put("RecordFront", "1")
+    Params().put("RecordFront", True)
 
     expected_files = {"rlog.zst", "qlog.zst", "qcamera.ts", "fcamera.hevc", "dcamera.hevc", "ecamera.hevc"}
 

system/manager/test/test_manager.py

@@ -47,7 +47,7 @@ class TestManager:
     for k in params.all_keys():
       default_value = params.get_default_value(k)
       if default_value:
-        assert params.get(k) == params.cast(params.get_type(k), default_value, None)
+        assert params.get(k) == default_value
     assert params.get("OpenpilotEnabledToggle")
 
   @pytest.mark.skip("this test is flaky the way it's currently written, should be moved to test_onroad")

system/updated/updated.py

@@ -61,7 +61,7 @@ class WaitTimeHelper:
 
 def write_time_to_param(params, param) -> None:
   t = datetime.datetime.now(datetime.UTC).replace(tzinfo=None)
-  params.put(param, t.isoformat().encode('utf8'))
+  params.put(param, t)
 
 def run(cmd: list[str], cwd: str = None) -> str:
   return subprocess.check_output(cmd, cwd=cwd, stderr=subprocess.STDOUT, encoding='utf8')
@@ -264,7 +264,7 @@ class Updater:
     return run(["git", "rev-parse", "HEAD"], path).rstrip()
 
   def set_params(self, update_success: bool, failed_count: int, exception: str | None) -> None:
-    self.params.put("UpdateFailedCount", str(failed_count))
+    self.params.put("UpdateFailedCount", failed_count)
     self.params.put("UpdaterTargetBranch", self.target_branch)
 
     self.params.put_bool("UpdaterFetchAvailable", self.update_available)
@@ -421,8 +421,8 @@ def main() -> None:
       cloudlog.event("update installed")
 
     if not params.get("InstallDate"):
-      t = datetime.datetime.now(datetime.UTC).replace(tzinfo=None).isoformat()
+      t = datetime.datetime.now(datetime.UTC).replace(tzinfo=None)
-      params.put("InstallDate", t.encode('utf8'))
+      params.put("InstallDate", t)
 
     updater = Updater()
     update_failed_count = 0 # TODO: Load from param?