mpd: revamped to event-driven, single-threaded

Fix MPD connection issues by converting/rewriting module into a
state-machine driven system. It is fully single-threaded and uses
events for transitioning between states. It supports all features
and functionality of the previous MPD module.

Signed-off-by: Joseph Benden <joe@benden.us>

include/modules/mpd.hpp

@@ -1,74 +0,0 @@
-#pragma once
-
-#include <fmt/format.h>
-#include <mpd/client.h>
-#include <condition_variable>
-#include <thread>
-#include "ALabel.hpp"
-
-namespace waybar::modules {
-
-class MPD : public ALabel {
- public:
-  MPD(const std::string&, const Json::Value&);
-  auto update() -> void;
-
- private:
-  std::thread periodic_updater();
-  std::string getTag(mpd_tag_type type, unsigned idx = 0);
-  void        setLabel();
-  std::string getStateIcon();
-  std::string getOptionIcon(std::string optionName, bool activated);
-
-  std::thread event_listener();
-
-  // Assumes `connection_lock_` is locked
-  void tryConnect();
-  // If checking errors on the main connection, make sure to lock it using
-  // `connection_lock_` before calling checkErrors
-  void checkErrors(mpd_connection* conn);
-
-  // Assumes `connection_lock_` is locked
-  void fetchState();
-  void waitForEvent();
-
-  bool handlePlayPause(GdkEventButton* const&);
-
-  bool stopped();
-  bool playing();
-  bool paused();
-
-  const std::string module_name_;
-
-  using unique_connection = std::unique_ptr<mpd_connection, decltype(&mpd_connection_free)>;
-  using unique_status = std::unique_ptr<mpd_status, decltype(&mpd_status_free)>;
-  using unique_song = std::unique_ptr<mpd_song, decltype(&mpd_song_free)>;
-
-  // Not using unique_ptr since we don't manage the pointer
-  // (It's either nullptr, or from the config)
-  const char*    server_;
-  const unsigned port_;
-
-  unsigned timeout_;
-
-  // We need a mutex here because we can trigger updates from multiple thread:
-  // the event based updates, the periodic updates needed for the elapsed time,
-  // and the click play/pause feature
-  std::mutex        connection_lock_;
-  unique_connection connection_;
-  // The alternate connection will be used to wait for events: since it will
-  // be blocking (idle) we can't send commands via this connection
-  //
-  // No lock since only used in the event listener thread
-  unique_connection alternate_connection_;
-
-  // Protect them using the `connection_lock_`
-  unique_status status_;
-  mpd_state     state_;
-  unique_song   song_;
-
-  // To make sure the previous periodic_updater stops before creating a new one
-  std::mutex periodic_lock_;
-};
-
-}  // namespace waybar::modules

include/modules/mpd/mpd.hpp

@@ -0,0 +1,66 @@
+#pragma once
+
+#include <mpd/client.h>
+#include <spdlog/fmt/bundled/format.h>
+#include <spdlog/spdlog.h>
+
+#include <condition_variable>
+#include <thread>
+
+#include "ALabel.hpp"
+#include "modules/mpd/state.hpp"
+
+namespace waybar::modules {
+
+class MPD : public ALabel {
+  friend class detail::Context;
+
+  // State machine
+  detail::Context context_{this};
+
+  const std::string module_name_;
+
+  // Not using unique_ptr since we don't manage the pointer
+  // (It's either nullptr, or from the config)
+  const char*    server_;
+  const unsigned port_;
+
+  unsigned timeout_;
+
+  detail::unique_connection connection_;
+
+  detail::unique_status status_;
+  mpd_state             state_;
+  detail::unique_song   song_;
+
+ public:
+  MPD(const std::string&, const Json::Value&);
+  virtual ~MPD() noexcept = default;
+  auto update() -> void;
+
+ private:
+  std::string getTag(mpd_tag_type type, unsigned idx = 0) const;
+  void        setLabel();
+  std::string getStateIcon() const;
+  std::string getOptionIcon(std::string optionName, bool activated) const;
+
+  // GUI-side methods
+  bool handlePlayPause(GdkEventButton* const&);
+  void emit() { dp.emit(); }
+
+  // MPD-side, Non-GUI methods.
+  void tryConnect();
+  void checkErrors(mpd_connection* conn);
+  void fetchState();
+  void queryMPD();
+
+  inline bool stopped() const { return connection_ && state_ == MPD_STATE_STOP; }
+  inline bool playing() const { return connection_ && state_ == MPD_STATE_PLAY; }
+  inline bool paused() const { return connection_ && state_ == MPD_STATE_PAUSE; }
+};
+
+#if !defined(MPD_NOINLINE)
+#include "modules/mpd/state.inl.hpp"
+#endif
+
+}  // namespace waybar::modules

