From 21fdcf41c3aacf381bf4756b6a4ec765e5f8c1db Mon Sep 17 00:00:00 2001 From: Joseph Benden Date: Sat, 3 Oct 2020 22:01:51 -0700 Subject: [PATCH 1/4] 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 --- include/factory.hpp | 2 +- include/modules/mpd.hpp | 74 ------ include/modules/mpd/mpd.hpp | 66 ++++++ include/modules/mpd/state.hpp | 217 +++++++++++++++++ include/modules/mpd/state.inl.hpp | 24 ++ meson.build | 3 +- src/modules/{ => mpd}/mpd.cpp | 156 ++++-------- src/modules/mpd/state.cpp | 382 ++++++++++++++++++++++++++++++ 8 files changed, 736 insertions(+), 188 deletions(-) delete mode 100644 include/modules/mpd.hpp create mode 100644 include/modules/mpd/mpd.hpp create mode 100644 include/modules/mpd/state.hpp create mode 100644 include/modules/mpd/state.inl.hpp rename src/modules/{ => mpd}/mpd.cpp (72%) create mode 100644 src/modules/mpd/state.cpp diff --git a/include/factory.hpp b/include/factory.hpp index 3efe6cb..f73c6e1 100644 --- a/include/factory.hpp +++ b/include/factory.hpp @@ -37,7 +37,7 @@ #include "modules/pulseaudio.hpp" #endif #ifdef HAVE_LIBMPDCLIENT -#include "modules/mpd.hpp" +#include "modules/mpd/mpd.hpp" #endif #ifdef HAVE_LIBSNDIO #include "modules/sndio.hpp" diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp deleted file mode 100644 index d08b28b..0000000 --- a/include/modules/mpd.hpp +++ /dev/null @@ -1,74 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#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; - using unique_status = std::unique_ptr; - using unique_song = std::unique_ptr; - - // 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 diff --git a/include/modules/mpd/mpd.hpp b/include/modules/mpd/mpd.hpp new file mode 100644 index 0000000..effe633 --- /dev/null +++ b/include/modules/mpd/mpd.hpp @@ -0,0 +1,66 @@ +#pragma once + +#include +#include +#include + +#include +#include + +#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 diff --git a/include/modules/mpd/state.hpp b/include/modules/mpd/state.hpp new file mode 100644 index 0000000..79e4f63 --- /dev/null +++ b/include/modules/mpd/state.hpp @@ -0,0 +1,217 @@ +#pragma once + +#include +#include +#include + +#include +#include + +#include "ALabel.hpp" + +namespace waybar::modules { +class MPD; +} // namespace waybar::modules + +namespace waybar::modules::detail { + +using unique_connection = std::unique_ptr; +using unique_status = std::unique_ptr; +using unique_song = std::unique_ptr; + +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_; + 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&& 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(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 diff --git a/include/modules/mpd/state.inl.hpp b/include/modules/mpd/state.inl.hpp new file mode 100644 index 0000000..0d83b0b --- /dev/null +++ b/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 diff --git a/meson.build b/meson.build index 023894c..de20382 100644 --- a/meson.build +++ b/meson.build @@ -213,7 +213,8 @@ endif if libmpdclient.found() add_project_arguments('-DHAVE_LIBMPDCLIENT', language: 'cpp') - src_files += 'src/modules/mpd.cpp' + src_files += 'src/modules/mpd/mpd.cpp' + src_files += 'src/modules/mpd/state.cpp' endif if gtk_layer_shell.found() diff --git a/src/modules/mpd.cpp b/src/modules/mpd/mpd.cpp similarity index 72% rename from src/modules/mpd.cpp rename to src/modules/mpd/mpd.cpp index 26878b1..50f2817 100644 --- a/src/modules/mpd.cpp +++ b/src/modules/mpd/mpd.cpp @@ -1,8 +1,15 @@ -#include "modules/mpd.hpp" +#include "modules/mpd/mpd.hpp" -#include +#include #include +#include "modules/mpd/state.hpp" +#if defined(MPD_NOINLINE) +namespace waybar::modules { +#include "modules/mpd/state.inl.hpp" +} // namespace waybar::modules +#endif + waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) : ALabel(config, "mpd", id, "{album} - {artist} - {title}", 5), module_name_(id.empty() ? "mpd" : "mpd#" + id), @@ -10,7 +17,6 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) port_(config_["port"].isUInt() ? config["port"].asUInt() : 0), timeout_(config_["timeout"].isUInt() ? config_["timeout"].asUInt() * 1'000 : 30'000), connection_(nullptr, &mpd_connection_free), - alternate_connection_(nullptr, &mpd_connection_free), status_(nullptr, &mpd_status_free), song_(nullptr, &mpd_song_free) { if (!config_["port"].isNull() && !config_["port"].isUInt()) { @@ -28,73 +34,33 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) server_ = config["server"].asCString(); } - event_listener().detach(); - event_box_.add_events(Gdk::BUTTON_PRESS_MASK); event_box_.signal_button_press_event().connect(sigc::mem_fun(*this, &MPD::handlePlayPause)); } auto waybar::modules::MPD::update() -> void { - std::lock_guard guard(connection_lock_); - tryConnect(); - - if (connection_ != nullptr) { - try { - bool wasPlaying = playing(); - if(!wasPlaying) { - // Wait until the periodic_updater has stopped - std::lock_guard periodic_guard(periodic_lock_); - } - fetchState(); - if (!wasPlaying && playing()) { - periodic_updater().detach(); - } - } catch (const std::exception& e) { - spdlog::error("{}: {}", module_name_, e.what()); - state_ = MPD_STATE_UNKNOWN; - } - } - - setLabel(); + context_.update(); // Call parent update ALabel::update(); } -std::thread waybar::modules::MPD::event_listener() { - return std::thread([this] { - while (true) { - try { - if (connection_ == nullptr) { - // Retry periodically if no connection - dp.emit(); - std::this_thread::sleep_for(interval_); - } else { - waitForEvent(); - dp.emit(); - } - } catch (const std::exception& e) { - if (strcmp(e.what(), "Connection to MPD closed") == 0) { - spdlog::debug("{}: {}", module_name_, e.what()); - } else { - spdlog::warn("{}: {}", module_name_, e.what()); - } - } +void waybar::modules::MPD::queryMPD() { + if (connection_ != nullptr) { + spdlog::debug("{}: fetching state information", module_name_); + try { + fetchState(); + spdlog::debug("{}: fetch complete", module_name_); + } catch (std::exception const& e) { + spdlog::error("{}: {}", module_name_, e.what()); + state_ = MPD_STATE_UNKNOWN; } - }); + + dp.emit(); + } } -std::thread waybar::modules::MPD::periodic_updater() { - return std::thread([this] { - std::lock_guard guard(periodic_lock_); - while (connection_ != nullptr && playing()) { - dp.emit(); - std::this_thread::sleep_for(std::chrono::seconds(1)); - } - }); -} - -std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) { +std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) const { std::string result = config_["unknown-tag"].isString() ? config_["unknown-tag"].asString() : "N/A"; const char* tag = mpd_song_get_tag(song_.get(), type, idx); @@ -133,7 +99,7 @@ void waybar::modules::MPD::setLabel() { auto format = format_; std::string artist, album_artist, album, title, date; - int song_pos, queue_length; + int song_pos, queue_length; std::chrono::seconds elapsedTime, totalTime; std::string stateIcon = ""; @@ -149,8 +115,8 @@ void waybar::modules::MPD::setLabel() { label_.get_style_context()->add_class("playing"); label_.get_style_context()->remove_class("paused"); } else if (paused()) { - format = - config_["format-paused"].isString() ? config_["format-paused"].asString() : config_["format"].asString(); + format = config_["format-paused"].isString() ? config_["format-paused"].asString() + : config_["format"].asString(); label_.get_style_context()->add_class("paused"); label_.get_style_context()->remove_class("playing"); } @@ -216,7 +182,7 @@ void waybar::modules::MPD::setLabel() { } } -std::string waybar::modules::MPD::getStateIcon() { +std::string waybar::modules::MPD::getStateIcon() const { if (!config_["state-icons"].isObject()) { return ""; } @@ -238,7 +204,7 @@ std::string waybar::modules::MPD::getStateIcon() { } } -std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) { +std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) const { if (!config_[optionName + "-icons"].isObject()) { return ""; } @@ -261,15 +227,11 @@ void waybar::modules::MPD::tryConnect() { } connection_ = - unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); + detail::unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); - alternate_connection_ = - unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); - - if (connection_ == nullptr || alternate_connection_ == nullptr) { + if (connection_ == nullptr) { spdlog::error("{}: Failed to connect to MPD", module_name_); connection_.reset(); - alternate_connection_.reset(); return; } @@ -279,7 +241,6 @@ void waybar::modules::MPD::tryConnect() { } catch (std::runtime_error& e) { spdlog::error("{}: Failed to connect to MPD: {}", module_name_, e.what()); connection_.reset(); - alternate_connection_.reset(); } } @@ -292,7 +253,6 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { case MPD_ERROR_CLOSED: mpd_connection_clear_error(conn); connection_.reset(); - alternate_connection_.reset(); state_ = MPD_STATE_UNKNOWN; throw std::runtime_error("Connection to MPD closed"); default: @@ -306,37 +266,20 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { } void waybar::modules::MPD::fetchState() { + if (connection_ == nullptr) { + spdlog::error("{}: Not connected to MPD", module_name_); + return; + } + auto conn = connection_.get(); - status_ = unique_status(mpd_run_status(conn), &mpd_status_free); + + status_ = detail::unique_status(mpd_run_status(conn), &mpd_status_free); checkErrors(conn); + state_ = mpd_status_get_state(status_.get()); checkErrors(conn); - song_ = unique_song(mpd_run_current_song(conn), &mpd_song_free); - checkErrors(conn); -} - -void waybar::modules::MPD::waitForEvent() { - auto conn = alternate_connection_.get(); - // Wait for a player (play/pause), option (random, shuffle, etc.), or playlist - // change - if (!mpd_send_idle_mask( - conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { - checkErrors(conn); - return; - } - // alternate_idle_ = true; - - // See issue #277: - // https://github.com/Alexays/Waybar/issues/277 - mpd_recv_idle(conn, /* disable_timeout = */ false); - // See issue #281: - // https://github.com/Alexays/Waybar/issues/281 - std::lock_guard guard(connection_lock_); - - checkErrors(conn); - mpd_response_finish(conn); - + song_ = detail::unique_song(mpd_run_current_song(conn), &mpd_song_free); checkErrors(conn); } @@ -346,24 +289,13 @@ bool waybar::modules::MPD::handlePlayPause(GdkEventButton* const& e) { } if (e->button == 1) { - std::lock_guard guard(connection_lock_); - if (stopped()) { - mpd_run_play(connection_.get()); - } else { - mpd_run_toggle_pause(connection_.get()); - } + if (state_ == MPD_STATE_PLAY) + context_.pause(); + else + context_.play(); } else if (e->button == 3) { - std::lock_guard guard(connection_lock_); - mpd_run_stop(connection_.get()); + context_.stop(); } return true; } - -bool waybar::modules::MPD::stopped() { - return connection_ == nullptr || state_ == MPD_STATE_UNKNOWN || state_ == MPD_STATE_STOP || status_ == nullptr; -} - -bool waybar::modules::MPD::playing() { return connection_ != nullptr && state_ == MPD_STATE_PLAY; } - -bool waybar::modules::MPD::paused() { return connection_ != nullptr && state_ == MPD_STATE_PAUSE; } diff --git a/src/modules/mpd/state.cpp b/src/modules/mpd/state.cpp new file mode 100644 index 0000000..21b67ae --- /dev/null +++ b/src/modules/mpd/state.cpp @@ -0,0 +1,382 @@ +#include "modules/mpd/state.hpp" + +#include +#include + +#include "modules/mpd/mpd.hpp" +#if defined(MPD_NOINLINE) +namespace waybar::modules { +#include "modules/mpd/state.inl.hpp" +} // namespace waybar::modules +#endif + +namespace waybar::modules::detail { + +#define IDLE_RUN_NOIDLE_AND_CMD(...) \ + if (idle_connection_.connected()) { \ + idle_connection_.disconnect(); \ + auto conn = ctx_->connection().get(); \ + if (!mpd_run_noidle(conn)) { \ + if (mpd_connection_get_error(conn) != MPD_ERROR_SUCCESS) { \ + spdlog::error("mpd: Idle: failed to unregister for IDLE events"); \ + ctx_->checkErrors(conn); \ + } \ + } \ + __VA_ARGS__; \ + } + +void Idle::play() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_play(conn)); + + ctx_->setState(std::make_unique(ctx_)); +} + +void Idle::pause() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_pause(conn, true)); + + ctx_->setState(std::make_unique(ctx_)); +} + +void Idle::stop() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_stop(conn)); + + ctx_->setState(std::make_unique(ctx_)); +} + +#undef IDLE_RUN_NOIDLE_AND_CMD + +void Idle::update() noexcept { + // This is intentionally blank. +} + +void Idle::entry() noexcept { + auto conn = ctx_->connection().get(); + assert(conn != nullptr); + + if (!mpd_send_idle_mask( + conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { + ctx_->checkErrors(conn); + spdlog::error("mpd: Idle: failed to register for IDLE events"); + } else { + spdlog::debug("mpd: Idle: watching FD"); + sigc::slot idle_slot = sigc::mem_fun(*this, &Idle::on_io); + idle_connection_ = + Glib::signal_io().connect(idle_slot, + mpd_connection_get_fd(conn), + Glib::IO_IN | Glib::IO_PRI | Glib::IO_ERR | Glib::IO_HUP); + } +} + +void Idle::exit() noexcept { + if (idle_connection_.connected()) { + idle_connection_.disconnect(); + spdlog::debug("mpd: Idle: unwatching FD"); + } +} + +bool Idle::on_io(Glib::IOCondition const&) { + auto conn = ctx_->connection().get(); + + // callback should do this: + enum mpd_idle events = mpd_recv_idle(conn, /* ignore_timeout?= */ false); + spdlog::debug("mpd: Idle: recv_idle events -> {}", events); + + mpd_response_finish(conn); + try { + ctx_->checkErrors(conn); + } catch (std::exception const& e) { + spdlog::warn("mpd: Idle: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + mpd_state state = ctx_->state(); + + if (state == MPD_STATE_STOP) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else if (state == MPD_STATE_PLAY) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else if (state == MPD_STATE_PAUSE) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->emit(); + // self transition + ctx_->setState(std::make_unique(ctx_)); + } + + return false; +} + +void Playing::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Playing::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 1'000); + spdlog::debug("mpd: Playing: enabled 1 second periodic timer."); +} + +void Playing::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Playing: disabled 1 second periodic timer."); + } +} + +bool Playing::on_timer() { + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + if (!ctx_->is_playing()) { + if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->setState(std::make_unique(ctx_)); + } + return false; + } + + ctx_->queryMPD(); + ctx_->emit(); + } catch (std::exception const& e) { + spdlog::warn("mpd: Playing: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + return true; +} + +void Playing::stop() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_stop(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Playing::pause() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_pause(ctx_->connection().get(), true); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Playing::update() noexcept { ctx_->do_update(); } + +void Paused::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Paused::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); + spdlog::debug("mpd: Paused: enabled 200 ms periodic timer."); +} + +void Paused::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Paused: disabled 200 ms periodic timer."); + } +} + +bool Paused::on_timer() { + bool rc = true; + + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + ctx_->emit(); + + if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_stopped()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Paused: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + + return rc; +} + +void Paused::play() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_play(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Paused::stop() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_stop(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Paused::update() noexcept { ctx_->do_update(); } + +void Stopped::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Stopped::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); + spdlog::debug("mpd: Stopped: enabled 200 ms periodic timer."); +} + +void Stopped::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Stopped: disabled 200 ms periodic timer."); + } +} + +bool Stopped::on_timer() { + bool rc = true; + + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + ctx_->emit(); + + if (ctx_->is_stopped()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Stopped: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + + return rc; +} + +void Stopped::play() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_play(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Stopped::pause() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_pause(ctx_->connection().get(), true); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Stopped::update() noexcept { ctx_->do_update(); } + +void Disconnected::arm_timer(int interval) noexcept { + // unregister timer, if present + disarm_timer(); + + // register timer + sigc::slot timer_slot = sigc::mem_fun(*this, &Disconnected::on_timer); + timer_connection_ = + Glib::signal_timeout().connect(timer_slot, interval); + spdlog::debug("mpd: Disconnected: enabled interval timer."); +} + +void Disconnected::disarm_timer() noexcept { + // unregister timer, if present + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Disconnected: disabled interval timer."); + } +} + +void Disconnected::entry() noexcept { + arm_timer(1'000); +} + +void Disconnected::exit() noexcept { + disarm_timer(); +} + +bool Disconnected::on_timer() { + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (ctx_->is_connected()) { + ctx_->fetchState(); + ctx_->emit(); + + if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + } else if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->setState(std::make_unique(ctx_)); + } + + return false; // do not rearm timer + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Disconnected: error: {}", e.what()); + } + + arm_timer(ctx_->interval() * 1'000); + + return false; +} + +void Disconnected::update() noexcept { ctx_->do_update(); } + +} // namespace waybar::modules::detail From 54beabb9dc7ce3bc3a80488c948ca6bfd437336d Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 18 Oct 2020 10:45:31 +0200 Subject: [PATCH 2/4] Revert "mpd: revamped to event-driven, single-threaded" --- include/factory.hpp | 2 +- include/modules/mpd.hpp | 74 ++++++ include/modules/mpd/mpd.hpp | 66 ------ include/modules/mpd/state.hpp | 217 ----------------- include/modules/mpd/state.inl.hpp | 24 -- meson.build | 3 +- src/modules/{mpd => }/mpd.cpp | 156 ++++++++---- src/modules/mpd/state.cpp | 382 ------------------------------ 8 files changed, 188 insertions(+), 736 deletions(-) create mode 100644 include/modules/mpd.hpp delete mode 100644 include/modules/mpd/mpd.hpp delete mode 100644 include/modules/mpd/state.hpp delete mode 100644 include/modules/mpd/state.inl.hpp rename src/modules/{mpd => }/mpd.cpp (72%) delete mode 100644 src/modules/mpd/state.cpp diff --git a/include/factory.hpp b/include/factory.hpp index f73c6e1..3efe6cb 100644 --- a/include/factory.hpp +++ b/include/factory.hpp @@ -37,7 +37,7 @@ #include "modules/pulseaudio.hpp" #endif #ifdef HAVE_LIBMPDCLIENT -#include "modules/mpd/mpd.hpp" +#include "modules/mpd.hpp" #endif #ifdef HAVE_LIBSNDIO #include "modules/sndio.hpp" diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp new file mode 100644 index 0000000..d08b28b --- /dev/null +++ b/include/modules/mpd.hpp @@ -0,0 +1,74 @@ +#pragma once + +#include +#include +#include +#include +#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; + using unique_status = std::unique_ptr; + using unique_song = std::unique_ptr; + + // 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 diff --git a/include/modules/mpd/mpd.hpp b/include/modules/mpd/mpd.hpp deleted file mode 100644 index effe633..0000000 --- a/include/modules/mpd/mpd.hpp +++ /dev/null @@ -1,66 +0,0 @@ -#pragma once - -#include -#include -#include - -#include -#include - -#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 diff --git a/include/modules/mpd/state.hpp b/include/modules/mpd/state.hpp deleted file mode 100644 index 79e4f63..0000000 --- a/include/modules/mpd/state.hpp +++ /dev/null @@ -1,217 +0,0 @@ -#pragma once - -#include -#include -#include - -#include -#include - -#include "ALabel.hpp" - -namespace waybar::modules { -class MPD; -} // namespace waybar::modules - -namespace waybar::modules::detail { - -using unique_connection = std::unique_ptr; -using unique_status = std::unique_ptr; -using unique_song = std::unique_ptr; - -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_; - 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&& 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(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 diff --git a/include/modules/mpd/state.inl.hpp b/include/modules/mpd/state.inl.hpp deleted file mode 100644 index 0d83b0b..0000000 --- a/include/modules/mpd/state.inl.hpp +++ /dev/null @@ -1,24 +0,0 @@ -#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 diff --git a/meson.build b/meson.build index de20382..023894c 100644 --- a/meson.build +++ b/meson.build @@ -213,8 +213,7 @@ endif if libmpdclient.found() add_project_arguments('-DHAVE_LIBMPDCLIENT', language: 'cpp') - src_files += 'src/modules/mpd/mpd.cpp' - src_files += 'src/modules/mpd/state.cpp' + src_files += 'src/modules/mpd.cpp' endif if gtk_layer_shell.found() diff --git a/src/modules/mpd/mpd.cpp b/src/modules/mpd.cpp similarity index 72% rename from src/modules/mpd/mpd.cpp rename to src/modules/mpd.cpp index 50f2817..26878b1 100644 --- a/src/modules/mpd/mpd.cpp +++ b/src/modules/mpd.cpp @@ -1,15 +1,8 @@ -#include "modules/mpd/mpd.hpp" +#include "modules/mpd.hpp" -#include +#include #include -#include "modules/mpd/state.hpp" -#if defined(MPD_NOINLINE) -namespace waybar::modules { -#include "modules/mpd/state.inl.hpp" -} // namespace waybar::modules -#endif - waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) : ALabel(config, "mpd", id, "{album} - {artist} - {title}", 5), module_name_(id.empty() ? "mpd" : "mpd#" + id), @@ -17,6 +10,7 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) port_(config_["port"].isUInt() ? config["port"].asUInt() : 0), timeout_(config_["timeout"].isUInt() ? config_["timeout"].asUInt() * 1'000 : 30'000), connection_(nullptr, &mpd_connection_free), + alternate_connection_(nullptr, &mpd_connection_free), status_(nullptr, &mpd_status_free), song_(nullptr, &mpd_song_free) { if (!config_["port"].isNull() && !config_["port"].isUInt()) { @@ -34,33 +28,73 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) server_ = config["server"].asCString(); } + event_listener().detach(); + event_box_.add_events(Gdk::BUTTON_PRESS_MASK); event_box_.signal_button_press_event().connect(sigc::mem_fun(*this, &MPD::handlePlayPause)); } auto waybar::modules::MPD::update() -> void { - context_.update(); + std::lock_guard guard(connection_lock_); + tryConnect(); + + if (connection_ != nullptr) { + try { + bool wasPlaying = playing(); + if(!wasPlaying) { + // Wait until the periodic_updater has stopped + std::lock_guard periodic_guard(periodic_lock_); + } + fetchState(); + if (!wasPlaying && playing()) { + periodic_updater().detach(); + } + } catch (const std::exception& e) { + spdlog::error("{}: {}", module_name_, e.what()); + state_ = MPD_STATE_UNKNOWN; + } + } + + setLabel(); // Call parent update ALabel::update(); } -void waybar::modules::MPD::queryMPD() { - if (connection_ != nullptr) { - spdlog::debug("{}: fetching state information", module_name_); - try { - fetchState(); - spdlog::debug("{}: fetch complete", module_name_); - } catch (std::exception const& e) { - spdlog::error("{}: {}", module_name_, e.what()); - state_ = MPD_STATE_UNKNOWN; +std::thread waybar::modules::MPD::event_listener() { + return std::thread([this] { + while (true) { + try { + if (connection_ == nullptr) { + // Retry periodically if no connection + dp.emit(); + std::this_thread::sleep_for(interval_); + } else { + waitForEvent(); + dp.emit(); + } + } catch (const std::exception& e) { + if (strcmp(e.what(), "Connection to MPD closed") == 0) { + spdlog::debug("{}: {}", module_name_, e.what()); + } else { + spdlog::warn("{}: {}", module_name_, e.what()); + } + } } - - dp.emit(); - } + }); } -std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) const { +std::thread waybar::modules::MPD::periodic_updater() { + return std::thread([this] { + std::lock_guard guard(periodic_lock_); + while (connection_ != nullptr && playing()) { + dp.emit(); + std::this_thread::sleep_for(std::chrono::seconds(1)); + } + }); +} + +std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) { std::string result = config_["unknown-tag"].isString() ? config_["unknown-tag"].asString() : "N/A"; const char* tag = mpd_song_get_tag(song_.get(), type, idx); @@ -99,7 +133,7 @@ void waybar::modules::MPD::setLabel() { auto format = format_; std::string artist, album_artist, album, title, date; - int song_pos, queue_length; + int song_pos, queue_length; std::chrono::seconds elapsedTime, totalTime; std::string stateIcon = ""; @@ -115,8 +149,8 @@ void waybar::modules::MPD::setLabel() { label_.get_style_context()->add_class("playing"); label_.get_style_context()->remove_class("paused"); } else if (paused()) { - format = config_["format-paused"].isString() ? config_["format-paused"].asString() - : config_["format"].asString(); + format = + config_["format-paused"].isString() ? config_["format-paused"].asString() : config_["format"].asString(); label_.get_style_context()->add_class("paused"); label_.get_style_context()->remove_class("playing"); } @@ -182,7 +216,7 @@ void waybar::modules::MPD::setLabel() { } } -std::string waybar::modules::MPD::getStateIcon() const { +std::string waybar::modules::MPD::getStateIcon() { if (!config_["state-icons"].isObject()) { return ""; } @@ -204,7 +238,7 @@ std::string waybar::modules::MPD::getStateIcon() const { } } -std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) const { +std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) { if (!config_[optionName + "-icons"].isObject()) { return ""; } @@ -227,11 +261,15 @@ void waybar::modules::MPD::tryConnect() { } connection_ = - detail::unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); + unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); - if (connection_ == nullptr) { + alternate_connection_ = + unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); + + if (connection_ == nullptr || alternate_connection_ == nullptr) { spdlog::error("{}: Failed to connect to MPD", module_name_); connection_.reset(); + alternate_connection_.reset(); return; } @@ -241,6 +279,7 @@ void waybar::modules::MPD::tryConnect() { } catch (std::runtime_error& e) { spdlog::error("{}: Failed to connect to MPD: {}", module_name_, e.what()); connection_.reset(); + alternate_connection_.reset(); } } @@ -253,6 +292,7 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { case MPD_ERROR_CLOSED: mpd_connection_clear_error(conn); connection_.reset(); + alternate_connection_.reset(); state_ = MPD_STATE_UNKNOWN; throw std::runtime_error("Connection to MPD closed"); default: @@ -266,20 +306,37 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { } void waybar::modules::MPD::fetchState() { - if (connection_ == nullptr) { - spdlog::error("{}: Not connected to MPD", module_name_); - return; - } - auto conn = connection_.get(); - - status_ = detail::unique_status(mpd_run_status(conn), &mpd_status_free); + status_ = unique_status(mpd_run_status(conn), &mpd_status_free); checkErrors(conn); - state_ = mpd_status_get_state(status_.get()); checkErrors(conn); - song_ = detail::unique_song(mpd_run_current_song(conn), &mpd_song_free); + song_ = unique_song(mpd_run_current_song(conn), &mpd_song_free); + checkErrors(conn); +} + +void waybar::modules::MPD::waitForEvent() { + auto conn = alternate_connection_.get(); + // Wait for a player (play/pause), option (random, shuffle, etc.), or playlist + // change + if (!mpd_send_idle_mask( + conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { + checkErrors(conn); + return; + } + // alternate_idle_ = true; + + // See issue #277: + // https://github.com/Alexays/Waybar/issues/277 + mpd_recv_idle(conn, /* disable_timeout = */ false); + // See issue #281: + // https://github.com/Alexays/Waybar/issues/281 + std::lock_guard guard(connection_lock_); + + checkErrors(conn); + mpd_response_finish(conn); + checkErrors(conn); } @@ -289,13 +346,24 @@ bool waybar::modules::MPD::handlePlayPause(GdkEventButton* const& e) { } if (e->button == 1) { - if (state_ == MPD_STATE_PLAY) - context_.pause(); - else - context_.play(); + std::lock_guard guard(connection_lock_); + if (stopped()) { + mpd_run_play(connection_.get()); + } else { + mpd_run_toggle_pause(connection_.get()); + } } else if (e->button == 3) { - context_.stop(); + std::lock_guard guard(connection_lock_); + mpd_run_stop(connection_.get()); } return true; } + +bool waybar::modules::MPD::stopped() { + return connection_ == nullptr || state_ == MPD_STATE_UNKNOWN || state_ == MPD_STATE_STOP || status_ == nullptr; +} + +bool waybar::modules::MPD::playing() { return connection_ != nullptr && state_ == MPD_STATE_PLAY; } + +bool waybar::modules::MPD::paused() { return connection_ != nullptr && state_ == MPD_STATE_PAUSE; } diff --git a/src/modules/mpd/state.cpp b/src/modules/mpd/state.cpp deleted file mode 100644 index 21b67ae..0000000 --- a/src/modules/mpd/state.cpp +++ /dev/null @@ -1,382 +0,0 @@ -#include "modules/mpd/state.hpp" - -#include -#include - -#include "modules/mpd/mpd.hpp" -#if defined(MPD_NOINLINE) -namespace waybar::modules { -#include "modules/mpd/state.inl.hpp" -} // namespace waybar::modules -#endif - -namespace waybar::modules::detail { - -#define IDLE_RUN_NOIDLE_AND_CMD(...) \ - if (idle_connection_.connected()) { \ - idle_connection_.disconnect(); \ - auto conn = ctx_->connection().get(); \ - if (!mpd_run_noidle(conn)) { \ - if (mpd_connection_get_error(conn) != MPD_ERROR_SUCCESS) { \ - spdlog::error("mpd: Idle: failed to unregister for IDLE events"); \ - ctx_->checkErrors(conn); \ - } \ - } \ - __VA_ARGS__; \ - } - -void Idle::play() { - IDLE_RUN_NOIDLE_AND_CMD(mpd_run_play(conn)); - - ctx_->setState(std::make_unique(ctx_)); -} - -void Idle::pause() { - IDLE_RUN_NOIDLE_AND_CMD(mpd_run_pause(conn, true)); - - ctx_->setState(std::make_unique(ctx_)); -} - -void Idle::stop() { - IDLE_RUN_NOIDLE_AND_CMD(mpd_run_stop(conn)); - - ctx_->setState(std::make_unique(ctx_)); -} - -#undef IDLE_RUN_NOIDLE_AND_CMD - -void Idle::update() noexcept { - // This is intentionally blank. -} - -void Idle::entry() noexcept { - auto conn = ctx_->connection().get(); - assert(conn != nullptr); - - if (!mpd_send_idle_mask( - conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { - ctx_->checkErrors(conn); - spdlog::error("mpd: Idle: failed to register for IDLE events"); - } else { - spdlog::debug("mpd: Idle: watching FD"); - sigc::slot idle_slot = sigc::mem_fun(*this, &Idle::on_io); - idle_connection_ = - Glib::signal_io().connect(idle_slot, - mpd_connection_get_fd(conn), - Glib::IO_IN | Glib::IO_PRI | Glib::IO_ERR | Glib::IO_HUP); - } -} - -void Idle::exit() noexcept { - if (idle_connection_.connected()) { - idle_connection_.disconnect(); - spdlog::debug("mpd: Idle: unwatching FD"); - } -} - -bool Idle::on_io(Glib::IOCondition const&) { - auto conn = ctx_->connection().get(); - - // callback should do this: - enum mpd_idle events = mpd_recv_idle(conn, /* ignore_timeout?= */ false); - spdlog::debug("mpd: Idle: recv_idle events -> {}", events); - - mpd_response_finish(conn); - try { - ctx_->checkErrors(conn); - } catch (std::exception const& e) { - spdlog::warn("mpd: Idle: error: {}", e.what()); - ctx_->setState(std::make_unique(ctx_)); - return false; - } - - ctx_->fetchState(); - mpd_state state = ctx_->state(); - - if (state == MPD_STATE_STOP) { - ctx_->emit(); - ctx_->setState(std::make_unique(ctx_)); - } else if (state == MPD_STATE_PLAY) { - ctx_->emit(); - ctx_->setState(std::make_unique(ctx_)); - } else if (state == MPD_STATE_PAUSE) { - ctx_->emit(); - ctx_->setState(std::make_unique(ctx_)); - } else { - ctx_->emit(); - // self transition - ctx_->setState(std::make_unique(ctx_)); - } - - return false; -} - -void Playing::entry() noexcept { - sigc::slot timer_slot = sigc::mem_fun(*this, &Playing::on_timer); - timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 1'000); - spdlog::debug("mpd: Playing: enabled 1 second periodic timer."); -} - -void Playing::exit() noexcept { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - spdlog::debug("mpd: Playing: disabled 1 second periodic timer."); - } -} - -bool Playing::on_timer() { - // Attempt to connect with MPD. - try { - ctx_->tryConnect(); - - // Success? - if (!ctx_->is_connected()) { - ctx_->setState(std::make_unique(ctx_)); - return false; - } - - ctx_->fetchState(); - - if (!ctx_->is_playing()) { - if (ctx_->is_paused()) { - ctx_->setState(std::make_unique(ctx_)); - } else { - ctx_->setState(std::make_unique(ctx_)); - } - return false; - } - - ctx_->queryMPD(); - ctx_->emit(); - } catch (std::exception const& e) { - spdlog::warn("mpd: Playing: error: {}", e.what()); - ctx_->setState(std::make_unique(ctx_)); - return false; - } - - return true; -} - -void Playing::stop() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_stop(ctx_->connection().get()); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Playing::pause() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_pause(ctx_->connection().get(), true); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Playing::update() noexcept { ctx_->do_update(); } - -void Paused::entry() noexcept { - sigc::slot timer_slot = sigc::mem_fun(*this, &Paused::on_timer); - timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); - spdlog::debug("mpd: Paused: enabled 200 ms periodic timer."); -} - -void Paused::exit() noexcept { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - spdlog::debug("mpd: Paused: disabled 200 ms periodic timer."); - } -} - -bool Paused::on_timer() { - bool rc = true; - - // Attempt to connect with MPD. - try { - ctx_->tryConnect(); - - // Success? - if (!ctx_->is_connected()) { - ctx_->setState(std::make_unique(ctx_)); - return false; - } - - ctx_->fetchState(); - - ctx_->emit(); - - if (ctx_->is_paused()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } else if (ctx_->is_playing()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } else if (ctx_->is_stopped()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } - } catch (std::exception const& e) { - spdlog::warn("mpd: Paused: error: {}", e.what()); - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } - - return rc; -} - -void Paused::play() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_play(ctx_->connection().get()); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Paused::stop() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_stop(ctx_->connection().get()); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Paused::update() noexcept { ctx_->do_update(); } - -void Stopped::entry() noexcept { - sigc::slot timer_slot = sigc::mem_fun(*this, &Stopped::on_timer); - timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); - spdlog::debug("mpd: Stopped: enabled 200 ms periodic timer."); -} - -void Stopped::exit() noexcept { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - spdlog::debug("mpd: Stopped: disabled 200 ms periodic timer."); - } -} - -bool Stopped::on_timer() { - bool rc = true; - - // Attempt to connect with MPD. - try { - ctx_->tryConnect(); - - // Success? - if (!ctx_->is_connected()) { - ctx_->setState(std::make_unique(ctx_)); - return false; - } - - ctx_->fetchState(); - - ctx_->emit(); - - if (ctx_->is_stopped()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } else if (ctx_->is_playing()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } else if (ctx_->is_paused()) { - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } - } catch (std::exception const& e) { - spdlog::warn("mpd: Stopped: error: {}", e.what()); - ctx_->setState(std::make_unique(ctx_)); - rc = false; - } - - return rc; -} - -void Stopped::play() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_play(ctx_->connection().get()); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Stopped::pause() { - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - - mpd_run_pause(ctx_->connection().get(), true); - } - - ctx_->setState(std::make_unique(ctx_)); -} - -void Stopped::update() noexcept { ctx_->do_update(); } - -void Disconnected::arm_timer(int interval) noexcept { - // unregister timer, if present - disarm_timer(); - - // register timer - sigc::slot timer_slot = sigc::mem_fun(*this, &Disconnected::on_timer); - timer_connection_ = - Glib::signal_timeout().connect(timer_slot, interval); - spdlog::debug("mpd: Disconnected: enabled interval timer."); -} - -void Disconnected::disarm_timer() noexcept { - // unregister timer, if present - if (timer_connection_.connected()) { - timer_connection_.disconnect(); - spdlog::debug("mpd: Disconnected: disabled interval timer."); - } -} - -void Disconnected::entry() noexcept { - arm_timer(1'000); -} - -void Disconnected::exit() noexcept { - disarm_timer(); -} - -bool Disconnected::on_timer() { - // Attempt to connect with MPD. - try { - ctx_->tryConnect(); - - // Success? - if (ctx_->is_connected()) { - ctx_->fetchState(); - ctx_->emit(); - - if (ctx_->is_playing()) { - ctx_->setState(std::make_unique(ctx_)); - } else if (ctx_->is_paused()) { - ctx_->setState(std::make_unique(ctx_)); - } else { - ctx_->setState(std::make_unique(ctx_)); - } - - return false; // do not rearm timer - } - } catch (std::exception const& e) { - spdlog::warn("mpd: Disconnected: error: {}", e.what()); - } - - arm_timer(ctx_->interval() * 1'000); - - return false; -} - -void Disconnected::update() noexcept { ctx_->do_update(); } - -} // namespace waybar::modules::detail From 8f961ac3976048d7a389d77737d9694f26fe863d Mon Sep 17 00:00:00 2001 From: Joseph Benden Date: Sat, 3 Oct 2020 22:01:51 -0700 Subject: [PATCH 3/4] 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 --- include/factory.hpp | 2 +- include/modules/mpd.hpp | 74 ------ include/modules/mpd/mpd.hpp | 66 ++++++ include/modules/mpd/state.hpp | 217 +++++++++++++++++ include/modules/mpd/state.inl.hpp | 24 ++ meson.build | 3 +- src/modules/{ => mpd}/mpd.cpp | 154 ++++-------- src/modules/mpd/state.cpp | 382 ++++++++++++++++++++++++++++++ 8 files changed, 735 insertions(+), 187 deletions(-) delete mode 100644 include/modules/mpd.hpp create mode 100644 include/modules/mpd/mpd.hpp create mode 100644 include/modules/mpd/state.hpp create mode 100644 include/modules/mpd/state.inl.hpp rename src/modules/{ => mpd}/mpd.cpp (73%) create mode 100644 src/modules/mpd/state.cpp diff --git a/include/factory.hpp b/include/factory.hpp index 3efe6cb..f73c6e1 100644 --- a/include/factory.hpp +++ b/include/factory.hpp @@ -37,7 +37,7 @@ #include "modules/pulseaudio.hpp" #endif #ifdef HAVE_LIBMPDCLIENT -#include "modules/mpd.hpp" +#include "modules/mpd/mpd.hpp" #endif #ifdef HAVE_LIBSNDIO #include "modules/sndio.hpp" diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp deleted file mode 100644 index d08b28b..0000000 --- a/include/modules/mpd.hpp +++ /dev/null @@ -1,74 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#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; - using unique_status = std::unique_ptr; - using unique_song = std::unique_ptr; - - // 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 diff --git a/include/modules/mpd/mpd.hpp b/include/modules/mpd/mpd.hpp new file mode 100644 index 0000000..f92df93 --- /dev/null +++ b/include/modules/mpd/mpd.hpp @@ -0,0 +1,66 @@ +#pragma once + +#include +#include +#include + +#include +#include + +#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 diff --git a/include/modules/mpd/state.hpp b/include/modules/mpd/state.hpp new file mode 100644 index 0000000..3b18159 --- /dev/null +++ b/include/modules/mpd/state.hpp @@ -0,0 +1,217 @@ +#pragma once + +#include +#include +#include + +#include +#include + +#include "ALabel.hpp" + +namespace waybar::modules { +class MPD; +} // namespace waybar::modules + +namespace waybar::modules::detail { + +using unique_connection = std::unique_ptr; +using unique_status = std::unique_ptr; +using unique_song = std::unique_ptr; + +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_; + 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&& 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(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 diff --git a/include/modules/mpd/state.inl.hpp b/include/modules/mpd/state.inl.hpp new file mode 100644 index 0000000..0d83b0b --- /dev/null +++ b/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 diff --git a/meson.build b/meson.build index 023894c..de20382 100644 --- a/meson.build +++ b/meson.build @@ -213,7 +213,8 @@ endif if libmpdclient.found() add_project_arguments('-DHAVE_LIBMPDCLIENT', language: 'cpp') - src_files += 'src/modules/mpd.cpp' + src_files += 'src/modules/mpd/mpd.cpp' + src_files += 'src/modules/mpd/state.cpp' endif if gtk_layer_shell.found() diff --git a/src/modules/mpd.cpp b/src/modules/mpd/mpd.cpp similarity index 73% rename from src/modules/mpd.cpp rename to src/modules/mpd/mpd.cpp index 26878b1..edce941 100644 --- a/src/modules/mpd.cpp +++ b/src/modules/mpd/mpd.cpp @@ -1,8 +1,15 @@ -#include "modules/mpd.hpp" +#include "modules/mpd/mpd.hpp" #include #include +#include "modules/mpd/state.hpp" +#if defined(MPD_NOINLINE) +namespace waybar::modules { +#include "modules/mpd/state.inl.hpp" +} // namespace waybar::modules +#endif + waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) : ALabel(config, "mpd", id, "{album} - {artist} - {title}", 5), module_name_(id.empty() ? "mpd" : "mpd#" + id), @@ -10,7 +17,6 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) port_(config_["port"].isUInt() ? config["port"].asUInt() : 0), timeout_(config_["timeout"].isUInt() ? config_["timeout"].asUInt() * 1'000 : 30'000), connection_(nullptr, &mpd_connection_free), - alternate_connection_(nullptr, &mpd_connection_free), status_(nullptr, &mpd_status_free), song_(nullptr, &mpd_song_free) { if (!config_["port"].isNull() && !config_["port"].isUInt()) { @@ -28,73 +34,33 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) server_ = config["server"].asCString(); } - event_listener().detach(); - event_box_.add_events(Gdk::BUTTON_PRESS_MASK); event_box_.signal_button_press_event().connect(sigc::mem_fun(*this, &MPD::handlePlayPause)); } auto waybar::modules::MPD::update() -> void { - std::lock_guard guard(connection_lock_); - tryConnect(); - - if (connection_ != nullptr) { - try { - bool wasPlaying = playing(); - if(!wasPlaying) { - // Wait until the periodic_updater has stopped - std::lock_guard periodic_guard(periodic_lock_); - } - fetchState(); - if (!wasPlaying && playing()) { - periodic_updater().detach(); - } - } catch (const std::exception& e) { - spdlog::error("{}: {}", module_name_, e.what()); - state_ = MPD_STATE_UNKNOWN; - } - } - - setLabel(); + context_.update(); // Call parent update ALabel::update(); } -std::thread waybar::modules::MPD::event_listener() { - return std::thread([this] { - while (true) { - try { - if (connection_ == nullptr) { - // Retry periodically if no connection - dp.emit(); - std::this_thread::sleep_for(interval_); - } else { - waitForEvent(); - dp.emit(); - } - } catch (const std::exception& e) { - if (strcmp(e.what(), "Connection to MPD closed") == 0) { - spdlog::debug("{}: {}", module_name_, e.what()); - } else { - spdlog::warn("{}: {}", module_name_, e.what()); - } - } +void waybar::modules::MPD::queryMPD() { + if (connection_ != nullptr) { + spdlog::debug("{}: fetching state information", module_name_); + try { + fetchState(); + spdlog::debug("{}: fetch complete", module_name_); + } catch (std::exception const& e) { + spdlog::error("{}: {}", module_name_, e.what()); + state_ = MPD_STATE_UNKNOWN; } - }); + + dp.emit(); + } } -std::thread waybar::modules::MPD::periodic_updater() { - return std::thread([this] { - std::lock_guard guard(periodic_lock_); - while (connection_ != nullptr && playing()) { - dp.emit(); - std::this_thread::sleep_for(std::chrono::seconds(1)); - } - }); -} - -std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) { +std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) const { std::string result = config_["unknown-tag"].isString() ? config_["unknown-tag"].asString() : "N/A"; const char* tag = mpd_song_get_tag(song_.get(), type, idx); @@ -133,7 +99,7 @@ void waybar::modules::MPD::setLabel() { auto format = format_; std::string artist, album_artist, album, title, date; - int song_pos, queue_length; + int song_pos, queue_length; std::chrono::seconds elapsedTime, totalTime; std::string stateIcon = ""; @@ -149,8 +115,8 @@ void waybar::modules::MPD::setLabel() { label_.get_style_context()->add_class("playing"); label_.get_style_context()->remove_class("paused"); } else if (paused()) { - format = - config_["format-paused"].isString() ? config_["format-paused"].asString() : config_["format"].asString(); + format = config_["format-paused"].isString() ? config_["format-paused"].asString() + : config_["format"].asString(); label_.get_style_context()->add_class("paused"); label_.get_style_context()->remove_class("playing"); } @@ -216,7 +182,7 @@ void waybar::modules::MPD::setLabel() { } } -std::string waybar::modules::MPD::getStateIcon() { +std::string waybar::modules::MPD::getStateIcon() const { if (!config_["state-icons"].isObject()) { return ""; } @@ -238,7 +204,7 @@ std::string waybar::modules::MPD::getStateIcon() { } } -std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) { +std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool activated) const { if (!config_[optionName + "-icons"].isObject()) { return ""; } @@ -261,15 +227,11 @@ void waybar::modules::MPD::tryConnect() { } connection_ = - unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); + detail::unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); - alternate_connection_ = - unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free); - - if (connection_ == nullptr || alternate_connection_ == nullptr) { + if (connection_ == nullptr) { spdlog::error("{}: Failed to connect to MPD", module_name_); connection_.reset(); - alternate_connection_.reset(); return; } @@ -279,7 +241,6 @@ void waybar::modules::MPD::tryConnect() { } catch (std::runtime_error& e) { spdlog::error("{}: Failed to connect to MPD: {}", module_name_, e.what()); connection_.reset(); - alternate_connection_.reset(); } } @@ -292,7 +253,6 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { case MPD_ERROR_CLOSED: mpd_connection_clear_error(conn); connection_.reset(); - alternate_connection_.reset(); state_ = MPD_STATE_UNKNOWN; throw std::runtime_error("Connection to MPD closed"); default: @@ -306,37 +266,20 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) { } void waybar::modules::MPD::fetchState() { + if (connection_ == nullptr) { + spdlog::error("{}: Not connected to MPD", module_name_); + return; + } + auto conn = connection_.get(); - status_ = unique_status(mpd_run_status(conn), &mpd_status_free); + + status_ = detail::unique_status(mpd_run_status(conn), &mpd_status_free); checkErrors(conn); + state_ = mpd_status_get_state(status_.get()); checkErrors(conn); - song_ = unique_song(mpd_run_current_song(conn), &mpd_song_free); - checkErrors(conn); -} - -void waybar::modules::MPD::waitForEvent() { - auto conn = alternate_connection_.get(); - // Wait for a player (play/pause), option (random, shuffle, etc.), or playlist - // change - if (!mpd_send_idle_mask( - conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { - checkErrors(conn); - return; - } - // alternate_idle_ = true; - - // See issue #277: - // https://github.com/Alexays/Waybar/issues/277 - mpd_recv_idle(conn, /* disable_timeout = */ false); - // See issue #281: - // https://github.com/Alexays/Waybar/issues/281 - std::lock_guard guard(connection_lock_); - - checkErrors(conn); - mpd_response_finish(conn); - + song_ = detail::unique_song(mpd_run_current_song(conn), &mpd_song_free); checkErrors(conn); } @@ -346,24 +289,13 @@ bool waybar::modules::MPD::handlePlayPause(GdkEventButton* const& e) { } if (e->button == 1) { - std::lock_guard guard(connection_lock_); - if (stopped()) { - mpd_run_play(connection_.get()); - } else { - mpd_run_toggle_pause(connection_.get()); - } + if (state_ == MPD_STATE_PLAY) + context_.pause(); + else + context_.play(); } else if (e->button == 3) { - std::lock_guard guard(connection_lock_); - mpd_run_stop(connection_.get()); + context_.stop(); } return true; } - -bool waybar::modules::MPD::stopped() { - return connection_ == nullptr || state_ == MPD_STATE_UNKNOWN || state_ == MPD_STATE_STOP || status_ == nullptr; -} - -bool waybar::modules::MPD::playing() { return connection_ != nullptr && state_ == MPD_STATE_PLAY; } - -bool waybar::modules::MPD::paused() { return connection_ != nullptr && state_ == MPD_STATE_PAUSE; } diff --git a/src/modules/mpd/state.cpp b/src/modules/mpd/state.cpp new file mode 100644 index 0000000..570c167 --- /dev/null +++ b/src/modules/mpd/state.cpp @@ -0,0 +1,382 @@ +#include "modules/mpd/state.hpp" + +#include +#include + +#include "modules/mpd/mpd.hpp" +#if defined(MPD_NOINLINE) +namespace waybar::modules { +#include "modules/mpd/state.inl.hpp" +} // namespace waybar::modules +#endif + +namespace waybar::modules::detail { + +#define IDLE_RUN_NOIDLE_AND_CMD(...) \ + if (idle_connection_.connected()) { \ + idle_connection_.disconnect(); \ + auto conn = ctx_->connection().get(); \ + if (!mpd_run_noidle(conn)) { \ + if (mpd_connection_get_error(conn) != MPD_ERROR_SUCCESS) { \ + spdlog::error("mpd: Idle: failed to unregister for IDLE events"); \ + ctx_->checkErrors(conn); \ + } \ + } \ + __VA_ARGS__; \ + } + +void Idle::play() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_play(conn)); + + ctx_->setState(std::make_unique(ctx_)); +} + +void Idle::pause() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_pause(conn, true)); + + ctx_->setState(std::make_unique(ctx_)); +} + +void Idle::stop() { + IDLE_RUN_NOIDLE_AND_CMD(mpd_run_stop(conn)); + + ctx_->setState(std::make_unique(ctx_)); +} + +#undef IDLE_RUN_NOIDLE_AND_CMD + +void Idle::update() noexcept { + // This is intentionally blank. +} + +void Idle::entry() noexcept { + auto conn = ctx_->connection().get(); + assert(conn != nullptr); + + if (!mpd_send_idle_mask( + conn, static_cast(MPD_IDLE_PLAYER | MPD_IDLE_OPTIONS | MPD_IDLE_QUEUE))) { + ctx_->checkErrors(conn); + spdlog::error("mpd: Idle: failed to register for IDLE events"); + } else { + spdlog::debug("mpd: Idle: watching FD"); + sigc::slot idle_slot = sigc::mem_fun(*this, &Idle::on_io); + idle_connection_ = + Glib::signal_io().connect(idle_slot, + mpd_connection_get_fd(conn), + Glib::IO_IN | Glib::IO_PRI | Glib::IO_ERR | Glib::IO_HUP); + } +} + +void Idle::exit() noexcept { + if (idle_connection_.connected()) { + idle_connection_.disconnect(); + spdlog::debug("mpd: Idle: unwatching FD"); + } +} + +bool Idle::on_io(Glib::IOCondition const&) { + auto conn = ctx_->connection().get(); + + // callback should do this: + enum mpd_idle events = mpd_recv_idle(conn, /* ignore_timeout?= */ false); + spdlog::debug("mpd: Idle: recv_idle events -> {}", events); + + mpd_response_finish(conn); + try { + ctx_->checkErrors(conn); + } catch (std::exception const& e) { + spdlog::warn("mpd: Idle: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + mpd_state state = ctx_->state(); + + if (state == MPD_STATE_STOP) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else if (state == MPD_STATE_PLAY) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else if (state == MPD_STATE_PAUSE) { + ctx_->emit(); + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->emit(); + // self transition + ctx_->setState(std::make_unique(ctx_)); + } + + return false; +} + +void Playing::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Playing::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 1'000); + spdlog::debug("mpd: Playing: enabled 1 second periodic timer."); +} + +void Playing::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Playing: disabled 1 second periodic timer."); + } +} + +bool Playing::on_timer() { + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + if (!ctx_->is_playing()) { + if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->setState(std::make_unique(ctx_)); + } + return false; + } + + ctx_->queryMPD(); + ctx_->emit(); + } catch (std::exception const& e) { + spdlog::warn("mpd: Playing: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + return true; +} + +void Playing::stop() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_stop(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Playing::pause() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_pause(ctx_->connection().get(), true); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Playing::update() noexcept { ctx_->do_update(); } + +void Paused::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Paused::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); + spdlog::debug("mpd: Paused: enabled 200 ms periodic timer."); +} + +void Paused::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Paused: disabled 200 ms periodic timer."); + } +} + +bool Paused::on_timer() { + bool rc = true; + + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + ctx_->emit(); + + if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_stopped()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Paused: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + + return rc; +} + +void Paused::play() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_play(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Paused::stop() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_stop(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Paused::update() noexcept { ctx_->do_update(); } + +void Stopped::entry() noexcept { + sigc::slot timer_slot = sigc::mem_fun(*this, &Stopped::on_timer); + timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 200); + spdlog::debug("mpd: Stopped: enabled 200 ms periodic timer."); +} + +void Stopped::exit() noexcept { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Stopped: disabled 200 ms periodic timer."); + } +} + +bool Stopped::on_timer() { + bool rc = true; + + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (!ctx_->is_connected()) { + ctx_->setState(std::make_unique(ctx_)); + return false; + } + + ctx_->fetchState(); + + ctx_->emit(); + + if (ctx_->is_stopped()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } else if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Stopped: error: {}", e.what()); + ctx_->setState(std::make_unique(ctx_)); + rc = false; + } + + return rc; +} + +void Stopped::play() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_play(ctx_->connection().get()); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Stopped::pause() { + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + + mpd_run_pause(ctx_->connection().get(), true); + } + + ctx_->setState(std::make_unique(ctx_)); +} + +void Stopped::update() noexcept { ctx_->do_update(); } + +void Disconnected::arm_timer(int interval) noexcept { + // unregister timer, if present + disarm_timer(); + + // register timer + sigc::slot timer_slot = sigc::mem_fun(*this, &Disconnected::on_timer); + timer_connection_ = + Glib::signal_timeout().connect(timer_slot, interval); + spdlog::debug("mpd: Disconnected: enabled interval timer."); +} + +void Disconnected::disarm_timer() noexcept { + // unregister timer, if present + if (timer_connection_.connected()) { + timer_connection_.disconnect(); + spdlog::debug("mpd: Disconnected: disabled interval timer."); + } +} + +void Disconnected::entry() noexcept { + arm_timer(1'000); +} + +void Disconnected::exit() noexcept { + disarm_timer(); +} + +bool Disconnected::on_timer() { + // Attempt to connect with MPD. + try { + ctx_->tryConnect(); + + // Success? + if (ctx_->is_connected()) { + ctx_->fetchState(); + ctx_->emit(); + + if (ctx_->is_playing()) { + ctx_->setState(std::make_unique(ctx_)); + } else if (ctx_->is_paused()) { + ctx_->setState(std::make_unique(ctx_)); + } else { + ctx_->setState(std::make_unique(ctx_)); + } + + return false; // do not rearm timer + } + } catch (std::exception const& e) { + spdlog::warn("mpd: Disconnected: error: {}", e.what()); + } + + arm_timer(ctx_->interval() * 1'000); + + return false; +} + +void Disconnected::update() noexcept { ctx_->do_update(); } + +} // namespace waybar::modules::detail From 587eb5fdb4d7de2891fa0a874f7c4ea0c9d4d882 Mon Sep 17 00:00:00 2001 From: Joseph Benden Date: Mon, 19 Oct 2020 11:54:36 -0700 Subject: [PATCH 4/4] mpd: support password protected MPD - Add MPD module option `password`, and document it. - Add logic to send the password, directly after connecting to MPD. Fixes: #576 Signed-off-by: Joseph Benden --- include/modules/mpd/mpd.hpp | 7 ++++--- man/waybar-mpd.5.scd | 4 ++++ src/modules/mpd/mpd.cpp | 11 +++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/include/modules/mpd/mpd.hpp b/include/modules/mpd/mpd.hpp index f92df93..0fc1ce9 100644 --- a/include/modules/mpd/mpd.hpp +++ b/include/modules/mpd/mpd.hpp @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include #include @@ -22,8 +22,9 @@ class MPD : public ALabel { // 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_; + const char* server_; + const unsigned port_; + const std::string password_; unsigned timeout_; diff --git a/man/waybar-mpd.5.scd b/man/waybar-mpd.5.scd index e8105de..8c33c62 100644 --- a/man/waybar-mpd.5.scd +++ b/man/waybar-mpd.5.scd @@ -20,6 +20,10 @@ Addressed by *mpd* typeof: integer ++ The port MPD listens to. If empty, use the default port. +*password*: ++ + typeof: string ++ + The password required to connect to the MPD server. If empty, no password is sent to MPD. + *interval*: ++ typeof: integer++ default: 5 ++ diff --git a/src/modules/mpd/mpd.cpp b/src/modules/mpd/mpd.cpp index edce941..6dc24e7 100644 --- a/src/modules/mpd/mpd.cpp +++ b/src/modules/mpd/mpd.cpp @@ -15,6 +15,7 @@ waybar::modules::MPD::MPD(const std::string& id, const Json::Value& config) module_name_(id.empty() ? "mpd" : "mpd#" + id), server_(nullptr), port_(config_["port"].isUInt() ? config["port"].asUInt() : 0), + password_(config_["password"].empty() ? "" : config_["password"].asString()), timeout_(config_["timeout"].isUInt() ? config_["timeout"].asUInt() * 1'000 : 30'000), connection_(nullptr, &mpd_connection_free), status_(nullptr, &mpd_status_free), @@ -238,6 +239,16 @@ void waybar::modules::MPD::tryConnect() { try { checkErrors(connection_.get()); spdlog::debug("{}: Connected to MPD", module_name_); + + if (!password_.empty()) { + bool res = mpd_run_password(connection_.get(), password_.c_str()); + if (!res) { + spdlog::error("{}: Wrong MPD password", module_name_); + connection_.reset(); + return; + } + checkErrors(connection_.get()); + } } catch (std::runtime_error& e) { spdlog::error("{}: Failed to connect to MPD: {}", module_name_, e.what()); connection_.reset();