Params: move keys from cython to cc (#20814)

* move keys from cython to cc * consistency * passes tests * more consistency Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
2021-05-05 01:49:26 +08:00 · 2021-05-05 01:49:26 +08:00 · 27d3f820bf
parent ed3b6bd90c
commit 27d3f820bf
6 changed files with 128 additions and 95 deletions
--- a/common/params.py
+++ b/common/params.py
@ -1,5 +1,6 @@
-from common.params_pyx import Params, UnknownKeyName, put_nonblocking # pylint: disable=no-name-in-module, import-error
+from common.params_pyx import Params, ParamKeyType, UnknownKeyName, put_nonblocking # pylint: disable=no-name-in-module, import-error
 assert Params
+assert ParamKeyType
 assert UnknownKeyName
 assert put_nonblocking

--- a/common/params_pxd.pxd
+++ b/common/params_pxd.pxd
@ -8,6 +8,12 @@ cdef extern from "selfdrive/common/util.cc":
  pass

 cdef extern from "selfdrive/common/params.h":
+  cpdef enum ParamKeyType:
+    PERSISTENT
+    CLEAR_ON_MANAGER_START
+    CLEAR_ON_PANDA_DISCONNECT
+    ALL
+
  cdef cppclass Params:
    Params(bool)
    Params(string)
@ -16,3 +22,5 @@ cdef extern from "selfdrive/common/params.h":
    int remove(string)
    int put(string, string)
    int putBool(string, bool)
+    bool checkKey(string)
+    void clearAll(ParamKeyType)
--- a/common/params_pyx.pyx
+++ b/common/params_pyx.pyx
@ -2,90 +2,18 @@
 # cython: language_level = 3
 from libcpp cimport bool
 from libcpp.string cimport string
-from common.params_pxd cimport Params as c_Params
+from common.params_pxd cimport Params as c_Params, ParamKeyType as c_ParamKeyType

 import os
 import threading
 from common.basedir import BASEDIR

-cdef enum TxType:
-  PERSISTENT = 1
-  CLEAR_ON_MANAGER_START = 2
-  CLEAR_ON_PANDA_DISCONNECT = 3

-keys = {
-  b"AccessToken": [TxType.CLEAR_ON_MANAGER_START],
-  b"ApiCache_DriveStats": [TxType.PERSISTENT],
-  b"ApiCache_Device": [TxType.PERSISTENT],
-  b"ApiCache_Owner": [TxType.PERSISTENT],
-  b"AthenadPid": [TxType.PERSISTENT],
-  b"CalibrationParams": [TxType.PERSISTENT],
-  b"CarBatteryCapacity": [TxType.PERSISTENT],
-  b"CarParams": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"CarParamsCache": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"CarVin": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"CommunityFeaturesToggle": [TxType.PERSISTENT],
-  b"ControlsReady": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"EnableLteOnroad": [TxType.PERSISTENT],
-  b"EndToEndToggle": [TxType.PERSISTENT],
-  b"CompletedTrainingVersion": [TxType.PERSISTENT],
-  b"DisablePowerDown": [TxType.PERSISTENT],
-  b"DisableUpdates": [TxType.PERSISTENT],
-  b"EnableWideCamera": [TxType.PERSISTENT],
-  b"DoUninstall": [TxType.CLEAR_ON_MANAGER_START],
-  b"DongleId": [TxType.PERSISTENT],
-  b"GitDiff": [TxType.PERSISTENT],
-  b"GitBranch": [TxType.PERSISTENT],
-  b"GitCommit": [TxType.PERSISTENT],
-  b"GitRemote": [TxType.PERSISTENT],
-  b"GithubSshKeys": [TxType.PERSISTENT],
-  b"GithubUsername": [TxType.PERSISTENT],
-  b"HardwareSerial": [TxType.PERSISTENT],
-  b"HasAcceptedTerms": [TxType.PERSISTENT],
-  b"IsDriverViewEnabled": [TxType.CLEAR_ON_MANAGER_START],
-  b"IMEI": [TxType.PERSISTENT],
-  b"IsLdwEnabled": [TxType.PERSISTENT],
-  b"IsMetric": [TxType.PERSISTENT],
-  b"IsOffroad": [TxType.CLEAR_ON_MANAGER_START],
-  b"IsRHD": [TxType.PERSISTENT],
-  b"IsTakingSnapshot": [TxType.CLEAR_ON_MANAGER_START],
-  b"IsUpdateAvailable": [TxType.CLEAR_ON_MANAGER_START],
-  b"IsUploadRawEnabled": [TxType.PERSISTENT],
-  b"LastAthenaPingTime": [TxType.PERSISTENT],
-  b"LastGPSPosition": [TxType.PERSISTENT],
-  b"LastUpdateException": [TxType.PERSISTENT],
-  b"LastUpdateTime": [TxType.PERSISTENT],
-  b"LiveParameters": [TxType.PERSISTENT],
-  b"OpenpilotEnabledToggle": [TxType.PERSISTENT],
-  b"PandaFirmware": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"PandaFirmwareHex": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"PandaDongleId": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"Passive": [TxType.PERSISTENT],
-  b"RecordFront": [TxType.PERSISTENT],
-  b"RecordFrontLock": [TxType.PERSISTENT],  # for the internal fleet
-  b"ReleaseNotes": [TxType.PERSISTENT],
-  b"ShouldDoUpdate": [TxType.CLEAR_ON_MANAGER_START],
-  b"SubscriberInfo": [TxType.PERSISTENT],
-  b"SshEnabled": [TxType.PERSISTENT],
-  b"TermsVersion": [TxType.PERSISTENT],
-  b"Timezone": [TxType.PERSISTENT],
-  b"TrainingVersion": [TxType.PERSISTENT],
-  b"UpdateAvailable": [TxType.CLEAR_ON_MANAGER_START],
-  b"UpdateFailedCount": [TxType.CLEAR_ON_MANAGER_START],
-  b"Version": [TxType.PERSISTENT],
-  b"VisionRadarToggle": [TxType.PERSISTENT],
-  b"Offroad_ChargeDisabled": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"Offroad_ConnectivityNeeded": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_ConnectivityNeededPrompt": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_TemperatureTooHigh": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_PandaFirmwareMismatch": [TxType.CLEAR_ON_MANAGER_START, TxType.CLEAR_ON_PANDA_DISCONNECT],
-  b"Offroad_InvalidTime": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_IsTakingSnapshot": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_NeosUpdate": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_UpdateFailed": [TxType.CLEAR_ON_MANAGER_START],
-  b"Offroad_HardwareUnsupported": [TxType.CLEAR_ON_MANAGER_START],
-  b"ForcePowerDown": [TxType.CLEAR_ON_MANAGER_START],
-}
+cdef class ParamKeyType:
+  PERSISTENT = c_ParamKeyType.PERSISTENT
+  CLEAR_ON_MANAGER_START = c_ParamKeyType.CLEAR_ON_MANAGER_START
+  CLEAR_ON_PANDA_DISCONNECT = c_ParamKeyType.CLEAR_ON_PANDA_DISCONNECT
+  ALL = c_ParamKeyType.ALL

 def ensure_bytes(v):
  if isinstance(v, str):
@ -110,20 +38,21 @@ cdef class Params:
    del self.p

  def clear_all(self, tx_type=None):
-    for key in keys:
-      if tx_type is None or tx_type in keys[key]:
-        self.delete(key)
+    if tx_type is None:
+      tx_type = ParamKeyType.ALL
+
+    self.p.clearAll(tx_type)

  def manager_start(self):
-    self.clear_all(TxType.CLEAR_ON_MANAGER_START)
+    self.clear_all(ParamKeyType.CLEAR_ON_MANAGER_START)

  def panda_disconnect(self):
-    self.clear_all(TxType.CLEAR_ON_PANDA_DISCONNECT)
+    self.clear_all(ParamKeyType.CLEAR_ON_PANDA_DISCONNECT)

  def check_key(self, key):
    key = ensure_bytes(key)

-    if key not in keys:
+    if not self.p.checkKey(key):
      raise UnknownKeyName(key)

    return key
@ -160,9 +89,8 @@ cdef class Params:
    Use the put_nonblocking helper function in time sensitive code, but
    in general try to avoid writing params as much as possible.
    """
-    dat = ensure_bytes(dat)
-
    cdef string k = self.check_key(key)
+    dat = ensure_bytes(dat)
    self.p.put(k, dat)

  def put_bool(self, key, val):
@ -177,7 +105,8 @@ cdef class Params:
 def put_nonblocking(key, val, d=None):
  def f(key, val):
    params = Params(d)
-    params.put(key, val)
+    cdef string k = ensure_bytes(key)
+    params.put(k, val)

  t = threading.Thread(target=f, args=(key, val))
  t.start()
--- a/selfdrive/common/params.cc
+++ b/selfdrive/common/params.cc
@ -13,7 +13,7 @@
 #include <mutex>
 #include <csignal>
 #include <string.h>
-
+#include <unordered_map>
 #include "common/util.h"
 #include "common/swaglog.h"

@ -141,6 +141,80 @@ private:
  std::string fn_;
 };

+std::unordered_map<std::string, uint32_t> keys = {
+    {"AccessToken", CLEAR_ON_MANAGER_START},
+    {"ApiCache_DriveStats", PERSISTENT},
+    {"ApiCache_Device", PERSISTENT},
+    {"ApiCache_Owner", PERSISTENT},
+    {"AthenadPid", PERSISTENT},
+    {"CalibrationParams", PERSISTENT},
+    {"CarBatteryCapacity", PERSISTENT},
+    {"CarParams", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"CarParamsCache", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"CarVin", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"CommunityFeaturesToggle", PERSISTENT},
+    {"ControlsReady", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"EnableLteOnroad", PERSISTENT},
+    {"EndToEndToggle", PERSISTENT},
+    {"CompletedTrainingVersion", PERSISTENT},
+    {"DisablePowerDown", PERSISTENT},
+    {"DisableUpdates", PERSISTENT},
+    {"EnableWideCamera", PERSISTENT},
+    {"DoUninstall", CLEAR_ON_MANAGER_START},
+    {"DongleId", PERSISTENT},
+    {"GitDiff", PERSISTENT},
+    {"GitBranch", PERSISTENT},
+    {"GitCommit", PERSISTENT},
+    {"GitRemote", PERSISTENT},
+    {"GithubSshKeys", PERSISTENT},
+    {"GithubUsername", PERSISTENT},
+    {"HardwareSerial", PERSISTENT},
+    {"HasAcceptedTerms", PERSISTENT},
+    {"IsDriverViewEnabled", CLEAR_ON_MANAGER_START},
+    {"IMEI", PERSISTENT},
+    {"IsLdwEnabled", PERSISTENT},
+    {"IsMetric", PERSISTENT},
+    {"IsOffroad", CLEAR_ON_MANAGER_START},
+    {"IsRHD", PERSISTENT},
+    {"IsTakingSnapshot", CLEAR_ON_MANAGER_START},
+    {"IsUpdateAvailable", CLEAR_ON_MANAGER_START},
+    {"IsUploadRawEnabled", PERSISTENT},
+    {"LastAthenaPingTime", PERSISTENT},
+    {"LastGPSPosition", PERSISTENT},
+    {"LastUpdateException", PERSISTENT},
+    {"LastUpdateTime", PERSISTENT},
+    {"LiveParameters", PERSISTENT},
+    {"OpenpilotEnabledToggle", PERSISTENT},
+    {"PandaFirmware", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"PandaFirmwareHex", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"PandaDongleId", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"Passive", PERSISTENT},
+    {"RecordFront", PERSISTENT},
+    {"RecordFrontLock", PERSISTENT},  // for the internal fleet
+    {"ReleaseNotes", PERSISTENT},
+    {"ShouldDoUpdate", CLEAR_ON_MANAGER_START},
+    {"SubscriberInfo", PERSISTENT},
+    {"SshEnabled", PERSISTENT},
+    {"TermsVersion", PERSISTENT},
+    {"Timezone", PERSISTENT},
+    {"TrainingVersion", PERSISTENT},
+    {"UpdateAvailable", CLEAR_ON_MANAGER_START},
+    {"UpdateFailedCount", CLEAR_ON_MANAGER_START},
+    {"Version", PERSISTENT},
+    {"VisionRadarToggle", PERSISTENT},
+    {"Offroad_ChargeDisabled", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"Offroad_ConnectivityNeeded", CLEAR_ON_MANAGER_START},
+    {"Offroad_ConnectivityNeededPrompt", CLEAR_ON_MANAGER_START},
+    {"Offroad_TemperatureTooHigh", CLEAR_ON_MANAGER_START},
+    {"Offroad_PandaFirmwareMismatch", CLEAR_ON_MANAGER_START | CLEAR_ON_PANDA_DISCONNECT},
+    {"Offroad_InvalidTime", CLEAR_ON_MANAGER_START},
+    {"Offroad_IsTakingSnapshot", CLEAR_ON_MANAGER_START},
+    {"Offroad_NeosUpdate", CLEAR_ON_MANAGER_START},
+    {"Offroad_UpdateFailed", CLEAR_ON_MANAGER_START},
+    {"Offroad_HardwareUnsupported", CLEAR_ON_MANAGER_START},
+    {"ForcePowerDown", CLEAR_ON_MANAGER_START},
+};
+
 } // namespace

 Params::Params(bool persistent_param) : Params(persistent_param ? persistent_params_path : default_params_path) {}
@ -151,6 +225,10 @@ Params::Params(const std::string &path) : params_path(path) {
  }
 }

+bool Params::checkKey(const std::string &key) {
+  return keys.find(key) != keys.end();
+}
+
 int Params::put(const char* key, const char* value, size_t value_size) {
  // Information about safely and atomically writing a file: https://lwn.net/Articles/457667/
  // 1) Create temp file
@ -158,7 +236,6 @@ int Params::put(const char* key, const char* value, size_t value_size) {
  // 3) fsync() the temp file
  // 4) rename the temp file to the real name
  // 5) fsync() the containing directory
-
  std::string tmp_path = params_path + "/.tmp_value_XXXXXX";
  int tmp_fd = mkstemp((char*)tmp_path.c_str());
  if (tmp_fd < 0) return -1;
@ -233,7 +310,7 @@ std::string Params::get(const char *key, bool block) {
  }
 }

-int Params::read_db_all(std::map<std::string, std::string> *params) {
+int Params::readAll(std::map<std::string, std::string> *params) {
  FileLock file_lock(params_path + "/.lock", LOCK_SH);
  std::lock_guard<FileLock> lk(file_lock);

@ -251,3 +328,11 @@ int Params::read_db_all(std::map<std::string, std::string> *params) {
  closedir(d);
  return 0;
 }
+
+void Params::clearAll(ParamKeyType key_type) {
+  for (auto &[key, type] : keys) {
+    if (type & key_type) {
+      remove(key);
+    }
+  }
+}
--- a/selfdrive/common/params.h
+++ b/selfdrive/common/params.h
@ -7,6 +7,13 @@

 #define ERR_NO_VALUE -33

+enum ParamKeyType {
+  PERSISTENT = 0x02,
+  CLEAR_ON_MANAGER_START = 0x04,
+  CLEAR_ON_PANDA_DISCONNECT = 0x08,
+  ALL = 0x02 | 0x04 | 0x08
+};
+
 class Params {
 private:
  std::string params_path;
@ -15,16 +22,19 @@ public:
  Params(bool persistent_param = false);
  Params(const std::string &path);

+  bool checkKey(const std::string &key);
+
  // Delete a value
  int remove(const char *key);
  inline int remove(const std::string &key) {
    return remove (key.c_str());
  }
+  void clearAll(ParamKeyType type);

  // read all values
-  int read_db_all(std::map<std::string, std::string> *params);
+  int readAll(std::map<std::string, std::string> *params);

-  // read a value
+  // helpers for reading values
  std::string get(const char *key, bool block = false);

  inline std::string get(const std::string &key, bool block = false) {
@ -47,7 +57,7 @@ public:
    return get(key) == "1";
  }

-  // write a value
+  // helpers for writing values
  int put(const char* key, const char* val, size_t value_size);

  inline int put(const std::string &key, const std::string &val) {
--- a/selfdrive/loggerd/logger.cc
+++ b/selfdrive/loggerd/logger.cc
@ -102,7 +102,7 @@ kj::Array<capnp::word> logger_build_init_data() {
  init.setDongleId(params.get("DongleId"));
  {
    std::map<std::string, std::string> params_map;
-    params.read_db_all(&params_map);
+    params.readAll(&params_map);
    auto lparams = init.initParams().initEntries(params_map.size());
    int i = 0;
    for (auto& kv : params_map) {