Added 2 new options for BACKLIGHT and DIMMER: 'trans_fixed'.

New options let users set a fixed smooth transitions duration; the new options are off by default for now,
but are most probably going to be enabled by default as I found that having a fixed transition duration (eg: 1000ms)
for any backlight-related update enables a much better (and smooth) user experience.

In the event that the new options will be enabled by default, it will be obviously stated in the release notes.
For now, I don't really want to break existing configurations (as 'trans_step' and 'trans_timeout' become useless when 'trans_fixed' is enabled.

Refs #233

Signed-off-by: Federico Di Pierro <[email protected]>

Extra/clight.conf

@@ -64,6 +64,10 @@ backlight:
 
     ## Transition timeout in ms
     # trans_timeout = 30;
+
+    ## When > 0, use a fixed transition duration (in ms),
+    ## overriding trans_timeout and trans_step configs.
+    # trans_fixed = 1000;
 
     ## Timeouts between captures during day/night/event on AC
     ## Set any of these to <= 0 to disable captures
@@ -285,6 +289,10 @@ dimmer:
     ## Transition timeout in ms.
     ## Entering dimmed state fade, and leaving fade
     # trans_timeouts = [ 30, 30 ];
+
+    ## When > 0, use a fixed transition duration (in ms), 
+    ## for enter or leave, overriding trans_timeouts and trans_steps configs.
+    # trans_fixed = [ 1000, 1000 ];
 
     ## Timeouts on AC/on BATT. 
     ## Set any of these to <= 0 to disable dimmer

TODO.md

@@ -7,6 +7,7 @@
 - [x] Always start DISPLAY module
 - [x] Only hook logind.IdleHint when DIMMER is disabled (otherwise it is a secondary source for DIMMED state...useless)
 - [x] If DIMMER is disabled but we receive an IdleHint, do not touch backlight; just manage the new DIMMED state eventually pausing backlight and other modules
+- [x] Allow to set a fixed transition duration for BACKLIGHT and DIMMER
 
 ### Interface
 - [x] Fixed null ptr dereference in inhibit_parse_msg()

src/commons.h

@@ -35,6 +35,13 @@ typedef struct {
     double fit_parameters[DEGREE]; // best-fit parameters
 } curve_t;
 
