chore(android): UPnP picker - log selectUpnp failures + drop dead fetchJob
android / Build + lint + test (push) Successful in 3m54s
android / Build + lint + test (push) Successful in 3m54s
Code-quality review flagged two non-blockers on commit 03cdff54:
1. selectUpnp's runCatching swallowed SOAP / token-mint failures
silently - OkHttp's logger doesn't see them since they happen in
our own deserialize / parse code. Adds Timber.w on the failure
path so operator's on-device Sonos verification can find the
cause in logcat instead of staring at "nothing happened".
2. UpnpDiscoveryController's fetchJob field was assigned but never
read or cancelled. appScope is process-lifetime so the launched
coroutine dies with the process - no explicit cancellation is
needed. Drop the field + the now-unused Job import.
This commit is contained in:
+7
-4
@@ -19,6 +19,7 @@ import kotlinx.coroutines.flow.stateIn
|
|||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import retrofit2.Retrofit
|
import retrofit2.Retrofit
|
||||||
import retrofit2.create
|
import retrofit2.create
|
||||||
|
import timber.log.Timber
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
import javax.inject.Singleton
|
import javax.inject.Singleton
|
||||||
|
|
||||||
@@ -171,10 +172,10 @@ class OutputPickerController @Inject constructor(
|
|||||||
* track, set the renderer's URI, play, then pause local playback so
|
* track, set the renderer's URI, play, then pause local playback so
|
||||||
* audio yields to the speaker. Wrapped in `runCatching` at each
|
* audio yields to the speaker. Wrapped in `runCatching` at each
|
||||||
* step — token failure, transport-lookup failure, and SOAP failure
|
* step — token failure, transport-lookup failure, and SOAP failure
|
||||||
* each abandon the selection cleanly rather than crashing. Errors
|
* each abandon the selection cleanly rather than crashing. Failures
|
||||||
* surface to the operator via OkHttp logging for now; spec'd UI
|
* log at warn level via Timber so on-device verification can find
|
||||||
* error surfacing lands in a follow-up if hardware reveals it's
|
* the cause in logcat (OkHttp's logger doesn't cover our own
|
||||||
* needed.
|
* deserialize / SOAP-parse code paths).
|
||||||
*/
|
*/
|
||||||
private suspend fun selectUpnp(route: OutputRoute) {
|
private suspend fun selectUpnp(route: OutputRoute) {
|
||||||
val trackId = playerController.uiState.value.currentTrack?.id ?: return
|
val trackId = playerController.uiState.value.currentTrack?.id ?: return
|
||||||
@@ -184,6 +185,8 @@ class OutputPickerController @Inject constructor(
|
|||||||
transport.setAVTransportURI(token.url)
|
transport.setAVTransportURI(token.url)
|
||||||
transport.play()
|
transport.play()
|
||||||
playerController.pause()
|
playerController.pause()
|
||||||
|
}.onFailure { e ->
|
||||||
|
Timber.w(e, "UPnP select failed for route ${route.id}")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+4
-4
@@ -5,7 +5,6 @@ import com.fabledsword.minstrel.di.ApplicationScope
|
|||||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.Job
|
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
@@ -45,11 +44,12 @@ class UpnpDiscoveryController @Inject constructor(
|
|||||||
private val routesInternal = MutableStateFlow<List<UpnpRoute>>(emptyList())
|
private val routesInternal = MutableStateFlow<List<UpnpRoute>>(emptyList())
|
||||||
val routes: StateFlow<List<UpnpRoute>> = routesInternal.asStateFlow()
|
val routes: StateFlow<List<UpnpRoute>> = routesInternal.asStateFlow()
|
||||||
|
|
||||||
private var fetchJob: Job? = null
|
|
||||||
|
|
||||||
init {
|
init {
|
||||||
ssdp.start(appScope)
|
ssdp.start(appScope)
|
||||||
fetchJob = appScope.launch(Dispatchers.IO) {
|
// appScope is process-lifetime (SupervisorJob + Dispatchers.Default),
|
||||||
|
// so the launched collector dies with the process — no explicit
|
||||||
|
// cancellation needed.
|
||||||
|
appScope.launch(Dispatchers.IO) {
|
||||||
ssdp.discoveries.collect { locationUrl -> handleDiscovery(locationUrl) }
|
ssdp.discoveries.collect { locationUrl -> handleDiscovery(locationUrl) }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user