From 74ac5ef9a01362bb09512b4a522f10a31199d4eb Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 22 Jul 2026 13:26:28 -0700 Subject: [PATCH] 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 | 13 +- openpilot/common/params.py | 208 +++++++++++++++++- openpilot/common/params_c.cc | 163 ++++++++++++++ openpilot/common/params_pyx.pyx | 191 ---------------- openpilot/common/tests/test_params.py | 5 + openpilot/selfdrive/ui/mici/widgets/button.py | 11 +- openpilot/system/manager/process.py | 2 +- openpilot/system/ui/lib/multilang.py | 10 +- openpilot/system/ui/lib/wifi_manager.py | 11 +- 9 files changed, 396 insertions(+), 218 deletions(-) create mode 100644 openpilot/common/params_c.cc delete mode 100644 openpilot/common/params_pyx.pyx diff --git a/openpilot/common/SConscript b/openpilot/common/SConscript index 600cbec2e5..4e06ae88a1 100644 --- a/openpilot/common/SConscript +++ b/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') diff --git a/openpilot/common/params.py b/openpilot/common/params.py index 8af80c04bb..fc292b56dd 100644 --- a/openpilot/common/params.py +++ b/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}") diff --git a/openpilot/common/params_c.cc b/openpilot/common/params_c.cc new file mode 100644 index 0000000000..ddc9d893a9 --- /dev/null +++ b/openpilot/common/params_c.cc @@ -0,0 +1,163 @@ +#include +#include +#include +#include +#include +#include + +#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 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 +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 +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(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(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(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" diff --git a/openpilot/common/params_pyx.pyx b/openpilot/common/params_pyx.pyx deleted file mode 100644 index c919c85f0c..0000000000 --- a/openpilot/common/params_pyx.pyx +++ /dev/null @@ -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 = 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) diff --git a/openpilot/common/tests/test_params.py b/openpilot/common/tests/test_params.py index 4fcc5c7f58..a81d346b06 100644 --- a/openpilot/common/tests/test_params.py +++ b/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") diff --git a/openpilot/selfdrive/ui/mici/widgets/button.py b/openpilot/selfdrive/ui/mici/widgets/button.py index 3dd7ad9a8a..dcb1dc2fd0 100644 --- a/openpilot/selfdrive/ui/mici/widgets/button.py +++ b/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 diff --git a/openpilot/system/manager/process.py b/openpilot/system/manager/process.py index 7f7eabd7be..418e16fa8f 100644 --- a/openpilot/system/manager/process.py +++ b/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 = [] diff --git a/openpilot/system/ui/lib/multilang.py b/openpilot/system/ui/lib/multilang.py index 2305404679..3d786a7248 100644 --- a/openpilot/system/ui/lib/multilang.py +++ b/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") diff --git a/openpilot/system/ui/lib/wifi_manager.py b/openpilot/system/ui/lib/wifi_manager.py index c52e5a2ddf..edf91ae2ea 100644 --- a/openpilot/system/ui/lib/wifi_manager.py +++ b/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"