+typedef struct {
+    int no_smooth;                          // disable smooth backlight changes for module
+    double trans_step;                      // every backlight transition step value (in pct), used when smooth BACKLIGHT transitions are enabled
+    int trans_timeout;                      // every backlight transition timeout value, used when smooth BACKLIGHT transitions are enabled
+    int trans_fixed;                        // fixed transitions duration
+} bl_smooth_t;
+
 typedef struct {
     int num_captures[SIZE_AC];
     char *dev_name;
@@ -46,9 +53,7 @@ typedef struct {
 typedef struct {
     int disabled;
     int timeout[SIZE_AC][SIZE_STATES + 1];
-    int no_smooth;                          // disable smooth backlight changes for BACKLIGHT module
-    double trans_step;                      // every backlight transition step value (in pct), used when smooth BACKLIGHT transitions are enabled
-    int trans_timeout;                      // every backlight transition timeout value, used when smooth BACKLIGHT transitions are enabled
+    bl_smooth_t smooth;
     int no_auto_calib;                      // disable automatic calibration for both BACKLIGHT and GAMMA
     double shutter_threshold;               // capture values below this threshold will be considered "shuttered"
     int pause_on_lid_closed;                // whether clight should inhibit autocalibration on lid closed
@@ -85,9 +90,7 @@ typedef struct {
     int disabled;
     double dimmed_pct;                      // pct of max backlight to be used while dimming
     int timeout[SIZE_AC];                   // dimmer timeout
-    int no_smooth[SIZE_DIM];                // disable smooth backlight changes for DIMMER module
-    double trans_step[SIZE_DIM];            // every backlight transition step value (in pct), used when smooth DIMMER transitions are enabled
-    int trans_timeout[SIZE_DIM];            // every backlight transition timeout value, used when smooth DIMMER transitions are enabled
+    bl_smooth_t smooth[SIZE_DIM];
 } dimmer_conf_t;
 
 typedef struct {

src/conf/config.c

@@ -48,9 +48,10 @@ static void load_backlight_settings(config_t *cfg, bl_conf_t *bl_conf) {
     if (bl) {
         config_setting_lookup_bool(bl, "disabled", &bl_conf->disabled);
         config_setting_lookup_bool(bl, "restore_on_exit", &bl_conf->restore);
-        config_setting_lookup_bool(bl, "no_smooth_transition", &bl_conf->no_smooth);
-        config_setting_lookup_float(bl, "trans_step", &bl_conf->trans_step);
-        config_setting_lookup_int(bl, "trans_timeout", &bl_conf->trans_timeout);
+        config_setting_lookup_bool(bl, "no_smooth_transition", &bl_conf->smooth.no_smooth);
+        config_setting_lookup_float(bl, "trans_step", &bl_conf->smooth.trans_step);
+        config_setting_lookup_int(bl, "trans_timeout", &bl_conf->smooth.trans_timeout);
+        config_setting_lookup_int(bl, "trans_fixed", &bl_conf->smooth.trans_fixed);
         config_setting_lookup_float(bl, "shutter_threshold", &bl_conf->shutter_threshold);
         config_setting_lookup_bool(bl, "no_auto_calibration", &bl_conf->no_auto_calib);
         config_setting_lookup_bool(bl, "pause_on_lid_closed", &bl_conf->pause_on_lid_closed);
@@ -289,13 +290,13 @@ static void load_dimmer_settings(config_t *cfg, dimmer_conf_t *dim_conf) {
     if (dim) {
         config_setting_lookup_bool(dim, "disabled", &dim_conf->disabled);
         config_setting_lookup_float(dim, "dimmed_pct", &dim_conf->dimmed_pct);
-
+         
         config_setting_t *points, *timeouts;
         /* Load no_smooth_dimmer options */
         if ((points = config_setting_get_member(dim, "no_smooth_transition"))) {
             if (config_setting_length(points) == SIZE_DIM) {
                 for (int i = 0; i < SIZE_DIM; i++) {
-                    dim_conf->no_smooth[i] = config_setting_get_float_elem(points, i);
+                    dim_conf->smooth[i].no_smooth = config_setting_get_float_elem(points, i);
                 }
             } else {
                 WARN("Wrong number of dimmer 'no_smooth_transition' array elements.\n");
@@ -306,7 +307,7 @@ static void load_dimmer_settings(config_t *cfg, dimmer_conf_t *dim_conf) {
         if ((points = config_setting_get_member(dim, "trans_steps"))) {
             if (config_setting_length(points) == SIZE_DIM) {
                 for (int i = 0; i < SIZE_DIM; i++) {
-                    dim_conf->trans_step[i] = config_setting_get_float_elem(points, i);
+                    dim_conf->smooth[i].trans_step = config_setting_get_float_elem(points, i);
                 }
             } else {
                 WARN("Wrong number of dimmer 'trans_steps' array elements.\n");
@@ -317,13 +318,24 @@ static void load_dimmer_settings(config_t *cfg, dimmer_conf_t *dim_conf) {
         if ((points = config_setting_get_member(dim, "trans_timeouts"))) {
             if (config_setting_length(points) == SIZE_DIM) {
                 for (int i = 0; i < SIZE_DIM; i++) {
-                    dim_conf->trans_timeout[i] = config_setting_get_int_elem(points, i);
+                    dim_conf->smooth[i].trans_timeout = config_setting_get_int_elem(points, i);
                 }
             } else {
                 WARN("Wrong number of dimmer 'trans_timeouts' array elements.\n");
             }
         }
 
+        /* Load dimmer_trans_timeouts options */
+        if ((points = config_setting_get_member(dim, "trans_fixed"))) {
+            if (config_setting_length(points) == SIZE_DIM) {
+                for (int i = 0; i < SIZE_DIM; i++) {
+                    dim_conf->smooth[i].trans_fixed = config_setting_get_int_elem(points, i);
+                }
+            } else {
+                WARN("Wrong number of dimmer 'trans_fixed' array elements.\n");
+            }
+        }
+
         /* Load dimmer timeouts */
         if ((timeouts = config_setting_get_member(dim, "timeouts"))) {
             if (config_setting_length(timeouts) == SIZE_AC) {
@@ -434,13 +446,16 @@ static void store_backlight_settings(config_t *cfg, bl_conf_t *bl_conf) {
     config_setting_set_bool(setting, bl_conf->restore);
 
     setting = config_setting_add(bl, "no_smooth_transition", CONFIG_TYPE_BOOL);
-    config_setting_set_bool(setting, bl_conf->no_smooth);
+    config_setting_set_bool(setting, bl_conf->smooth.no_smooth);
 
     setting = config_setting_add(bl, "trans_step", CONFIG_TYPE_FLOAT);
-    config_setting_set_float(setting, bl_conf->trans_step);
+    config_setting_set_float(setting, bl_conf->smooth.trans_step);
 
     setting = config_setting_add(bl, "trans_timeout", CONFIG_TYPE_INT);
-    config_setting_set_int(setting, bl_conf->trans_timeout);
+    config_setting_set_int(setting, bl_conf->smooth.trans_timeout);
+
+    setting = config_setting_add(bl, "trans_fixed", CONFIG_TYPE_INT);
+    config_setting_set_int(setting, bl_conf->smooth.trans_fixed);
 
     setting = config_setting_add(bl, "no_auto_calibration", CONFIG_TYPE_BOOL);
     config_setting_set_bool(setting, bl_conf->no_auto_calib);
@@ -609,17 +624,22 @@ static void store_dimmer_settings(config_t *cfg, dimmer_conf_t *dim_conf) {
 
     setting = config_setting_add(dimmer, "no_smooth_transition", CONFIG_TYPE_ARRAY);
     for (int i = 0; i < SIZE_DIM; i++) {
-        config_setting_set_bool_elem(setting, -1, dim_conf->no_smooth[i]);
+        config_setting_set_bool_elem(setting, -1, dim_conf->smooth[i].no_smooth);
     }
 
     setting = config_setting_add(dimmer, "trans_steps", CONFIG_TYPE_ARRAY);
     for (int i = 0; i < SIZE_DIM; i++) {
-        config_setting_set_float_elem(setting, -1, dim_conf->trans_step[i]);
+        config_setting_set_float_elem(setting, -1, dim_conf->smooth[i].trans_step);
     }
 
     setting = config_setting_add(dimmer, "trans_timeouts", CONFIG_TYPE_ARRAY);
     for (int i = 0; i < SIZE_DIM; i++) {
-        config_setting_set_int_elem(setting, -1, dim_conf->trans_timeout[i]);
+        config_setting_set_int_elem(setting, -1, dim_conf->smooth[i].trans_timeout);
+    }
+
+    setting = config_setting_add(dimmer, "trans_fixed", CONFIG_TYPE_ARRAY);
+    for (int i = 0; i < SIZE_DIM; i++) {
+        config_setting_set_int_elem(setting, -1, dim_conf->smooth[i].trans_fixed);
     }
 
     setting = config_setting_add(dimmer, "dimmed_pct", CONFIG_TYPE_FLOAT);

src/conf/opts.c

@@ -42,8 +42,8 @@ static void init_backlight_opts(bl_conf_t *bl_conf) {
     bl_conf->timeout[ON_BATTERY][DAY] = 2 * conf.bl_conf.timeout[ON_AC][DAY];
     bl_conf->timeout[ON_BATTERY][NIGHT] = 2 * conf.bl_conf.timeout[ON_AC][NIGHT];
     bl_conf->timeout[ON_BATTERY][IN_EVENT] = 2 * conf.bl_conf.timeout[ON_AC][IN_EVENT];
-    bl_conf->trans_step = 0.05;
-    bl_conf->trans_timeout = 30;
+    bl_conf->smooth.trans_step = 0.05;
+    bl_conf->smooth.trans_timeout = 30;
 }
 
 static void init_sens_opts(sensor_conf_t *sens_conf) {
@@ -110,10 +110,10 @@ static void init_dimmer_opts(dimmer_conf_t *dim_conf) {
     dim_conf->timeout[ON_AC] = 45;
     dim_conf->timeout[ON_BATTERY] = 20;
     dim_conf->dimmed_pct = 0.2;
-    dim_conf->trans_step[ENTER] = 0.05;
-    dim_conf->trans_step[EXIT] = 0.05;
-    dim_conf->trans_timeout[ENTER] = 30;
-    dim_conf->trans_timeout[EXIT] = 30;
+    dim_conf->smooth[ENTER].trans_step = 0.05;
+    dim_conf->smooth[EXIT].trans_step = 0.05;
+    dim_conf->smooth[ENTER].trans_timeout = 30;
+    dim_conf->smooth[EXIT].trans_timeout = 30;
 }
 
 static void init_dpms_opts(dpms_conf_t *dpms_conf) {
@@ -170,10 +170,10 @@ static void parse_cmd(int argc, char *const argv[], char *conf_file, size_t size
     const struct poptOption po[] = {
         {"frames", 'f', POPT_ARG_INT, NULL, 5, "Frames taken for each capture, Between 1 and 20", NULL},
         {"device", 'd', POPT_ARG_STRING, &conf.sens_conf.dev_name, 100, "Path to sensor device. If empty, first matching device is used", "video0"},
-        {"no-backlight-smooth", 0, POPT_ARG_NONE, &conf.bl_conf.no_smooth, 100, "Disable smooth backlight transitions", NULL},
+        {"no-backlight-smooth", 0, POPT_ARG_NONE, &conf.bl_conf.smooth.no_smooth, 100, "Disable smooth backlight transitions", NULL},
         {"no-gamma-smooth", 0, POPT_ARG_NONE, &conf.gamma_conf.no_smooth, 100, "Disable smooth gamma transitions", NULL},
-        {"no-dimmer-smooth-enter", 0, POPT_ARG_NONE, &conf.dim_conf.no_smooth[ENTER], 100, "Disable smooth dimmer transitions while entering dimmed state", NULL},
-        {"no-dimmer-smooth-exit", 0, POPT_ARG_NONE, &conf.dim_conf.no_smooth[EXIT], 100, "Disable smooth dimmer transitions while leaving dimmed state", NULL},
+        {"no-dimmer-smooth-enter", 0, POPT_ARG_NONE, &conf.dim_conf.smooth[ENTER].no_smooth, 100, "Disable smooth dimmer transitions while entering dimmed state", NULL},
+        {"no-dimmer-smooth-exit", 0, POPT_ARG_NONE, &conf.dim_conf.smooth[EXIT].no_smooth, 100, "Disable smooth dimmer transitions while leaving dimmed state", NULL},
         {"day-temp", 0, POPT_ARG_INT | POPT_ARGFLAG_SHOW_DEFAULT, &conf.gamma_conf.temp[DAY], 100, "Daily gamma temperature, between 1000 and 10000", NULL},
         {"night-temp", 0, POPT_ARG_INT | POPT_ARGFLAG_SHOW_DEFAULT, &conf.gamma_conf.temp[NIGHT], 100, "Nightly gamma temperature, between 1000 and 10000", NULL},
         {"lat", 0, POPT_ARG_DOUBLE, &conf.day_conf.loc.lat, 100, "Your desired latitude", NULL},
@@ -283,14 +283,14 @@ static void check_clightd_features(void) {
 }
 
 static void check_bl_conf(bl_conf_t *bl_conf) {
-    if (bl_conf->trans_step <= 0.0f || bl_conf->trans_step >= 1.0f) {
+    if (bl_conf->smooth.trans_step <= 0.0f || bl_conf->smooth.trans_step >= 1.0f) {
         WARN("Wrong backlight_trans_step value. Resetting default value.\n");
-        bl_conf->trans_step = 0.05;
+        bl_conf->smooth.trans_step = 0.05;
     }
 
-    if (bl_conf->trans_timeout <= 0) {
+    if (bl_conf->smooth.trans_timeout <= 0) {
         WARN("Wrong backlight_trans_timeout value. Resetting default value.\n");
-        bl_conf->trans_timeout = 30;
+        bl_conf->smooth.trans_timeout = 30;
     }
 
     if (bl_conf->shutter_threshold < 0 || bl_conf->shutter_threshold >= 1) {
@@ -409,22 +409,22 @@ static void check_dim_conf(dimmer_conf_t *dim_conf) {
         dim_conf->dimmed_pct = 0.2;
     }
 
-    if (dim_conf->trans_step[ENTER] <= 0.0f || dim_conf->trans_step[ENTER] >= 1.0f) {
+    if (dim_conf->smooth[ENTER].trans_step <= 0.0f || dim_conf->smooth[ENTER].trans_step >= 1.0f) {
         WARN("Wrong dimmer_trans_step[ENTER] value. Resetting default value.\n");
-        dim_conf->trans_step[ENTER] = 0.05;
+        dim_conf->smooth[ENTER].trans_step = 0.05;
     }
-    if (dim_conf->trans_step[EXIT] <= 0.0f || dim_conf->trans_step[EXIT] >= 1.0f) {
+    if (dim_conf->smooth[EXIT].trans_step <= 0.0f || dim_conf->smooth[EXIT].trans_step >= 1.0f) {
         WARN("Wrong dimmer_trans_step[EXIT] value. Resetting default value.\n");
-        dim_conf->trans_step[EXIT] = 0.05;
+        dim_conf->smooth[EXIT].trans_step = 0.05;
     }
 
-    if (dim_conf->trans_timeout[ENTER] <= 0) {
+    if (dim_conf->smooth[ENTER].trans_timeout <= 0) {
         WARN("Wrong dimmer_trans_timeout[ENTER] value. Resetting default value.\n");
-        dim_conf->trans_timeout[ENTER] = 30;
+        dim_conf->smooth[ENTER].trans_timeout = 30;
     }
-    if (dim_conf->trans_timeout[EXIT] <= 0) {
+    if (dim_conf->smooth[EXIT].trans_timeout <= 0) {
         WARN("Wrong dimmer_trans_timeout[EXIT] value. Resetting default value.\n");
-        dim_conf->trans_timeout[EXIT] = 30;
+        dim_conf->smooth[EXIT].trans_timeout = 30;
     }
 }
 

src/modules/backlight.c

@@ -46,9 +46,10 @@ static const sd_bus_vtable conf_bl_vtable[] = {
     SD_BUS_WRITABLE_PROPERTY("NoAutoCalib", "b", NULL, set_auto_calib, offsetof(bl_conf_t, no_auto_calib), 0),
     SD_BUS_WRITABLE_PROPERTY("InhibitOnLidClosed", "b", NULL, NULL, offsetof(bl_conf_t, pause_on_lid_closed), 0),
     SD_BUS_WRITABLE_PROPERTY("CaptureOnLidOpened", "b", NULL, NULL, offsetof(bl_conf_t, capture_on_lid_opened), 0),
-    SD_BUS_WRITABLE_PROPERTY("NoSmooth", "b", NULL, NULL, offsetof(bl_conf_t, no_smooth), 0),
-    SD_BUS_WRITABLE_PROPERTY("TransStep", "d", NULL, NULL, offsetof(bl_conf_t, trans_step), 0),
-    SD_BUS_WRITABLE_PROPERTY("TransDuration", "i", NULL, NULL, offsetof(bl_conf_t, trans_timeout), 0),
+    SD_BUS_WRITABLE_PROPERTY("NoSmooth", "b", NULL, NULL, offsetof(bl_conf_t, smooth.no_smooth), 0),
+    SD_BUS_WRITABLE_PROPERTY("TransStep", "d", NULL, NULL, offsetof(bl_conf_t, smooth.trans_step), 0),
+    SD_BUS_WRITABLE_PROPERTY("TransDuration", "i", NULL, NULL, offsetof(bl_conf_t, smooth.trans_timeout), 0),
+    SD_BUS_WRITABLE_PROPERTY("TransFixed", "i", NULL, NULL, offsetof(bl_conf_t, smooth.trans_fixed), 0),
     SD_BUS_WRITABLE_PROPERTY("ShutterThreshold", "d", NULL, NULL, offsetof(bl_conf_t, shutter_threshold), 0),
     SD_BUS_WRITABLE_PROPERTY("AcDayTimeout", "i", NULL, set_timeouts, offsetof(bl_conf_t, timeout[ON_AC][DAY]), 0),
     SD_BUS_WRITABLE_PROPERTY("AcNightTimeout", "i", NULL, set_timeouts, offsetof(bl_conf_t, timeout[ON_AC][NIGHT]), 0),
@@ -82,6 +83,7 @@ DECLARE_MSG(amb_msg, AMBIENT_BR_UPD);
 DECLARE_MSG(capture_req, CAPTURE_REQ);
 DECLARE_MSG(sens_msg, SENS_UPD);
 DECLARE_MSG(calib_req, NO_AUTOCALIB_REQ);
+DECLARE_MSG(bl_req, BL_REQ);
 
 API(Backlight, conf_bl_vtable, conf.bl_conf);
 API(Sensor, conf_sens_vtable, conf.sens_conf);
@@ -92,6 +94,7 @@ static void init(void) {
     bls = map_new(true, free);
     capture_req.capture.reset_timer = true;
     capture_req.capture.capture_only = false;
+    bl_req.bl.smooth = -1; // Use conf values
 
     // Disabled while in wizard mode as it is useless and spams to stdout
     if (!conf.wizard) {
@@ -213,7 +216,7 @@ static void receive_waiting_init(const msg_t *const msg, UNUSED const void* user
              * Note: set smooth field from conf to allow keyboard and gamma to react;
              * > if (up->smooth || conf.bl_conf.no_smooth) { ... }
              */
-            set_backlight_level(1.0, !conf.bl_conf.no_smooth, 0, 0);
+            set_backlight_level(1.0, !conf.bl_conf.smooth.no_smooth, 0, 0);
             pause_mod(AUTOCALIB);
         }
         if (state.lid_state) {
@@ -468,10 +471,9 @@ static void do_capture(bool reset_timer, bool capture_only) {
 static double set_new_backlight(const double perc) {
     sensor_conf_t *sens_conf = &conf.sens_conf;
     enum ac_states st = state.ac_state;
-    const double new_bl_pct = get_value_from_curve(perc, &sens_conf->default_curve[st]);
-    set_backlight_level(new_bl_pct, !conf.bl_conf.no_smooth, 
-                        conf.bl_conf.trans_step, conf.bl_conf.trans_timeout);
-    return new_bl_pct;
+    bl_req.bl.new = get_value_from_curve(perc, &sens_conf->default_curve[st]);
+    M_PUB(&bl_req);
+    return bl_req.bl.new;
 }
 
 static void publish_bl_upd(const double pct, const bool is_smooth, const double step, const int timeout) {