feat(mpris): Restructuring and cleanup

* MPRIS: Restructured existing implementation

My pr addresses some of the inconsistencies in ncspot's mpris
implementation. While the previous logic was technically good enough, it
was inflexible and reported redundant information.
This will make it easier for software, that processes mpris events in
some way, to accurately react to player updates.

- 'Metadata' and 'Playback' updates have been separated into there own
  command
- Mpris commands are only emitted from within spotify.rs
- Some parts of the application creation logic has been
  restructured to allow for mpris events to be emitted upon startup
    - The initial song loading code has been moved from 'Queue::new'
      into 'Application::new'.

* MPRIS: implemented most clippy suggestions

* Fix: applied clippy suggestions + format

* MPRIS: Renamed MprisCommands to be more resonable

I've added a clippy exception so it does not complain to us about enum
variants starting with the same prefix.

---------

Co-authored-by: haruInDisguise <--help>

src/application.rs

@@ -12,7 +12,7 @@ use signal_hook::{consts::SIGHUP, consts::SIGTERM, iterator::Signals};
 
 use crate::command::Command;
 use crate::commands::CommandManager;
-use crate::config::Config;
+use crate::config::{Config, PlaybackState};
 use crate::events::{Event, EventManager};
 use crate::library::Library;
 use crate::queue::Queue;
@@ -22,7 +22,7 @@ use crate::{authentication, ui, utils};
 use crate::{command, queue, spotify};
 
 #[cfg(feature = "mpris")]
-use crate::mpris::{self, MprisCommand, MprisManager};
+use crate::mpris::MprisManager;
 
 #[cfg(unix)]
 use crate::ipc::{self, IpcSocket};
@@ -68,9 +68,6 @@ pub struct Application {
     /// Internally shared
     event_manager: EventManager,
     /// An IPC implementation using the D-Bus MPRIS protocol, used to control and inspect ncspot.
-    #[cfg(feature = "mpris")]
-    mpris_manager: MprisManager,
-    /// An IPC implementation using a Unix domain socket, used to control and inspect ncspot.
     #[cfg(unix)]
     ipc: Option<IpcSocket>,
     /// The object to render to the terminal.
@@ -130,7 +127,7 @@ impl Application {
         ));
 
         #[cfg(feature = "mpris")]
