fix(backlight): Poll if necessary

The `backlight` file seems to not receive modification events for
inotify (it does receive other events though).
These other events still trigger an update, but the value read is still
the old value because the events arrive just slightly before the file is
updated.

The new `poll-interval` setting will inject an event every X seconds to
force an update.
If the actual_brightness file is used, the interval is set to 0 and thus
turned off.

This does not add any more wakeups to the module, the inotify modules
wake up every second anyway and during that time, the interval is
checked.

Fixes #2835

CHANGELOG.md

@@ -32,7 +32,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `internal/pulseaudio`: `reverse-scroll` option ([`#2664`](https://github.com/polybar/polybar/pull/2664))
 - `custom/script`: Repeat interval for script failure (`interval-fail`) and `exec-if` (`interval-if`) ([`#943`](https://github.com/polybar/polybar/issues/943), [`#2606`](https://github.com/polybar/polybar/issues/2606), [`#2630`](https://github.com/polybar/polybar/pull/2630))
 - `custom/text`: Loads the `format` setting, which supports the `<label>` tag, if the deprecated `content` is not defined ([`#1331`](https://github.com/polybar/polybar/issues/1331), [`#2673`](https://github.com/polybar/polybar/pull/2673), [`#2676`](https://github.com/polybar/polybar/pull/2676))
-- `internal/backlight`: `scroll-interval` option ([`#2696`](https://github.com/polybar/polybar/issues/2696), [`#2700`](https://github.com/polybar/polybar/pull/2700))
+- `internal/backlight`:
+  - `scroll-interval` option ([`#2696`](https://github.com/polybar/polybar/issues/2696), [`#2700`](https://github.com/polybar/polybar/pull/2700))
+  - `poll-interval` setting controls how often the module is updated (in case it does not happen when the brightness changes) ([`#2835`](https://github.com/polybar/polybar/issues/2835), [`#3028`](https://github.com/polybar/polybar/pull/3028))
 - `internal/temperature`: Added `zone-type` setting ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2752`](https://github.com/polybar/polybar/pull/2752)) by [@xphoniex](https://github.com/xphoniex)
 - `internal/xwindow`: `%class%` and `%instance%` tokens, which show the contents of the `WM_CLASS` property of the active window ([`#2830`](https://github.com/polybar/polybar/pull/2830))
 - Added `enable-struts` option in bar section to enable/disable struts ([`#2769`](https://github.com/polybar/polybar/issues/2769), [`#2844`](https://github.com/polybar/polybar/pull/2844)) by [@VanillaViking](https://github.com/VanillaViking).
@@ -47,7 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `internal/fs`: Use `/` as a fallback if no mountpoints are specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2705`](https://github.com/polybar/polybar/pull/2705))
 - `internal/backlight`:
   - Detect backlight if none specified ([`#2572`](https://github.com/polybar/polybar/issues/2572), [`#2728`](https://github.com/polybar/polybar/pull/2728))
-  - `use-actual-brightness` defaults to `true` for `amdgpu` backlights ([`#2835`](https://github.com/polybar/polybar/issues/2835), [`2839`](https://github.com/polybar/polybar/pull/2839))
+  - `use-actual-brightness` now always defaults to `true` (even for `amdgpu` backlights) ([`#2835`](https://github.com/polybar/polybar/issues/2835), [`2839`](https://github.com/polybar/polybar/pull/2839))
 - Providing a negative min-width to a token adds right-padding ([`#2789`](https://github.com/polybar/polybar/issues/2789), [`#2801`](https://github.com/polybar/polybar/pull/2801)) by [@VanillaViking](https://github.com/VanillaViking).
 - Changed fuzzy match option on i3 and bspwm modules to find longest match instead of the first match ([`#2831`](https://github.com/polybar/polybar/pull/2831), [`#2829`](https://github.com/polybar/polybar/issues/2829)) by [@Ron0Studios](https://github.com/ron0studios/).
 - `wm-restack`
@@ -65,6 +67,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Also monitor include-files for changes when --reload is set ([`#675`](https://github.com/polybar/polybar/issues/675), [`#2759`](https://github.com/polybar/polybar/pull/2759))
 - `internal/xwindow`: module does not crash when a tag is not provided in format ([`#2826`](https://github.com/polybar/polybar/issues/2826), [`#2833`](https://github.com/polybar/polybar/pull/2833)) by [@VanillaViking](https://github.com/VanillaViking)
 - `internal/i3`: module errors when i3 has negative gaps ([`#2888`](https://github.com/polybar/polybar/issues/2888), [`#2889`](https://github.com/polybar/polybar/pull/2889))
+- `internal/backlight`: Fix module being one step behind every update ([`#2835`](https://github.com/polybar/polybar/issues/2835), [`#3028`](https://github.com/polybar/polybar/pull/3028))
 - `wm-restack = bspwm`: bar may become unclickable if there are overlapping monitors ([`#2873`](https://github.com/polybar/polybar/issues/2873), [`#2961`](https://github.com/polybar/polybar/pull/2961))
 
 ## [3.6.3] - 2022-05-04

include/modules/backlight.hpp

@@ -8,6 +8,19 @@
 POLYBAR_NS
 
 namespace modules {
+  /**
+   * Reads value from `/sys/class/backlight/` to get a brightness value for some device.
+   *
+   * There are two file providing brightness values: `brightness` and `actual_brightness`.
+   * The `actual_brightness` file is usually more reliable, but in some cases does not work (provides completely wrong
+   * values, doesn't update) depending on kernel version, graphics driver, and/or graphics card.
+   * Which file is used is controlled by the use-actual-brightness setting.
+   *
+   * The general issue with the `brightness` file seems to be that, while it does receive inotify events, the events it
+   * receives are not for modification of the file and arrive just before the file is updated with a new value. The module
+   * thus reads and displays an outdated brightness value. To compensate for this, the module periodically (controlled by
+   * `poll-interval`) forces an update. By default, this is only enabled if the `backlight` file is used.
+   */
   class backlight_module : public inotify_module<backlight_module> {
    public:
     struct brightness_handle {
@@ -20,7 +33,6 @@ namespace modules {
 
     string get_output();
 
-   public:
     explicit backlight_module(const bar_settings&, string, const config&);
 
     void idle();
@@ -47,7 +59,7 @@ namespace modules {
     label_t m_label;
     progressbar_t m_progressbar;
     string m_path_backlight;
-    float m_max_brightness;
+    float m_max_brightness{};
     bool m_scroll{false};
     int m_scroll_interval{5};
     bool m_use_actual_brightness{true};
@@ -56,6 +68,9 @@ namespace modules {
     brightness_handle m_max;
 
     int m_percentage = 0;
+
+    chrono::duration<double> m_interval{};
+    chrono::steady_clock::time_point m_lastpoll;
   };
 } // namespace modules
 

src/modules/backlight.cpp

@@ -72,6 +72,9 @@ namespace modules {
 
     m_use_actual_brightness = m_conf.get(name(), "use-actual-brightness", m_use_actual_brightness);
 
+    m_interval = m_conf.get<decltype(m_interval)>(name(), "poll-interval", m_use_actual_brightness? 0s : 5s);
+    m_lastpoll = chrono::steady_clock::now();
+
     std::string brightness_type = (m_use_actual_brightness ? "actual_brightness" : "brightness");
     auto path_backlight_val = m_path_backlight + "/" + brightness_type;
 
@@ -83,16 +86,38 @@ namespace modules {
   }
 
   void backlight_module::idle() {
-    sleep(75ms);
+    if (m_interval.count() > 0) {
+      /*
+       * Update module after interval (if any).
+       * We don't always receive inotify events for the backlight files.
+       * Mainly, the `backlight` file never receive an event on modification.
+       * In that case, updating at an interval is the only way to get the new value.
+       */
+      auto now = chrono::steady_clock::now();
+      if (chrono::duration_cast<decltype(m_interval)>(now - m_lastpoll) > m_interval) {
+        m_lastpoll = now;
+        if (on_event({})) {
+          broadcast();
+        }
+      }
+    }
+
+    this->inotify_module::idle();
   }
 
   bool backlight_module::on_event(const inotify_event& event) {
     if (event.is_valid) {
-      m_log.trace("%s: %s", name(), event.filename);
+      m_log.trace("%s: on_event{filename: %s, is_dir: %s, wd: %d, cookie: %d, mask: 0x%x}", name(), event.filename,
+          event.is_dir? "true" : "false", event.wd, event.cookie, event.mask);
     }
 
     m_max_brightness = m_max.read();
-    m_percentage = static_cast<int>(m_val.read() / m_max_brightness * 100.0f + 0.5f);
+    float val = m_val.read();
+    int percentage = math_util::percentage(val, m_max_brightness);
+
+    if (m_percentage != percentage) {
+      m_log.trace("%s: %d%% -> %d%% (val: %f, max: %f)", name(), m_percentage, percentage, val, m_max_brightness);
+      m_percentage = percentage;
 
       if (m_label) {
         m_label->reset_tokens();
@@ -102,6 +127,9 @@ namespace modules {
       return true;
     }
 
+    return false;
+  }
+
   string backlight_module::get_output() {
     // Get the module output early so that
     // the format prefix/suffix also gets wrapped