From ee8a1fdc93aa92260be3e59c9137644fce94c571 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 4 Jun 2026 07:00:22 -0400 Subject: [PATCH] fix(android): event-driven pending-transport clear (Sonos ack OR 5s safety) --- .../player/MinstrelForwardingPlayer.kt | 17 +++++++++-- .../minstrel/player/PlayerController.kt | 30 +++++++++++-------- .../minstrel/player/RemotePlayerState.kt | 28 ++++++++++------- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt index 6b221739..f1f610c1 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt @@ -189,7 +189,9 @@ class MinstrelForwardingPlayer( return } super.seekTo(mediaItemIndex, positionMs) - remoteState.markUserTransport(SystemClock.elapsedRealtime()) + remoteState.beginPendingTransport( + SystemClock.elapsedRealtime() + PENDING_TRANSPORT_SAFETY_TIMEOUT_MS, + ) scope.launch { runCatching { active.avTransport.seekToTrack(mediaItemIndex + 1) @@ -215,7 +217,9 @@ class MinstrelForwardingPlayer( // then delegate to Sonos Next. PollLoop reconciles cursor via // Track index if they diverge. super.seekToNextMediaItem() - remoteState.markUserTransport(SystemClock.elapsedRealtime()) + remoteState.beginPendingTransport( + SystemClock.elapsedRealtime() + PENDING_TRANSPORT_SAFETY_TIMEOUT_MS, + ) scope.launch { runCatching { active.avTransport.next() } .onFailure { handleSoapFailure(active, it) } @@ -236,7 +240,9 @@ class MinstrelForwardingPlayer( // Super first for immediate local cursor advance (UI feedback); // then delegate to Sonos Previous. PollLoop reconciles. super.seekToPreviousMediaItem() - remoteState.markUserTransport(SystemClock.elapsedRealtime()) + remoteState.beginPendingTransport( + SystemClock.elapsedRealtime() + PENDING_TRANSPORT_SAFETY_TIMEOUT_MS, + ) scope.launch { runCatching { active.avTransport.previous() } .onFailure { handleSoapFailure(active, it) } @@ -389,5 +395,10 @@ class MinstrelForwardingPlayer( const val POLL_INTERVAL_MS = 1_000L const val NON_PLAYING_CONFIRM = 2 const val SEEK_ACK_WINDOW_MS = 2_000L + // Safety upper bound on how long the polling tick will wait for + // Sonos to ack a user transport. The common case clears event-driven + // when Sonos's reported Track matches the wrapped player; this only + // kicks in if SOAP fails or Sonos drops the ack entirely. + const val PENDING_TRANSPORT_SAFETY_TIMEOUT_MS = 5_000L } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index 4adc375a..fd0251aa 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -513,6 +513,20 @@ class PlayerController @Inject constructor( */ private fun tickPositionPoll(controller: MediaController) { val upnpActive = activeUpnpHolder.active.value != null + // Pending-transport handling. Event-driven primary: as soon as Sonos's + // reported Track matches the wrapped player's currentMediaItemIndex, + // the user's transport press has been ack'd and pending clears. + // Safety fallback: if the deadline expires without an ack, clear + // anyway so we don't ignore Sonos's actual state forever. + if (upnpActive && remoteState.pendingTransportDeadlineMs > 0L) { + val sonosIdx = (remoteState.trackNumber - 1).coerceAtLeast(0) + val timedOut = SystemClock.elapsedRealtime() > remoteState.pendingTransportDeadlineMs + if (sonosIdx == controller.currentMediaItemIndex || timedOut) { + remoteState.clearPendingTransport() + } + } + val pendingTransport = upnpActive && remoteState.pendingTransportDeadlineMs > 0L + val effectiveIsPlaying = if (upnpActive) remoteState.isPlaying else controller.isPlaying val effectivePosition = if (upnpActive) { @@ -525,17 +539,10 @@ class PlayerController @Inject constructor( val newPos = effectivePosition.coerceAtLeast(0) val newDur = effectiveDuration(upnpActive, remoteState.durationMs, controller.duration) val newBuf = controller.bufferedPosition.coerceAtLeast(0) - // Two guards against undoing the user's transport action while - // remoteState is mid-refresh from the next SOAP poll: - // 1. Forward-only: a backward move from desiredIdx < queueIndex - // would undo a Next press (logcat 2026-06-04). - // 2. Transport-ack lockout: even forward moves are ignored within - // TRANSPORT_ACK_WINDOW_MS of a user transport action, so a Prev - // press isn't undone by a stale remoteState reading the older - // pre-press track. - val sinceTransport = SystemClock.elapsedRealtime() - remoteState.lastUserTransportAtMs - val inTransportAckWindow = sinceTransport < TRANSPORT_ACK_WINDOW_MS - val trackChanged = !inTransportAckWindow && + // Track adjustments are forward-only AND suppressed while a user + // transport press is pending Sonos confirmation. Together those keep + // either direction of user input from being undone by a stale poll. + val trackChanged = !pendingTransport && desiredIdx > current.queueIndex && desiredIdx in queueRefs.indices val somethingChanged = trackChanged || @@ -675,7 +682,6 @@ class PlayerController @Inject constructor( const val MINSTREL_SOURCE_KEY: String = "minstrel_source" private const val MS_PER_SECOND = 1_000L private const val MAX_INTERPOLATION_DRIFT_MS = 5_000L - private const val TRANSPORT_ACK_WINDOW_MS = 2_000L } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt index 1e44947c..0ffde318 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/RemotePlayerState.kt @@ -27,16 +27,24 @@ class RemotePlayerState @Inject constructor() { @Volatile var lastError: Throwable? = null; private set @Volatile var trackNumber: Int = 0; private set - // SystemClock.elapsedRealtime() at the last user-initiated transport - // (next/prev/seekToIndex). PlayerController.tickPositionPoll uses this to - // suppress track-change adjustments during the window where remoteState - // hasn't yet refreshed from the next SOAP poll -- without it the polling - // tick would walk the cursor back to Sonos's pre-press track and undo - // user input. Set from MinstrelForwardingPlayer's transport overrides. - @Volatile var lastUserTransportAtMs: Long = 0L; private set + // Pending-transport deadline (SystemClock.elapsedRealtime() at which we + // give up waiting). When > 0, a user transport action (next/prev/seekTo + // idx) is in flight: ForwardingPlayer has already moved the wrapped + // player's index, but Sonos's reported Track hasn't refreshed via a + // SOAP poll yet. PlayerController.tickPositionPoll skips track + // adjustments while pending is non-zero. Pending clears when: + // (a) [event-driven, primary] a poll lands and Sonos's reported Track + // matches the wrapped player's currentMediaItemIndex; or + // (b) [safety fallback] the deadline expires (covers SOAP-fail cases + // where Sonos never acks). + @Volatile var pendingTransportDeadlineMs: Long = 0L; private set - fun markUserTransport(nowMs: Long) { - lastUserTransportAtMs = nowMs + fun beginPendingTransport(deadlineMs: Long) { + pendingTransportDeadlineMs = deadlineMs + } + + fun clearPendingTransport() { + pendingTransportDeadlineMs = 0L } @Volatile private var consecutivePollFailures: Int = 0 @@ -76,7 +84,7 @@ class RemotePlayerState @Inject constructor() { lastError = null consecutivePollFailures = 0 trackNumber = 0 - lastUserTransportAtMs = 0L + pendingTransportDeadlineMs = 0L } private companion object {