connect: fixes for context and transfer

- fix context_metadata and restriction incorrect reset
- do some state updates earlier
- add more logging

connect/src/context_resolver.rs

@@ -180,6 +180,11 @@ impl ContextResolver {
         } else if self.queue.contains(&resolve) {
             debug!("update for {resolve} is already added");
             return;
+        } else {
+            trace!(
+                "added {} to resolver queue",
+                resolve.resolve_uri().unwrap_or(resolve.context_uri())
+            )
         }
 
         self.queue.push_back(resolve)
@@ -265,7 +270,7 @@ impl ContextResolver {
         }
     }
 
-    pub fn handle_next_context(
+    pub fn apply_next_context(
         &self,
         state: &mut ConnectState,
         mut context: Context,
@@ -319,15 +324,17 @@ impl ContextResolver {
             return false;
         }
 
-        debug!("last item of type <{:?}>, finishing state", next.update);
-
         match (next.update, state.active_context) {
-            (UpdateContext::Default, ContextType::Default) => {}
+            (UpdateContext::Default, ContextType::Default) | (UpdateContext::Autoplay, _) => {
+                debug!(
+                    "last item of type <{:?}>, finishing state setup",
+                    next.update
+                );
+            }
             (UpdateContext::Default, _) => {
                 debug!("skipped finishing default, because it isn't the active context");
                 return false;
             }
-            (UpdateContext::Autoplay, _) => {}
         }
 
         if let Some(transfer_state) = transfer_state.take() {
@@ -336,25 +343,6 @@ impl ContextResolver {
             }
         }
 
-        let res = if state.shuffling_context() {
-            state.shuffle()
-        } else if let Ok(ctx) = state.get_context(state.active_context) {
-            let idx = ConnectState::find_index_in_context(ctx, |t| {
-                state.current_track(|c| t.uri == c.uri)
-            })
-            .ok();
-
-            state
-                .reset_playback_to_position(idx)
-                .and_then(|_| state.fill_up_next_tracks())
-        } else {
-            state.fill_up_next_tracks()
-        };
-
-        if let Err(why) = res {
-            error!("setting up state failed after updating contexts: {why}")
-        }
-
         state.update_restrictions();
         state.update_queue_revision();
 

connect/src/spirc.rs

@@ -139,7 +139,7 @@ const VOLUME_STEP_SIZE: u16 = 1024; // (u16::MAX + 1) / VOLUME_STEPS
 // delay to update volume after a certain amount of time, instead on each update request
 const VOLUME_UPDATE_DELAY: Duration = Duration::from_secs(2);
 // to reduce updates to remote, we group some request by waiting for a set amount of time
-const UPDATE_STATE_DELAY: Duration = Duration::from_millis(300);
+const UPDATE_STATE_DELAY: Duration = Duration::from_millis(200);
 
 pub struct Spirc {
     commands: mpsc::UnboundedSender<SpircCommand>,
@@ -452,7 +452,12 @@ impl SpircTask {
                         self.connect_state.prev_autoplay_track_uris()
                     }).await
                 }, if allow_context_resolving && self.context_resolver.has_next() => {
-                    self.handle_next_context(next_context)
+                    let update_state = self.handle_next_context(next_context);
+                    if update_state {
+                        if let Err(why) = self.notify().await {
+                            error!("update after context resolving failed: {why}")
+                        }
+                    }
                 },
                 else => break
             }
@@ -471,20 +476,22 @@ impl SpircTask {
         self.session.dealer().close().await;
     }
 
-    fn handle_next_context(&mut self, next_context: Result<Context, Error>) {
+    fn handle_next_context(&mut self, next_context: Result<Context, Error>) -> bool {
         let next_context = match next_context {
             Err(why) => {
                 self.context_resolver.mark_next_unavailable();
                 self.context_resolver.remove_used_and_invalid();
                 error!("{why}");
-                return;
+                return false;
             }
             Ok(ctx) => ctx,
         };
 
+        debug!("handling next context {}", next_context.uri);
+
         match self
             .context_resolver
-            .handle_next_context(&mut self.connect_state, next_context)
+            .apply_next_context(&mut self.connect_state, next_context)
         {
             Ok(remaining) => {
                 if let Some(remaining) = remaining {
@@ -496,15 +503,18 @@ impl SpircTask {
             }
         }
 
-        if self
+        let update_state = if self
             .context_resolver
             .try_finish(&mut self.connect_state, &mut self.transfer_state)
         {
             self.add_autoplay_resolving_when_required();
-            self.update_state = true;
+            true
-        }
+        } else {
+            false
+        };
 
         self.context_resolver.remove_used_and_invalid();
+        update_state
     }
 
     // todo: is the time_delta still necessary?
@@ -561,6 +571,7 @@ impl SpircTask {
     fn handle_player_event(&mut self, event: PlayerEvent) -> Result<(), Error> {
         if let PlayerEvent::TrackChanged { audio_item } = event {
             self.connect_state.update_duration(audio_item.duration_ms);
+            self.update_state = true;
             return Ok(());
         }
 
@@ -574,6 +585,7 @@ impl SpircTask {
             (event.get_play_request_id(), self.play_request_id),
             (Some(event_id), Some(current_id)) if event_id == current_id
         };
+
         // we only process events if the play_request_id matches. If it doesn't, it is
         // an event that belongs to a previous track and only arrives now due to a race
         // condition. In this case we have updated the state already and don't want to
@@ -878,7 +890,8 @@ impl SpircTask {
             }
             // modification and update of the connect_state
             Transfer(transfer) => {
-                self.handle_transfer(transfer.data.expect("by condition checked"))?
+                self.handle_transfer(transfer.data.expect("by condition checked"))?;
+                return self.notify().await;
             }
             Play(play) => {
                 let shuffle = play

connect/src/state/context.rs

@@ -104,9 +104,6 @@ impl ConnectState {
     }
 
     pub fn reset_context(&mut self, mut reset_as: ResetContext) {
-        self.set_active_context(ContextType::Default);
-        self.fill_up_context = ContextType::Default;
-
         if matches!(reset_as, ResetContext::WhenDifferent(ctx) if self.different_context_uri(ctx)) {
             reset_as = ResetContext::Completely
         }
@@ -129,6 +126,8 @@ impl ConnectState {
             }
         }
 
+        self.fill_up_context = ContextType::Default;
+        self.set_active_context(ContextType::Default);
         self.update_restrictions()
     }
 
@@ -149,6 +148,10 @@ impl ConnectState {
     pub fn set_active_context(&mut self, new_context: ContextType) {
         self.active_context = new_context;
 
+        let player = self.player_mut();
+        player.context_metadata.clear();
+        player.restrictions.clear();
+
         let ctx = match self.get_context(new_context) {
             Err(why) => {
                 warn!("couldn't load context info because: {why}");
@@ -162,9 +165,6 @@ impl ConnectState {
 
         let player = self.player_mut();
 
-        player.context_metadata.clear();
-        player.restrictions.clear();
-
         if let Some(restrictions) = restrictions.take() {
             player.restrictions = MessageField::some(restrictions);
         }

connect/src/state/transfer.rs

@@ -68,6 +68,7 @@ impl ConnectState {
 
         self.clear_prev_track();
         self.clear_next_tracks(false);
+        self.update_queue_revision()
     }
 
     /// completes the transfer, loading the queue and updating metadata