From 27d3f820bf4d34bbc799cfc7ce5f9e006f0fe774 Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Wed, 5 May 2021 01:49:26 +0800 Subject: [PATCH] Params: move keys from cython to cc (#20814) * move keys from cython to cc * consistency * passes tests * more consistency Co-authored-by: Adeeb Shihadeh --- common/params.py | 3 +- common/params_pxd.pxd | 8 +++ common/params_pyx.pyx | 103 ++++++------------------------------ selfdrive/common/params.cc | 91 +++++++++++++++++++++++++++++-- selfdrive/common/params.h | 16 ++++-- selfdrive/loggerd/logger.cc | 2 +- 6 files changed, 128 insertions(+), 95 deletions(-) diff --git a/common/params.py b/common/params.py index 0acb61fa6..d0a37cce1 100644 --- 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 diff --git a/common/params_pxd.pxd b/common/params_pxd.pxd index 28d54c0c5..979805d23 100644 --- 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) diff --git a/common/params_pyx.pyx b/common/params_pyx.pyx index 87d8c35f6..78187fb61 100755 --- 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() diff --git a/selfdrive/common/params.cc b/selfdrive/common/params.cc index 50025a0aa..670e29367 100644 --- a/selfdrive/common/params.cc +++ b/selfdrive/common/params.cc @@ -13,7 +13,7 @@ #include #include #include - +#include #include "common/util.h" #include "common/swaglog.h" @@ -141,6 +141,80 @@ private: std::string fn_; }; +std::unordered_map 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 *params) { +int Params::readAll(std::map *params) { FileLock file_lock(params_path + "/.lock", LOCK_SH); std::lock_guard lk(file_lock); @@ -251,3 +328,11 @@ int Params::read_db_all(std::map *params) { closedir(d); return 0; } + +void Params::clearAll(ParamKeyType key_type) { + for (auto &[key, type] : keys) { + if (type & key_type) { + remove(key); + } + } +} diff --git a/selfdrive/common/params.h b/selfdrive/common/params.h index 9670abd13..7ee1e2602 100644 --- 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 *params); + int readAll(std::map *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) { diff --git a/selfdrive/loggerd/logger.cc b/selfdrive/loggerd/logger.cc index 2fa72a1db..cb5987502 100644 --- a/selfdrive/loggerd/logger.cc +++ b/selfdrive/loggerd/logger.cc @@ -102,7 +102,7 @@ kj::Array logger_build_init_data() { init.setDongleId(params.get("DongleId")); { std::map params_map; - params.read_db_all(¶ms_map); + params.readAll(¶ms_map); auto lparams = init.initParams().initEntries(params_map.size()); int i = 0; for (auto& kv : params_map) {