diff --git a/common/params_pyx.pyx b/common/params_pyx.pyx index 3204eacff4..7b35e815df 100755 --- a/common/params_pyx.pyx +++ b/common/params_pyx.pyx @@ -14,7 +14,6 @@ cdef extern from "selfdrive/common/params.h": ALL cdef cppclass c_Params "Params": - c_Params() nogil c_Params(string) nogil string get(string, bool) nogil bool getBool(string) nogil @@ -34,36 +33,25 @@ class UnknownKeyName(Exception): cdef class Params: cdef c_Params* p - def __cinit__(self, d=None): - cdef string path - if d is None: - with nogil: - self.p = new c_Params() - else: - path = d.encode() - with nogil: - self.p = new c_Params(path) + def __cinit__(self, d=""): + cdef string path = d.encode() + with nogil: + self.p = new c_Params(path) def __dealloc__(self): del self.p - def clear_all(self, tx_type=None): - if tx_type is None: - tx_type = ParamKeyType.ALL - + def clear_all(self, tx_type=ParamKeyType.ALL): self.p.clearAll(tx_type) def check_key(self, key): key = ensure_bytes(key) - if not self.p.checkKey(key): raise UnknownKeyName(key) - return key def get(self, key, bool block=False, encoding=None): cdef string k = self.check_key(key) - cdef string val with nogil: val = self.p.get(k, block) @@ -76,10 +64,7 @@ cdef class Params: else: return None - if encoding is not None: - return val.decode(encoding) - else: - return val + return val if encoding is None else val.decode(encoding) def get_bool(self, key): cdef string k = self.check_key(key) @@ -110,8 +95,7 @@ cdef class Params: with nogil: self.p.remove(k) - -def put_nonblocking(key, val, d=None): +def put_nonblocking(key, val, d=""): def f(key, val): params = Params(d) cdef string k = ensure_bytes(key) diff --git a/selfdrive/common/params.cc b/selfdrive/common/params.cc index 242b9bac15..f3a36b3bb4 100644 --- a/selfdrive/common/params.cc +++ b/selfdrive/common/params.cc @@ -1,19 +1,9 @@ #include "selfdrive/common/params.h" -#ifndef _GNU_SOURCE -#define _GNU_SOURCE -#endif // _GNU_SOURCE - #include #include -#include -#include #include -#include -#include -#include -#include #include #include "selfdrive/common/swaglog.h" @@ -27,21 +17,16 @@ void params_sig_handler(int signal) { params_do_exit = 1; } -int fsync_dir(const char* path) { - int fd = HANDLE_EINTR(open(path, O_RDONLY, 0755)); - if (fd < 0) { - return -1; - } - - int result = fsync(fd); - int result_close = close(fd); - if (result_close < 0) { - result = result_close; +int fsync_dir(const std::string &path) { + int result = -1; + int fd = HANDLE_EINTR(open(path.c_str(), O_RDONLY, 0755)); + if (fd >= 0) { + result = fsync(fd); + close(fd); } return result; } - bool create_params_path(const std::string ¶m_path, const std::string &key_path) { // Make sure params path exists if (!util::file_exists(param_path) && !util::create_directories(param_path, 0775)) { @@ -51,9 +36,8 @@ bool create_params_path(const std::string ¶m_path, const std::string &key_pa // See if the symlink exists, otherwise create it if (!util::file_exists(key_path)) { // 1) Create temp folder - // 2) Set permissions - // 3) Symlink it to temp link - // 4) Move symlink to /d + // 2) Symlink it to temp link + // 3) Move symlink to /d std::string tmp_path = param_path + "/.tmp_XXXXXX"; // this should be OK since mkdtemp just replaces characters in place @@ -76,32 +60,26 @@ bool create_params_path(const std::string ¶m_path, const std::string &key_pa return true; } -void ensure_params_path(const std::string ¶ms_path) { +std::string ensure_params_path(const std::string &path = {}) { + std::string params_path = path.empty() ? Path::params() : path; if (!create_params_path(params_path, params_path + "/d")) { throw std::runtime_error(util::string_format("Failed to ensure params path, errno=%d", errno)); } + return params_path; } class FileLock { - public: - FileLock(const std::string& file_name, int op) : fn_(file_name), op_(op) {} - - void lock() { - fd_ = HANDLE_EINTR(open(fn_.c_str(), O_CREAT, 0775)); - if (fd_ < 0) { - LOGE("Failed to open lock file %s, errno=%d", fn_.c_str(), errno); - return; - } - if (HANDLE_EINTR(flock(fd_, op_)) < 0) { - LOGE("Failed to lock file %s, errno=%d", fn_.c_str(), errno); +public: + FileLock(const std::string &fn) { + fd_ = HANDLE_EINTR(open(fn.c_str(), O_CREAT, 0775)); + if (fd_ < 0 || HANDLE_EINTR(flock(fd_, LOCK_EX)) < 0) { + LOGE("Failed to lock file %s, errno=%d", fn.c_str(), errno); } } - - void unlock() { close(fd_); } + ~FileLock() { close(fd_); } private: - int fd_ = -1, op_; - std::string fn_; + int fd_ = -1; }; std::unordered_map keys = { @@ -192,13 +170,9 @@ std::unordered_map keys = { } // namespace -Params::Params() : params_path(Path::params()) { - static std::once_flag once_flag; - std::call_once(once_flag, ensure_params_path, params_path); -} - -Params::Params(const std::string &path) : params_path(path) { - ensure_params_path(params_path); +Params::Params(const std::string &path) { + static std::string default_param_path = ensure_params_path(); + params_path = path.empty() ? default_param_path : ensure_params_path(path); } bool Params::checkKey(const std::string &key) { @@ -232,16 +206,13 @@ int Params::put(const char* key, const char* value, size_t value_size) { // fsync to force persist the changes. if ((result = fsync(tmp_fd)) < 0) break; - FileLock file_lock(params_path + "/.lock", LOCK_EX); - std::lock_guard lk(file_lock); + FileLock file_lock(params_path + "/.lock"); // Move temp into place. - std::string path = params_path + "/d/" + std::string(key); - if ((result = rename(tmp_path.c_str(), path.c_str())) < 0) break; + if ((result = rename(tmp_path.c_str(), getParamPath(key).c_str())) < 0) break; // fsync parent directory - path = params_path + "/d"; - result = fsync_dir(path.c_str()); + result = fsync_dir(getParamPath()); } while (false); close(tmp_fd); @@ -249,24 +220,18 @@ int Params::put(const char* key, const char* value, size_t value_size) { return result; } -int Params::remove(const char *key) { - FileLock file_lock(params_path + "/.lock", LOCK_EX); - std::lock_guard lk(file_lock); - // Delete value. - std::string path = params_path + "/d/" + key; - int result = unlink(path.c_str()); +int Params::remove(const std::string &key) { + FileLock file_lock(params_path + "/.lock"); + int result = unlink(getParamPath(key).c_str()); if (result != 0) { return result; } - // fsync parent directory - path = params_path + "/d"; - return fsync_dir(path.c_str()); + return fsync_dir(getParamPath()); } -std::string Params::get(const char *key, bool block) { - std::string path = params_path + "/d/" + key; +std::string Params::get(const std::string &key, bool block) { if (!block) { - return util::read_file(path); + return util::read_file(getParamPath(key)); } else { // blocking read until successful params_do_exit = 0; @@ -275,7 +240,7 @@ std::string Params::get(const char *key, bool block) { std::string value; while (!params_do_exit) { - if (value = util::read_file(path); !value.empty()) { + if (value = util::read_file(getParamPath(key)); !value.empty()) { break; } util::sleep_for(100); // 0.1 s @@ -288,26 +253,19 @@ std::string Params::get(const char *key, bool block) { } std::map Params::readAll() { - FileLock file_lock(params_path + "/.lock", LOCK_SH); - std::lock_guard lk(file_lock); - - std::string key_path = params_path + "/d"; - return util::read_files_in_dir(key_path); + FileLock file_lock(params_path + "/.lock"); + return util::read_files_in_dir(getParamPath()); } void Params::clearAll(ParamKeyType key_type) { - FileLock file_lock(params_path + "/.lock", LOCK_EX); - std::lock_guard lk(file_lock); + FileLock file_lock(params_path + "/.lock"); std::string path; for (auto &[key, type] : keys) { if (type & key_type) { - path = params_path + "/d/" + key; - unlink(path.c_str()); + unlink(getParamPath(key).c_str()); } } - // fsync parent directory - path = params_path + "/d"; - fsync_dir(path.c_str()); + fsync_dir(getParamPath()); } diff --git a/selfdrive/common/params.h b/selfdrive/common/params.h index 77d00b76e7..c4bdde0012 100644 --- a/selfdrive/common/params.h +++ b/selfdrive/common/params.h @@ -1,9 +1,7 @@ #pragma once #include -#include #include -#include enum ParamKeyType { PERSISTENT = 0x02, @@ -17,68 +15,33 @@ enum ParamKeyType { class Params { public: - Params(); - Params(const std::string &path); - + Params(const std::string &path = {}); bool checkKey(const std::string &key); ParamKeyType getKeyType(const std::string &key); + inline std::string getParamPath(const std::string &key = {}) { + return key.empty() ? params_path + "/d" : params_path + "/d/" + key; + } // Delete a value - int remove(const char *key); - inline int remove(const std::string &key) { - return remove (key.c_str()); - } + int remove(const std::string &key); void clearAll(ParamKeyType type); - // read all values - std::map readAll(); - // helpers for reading values - std::string get(const char *key, bool block = false); - - inline std::string get(const std::string &key, bool block = false) { - return get(key.c_str(), block); - } - - inline std::string getParamsPath() { - return params_path; - } - - inline std::string getParamPath(std::string key) { - return params_path + "/d/" + key; - } - - template - std::optional get(const char *key, bool block = false) { - std::istringstream iss(get(key, block)); - T value{}; - iss >> value; - return iss.fail() ? std::nullopt : std::optional(value); - } - + std::string get(const std::string &key, bool block = false); inline bool getBool(const std::string &key) { - return getBool(key.c_str()); - } - - inline bool getBool(const char *key) { return get(key) == "1"; } + std::map readAll(); // helpers for writing values - int put(const char* key, const char* val, size_t value_size); - + int put(const char *key, const char *val, size_t value_size); inline int put(const std::string &key, const std::string &val) { return put(key.c_str(), val.data(), val.size()); } - - inline int putBool(const char *key, bool val) { - return put(key, val ? "1" : "0", 1); - } - inline int putBool(const std::string &key, bool val) { - return putBool(key.c_str(), val); + return put(key.c_str(), val ? "1" : "0", 1); } private: - const std::string params_path; + std::string params_path; }; diff --git a/selfdrive/ui/qt/offroad/settings.cc b/selfdrive/ui/qt/offroad/settings.cc index 35ae2fc919..5528996e77 100644 --- a/selfdrive/ui/qt/offroad/settings.cc +++ b/selfdrive/ui/qt/offroad/settings.cc @@ -227,8 +227,7 @@ SoftwarePanel::SoftwarePanel(QWidget* parent) : ListWidget(parent) { fs_watch = new QFileSystemWatcher(this); QObject::connect(fs_watch, &QFileSystemWatcher::fileChanged, [=](const QString path) { - int update_failed_count = params.get("UpdateFailedCount").value_or(0); - if (path.contains("UpdateFailedCount") && update_failed_count > 0) { + if (path.contains("UpdateFailedCount") && std::atoi(params.get("UpdateFailedCount").c_str()) > 0) { lastUpdateLbl->setText("failed to fetch update"); updateBtn->setText("CHECK"); updateBtn->setEnabled(true);