diff --git a/common/params.cc b/common/params.cc index 5cffc5c4c..6af00fe95 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 b91ba4553..8169063ac 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_pyx.pyx b/common/params_pyx.pyx index 9956da81c..d8fefd63a 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, bool return_default=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_val = self.get_default_value(k) 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 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_val, None) + return self.cast(t, val, default_val) def get_bool(self, key, bool block=False): cdef string k = self.check_key(key) @@ -152,8 +158,12 @@ cdef class Params: 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): - 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 ff670f1b1..337f9da09 100644 --- a/common/tests/test_params.py +++ b/common/tests/test_params.py @@ -110,10 +110,17 @@ 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 self.params.get("LanguageSetting") is None + assert self.params.get("LanguageSetting", return_default=False) is None + assert isinstance(self.params.get("LanguageSetting", return_default=True), str) + assert isinstance(self.params.get("LongitudinalPersonality", return_default=True), int) + assert self.params.get("LiveParameters") is None + assert self.params.get("LiveParameters", return_default=True) is None def test_params_get_type(self): # json @@ -127,18 +134,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 c7b77f32e..6e3e62243 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", return_default=True) 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", return_default=True) time.sleep(0.1) def run(self): diff --git a/selfdrive/ui/layouts/settings/device.py b/selfdrive/ui/layouts/settings/device.py index fcabd10a2..df8bba030 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 1337e29f9..86130b901 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", return_default=True), icon="speed_limit.png" ), toggle_item( diff --git a/selfdrive/ui/widgets/pairing_dialog.py b/selfdrive/ui/widgets/pairing_dialog.py index 32b7c91c4..63298914e 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 71fde0591..7814905e6 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 739f8bbc1..22de6e053 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") or 0 # Reset capacity if it's low self.car_battery_capacity_uWh = max((CAR_BATTERY_CAPACITY_uWh / 10), car_battery_capacity_uWh) diff --git a/system/manager/manager.py b/system/manager/manager.py index 25e58000b..91916f708 100755 --- a/system/manager/manager.py +++ b/system/manager/manager.py @@ -39,7 +39,7 @@ def manager_init() -> None: # set unset params to their default value for k in params.all_keys(): default_value = params.get_default_value(k) - if default_value and not params.get(k): + if default_value and params.get(k) is None: params.put(k, default_value) # Create folders needed for msgq diff --git a/system/manager/test/test_manager.py b/system/manager/test/test_manager.py index 9c0b66400..3dadacf32 100644 --- a/system/manager/test/test_manager.py +++ b/system/manager/test/test_manager.py @@ -38,6 +38,18 @@ class TestManager: # TODO: ensure there are blacklisted procs until we have a dedicated test assert len(BLACKLIST_PROCS), "No blacklisted procs to test not_run" + def test_set_params_with_default_value(self): + params = Params() + params.clear_all() + + os.environ['PREPAREONLY'] = '1' + manager.main() + 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("OpenpilotEnabledToggle") + @pytest.mark.skip("this test is flaky the way it's currently written, should be moved to test_onroad") def test_clean_exit(self, subtests): """