include/modules/mpd/state.hpp

@@ -0,0 +1,217 @@
+#pragma once
+
+#include <mpd/client.h>
+#include <spdlog/fmt/bundled/format.h>
+#include <spdlog/spdlog.h>
+
+#include <condition_variable>
+#include <thread>
+
+#include "ALabel.hpp"
+
+namespace waybar::modules {
+class MPD;
+}  // namespace waybar::modules
+
+namespace waybar::modules::detail {
+
+using unique_connection = std::unique_ptr<mpd_connection, decltype(&mpd_connection_free)>;
+using unique_status = std::unique_ptr<mpd_status, decltype(&mpd_status_free)>;
+using unique_song = std::unique_ptr<mpd_song, decltype(&mpd_song_free)>;
+
+class Context;
+
+/// This state machine loosely follows a non-hierarchical, statechart
+/// pattern, and includes ENTRY and EXIT actions.
+///
+/// The State class is the base class for all other states. The
+/// entry and exit methods are automatically called when entering
+/// into a new state and exiting from the current state. This
+/// includes initially entering (Disconnected class) and exiting
+/// Waybar.
+///
+/// The following nested "top-level" states are represented:
+/// 1. Idle - await notification of MPD activity.
+/// 2. All Non-Idle states:
+///    1. Playing - An active song is producing audio output.
+///    2. Paused - The current song is paused.
+///    3. Stopped - No song is actively playing.
+/// 3. Disconnected - periodically attempt MPD (re-)connection.
+///
+/// NOTE: Since this statechart is non-hierarchical, the above
+/// states are flattened into a set.
+
+class State {
+ public:
+  virtual ~State() noexcept = default;
+
+  virtual void entry() noexcept { spdlog::debug("mpd: ignore entry action"); }
+  virtual void exit() noexcept { spdlog::debug("mpd: ignore exit action"); }
+
+  virtual void play() { spdlog::debug("mpd: ignore play state transition"); }
+  virtual void stop() { spdlog::debug("mpd: ignore stop state transition"); }
+  virtual void pause() { spdlog::debug("mpd: ignore pause state transition"); }
+
+  /// Request state update the GUI.
+  virtual void update() noexcept { spdlog::debug("mpd: ignoring update method request"); }
+};
+
+class Idle : public State {
+  Context* const   ctx_;
+  sigc::connection idle_connection_;
+
+ public:
+  Idle(Context* const ctx) : ctx_{ctx} {}
+  virtual ~Idle() noexcept { this->exit(); };
+
+  void entry() noexcept override;
+  void exit() noexcept override;
+
+  void play() override;
+  void stop() override;
+  void pause() override;
+  void update() noexcept override;
+
+ private:
+  Idle(const Idle&) = delete;
+  Idle& operator=(const Idle&) = delete;
+
+  bool on_io(Glib::IOCondition const&);
+};
+
+class Playing : public State {
+  Context* const   ctx_;
+  sigc::connection timer_connection_;
+
+ public:
+  Playing(Context* const ctx) : ctx_{ctx} {}
+  virtual ~Playing() noexcept { this->exit(); }
+
+  void entry() noexcept override;
+  void exit() noexcept override;
+
+  void pause() override;
+  void stop() override;
+  void update() noexcept override;
+
+ private:
+  Playing(Playing const&) = delete;
+  Playing& operator=(Playing const&) = delete;
+
+  bool on_timer();
+};
+
+class Paused : public State {
+  Context* const   ctx_;
+  sigc::connection timer_connection_;
+
+ public:
+  Paused(Context* const ctx) : ctx_{ctx} {}
+  virtual ~Paused() noexcept { this->exit(); }
+
+  void entry() noexcept override;
+  void exit() noexcept override;
+
+  void play() override;
+  void stop() override;
+  void update() noexcept override;
+
+ private:
+  Paused(Paused const&) = delete;
+  Paused& operator=(Paused const&) = delete;
+
+  bool on_timer();
+};
+
+class Stopped : public State {
+  Context* const   ctx_;
+  sigc::connection timer_connection_;
+
+ public:
+  Stopped(Context* const ctx) : ctx_{ctx} {}
+  virtual ~Stopped() noexcept { this->exit(); }
+
+  void entry() noexcept override;
+  void exit() noexcept override;
+
+  void play() override;
+  void pause() override;
+  void update() noexcept override;
+
+ private:
+  Stopped(Stopped const&) = delete;
+  Stopped& operator=(Stopped const&) = delete;
+
+  bool on_timer();
+};
+
+class Disconnected : public State {
+  Context* const   ctx_;
+  sigc::connection timer_connection_;
+
+ public:
+  Disconnected(Context* const ctx) : ctx_{ctx} {}
+  virtual ~Disconnected() noexcept { this->exit(); }
+
+  void entry() noexcept override;
+  void exit() noexcept override;
+
+  void update() noexcept override;
+
+ private:
+  Disconnected(Disconnected const&) = delete;
+  Disconnected& operator=(Disconnected const&) = delete;
+
+  void arm_timer(int interval) noexcept;
+  void disarm_timer() noexcept;
+
+  bool on_timer();
+};
+
+class Context {
+  std::unique_ptr<State> state_;
+  waybar::modules::MPD*  mpd_module_;
+
+  friend class State;
+  friend class Playing;
+  friend class Paused;
+  friend class Stopped;
+  friend class Disconnected;
+  friend class Idle;
+
+ protected:
+  void setState(std::unique_ptr<State>&& new_state) noexcept {
+    if (state_.get() != nullptr) {
+      state_->exit();
+    }
+    state_ = std::move(new_state);
+    state_->entry();
+  }
+
+  bool                             is_connected() const;
+  bool                             is_playing() const;
+  bool                             is_paused() const;
+  bool                             is_stopped() const;
+  constexpr std::size_t            interval() const;
+  void                             tryConnect() const;
+  void                             checkErrors(mpd_connection*) const;
+  void                             do_update();
+  void                             queryMPD() const;
+  void                             fetchState() const;
+  constexpr mpd_state              state() const;
+  void                             emit() const;
+  [[nodiscard]] unique_connection& connection();
+
+ public:
+  explicit Context(waybar::modules::MPD* const mpd_module)
+      : state_{std::make_unique<Disconnected>(this)}, mpd_module_{mpd_module} {
+    state_->entry();
+  }
+
+  void play() { state_->play(); }
+  void stop() { state_->stop(); }
+  void pause() { state_->pause(); }
+  void update() noexcept { state_->update(); }
+};
+
+}  // namespace waybar::modules::detail

include/modules/mpd/state.inl.hpp

@@ -0,0 +1,24 @@
+#pragma once
+
+namespace detail {
+
+inline bool Context::is_connected() const { return mpd_module_->connection_ != nullptr; }
+inline bool Context::is_playing() const { return mpd_module_->playing(); }
+inline bool Context::is_paused() const { return mpd_module_->paused(); }
+inline bool Context::is_stopped() const { return mpd_module_->stopped(); }
+
+constexpr inline std::size_t Context::interval() const { return mpd_module_->interval_.count(); }
+inline void                  Context::tryConnect() const { mpd_module_->tryConnect(); }
+inline unique_connection&    Context::connection() { return mpd_module_->connection_; }
+constexpr inline mpd_state   Context::state() const { return mpd_module_->state_; }
+
+inline void Context::do_update() {
+  mpd_module_->setLabel();
+}
+
+inline void Context::checkErrors(mpd_connection* conn) const { mpd_module_->checkErrors(conn); }
+inline void Context::queryMPD() const { mpd_module_->queryMPD(); }
+inline void Context::fetchState() const { mpd_module_->fetchState(); }
+inline void Context::emit() const { mpd_module_->emit(); }
+
+}  // namespace detail