From abd657edfa6d9c3a2f1838c92d3335e9a87d8129 Mon Sep 17 00:00:00 2001 From: Maxime Desroches Date: Wed, 23 Jul 2025 21:17:21 -0700 Subject: [PATCH] params: unique default value (#35798) * default * None vs "" * athena * more * more * this * better * better * now * name * better --- common/params.cc | 2 +- common/params.h | 5 ++- common/params_keys.h | 2 +- common/params_pyx.pyx | 57 +++++++++++++----------- common/tests/test_params.py | 23 ++++------ selfdrive/selfdrived/selfdrived.py | 7 +-- selfdrive/ui/layouts/settings/device.py | 4 +- selfdrive/ui/layouts/settings/toggles.py | 2 +- selfdrive/ui/widgets/pairing_dialog.py | 2 +- system/athena/athenad.py | 4 +- system/hardware/power_monitoring.py | 2 +- 11 files changed, 55 insertions(+), 55 deletions(-) diff --git a/common/params.cc b/common/params.cc index 5cffc5c4cf..6af00fe95c 100644 --- a/common/params.cc +++ b/common/params.cc @@ -123,7 +123,7 @@ ParamKeyType Params::getKeyType(const std::string &key) { return keys[key].type; } -std::string Params::getKeyDefaultValue(const std::string &key) { +std::optional Params::getKeyDefaultValue(const std::string &key) { return keys[key].default_value; } diff --git a/common/params.h b/common/params.h index b91ba4553d..8169063ac0 100644 --- a/common/params.h +++ b/common/params.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -33,7 +34,7 @@ enum ParamKeyType { struct ParamKeyAttributes { uint32_t flags; ParamKeyType type; - std::string default_value = ""; + std::optional default_value = std::nullopt; }; class Params { @@ -48,7 +49,7 @@ public: bool checkKey(const std::string &key); ParamKeyFlag getKeyFlag(const std::string &key); ParamKeyType getKeyType(const std::string &key); - std::string getKeyDefaultValue(const std::string &key); + std::optional getKeyDefaultValue(const std::string &key); inline std::string getParamPath(const std::string &key = {}) { return params_path + params_prefix + (key.empty() ? "" : "/" + key); } diff --git a/common/params_keys.h b/common/params_keys.h index d3b75e4abd..f5d3ebc6d9 100644 --- a/common/params_keys.h +++ b/common/params_keys.h @@ -19,7 +19,7 @@ inline static std::unordered_map keys = { {"CalibrationParams", {PERSISTENT, BYTES}}, {"CameraDebugExpGain", {CLEAR_ON_MANAGER_START, STRING}}, {"CameraDebugExpTime", {CLEAR_ON_MANAGER_START, STRING}}, - {"CarBatteryCapacity", {PERSISTENT, INT}}, + {"CarBatteryCapacity", {PERSISTENT, INT, "0"}}, {"CarParams", {CLEAR_ON_MANAGER_START | CLEAR_ON_ONROAD_TRANSITION, BYTES}}, {"CarParamsCache", {CLEAR_ON_MANAGER_START, BYTES}}, {"CarParamsPersistent", {PERSISTENT, BYTES}}, diff --git a/common/params_pyx.pyx b/common/params_pyx.pyx index 9956da81cf..ef53ccfb1c 100644 --- a/common/params_pyx.pyx +++ b/common/params_pyx.pyx @@ -5,6 +5,7 @@ import json from libcpp cimport bool from libcpp.string cimport string from libcpp.vector cimport vector +from libcpp.optional cimport optional cdef extern from "common/params.h": cpdef enum ParamKeyFlag: @@ -36,7 +37,7 @@ cdef extern from "common/params.h": int putBool(string, bool) nogil bool checkKey(string) nogil ParamKeyType getKeyType(string) nogil - string getKeyDefaultValue(string) nogil + optional[string] getKeyDefaultValue(string) nogil string getParamPath(string) nogil void clearAll(ParamKeyFlag) vector[string] allKeys() @@ -73,40 +74,45 @@ cdef class Params: raise UnknownKeyName(key) return key - def get(self, key, bool block=False, default=None): + def cast(self, t, value, default): + if value is None: + return None + try: + if t == STRING: + return value.decode("utf-8") + elif t == BOOL: + return value == b"1" + 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): cdef string k = self.check_key(key) - cdef ParamKeyType t = self.p.getKeyType(ensure_bytes(key)) + cdef ParamKeyType t = self.p.getKeyType(k) cdef string val with nogil: val = self.p.get(k, block) + default = self.get_default_value(k) 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 default - - try: - if t == STRING: - return val.decode("utf-8") - elif t == BOOL: - return val == b"1" - elif t == INT: - return int(val) - elif t == FLOAT: - return float(val) - elif t == TIME: - return datetime.datetime.fromisoformat(val.decode("utf-8")) - elif t == JSON: - return json.loads(val) - elif t == BYTES: - return val - else: - return default - except (TypeError, ValueError): - return default + return self.cast(t, default, None) + return self.cast(t, val, default) def get_bool(self, key, bool block=False): cdef string k = self.check_key(key) @@ -156,4 +162,5 @@ cdef class Params: return self.p.allKeys() def get_default_value(self, key): - return self.p.getKeyDefaultValue(self.check_key(key)) + cdef optional[string] default = self.p.getKeyDefaultValue(self.check_key(key)) + return default.value() if default.has_value() else None diff --git a/common/tests/test_params.py b/common/tests/test_params.py index ff670f1b15..6f07e77f5e 100644 --- a/common/tests/test_params.py +++ b/common/tests/test_params.py @@ -110,10 +110,14 @@ class TestParams: assert len(keys) == len(set(keys)) assert b"CarParams" in keys - def test_params_default_init_value(self): - assert self.params.get_default_value("LanguageSetting") - assert self.params.get_default_value("LongitudinalPersonality") - assert not self.params.get_default_value("LiveParameters") + def test_params_default_value(self): + self.params.remove("LanguageSetting") + self.params.remove("LongitudinalPersonality") + self.params.remove("LiveParameters") + + assert isinstance(self.params.get("LanguageSetting"), str) + assert isinstance(self.params.get("LongitudinalPersonality"), int) + assert self.params.get("LiveParameters") is None def test_params_get_type(self): # json @@ -127,18 +131,9 @@ class TestParams: # bool self.params.put("AdbEnabled", "1") 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)) assert self.params.get("InstallDate") == now - - def test_params_get_default(self): - now = datetime.datetime.now(datetime.UTC) - self.params.remove("InstallDate") - assert self.params.get("InstallDate") is None - assert self.params.get("InstallDate", default=now) == now - - self.params.put("BootCount", "1xx1") - assert self.params.get("BootCount") is None - assert self.params.get("BootCount", default=1441) == 1441 diff --git a/selfdrive/selfdrived/selfdrived.py b/selfdrive/selfdrived/selfdrived.py index c7b77f32e7..2587c5fb53 100755 --- a/selfdrive/selfdrived/selfdrived.py +++ b/selfdrive/selfdrived/selfdrived.py @@ -109,7 +109,7 @@ class SelfdriveD: self.logged_comm_issue = None self.not_running_prev = None self.experimental_mode = False - self.personality = self.read_personality_param() + self.personality = self.params.get('LongitudinalPersonality') self.recalibrating_seen = False self.state_machine = StateMachine() self.rk = Ratekeeper(100, print_delay_threshold=None) @@ -477,16 +477,13 @@ class SelfdriveD: self.CS_prev = CS - def read_personality_param(self): - return self.params.get('LongitudinalPersonality', default=log.LongitudinalPersonality.standard) - def params_thread(self, evt): while not evt.is_set(): self.is_metric = self.params.get_bool("IsMetric") self.is_ldw_enabled = self.params.get_bool("IsLdwEnabled") self.disengage_on_accelerator = self.params.get_bool("DisengageOnAccelerator") self.experimental_mode = self.params.get_bool("ExperimentalMode") and self.CP.openpilotLongitudinalControl - self.personality = self.read_personality_param() + self.personality = self.params.get('LongitudinalPersonality') time.sleep(0.1) def run(self): diff --git a/selfdrive/ui/layouts/settings/device.py b/selfdrive/ui/layouts/settings/device.py index fcabd10a22..df8bba030c 100644 --- a/selfdrive/ui/layouts/settings/device.py +++ b/selfdrive/ui/layouts/settings/device.py @@ -41,8 +41,8 @@ class DeviceLayout(Widget): self._scroller = Scroller(items, line_separator=True, spacing=0) def _initialize_items(self): - dongle_id = self._params.get("DongleId", default="N/A") - serial = self._params.get("HardwareSerial", default="N/A") + dongle_id = self._params.get("DongleId") or "N/A" + serial = self._params.get("HardwareSerial") or "N/A" items = [ text_item("Dongle ID", dongle_id), diff --git a/selfdrive/ui/layouts/settings/toggles.py b/selfdrive/ui/layouts/settings/toggles.py index 1337e29f92..f0ae40c647 100644 --- a/selfdrive/ui/layouts/settings/toggles.py +++ b/selfdrive/ui/layouts/settings/toggles.py @@ -54,7 +54,7 @@ class TogglesLayout(Widget): buttons=["Aggressive", "Standard", "Relaxed"], button_width=255, callback=self._set_longitudinal_personality, - selected_index=self._params.get("LongitudinalPersonality", default=0), + selected_index=self._params.get("LongitudinalPersonality"), icon="speed_limit.png" ), toggle_item( diff --git a/selfdrive/ui/widgets/pairing_dialog.py b/selfdrive/ui/widgets/pairing_dialog.py index 32b7c91c4d..63298914ea 100644 --- a/selfdrive/ui/widgets/pairing_dialog.py +++ b/selfdrive/ui/widgets/pairing_dialog.py @@ -23,7 +23,7 @@ class PairingDialog: def _get_pairing_url(self) -> str: try: - dongle_id = self.params.get("DongleId", default="") + dongle_id = self.params.get("DongleId") or "" token = Api(dongle_id).get_token() except Exception as e: cloudlog.warning(f"Failed to get pairing token: {e}") diff --git a/system/athena/athenad.py b/system/athena/athenad.py index 71fde0591e..7814905e60 100755 --- a/system/athena/athenad.py +++ b/system/athena/athenad.py @@ -532,12 +532,12 @@ def getPublicKey() -> str | None: @dispatcher.add_method def getSshAuthorizedKeys() -> str: - return cast(str, Params().get("GithubSshKeys", default="")) + return cast(str, Params().get("GithubSshKeys") or "") @dispatcher.add_method def getGithubUsername() -> str: - return cast(str, Params().get("GithubUsername", default="")) + return cast(str, Params().get("GithubUsername") or "") @dispatcher.add_method def getSimInfo(): diff --git a/system/hardware/power_monitoring.py b/system/hardware/power_monitoring.py index 739f8bbc18..c0120ab383 100644 --- a/system/hardware/power_monitoring.py +++ b/system/hardware/power_monitoring.py @@ -29,7 +29,7 @@ class PowerMonitoring: self.car_voltage_instant_mV = 12e3 # Last value of peripheralState voltage self.integration_lock = threading.Lock() - car_battery_capacity_uWh = self.params.get("CarBatteryCapacity", default=0) + car_battery_capacity_uWh = self.params.get("CarBatteryCapacity") # Reset capacity if it's low self.car_battery_capacity_uWh = max((CAR_BATTERY_CAPACITY_uWh / 10), car_battery_capacity_uWh)