Ambient light calibration parameters on persist partition (#1429)

* WIP: light sensor persistent params

* fix params writing

* Persistent params argument in C params lib. Fixed symlink flow

* Removed debug prints

* fix boardd

* fix ui

* remove debug print
old-commit-hash: 71bebc4fca

selfdrive/boardd/boardd.cc

@@ -95,7 +95,7 @@ void *safety_setter_thread(void *s) {
   // switch to SILENT when CarVin param is read
   while (1) {
     if (do_exit) return NULL;
-    const int result = read_db_value(NULL, "CarVin", &value_vin, &value_vin_sz);
+    const int result = read_db_value("CarVin", &value_vin, &value_vin_sz);
     if (value_vin_sz > 0) {
       // sanity check VIN format
       assert(value_vin_sz == 17);
@@ -118,7 +118,7 @@ void *safety_setter_thread(void *s) {
   while (1) {
     if (do_exit) return NULL;
 
-    const int result = read_db_value(NULL, "CarParams", &value, &value_sz);
+    const int result = read_db_value("CarParams", &value, &value_sz);
     if (value_sz > 0) break;
     usleep(100*1000);
   }
@@ -183,13 +183,13 @@ bool usb_connect() {
   err2 = libusb_control_transfer(dev_handle, 0xc0, 0xd4, 0, 0, fw_sig_buf + 64, 64, TIMEOUT);
   if ((err == 64) && (err2 == 64)) {
     printf("FW signature read\n");
-    write_db_value(NULL, "PandaFirmware", (const char *)fw_sig_buf, 128);
+    write_db_value("PandaFirmware", (const char *)fw_sig_buf, 128);
 
     for (size_t i = 0; i < 8; i++){
       fw_sig_hex_buf[2*i] = NIBBLE_TO_HEX(fw_sig_buf[i] >> 4);
       fw_sig_hex_buf[2*i+1] = NIBBLE_TO_HEX(fw_sig_buf[i] & 0xF);
     }
-    write_db_value(NULL, "PandaFirmwareHex", (const char *)fw_sig_hex_buf, 16);
+    write_db_value("PandaFirmwareHex", (const char *)fw_sig_hex_buf, 16);
   }
   else { goto fail; }
 
@@ -199,7 +199,7 @@ bool usb_connect() {
   if (err > 0) {
     serial = (const char *)serial_buf;
     serial_sz = strnlen(serial, err);
-    write_db_value(NULL, "PandaDongleId", serial, serial_sz);
+    write_db_value("PandaDongleId", serial, serial_sz);
     printf("panda serial: %.*s\n", serial_sz, serial);
   }
   else { goto fail; }
@@ -418,7 +418,7 @@ void can_health(PubSocket *publisher) {
   if ((no_ignition_exp || (voltage_f < VBATT_PAUSE_CHARGING)) && cdp_mode && !ignition) {
     char *disable_power_down = NULL;
     size_t disable_power_down_sz = 0;
-    const int result = read_db_value(NULL, "DisablePowerDown", &disable_power_down, &disable_power_down_sz);
+    const int result = read_db_value("DisablePowerDown", &disable_power_down, &disable_power_down_sz);
     if (disable_power_down_sz != 1 || disable_power_down[0] != '1') {
       printf("TURN OFF CHARGING!\n");
       pthread_mutex_lock(&usb_lock);
@@ -456,9 +456,9 @@ void can_health(PubSocket *publisher) {
 
   // clear VIN, CarParams, and set new safety on car start
   if (ignition && !ignition_last) {
-    int result = delete_db_value(NULL, "CarVin");
+    int result = delete_db_value("CarVin");
     assert((result == 0) || (result == ERR_NO_VALUE));
-    result = delete_db_value(NULL, "CarParams");
+    result = delete_db_value("CarParams");
     assert((result == 0) || (result == ERR_NO_VALUE));
 
     if (!safety_setter_thread_initialized) {

selfdrive/common/params.cc

@@ -25,10 +25,16 @@ T* null_coalesce(T* a, T* b) {
   return a != NULL ? a : b;
 }
 
-static const char* default_params_path = null_coalesce(
-    const_cast<const char*>(getenv("PARAMS_PATH")), "/data/params");
+static const char* default_params_path = null_coalesce(const_cast<const char*>(getenv("PARAMS_PATH")), "/data/params");
+
+#ifdef QCOM
+static const char* persistent_params_path = null_coalesce(const_cast<const char*>(getenv("PERSISTENT_PARAMS_PATH")), "/persist/comma/params");
+#else
+static const char* persistent_params_path = default_params_path;
+#endif
+
+} //namespace
 
-}  // namespace
 
 static int fsync_dir(const char* path){
   int result = 0;
@@ -57,8 +63,15 @@ static int fsync_dir(const char* path){
   }
 }
 
-int write_db_value(const char* params_path, const char* key, const char* value,
-                   size_t value_size) {
+static int ensure_dir_exists(const char* path) {
+  struct stat st;
+  if (stat(path, &st) == -1) {
+    return mkdir(path, 0700);
+  }
+  return 0;
+}
+
+int write_db_value(const char* key, const char* value, size_t value_size, bool persistent_param) {
   // Information about safely and atomically writing a file: https://lwn.net/Articles/457667/
   // 1) Create temp file
   // 2) Write data to temp file
@@ -71,10 +84,59 @@ int write_db_value(const char* params_path, const char* key, const char* value,
   int result;
   char tmp_path[1024];
   char path[1024];
+  char *tmp_dir;
   ssize_t bytes_written;
+  const char* params_path = persistent_param ? persistent_params_path : default_params_path;
+
+  // Make sure params path exists
+  result = ensure_dir_exists(params_path);
+  if (result < 0) {
+    goto cleanup;
+  }
 
-  if (params_path == NULL) {
-    params_path = default_params_path;
+  result = snprintf(path, sizeof(path), "%s/d", params_path);
+  if (result < 0) {
+    goto cleanup;
+  }
+
+  // See if the symlink exists, otherwise create it
+  struct stat st;
+  if (stat(path, &st) == -1) {
+    // Create temp folder
+    result = snprintf(path, sizeof(path), "%s/.tmp_XXXXXX", params_path);
+    if (result < 0) {
+      goto cleanup;
+    }
+    tmp_dir = mkdtemp(path);
+    if (tmp_dir == NULL){
+      goto cleanup;
+    }
+
+    // Set permissions
+    result = chmod(tmp_dir, 0777);
+    if (result < 0) {
+      goto cleanup;
+    }
+
+    // Symlink it to temp link
+    result = snprintf(tmp_path, sizeof(tmp_path), "%s.link", tmp_dir);
+    if (result < 0) {
+      goto cleanup;
+    }
+    result = symlink(tmp_dir, tmp_path);
+    if (result < 0) {
+      goto cleanup;
+    }
+
+    // Move symlink to <params>/d
+    result = snprintf(path, sizeof(path), "%s/d", params_path);
+    if (result < 0) {
+      goto cleanup;
+    }
+    result = rename(tmp_path, path);
+    if (result < 0) {
+      goto cleanup;
+    }
   }
 
   // Write value to temp.
@@ -96,7 +158,7 @@ int write_db_value(const char* params_path, const char* key, const char* value,
   if (result < 0) {
     goto cleanup;
   }
-  lock_fd = open(path, 0);
+  lock_fd = open(path, O_CREAT);
 
   // Build key path
   result = snprintf(path, sizeof(path), "%s/d/%s", params_path, key);
@@ -153,21 +215,18 @@ cleanup:
   return result;
 }
 
-int delete_db_value(const char* params_path, const char* key) {
+int delete_db_value(const char* key, bool persistent_param) {
   int lock_fd = -1;
   int result;
   char path[1024];
-
-  if (params_path == NULL) {
-    params_path = default_params_path;
-  }
+  const char* params_path = persistent_param ? persistent_params_path : default_params_path;
 
   // Build lock path, and open lockfile
   result = snprintf(path, sizeof(path), "%s/.lock", params_path);
   if (result < 0) {
     goto cleanup;
   }
-  lock_fd = open(path, 0);
+  lock_fd = open(path, O_CREAT);
 
   // Take lock.
   result = flock(lock_fd, LOCK_EX);
@@ -207,15 +266,11 @@ cleanup:
   return result;
 }
 
-int read_db_value(const char* params_path, const char* key, char** value,
-                  size_t* value_sz) {
+int read_db_value(const char* key, char** value, size_t* value_sz, bool persistent_param) {
   int lock_fd = -1;
   int result;
   char path[1024];
-
-  if (params_path == NULL) {
-    params_path = default_params_path;
-  }
+  const char* params_path = persistent_param ? persistent_params_path : default_params_path;
 
   result = snprintf(path, sizeof(path), "%s/.lock", params_path);
   if (result < 0) {
@@ -253,10 +308,9 @@ cleanup:
   return result;
 }
 
-void read_db_value_blocking(const char* params_path, const char* key,
-                            char** value, size_t* value_sz) {
+void read_db_value_blocking(const char* key, char** value, size_t* value_sz, bool persistent_param) {
   while (1) {
-    const int result = read_db_value(params_path, key, value, value_sz);
+    const int result = read_db_value(key, value, value_sz, persistent_param);
     if (result == 0) {
       return;
     } else {
@@ -266,12 +320,9 @@ void read_db_value_blocking(const char* params_path, const char* key,
   }
 }
 
-int read_db_all(const char* params_path, std::map<std::string, std::string> *params) {
+int read_db_all(std::map<std::string, std::string> *params, bool persistent_param) {
   int err = 0;
-
-  if (params_path == NULL) {
-    params_path = default_params_path;
-  }
+  const char* params_path = persistent_param ? persistent_params_path : default_params_path;
 
   std::string lock_path = util::string_format("%s/.lock", params_path);
 

selfdrive/common/params.h

@@ -9,30 +9,28 @@ extern "C" {
 
 #define ERR_NO_VALUE -33
 
-int write_db_value(const char* params_path, const char* key, const char* value,
-                   size_t value_size);
+int write_db_value(const char* key, const char* value, size_t value_size, bool persistent_param = false);
 
 // Reads a value from the params database.
 // Inputs:
-//  params_path: The path of the database, or NULL to use the default.
 //  key: The key to read.
 //  value: A pointer where a newly allocated string containing the db value will
 //         be written.
 //  value_sz: A pointer where the size of value will be written. Does not
 //            include the NULL terminator.
+//  persistent_param: Boolean indicating if the param store in the /persist partition is to be used.
+//                    e.g. for sensor calibration files. Will not be cleared after wipe or re-install.
 //
 // Returns: Negative on failure, otherwise 0.
-int read_db_value(const char* params_path, const char* key, char** value,
-                  size_t* value_sz);
+int read_db_value(const char* key, char** value, size_t* value_sz, bool persistent_param = false);
 
 // Delete a value from the params database.
 // Inputs are the same as read_db_value, without value and value_sz.
-int delete_db_value(const char* params_path, const char* key);
+int delete_db_value(const char* key, bool persistent_param = false);
 
 // Reads a value from the params database, blocking until successful.
 // Inputs are the same as read_db_value.
-void read_db_value_blocking(const char* params_path, const char* key,
-                            char** value, size_t* value_sz);
+void read_db_value_blocking(const char* key, char** value, size_t* value_sz, bool persistent_param = false);
 
 #ifdef __cplusplus
 }  // extern "C"
@@ -41,7 +39,7 @@ void read_db_value_blocking(const char* params_path, const char* key,
 #ifdef __cplusplus
 #include <map>
 #include <string>
-int read_db_all(const char* params_path, std::map<std::string, std::string> *params);
+int read_db_all(std::map<std::string, std::string> *params, bool persistent_param = false);
 #endif
 
 #endif  // _SELFDRIVE_COMMON_PARAMS_H_

selfdrive/locationd/paramsd.cc

@@ -51,7 +51,7 @@ int main(int argc, char *argv[]) {
 
   LOGW("waiting for params to set vehicle model");
   while (true) {
-    read_db_value(NULL, "CarParams", &value, &value_sz);
+    read_db_value("CarParams", &value, &value_sz);
     if (value_sz > 0) break;
     usleep(100*1000);
   }
@@ -66,7 +66,7 @@ int main(int argc, char *argv[]) {
   cereal::CarParams::Reader car_params = cmsg.getRoot<cereal::CarParams>();
 
   // Read params from previous run
-  const int result = read_db_value(NULL, "LiveParameters", &value, &value_sz);
+  const int result = read_db_value("LiveParameters", &value, &value_sz);
 
   std::string fingerprint = car_params.getCarFingerprint();
   std::string vin = car_params.getCarVin();
@@ -168,7 +168,7 @@ int main(int argc, char *argv[]) {
           std::string out = json.dump();
           std::async(std::launch::async,
                      [out]{
-                       write_db_value(NULL, "LiveParameters", out.c_str(), out.length());
+                       write_db_value("LiveParameters", out.c_str(), out.length());
                      });
         }
       }

selfdrive/loggerd/loggerd.cc

@@ -95,7 +95,7 @@ void encoder_thread(bool is_streaming, bool raw_clips, bool front) {
 
   if (front) {
     char *value;
-    const int result = read_db_value(NULL, "RecordFront", &value, NULL);
+    const int result = read_db_value("RecordFront", &value, NULL);
     if (result != 0) return;
     if (value[0] != '1') { free(value); return; }
     free(value);
@@ -459,32 +459,32 @@ kj::Array<capnp::word> gen_init_data() {
 
   char* git_commit = NULL;
   size_t size;
-  read_db_value(NULL, "GitCommit", &git_commit, &size);
+  read_db_value("GitCommit", &git_commit, &size);
   if (git_commit) {
     init.setGitCommit(capnp::Text::Reader(git_commit, size));
   }
 
   char* git_branch = NULL;
-  read_db_value(NULL, "GitBranch", &git_branch, &size);
+  read_db_value("GitBranch", &git_branch, &size);
   if (git_branch) {
     init.setGitBranch(capnp::Text::Reader(git_branch, size));
   }
 
   char* git_remote = NULL;
-  read_db_value(NULL, "GitRemote", &git_remote, &size);
+  read_db_value("GitRemote", &git_remote, &size);
   if (git_remote) {
     init.setGitRemote(capnp::Text::Reader(git_remote, size));
   }
 
   char* passive = NULL;
-  read_db_value(NULL, "Passive", &passive, NULL);
+  read_db_value("Passive", &passive, NULL);
   init.setPassive(passive && strlen(passive) && passive[0] == '1');
 
 
   {
     // log params
     std::map<std::string, std::string> params;
-    read_db_all(NULL, &params);
+    read_db_all(&params);
     auto lparams = init.initParams().initEntries(params.size());
     int i = 0;
     for (auto& kv : params) {

selfdrive/modeld/models/driving.cc

@@ -56,7 +56,7 @@ void model_init(ModelState* s, cl_device_id device_id, cl_context context, int t
   s->m->addTrafficConvention(s->traffic_convention, TRAFFIC_CONVENTION_LEN);
 
   char *string;
-  const int result = read_db_value(NULL, "IsRHD", &string, NULL);
+  const int result = read_db_value("IsRHD", &string, NULL);
   if (result == 0) {
     bool is_rhd = string[0] == '1';
     free(string);

selfdrive/ui/ui.cc

@@ -118,7 +118,7 @@ static void handle_sidebar_touch(UIState *s, int touch_x, int touch_y) {
 }
 
 static void handle_driver_view_touch(UIState *s, int touch_x, int touch_y) {
-  int err = write_db_value(NULL, "IsDriverViewEnabled", "0", 1);
+  int err = write_db_value("IsDriverViewEnabled", "0", 1);
 }
 
 static void handle_vision_touch(UIState *s, int touch_x, int touch_y) {
@@ -138,27 +138,28 @@ static void set_do_exit(int sig) {
   do_exit = 1;
 }
 
-static void read_param_bool(bool* param, const char* param_name) {
+static void read_param_bool(bool* param, const char* param_name, bool persistent_param = false) {
   char *s;
-  const int result = read_db_value(NULL, param_name, &s, NULL);
+  const int result = read_db_value(param_name, &s, NULL, persistent_param);
   if (result == 0) {
     *param = s[0] == '1';
     free(s);
   }
 }
 
-static void read_param_float(float* param, const char* param_name) {
+static int read_param_float(float* param, const char* param_name, bool persistent_param = false) {
   char *s;
-  const int result = read_db_value(NULL, param_name, &s, NULL);
+  const int result = read_db_value(param_name, &s, NULL, persistent_param);
   if (result == 0) {
     *param = strtod(s, NULL);
     free(s);
   }
+  return result;
 }
 
-static int read_param_uint64(uint64_t* dest, const char* param_name) {
+static int read_param_uint64(uint64_t* dest, const char* param_name, bool persistent_param = false) {
   char *s;
-  const int result = read_db_value(NULL, param_name, &s, NULL);
+  const int result = read_db_value(param_name, &s, NULL, persistent_param);
   if (result == 0) {
     *dest = strtoull(s, NULL, 0);
     free(s);
@@ -166,34 +167,40 @@ static int read_param_uint64(uint64_t* dest, const char* param_name) {
   return result;
 }
 
-static void read_param_bool_timeout(bool* param, const char* param_name, int* timeout) {
+static void read_param_bool_timeout(bool* param, const char* param_name, int* timeout, bool persistent_param = false) {
   if (*timeout > 0){
     (*timeout)--;
   } else {
-    read_param_bool(param, param_name);
+    read_param_bool(param, param_name, persistent_param);
     *timeout = 2 * UI_FREQ; // 0.5Hz
   }
 }
 
-static void read_param_float_timeout(float* param, const char* param_name, int* timeout) {
+static void read_param_float_timeout(float* param, const char* param_name, int* timeout, bool persistent_param = false) {
   if (*timeout > 0){
     (*timeout)--;
   } else {
-    read_param_float(param, param_name);
+    read_param_float(param, param_name, persistent_param);
     *timeout = 2 * UI_FREQ; // 0.5Hz
   }
 }
 
-static int read_param_uint64_timeout(uint64_t* dest, const char* param_name, int* timeout) {
+static int read_param_uint64_timeout(uint64_t* dest, const char* param_name, int* timeout, bool persistent_param = false) {
   if (*timeout > 0){
     (*timeout)--;
     return 0;
   } else {
-    return read_param_uint64(dest, param_name);
+    return read_param_uint64(dest, param_name, persistent_param);
     *timeout = 2 * UI_FREQ; // 0.5Hz
   }
 }
 
+static int write_param_float(float param, const char* param_name, bool persistent_param = false) {
+  char s[16];
+  int size = snprintf(s, sizeof(s), "%f", param);
+  return write_db_value(param_name, s, MIN(size, sizeof(s)), persistent_param);
+}
+
 static void update_offroad_layout_timeout(UIState *s, int* timeout) {
   if (*timeout > 0) {
     (*timeout)--;
@@ -885,10 +892,18 @@ int main(int argc, char* argv[]) {
   // light sensor scaling params
   const int LEON = is_leon();
 
-  const float BRIGHTNESS_B = LEON ? 10.0 : 5.0;
-  const float BRIGHTNESS_M = LEON ? 2.6 : 1.3;
+  float brightness_b, brightness_m;  
+  int result = read_param_float(&brightness_b, "BRIGHTNESS_B", true);
+  result += read_param_float(&brightness_m, "BRIGHTNESS_M", true);
+
+  if(result != 0){
+    brightness_b = LEON ? 10.0 : 5.0;
+    brightness_m = LEON ? 2.6 : 1.3;
+    write_param_float(brightness_b, "BRIGHTNESS_B", true);
+    write_param_float(brightness_m, "BRIGHTNESS_M", true);
+  }
 
-  float smooth_brightness = BRIGHTNESS_B;
+  float smooth_brightness = brightness_b;
 
   const int MIN_VOLUME = LEON ? 12 : 9;
   const int MAX_VOLUME = LEON ? 15 : 12;
@@ -912,7 +927,7 @@ int main(int argc, char* argv[]) {
     double u1 = millis_since_boot();
 
     // light sensor is only exposed on EONs
-    float clipped_brightness = (s->light_sensor*BRIGHTNESS_M) + BRIGHTNESS_B;
+    float clipped_brightness = (s->light_sensor*brightness_m) + brightness_b;
     if (clipped_brightness > 512) clipped_brightness = 512;
     smooth_brightness = clipped_brightness * 0.01 + smooth_brightness * 0.99;
     if (smooth_brightness > 255) smooth_brightness = 255;