From 570c0d4a18f831c2fcb4c7dedd1081217c39f8d9 Mon Sep 17 00:00:00 2001 From: ralismark <13449732+ralismark@users.noreply.github.com> Date: Sat, 9 Mar 2024 17:37:06 +1100 Subject: [PATCH] Various improvements --- Cargo.lock | 1 + crates/eww/Cargo.toml | 1 + crates/eww/src/widgets/systray.rs | 4 +- crates/notifier_host/src/host.rs | 9 +++-- crates/notifier_host/src/icon.rs | 2 +- crates/notifier_host/src/item.rs | 60 +++++++++++++++-------------- crates/notifier_host/src/lib.rs | 9 +++-- crates/notifier_host/src/watcher.rs | 55 +++++++++++++++----------- 8 files changed, 80 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a892132e..0ab6a187 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -950,6 +950,7 @@ dependencies = [ "notifier_host", "notify", "once_cell", + "ordered-stream", "pretty_env_logger", "regex", "serde", diff --git a/crates/eww/Cargo.toml b/crates/eww/Cargo.toml index 78abc8e9..2aa461a4 100644 --- a/crates/eww/Cargo.toml +++ b/crates/eww/Cargo.toml @@ -36,6 +36,7 @@ gdkx11 = { version = "0.17", optional = true } x11rb = { version = "0.11.1", features = ["randr"], optional = true } zbus = { version = "3.7.0", default-features = false, features = ["tokio"] } +ordered-stream = "0.2.0" anyhow.workspace = true bincode.workspace = true diff --git a/crates/eww/src/widgets/systray.rs b/crates/eww/src/widgets/systray.rs index 1a40ba90..2360abed 100644 --- a/crates/eww/src/widgets/systray.rs +++ b/crates/eww/src/widgets/systray.rs @@ -1,5 +1,6 @@ use gtk::{cairo::Surface, gdk::ffi::gdk_cairo_surface_create_from_pixbuf, prelude::*}; -use notifier_host::{self, export::ordered_stream::OrderedStreamExt}; +use notifier_host; +use futures::StreamExt; // DBus state shared between systray instances, to avoid creating too many connections etc. struct DBusSession { @@ -12,7 +13,6 @@ async fn dbus_session() -> zbus::Result<&'static DBusSession> { static DBUS_STATE: tokio::sync::OnceCell = tokio::sync::OnceCell::const_new(); DBUS_STATE .get_or_try_init(|| async { - // TODO error handling? let con = zbus::Connection::session().await?; notifier_host::Watcher::new().attach_to(&con).await?; diff --git a/crates/notifier_host/src/host.rs b/crates/notifier_host/src/host.rs index d1bb3824..b937395c 100644 --- a/crates/notifier_host/src/host.rs +++ b/crates/notifier_host/src/host.rs @@ -26,7 +26,9 @@ pub trait Host { /// /// You still need to call [`run_host`] to have the instance of [`Host`] be notified of new and /// removed items. -pub async fn register_as_host(con: &zbus::Connection) -> zbus::Result<(zbus::names::WellKnownName<'static>, proxy::StatusNotifierWatcherProxy<'static>)> { +pub async fn register_as_host( + con: &zbus::Connection, +) -> zbus::Result<(zbus::names::WellKnownName<'static>, proxy::StatusNotifierWatcherProxy<'static>)> { let snw = proxy::StatusNotifierWatcherProxy::new(con).await?; // get a well-known name @@ -62,8 +64,7 @@ pub async fn register_as_host(con: &zbus::Connection) -> zbus::Result<(zbus::nam /// This async function runs forever, and only returns if it gets an error! As such, it is /// recommended to call this via something like `tokio::spawn` that runs this in the /// background. -pub async fn run_host(host: &mut dyn Host, snw: &proxy::StatusNotifierWatcherProxy<'static>) -> zbus::Error -{ +pub async fn run_host(host: &mut dyn Host, snw: &proxy::StatusNotifierWatcherProxy<'static>) -> zbus::Error { // Replacement for ? operator since we're not returning a Result. macro_rules! try_ { ($e:expr) => { @@ -71,7 +72,7 @@ pub async fn run_host(host: &mut dyn Host, snw: &proxy::StatusNotifierWatcherPro Ok(x) => x, Err(e) => return e, } - } + }; } enum ItemEvent { diff --git a/crates/notifier_host/src/icon.rs b/crates/notifier_host/src/icon.rs index 1c29af8d..bcc994f2 100644 --- a/crates/notifier_host/src/icon.rs +++ b/crates/notifier_host/src/icon.rs @@ -3,7 +3,7 @@ use crate::*; use gtk::{self, prelude::*}; #[derive(thiserror::Error, Debug)] -pub enum IconError { +enum IconError { #[error("while fetching icon name: {0}")] DBusIconName(#[source] zbus::Error), #[error("while fetching icon theme path: {0}")] diff --git a/crates/notifier_host/src/item.rs b/crates/notifier_host/src/item.rs index 45acfba4..c258957e 100644 --- a/crates/notifier_host/src/item.rs +++ b/crates/notifier_host/src/item.rs @@ -2,11 +2,9 @@ use crate::*; use gtk::{self, prelude::*}; -/// Recognised values of org.freedesktop.StatusNotifierItem.Status +/// Recognised values of [`org.freedesktop.StatusNotifierItem.Status`]. /// -/// See -/// -/// for details. +/// [`org.freedesktop.StatusNotifierItem.Status`]: https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/#org.freedesktop.statusnotifieritem.status #[derive(Debug, Clone, Copy)] pub enum Status { /// The item doesn't convey important information to the user, it can be considered an "idle" @@ -37,35 +35,37 @@ impl std::str::FromStr for Status { } } -/// Split a sevice name e.g. `:1.50:/org/ayatana/NotificationItem/nm_applet` into the address and -/// path. +/// A StatusNotifierItem (SNI). /// -/// Original logic from -fn split_service_name(service: &str) -> zbus::Result<(String, String)> { - if let Some((addr, path)) = service.split_once('/') { - Ok((addr.to_owned(), format!("/{}", path))) - } else if service.contains(':') { - // TODO why? - let addr = service.split(':').nth(1); - // Some StatusNotifierItems will not return an object path, in that case we fallback - // to the default path. - if let Some(addr) = addr { - Ok((addr.to_owned(), "/StatusNotifierItem".to_owned())) - } else { - Err(zbus::Error::Address(service.to_owned())) - } - } else { - Err(zbus::Error::Address(service.to_owned())) - } -} - +/// At the moment, this does not wrap much of the SNI's properties and methods. As such, you should +/// directly access the `sni` member as needed for functionalty that is not provided. pub struct Item { + /// The StatusNotifierItem that is wrapped by this instance. pub sni: proxy::StatusNotifierItemProxy<'static>, } impl Item { - pub async fn from_address(con: &zbus::Connection, addr: &str) -> zbus::Result { - let (addr, path) = split_service_name(addr)?; + /// Create an instance from the service's address. + /// + /// The format of `addr` is `{bus}{object_path}` (e.g. + /// `:1.50/org/ayatana/NotificationItem/nm_applet`), which is the format that is used for + /// StatusNotifierWatcher's [RegisteredStatusNotifierItems property][rsni]). + /// + /// [rsni]: https://freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierWatcher/#registeredstatusnotifieritems + pub async fn from_address(con: &zbus::Connection, service: &str) -> zbus::Result { + let (addr, path) = { + // Based on + // + // TODO is the service name format actually documented anywhere? + if let Some((addr, path)) = service.split_once('/') { + (addr.to_owned(), format!("/{}", path)) + } else if service.starts_with(':') { + (service[0..6].to_owned(), names::ITEM_OBJECT.to_owned()) + } else { + return Err(zbus::Error::Address(service.to_owned())); + } + }; + let sni = proxy::StatusNotifierItemProxy::builder(con).destination(addr)?.path(path)?.build().await?; Ok(Item { sni }) @@ -80,13 +80,17 @@ impl Item { } } + /// Get the menu of this item. pub async fn menu(&self) -> zbus::Result { - // TODO better handling if menu() method doesn't exist + // TODO document what this returns if there is no menu. let menu = dbusmenu_gtk3::Menu::new(self.sni.destination(), &self.sni.menu().await?); Ok(menu.upcast()) } + /// Get the current icon. pub async fn icon(&self, size: i32, scale: i32) -> Option { + // TODO explain what size and scale mean here + // see icon.rs load_icon_from_sni(&self.sni, size, scale).await } diff --git a/crates/notifier_host/src/lib.rs b/crates/notifier_host/src/lib.rs index c636cf0a..8a8c46dd 100644 --- a/crates/notifier_host/src/lib.rs +++ b/crates/notifier_host/src/lib.rs @@ -25,7 +25,7 @@ //! //! The actual tray implements the [`Host`] trait to be notified of when items (called //! `StatusNotifierItem` in the spec and represented by [`Item`]) appear and disappear, then calls -//! [`run_host_forever`] to run the DBus side of the protocol. +//! [`run_host`] to run the DBus side of the protocol. //! //! If there are multiple trays running on the system, there can be multiple `StatusNotifierHost`s, //! but only one `StatusNotifierWatcher` (usually from whatever tray was started first). @@ -44,6 +44,9 @@ pub use item::*; mod watcher; pub use watcher::*; -pub mod export { - pub use zbus::export::ordered_stream; +pub(crate) mod names { + pub const WATCHER_BUS: &str = "org.kde.StatusNotifierWatcher"; + pub const WATCHER_OBJECT: &str = "/StatusNotifierWatcher"; + + pub const ITEM_OBJECT: &str = "/StatusNotifierItem"; } diff --git a/crates/notifier_host/src/watcher.rs b/crates/notifier_host/src/watcher.rs index dc75c8fb..a0e7c139 100644 --- a/crates/notifier_host/src/watcher.rs +++ b/crates/notifier_host/src/watcher.rs @@ -1,12 +1,8 @@ -use crate::*; - +use crate::names; use zbus::{dbus_interface, export::ordered_stream::OrderedStreamExt, Interface}; -pub const WATCHER_BUS_NAME: &str = "org.kde.StatusNotifierWatcher"; -pub const WATCHER_OBJECT_NAME: &str = "/StatusNotifierWatcher"; - /// An instance of [`org.kde.StatusNotifierWatcher`]. It only tracks what tray items and trays -/// exist, and doesn't have any logic for displaying items (for that, see [`Host`]). +/// exist, and doesn't have any logic for displaying items (for that, see [`Host`][`crate::Host`]). /// /// While this is usually run alongside the tray, it can also be used standalone. /// @@ -15,8 +11,8 @@ pub const WATCHER_OBJECT_NAME: &str = "/StatusNotifierWatcher"; pub struct Watcher { tasks: tokio::task::JoinSet<()>, - // NOTE Intentionally using std::sync::Mutex instead of tokio's async mutex, since we don't - // need to hold the mutex across an await. + // Intentionally using std::sync::Mutex instead of tokio's async mutex, since we don't need to + // hold the mutex across an await. // // See hosts: std::sync::Arc>>, @@ -37,6 +33,13 @@ impl Watcher { #[zbus(connection)] con: &zbus::Connection, #[zbus(signal_context)] ctxt: zbus::SignalContext<'_>, ) -> zbus::fdo::Result<()> { + // TODO right now, we convert everything to the unique bus name (something like :1.234). + // However, it might make more sense to listen to the actual name they give us, so that if + // the connection dissociates itself from the org.kde.StatusNotifierHost-{pid}-{nr} name + // but still remains around, we drop them as a host. + // + // (This also applies to RegisterStatusNotifierItem) + let (service, _) = parse_service(service, hdr, con).await?; log::info!("new host: {}", service); @@ -60,7 +63,7 @@ impl Watcher { let ctxt = ctxt.to_owned(); let con = con.to_owned(); async move { - if let Err(e) = wait_for_service_exit(con.clone(), service.as_ref().into()).await { + if let Err(e) = wait_for_service_exit(&con, service.as_ref().into()).await { log::error!("failed to wait for service exit: {}", e); } log::info!("lost host: {}", service); @@ -133,7 +136,7 @@ impl Watcher { let ctxt = ctxt.to_owned(); let con = con.to_owned(); async move { - if let Err(e) = wait_for_service_exit(con.clone(), service.as_ref()).await { + if let Err(e) = wait_for_service_exit(&con, service.as_ref()).await { log::error!("failed to wait for service exit: {}", e); } println!("gone item: {}", &item); @@ -185,18 +188,18 @@ impl Watcher { Default::default() } - /// Attach and run the Watcher on a connection. + /// Attach and run the Watcher (in the background) on a connection. pub async fn attach_to(self, con: &zbus::Connection) -> zbus::Result<()> { - if !con.object_server().at(WATCHER_OBJECT_NAME, self).await? { + if !con.object_server().at(names::WATCHER_OBJECT, self).await? { return Err(zbus::Error::Failure(format!( "Object already exists at {} on this connection -- is StatusNotifierWatcher already running?", - WATCHER_OBJECT_NAME + names::WATCHER_OBJECT ))); } // not AllowReplacement, not ReplaceExisting, not DoNotQueue let flags: [zbus::fdo::RequestNameFlags; 0] = []; - match con.request_name_with_flags(WATCHER_BUS_NAME, flags.into_iter().collect()).await { + match con.request_name_with_flags(names::WATCHER_BUS, flags.into_iter().collect()).await { Ok(zbus::fdo::RequestNameReply::PrimaryOwner) => Ok(()), Ok(_) | Err(zbus::Error::NameTaken) => Ok(()), // defer to existing Err(e) => Err(e), @@ -215,7 +218,7 @@ impl Watcher { .await } - /// Equivalen to `registered_status_notifier_items_invalidate`, but without requiring `self`. + /// Equivalent to `registered_status_notifier_items_invalidate`, but without requiring `self`. async fn registered_status_notifier_items_refresh(ctxt: &zbus::SignalContext<'_>) -> zbus::Result<()> { zbus::fdo::Properties::properties_changed( ctxt, @@ -231,13 +234,16 @@ impl Watcher { /// name](https://dbus2.github.io/zbus/concepts.html#bus-name--service-name) and the [object /// path](https://dbus2.github.io/zbus/concepts.html#objects-and-object-paths) within the /// connection. +/// +/// The freedesktop.org specification has the format of this be just the bus name, however some +/// status items pass non-conforming values. One common one is just the object path. async fn parse_service<'a>( service: &'a str, hdr: zbus::MessageHeader<'_>, con: &zbus::Connection, ) -> zbus::fdo::Result<(zbus::names::UniqueName<'static>, &'a str)> { if service.starts_with('/') { - // they sent us just the object path :( + // they sent us just the object path if let Some(sender) = hdr.sender()? { Ok((sender.to_owned(), service)) } else { @@ -245,6 +251,7 @@ async fn parse_service<'a>( Err(zbus::fdo::Error::InvalidArgs("Unknown bus address".into())) } } else { + // parse the bus name they gave us let busname: zbus::names::BusName = match service.try_into() { Ok(x) => x, Err(e) => { @@ -254,11 +261,14 @@ async fn parse_service<'a>( }; if let zbus::names::BusName::Unique(unique) = busname { - Ok((unique.to_owned(), "/StatusNotifierItem")) + Ok((unique.to_owned(), names::ITEM_OBJECT)) } else { + // they gave us a "well-known name" like org.kde.StatusNotifierHost-81830-0, we need to + // convert this into the actual identifier for their bus (e.g. :1.234), so that even if + // they remove that well-known name it's fine. let dbus = zbus::fdo::DBusProxy::new(con).await?; match dbus.get_name_owner(busname).await { - Ok(owner) => Ok((owner.into_inner(), "/StatusNotifierItem")), + Ok(owner) => Ok((owner.into_inner(), names::ITEM_OBJECT)), Err(e) => { log::warn!("failed to get owner of {:?}: {}", service, e); Err(e) @@ -268,14 +278,13 @@ async fn parse_service<'a>( } } -/// Wait for a DBus service to exit -async fn wait_for_service_exit(connection: zbus::Connection, service: zbus::names::BusName<'_>) -> zbus::fdo::Result<()> { - // TODO do we also want to catch when the object disappears? - - let dbus = zbus::fdo::DBusProxy::new(&connection).await?; +/// Wait for a DBus service to disappear +async fn wait_for_service_exit(con: &zbus::Connection, service: zbus::names::BusName<'_>) -> zbus::fdo::Result<()> { + let dbus = zbus::fdo::DBusProxy::new(con).await?; let mut owner_changes = dbus.receive_name_owner_changed_with_args(&[(0, &service)]).await?; if !dbus.name_has_owner(service.as_ref()).await? { + // service has already disappeared return Ok(()); }