diff --git a/src/core/app.rs b/src/core/app.rs index aefffab..308f840 100644 --- a/src/core/app.rs +++ b/src/core/app.rs @@ -57,6 +57,9 @@ const DEFAULT_ROUTE: Route = Route { /// This prevents the UI from jumping back to old positions while the seek completes pub const SEEK_POSITION_IGNORE_MS: u128 = 500; +#[cfg(feature = "streaming")] +const FRESH_NATIVE_ACTIVITY_WINDOW: Duration = Duration::from_secs(5); + #[derive(Clone)] pub struct ScrollableResultPages { pub index: usize, @@ -2089,6 +2092,15 @@ impl App { self.api_error = e.to_string(); } + #[cfg(feature = "streaming")] + pub fn has_fresh_native_activity(&self) -> bool { + self.native_track_info.is_some() + || self.native_is_playing == Some(true) + || self + .last_device_activation + .is_some_and(|instant| instant.elapsed() < FRESH_NATIVE_ACTIVITY_WINDOW) + } + /// Check if native streaming is the active playback device /// Returns true only if the player is connected AND it's the currently active device #[cfg(feature = "streaming")] @@ -2124,10 +2136,13 @@ impl App { } } - // Fallback: strict name match (case-insensitive) + // Fallback: strict name match (case-insensitive), but only while we have + // fresh native activity or a recent explicit activation. After a recovery, + // Spotify can keep returning the old "spotatui" device while the new native + // player is connected but stopped/not active. if let Some(native_name) = native_device_name.as_ref() { let current_device_name = ctx.device.name.to_lowercase(); - if current_device_name == native_name.as_str() { + if current_device_name == native_name.as_str() && self.has_fresh_native_activity() { return true; } } @@ -4317,6 +4332,46 @@ mod tests { } } + #[cfg(feature = "streaming")] + #[test] + fn fresh_native_activity_is_true_when_native_metadata_exists() { + let mut app = App { + native_track_info: Some(NativeTrackInfo::default()), + ..Default::default() + }; + + assert!(app.has_fresh_native_activity()); + + app.native_track_info = None; + assert!(!app.has_fresh_native_activity()); + } + + #[cfg(feature = "streaming")] + #[test] + fn fresh_native_activity_is_true_when_native_is_playing() { + let app = App { + native_is_playing: Some(true), + ..Default::default() + }; + + assert!(app.has_fresh_native_activity()); + } + + #[cfg(feature = "streaming")] + #[test] + fn fresh_native_activity_uses_recent_activation_window() { + let mut app = App { + last_device_activation: Some(Instant::now()), + ..Default::default() + }; + + assert!(app.has_fresh_native_activity()); + + app.last_device_activation = Some(Instant::now() - Duration::from_secs(6)); + + assert!(!app.has_fresh_native_activity()); + } + fn saved_track(id: &str, name: &str) -> SavedTrack { SavedTrack { added_at: Utc::now(), diff --git a/src/infra/network/playback.rs b/src/infra/network/playback.rs index 48d8749..18f9fee 100644 --- a/src/infra/network/playback.rs +++ b/src/infra/network/playback.rs @@ -138,6 +138,47 @@ enum NativePlaybackRoute { NativeLoad, } +#[cfg(feature = "streaming")] +#[derive(Clone, Copy, Debug, Default)] +struct NativeActivationContext { + player_connected: bool, + current_device_id_present: bool, + current_device_is_confirmed_native: bool, + current_device_name_matches_native: bool, + native_has_fresh_activity: bool, + saved_device_matches_native: bool, + saved_external_confirmed_available: bool, +} + +#[cfg(feature = "streaming")] +fn should_activate_native_for_playback(context: NativeActivationContext) -> bool { + if !context.player_connected { + return false; + } + + let current_device_is_stale_native_name = + context.current_device_name_matches_native && !context.native_has_fresh_activity; + let current_device_is_usable_external = context.current_device_id_present + && !context.current_device_is_confirmed_native + && !current_device_is_stale_native_name; + + if current_device_is_usable_external { + return false; + } + + if context.saved_device_matches_native { + return true; + } + + !context.saved_external_confirmed_available +} + +#[cfg(feature = "streaming")] +fn is_no_active_device_error(e: &anyhow::Error) -> bool { + let text = e.to_string().to_ascii_lowercase(); + text.contains("no_active_device") || text.contains("no active device") +} + fn api_confirms_native_info_is_current( native_name: &str, item: &PlayableItem, @@ -217,10 +258,13 @@ async fn is_native_streaming_active_for_playback(network: &Network) -> bool { } } - // Fallback: strict name match (case-insensitive) + // Fallback: strict name match (case-insensitive), but only while we have + // fresh native activity or a recent explicit activation. After a recovery, + // Spotify can keep returning the old "spotatui" device while the new native + // player is connected but stopped/not active. if let Some(native_name) = native_device_name.as_ref() { let current_device_name = ctx.device.name.to_lowercase(); - if current_device_name == native_name.as_str() { + if current_device_name == native_name.as_str() && app.has_fresh_native_activity() { return true; } } @@ -243,29 +287,50 @@ async fn is_native_streaming_active_for_playback(network: &Network) -> bool { #[cfg(feature = "streaming")] async fn should_activate_native_streaming_for_playback(network: &Network) -> bool { - let saved_device_id = network.client_config.device_id.as_ref(); + let saved_device_id = network.client_config.device_id.as_deref(); let app = network.app.lock().await; let Some(player) = app.streaming_player.as_ref() else { return false; }; - if !player.is_connected() || app.current_playback_context.is_some() { + if !player.is_connected() { return false; } - let Some(saved_device_id) = saved_device_id else { - return true; - }; - - if app.native_device_id.as_ref() == Some(saved_device_id) { - return true; - } - - app.devices.as_ref().is_some_and(|payload| { - payload.devices.iter().any(|device| { - device.id.as_ref() == Some(saved_device_id) - && device.name.eq_ignore_ascii_case(player.device_name()) + let native_name = player.device_name(); + let native_device_id = app.native_device_id.as_deref(); + let current_device = app.current_playback_context.as_ref().map(|ctx| &ctx.device); + let current_device_id = current_device.and_then(|device| device.id.as_deref()); + let current_device_name_matches_native = + current_device.is_some_and(|device| device.name.eq_ignore_ascii_case(native_name)); + let native_has_fresh_activity = app.has_fresh_native_activity(); + + let saved_device_matches_native = saved_device_id.is_some_and(|saved| { + native_device_id == Some(saved) + || app.devices.as_ref().is_some_and(|payload| { + payload.devices.iter().any(|device| { + device.id.as_deref() == Some(saved) && device.name.eq_ignore_ascii_case(native_name) + }) + }) + }); + + let saved_external_confirmed_available = saved_device_id.is_some_and(|saved| { + app.devices.as_ref().is_some_and(|payload| { + payload.devices.iter().any(|device| { + device.id.as_deref() == Some(saved) && !device.name.eq_ignore_ascii_case(native_name) + }) }) + }); + + should_activate_native_for_playback(NativeActivationContext { + player_connected: true, + current_device_id_present: current_device_id.is_some(), + current_device_is_confirmed_native: native_device_id + .is_some_and(|id| current_device_id == Some(id)), + current_device_name_matches_native, + native_has_fresh_activity, + saved_device_matches_native, + saved_external_confirmed_available, }) } @@ -322,6 +387,45 @@ async fn resolve_native_playback_route( } } +#[cfg(feature = "streaming")] +fn native_load_request( + context_id: Option>, + uris: Option>>, + offset: Option, +) -> Option { + let mut options = LoadRequestOptions { + start_playing: true, + seek_to: 0, + context_options: None, + playing_track: None, + }; + + match (context_id, uris) { + (Some(context), Some(track_uris)) => { + if let Some(first_uri) = track_uris.first() { + options.playing_track = Some(PlayingTrack::Uri(first_uri.uri())); + } else if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { + options.playing_track = Some(PlayingTrack::Index(i)); + } + Some(LoadRequest::from_context_uri(context.uri(), options)) + } + (Some(context), None) => { + if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { + options.playing_track = Some(PlayingTrack::Index(i)); + } + Some(LoadRequest::from_context_uri(context.uri(), options)) + } + (None, Some(track_uris)) => { + if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { + options.playing_track = Some(PlayingTrack::Index(i)); + } + let uris = track_uris.into_iter().map(|u| u.uri()).collect::>(); + Some(LoadRequest::from_tracks(uris, options)) + } + (None, None) => None, + } +} + impl PlaybackNetwork for Network { async fn get_current_playback(&mut self) { // When using native streaming, the Spotify API returns stale server-side state @@ -372,8 +476,9 @@ impl PlaybackNetwork for Network { { return current_id == native_id; } + let native_name = p.device_name().to_lowercase(); - c.device.name.to_lowercase() == native_name + c.device.name.to_lowercase() == native_name && app.has_fresh_native_activity() }); #[cfg(feature = "streaming")] @@ -698,12 +803,61 @@ impl PlaybackNetwork for Network { // For resume playback (no context, no uris) if context_id.is_none() && uris.is_none() { - info!("starting native resume playback via direct player route"); - player.play(); - // Update UI state immediately - let mut app = self.app.lock().await; - if let Some(ctx) = &mut app.current_playback_context { - ctx.is_playing = true; + let (can_resume_direct_native, native_device_id) = { + let app = self.app.lock().await; + ( + app.native_track_info.is_some() || app.last_track_id.is_some(), + app + .native_device_id + .clone() + .or_else(|| Some(player.device_id())), + ) + }; + + if can_resume_direct_native { + info!("starting native resume playback via direct player route"); + player.play(); + let mut app = self.app.lock().await; + if let Some(ctx) = &mut app.current_playback_context { + ctx.is_playing = true; + } + } else if let Some(device_id) = native_device_id { + info!( + "starting native resume playback via Spotify API on device {}", + device_id + ); + match self + .spotify_api_request_json( + Method::PUT, + "me/player/play", + &[("device_id", device_id.clone())], + None, + ) + .await + { + Ok(_) => { + let mut app = self.app.lock().await; + app.native_device_id = Some(device_id); + if let Some(ctx) = &mut app.current_playback_context { + ctx.is_playing = true; + } + app.dispatch(IoEvent::GetCurrentPlayback); + } + Err(e) => { + let mut app = self.app.lock().await; + app.set_status_message( + format!("No playback to resume on {}.", player.device_name()), + 4, + ); + info!("native resume via Spotify API failed: {}", e); + } + } + } else { + let mut app = self.app.lock().await; + app.set_status_message( + format!("No playback to resume on {}.", player.device_name()), + 4, + ); } return; } @@ -762,44 +916,8 @@ impl PlaybackNetwork for Network { } } - // For URI-based or context playback, use Spirc load directly. - let mut options = LoadRequestOptions { - start_playing: true, - seek_to: 0, - context_options: None, - playing_track: None, - }; - - let request = match (context_id, uris) { - (Some(context), Some(track_uris)) => { - if let Some(first_uri) = track_uris.first() { - options.playing_track = Some(PlayingTrack::Uri(first_uri.uri())); - } else if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { - options.playing_track = Some(PlayingTrack::Index(i)); - } - LoadRequest::from_context_uri(context.uri(), options) - } - (Some(context), None) => { - if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { - options.playing_track = Some(PlayingTrack::Index(i)); - } - LoadRequest::from_context_uri(context.uri(), options) - } - (None, Some(track_uris)) => { - if let Some(i) = offset.and_then(|i| u32::try_from(i).ok()) { - options.playing_track = Some(PlayingTrack::Index(i)); - } - let uris = track_uris.into_iter().map(|u| u.uri()).collect::>(); - LoadRequest::from_tracks(uris, options) - } - (None, None) => { - player.play(); - let mut app = self.app.lock().await; - if let Some(ctx) = &mut app.current_playback_context { - ctx.is_playing = true; - } - return; - } + let Some(request) = native_load_request(context_id, uris, offset) else { + return; }; info!("starting native playback via direct load route"); @@ -848,8 +966,79 @@ impl PlaybackNetwork for Network { app.user_config.behavior.shuffle_enabled = desired_shuffle_state; } Err(e) => { + #[cfg(feature = "streaming")] + if is_no_active_device_error(&e) { + if let Some(player) = current_streaming_player(self).await { + if player.is_connected() { + let requested_origin = + requested_native_playback_origin(self, &context_id, &uris).await; + let activation_time = Instant::now(); + let native_device_id = player.device_id(); + player.activate(); + { + let mut app = self.app.lock().await; + app.is_streaming_active = true; + app.native_activation_pending = false; + app.native_playback_origin = Some(requested_origin); + app.native_device_id = Some(native_device_id.clone()); + app.last_device_activation = Some(activation_time); + app.instant_since_last_current_playback_poll = + activation_time - Duration::from_secs(6); + } + + if let Some(request) = native_load_request(context_id, uris, offset) { + info!("default Spotify playback had no active device; falling back to native load"); + if let Err(load_err) = player.load(request) { + let mut app = self.app.lock().await; + app.handle_error(anyhow!("Failed to start native playback: {}", load_err)); + } else { + let _ = player.set_shuffle(desired_shuffle_state); + let mut app = self.app.lock().await; + if let Some(ctx) = &mut app.current_playback_context { + ctx.is_playing = true; + ctx.shuffle_state = desired_shuffle_state; + } + app.user_config.behavior.shuffle_enabled = desired_shuffle_state; + } + return; + } + + info!( + "default Spotify resume had no active device; retrying on native device {}", + native_device_id + ); + match self + .spotify_api_request_json( + Method::PUT, + "me/player/play", + &[("device_id", native_device_id.clone())], + None, + ) + .await + { + Ok(_) => { + let mut app = self.app.lock().await; + if let Some(ctx) = &mut app.current_playback_context { + ctx.is_playing = true; + } + app.dispatch(IoEvent::GetCurrentPlayback); + } + Err(resume_err) => { + let mut app = self.app.lock().await; + app.set_status_message( + format!("No playback to resume on {}.", player.device_name()), + 4, + ); + info!("native resume fallback failed: {}", resume_err); + } + } + return; + } + } + } + let mut app = self.app.lock().await; - app.handle_error(anyhow!(e)); + app.handle_error(e); } } } @@ -1471,6 +1660,115 @@ mod tests { )); } + #[cfg(feature = "streaming")] + #[test] + fn no_active_device_error_matches_spotify_no_device_signals() { + assert!(is_no_active_device_error(&anyhow!( + "{}", + r#"Spotify API 404 Not Found failed: {"error":{"reason":"NO_ACTIVE_DEVICE"}}"# + ))); + assert!(is_no_active_device_error(&anyhow!( + "Spotify API 404 Not Found failed: No active device found" + ))); + } + + #[cfg(feature = "streaming")] + #[test] + fn no_active_device_error_does_not_match_generic_not_found() { + assert!(!is_no_active_device_error(&anyhow!( + "Spotify API 404 Not Found failed: playlist not found" + ))); + assert!(!is_no_active_device_error(&anyhow!( + "Spotify API 404 failed for https://example.test/404" + ))); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_uses_native_when_no_current_device_or_saved_device() { + assert!(should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_uses_native_when_saved_device_is_unavailable() { + assert!(should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + saved_external_confirmed_available: false, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_keeps_confirmed_external_device() { + assert!(!should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + current_device_id_present: true, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_keeps_confirmed_saved_external_device() { + assert!(!should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + saved_external_confirmed_available: true, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_uses_native_for_saved_native_device() { + assert!(should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + saved_device_matches_native: true, + saved_external_confirmed_available: true, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_uses_native_for_stale_native_name_match() { + assert!(should_activate_native_for_playback( + NativeActivationContext { + player_connected: true, + current_device_id_present: true, + current_device_name_matches_native: true, + native_has_fresh_activity: false, + ..Default::default() + }, + )); + } + + #[cfg(feature = "streaming")] + #[test] + fn native_activation_ignores_disconnected_player() { + assert!(!should_activate_native_for_playback( + NativeActivationContext { + player_connected: false, + saved_device_matches_native: true, + ..Default::default() + }, + )); + } + #[cfg(feature = "streaming")] #[test] fn stale_api_item_keeps_native_metadata_when_native_was_active() { diff --git a/src/infra/player/events.rs b/src/infra/player/events.rs index 2e0350c..512a108 100644 --- a/src/infra/player/events.rs +++ b/src/infra/player/events.rs @@ -610,7 +610,7 @@ fn current_playback_matches_native(app: &App, player: &StreamingPlayer) -> bool } } - ctx.device.name.eq_ignore_ascii_case(player.device_name()) + ctx.device.name.eq_ignore_ascii_case(player.device_name()) && app.has_fresh_native_activity() } async fn disconnect_streaming_player( @@ -643,9 +643,9 @@ async fn disconnect_streaming_player( app_lock.last_track_id = None; app_lock.last_device_activation = None; app_lock.seek_ms = None; - if reselect_device { - app_lock.current_playback_context = None; - } + // The cached API context may still point at the stale native session; the + // dispatch below repopulates it if Spotify has already moved to another device. + app_lock.current_playback_context = None; app_lock.set_status_message(status_message, 8); app_lock.dispatch(IoEvent::GetCurrentPlayback);