fix(wear): confiabilidad de la transferencia de playlists — hallazgos de la primera prueba en hardware - #14
Merged
PonceGL merged 3 commits intoAug 10, 2026
Conversation
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.
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
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.
Contexto
Primera corrida real en dispositivo de la Fase 1 completa: playlist de 6 canciones, teléfono S25 Ultra + reloj Galaxy Watch (SM_R920), audífonos Bluetooth conectados. Encontró 3 bugs reales, todos con causa raíz confirmada en el código (no especulación) — este PR los corrige.
Qué corrige
1. El watchdog de transferencia no cancelaba nada de verdad
armTransferWatchdogdisparaba a los 120s de inactividad y solo actualizaba el mapa de estado (activeTransfers→FAILED, limpiabapendingMetadata) — pero la corrutina que de verdad estaba leyendo elInputStreamdelChannelClientseguía corriendo, sin enterarse. En una sola transferencia de 6 canciones esto pasó dos veces, de dos formas distintas:requestIdpodía disparar varias veces (vi 2, 3 y hasta 4 disparos sobre la misma transferencia).pendingMetadatadebajo del loop que seguía vivo — cuando por fin terminó de recibir bytes y quiso resolver su metadata para escribir el archivo, ya no la encontró (Transfer metadata missing), se borró el archivo temporal, y la canción se perdió de verdad a pesar de que todos los bytes habían llegado.Corrección: el watchdog ahora cierra el
InputStreamreal (rastreado porrequestId) cuando dispara, lo que desbloquea elread()con unIOExceptiony lo enruta por el mismocatchque ya maneja la limpieza — un solo camino consistente en vez de dos procesos independientes pisándose.No es testeable en JVM tal cual está: el watchdog es un
delay()real sin reloj inyectado, y la función relevante es privada. Verificado por revisión de código contra la secuencia exacta observada en hardware — necesita reverificarse en dispositivo (mismo escenario: audífonos BT conectados, transferencia corriendo) antes de darlo por confirmado.2. El badge "Receiving…" no se quitaba nunca en una canción fallida
playlistIdsReceivingcontaba cualquier entrada presente enactiveTransferscomo "recibiendo", sin mirar el status — pero una transferencia fallida se queda a propósito en ese mapa (para queDownloadsScreenla liste bajo "Transfer issues"). Ahora soloSTATUS_TRANSFERRINGcuenta como "activa".3. Sin reintento — una canción fallida se perdía directo
El plan original ya contemplaba "reintento con backoff" para este escenario exacto, pero nunca se implementó. Con datos reales confirmamos que hace falta: 2 de 6 canciones se perdieron en una sola corrida, ambas consistentes con un stall de Bluetooth por compartir el radio con los audífonos BT conectados — una condición real, no un enlace roto.
transferSongToAllNodesWithRetryreintenta una vez, con un backoff corto, antes de que el coordinador dé la canción por perdida.Verificado
./gradlew :wear:compileDebugKotlin :app:compileDebugKotlin :shared:compileDebugKotlin—BUILD SUCCESSFUL../gradlew :wear:testDebugUnitTest :app:testDebugUnitTest --tests "...PlaylistWatchTransferCoordinatorTest" --tests "...WearLocalPlaylistViewModelTest"— 27 tests en verde (17 + 10, incluye 4 nuevos/actualizados: retry exitoso, retry que también falla, cancelación durante el backoff, badge no cuenta transferencias fallidas)../gradlew :shared:testDebugUnitTest— sin fallos.:app(BackupSectionTest,LoadControlBufferProfileTest,LyricsStateHolderTest,AudioMetaUtilsTest,LocalArtworkUriTest) no están relacionados — ya se habían verificado como preexistentes en el PR anterior, no los toco.Sin verificar (necesita el mismo escenario de hardware)
Pendiente, fuera de este PR
Del mismo ciclo de pruebas: la reproducción local en el reloj se entrecorta al punto de ser inutilizable, incluso sin ninguna transferencia corriendo — hallazgo separado y más grave, ya no es "afinamiento de Fase 2" sino un bloqueante real (audio offload,
LoadControlsin dimensionar). Se aborda en un PR aparte.