Wear playlist transfer — PR 1+2: contrato compartido + Room de playlists locales - #8
Open
PonceGL wants to merge 49 commits into
Open
Wear playlist transfer — PR 1+2: contrato compartido + Room de playlists locales#8PonceGL wants to merge 49 commits into
PonceGL wants to merge 49 commits into
Conversation
The module had no test source set at all. Mirrors :app's JUnit 5 + Truth setup so the DTOs gain round-trip tests before other modules start depending on their shape.
WearPlaylistSync carries a phone playlist's membership and order to the watch, sent once up front so the watch can show and start playing the list before every song has finished transferring. Adds the /playlist_sync Data Layer path it travels on.
…ress The playlist batch coordinator needs to report a transcode phase before streaming starts, and to hand callers a machine-readable failure reason instead of a free-text string. Both fields are additive with safe defaults, so older payloads still decode.
The module had no test source set at all. JUnit 5 + MockK + Turbine + Truth for unit tests, mirroring :app's stack; Room in-memory testing + AndroidJUnitRunner for instrumentation, needed for the upcoming local-playlist DAO. No legacy JUnit 4 tests are planned here, so the vintage engine isn't pulled in. MainCoroutineExtension mirrors :app's copy — dispatchers aren't injected anywhere in this repo yet, so tests swap the main dispatcher globally instead.
LocalPlaylistEntity + LocalPlaylistSongCrossRef store a phone playlist's identity, membership and order. No foreign key from the cross-ref to local_songs on purpose: a playlist syncs its full order before every song's audio has finished transferring, so a cross-ref routinely points at a songId that doesn't have a local row yet. Not wired into WearMusicDatabase yet — that's the next commit, since it also needs a schema bump and a migration.
…igration chain Adds MIGRATION_5_6 (creates local_playlists and local_playlist_songs, plus their indices) and registers local playlist tables/DAO on WearMusicDatabase. Also fixes a preexisting bug while touching the same builder call: MIGRATION_1_2 through MIGRATION_4_5 were declared but never passed to .addMigrations(), so any watch still on an old schema would have crashed on the next open instead of migrating. Wiring my own migration through the same line means wiring all of them costs nothing extra and actually fixes the app for those devices.
…>v6 migration LocalPlaylistDaoTest covers upsert-replaces-not-merges, per-playlist song ordering, and that a re-sync of one playlist leaves others alone. WearMusicDatabaseMigrationTest hand-builds a v5 SQLite file (:wear has exportSchema = false, so MigrationTestHelper's schema fixtures aren't available here) and verifies MIGRATION_5_6 both creates the new tables and leaves existing local_songs rows untouched. Both need androidTestImplementation(kotlinx-coroutines-test) for runTest, which the module's unit test config didn't cover. Not yet run on a device or emulator — none is connected this session. Verified so far: compiles clean (:wear:compileDebugAndroidTestKotlin), including KSP/Hilt. Actual execution is deferred to the dedicated hardware-verification pass.
PonceGL
changed the title
Nothing in this codebase injects CoroutineDispatchers today — repo-wide convention is Dispatchers.IO/.Main referenced directly, which fails AND-CONC-03 and forces tests to depend on the real dispatcher. Rather than migrate existing code (out of scope, high blast radius), these qualifiers are for new code only: WatchAudioTranscoder is the first consumer, needing an injected Main dispatcher so its Transformer setup (which requires a Looper thread) is swappable in tests.
…h handoff WatchAudioTranscoder decides whether a song needs re-encoding before it goes to the watch, and performs that encode with media3-transformer (already a declared dependency, previously unused anywhere in the codebase). Lossless/high-bitrate sources are transcoded to AAC-LC 128kbps — watches decode AAC in hardware, and 128kbps keeps the Bluetooth transfer time down until a Wi-Fi transport exists (a later phase). Already-lossy sources at or under 256kbps pass through untouched: re-encoding an already-small MP3 only costs CPU and quality for no transfer-time benefit. requiresTranscoding is a pure function, split out from the actual encode specifically so it stays cheap to unit test — the encode path itself needs a real hardware encoder and a Looper thread, so it can only be verified on a device.
Pure size/time heuristics for the send-to-watch confirmation sheet, computed only over songs that aren't already on the watch. Sizes a song by its transcoded target bitrate when it'll be re-encoded, or by its own bitrate when it'll pass through untouched. The Bluetooth throughput assumption is deliberately conservative and needs re-measuring against a real phone+watch pair once device testing resumes — an estimate that undershoots erodes trust in the sheet more than one that's a bit pessimistic.
…timator WatchAudioTranscoderTest covers requiresTranscoding's full branch matrix (lossless, passthrough bitrate boundary, unknown mime/bitrate, case sensitivity) — 7/7 passing. WatchPlaylistTransferEstimatorTest covers passthrough vs transcoded sizing, that only pending songs are summed, and the zero/near-zero edge cases — 5/5 passing. Verified: :app:testDebugUnitTest, full suite, 426 tests, only the same 5 pre-existing failures unrelated to this branch (confirmed by running the identical suite against a clean dev-personal worktree: 414 tests, same 5 failures, same names).
…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.
Found on real hardware: the watchdog firing only flipped bookkeeping state (activeTransfers -> FAILED, pendingMetadata cleared) while the coroutine actually reading the ChannelClient InputStream kept running, completely unaware anything had happened. Two ways that went wrong in one 6-song transfer over Bluetooth with a connected BT headset: - A song reported 'timed out' but the read loop kept going anyway and finished successfully seconds later — a false alarm, but the watchdog re-armed on every remaining read, so the same requestId could fire repeatedly (observed 2, 3, and 4 times on one transfer). - Worse: firing cleared pendingMetadata out from under the still-live loop, so when it finally tried to resolve metadata to finish writing the file, it found nothing — 'Transfer metadata missing', the file got deleted, and the song was lost for real despite every byte having arrived over the wire. Root cause: no link between the watchdog and the actual I/O. Now armTransferWatchdog closes the live InputStream (tracked per requestId in openAudioStreams) when it fires, which unblocks the loop's read() with an IOException and routes it through onAudioChannelOpened's own catch block for one consistent cleanup path — instead of the watchdog declaring failure independently. watchdogTimedOutRequestIds lets that catch block report 'Transfer timed out' instead of a generic stream-closed message. Not unit-testable as-is: the watchdog is a real delay() with no injected clock, and onAudioChannelOpened is private. Verified by code review against the exact failure sequence from hardware; needs re-verification on-device (same scenario: BT headphones connected, transfer running) before calling it confirmed fixed.
Found on real hardware: once a song's transfer failed, its playlist kept showing the 'Receiving…' badge forever. playlistIdsReceiving treated mere presence in WearTransferRepository.activeTransfers as 'in progress' — but a failed entry deliberately stays in that map (DownloadsScreen lists it under 'Transfer issues'), it's just no longer active. Now only STATUS_TRANSFERRING counts.
The plan's own risk table already called for this ('reintento con
backoff') but it was never implemented. Real hardware testing showed
why it matters: 2 of 6 songs in one batch failed outright, both
consistent with a Bluetooth stall from the watch's radio being shared
with a connected BT headset — a real, non-theoretical condition, not
a broken link.
transferSongToAllNodesWithRetry re-attempts once, after a short fixed
backoff, before the coordinator gives up on a song. Re-transcodes on
the retry rather than caching the first attempt's output — simpler,
and cheap enough on a modern phone's hardware encoder to not be worth
the extra bookkeeping.
…ility fix(wear): confiabilidad de la transferencia de playlists — hallazgos de la primera prueba en hardware
A playlist synced to the watch used to show a song's raw internal id in the pending-songs list — the only thing it knew about a song it hadn't received yet. WearPlaylistSync now carries songTitles, a parallel list to songIds (kept separate rather than restructuring songIds itself, so an older watch or phone build still decodes the payload cleanly via default values / ignoreUnknownKeys). The phone resolves titles once, up front, alongside the existing per-song library lookup used for transcoding — no extra querying. The watch stores each cross-ref's pendingTitle (Room v6->v7, MIGRATION_6_7 adds the column, defaulting existing rows to ''). WearLocalPlaylistSongItem.displayTitle picks the real title once the song has actually arrived, falling back to the phone-provided pending title, and only the raw id as a last resort for a sync from a phone build old enough to not send one.
The banner PlaylistDetailScreen shows while a playlist batch is sending to the watch looked bolted onto the screen rather than part of it: 20dp horizontal padding against the 16dp every other row in that same options list (PlaylistActionItem, right below it) actually uses, a plain LinearProgressIndicator, and a full-width text-only cancel button laid out awkwardly next to it. Now it matches PlaylistActionItem's own treatment — the same 40dp icon badge, 16dp padding, 18dp corner radius — and uses LinearWavyProgressIndicator, matching WatchPlaylistBatchProgressDialog (LibraryScreen's own send-to-watch progress modal) instead of a plain bar. The cancel action is now an icon-only IconButton instead of a separate labeled TextButton competing for space in the same row. Also adds the currently-transferring song's name to both this banner and WatchPlaylistBatchProgressDialog — until now the phone's own system notification showed more detail than the in-app UI did.
Starting a song from deep inside Downloads -> Playlists -> a playlist, or from Downloads' own song menu, left the user several swipes-back away from the transport controls they just asked for. Both screens now take an onPlaybackStarted callback, fired right after the local (or phone) playback call; WearNavigation wires it to navigate to PLAYER with the intermediate stack popped, so a swipe-back from Player doesn't retrace the whole browse path either.
…ture 25 strings introduced across PR1-7 and the two commits above only existed in the default (English) resources — values-es never got updated alongside them. Backfills all of them, matching each file's existing key order and tone.
feat: pulido de UI/UX de la transferencia de playlists al reloj
…ch's RAM Two zero-risk fixes identified from on-device evidence during the watch-transfer hardware test cycle (choppy local playback, both with and without a concurrent transfer): - WearPlaybackService built its ExoPlayer with the default MediaSourceFactory, which registers ~15 extractor types (Matroska, FLV, AVI, MPEG-TS…). ART verifies each extractor class the first time DefaultExtractorsFactory touches it while sniffing the container — on the main thread. Measured on-device this cost 120-300ms per unused class, ~2s total, stacked right on top of playback start (visible as `Choreographer: Skipped N frames` right after pressing play). Every file this service plays comes from WatchAudioTranscoder on the phone, which always writes plain (non-fragmented) MP4/AAC-LC, so the extractor is now scoped to just that. - ExoPlayer's default DefaultLoadControl buffer window isn't sized for a Wear OS SoC. Measured on-device: sustained PLAYING/BUFFERING oscillation every 0.5-1s during local playback. Mirrors the phone's DualPlayerEngine.buildAdaptiveLoadControl() RAM tiering (same ActivityManager.isLowRamDevice signal, same reasoning, amplified — a watch has far less RAM/CPU headroom than even a low-end phone, and shares both with whatever fitness app is running alongside). Neither fix addresses the dominant cause (no audio offload — decode/render still competes with Compose on the main thread) or the confirmed risk of the whole process being recycled under system-wide memory pressure while another app runs concurrently; both are follow-up work.
…t fallback The dominant cause of the choppy local-playback experience confirmed on-device this session: WearPlaybackService's ExoPlayer decodes and renders audio on the same CPU/main-thread the Compose player screen recomposes on. Screen off/AOD (far less Compose work) measured noticeably smoother than screen-on, pointing squarely at CPU contention rather than the Bluetooth radio. Requests AUDIO_OFFLOAD_MODE_ENABLED (soft — if the watch's audio HAL doesn't support offloading AAC-LC, ExoPlayer silently falls back to normal decode on its own, no capability probing needed here for that path). What ExoPlayer does *not* self-heal is a HAL that accepts the offloaded track and then stalls/resets shortly after — the exact failure mode that already forced the phone's DualPlayerEngine to build a runtime fallback (shouldDisableAudioOffloadOnEarlyBuffering / AudioOffloadPolicyTest in :app). There's no comparable per-chipset denylist for Wear OS yet — no field evidence of which watch chipsets misbehave — so this mirrors the phone's *runtime* safety net (wearShouldFallBackFromAudioOffload, simplified for this service's single-player model, no crossfade state to account for) instead of guessing at one. On a detected reset it rebuilds the player with offload off and swaps it into the existing MediaSession via setPlayer(), preserving playlist/position/ playWhenReady and keeping the phone's remote-control MediaController connected across the swap. Intentionally duplicated rather than extracted to :shared: DualPlayerEngine is already flagged as a God Object (pre-existing debt, out of scope here), and the two services differ enough (single player vs. dual-player crossfade) that sharing would mean threading wear-only parameters through phone-only code for a ~30-line function. 6 new tests for the pure fallback decision (WearAudioOffloadPolicyTest, mirrors AudioOffloadPolicyTest's early-buffering cases). The player-swap wiring itself (Player.Listener → MediaSession.setPlayer) is not unit-testable without instrumentation — needs on-device confirmation via `dumpsys media.audio_flinger | grep -A20 -i offload` to see whether offload actually engages on this watch's hardware, and whether the fallback fires correctly if it doesn't behave.
…ayback
Two things flagged by the original plan's audit of this module, acted on now:
1. `collectAsState()` instead of `collectAsStateWithLifecycle()`, across all of
`:wear` (11 files, ~45 call sites) — every single StateFlow collection in
the module was lifecycle-unaware before this commit. `collectAsState` keeps
collecting with the screen off/backgrounded; the watch's own screen-off
rule ("the UI should cost zero") is incompatible with that, and it's
exactly what a battery-constrained device with a running app fighting for
the same CPU cannot afford. PR15 already fixed 3 call sites in the two
local-playlist screens; this closes the rest of the module in one pass
since the fix is identical and mechanical everywhere.
2. `rememberLivePositionMs` (PlayerScreen) ticked every 250ms purely off
`state.isPlaying`, with no regard for whether the screen was actually
interactive — despite `WearLifecycleState`'s own doc comment explicitly
naming this as the pattern it exists to prevent: "position-update jobs...
should gate on isInteractive so they pause as soon as the activity moves
to the background or the watch enters ambient mode." Confirmed on-device
this session: local playback measured noticeably more stutter with the
screen on (more Compose work) than in ambient/off (WearLifecycleState
already reports ambient correctly), and ambient still wasn't perfectly
smooth — this loop, un-gated, is a plausible contributor to that residual
stutter. Gated it, and applied the identical fix to
`rememberActiveLyricLineIndex`, which sits one page away in the pager
(`beyondViewportPageCount = 1`) and can keep ticking while the user is
looking at the main player page instead.
`livePositionFromAnchor` always computes from elapsed real time, so
freezing the display between ticks while not interactive loses nothing —
the position snaps to the correct value the moment interactivity returns.
Left untouched (documented, not silently ignored): `CenterPlayButton`'s
continuous 13.8s-rotation animation on the play button is already gated on
`WearLifecycleState.isInteractive` via its LaunchedEffect, and is a
deliberate decorative choice, not a bug — redesigning it wasn't asked for and
isn't done here. Its ring-drawing Canvas already computes on the draw phase;
the meaningful cost while playing is the animation itself, which offload
(the previous PR in this stack) addresses more directly than a recomposition
fix could, by taking audio decode off this same CPU/thread entirely.
`./gradlew :wear:compileDebugKotlin :wear:testDebugUnitTest` — clean build,
no warnings, 29 tests in verde. Mechanical/low-risk except the two
produceState gating changes, which are behavior changes confirmable only on
a real watch (does the position display correctly resume on waking from
ambient / foregrounding?).
Confirmed on-device this session, not theoretical: the app's process was recycled during a system-wide low-memory episode while a fitness-tracking app ran alongside local playback (PID changed across ~90s in which 23 other system processes were also killed for memory). WearPlaybackService being a foreground MediaSessionService makes that less likely, not impossible — Wear OS watches have very little RAM. Before this commit, that kind of process death silently dropped the queue and position with no way back except manually re-navigating to the playlist and hitting play again. Adds WearPlaybackStatePersistence (DataStore<Preferences>-backed, mirrors :app's PlaylistBatchTransferPersistence — same single-slot, JSON-via-kotlinx.serialization shape, separate DataStore file since :wear and :app are different processes with no shared storage). WearLocalPlayerRepository saves (queueSongIds, currentIndex, positionMs) on the events that matter (play/pause, track change) plus a coarse 10s tick while playing — not on every 1s UI tick, which would be pure DataStore-write battery cost for no benefit — and clears it on a deliberate stop. On a fresh WearPlayerViewModel (i.e. a fresh process), restorePersistedPlaybackIfAvailable() resolves the persisted song ids against the watch's current local library (songs may have been deleted since the snapshot), restores what's still there paused — not auto-playing, since starting audio without a fresh user gesture on app open would be surprising — and, if it actually restored something, flips WearStateRepository's outputTarget to WATCH. That target flip matters: outputTarget isn't itself persisted (always starts at PHONE), so without it the restored queue would sit in WearLocalPlayerRepository correctly but never surface in the unified playerState the Player screen reads. A successful restore is exactly the signal that the user was on watch-local playback when the process died. isPersistedLocalPlaybackStateRestorable additionally requires the snapshot to be no older than 6 hours and to have a valid queue/index — a conservative, undedicated-hardware-calibrated window (no on-device data exists to tune it precisely), picked to cover realistic crash-recovery gaps without silently resurrecting a queue from days ago the next time the app happens to open. 12 new tests (WearPlaybackStatePersistenceTest, mirrors PlaylistBatchTransferPersistenceTest's DataStore-round-trip style; WearPlaybackStateRestorabilityTest for the pure freshness/validity check). The actual restore-on-boot wiring and the "does the notification/foreground state look right" behavior are not unit-testable — need on-device confirmation, ideally by reproducing a real process kill (not just a normal app close) during active local playback. `./gradlew :wear:compileDebugKotlin :wear:testDebugUnitTest` — clean build, 41 tests in verde.
…M del reloj (#17) perf(wear): PR1+PR2 — extractor MP4-only y buffering ajustado a la RAM del reloj
feat(wear): PR3 — audio offload con fallback ante reset del HAL
…de vida (#19) perf(wear): PR4 — auditoría de recomposición sin conciencia de ciclo de vida
…erte del proceso (#20) feat(wear): PR5 — persistir y recuperar la reproducción local tras muerte del proceso
Real hardware testing (transferring a 2nd playlist while local playback kept running on the watch) showed the watch ending up with every song's audio on disk but no playlist row to show them under: the playlist_sync message the phone sends before any song audio has no retry and no delivery confirmation, so a watch that's mid Wi-Fi/ADB reconnect right when it's sent loses it silently — confirmed by pulling wear_music.db off the device and finding the songs in local_songs with zero rows in local_playlist_songs for that playlist. WearPlaylistSync now carries a requestId; the watch acks back (WearPlaylistSyncAck, new /playlist_sync_ack path) once the sync is durably applied. PlaylistWatchTransferCoordinator waits for that ack per node and retries once (same shape as the existing per-song retry) before giving up — giving up doesn't fail the batch, songs still transfer, and the next explicit re-sync is idempotent and gets another chance. Both DTO fields default for backward compatibility, same pattern as the existing songTitles field.
Real hardware testing on a Samsung Galaxy Watch (release build) showed songs stopping partway through and never resuming: Play did nothing (playWhenReady was already true, so there was nothing to resume) while Next still worked (it rebuilds the sink for the new item). Confirmed that's not a process death — the UI stayed responsive and Next kept working — so the existing audio-offload safety net from PR18 (WearAudioOffloadPolicy) doesn't cover this: it only reacts to a stall within ~500ms of playback starting, surfaced as STATE_BUFFERING. This one happens well after that window and the player's reported state never changes, so there's no listener callback to hook. Added a position-polling watchdog (WearPlaybackStallWatchdog) that ticks once a second and compares the player's own reported position against the previous tick: 3 consecutive ticks with zero movement while isPlaying=true is read as a wedge and triggers the same player-rebuild recovery PR18 already has (extracted the shared "preserve queue/ position/play-state, build a fresh ExoPlayer, swap via MediaSession.setPlayer" logic out of fallBackFromAudioOffload so both paths use it). Unlike the early check, this doesn't require offload to still be enabled — the rebuild is the actual fix regardless of cause. Verified :wear release (assembleRelease, R8 + shrinkResources) still builds clean with the new coroutine-based watchdog.
…ease
Real hardware testing with release builds on both sides showed the
phone's "send playlist to watch" dialog always reporting "no watch
connected" and the button disabled — even right after confirming the
watch was reachable (phone->watch remote-control mode worked fine).
Waiting, toggling Bluetooth, and reinstalling didn't help, which ruled
out a capability-sync timing issue.
Root cause, confirmed via wear/build/outputs/mapping/release/resources.txt:
array:android_wear_capabilities:... is not reachable.
isShrinkResources=true (release only) strips
res/values/wear_capabilities.xml's <string-array
name="android_wear_capabilities"> because nothing in the app's own code
ever references it by R.array id — it's discovered purely by resource
NAME convention by Play Services, to advertise this app's
"pixelplay_wear_app" capability (WearCapabilities.PIXELPLAY_WEAR_APP).
The shrinker's reachability analysis has no way to know that, so it
correctly-by-its-own-rules but wrongly-for-us marks it unused and
removes it — meaning the release watch app never advertised the
capability the phone's CapabilityClient.getCapability(FILTER_REACHABLE)
call looks for at all. Remote-control mode kept working because that
uses DataItem/message sync to connected nodes directly, unrelated to
capability advertisement.
Added res/raw/keep.xml with tools:keep, the standard AGP resource-
shrinker escape hatch for resources reached only by reflection/naming
convention rather than code reference. Verified against the rebuilt
release mapping report: the same line now reads "reachable from keep
xml file".
fix(wear): retry + ack playlist sync so it can't be silently lost
…22) fix(wear): recover from mid-song offload stalls, not just early ones
…ease (#23) fix(wear): keep the Wear capability resource from being shrunk in release
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.
Primeros dos PRs del plan de transferencia de playlists al reloj, combinados en este PR tras un ajuste de alcance a mitad de trabajo (ver historial de comentarios). Sin lógica de negocio de transferencia todavía — solo contrato compartido e infraestructura de datos local en el reloj.
PR 1 — Contrato compartido + infra de test
WearPlaylistSync(nuevo DTO en:shared): snapshot de playlist (id, nombre, orden de canciones) que el teléfono envía al reloj antes que los audios.WearDataPaths.PLAYLIST_SYNC: ruta Data Layer nueva para ese mensaje.WearTransferProgress: añadeSTATUS_TRANSCODING,STATUS_AWAITING_WATCH_ACKyerrorCode(defaultnull, compatible con payloads antiguos).:sharedy:wear— ninguno tenía source set de test. JUnit 5 + Truth en ambos; MockK + Turbine + Room-testing en:wear.PR 2 — Room de playlists locales en el reloj + migración 5→6
LocalPlaylistEntity+LocalPlaylistSongCrossRef: identidad, membresía y orden de una playlist sincronizada. Sin FK hacialocal_songsa propósito — la playlist sincroniza su orden completo antes de que termine de llegar el audio de cada canción.LocalPlaylistDao:upsertPlaylisttransaccional (reemplaza membresía completa, no la fusiona), consultas ordenadas por posición.WearMusicDatabase: v5→v6, con índices en(playlistId, position)ysongId.Room.databaseBuilder(...)— cualquier reloj actualizando desde un esquema antiguo habría crasheado. Tocaba la misma línea para registrar la mía, así que registré las 5.LocalPlaylistDaoTest(upsert reemplaza no fusiona, orden por posición, aislamiento entre playlists) yWearMusicDatabaseMigrationTest(v5→v6 sobre un archivo SQLite construido a mano, ya que:wearno exporta esquema Room).Riesgo conocido, sin verificar
WearMusicDatabaseMigrationTestconstruye elCREATE TABLE local_songsde v5 a mano porque:weartieneexportSchema = false. Si esa DDL no coincide exactamente (tipos, nulabilidad) con lo que Room generaría deLocalSongEntity, la validación de esquema post-migración de Room podría fallar aunque la migración en sí sea correcta. No puedo descartarlo sin ejecutar en dispositivo — es el primer punto a verificar en la sesión de hardware.Verificado
:shared:testDebugUnitTest→ 7/7 en verde.:wear:compileDebugKotlin,:wear:compileDebugAndroidTestKotlin,:wear:assembleDebug→ compilan y ensamblan limpio (KSP/Hilt incluido).:app:compileDebugKotlin→ sin regresiones.:wear:testDebugUnitTest→ sin tests unitarios propios todavía (los tests de Room son instrumentados, no hay Robolectric en el repo); esperado.Ejecutado en local (JBR de Android Studio como JDK 21); ningún workflow de CI corre tests unitarios hoy.