Wear playlist transfer — PR 4: coordinador de lote + notificación de progreso - #10
Merged
PonceGL merged 15 commits intoAug 9, 2026
Conversation
…transcoded file Adds a WatchAudioOverride hook to startTransferToWatch/performTransfer: when present, it substitutes the song's own file resolution and eligibility check entirely, streaming the given file with its own mimeType/bitrate reported in the transfer metadata instead of the original song's. The override file was just written locally by WatchAudioTranscoder, so it's unconditionally eligible regardless of where the source song actually lives (local file, cloud proxy, ...). No behavior change for existing callers — audioOverride defaults to null.
The rest of the wear/ package resolves these via Wearable.getXClient(application) internally, which is fine for production but can't be faked in a JVM unit test — mocking Wearable's static factory methods needs MockK's inline-mocking agent, and its dynamic self-attach hangs indefinitely in this environment's sandboxing (confirmed: the worker process sat at 0% CPU for 5+ minutes). CapabilityClient/MessageClient are non-final abstract classes, so injecting them lets a test construct a coordinator with mocked instances directly, with no agent involved. Only used by PlaylistWatchTransferCoordinator so far; the rest of the package is unchanged.
PhoneWatchBatchTransferState is the aggregate (song counts, current song, overall status) driven by PlaylistWatchTransferCoordinator; PhoneWatchTransferState (existing) keeps tracking the active song's byte-level progress under its own requestId. Same StateFlow-per-map shape and terminal-cleanup pattern as the existing per-song transfers, just keyed by batchId instead of requestId.
Sends a whole playlist to the watch: syncs membership/order first (so the watch can show and start playing it before every song has arrived), then transcodes and transfers pending songs one at a time — never in parallel, to avoid saturating the single Bluetooth channel. Reuses the existing single-song pipeline end to end via PhoneDirectWatchTransferCoordinator's WatchAudioOverride hook. A song missing from the library or that never reaches a terminal state within the timeout is counted as failed rather than silently skipped, so completed+failed always accounts for every song in the batch. Deviates from the usual CoroutineScope(SupervisorJob() + Dispatchers.X) pattern seen elsewhere in this package: takes the injected @AppScope scope instead of constructing its own, so the scope has an owner (GEN-CONC-01). The song-transfer-await timeout is a settable instance property, not a companion var, so tests can shrink it on their own instance without mutating shared state other tests could see.
An active or just-finished batch takes priority over any concurrent lone single-song transfer (e.g. from the song info sheet) in the notification — it's the longer-running, more significant operation, and showing both at once would make a single notification unreadable. Content text reads "N of M songs" rather than a byte count, matching the confirmation-sheet-to-notification UX: the user cares about song progress, not bytes, for a playlist send. The service now stays foreground as long as either transfers or batchTransfers is non-empty, not just transfers.
…ne transfer state store PlaylistWatchTransferCoordinatorTest (9 cases): empty playlist, no reachable watch, playlist order preserved, dedupe of songs already on every reachable watch, one song failing doesn't abort the batch, mid-batch cancellation, timeout on a song that never confirms, fan-out to multiple reachable nodes counted once per song, a missing song counted as failed rather than dropped. PhoneWatchTransferStateStoreTest (20 cases): covers both the batch state this PR adds and the pre-existing per-song transfer state, which had no test coverage at all before this. Doesn't assert on the store's terminal-state cleanup — it runs on an internal, non-injectable Dispatchers.Default scope after a real-time delay, so testing it here would mean either a real sleep (GEN-TEST-04) or refactoring the store's scope handling, out of scope for this change. capabilityClient/messageClient are constructor-injected into the coordinator specifically so they can be faked directly in the test without mocking Wearable's static factory methods, which needs an agent that hangs in this environment. Verified: :app:testDebugUnitTest, full suite, 455 tests. Only the same 5 pre-existing failures unrelated to this branch (confirmed against a clean dev-personal worktree earlier in this feature). The 29 new tests in this PR (9 + 20) are all green.
estimateWatchTransfer, isPlaylistFullyOnWatch, sendPlaylistToWatch, cancelPlaylistTransfer, and activePlaylistBatchTransfer — thin delegation to PlaylistWatchTransferCoordinator/PhoneWatchTransferStateStore/ WearPhoneTransferSender, mirroring the exact pattern SongInfoBottomSheetViewModel already uses for the single-song case. Adds 4 constructor dependencies to an already-1200-line ViewModel. Flagged, not fixed here — splitting it is a separate, unrelated refactor.
…aylist screen New action in the playlist options sheet — labeled "Send to Watch" or "Update on Watch" depending on whether any of its songs are already there. Tapping it refreshes watch availability and opens a confirmation dialog showing pending-song count and the size/time estimate (WatchPlaylistTransferEstimator, already built) before anything is sent. A non-blocking progress banner appears at the top of the songs list once a batch is running for this playlist, with a cancel action — the user can navigate away or leave the app while it continues; the foreground notification (already built) is what tracks it from there.
A playlist batch takes priority over a concurrent lone single-song transfer in the top bar badge and compact-navigation pill — same priority rule as the transfer notification (WatchTransferForegroundService) and the playlist screen's own banner: it's the longer-running, more significant operation, and showing both at once would be unreadable. WatchPlaylistBatchProgressDialog mirrors the existing single-song WatchTransferProgressDialog's look (loading ring + percent, wavy progress bar, cancel button) rather than reusing PlaylistDetailScreen's banner — LibraryScreen already establishes badge-tap-opens-dialog as its own convention for this, and a lone playlist name/song-count doesn't need the full list context a banner implies.
Covers estimateWatchTransfer, isPlaylistFullyOnWatch (empty list,
partial, and fully-on-watch cases), sendPlaylistToWatch,
cancelPlaylistTransfer, activePlaylistBatchTransfer, and
refreshWatchAvailability. The rest of PlaylistViewModel's existing
surface (CRUD, sorting, AI generation, M3U import/export) is untouched
and out of scope — no PlaylistViewModelTest existed before this.
activePlaylistBatchTransfer is a stateIn(WhileSubscribed) flow —
reading .value directly never triggers the upstream collection, so
that test uses Turbine's test{} for a real subscriber instead.
Verified: :app:testDebugUnitTest, full suite, 462 tests. Only the same
5 pre-existing failures unrelated to this branch. The 7 new tests in
this PR are green.
WearDataListenerService now routes the PLAYLIST_SYNC message path to WearTransferRepository.onPlaylistSyncReceived, which upserts the playlist entity and its song cross-refs (order preserved via position) into LocalPlaylistDao in one transaction. Re-syncing an existing playlistId (e.g. after editing it on the phone) replaces membership/order rather than merging with stale cross-refs, and preserves the original createdAt while bumping updatedAt — the DAO's upsertPlaylist already had this transactional behavior from PR2, this just starts calling it. The manifest's MESSAGE_RECEIVED intent filter gets a matching <data> entry for /playlist_sync, mirroring the existing entries for the other message paths. Unlike the two pre-existing branches in the same when-block (TRANSFER_METADATA, FAVORITES_SYNC_STATE), this new branch's catch re-throws CancellationException instead of swallowing it — left the other two alone since fixing them is out of scope here.
Backs the upcoming local-playlists screens. Resolves each playlist song's availability reactively by joining its cross-ref order against LocalSongDao.getAllSongs(), so a song that finishes transferring while the detail screen is open flips from pending to playable without the user backing out and re-entering. playlistIdsReceiving surfaces which playlists currently have an in-flight song transfer, for a receiving indicator on the list screen. playAll/playFrom skip songs still pending transfer.
LocalPlaylistsScreen lists playlists synced from the phone with a
receiving indicator; LocalPlaylistDetailScreen shows a playlist's
songs in sync order, marking pending ones as disabled with a
'waiting to transfer' label instead of hiding them, so the list's
shape matches the phone immediately even before every song has
arrived. Both use androidx.wear.compose.foundation.lazy's items()
overload with an explicit key (playlist.playlistId / item.songId)
instead of the module's usual count-based items(n){} — needed here
because playlists reorder by updatedAt on every sync and songs flip
availability while the screen is open, both of which lose state and
break animations without a stable key.
DownloadsScreen gets a new 'Playlists' entry navigating into the new
screens. Reachable via Downloads → Playlists → a playlist → its
songs.
Persists at most one in-flight playlist batch transfer intent (batchId, playlistId, playlistName, songIds, requestedAtMillis) to the app's shared DataStore<Preferences>, matching the existing *PreferencesRepository convention. Deliberately doesn't persist the rest of PhoneWatchTransferStateStore (per-song byte progress, reachable nodes, ...) — that's UI-only state, cheap to rebuild, and churns too fast to persist sensibly. Only the intent needs to survive a process restart; the coordinator already re-derives everything else when it runs a batch. clearInFlightBatch(batchId) only removes the stored intent if its batchId still matches — if a newer batch already overwrote it (e.g. the user sent another playlist before the first one's cleanup ran), clearing unconditionally would drop that newer intent instead.
PlaylistWatchTransferCoordinator now persists its batch intent when a transfer starts and clears it on every terminal outcome (completed, failed with no reachable watch, cancelled) — so a batch surviving to the next app start is exactly the ones that were cut off by the process dying mid-transfer, not a theoretical case for a transfer that can run tens of minutes over Bluetooth. resumePersistedBatchIfNeeded() re-runs any such orphaned batch: it refreshes the watch-library snapshot first (empty right after a cold start) and waits briefly for it to resolve, so the existing dedup against what's already on the watch is accurate on the first pass instead of relying solely on the watch's own duplicate rejection. Re-running from scratch is safe either way — the watch rejects a transfer for a song it already has. Wired into PixelPlayApplication.onCreate(), alongside the app's other one-shot startup work. Best-effort: a cold start not directly triggered by the user may be too restricted to start the foreground service this resumes into, so failures here are logged and skipped rather than crashing app startup — the persisted intent stays put for the next launch that can.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apilado sobre #9 (transcodificado + estimador). Cuarto PR del plan: el orquestador que envía una playlist entera al reloj, reutilizando el pipeline de una sola canción existente.
Contenido
PlaylistWatchTransferCoordinator(nuevo): sincroniza membresía/orden de la playlist primero (para que el reloj pueda mostrarla y empezar a reproducir antes de que lleguen todos los audios), luego transcodifica y transfiere las canciones pendientes una a una — nunca en paralelo, para no saturar el único canal Bluetooth. ReutilizaPhoneDirectWatchTransferCoordinatorcompleto vía un hook nuevo.WatchAudioOverrideenPhoneDirectWatchTransferCoordinator: permite sustituir el origen del audio (y su mimeType/bitrate reportado) por un archivo ya transcodificado, sin tocar el manejo de excepciones existente de esa clase (fuera de alcance).PhoneWatchBatchTransferStateenPhoneWatchTransferStateStore: estado agregado del lote (contadores, canción activa, estado global), mismo patrón que el estado por canción ya existente.CapabilityClient/MessageClientinyectados vía Hilt: solo para este coordinador nuevo (el resto del paquete sigue resolviéndolos conWearable.getXClient()internamente) — necesario para poder testear sin mockear un método estático de Java.Dos correcciones sobre el diseño de referencia que audité en la fase de planificación
runCatching { X.await() }.getOrElse { emptyList() }sobre.await()se sustituyó portry/catch(CancellationException) { throw } catch(Exception)— el original se tragaba la cancelación (AND-CONC-04).markBatchSongFailed) en vez de saltarse en silencio — si no,completedSongCount + failedSongCountnunca sumabatotalSongCount.CoroutineScope(SupervisorJob() + Dispatchers.IO)sin dueño) — se inyecta@AppScope.varde companion mutado por los tests — evita fuga de estado entre tests.Un hallazgo del entorno, no del código
Intenté inicialmente testear con
mockkStatic(Wearable::class)(el patrón que usa el resto del paquete). El agente de auto-attach dinámico que MockK necesita para mockear métodos estáticos se cuelga indefinidamente en el sandboxing de este entorno (proceso confirmado a 0% CPU durante 5+ minutos). Por esoCapabilityClient/MessageClientpasaron a inyectarse por constructor en este coordinador: son clases abstractas no-final, así que MockK las subclasifica sin necesitar ese agente. Dejo esto documentado en el propio test por si se repite en otro punto del proyecto.Verificado
:app:testDebugUnitTest, suite completa → 455 tests, solo los mismos 5 fallos preexistentes (confirmados contra un worktree limpio dedev-personalen el PR anterior). Los 29 tests nuevos de este PR (PlaylistWatchTransferCoordinatorTest9/9,PhoneWatchTransferStateStoreTest20/20) están en verde.:app:assembleDebug→ compila y empaqueta limpio.Sin verificar (necesita dispositivo)
El streaming real por
ChannelCliententre teléfono y reloj — necesita dos dispositivos físicos emparejados, que es exactamente lo que estamos posponiendo hasta la sesión de hardware dedicada.Ejecutado en local (JBR de Android Studio como JDK 21).