Merge pull request #5167 from Alexays/fix/bug-batch

fix: backlight leds subsystem (#2848), sway/ipc reconnect (#3166), ethernet unplug regression (#4364)

include/modules/network.hpp

@@ -51,6 +51,7 @@ class Network : public ALabel {
   bool matchInterface(const std::string& ifname, const std::vector<std::string>& altnames,
                       std::string& matched) const;
   auto getInfo() -> void;
+  bool isWireless() const;
   const std::string getNetworkState() const;
   void clearIface();
   std::optional<std::pair<unsigned long long, unsigned long long>> readBandwidthUsage();

include/modules/sway/ipc/client.hpp

@@ -2,10 +2,12 @@
 
 #include <sigc++/sigc++.h>
 
+#include <atomic>
 #include <cstdint>
 #include <functional>
 #include <mutex>
 #include <string>
+#include <vector>
 
 #include "ipc.hpp"
 #include "util/SafeSignal.hpp"
@@ -43,6 +45,14 @@ class Ipc {
   struct ipc_response send(int fd, uint32_t type, const std::string& payload = "");
   struct ipc_response recv(int fd);
 
+  // Re-establish the event socket and re-subscribe after sway drops us, backing
+  // off between attempts so we don't busy-loop while sway is unavailable.
+  void reconnectEvent();
+
+  std::string socketPath_;
+  std::vector<std::string> subscribed_events_;
+  std::atomic<bool> running_{true};
+
   util::ScopedFd fd_;
   util::ScopedFd fd_event_;
   std::mutex mutex_;

include/util/backlight_backend.hpp

@@ -27,9 +27,11 @@ namespace waybar::util {
 class BacklightDevice {
  public:
   BacklightDevice() = default;
-  BacklightDevice(std::string name, int actual, int max, bool powered);
+  BacklightDevice(std::string name, int actual, int max, bool powered,
+                  std::string subsystem = "backlight");
 
   std::string name() const;
+  std::string subsystem() const;
   int get_actual() const;
   void set_actual(int actual);
   int get_max() const;
@@ -45,6 +47,7 @@ class BacklightDevice {
   int actual_ = 1;
   int max_ = 1;
   bool powered_ = true;
+  std::string subsystem_ = "backlight";
 };
 
 class BacklightBackend {
@@ -70,7 +73,8 @@ class BacklightBackend {
   std::mutex udev_thread_mutex_;
 
  private:
-  void set_brightness_internal(const std::string& device_name, int brightness, int max_brightness);
+  void set_brightness_internal(const std::string& device_name, int brightness, int max_brightness,
+                               const std::string& subsystem = "backlight");
 
   std::function<void()> on_updated_cb_;
   std::chrono::milliseconds polling_interval_;

man/waybar-backlight-slider.5.scd

@@ -29,7 +29,8 @@ The brightness can be controlled by dragging the slider across the bar or clicki
 
 *device*: ++
     typeof: string ++
-    The name of the preferred device to control. If left empty, a device will be chosen automatically.
+    The name of the preferred device to control. If left empty, a device will be chosen automatically. ++
+    Both screen backlights (the udev *backlight* subsystem) and keyboard backlights (LEDs in the udev *leds* subsystem, e.g. *white:kbd_backlight*) are supported; name such an LED here to control it. When left empty, a screen backlight is always preferred for automatic selection.
 
 *interval*: ++
     typeof: uint ++

man/waybar-backlight.5.scd

@@ -17,7 +17,8 @@ The *backlight* module displays the current backlight level.
 
 *device*: ++
 	typeof: string ++
-	The name of the preferred backlight device to display. If left empty, a device will be chosen automatically.
+	The name of the preferred backlight device to display. If left empty, a device will be chosen automatically. ++
+	Both screen backlights (the udev *backlight* subsystem) and keyboard backlights (LEDs in the udev *leds* subsystem, e.g. *white:kbd_backlight*) are supported; name such an LED here to control it. When left empty, a screen backlight is always preferred for automatic selection.
 
 *format*: ++
 	typeof: string ++

src/modules/network.cpp

@@ -8,6 +8,7 @@
 
 #include <cassert>
 #include <cstring>
+#include <filesystem>
 #include <fstream>
 #include <optional>
 #include <sstream>
@@ -288,6 +289,19 @@ void waybar::modules::Network::worker() {
   };
 }
 
+bool waybar::modules::Network::isWireless() const {
+  // The rfkill switch we monitor (and thus the "disabled" state) only applies
+  // to wireless radios. An interface is wireless if the kernel exposes an
+  // 802.11 phy (cfg80211/mac80211) or a legacy "wireless" node for it in sysfs.
+  if (ifname_.empty()) {
+    return false;
+  }
+  const auto base = "/sys/class/net/" + ifname_;
+  std::error_code ec;
+  return std::filesystem::exists(base + "/phy80211", ec) ||
+         std::filesystem::exists(base + "/wireless", ec);
+}
+
 const std::string waybar::modules::Network::getNetworkState() const {
   if (ifid_ == -1 || !carrier_) {
 #ifdef WANT_RFKILL
@@ -295,7 +309,14 @@ const std::string waybar::modules::Network::getNetworkState() const {
     if (config_["rfkill"].isBool()) {
       display_rfkill = config_["rfkill"].asBool();
     }
-    if (rfkill_.getState() && display_rfkill) return "disabled";
+    // The rfkill switch is for wireless (WLAN) radios only, so it must not mask
+    // a wired interface that merely lost its carrier (e.g. an unplugged ethernet
+    // cable): such an interface has to report "disconnected", not "disabled",
+    // otherwise cable-unplug detection breaks on ethernet modules (#4364).
+    // Only honor rfkill when there is no interface or the interface is wireless.
+    if (rfkill_.getState() && display_rfkill && (ifname_.empty() || isWireless())) {
+      return "disabled";
+    }
 #endif
     return "disconnected";
   }

src/modules/sway/ipc/client.cpp

@@ -8,12 +8,14 @@
 #include <unistd.h>
 
 #include <cerrno>
+#include <chrono>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <limits>
 #include <stdexcept>
 #include <string_view>
+#include <thread>
 #include <utility>
 
 #include "modules/sway/ipc/ipc.hpp"
@@ -41,12 +43,15 @@ void sendAll(int fd, const char* data, size_t size, const char* what) {
 }  // namespace
 
 Ipc::Ipc() {
-  const std::string socketPath = getSocketPath();
-  fd_ = util::ScopedFd(open(socketPath));
-  fd_event_ = util::ScopedFd(open(socketPath));
+  socketPath_ = getSocketPath();
+  fd_ = util::ScopedFd(open(socketPath_));
+  fd_event_ = util::ScopedFd(open(socketPath_));
 }
 
 Ipc::~Ipc() {
+  // Signal the worker before stopping it so an in-flight recv/reconnect bails
+  // out instead of trying to reconnect to a socket we're tearing down.
+  running_ = false;
   thread_.stop();
 
   if (fd_ > 0) {
@@ -191,11 +196,48 @@ void Ipc::subscribe(const std::string& payload) {
   if (res.payload != "{\"success\": true}") {
     throw std::runtime_error("Unable to subscribe ipc event");
   }
+  // Remember the subscription so we can replay it if we have to reconnect.
+  subscribed_events_.push_back(payload);
+}
+
+void Ipc::reconnectEvent() {
+  // Sway closed our event connection (typically because its send buffer filled
+  // up during an event flood). Re-establish the socket and re-subscribe to the
+  // same events, backing off between attempts so we don't busy-loop and peg a
+  // CPU while sway is unavailable or keeps dropping us.
+  while (running_) {
+    std::this_thread::sleep_for(std::chrono::seconds(2));
+    if (!running_) {
+      return;
+    }
+    try {
+      fd_event_.reset(open(socketPath_));
+      for (const auto& payload : subscribed_events_) {
+        const auto res = Ipc::send(fd_event_, IPC_SUBSCRIBE, payload);
+        if (res.payload != "{\"success\": true}") {
+          throw std::runtime_error("Unable to re-subscribe ipc event");
+        }
+      }
+      spdlog::info("Reconnected to sway IPC event socket");
+      return;
+    } catch (const std::exception& e) {
+      spdlog::warn("Failed to reconnect to sway IPC ({}), retrying", e.what());
+    }
+  }
 }
 
 void Ipc::handleEvent() {
-  const auto res = Ipc::recv(fd_event_);
-  signal_event.emit(res);
+  try {
+    const auto res = Ipc::recv(fd_event_);
+    signal_event.emit(res);
+  } catch (const std::exception& e) {
+    if (!running_) {
+      // The Ipc is being torn down; the socket was closed on purpose.
+      return;
+    }
+    spdlog::warn("Lost sway IPC event connection ({}), reconnecting", e.what());
+    reconnectEvent();
+  }
 }
 
 }  // namespace waybar::modules::sway

src/util/backlight_backend.cpp

@@ -73,6 +73,7 @@ static void upsert_device(std::vector<BacklightDevice>& devices, udev_device* de
   const char* actual = udev_device_get_sysattr_value(dev, actual_brightness_attr);
   const char* max = udev_device_get_sysattr_value(dev, "max_brightness");
   const char* power = udev_device_get_sysattr_value(dev, "bl_power");
+  const char* subsystem = udev_device_get_subsystem(dev);
 
   auto found = std::find_if(devices.begin(), devices.end(), [name](const BacklightDevice& device) {
     return device.name() == name;
@@ -111,13 +112,18 @@ static void upsert_device(std::vector<BacklightDevice>& devices, udev_device* de
       if (power != nullptr) power_bool = std::stoi(power) == 0;
     } catch (const std::exception&) {
     }
-    devices.emplace_back(name, actual_int, max_int, power_bool);
+    devices.emplace_back(name, actual_int, max_int, power_bool,
+                         subsystem != nullptr ? subsystem : "backlight");
   }
 }
 
 static void enumerate_devices(std::vector<BacklightDevice>& devices, udev* udev) {
   std::unique_ptr<udev_enumerate, UdevEnumerateDeleter> enumerate{udev_enumerate_new(udev)};
   udev_enumerate_add_match_subsystem(enumerate.get(), "backlight");
+  // Also enumerate keyboard-backlight LEDs (e.g. "white:kbd_backlight"), which
+  // live in the "leds" subsystem but expose the same brightness/max_brightness
+  // attributes the read path uses.
+  udev_enumerate_add_match_subsystem(enumerate.get(), "leds");
   udev_enumerate_scan_devices(enumerate.get());
   udev_list_entry* enum_devices = udev_enumerate_get_list_entry(enumerate.get());
   udev_list_entry* dev_list_entry;
@@ -129,11 +135,18 @@ static void enumerate_devices(std::vector<BacklightDevice>& devices, udev* udev)
   }
 }
 
-BacklightDevice::BacklightDevice(std::string name, int actual, int max, bool powered)
-    : name_(std::move(name)), actual_(actual), max_(max), powered_(powered) {}
+BacklightDevice::BacklightDevice(std::string name, int actual, int max, bool powered,
+                                 std::string subsystem)
+    : name_(std::move(name)),
+      actual_(actual),
+      max_(max),
+      powered_(powered),
+      subsystem_(std::move(subsystem)) {}
 
 std::string BacklightDevice::name() const { return name_; }
 
+std::string BacklightDevice::subsystem() const { return subsystem_; }
+
 int BacklightDevice::get_actual() const { return actual_; }
 
 void BacklightDevice::set_actual(int actual) { actual_ = actual; }
@@ -178,6 +191,10 @@ BacklightBackend::BacklightBackend(std::chrono::milliseconds interval,
     check_nn(mon.get(), "udev monitor new failed");
     check_gte(udev_monitor_filter_add_match_subsystem_devtype(mon.get(), "backlight", nullptr), 0,
               "udev failed to add monitor filter: ");
+    // Also monitor the "leds" subsystem so keyboard-backlight changes are
+    // reflected live, mirroring the enumeration above.
+    check_gte(udev_monitor_filter_add_match_subsystem_devtype(mon.get(), "leds", nullptr), 0,
+              "udev failed to add monitor filter: ");
     udev_monitor_enable_receiving(mon.get());
 
     auto udev_fd = udev_monitor_get_fd(mon.get());
@@ -235,9 +252,22 @@ const BacklightDevice* BacklightBackend::best_device(const std::vector<Backlight
     return &(*found);
   }
 
-  const auto max = std::max_element(
-      devices.begin(), devices.end(),
-      [](const BacklightDevice& l, const BacklightDevice& r) { return l.get_max() < r.get_max(); });
+  // No device was explicitly configured (or the configured name did not match).
+  // Automatic selection must keep preferring a screen backlight (the "backlight"
+  // subsystem) so that a keyboard-backlight LED, now that the "leds" subsystem is
+  // also enumerated, is never accidentally picked as the default. Only fall back
+  // to other subsystems (e.g. "leds") when no "backlight" device exists at all.
+  const auto max = std::max_element(devices.begin(), devices.end(),
+                                    [](const BacklightDevice& l, const BacklightDevice& r) {
+                                      const bool l_backlight = l.subsystem() == "backlight";
+                                      const bool r_backlight = r.subsystem() == "backlight";
+                                      if (l_backlight != r_backlight) {
+                                        // Rank any non-backlight device below every backlight
+                                        // device.
+                                        return r_backlight;
+                                      }
+                                      return l.get_max() < r.get_max();
+                                    });
 
   return max == devices.end() ? nullptr : &(*max);
 }
@@ -260,7 +290,7 @@ void BacklightBackend::set_scaled_brightness(const std::string& preferred_device
   if (best != nullptr) {
     const auto max = best->get_max();
     const auto abs_val = static_cast<int>(std::round(brightness * max / 100.0F));
-    set_brightness_internal(best->name(), abs_val, best->get_max());
+    set_brightness_internal(best->name(), abs_val, best->get_max(), best->subsystem());
   }
 }
 
@@ -275,12 +305,12 @@ void BacklightBackend::set_brightness(const std::string& preferred_device, Chang
 
     const int new_brightness = change_type == ChangeType::Increase ? best->get_actual() + abs_step
                                                                    : best->get_actual() - abs_step;
-    set_brightness_internal(best->name(), new_brightness, max);
+    set_brightness_internal(best->name(), new_brightness, max, best->subsystem());
   }
 }
 
 void BacklightBackend::set_brightness_internal(const std::string& device_name, int brightness,
-                                               int max_brightness) {
+                                               int max_brightness, const std::string& subsystem) {
   if (!login_proxy_) {
     spdlog::error("Login proxy not available, cannot set brightness");
     return;
@@ -289,7 +319,7 @@ void BacklightBackend::set_brightness_internal(const std::string& device_name, i
   brightness = std::clamp(brightness, 0, max_brightness);
 
   auto call_args = Glib::VariantContainerBase(
-      g_variant_new("(ssu)", "backlight", device_name.c_str(), brightness));
+      g_variant_new("(ssu)", subsystem.c_str(), device_name.c_str(), brightness));
 
   login_proxy_->call_sync("SetBrightness", call_args);
 }