-        let mpris_manager = mpris::MprisManager::new(
+        let mpris_manager = MprisManager::new(
             event_manager.clone(),
             queue.clone(),
             library.clone(),
@@ -140,6 +137,27 @@ impl Application {
         #[cfg(feature = "mpris")]
         spotify.set_mpris(mpris_manager.clone());
 
+        // Load the last played track into the player
+        let playback_state = configuration.state().playback_state.clone();
+        let queue_state = configuration.state().queuestate.clone();
+
+        if let Some(playable) = queue.get_current() {
+            spotify.load(
+                &playable,
+                playback_state == PlaybackState::Playing,
+                queue_state.track_progress.as_millis() as u32,
+            );
+            spotify.update_track();
+            match playback_state {
+                PlaybackState::Stopped => {
+                    spotify.stop();
+                }
+                PlaybackState::Paused | PlaybackState::Playing | PlaybackState::Default => {
+                    spotify.pause();
+                }
+            }
+        }
+
         #[cfg(unix)]
         let ipc = if let Ok(runtime_directory) = utils::create_runtime_directory() {
             Some(
@@ -208,8 +226,6 @@ impl Application {
             queue,
             spotify,
             event_manager,
-            #[cfg(feature = "mpris")]
-            mpris_manager,
             #[cfg(unix)]
             ipc,
             cursive,
@@ -240,9 +256,6 @@ impl Application {
                         trace!("event received: {:?}", state);
                         self.spotify.update_status(state.clone());
 
-                        #[cfg(feature = "mpris")]
-                        self.mpris_manager.send(MprisCommand::NotifyPlaybackUpdate);
-
                         #[cfg(unix)]
                         if let Some(ref ipc) = self.ipc {
                             ipc.publish(&state, self.queue.get_current());

src/mpris.rs

@@ -463,11 +463,15 @@ impl MprisPlayer {
 }
 
 /// Commands to control the [MprisManager] worker thread.
+#[derive(Debug)]
+#[allow(clippy::enum_variant_names)]
 pub enum MprisCommand {
-    /// Notify about playback status and metadata updates.
-    NotifyPlaybackUpdate,
-    /// Notify about volume updates.
-    NotifyVolumeUpdate,
+    /// Emit playback status
+    EmitPlaybackStatus,
+    /// Emit volume
+    EmitVolumeStatus,
+    /// Emit metadata
+    EmitMetadataStatus,
 }
 
 /// An MPRIS server that internally manager a thread which can be sent commands. This is internally
@@ -525,14 +529,16 @@ impl MprisManager {
         loop {
             let ctx = player_iface_ref.signal_context();
             match rx.next().await {
-                Some(MprisCommand::NotifyPlaybackUpdate) => {
+                Some(MprisCommand::EmitPlaybackStatus) => {
                     player_iface.playback_status_changed(ctx).await?;
-                    player_iface.metadata_changed(ctx).await?;
                 }
-                Some(MprisCommand::NotifyVolumeUpdate) => {
+                Some(MprisCommand::EmitVolumeStatus) => {
                     info!("sending MPRIS volume update signal");
                     player_iface.volume_changed(ctx).await?;
                 }
+                Some(MprisCommand::EmitMetadataStatus) => {
+                    player_iface.metadata_changed(ctx).await?;
+                }
                 None => break,
             }
         }

src/queue.rs

@@ -8,7 +8,7 @@ use notify_rust::Notification;
 use rand::prelude::*;
 use strum_macros::Display;
 
-use crate::config::{Config, PlaybackState};
+use crate::config::Config;
 use crate::library::Library;
 use crate::model::playable::Playable;
 use crate::spotify::PlayerEvent;
@@ -50,34 +50,15 @@ pub struct Queue {
 impl Queue {
     pub fn new(spotify: Spotify, cfg: Arc<Config>, library: Arc<Library>) -> Self {
         let queue_state = cfg.state().queuestate.clone();
-        let playback_state = cfg.state().playback_state.clone();
-        let queue = Self {
+
+        Self {
             queue: Arc::new(RwLock::new(queue_state.queue)),
             spotify: spotify.clone(),
             current_track: RwLock::new(queue_state.current_track),
             random_order: RwLock::new(queue_state.random_order),
             cfg,
             library,
-        };
-
-        if let Some(playable) = queue.get_current() {
-            spotify.load(
-                &playable,
-                playback_state == PlaybackState::Playing,
-                queue_state.track_progress.as_millis() as u32,
-            );
-            spotify.update_track();
-            match playback_state {
-                PlaybackState::Stopped => {
-                    spotify.stop();
-                }
-                PlaybackState::Paused | PlaybackState::Playing | PlaybackState::Default => {
-                    spotify.pause();
-                }
-            }
         }
-
-        queue
     }
 
     /// The index of the next item in `self.queue` that should be played. None

src/spotify.rs

@@ -16,7 +16,7 @@ use librespot_playback::config::PlayerConfig;
 use librespot_playback::mixer::softmixer::SoftMixer;
 use librespot_playback::mixer::MixerConfig;
 use librespot_playback::player::Player;
-use log::{debug, error, info};
+use log::{debug, error, info, warn};
 use tokio::sync::mpsc;
 use url::Url;
 
@@ -319,6 +319,9 @@ impl Spotify {
             start_playing,
             position_ms,
         ));
+
+        #[cfg(feature = "mpris")]
+        self.send_mpris(MprisCommand::EmitMetadataStatus);
     }
 
     /// Update the cached status of the [Player]. This makes sure the status
@@ -342,6 +345,9 @@ impl Spotify {
 
         let mut status = self.status.write().unwrap();
         *status = new_status;
+
+        #[cfg(feature = "mpris")]
+        self.send_mpris(MprisCommand::EmitPlaybackStatus);
     }
 
     /// Reset the time tracking stats for the current song. This should be called when a new song is
@@ -366,6 +372,17 @@ impl Spotify {
         }
     }
 
+    /// Send an [MprisCommand] to the mpris thread.
+    #[cfg(feature = "mpris")]
+    fn send_mpris(&self, cmd: MprisCommand) {
+        debug!("Sending mpris command: {:?}", cmd);
+        if let Some(mpris_manager) = self.mpris.lock().unwrap().as_ref() {
+            mpris_manager.send(cmd);
+        } else {
+            warn!("mpris context is unitialized");
+        }
+    }
+
     /// Send a [WorkerCommand] to the worker thread.
     fn send_worker(&self, cmd: WorkerCommand) {
         info!("sending command to worker: {:?}", cmd);
@@ -422,10 +439,7 @@ impl Spotify {
         // MPRIS implementation.
         if notify {
             #[cfg(feature = "mpris")]
-            if let Some(mpris_manager) = self.mpris.lock().unwrap().as_ref() {
-                info!("updating MPRIS volume");
-                mpris_manager.send(MprisCommand::NotifyVolumeUpdate);
-            }
+            self.send_mpris(MprisCommand::EmitVolumeStatus)
         }
     }