From 2dfef1c213fe008e3520b168ba96f59db6d55d58 Mon Sep 17 00:00:00 2001 From: Rowan Leeder Date: Wed, 25 Sep 2024 04:03:31 +1000 Subject: [PATCH] Issue-3092 Add node type to wireplumber logs - The module only fetches nodes for "node-type". This causes the 'onMixerChanged' log to spam whenever two or more wireplumber modules were registered on different nodes. To reduce this the unknown node warning will now only print if the node is not the focus of any current module. --- include/modules/wireplumber.hpp | 2 + src/modules/wireplumber.cpp | 81 ++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/include/modules/wireplumber.hpp b/include/modules/wireplumber.hpp index eb39653a..e0033e8a 100644 --- a/include/modules/wireplumber.hpp +++ b/include/modules/wireplumber.hpp @@ -32,6 +32,8 @@ class Wireplumber : public ALabel { bool handleScroll(GdkEventScroll* e) override; + static std::list modules; + WpCore* wp_core_; GPtrArray* apis_; WpObjectManager* om_; diff --git a/src/modules/wireplumber.cpp b/src/modules/wireplumber.cpp index d6d1e594..c25b3b51 100644 --- a/src/modules/wireplumber.cpp +++ b/src/modules/wireplumber.cpp @@ -4,6 +4,8 @@ bool isValidNodeId(uint32_t id) { return id > 0 && id < G_MAXUINT32; } +std::list waybar::modules::Wireplumber::modules; + waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Value& config) : ALabel(config, "wireplumber", id, "{volume}%"), wp_core_(nullptr), @@ -18,6 +20,8 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val min_step_(0.0), node_id_(0), type_(nullptr) { + waybar::modules::Wireplumber::modules.push_back(this); + wp_init(WP_INIT_PIPEWIRE); wp_core_ = wp_core_new(nullptr, nullptr, nullptr); apis_ = g_ptr_array_new_with_free_func(g_object_unref); @@ -28,14 +32,14 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val prepare(this); - spdlog::debug("[{}]: connecting to pipewire...", name_); + spdlog::debug("[{}]: connecting to pipewire: '{}'...", name_, type_); if (wp_core_connect(wp_core_) == 0) { - spdlog::error("[{}]: Could not connect to PipeWire", name_); + spdlog::error("[{}]: Could not connect to PipeWire: '{}'", name_, type_); throw std::runtime_error("Could not connect to PipeWire\n"); } - spdlog::debug("[{}]: connected!", name_); + spdlog::debug("[{}]: {} connected!", name_, type_); g_signal_connect_swapped(om_, "installed", (GCallback)onObjectManagerInstalled, this); @@ -43,6 +47,7 @@ waybar::modules::Wireplumber::Wireplumber(const std::string& id, const Json::Val } waybar::modules::Wireplumber::~Wireplumber() { + waybar::modules::Wireplumber::modules.remove(this); wp_core_disconnect(wp_core_); g_clear_pointer(&apis_, g_ptr_array_unref); g_clear_object(&om_); @@ -54,10 +59,11 @@ waybar::modules::Wireplumber::~Wireplumber() { } void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* self, uint32_t id) { - spdlog::debug("[{}]: updating node name with node.id {}", self->name_, id); + spdlog::debug("[{}]: updating '{}' node name with node.id {}", self->name_, self->type_, id); if (!isValidNodeId(id)) { - spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring node name update.", self->name_, id); + spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring '{}' node name update.", self->name_, + id, self->type_); return; } @@ -85,7 +91,7 @@ void waybar::modules::Wireplumber::updateNodeName(waybar::modules::Wireplumber* self->node_name_ = nick != nullptr ? nick : description != nullptr ? description : "Unknown node name"; - spdlog::debug("[{}]: Updating node name to: {}", self->name_, self->node_name_); + spdlog::debug("[{}]: Updating '{}' node name to: {}", self->name_, self->type_, self->node_name_); } void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* self, uint32_t id) { @@ -93,7 +99,8 @@ void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* se GVariant* variant = nullptr; if (!isValidNodeId(id)) { - spdlog::error("[{}]: '{}' is not a valid node ID. Ignoring volume update.", self->name_, id); + spdlog::error("[{}]: '{}' is not a valid '{}' node ID. Ignoring volume update.", self->name_, + id, self->type_); return; } @@ -114,13 +121,22 @@ void waybar::modules::Wireplumber::updateVolume(waybar::modules::Wireplumber* se } void waybar::modules::Wireplumber::onMixerChanged(waybar::modules::Wireplumber* self, uint32_t id) { - spdlog::debug("[{}]: (onMixerChanged) - id: {}", self->name_, id); - g_autoptr(WpNode) node = static_cast(wp_object_manager_lookup( self->om_, WP_TYPE_NODE, WP_CONSTRAINT_TYPE_G_PROPERTY, "bound-id", "=u", id, nullptr)); if (node == nullptr) { - spdlog::warn("[{}]: (onMixerChanged) - Object with id {} not found", self->name_, id); + // log a warning only if no other widget is targeting the id. + // this reduces log spam when multiple instances of the module are used on different node types. + if (id != self->node_id_) { + for (auto const& module : waybar::modules::Wireplumber::modules) { + if (module->node_id_ == id) { + return; + } + } + } + + spdlog::warn("[{}]: (onMixerChanged: {}) - Object with id {} not found", self->name_, + self->type_, id); return; } @@ -128,26 +144,27 @@ void waybar::modules::Wireplumber::onMixerChanged(waybar::modules::Wireplumber* if (self->node_id_ != id) { spdlog::debug( - "[{}]: (onMixerChanged) - ignoring mixer update for node: id: {}, name: {} as it is not " - "the default node: {} with id: {}", - self->name_, id, name, self->default_node_name_, self->node_id_); + "[{}]: (onMixerChanged: {}) - ignoring mixer update for node: id: {}, name: {} as it is " + "not the default node: {} with id: {}", + self->name_, self->type_, id, name, self->default_node_name_, self->node_id_); return; } - spdlog::debug("[{}]: (onMixerChanged) - Need to update volume for node with id {} and name {}", - self->name_, id, name); + spdlog::debug( + "[{}]: (onMixerChanged: {}) - Need to update volume for node with id {} and name {}", + self->name_, self->type_, id, name); updateVolume(self, id); } void waybar::modules::Wireplumber::onDefaultNodesApiChanged(waybar::modules::Wireplumber* self) { - spdlog::debug("[{}]: (onDefaultNodesApiChanged)", self->name_); + spdlog::debug("[{}]: (onDefaultNodesApiChanged: {})", self->name_, self->type_); uint32_t defaultNodeId; g_signal_emit_by_name(self->def_nodes_api_, "get-default-node", self->type_, &defaultNodeId); if (!isValidNodeId(defaultNodeId)) { - spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring node change.", self->name_, - defaultNodeId); + spdlog::warn("[{}]: '{}' is not a valid node ID. Ignoring '{}' node change.", self->name_, + defaultNodeId, self->type_); return; } @@ -156,8 +173,8 @@ void waybar::modules::Wireplumber::onDefaultNodesApiChanged(waybar::modules::Wir "=u", defaultNodeId, nullptr)); if (node == nullptr) { - spdlog::warn("[{}]: (onDefaultNodesApiChanged) - Object with id {} not found", self->name_, - defaultNodeId); + spdlog::warn("[{}]: (onDefaultNodesApiChanged: {}) - Object with id {} not found", self->name_, + self->type_, defaultNodeId); return; } @@ -165,21 +182,22 @@ void waybar::modules::Wireplumber::onDefaultNodesApiChanged(waybar::modules::Wir wp_pipewire_object_get_property(WP_PIPEWIRE_OBJECT(node), "node.name"); spdlog::debug( - "[{}]: (onDefaultNodesApiChanged) - got the following default node: Node(name: {}, id: {})", - self->name_, defaultNodeName, defaultNodeId); + "[{}]: (onDefaultNodesApiChanged: {}) - got the following default node: Node(name: {}, id: " + "{})", + self->name_, self->type_, defaultNodeName, defaultNodeId); if (g_strcmp0(self->default_node_name_, defaultNodeName) == 0 && self->node_id_ == defaultNodeId) { spdlog::debug( - "[{}]: (onDefaultNodesApiChanged) - Default node has not changed. Node(name: {}, id: {}). " - "Ignoring.", - self->name_, self->default_node_name_, defaultNodeId); + "[{}]: (onDefaultNodesApiChanged: {}) - Default node has not changed. Node(name: {}, id: " + "{}). Ignoring.", + self->name_, self->type_, self->default_node_name_, defaultNodeId); return; } spdlog::debug( - "[{}]: (onDefaultNodesApiChanged) - Default node changed to -> Node(name: {}, id: {})", - self->name_, defaultNodeName, defaultNodeId); + "[{}]: (onDefaultNodesApiChanged: {}) - Default node changed to -> Node(name: {}, id: {})", + self->name_, self->type_, defaultNodeName, defaultNodeId); g_free(self->default_node_name_); self->default_node_name_ = g_strdup(defaultNodeName); @@ -210,8 +228,9 @@ void waybar::modules::Wireplumber::onObjectManagerInstalled(waybar::modules::Wir g_signal_emit_by_name(self->def_nodes_api_, "get-default-node", self->type_, &self->node_id_); if (self->default_node_name_ != nullptr) { - spdlog::debug("[{}]: (onObjectManagerInstalled) - default configured node name: {} and id: {}", - self->name_, self->default_node_name_, self->node_id_); + spdlog::debug( + "[{}]: (onObjectManagerInstalled: {}) - default configured node name: {} and id: {}", + self->name_, self->type_, self->default_node_name_, self->node_id_); } updateVolume(self, self->node_id_); @@ -248,8 +267,8 @@ void waybar::modules::Wireplumber::activatePlugins() { } } -void waybar::modules::Wireplumber::prepare() { - spdlog::debug("[{}]: preparing object manager", name_); +void waybar::modules::Wireplumber::prepare(waybar::modules::Wireplumber* self) { + spdlog::debug("[{}]: preparing object manager: '{}'", name_, self->type_); wp_object_manager_add_interest(om_, WP_TYPE_NODE, WP_CONSTRAINT_TYPE_PW_PROPERTY, "media.class", "=s", self->type_, nullptr); }