params: unique default value (#35798)

* default

* None vs ""

* athena

* more

* more

* this

* better

* better

* now

* name

* better

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 @@ 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<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_keys.h

@@ -19,7 +19,7 @@ inline static std::unordered_map<std::string, ParamKeyAttributes> 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}},

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

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