Reapply "params: unique default value (#35798)" (#35806)

* Reapply "params: unique default value (#35798)"

This reverts commit 267acfb.

* more

* more

* test for this

* better name;

Author: maxime-desroches

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<std::string> Params::getKeyDefaultValue(const std::string &key) {
   return keys[key].default_value;
 }
 

common/params.h

@@ -2,6 +2,7 @@
 
 #include <future>
 #include <map>
+#include <optional>
 #include <string>
 #include <tuple>
 #include <utility>
@@ -33,7 +34,7 @@ enum ParamKeyType {
 struct ParamKeyAttributes {
   uint32_t flags;
   ParamKeyType type;
-  std::string default_value = "";
+  std::optional<std::string> default_value = std::nullopt;
 };
 
 class Params {
@@ -48,7 +49,7 @@ class Params {
   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<std::string> getKeyDefaultValue(const std::string &key);
   inline std::string getParamPath(const std::string &key = {}) {
     return params_path + params_prefix + (key.empty() ? "" : "/" + key);
   }

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

common/tests/test_params.py

@@ -110,10 +110,17 @@ def test_params_all_keys(self):
     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 @@ def test_params_get_type(self):
     # 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

selfdrive/selfdrived/selfdrived.py

@@ -109,7 +109,7 @@ def __init__(self, CP=None):
     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 @@ def step(self):
 
     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):

selfdrive/ui/layouts/settings/device.py

@@ -41,8 +41,8 @@ def __init__(self):
     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),

selfdrive/ui/layouts/settings/toggles.py

@@ -54,7 +54,7 @@ def __init__(self):
         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(

selfdrive/ui/widgets/pairing_dialog.py

@@ -23,7 +23,7 @@ def __init__(self):
 
   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}")

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():

system/hardware/power_monitoring.py

@@ -29,7 +29,7 @@ def __init__(self):
     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)