From 9700fdc9b0495a14b26dde57060381730767018b Mon Sep 17 00:00:00 2001 From: Dave Patrick Caberto Date: Wed, 8 Nov 2023 14:06:03 +0800 Subject: [PATCH] refactor!: allow more control in PropertiesChanged signal emissions --- Cargo.toml | 1 - README.md | 7 ++- examples/local_server.rs | 5 ++- examples/server.rs | 5 ++- src/lib.rs | 1 - src/local_server.rs | 31 +++++++------ src/player.rs | 96 ++++++++++++++++++++++++++++------------ src/property.rs | 69 ++++++++++++++--------------- src/server.rs | 79 ++++++++++++++++----------------- 9 files changed, 167 insertions(+), 127 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6d21810..d483c83 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,6 @@ edition = "2021" [dependencies] async-trait = "0.1" -enumflags2 = "0.7" futures-channel = "0.3" futures-util = { version = "0.3", default-features = false, features = ["std"] } serde = "1.0" diff --git a/README.md b/README.md index 2152f25..c14c7a8 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,12 @@ async fn main() -> Result<()> { let server = Server::new("com.my.Application", MyPlayer).await?; // Emit `PropertiesChanged` signal for `CanSeek` and `Metadata` properties - server.properties_changed(Property::CanSeek | Property::Metadata).await?; + server + .properties_changed([ + Property::CanSeek(false), + Property::Metadata(Metadata::new()), + ]) + .await?; // Emit `Seeked` signal server diff --git a/examples/local_server.rs b/examples/local_server.rs index 35606df..0f6ca20 100644 --- a/examples/local_server.rs +++ b/examples/local_server.rs @@ -304,7 +304,10 @@ async fn main() -> Result<()> { // Emit `PropertiesChanged` signal for `CanSeek` and `Metadata` properties server - .properties_changed(Property::CanSeek | Property::Metadata) + .properties_changed([ + Property::CanSeek(false), + Property::Metadata(Metadata::new()), + ]) .await?; // Emit `Seeked` signal diff --git a/examples/server.rs b/examples/server.rs index 019bcc9..0f24ac5 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -310,7 +310,10 @@ async fn main() -> Result<()> { // Emit `PropertiesChanged` signal for `CanSeek` and `Metadata` properties server - .properties_changed(Property::CanSeek | Property::Metadata) + .properties_changed([ + Property::CanSeek(false), + Property::Metadata(Metadata::new()), + ]) .await?; // Emit `Seeked` signal diff --git a/src/lib.rs b/src/lib.rs index 5b8ba3c..c9db9e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,7 +69,6 @@ pub mod builder { /// } /// ``` pub use async_trait::async_trait; -pub use enumflags2; pub use zbus; use zbus::{fdo, zvariant::OwnedObjectPath, Result}; diff --git a/src/local_server.rs b/src/local_server.rs index 16386d1..85c9145 100644 --- a/src/local_server.rs +++ b/src/local_server.rs @@ -9,7 +9,6 @@ use std::{ }; use async_trait::async_trait; -use enumflags2::BitFlags; use futures_channel::{mpsc, oneshot}; use futures_util::{FutureExt, StreamExt}; use zbus::{fdo, Connection, Result}; @@ -74,6 +73,18 @@ enum PlayerAction { CanControl(oneshot::Sender>), } +enum TrackListAction { + // Methods + GetTracksMetadata(Vec, oneshot::Sender>>), + AddTrack(Uri, TrackId, bool, oneshot::Sender>), + RemoveTrack(TrackId, oneshot::Sender>), + GoTo(TrackId, oneshot::Sender>), + + // Properties + Tracks(oneshot::Sender>>), + CanEditTracks(oneshot::Sender>), +} + enum PlaylistsAction { // Methods ActivatePlaylist(PlaylistId, oneshot::Sender>), @@ -91,18 +102,6 @@ enum PlaylistsAction { ActivePlaylist(oneshot::Sender>), } -enum TrackListAction { - // Methods - GetTracksMetadata(Vec, oneshot::Sender>>), - AddTrack(Uri, TrackId, bool, oneshot::Sender>), - RemoveTrack(TrackId, oneshot::Sender>), - GoTo(TrackId, oneshot::Sender>), - - // Properties - Tracks(oneshot::Sender>>), - CanEditTracks(oneshot::Sender>), -} - enum Action { Root(RootAction), Player(PlayerAction), @@ -628,7 +627,7 @@ where #[inline] pub async fn properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { self.inner.properties_changed(properties).await } @@ -888,7 +887,7 @@ where #[inline] pub async fn track_list_properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { self.inner.track_list_properties_changed(properties).await } @@ -970,7 +969,7 @@ where #[inline] pub async fn playlists_properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { self.inner.playlists_properties_changed(properties).await } diff --git a/src/player.rs b/src/player.rs index 91ce1d1..278d1be 100644 --- a/src/player.rs +++ b/src/player.rs @@ -384,7 +384,9 @@ impl Player { } self.server.imp().can_quit.set(can_quit); - self.server.properties_changed(Property::CanQuit).await + self.server + .properties_changed([Property::CanQuit(can_quit)]) + .await } pub fn fullscreen(&self) -> bool { @@ -397,7 +399,9 @@ impl Player { } self.server.imp().fullscreen.set(fullscreen); - self.server.properties_changed(Property::Fullscreen).await + self.server + .properties_changed([Property::Fullscreen(fullscreen)]) + .await } pub fn can_set_fullscreen(&self) -> bool { @@ -411,7 +415,7 @@ impl Player { self.server.imp().can_set_fullscreen.set(can_set_fullscreen); self.server - .properties_changed(Property::CanSetFullscreen) + .properties_changed([Property::CanSetFullscreen(can_set_fullscreen)]) .await } @@ -425,7 +429,9 @@ impl Player { } self.server.imp().can_raise.set(can_raise); - self.server.properties_changed(Property::CanRaise).await + self.server + .properties_changed([Property::CanRaise(can_raise)]) + .await } pub fn has_track_list(&self) -> bool { @@ -438,7 +444,9 @@ impl Player { } self.server.imp().has_track_list.set(has_track_list); - self.server.properties_changed(Property::HasTrackList).await + self.server + .properties_changed([Property::HasTrackList(has_track_list)]) + .await } pub fn identity(&self) -> Ref<'_, String> { @@ -452,8 +460,10 @@ impl Player { return Ok(()); } - self.server.imp().identity.replace(identity); - self.server.properties_changed(Property::Identity).await + self.server.imp().identity.replace(identity.clone()); + self.server + .properties_changed([Property::Identity(identity)]) + .await } pub fn desktop_entry(&self) -> Ref<'_, String> { @@ -467,8 +477,13 @@ impl Player { return Ok(()); } - self.server.imp().desktop_entry.replace(desktop_entry); - self.server.properties_changed(Property::DesktopEntry).await + self.server + .imp() + .desktop_entry + .replace(desktop_entry.clone()); + self.server + .properties_changed([Property::DesktopEntry(desktop_entry)]) + .await } pub fn supported_uri_schemes(&self) -> Ref<'_, Vec> { @@ -482,7 +497,7 @@ impl Player { let supported_uri_schemes = supported_uri_schemes .into_iter() .map(|i| i.into()) - .collect(); + .collect::>(); if *self.supported_uri_schemes() == supported_uri_schemes { return Ok(()); @@ -491,9 +506,9 @@ impl Player { self.server .imp() .supported_uri_schemes - .replace(supported_uri_schemes); + .replace(supported_uri_schemes.clone()); self.server - .properties_changed(Property::SupportedUriSchemes) + .properties_changed([Property::SupportedUriSchemes(supported_uri_schemes)]) .await } @@ -505,7 +520,10 @@ impl Player { &self, supported_mime_types: impl IntoIterator>, ) -> Result<()> { - let supported_mime_types = supported_mime_types.into_iter().map(|i| i.into()).collect(); + let supported_mime_types = supported_mime_types + .into_iter() + .map(|i| i.into()) + .collect::>(); if *self.supported_mime_types() == supported_mime_types { return Ok(()); @@ -514,9 +532,9 @@ impl Player { self.server .imp() .supported_mime_types - .replace(supported_mime_types); + .replace(supported_mime_types.clone()); self.server - .properties_changed(Property::SupportedMimeTypes) + .properties_changed([Property::SupportedMimeTypes(supported_mime_types)]) .await } @@ -619,7 +637,7 @@ impl Player { self.server.imp().playback_status.set(playback_status); self.server - .properties_changed(Property::PlaybackStatus) + .properties_changed([Property::PlaybackStatus(playback_status)]) .await } @@ -633,7 +651,9 @@ impl Player { } self.server.imp().loop_status.set(loop_status); - self.server.properties_changed(Property::LoopStatus).await + self.server + .properties_changed([Property::LoopStatus(loop_status)]) + .await } pub fn rate(&self) -> PlaybackRate { @@ -646,7 +666,7 @@ impl Player { } self.server.imp().rate.set(rate); - self.server.properties_changed(Property::Rate).await + self.server.properties_changed([Property::Rate(rate)]).await } pub fn shuffle(&self) -> bool { @@ -659,7 +679,9 @@ impl Player { } self.server.imp().shuffle.set(shuffle); - self.server.properties_changed(Property::Shuffle).await + self.server + .properties_changed([Property::Shuffle(shuffle)]) + .await } pub fn metadata(&self) -> Ref<'_, Metadata> { @@ -671,8 +693,10 @@ impl Player { return Ok(()); } - self.server.imp().metadata.replace(metadata); - self.server.properties_changed(Property::Metadata).await + self.server.imp().metadata.replace(metadata.clone()); + self.server + .properties_changed([Property::Metadata(metadata)]) + .await } pub fn volume(&self) -> Volume { @@ -685,7 +709,9 @@ impl Player { } self.server.imp().volume.set(volume); - self.server.properties_changed(Property::Volume).await + self.server + .properties_changed([Property::Volume(volume)]) + .await } pub fn position(&self) -> Time { @@ -707,7 +733,9 @@ impl Player { } self.server.imp().minimum_rate.set(minimum_rate); - self.server.properties_changed(Property::MinimumRate).await + self.server + .properties_changed([Property::MinimumRate(minimum_rate)]) + .await } pub fn maximum_rate(&self) -> PlaybackRate { @@ -720,7 +748,9 @@ impl Player { } self.server.imp().maximum_rate.set(maximum_rate); - self.server.properties_changed(Property::MaximumRate).await + self.server + .properties_changed([Property::MaximumRate(maximum_rate)]) + .await } pub fn can_go_next(&self) -> bool { @@ -733,7 +763,9 @@ impl Player { } self.server.imp().can_go_next.set(can_go_next); - self.server.properties_changed(Property::CanGoNext).await + self.server + .properties_changed([Property::CanGoNext(can_go_next)]) + .await } pub fn can_go_previous(&self) -> bool { @@ -747,7 +779,7 @@ impl Player { self.server.imp().can_go_previous.set(can_go_previous); self.server - .properties_changed(Property::CanGoPrevious) + .properties_changed([Property::CanGoPrevious(can_go_previous)]) .await } @@ -761,7 +793,9 @@ impl Player { } self.server.imp().can_play.set(can_play); - self.server.properties_changed(Property::CanPlay).await + self.server + .properties_changed([Property::CanPlay(can_play)]) + .await } pub fn can_pause(&self) -> bool { @@ -774,7 +808,9 @@ impl Player { } self.server.imp().can_pause.set(can_pause); - self.server.properties_changed(Property::CanPause).await + self.server + .properties_changed([Property::CanPause(can_pause)]) + .await } pub fn can_seek(&self) -> bool { @@ -787,7 +823,9 @@ impl Player { } self.server.imp().can_seek.set(can_seek); - self.server.properties_changed(Property::CanSeek).await + self.server + .properties_changed([Property::CanSeek(can_seek)]) + .await } /// This can only be set on construct. diff --git a/src/property.rs b/src/property.rs index faa8aec..e71480b 100644 --- a/src/property.rs +++ b/src/property.rs @@ -1,37 +1,37 @@ -use enumflags2::bitflags; +use crate::{ + LoopStatus, MaybePlaylist, Metadata, PlaybackRate, PlaybackStatus, PlaylistOrdering, Volume, +}; /// Used for emitting `PropertiesChanged` signals on /// [`Server::properties_changed`] and [`LocalServer::properties_changed`]. /// /// [`Server::properties_changed`]: crate::Server::properties_changed /// [`LocalServer::properties_changed`]: crate::LocalServer::properties_changed -#[bitflags] -#[repr(u32)] -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq)] pub enum Property { - CanQuit, - Fullscreen, - CanSetFullscreen, - CanRaise, - HasTrackList, - Identity, - DesktopEntry, - SupportedUriSchemes, - SupportedMimeTypes, - PlaybackStatus, - LoopStatus, - Rate, - Shuffle, - Metadata, - Volume, + CanQuit(bool), + Fullscreen(bool), + CanSetFullscreen(bool), + CanRaise(bool), + HasTrackList(bool), + Identity(String), + DesktopEntry(String), + SupportedUriSchemes(Vec), + SupportedMimeTypes(Vec), + PlaybackStatus(PlaybackStatus), + LoopStatus(LoopStatus), + Rate(PlaybackRate), + Shuffle(bool), + Metadata(Metadata), + Volume(Volume), // Position (must use `Rate` property together with `Seeked` signal instead) - MinimumRate, - MaximumRate, - CanGoNext, - CanGoPrevious, - CanPlay, - CanPause, - CanSeek, + MinimumRate(PlaybackRate), + MaximumRate(PlaybackRate), + CanGoNext(bool), + CanGoPrevious(bool), + CanPlay(bool), + CanPause(bool), + CanSeek(bool), // CanControl (not expected to change) } @@ -44,12 +44,11 @@ pub enum Property { /// [`LocalServer::track_list_properties_changed`]: crate::LocalServer::track_list_properties_changed /// [`TrackListInterface`]: crate::TrackListInterface /// [`LocalTrackListInterface`]: crate::LocalTrackListInterface -#[bitflags] -#[repr(u32)] -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum TrackListProperty { + // The new value must not be sent according to the spec. Tracks, - CanEditTracks, + CanEditTracks(bool), } /// Used for emitting `PropertiesChanged` signals on @@ -61,11 +60,9 @@ pub enum TrackListProperty { /// [`LocalServer::playlists_properties_changed`]: crate::LocalServer::playlists_properties_changed /// [`PlaylistsInterface`]: crate::PlaylistsInterface /// [`LocalPlaylistsInterface`]: crate::LocalPlaylistsInterface -#[bitflags] -#[repr(u32)] -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum PlaylistsProperty { - PlaylistCount, - Orderings, - ActivePlaylist, + PlaylistCount(u32), + Orderings(Vec), + ActivePlaylist(MaybePlaylist), } diff --git a/src/server.rs b/src/server.rs index 0ef9465..02ce7c5 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,6 +1,5 @@ use std::{collections::HashMap, fmt, sync::Arc}; -use enumflags2::BitFlags; use serde::Serialize; use zbus::{ dbus_interface, fdo, @@ -368,12 +367,11 @@ where } macro_rules! insert_property { - ($item:ident, $property_type:ident, $source:ident => $($map:ident, $property:ident, $getter:ident);*) => { + ($item:ident, $property_type:ident => $($map:ident, $property:ident);*) => { match $item { $( - $property_type::$property => { - let value = Value::new($source.imp.$getter().await?); - $map.insert(stringify!($property), value); + $property_type::$property(val) => { + $map.insert(stringify!($property), Value::new(val)); } )* } @@ -441,36 +439,36 @@ where /// interfaces respectively. pub async fn properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { let mut root_changed = HashMap::new(); let mut player_changed = HashMap::new(); - for property in properties.into().iter() { + for property in properties.into_iter() { insert_property!( - property, Property, self => - root_changed, CanQuit, can_quit; - root_changed, Fullscreen, fullscreen; - root_changed, CanSetFullscreen, can_set_fullscreen; - root_changed, CanRaise, can_raise; - root_changed, HasTrackList, has_track_list; - root_changed, Identity, identity; - root_changed, DesktopEntry, desktop_entry; - root_changed, SupportedUriSchemes, supported_uri_schemes; - root_changed, SupportedMimeTypes, supported_mime_types; - player_changed, PlaybackStatus, playback_status; - player_changed, LoopStatus, loop_status; - player_changed, Rate, rate; - player_changed, Shuffle, shuffle; - player_changed, Metadata, metadata; - player_changed, Volume, volume; - player_changed, MinimumRate, minimum_rate; - player_changed, MaximumRate, maximum_rate; - player_changed, CanGoNext, can_go_next; - player_changed, CanGoPrevious, can_go_previous; - player_changed, CanPlay, can_play; - player_changed, CanPause, can_pause; - player_changed, CanSeek, can_seek + property, Property => + root_changed, CanQuit; + root_changed, Fullscreen; + root_changed, CanSetFullscreen; + root_changed, CanRaise; + root_changed, HasTrackList; + root_changed, Identity; + root_changed, DesktopEntry; + root_changed, SupportedUriSchemes; + root_changed, SupportedMimeTypes; + player_changed, PlaybackStatus; + player_changed, LoopStatus; + player_changed, Rate; + player_changed, Shuffle; + player_changed, Metadata; + player_changed, Volume; + player_changed, MinimumRate; + player_changed, MaximumRate; + player_changed, CanGoNext; + player_changed, CanGoPrevious; + player_changed, CanPlay; + player_changed, CanPause; + player_changed, CanSeek ); } @@ -621,19 +619,18 @@ where /// properties as defined by the spec. pub async fn track_list_properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { let mut changed = HashMap::new(); let mut invalidated = Vec::new(); - for property in properties.into().iter() { + for property in properties.into_iter() { match property { TrackListProperty::Tracks => { - // The new value must not be sent according to the spec. invalidated.push("Tracks"); } - TrackListProperty::CanEditTracks => { - let value = Value::new(self.imp.can_edit_tracks().await?); + TrackListProperty::CanEditTracks(can_edit_tracks) => { + let value = Value::new(can_edit_tracks); changed.insert("CanEditTracks", value); } } @@ -682,16 +679,16 @@ where /// properties as defined by the spec. pub async fn playlists_properties_changed( &self, - properties: impl Into>, + properties: impl IntoIterator, ) -> Result<()> { let mut changed = HashMap::new(); - for property in properties.into().iter() { + for property in properties.into_iter() { insert_property!( - property, PlaylistsProperty, self => - changed, PlaylistCount, playlist_count; - changed, Orderings, orderings; - changed, ActivePlaylist, active_playlist + property, PlaylistsProperty => + changed, PlaylistCount; + changed, Orderings; + changed, ActivePlaylist ); }