try ctypes params (#38411)

* try cffi params

* ctypes

* lil more

* rm cython

* lil more

* that was fine

* lil more

* Drop unrelated Params concurrency changes

* Clean up ctypes Params build integration

* just c

* Clarify Params exception translation

* Expand Params C wrappers

* lil more

openpilot/common/SConscript

@@ -1,4 +1,4 @@
-Import('env', 'envCython')
+Import('env')
 
 common_libs = [
   'params.cc',
@@ -11,12 +11,9 @@ common_libs = [
 _common = env.Library('common', common_libs, LIBS="json11")
 Export('_common')
 
+params_python = env.SharedLibrary('params_c', 'params_c.cc', LIBS=[_common, 'zmq', 'json11'])
+common_python = [params_python]
+Export('common_python')
+
 if GetOption('extras'):
   env.Program('tests/test_swaglog', 'tests/test_swaglog.cc', LIBS=[_common, 'json11', 'zmq', 'pthread'])
-
-# Cython bindings
-params_python = envCython.Program('params_pyx.so', 'params_pyx.pyx', LIBS=envCython['LIBS'] + [_common, 'zmq', 'json11'])
-
-common_python = [params_python]
-
-Export('common_python')

openpilot/common/params.py

@@ -1,16 +1,210 @@
-from openpilot.common.params_pyx import Params, ParamKeyFlag, ParamKeyType, UnknownKeyName
-assert Params
-assert ParamKeyFlag
-assert ParamKeyType
-assert UnknownKeyName
+import sys
+import json
+import ctypes
+import weakref
+import builtins
+import datetime
+from pathlib import Path
+from enum import IntEnum, IntFlag
+
+from openpilot.common.swaglog import cloudlog
+
+
+class ParamKeyFlag(IntFlag):
+  PERSISTENT = 0x02
+  CLEAR_ON_MANAGER_START = 0x04
+  CLEAR_ON_ONROAD_TRANSITION = 0x08
+  CLEAR_ON_OFFROAD_TRANSITION = 0x10
+  DEVELOPMENT_ONLY = 0x40
+  CLEAR_ON_IGNITION_ON = 0x80
+  ALL = 0xFFFFFFFF
+
+
+class ParamKeyType(IntEnum):
+  STRING = 0
+  BOOL = 1
+  INT = 2
+  FLOAT = 3
+  TIME = 4
+  JSON = 5
+  BYTES = 6
+
+
+_suffix = ".dylib" if sys.platform == "darwin" else ".so"
+lib = ctypes.CDLL(Path(__file__).with_name(f"libparams_c{_suffix}"))
+
+ParamsHandle = ctypes.c_void_p
+
+
+class ParamsBuffer(ctypes.Structure):
+  _fields_ = [("data", ctypes.c_void_p), ("size", ctypes.c_size_t)]
+
+
+def _bind_raw(name, args, result=None):
+  function = getattr(lib, name)
+  function.argtypes = args
+  function.restype = result
+  return function
+
+
+params_last_error = _bind_raw("params_last_error", [], ctypes.c_char_p)
+
+
+def _bind(name, args, result=None):
+  function = _bind_raw(name, args, result)
+
+  def checked(*call_args):
+    value = function(*call_args)
+    if error := params_last_error():
+      raise RuntimeError(error.decode())
+    return value
+
+  return checked
+
+
+params_create = _bind("params_create", [ctypes.c_char_p, ctypes.c_size_t], ParamsHandle)
+params_destroy = _bind("params_destroy", [ParamsHandle])
+params_clear_all = _bind("params_clear_all", [ParamsHandle, ctypes.c_uint])
+params_check_key = _bind("params_check_key", [ParamsHandle, ctypes.c_char_p], ctypes.c_bool)
+params_get_key_type = _bind("params_get_key_type", [ParamsHandle, ctypes.c_char_p], ctypes.c_int)
+params_get_default = _bind("params_get_default", [ParamsHandle, ctypes.c_char_p], ParamsBuffer)
+params_get = _bind("params_get", [ParamsHandle, ctypes.c_char_p, ctypes.c_bool], ParamsBuffer)
+params_get_bool = _bind("params_get_bool", [ParamsHandle, ctypes.c_char_p, ctypes.c_bool], ctypes.c_bool)
+params_put = _bind("params_put", [ParamsHandle, ctypes.c_char_p, ctypes.c_char_p, ctypes.c_size_t, ctypes.c_bool], ctypes.c_int)
+params_put_bool = _bind("params_put_bool", [ParamsHandle, ctypes.c_char_p, ctypes.c_bool, ctypes.c_bool], ctypes.c_int)
+params_remove = _bind("params_remove", [ParamsHandle, ctypes.c_char_p], ctypes.c_int)
+params_get_path = _bind("params_get_path", [ParamsHandle, ctypes.c_char_p, ctypes.c_size_t], ParamsBuffer)
+params_keys_size = _bind("params_keys_size", [ParamsHandle], ctypes.c_size_t)
+params_key_at = _bind("params_key_at", [ParamsHandle, ctypes.c_size_t], ParamsBuffer)
+
+PYTHON_2_CPP = {
+  (str, ParamKeyType.STRING): lambda v: v,
+  (builtins.bool, ParamKeyType.BOOL): lambda v: "1" if v else "0",
+  (int, ParamKeyType.INT): str,
+  (float, ParamKeyType.FLOAT): str,
+  (datetime.datetime, ParamKeyType.TIME): lambda v: v.isoformat(),
+  (dict, ParamKeyType.JSON): json.dumps,
+  (list, ParamKeyType.JSON): json.dumps,
+  (bytes, ParamKeyType.BYTES): lambda v: v,
+}
+CPP_2_PYTHON = {
+  ParamKeyType.STRING: lambda v: v.decode("utf-8"),
+  ParamKeyType.BOOL: lambda v: v == b"1",
+  ParamKeyType.INT: int,
+  ParamKeyType.FLOAT: float,
+  ParamKeyType.TIME: lambda v: datetime.datetime.fromisoformat(v.decode("utf-8")),
+  ParamKeyType.JSON: json.loads,
+  ParamKeyType.BYTES: lambda v: v,
+}
+
+
+def ensure_bytes(v):
+  return v.encode() if isinstance(v, str) else v
+
+
+def _copy_string(value):
+  if value.data is None:
+    return None
+  return ctypes.string_at(value.data, value.size)
+
+
+class UnknownKeyName(Exception):
+  pass
+
+
+class Params:
+  def __init__(self, d=""):
+    path = ensure_bytes(d)
+    self.p = params_create(path, len(path))
+    self._finalizer = weakref.finalize(self, params_destroy, self.p)
+    self.d = d
+
+  def __reduce__(self):
+    return (type(self), (self.d,))
+
+  def clear_all(self, tx_flag=ParamKeyFlag.ALL):
+    params_clear_all(self.p, int(tx_flag))
+
+  def check_key(self, key):
+    key = ensure_bytes(key)
+    if b"\0" in key or not params_check_key(self.p, key):
+      raise UnknownKeyName(key)
+    return key
+
+  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:
+      return CPP_2_PYTHON[t](value)
+    except (KeyError, TypeError, ValueError):
+      cloudlog.warning(f"Failed to cast param {key} with {value=} from type {t=}")
+      return self._cpp2python(t, default, None, key)
+
+  def _default(self, key):
+    return _copy_string(params_get_default(self.p, key))
+
+  def get(self, key, block=False, return_default=False):
+    k = self.check_key(key)
+    t = self.get_type(k)
+    default = self._default(k) if return_default else None
+    value = _copy_string(params_get(self.p, k, block))
+    if value == b"":
+      if block:
+        raise KeyboardInterrupt
+      return self._cpp2python(t, default, None, key)
+    return self._cpp2python(t, value, default, key)
+
+  def get_bool(self, key, block=False):
+    return bool(params_get_bool(self.p, self.check_key(key), block))
+
+  def _put_cast(self, key, dat):
+    return ensure_bytes(self.python2cpp(type(dat), self.get_type(key), dat, key))
+
+  def put(self, key, dat, block=False):
+    """Write a parameter. block=True waits until it is persisted to disk."""
+    k = self.check_key(key)
+    value = self._put_cast(k, dat)
+    params_put(self.p, k, value, len(value), block)
+
+  def put_bool(self, key, val, block=False):
+    params_put_bool(self.p, self.check_key(key), val, block)
+
+  def remove(self, key):
+    params_remove(self.p, self.check_key(key))
+
+  def get_param_path(self, key=""):
+    key = ensure_bytes(key)
+    return _copy_string(params_get_path(self.p, key, len(key))).decode()
+
+  def get_type(self, key):
+    return ParamKeyType(params_get_key_type(self.p, self.check_key(key)))
+
+  def all_keys(self):
+    keys = []
+    for i in range(params_keys_size(self.p)):
+      keys.append(_copy_string(params_key_at(self.p, i)))
+    return keys
+
+  def get_default_value(self, key):
+    k = self.check_key(key)
+    return self._cpp2python(self.get_type(k), self._default(k), None, key)
+
+  def cpp2python(self, key, value):
+    return self._cpp2python(self.get_type(key), value, None, key)
+
 
 if __name__ == "__main__":
   import sys
 
   params = Params()
   key = sys.argv[1]
-  assert params.check_key(key), f"unknown param: {key}"
-
+  params.check_key(key)
   if len(sys.argv) == 3:
     val = sys.argv[2]
     print(f"SET: {key} = {val}")

openpilot/common/params_c.cc

@@ -0,0 +1,163 @@
+#include <cstddef>
+#include <cstdio>
+#include <exception>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "common/params.h"
+
+typedef struct {
+  const char *data;
+  size_t size;
+} ParamsBuffer;
+
+struct ParamsHandle {
+  ParamsHandle(const char *path, size_t path_size) : params(std::string(path, path_size)), keys(params.allKeys()) {
+  }
+
+  Params params;
+  const std::vector<std::string> keys;
+};
+
+namespace {
+thread_local char last_error[512] = {};
+thread_local std::string result;
+
+void set_error(const char *error) {
+  snprintf(last_error, sizeof(last_error), "%s", error);
+}
+
+ParamsBuffer return_string(std::string value) {
+  result = std::move(value);
+  return {result.data(), result.size()};
+}
+
+template <typename Result, typename Callable>
+Result translate_exceptions(Result failure, Callable &&callable) noexcept {
+  last_error[0] = '\0';
+  try {
+    return callable();
+  } catch (const std::exception &e) {
+    set_error(e.what());
+  } catch (...) {
+    set_error("unknown C++ exception");
+  }
+  return failure;
+}
+
+template <typename Callable>
+void translate_exceptions(Callable &&callable) noexcept {
+  translate_exceptions(false, [&]() {
+    callable();
+    return true;
+  });
+}
+}  // namespace
+
+extern "C" {
+
+ParamsHandle *params_create(const char *path, size_t path_size) noexcept {
+  return translate_exceptions(static_cast<ParamsHandle *>(nullptr), [&]() {
+    return new ParamsHandle(path, path_size);
+  });
+}
+
+void params_destroy(ParamsHandle *handle) noexcept {
+  translate_exceptions([&]() {
+    delete handle;
+  });
+}
+
+const char *params_last_error() noexcept {
+  return last_error;
+}
+
+void params_clear_all(ParamsHandle *handle, unsigned int flag) noexcept {
+  translate_exceptions([&]() {
+    handle->params.clearAll(static_cast<ParamKeyFlag>(flag));
+  });
+}
+
+bool params_check_key(ParamsHandle *handle, const char *key) noexcept {
+  return translate_exceptions(false, [&]() {
+    return handle->params.checkKey(key);
+  });
+}
+
+int params_get_key_type(ParamsHandle *handle, const char *key) noexcept {
+  return translate_exceptions(-1, [&]() {
+    return static_cast<int>(handle->params.getKeyType(key));
+  });
+}
+
+ParamsBuffer params_get_default(ParamsHandle *handle, const char *key) noexcept {
+  return translate_exceptions(ParamsBuffer{nullptr, 0}, [&]() {
+    auto value = handle->params.getKeyDefaultValue(key);
+    if (!value.has_value()) {
+      return ParamsBuffer{nullptr, 0};
+    }
+    return return_string(*value);
+  });
+}
+
+ParamsBuffer params_get(ParamsHandle *handle, const char *key, bool block) noexcept {
+  return translate_exceptions(ParamsBuffer{nullptr, 0}, [&]() {
+    return return_string(handle->params.get(key, block));
+  });
+}
+
+bool params_get_bool(ParamsHandle *handle, const char *key, bool block) noexcept {
+  return translate_exceptions(false, [&]() {
+    return handle->params.getBool(key, block);
+  });
+}
+
+int params_put(ParamsHandle *handle, const char *key, const char *value, size_t size, bool block) noexcept {
+  return translate_exceptions(-1, [&]() {
+    if (block) {
+      return handle->params.put(key, value, size);
+    }
+    handle->params.putNonBlocking(key, std::string(value, size));
+    return 0;
+  });
+}
+
+int params_put_bool(ParamsHandle *handle, const char *key, bool value, bool block) noexcept {
+  return translate_exceptions(-1, [&]() {
+    if (block) {
+      return handle->params.putBool(key, value);
+    }
+    handle->params.putBoolNonBlocking(key, value);
+    return 0;
+  });
+}
+
+int params_remove(ParamsHandle *handle, const char *key) noexcept {
+  return translate_exceptions(-1, [&]() {
+    return handle->params.remove(key);
+  });
+}
+
+ParamsBuffer params_get_path(ParamsHandle *handle, const char *key, size_t key_size) noexcept {
+  return translate_exceptions(ParamsBuffer{nullptr, 0}, [&]() {
+    return return_string(handle->params.getParamPath(std::string(key, key_size)));
+  });
+}
+
+size_t params_keys_size(ParamsHandle *handle) noexcept {
+  return translate_exceptions(size_t{0}, [&]() {
+    return handle->keys.size();
+  });
+}
+
+ParamsBuffer params_key_at(ParamsHandle *handle, size_t index) noexcept {
+  return translate_exceptions(ParamsBuffer{nullptr, 0}, [&]() {
+    if (index >= handle->keys.size()) {
+      return ParamsBuffer{nullptr, 0};
+    }
+    return return_string(handle->keys[index]);
+  });
+}
+
+}  // extern "C"

openpilot/common/params_pyx.pyx

@@ -1,191 +0,0 @@
-# distutils: language = c++
-# cython: language_level = 3
-import builtins
-import datetime
-import json
-from libcpp cimport bool
-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
-    CLEAR_ON_MANAGER_START
-    CLEAR_ON_ONROAD_TRANSITION
-    CLEAR_ON_OFFROAD_TRANSITION
-    DEVELOPMENT_ONLY
-    CLEAR_ON_IGNITION_ON
-    ALL
-
-  cpdef enum ParamKeyType:
-    STRING
-    BOOL
-    INT
-    FLOAT
-    TIME
-    JSON
-    BYTES
-
-  cdef cppclass c_Params "Params":
-    c_Params(string) except + nogil
-    string get(string, bool) nogil
-    bool getBool(string, bool) nogil
-    int remove(string) nogil
-    int put(string, string) nogil
-    void putNonBlocking(string, string) nogil
-    void putBoolNonBlocking(string, bool) nogil
-    int putBool(string, bool) nogil
-    bool checkKey(string) nogil
-    ParamKeyType getKeyType(string) nogil
-    optional[string] getKeyDefaultValue(string) nogil
-    string getParamPath(string) nogil
-    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
-
-class UnknownKeyName(Exception):
-  pass
-
-cdef class Params:
-  cdef c_Params* p
-  cdef str d
-
-  def __cinit__(self, d=""):
-    cdef string path = <string>d.encode()
-    with nogil:
-      self.p = new c_Params(path)
-    self.d = d
-
-  def __reduce__(self):
-    return (type(self), (self.d,))
-
-  def __dealloc__(self):
-    del self.p
-
-  def clear_all(self, tx_flag=ParamKeyFlag.ALL):
-    self.p.clearAll(tx_flag)
-
-  def check_key(self, key):
-    key = ensure_bytes(key)
-    if not self.p.checkKey(key):
-      raise UnknownKeyName(key)
-    return key
-
-  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:
-      return CPP_2_PYTHON[t](value)
-    except (KeyError, TypeError, ValueError):
-      cloudlog.warning(f"Failed to cast param {key} with {value=} from type {t=}")
-      return self._cpp2python(t, default, None, key)
-
-  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 = (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._cpp2python(t, default_val, None, key)
-    return self._cpp2python(t, val, default_val, key)
-
-  def get_bool(self, key, bool block=False):
-    cdef string k = self.check_key(key)
-    cdef bool r
-    with nogil:
-      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, bool block = False):
-    """
-    Warning: block=True blocks until the param is written to disk!
-    In very rare cases this can take over a second, and your code will hang.
-    Use block=False in time sensitive code, but in general try to avoid
-    writing params as much as possible.
-    """
-    cdef string k = self.check_key(key)
-    cdef string dat_bytes = self._put_cast(key, dat)
-    with nogil:
-      if block:
-        self.p.put(k, dat_bytes)
-      else:
-        self.p.putNonBlocking(k, dat_bytes)
-
-  def put_bool(self, key, bool val, bool block = False):
-    cdef string k = self.check_key(key)
-    with nogil:
-      if block:
-        self.p.putBool(k, val)
-      else:
-        self.p.putBoolNonBlocking(k, val)
-
-  def remove(self, key):
-    cdef string k = self.check_key(key)
-    with nogil:
-      self.p.remove(k)
-
-  def get_param_path(self, key=""):
-    cdef string key_bytes = ensure_bytes(key)
-    return self.p.getParamPath(key_bytes).decode("utf-8")
-
-  def get_type(self, key):
-    return self.p.getKeyType(self.check_key(key))
-
-  def all_keys(self):
-    return self.p.allKeys()
-
-  def get_default_value(self, key):
-    cdef string k = self.check_key(key)
-    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
-
-  def cpp2python(self, key, value):
-    cdef string k = self.check_key(key)
-    cdef ParamKeyType t = self.p.getKeyType(k)
-    return self._cpp2python(t, value, None, key)

openpilot/common/tests/test_params.py

@@ -62,6 +62,11 @@ class TestParams(OpenpilotTestCase):
     with self.assertRaises(UnknownKeyName):
       self.params.put_bool("swag", True, block=True)
 
+    with self.assertRaises(UnknownKeyName):
+      self.params.put(b"DongleId\0suffix", "abc", block=True)
+
+    assert self.params.get_param_path(b"key\0suffix").endswith("/key\0suffix")
+
   def test_remove_not_there(self):
     assert self.params.get("CarParams") is None
     self.params.remove("CarParams")

openpilot/selfdrive/ui/mici/widgets/button.py

@@ -1,6 +1,6 @@
 import math
 import pyray as rl
-from typing import Union
+from typing import TYPE_CHECKING, Union
 from enum import Enum
 from collections.abc import Callable
 from openpilot.system.ui.widgets import Widget
@@ -9,10 +9,13 @@ from openpilot.system.ui.widgets.scroller import DO_ZOOM
 from openpilot.system.ui.lib.application import gui_app, FontWeight, MousePos
 from openpilot.common.filter_simple import BounceFilter
 
-try:
+if TYPE_CHECKING:
   from openpilot.common.params import Params
-except ImportError:
-  Params = None
+else:
+  try:
+    from openpilot.common.params import Params
+  except (ImportError, OSError):
+    Params = None
 
 SCROLLING_SPEED_PX_S = 50
 COMPLICATION_SIZE    = 36

openpilot/system/manager/process.py

@@ -228,7 +228,7 @@ class DaemonProcess(ManagerProcess):
     pass
 
 
-def ensure_running(procs: ValuesView[ManagerProcess], started: bool, params=None, CP: car.CarParams=None,
+def ensure_running(procs: ValuesView[ManagerProcess], started: bool, params: Params, CP: car.CarParams,
                    not_run: list[str] | None=None) -> list[ManagerProcess]:
   if not_run is None:
     not_run = []

openpilot/system/ui/lib/multilang.py

@@ -2,13 +2,17 @@ from importlib.resources import files
 import json
 import os
 import re
+from typing import TYPE_CHECKING
 from openpilot.common.basedir import BASEDIR
 from openpilot.common.swaglog import cloudlog
 
-try:
+if TYPE_CHECKING:
   from openpilot.common.params import Params
-except ImportError:
-  Params = None
+else:
+  try:
+    from openpilot.common.params import Params
+  except (ImportError, OSError):
+    Params = None
 
 SYSTEM_UI_DIR = os.path.join(BASEDIR, "openpilot/system", "ui")
 UI_DIR = files("openpilot.selfdrive.ui")

openpilot/system/ui/lib/wifi_manager.py

@@ -6,7 +6,7 @@ import subprocess
 from collections.abc import Callable
 from dataclasses import dataclass, replace
 from enum import IntEnum
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 from jeepney import DBusAddress, new_method_call
 from jeepney.bus_messages import MatchRule, message_bus
@@ -26,10 +26,13 @@ from openpilot.system.ui.lib.networkmanager import (NM, NM_WIRELESS_IFACE, NM_80
                                                     NM_DEVICE_TYPE_WIFI, NM_ACTIVE_CONNECTION_IFACE,
                                                     NM_IP4_CONFIG_IFACE, NM_PROPERTIES_IFACE, NMDeviceState, NMDeviceStateReason)
 
-try:
+if TYPE_CHECKING:
   from openpilot.common.params import Params
-except Exception:
-  Params = None
+else:
+  try:
+    from openpilot.common.params import Params
+  except (ImportError, OSError):
+    Params = None
 
 TETHERING_IP_ADDRESS = "192.168.43.1"
 DEFAULT_TETHERING_PASSWORD = "swagswagcomma"