Merge pull request #1015 from alebastr/rfkill-events

rfkill code refactoring

include/modules/bluetooth.hpp

@@ -1,10 +1,6 @@
 #pragma once
 
-#include <fmt/format.h>
 #include "ALabel.hpp"
-
-#include <fmt/chrono.h>
-#include "util/sleeper_thread.hpp"
 #include "util/rfkill.hpp"
 
 namespace waybar::modules {
@@ -16,10 +12,6 @@ class Bluetooth : public ALabel {
   auto update() -> void;
 
  private:
-  std::string           status_;
-  util::SleeperThread 	thread_;
-  util::SleeperThread 	intervall_thread_;
-
   util::Rfkill rfkill_;
 };
 

include/modules/network.hpp

@@ -75,8 +75,6 @@ class Network : public ALabel {
   util::SleeperThread thread_;
   util::SleeperThread thread_timer_;
 #ifdef WANT_RFKILL
-  util::SleeperThread thread_rfkill_;
-
   util::Rfkill rfkill_;
 #endif
 };

include/util/rfkill.hpp

@@ -1,19 +1,26 @@
 #pragma once
 
+#include <glibmm/iochannel.h>
 #include <linux/rfkill.h>
+#include <sigc++/signal.h>
+#include <sigc++/trackable.h>
 
 namespace waybar::util {
 
-class Rfkill {
+class Rfkill : public sigc::trackable {
  public:
   Rfkill(enum rfkill_type rfkill_type);
-  ~Rfkill() = default;
-  void waitForEvent();
+  ~Rfkill();
   bool getState() const;
 
+  sigc::signal<void(struct rfkill_event&)> on_update;
+
  private:
   enum rfkill_type rfkill_type_;
-  int              state_ = 0;
+  bool             state_ = false;
+  int              fd_ = -1;
+
+  bool on_event(Glib::IOCondition cond);
 };
 
 }  // namespace waybar::util

man/waybar-bluetooth.5.scd

@@ -12,11 +12,6 @@ The *bluetooth* module displays information about the status of the device's blu
 
 Addressed by *bluetooth*
 
-*interval*: ++
-	typeof: integer ++
-	default: 60 ++
-	The interval in which the bluetooth state gets updated.
-
 *format*: ++
 	typeof: string  ++
 	default: *{icon}* ++
@@ -88,7 +83,6 @@ Addressed by *bluetooth*
 "bluetooth": {
 	"format": "{icon}",
 	"format-alt": "bluetooth: {status}",
-	"interval": 30,
 	"format-icons": {
 		"enabled": "",
 		"disabled": ""

src/modules/bluetooth.cpp

@@ -1,45 +1,25 @@
 #include "modules/bluetooth.hpp"
-#include "util/rfkill.hpp"
-#include <linux/rfkill.h>
-#include <time.h>
+
+#include <fmt/format.h>
 
 waybar::modules::Bluetooth::Bluetooth(const std::string& id, const Json::Value& config)
-    : ALabel(config, "bluetooth", id, "{icon}", 10),
-      status_("disabled"),
-      rfkill_{RFKILL_TYPE_BLUETOOTH} {
-  thread_ = [this] {
-    dp.emit();
-    rfkill_.waitForEvent();
-  };
-  intervall_thread_ = [this] {
-    auto now = std::chrono::system_clock::now();
-    auto timeout = std::chrono::floor<std::chrono::seconds>(now + interval_);
-    auto diff = std::chrono::seconds(timeout.time_since_epoch().count() % interval_.count());
-    thread_.sleep_until(timeout - diff);
-    dp.emit();
-  };
+    : ALabel(config, "bluetooth", id, "{icon}", 10), rfkill_{RFKILL_TYPE_BLUETOOTH} {
+  rfkill_.on_update.connect(sigc::hide(sigc::mem_fun(*this, &Bluetooth::update)));
 }
 
 auto waybar::modules::Bluetooth::update() -> void {
-  if (rfkill_.getState()) {
-    status_ = "disabled";
-  } else {
-    status_ = "enabled";
-  }
+  std::string status = rfkill_.getState() ? "disabled" : "enabled";
 
   label_.set_markup(
-      fmt::format(
-        format_,
-        fmt::arg("status", status_),
-        fmt::arg("icon", getIcon(0, status_))));
+      fmt::format(format_, fmt::arg("status", status), fmt::arg("icon", getIcon(0, status))));
 
   if (tooltipEnabled()) {
     if (config_["tooltip-format"].isString()) {
       auto tooltip_format = config_["tooltip-format"].asString();
-      auto tooltip_text = fmt::format(tooltip_format, status_);
+      auto tooltip_text = fmt::format(tooltip_format, status);
       label_.set_tooltip_text(tooltip_text);
     } else {
-      label_.set_tooltip_text(status_);
+      label_.set_tooltip_text(status);
     }
   }
 }

src/modules/network.cpp

@@ -212,16 +212,13 @@ void waybar::modules::Network::worker() {
     thread_timer_.sleep_for(interval_);
   };
 #ifdef WANT_RFKILL
-  thread_rfkill_ = [this] {
-    rfkill_.waitForEvent();
-    {
-      std::lock_guard<std::mutex> lock(mutex_);
-      if (ifid_ > 0) {
-        getInfo();
-        dp.emit();
-      }
-    }
-  };
+  rfkill_.on_update.connect([this](auto &) {
+    /* If we are here, it's likely that the network thread already holds the mutex and will be
+     * holding it for a next few seconds.
+     * Let's delegate the update to the timer thread instead of blocking the main thread.
+     */
+    thread_timer_.wake_up();
+  });
 #else
   spdlog::warn("Waybar has been built without rfkill support.");
 #endif

src/util/rfkill.cpp

@@ -19,60 +19,64 @@
 #include "util/rfkill.hpp"
 
 #include <fcntl.h>
+#include <glibmm/main.h>
 #include <linux/rfkill.h>
-#include <poll.h>
-#include <stdlib.h>
+#include <spdlog/spdlog.h>
 #include <unistd.h>
 
 #include <cerrno>
-#include <cstring>
-#include <stdexcept>
 
-waybar::util::Rfkill::Rfkill(const enum rfkill_type rfkill_type) : rfkill_type_(rfkill_type) {}
-
-void waybar::util::Rfkill::waitForEvent() {
-  struct rfkill_event event;
-  struct pollfd       p;
-  ssize_t             len;
-  int                 fd, n;
-
-  fd = open("/dev/rfkill", O_RDONLY);
-  if (fd < 0) {
-    throw std::runtime_error("Can't open RFKILL control device");
+waybar::util::Rfkill::Rfkill(const enum rfkill_type rfkill_type) : rfkill_type_(rfkill_type) {
+  fd_ = open("/dev/rfkill", O_RDONLY);
+  if (fd_ < 0) {
+    spdlog::error("Can't open RFKILL control device");
     return;
   }
-
-  memset(&p, 0, sizeof(p));
-  p.fd = fd;
-  p.events = POLLIN | POLLHUP;
-
-  while (1) {
-    n = poll(&p, 1, -1);
-    if (n < 0) {
-      throw std::runtime_error("Failed to poll RFKILL control device");
-      break;
+  int rc = fcntl(fd_, F_SETFL, O_NONBLOCK);
+  if (rc < 0) {
+    spdlog::error("Can't set RFKILL control device to non-blocking: {}", errno);
+    close(fd_);
+    fd_ = -1;
+    return;
   }
+  Glib::signal_io().connect(
+      sigc::mem_fun(*this, &Rfkill::on_event), fd_, Glib::IO_IN | Glib::IO_ERR | Glib::IO_HUP);
+}
 
-    if (n == 0) continue;
+waybar::util::Rfkill::~Rfkill() {
+  if (fd_ >= 0) {
+    close(fd_);
+  }
+}
 
-    len = read(fd, &event, sizeof(event));
+bool waybar::util::Rfkill::on_event(Glib::IOCondition cond) {
+  if (cond & Glib::IO_IN) {
+    struct rfkill_event event;
+    ssize_t             len;
+
+    len = read(fd_, &event, sizeof(event));
     if (len < 0) {
-      throw std::runtime_error("Reading of RFKILL events failed");
-      break;
+      if (errno == EAGAIN) {
+        return true;
+      }
+      spdlog::error("Reading of RFKILL events failed: {}", errno);
+      return false;
     }
 
-    if (len != RFKILL_EVENT_SIZE_V1) {
-      throw std::runtime_error("Wrong size of RFKILL event");
-      continue;
+    if (len < RFKILL_EVENT_SIZE_V1) {
+      spdlog::error("Wrong size of RFKILL event: {} < {}", len, RFKILL_EVENT_SIZE_V1);
+      return true;
     }
 
-    if (event.type == rfkill_type_ && event.op == RFKILL_OP_CHANGE) {
+    if (event.type == rfkill_type_ && (event.op == RFKILL_OP_ADD || event.op == RFKILL_OP_CHANGE)) {
       state_ = event.soft || event.hard;
-      break;
+      on_update.emit(event);
     }
+    return true;
+  } else {
+    spdlog::error("Failed to poll RFKILL control device");
+    return false;
   }
-
-  close(fd);
 }
 
 bool waybar::util::Rfkill::getState() const { return state_; }