diff --git a/.gitignore b/.gitignore index 9469b02d..679494fd 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ RF_old.json .claude/ .claude.old Kilo-session.md +pmomusic_logs.txt diff --git a/Cargo.lock b/Cargo.lock index fd6c093e..450d1a02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "PMOMusic" -version = "0.3.48" +version = "0.3.49" dependencies = [ "axum 0.8.7", "console-subscriber", diff --git a/PMOMusic/Cargo.toml b/PMOMusic/Cargo.toml index 32f6d26a..18df9b9c 100644 --- a/PMOMusic/Cargo.toml +++ b/PMOMusic/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "PMOMusic" -version = "0.3.48" +version = "0.3.49" edition = "2024" [dependencies] diff --git a/pmoapp/webapp/index.html b/pmoapp/webapp/index.html index 3c01cf91..1802a0d7 100644 --- a/pmoapp/webapp/index.html +++ b/pmoapp/webapp/index.html @@ -3,7 +3,7 @@ - + webapp diff --git a/pmoapp/webapp/src/assets/styles/drawers.css b/pmoapp/webapp/src/assets/styles/drawers.css index 1d7adc95..bc87c1af 100644 --- a/pmoapp/webapp/src/assets/styles/drawers.css +++ b/pmoapp/webapp/src/assets/styles/drawers.css @@ -257,6 +257,9 @@ .drawer-footer { padding: var(--spacing-md); + /* On mobile, extend into the safe-area so the footer doesn't sit under + the system navigation bar / home indicator. */ + padding-bottom: max(var(--spacing-md), env(safe-area-inset-bottom, 0px)); border-top: 1px solid rgba(255, 255, 255, 0.1); flex-shrink: 0; } diff --git a/pmoapp/webapp/src/components/unified/BottomTabBar.vue b/pmoapp/webapp/src/components/unified/BottomTabBar.vue index 48d6383e..37721e90 100644 --- a/pmoapp/webapp/src/components/unified/BottomTabBar.vue +++ b/pmoapp/webapp/src/components/unified/BottomTabBar.vue @@ -147,7 +147,10 @@ function handleRendererDrawerClick() { align-items: center; gap: var(--spacing-md); height: 72px; - padding: 0 var(--spacing-md); + /* Extend background into the system navigation bar area (iOS home indicator, + Android gesture bar). Content stays in the 72px zone; only the visual + background bleeds into the safe-area below. */ + padding: 0 var(--spacing-md) env(safe-area-inset-bottom, 0px); background: rgba(22, 22, 32, 0.96); backdrop-filter: blur(8px); -webkit-backdrop-filter: blur(8px); @@ -323,7 +326,7 @@ function handleRendererDrawerClick() { @media (max-width: 768px) { .bottom-bar { height: 64px; - padding: 0 var(--spacing-sm); + padding: 0 var(--spacing-sm) env(safe-area-inset-bottom, 0px); gap: var(--spacing-sm); } diff --git a/pmoapp/webapp/src/components/unified/RendererTabContent.vue b/pmoapp/webapp/src/components/unified/RendererTabContent.vue index 769c2b91..9dd5c932 100644 --- a/pmoapp/webapp/src/components/unified/RendererTabContent.vue +++ b/pmoapp/webapp/src/components/unified/RendererTabContent.vue @@ -239,10 +239,11 @@ async function handleQueueItemClick(item: QueueItem) { -webkit-backdrop-filter: blur(8px); border-top: 1px solid rgba(255, 255, 255, 0.12); box-shadow: 0 -4px 32px rgba(0, 0, 0, 0.4); - /* Fermé: caché sauf le toggle (56px) qui dépasse au-dessus de la BottomTabBar (64px) */ - transform: translateY(calc(100% - 56px - 64px)); + /* Fermé: caché sauf le toggle (56px) qui dépasse au-dessus de la BottomTabBar. + La BottomTabBar fait 64px + env(safe-area-inset-bottom) de padding. */ + transform: translateY(calc(100% - 56px - 64px - env(safe-area-inset-bottom, 0px))); transition: transform 0.3s ease; - z-index: 95; /* Au-dessus de la BottomTabBar (z-index: 100) */ + z-index: 95; /* En dessous de la BottomTabBar (z-index: 100) */ max-height: 70vh; display: flex; flex-direction: column; @@ -251,8 +252,8 @@ async function handleQueueItemClick(item: QueueItem) { } .queue-drawer.open { - /* Ouvert: remonte mais s'arrête à 64px du bas pour laisser la BottomTabBar accessible */ - transform: translateY(64px); + /* Ouvert: remonte juste au-dessus de la BottomTabBar (64px + safe area) */ + transform: translateY(calc(64px + env(safe-area-inset-bottom, 0px))); pointer-events: auto; /* Ouvert: capture les clics */ } @@ -284,6 +285,8 @@ async function handleQueueItemClick(item: QueueItem) { max-height: calc(70vh - 56px); overflow-y: auto; padding: var(--spacing-md); + /* Ensure the last item clears the safe-area / system nav bar */ + padding-bottom: max(var(--spacing-md), env(safe-area-inset-bottom, 0px)); } .queue-drawer-backdrop { diff --git a/pmocontrol/src/errors.rs b/pmocontrol/src/errors.rs index 2d0ef137..8607f034 100644 --- a/pmocontrol/src/errors.rs +++ b/pmocontrol/src/errors.rs @@ -58,6 +58,15 @@ impl ControlPointError { ControlPointError::UpnpOperationNotSupported(operation.to_string(), service.to_string()) } + /// Returns true if the error is a transient transport-level failure that + /// may succeed on retry (TCP refused, connection reset, timeout). + /// + /// Protocol-level errors (UPnP fault, HTTP 4xx/5xx with a valid body) + /// are **not** transient: the device understood the request and rejected it. + pub fn is_transient_soap_error(&self) -> bool { + matches!(self, ControlPointError::SoapAction(_)) + } + pub fn upnp_missing_return_value(value: &str) -> Self { ControlPointError::UpnpMissingReturnValue(value.to_string()) } diff --git a/pmocontrol/src/music_renderer/arylic_tcp.rs b/pmocontrol/src/music_renderer/arylic_tcp.rs index 9b900d29..de91e3d1 100644 --- a/pmocontrol/src/music_renderer/arylic_tcp.rs +++ b/pmocontrol/src/music_renderer/arylic_tcp.rs @@ -243,61 +243,24 @@ impl PlaybackStatus for ArylicTcpRenderer { impl PlaybackPosition for ArylicTcpRenderer { fn playback_position(&self) -> Result { - let info = match self.fetch_playback_info() { - Ok(info) => { - tracing::debug!("ArylicTcp fetch_playback_info returned: {:?}", info); - info - } - Err(e) => { - tracing::warn!("ArylicTcp fetch_playback_info failed: {}", e); - return Err(e); - } - }; + let info = self.fetch_playback_info().map_err(|e| { + tracing::warn!("ArylicTcp fetch_playback_info failed: {}", e); + e + })?; - let mut position_info = info.position_info(); + tracing::debug!("ArylicTcp fetch_playback_info returned: {:?}", info); + + let mut position = info.position_info(); tracing::debug!( - "ArylicTcp position_info: track_duration={:?}, rel_time={:?}, track_metadata={:?}, track_uri={:?}", - position_info.track_duration, - position_info.rel_time, - position_info - .track_metadata - .as_ref() - .map(|s| &s[..s.len().min(100)]), - position_info.track_uri + "ArylicTcp position_info: track_duration={:?}, rel_time={:?}", + position.track_duration, + position.rel_time, ); - // Récupérer les métadonnées depuis la queue (avec protection contre diminution de durée) - // Normalement current_index est toujours Some() si la queue n'est pas vide (règle métier) - let mut queue_guard = self.queue.lock().expect("queue mutex poisoned"); - let queue_item = queue_guard.peek_current().ok().flatten(); + // Replace device metadata with queue metadata (queue is authoritative). + crate::music_renderer::musicrenderer::enrich_position_from_queue(self, &mut position); - if let Some((current_item, _)) = queue_item { - // Build DIDL metadata XML from cached/protected TrackMetadata - if let Some(ref metadata) = current_item.metadata { - tracing::debug!( - "ArylicTcp playback_position: using queue metadata - title={:?}, artist={:?}, duration={:?}, is_stream={}", - metadata.title, - metadata.artist, - metadata.duration, - metadata.is_continuous_stream - ); - position_info.track_metadata = Some( - crate::music_renderer::musicrenderer::build_didl_lite_metadata( - metadata, - ¤t_item.uri, - ¤t_item.protocol_info, - ), - ); - } else { - tracing::warn!("ArylicTcp playback_position: queue item has no metadata"); - } - position_info.track_uri = Some(current_item.uri.clone()); - } else { - tracing::warn!("ArylicTcp playback_position: no current queue item"); - } - drop(queue_guard); - - Ok(position_info) + Ok(position) } } diff --git a/pmocontrol/src/music_renderer/chromecast_renderer.rs b/pmocontrol/src/music_renderer/chromecast_renderer.rs index 28d79723..4a3934b5 100644 --- a/pmocontrol/src/music_renderer/chromecast_renderer.rs +++ b/pmocontrol/src/music_renderer/chromecast_renderer.rs @@ -36,7 +36,7 @@ use crate::DeviceIdentity; use rust_cast::{ channels::{ heartbeat::HeartbeatResponse, - media::{Media, PlayerState as CastPlayerState, StreamType}, + media::{Media, PlayerState as CastPlayerState, StatusEntry, StreamType}, receiver::CastDeviceApp, }, CastDevice, ChannelMessage, @@ -175,6 +175,50 @@ impl ChromecastRenderer { *self.continuous_stream.lock().expect("continuous_stream mutex poisoned") } + /// Returns `(transport_id, media_entry)` for the currently active Cast session. + /// + /// This encapsulates the repeated sequence: + /// connect device → get receiver status → get active app → connect to app → get media status + /// + /// Used by all transport operations (play/pause/stop/seek) and by + /// playback_state/playback_position to avoid duplicating this boilerplate. + fn get_active_media_entry<'d>( + &self, + device: &'d CastDevice<'d>, + ) -> Result<(String, StatusEntry), ControlPointError> { + let status = device.receiver.get_status().map_err(|e| { + ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) + })?; + + let app = status + .applications + .into_iter() + .next() + .ok_or_else(|| ControlPointError::ChromecastError("No active app found".into()))?; + + device + .connection + .connect(app.transport_id.as_str()) + .map_err(|e| { + ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) + })?; + + let media_status = device + .media + .get_status(app.transport_id.as_str(), None) + .map_err(|e| { + ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) + })?; + + let entry = media_status + .entries + .into_iter() + .next() + .ok_or_else(|| ControlPointError::ChromecastError("No media session found".into()))?; + + Ok((app.transport_id, entry)) + } + /// Connect to the device with retry on connection failures. /// Uses exponential backoff: 200ms, 400ms, 800ms fn connect_with_retry(&self) -> Result, ControlPointError> { @@ -203,15 +247,6 @@ impl TransportControl for ChromecastRenderer { fn play_uri(&self, uri: &str, meta: &str) -> Result<(), ControlPointError> { debug!("ChromecastRenderer: play_uri({})", uri); - // Détecte si l'URL est un flux continu - let is_stream = crate::music_renderer::is_continuous_stream_url(uri); - *self.continuous_stream.lock().expect("continuous_stream mutex poisoned") = is_stream; - tracing::debug!( - "ChromecastRenderer play_uri: URI={}, continuous_stream={}", - uri, - is_stream - ); - // Signal any existing play thread to stop if let Ok(mut stop) = self.stop_signal.lock() { *stop = true; @@ -378,186 +413,53 @@ impl TransportControl for ChromecastRenderer { fn play(&self) -> Result<(), ControlPointError> { debug!("ChromecastRenderer: play()"); - let device = self.connect_with_retry()?; - - // Get receiver status to find the active app - let status = device.receiver.get_status().map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) - })?; - - let app = status - .applications - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No active app found")))?; - - // Connect to the app - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - // Get media status - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - let media_entry = media_status - .entries - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No media session found")))?; - - // Send play command + let (transport_id, entry) = self.get_active_media_entry(&device)?; device .media - .play(app.transport_id.as_str(), media_entry.media_session_id) + .play(transport_id.as_str(), entry.media_session_id) .map_err(|e| ControlPointError::ChromecastError(format!("Failed to play: {}", e)))?; - Ok(()) } fn pause(&self) -> Result<(), ControlPointError> { debug!("ChromecastRenderer: pause()"); - let device = self.connect_with_retry()?; - - let status = device.receiver.get_status().map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) - })?; - - let app = status - .applications - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No active app found")))?; - - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - let media_entry = media_status - .entries - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No media session found")))?; - + let (transport_id, entry) = self.get_active_media_entry(&device)?; device .media - .pause(app.transport_id.as_str(), media_entry.media_session_id) + .pause(transport_id.as_str(), entry.media_session_id) .map_err(|e| ControlPointError::ChromecastError(format!("Failed to pause: {}", e)))?; - Ok(()) } fn stop(&self) -> Result<(), ControlPointError> { debug!("ChromecastRenderer: stop()"); - // Signal the play thread to stop + // Signal the play thread to stop first. + // The thread terminates on its own; play_uri() will wait for it if needed. if let Ok(mut stop) = self.stop_signal.lock() { *stop = true; } - // Note: We don't wait for the thread here as stop() should be quick. - // The thread will terminate on its own when it checks stop_signal. - // If a new play_uri() is called, it will properly wait for this thread. - - // Also send stop command to the device let device = self.connect_with_retry()?; - - let status = device.receiver.get_status().map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) - })?; - - let app = status - .applications - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No active app found")))?; - - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - let media_entry = media_status - .entries - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No media session found")))?; - + let (transport_id, entry) = self.get_active_media_entry(&device)?; device .media - .stop(app.transport_id.as_str(), media_entry.media_session_id) + .stop(transport_id.as_str(), entry.media_session_id) .map_err(|e| ControlPointError::ChromecastError(format!("Failed to stop: {}", e)))?; - Ok(()) } fn seek_rel_time(&self, hhmmss: &str) -> Result<(), ControlPointError> { debug!("ChromecastRenderer: seek_rel_time({})", hhmmss); - let total_seconds = parse_hhmmss_strict(hhmmss)? as f32; - let device = self.connect_with_retry()?; - - let status = device.receiver.get_status().map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) - })?; - - let app = status - .applications - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No active app found")))?; - - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - let media_entry = media_status - .entries - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No media session found")))?; - + let (transport_id, entry) = self.get_active_media_entry(&device)?; device .media - .seek( - app.transport_id.as_str(), - media_entry.media_session_id, - Some(total_seconds), - None, - ) + .seek(transport_id.as_str(), entry.media_session_id, Some(total_seconds), None) .map_err(|e| ControlPointError::ChromecastError(format!("Failed to seek: {}", e)))?; - Ok(()) } } @@ -566,125 +468,57 @@ impl PlaybackStatus for ChromecastRenderer { fn playback_state(&self) -> Result { let device = self.connect_with_retry()?; - // Get receiver status to find the active app + // If no app is running there is no media — return NoMedia without error. let status = device.receiver.get_status().map_err(|e| { ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) })?; + tracing::debug!("Chromecast playback_state: {} apps running", status.applications.len()); - tracing::debug!( - "Chromecast playback_state: {} apps running", - status.applications.len() - ); + if status.applications.is_empty() { + tracing::debug!("Chromecast playback_state: no apps running, returning NoMedia"); + return Ok(PlaybackState::NoMedia); + } - // If no app is running, return NoMedia - let app = match status.applications.first() { - Some(app) => { - tracing::debug!("Chromecast playback_state: app={}", app.display_name); - app - } - None => { - tracing::debug!("Chromecast playback_state: no apps running, returning NoMedia"); - return Ok(PlaybackState::NoMedia); - } - }; - - // Connect to the app - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - // Get media status - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - tracing::debug!( - "Chromecast playback_state: {} media entries", - media_status.entries.len() - ); - - // If no media entry, return NoMedia - let media_entry = match media_status.entries.first() { - Some(entry) => { + match self.get_active_media_entry(&device) { + Ok((_, entry)) => { tracing::debug!( "Chromecast playback_state: player_state={:?}, current_time={:?}", - entry.player_state, - entry.current_time + entry.player_state, entry.current_time ); - entry + Ok(map_player_state(&entry.player_state)) } - None => { - tracing::debug!("Chromecast playback_state: no media entries, returning NoMedia"); - return Ok(PlaybackState::NoMedia); + // No media session → device is idle + Err(_) => { + tracing::debug!("Chromecast playback_state: no media session, returning NoMedia"); + Ok(PlaybackState::NoMedia) } - }; - - Ok(map_player_state(&media_entry.player_state)) + } } } impl PlaybackPosition for ChromecastRenderer { fn playback_position(&self) -> Result { let device = self.connect_with_retry()?; + let (_, entry) = self.get_active_media_entry(&device)?; - // Get receiver status to find the active app - let status = device.receiver.get_status().map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get receiver status: {}", e)) - })?; + let rel_time = entry.current_time.map(|t| format_hhmmss_f64(t as f64)); + let track_duration = entry.media.as_ref().and_then(|m| m.duration).map(|d| format_hhmmss_f64(d as f64)); + let track_uri = entry.media.as_ref().map(|m| m.content_id.clone()); - let app = status - .applications - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No active app found")))?; - - // Connect to the app - device - .connection - .connect(app.transport_id.as_str()) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to connect to app: {}", e)) - })?; - - // Get media status - let media_status = device - .media - .get_status(app.transport_id.as_str(), None) - .map_err(|e| { - ControlPointError::ChromecastError(format!("Failed to get media status: {}", e)) - })?; - - let media_entry = media_status - .entries - .first() - .ok_or_else(|| ControlPointError::ChromecastError(format!("No media session found")))?; - - // Extract position information - let rel_time = media_entry - .current_time - .map(|time| format_hhmmss_f64(time as f64)); - - let track_duration = media_entry - .media - .as_ref() - .and_then(|m| m.duration) - .map(|dur| format_hhmmss_f64(dur as f64)); - - let track_uri = media_entry.media.as_ref().map(|m| m.content_id.clone()); - - Ok(PlaybackPositionInfo { + let mut position = PlaybackPositionInfo { track: Some(1), rel_time, abs_time: None, track_duration, - track_metadata: None, // Chromecast doesn't use DIDL-Lite + track_metadata: None, track_uri, - }) + }; + + // Replace device metadata with queue metadata (queue is authoritative; + // Chromecast does not return DIDL-Lite natively so the queue is the only source). + crate::music_renderer::musicrenderer::enrich_position_from_queue(self, &mut position); + + Ok(position) } } diff --git a/pmocontrol/src/music_renderer/linkplay_renderer.rs b/pmocontrol/src/music_renderer/linkplay_renderer.rs index 626f0962..3ebc193d 100644 --- a/pmocontrol/src/music_renderer/linkplay_renderer.rs +++ b/pmocontrol/src/music_renderer/linkplay_renderer.rs @@ -97,15 +97,6 @@ impl LinkPlayRenderer { impl TransportControl for LinkPlayRenderer { fn play_uri(&self, uri: &str, _meta: &str) -> Result<(), ControlPointError> { - // Détecte si l'URL est un flux continu - let is_stream = crate::music_renderer::is_continuous_stream_url(uri); - *self.continuous_stream.lock().expect("continuous_stream mutex poisoned") = is_stream; - tracing::debug!( - "LinkPlayRenderer play_uri: URI={}, continuous_stream={}", - uri, - is_stream - ); - let encoded = percent_encode(uri); self.send_player_command(&format!("play:{}", encoded)) } @@ -155,27 +146,10 @@ impl PlaybackStatus for LinkPlayRenderer { impl PlaybackPosition for LinkPlayRenderer { fn playback_position(&self) -> Result { - let mut position_info = self.fetch_status()?.position_info(); - - // Use queue metadata instead of direct status metadata to benefit from duration protection - let mut queue_guard = self.queue.lock().expect("queue mutex poisoned"); - let queue_item = queue_guard.peek_current().ok().flatten(); - - if let Some((current_item, _)) = queue_item { - if let Some(ref metadata) = current_item.metadata { - position_info.track_metadata = Some( - crate::music_renderer::musicrenderer::build_didl_lite_metadata( - metadata, - ¤t_item.uri, - ¤t_item.protocol_info, - ), - ); - } - position_info.track_uri = Some(current_item.uri.clone()); - } - drop(queue_guard); - - Ok(position_info) + let mut position = self.fetch_status()?.position_info(); + // Replace device metadata with queue metadata (queue is authoritative). + crate::music_renderer::musicrenderer::enrich_position_from_queue(self, &mut position); + Ok(position) } } diff --git a/pmocontrol/src/music_renderer/musicrenderer.rs b/pmocontrol/src/music_renderer/musicrenderer.rs index 99c03f09..5d347e8d 100644 --- a/pmocontrol/src/music_renderer/musicrenderer.rs +++ b/pmocontrol/src/music_renderer/musicrenderer.rs @@ -1948,6 +1948,33 @@ pub(crate) fn build_didl_lite_metadata( didl.to_xml() } +/// Enriches a [`PlaybackPositionInfo`] with metadata from the backend's queue. +/// +/// Overwrites `track_metadata` and `track_uri` with the values stored for the +/// current queue item. This is a no-op when the queue is empty or has no +/// current item. +/// +/// All backends that hold a local queue (UPnP, LinkPlay, Arylic, Chromecast) +/// should call this after fetching the raw device position, so that callers +/// always receive consistent, queue-authoritative metadata rather than +/// potentially stale or absent metadata coming from the device itself. +pub(crate) fn enrich_position_from_queue( + backend: &B, + position: &mut PlaybackPositionInfo, +) { + let mut queue = backend.queue().lock().expect("queue mutex poisoned"); + if let Ok(Some((current_item, _))) = queue.peek_current() { + if let Some(ref metadata) = current_item.metadata { + position.track_metadata = Some(build_didl_lite_metadata( + metadata, + ¤t_item.uri, + ¤t_item.protocol_info, + )); + } + position.track_uri = Some(current_item.uri.clone()); + } +} + impl DeviceIdentity for MusicRenderer { fn id(&self) -> DeviceId { self.info.id() diff --git a/pmocontrol/src/music_renderer/upnp_renderer.rs b/pmocontrol/src/music_renderer/upnp_renderer.rs index 78b364c0..01c1e57c 100644 --- a/pmocontrol/src/music_renderer/upnp_renderer.rs +++ b/pmocontrol/src/music_renderer/upnp_renderer.rs @@ -231,15 +231,6 @@ impl TransportControl for UpnpRenderer { ); } - // Détecte si l'URL est un flux continu en interrogeant le serveur HTTP - let is_stream = crate::music_renderer::is_continuous_stream_url(uri); - *self.continuous_stream.lock().expect("continuous_stream mutex poisoned") = is_stream; - tracing::debug!( - "UpnpRenderer play_uri: URI={}, continuous_stream={}", - uri, - is_stream - ); - // Parse le DIDL pour extraire la durée (fallback pour certains amplis) let duration = parse_didl_duration(meta); if let Some(ref dur) = duration { @@ -326,57 +317,30 @@ impl PlaybackPosition for UpnpRenderer { ); // Normalize "00:00:00" or "0:00:00" to None (some renderers return this for unknown duration) - let normalized_duration = raw.track_duration.as_ref().and_then(|d| { - if d == "00:00:00" || d == "0:00:00" { - None - } else { - Some(d.clone()) - } + let track_duration = raw.track_duration.as_ref().and_then(|d| { + if d == "00:00:00" || d == "0:00:00" { None } else { Some(d.clone()) } }); - let track_duration = normalized_duration; - - // Récupérer les métadonnées depuis la queue (avec protection contre diminution de durée) - // plutôt que depuis GetPositionInfo qui peut retourner des métadonnées obsolètes - let mut track_metadata_xml = None; - let mut track_uri = raw.track_uri.clone(); - - let mut queue_guard = self.queue.lock().expect("queue mutex poisoned"); - - // Récupérer l'item courant de la queue - // Normalement current_index est toujours Some() si la queue n'est pas vide (règle métier) - let queue_item = queue_guard.peek_current().ok().flatten(); - - if let Some((current_item, _)) = queue_item { - track_uri = Some(current_item.uri.clone()); - - // Build DIDL metadata XML from cached/protected TrackMetadata - if let Some(ref metadata) = current_item.metadata { - track_metadata_xml = Some( - crate::music_renderer::musicrenderer::build_didl_lite_metadata( - metadata, - ¤t_item.uri, - ¤t_item.protocol_info, - ), - ); - } - } - drop(queue_guard); - - tracing::trace!( - "UPnP playback_position: track_duration={:?}, rel_time={:?}, using_queue_metadata={}", - track_duration, - raw.rel_time, - track_metadata_xml.is_some() - ); - - Ok(PlaybackPositionInfo { + let mut position = PlaybackPositionInfo { track: Some(raw.track), rel_time: raw.rel_time, abs_time: raw.abs_time, track_duration, - track_metadata: track_metadata_xml, - track_uri, - }) + track_metadata: None, + track_uri: raw.track_uri, + }; + + // Replace device metadata with queue metadata (queue is authoritative and protected + // against stale / decreasing durations for streams). + crate::music_renderer::musicrenderer::enrich_position_from_queue(self, &mut position); + + tracing::trace!( + "UPnP playback_position: track_duration={:?}, rel_time={:?}, using_queue_metadata={}", + position.track_duration, + position.rel_time, + position.track_metadata.is_some() + ); + + Ok(position) } } diff --git a/pmocontrol/src/soap_client.rs b/pmocontrol/src/soap_client.rs index 4b91d4f6..3c54ef35 100644 --- a/pmocontrol/src/soap_client.rs +++ b/pmocontrol/src/soap_client.rs @@ -4,13 +4,18 @@ use std::time::Duration; use anyhow::{Context, Result}; use pmoupnp::soap::{build_soap_request, parse_soap_envelope, SoapEnvelope}; -use tracing::{debug, trace, warn}; +use tracing::{debug, trace, warn, info}; use ureq::Agent; use crate::errors::ControlPointError; static SOAP_AGENT: OnceLock> = OnceLock::new(); +/// Number of retries for transient transport failures on control actions. +const SOAP_CONTROL_MAX_RETRIES: usize = 2; +/// Delay between retries (ms). Kept short: devices usually recover in <200 ms. +const SOAP_CONTROL_RETRY_DELAY_MS: u64 = 300; + fn get_soap_agent() -> Arc { SOAP_AGENT .get_or_init(|| { @@ -43,19 +48,44 @@ pub fn build_soap_body( build_soap_request(service_type, action, args) } -/// Invoke a UPnP SOAP action on a control URL. +/// Invoke a UPnP SOAP control action with automatic retry on transient failures. /// -/// - `control_url`: full HTTP URL of the service control endpoint -/// - `service_type`: service URN -/// - `action`: action name -/// - `args`: list of (name, value) +/// Use this for **control operations** (Play, Pause, Stop, SetAVTransportURI, …) +/// where a transient TCP error should be absorbed silently. +/// +/// Retries are attempted only on [`ControlPointError::is_transient_soap_error`] +/// (i.e. transport-level failures). Protocol-level errors (UPnP faults, +/// HTTP 4xx/5xx with a valid body) propagate immediately without retry. +/// +/// For **polling reads** (GetTransportInfo, GetPositionInfo, …) prefer +/// [`invoke_upnp_action_with_timeout`] directly so that a slow device does +/// not hold the watcher thread for multiple retry cycles. pub fn invoke_upnp_action( control_url: &str, service_type: &str, action: &str, args: &[(&str, &str)], ) -> Result { - invoke_upnp_action_with_timeout(control_url, service_type, action, args, None) + let retry_delay = Duration::from_millis(SOAP_CONTROL_RETRY_DELAY_MS); + + for attempt in 0..=SOAP_CONTROL_MAX_RETRIES { + match invoke_upnp_action_with_timeout(control_url, service_type, action, args, None) { + Ok(result) => return Ok(result), + Err(e) if e.is_transient_soap_error() && attempt < SOAP_CONTROL_MAX_RETRIES => { + info!( + url = control_url, + action = action, + attempt = attempt + 1, + error = %e, + "Transient SOAP error, retrying" + ); + std::thread::sleep(retry_delay); + } + Err(e) => return Err(e), + } + } + // Unreachable: the loop either returns Ok or propagates Err above. + unreachable!() } pub fn invoke_upnp_action_with_timeout( diff --git a/pmocontrol/src/upnp_clients/avtransport_client.rs b/pmocontrol/src/upnp_clients/avtransport_client.rs index 7bd16166..b824a95e 100644 --- a/pmocontrol/src/upnp_clients/avtransport_client.rs +++ b/pmocontrol/src/upnp_clients/avtransport_client.rs @@ -12,7 +12,11 @@ use crate::{ use pmoupnp::soap::SoapEnvelope; use xmltree::{Element, XMLNode}; +/// Timeout for slow/long control actions (SetAVTransportURI, SetNextAVTransportURI). const AVTRANSPORT_ACTION_TIMEOUT: Duration = Duration::from_secs(5); +/// Timeout for fast polling-read actions (GetTransportInfo, GetPositionInfo). +/// Must be well below the watcher short-interval (500 ms) to avoid cascading lateness. +const AVTRANSPORT_POLL_TIMEOUT: Duration = Duration::from_secs(3); #[derive(Debug, Clone)] pub struct AvTransportClient { @@ -40,11 +44,12 @@ impl AvTransportClient { let instance_id_str = instance_id.to_string(); let args = [("InstanceID", instance_id_str.as_str())]; - let call_result = invoke_upnp_action( + let call_result = invoke_upnp_action_with_timeout( &self.control_url, &self.service_type, "GetTransportInfo", &args, + Some(AVTRANSPORT_POLL_TIMEOUT), )?; if !call_result.status.is_success() { @@ -381,11 +386,12 @@ impl AvTransportClient { let instance_id_str = instance_id.to_string(); let args = [("InstanceID", instance_id_str.as_str())]; - let call_result = invoke_upnp_action( + let call_result = invoke_upnp_action_with_timeout( &self.control_url, &self.service_type, "GetPositionInfo", &args, + Some(AVTRANSPORT_POLL_TIMEOUT), )?; if !call_result.status.is_success() { diff --git a/pmocontrol/src/upnp_clients/rendering_control_client.rs b/pmocontrol/src/upnp_clients/rendering_control_client.rs index 3e7108d1..900cdc62 100644 --- a/pmocontrol/src/upnp_clients/rendering_control_client.rs +++ b/pmocontrol/src/upnp_clients/rendering_control_client.rs @@ -1,12 +1,17 @@ +use std::time::Duration; + use crate::{ errors::ControlPointError, soap_client::{ ensure_success, extract_child_text, find_child_with_suffix, handle_action_response, - invoke_upnp_action, parse_upnp_error, + invoke_upnp_action, invoke_upnp_action_with_timeout, parse_upnp_error, }, }; use tracing::debug; +/// Timeout for fast polling-read actions (GetVolume, GetMute). +const RENDERING_CONTROL_POLL_TIMEOUT: Duration = Duration::from_secs(3); + #[derive(Debug, Clone)] pub struct RenderingControlClient { pub control_url: String, @@ -30,8 +35,13 @@ impl RenderingControlClient { ("Channel", channel), ]; - let call_result = - invoke_upnp_action(&self.control_url, &self.service_type, "GetVolume", &args)?; + let call_result = invoke_upnp_action_with_timeout( + &self.control_url, + &self.service_type, + "GetVolume", + &args, + Some(RENDERING_CONTROL_POLL_TIMEOUT), + )?; ensure_success("GetVolume", &call_result)?; @@ -90,8 +100,13 @@ impl RenderingControlClient { ("Channel", channel), ]; - let call_result = - invoke_upnp_action(&self.control_url, &self.service_type, "GetMute", &args)?; + let call_result = invoke_upnp_action_with_timeout( + &self.control_url, + &self.service_type, + "GetMute", + &args, + Some(RENDERING_CONTROL_POLL_TIMEOUT), + )?; ensure_success("GetMute", &call_result)?; diff --git a/version.txt b/version.txt index 2fb885a2..b3682c13 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.48 +0.3.49