From 036da9dea8920e3d87d2885cfee0d394bd769a7e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 14:54:39 -0400 Subject: [PATCH 1/4] fix(android): UPnP - clean Sonos friendlyName for picker display Sonos uses the friendlyName format 'Room - Device Type - RINCON_' The picker was showing it verbatim, so the user saw rows like 'Living Room - Sonos Play:1 Media Renderer - RINCON_5CAAFD79...' Now strips on the first ' - ' for Sonos manufacturer matches, so the chip shows just 'Living Room' / 'Kitchen' / etc. Subtitle (manufacturer + model) still renders below per the existing sheet design, so the device-type info isn't lost. Generic UPnP devices that append a '(192.168.x.x)' IP suffix get that stripped too via an end-of-string-anchored regex. Empty / blank friendlyName still falls back to 'Network speaker'. --- .../output/upnp/UpnpDiscoveryController.kt | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt index ad0a8ebb..33427cd0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt @@ -106,7 +106,7 @@ class UpnpDiscoveryController @Inject constructor( ?.let { desc -> UpnpRoute( id = desc.udn, - name = desc.friendlyName.ifBlank { "Network speaker" }, + name = displayName(desc.friendlyName, desc.manufacturer), manufacturer = desc.manufacturer, modelName = desc.modelName, avTransportControlUrl = desc.avTransportControlUrl, @@ -115,6 +115,30 @@ class UpnpDiscoveryController @Inject constructor( } } + /** + * Clean the raw UPnP friendlyName for picker display. Sonos uses + * the format `Room - Device Type - RINCON_`; we strip + * everything after the first " - " so the chip shows just "Living + * Room" / "Kitchen" / etc. Generic UPnP devices (Yamaha, Samsung + * TV, etc.) often append a "(192.168.x.x)" IP suffix; strip that + * too. Empty / blank → "Network speaker" fallback. + * + * Device subtitle (manufacturer + model) is rendered separately + * by the picker sheet, so room-only here doesn't lose information. + */ + private fun displayName(friendlyName: String, manufacturer: String): String { + val trimmed = friendlyName.trim() + if (trimmed.isBlank()) return "Network speaker" + val byVendor = when { + manufacturer.contains("Sonos", ignoreCase = true) -> { + trimmed.substringBefore(" - ", trimmed) + } + else -> trimmed + } + val withoutIpSuffix = byVendor.replace(IP_SUFFIX_REGEX, "").trim() + return withoutIpSuffix.ifBlank { "Network speaker" } + } + private fun fetchBody(url: HttpUrl): String? { val body = runCatching { okHttp.newCall(Request.Builder().url(url).build()).execute().use { @@ -123,4 +147,11 @@ class UpnpDiscoveryController @Inject constructor( }.getOrNull().orEmpty() return body.ifEmpty { null } } + + private companion object { + // " (192.168.0.77)" trailing host suffix some generic UPnP + // devices append. Anchored to end-of-string so it never eats + // a legitimate parenthetical inside a name. + val IP_SUFFIX_REGEX = Regex("""\s*\(\d+\.\d+\.\d+\.\d+\)$""") + } } -- 2.52.0 From 96f12d6aacd733bb0ed78bedfef095a2d07b1b6e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 14:59:57 -0400 Subject: [PATCH 2/4] fix(android): UPnP select - loud logs on every code path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent early returns in selectUpnp were swallowing the most likely failure modes: - currentTrack null (nothing playing locally → can't cast a track) - transportFor() returns null (route disappeared or id mismatch) On-device verification reported 'tap collapses the sheet but no audio routes', with logcat empty - one of these was firing without any signal. Each early-return now Timber.w's why; the runCatching block adds Timber.i breadcrumbs at every step (mint token, SetAVTransportURI, Play, done) so the next failure shows exactly how far we got. --- .../player/output/OutputPickerController.kt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt index 76ee9b9b..1f6a178b 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt @@ -178,13 +178,28 @@ class OutputPickerController @Inject constructor( * deserialize / SOAP-parse code paths). */ private suspend fun selectUpnp(route: OutputRoute) { - val trackId = playerController.uiState.value.currentTrack?.id ?: return - val transport = upnpDiscovery.transportFor(route.id) ?: return + val trackId = playerController.uiState.value.currentTrack?.id + if (trackId == null) { + Timber.w("UPnP select skipped: no currentTrack (start playback first)") + return + } + val transport = upnpDiscovery.transportFor(route.id) + if (transport == null) { + Timber.w( + "UPnP select skipped: no transport for route id=${route.id} " + + "(route disappeared or id mismatch with discovery list)", + ) + return + } runCatching { + Timber.i("UPnP select: mint token for track=$trackId, route=${route.name}") val token = castApi.streamToken(StreamTokenRequest(trackId = trackId)) + Timber.i("UPnP select: SetAVTransportURI to ${token.url}") transport.setAVTransportURI(token.url) + Timber.i("UPnP select: Play") transport.play() playerController.pause() + Timber.i("UPnP select: done") }.onFailure { e -> Timber.w(e, "UPnP select failed for route ${route.id}") } -- 2.52.0 From a9edc12523c51211787db80def4f3fe29be31665 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 15:01:03 -0400 Subject: [PATCH 3/4] fix(android): release-build Timber tree at WARN+ for operator diagnosis Debug builds got DebugTree; release builds had no tree planted at all, so Timber.w / Timber.e calls were dropped silently in production. That's how the UPnP select diagnostic-prints went invisible during on-device testing - the released APK had no Timber output reaching logcat. Plant a release-only Tree that emits at WARN and above via android.util.Log.println with the canonical 'Minstrel' tag (or the caller-supplied tag when present). Keeps DEBUG / INFO traffic out of production logcat (the chatty stuff is the part we don't want flooding the buffer) while letting operator-driven adb logcat sessions still see real failures. --- .../minstrel/MinstrelApplication.kt | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt b/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt index 9aa83efe..d04bb858 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/MinstrelApplication.kt @@ -158,10 +158,43 @@ class MinstrelApplication : override fun onCreate() { super.onCreate() - if (BuildConfig.DEBUG) Timber.plant(Timber.DebugTree()) + // Debug builds get the full DebugTree (verbose). Release builds + // get a WARN+ tree so operator-driven diagnosis via `adb logcat` + // still surfaces UPnP / cast failures, OkHttp errors, and our + // own Timber.w / Timber.e calls — without the chatty DEBUG / + // INFO traffic flooding the buffer in production. + if (BuildConfig.DEBUG) { + Timber.plant(Timber.DebugTree()) + } else { + Timber.plant(ReleaseTree()) + } appScope.launch { resumeController.restore() } } + /** + * Release-build Timber tree: emits at WARN and above only. + * `android.util.Log` with the canonical tag so `adb logcat` shows + * the line under the standard tag column without falling through + * to the package-stack-trace tag DebugTree produces. + */ + private class ReleaseTree : Timber.Tree() { + override fun isLoggable(tag: String?, priority: Int): Boolean = + priority >= android.util.Log.WARN + + override fun log(priority: Int, tag: String?, message: String, t: Throwable?) { + val resolvedTag = tag ?: "Minstrel" + if (t == null) { + android.util.Log.println(priority, resolvedTag, message) + } else { + android.util.Log.println( + priority, + resolvedTag, + message + '\n' + android.util.Log.getStackTraceString(t), + ) + } + } + } + override val workManagerConfiguration: Configuration get() = Configuration.Builder() .setWorkerFactory(workerFactory) -- 2.52.0 From 7d15f57e8642bcd639a7a1c026c320ff97e3cbdf Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 15:17:34 -0400 Subject: [PATCH 4/4] fix(server): cast token URL honors X-Forwarded-Proto / Host On-device test against Sonos showed SetAVTransportURI returning UPnP error 714 (IllegalMimeType). Logcat: POST /api/cast/stream-token -> 200 (token minted) SetAVTransportURI to http://minstrel.fabledsword.com/... <-- 500 from Sonos: SoapFaultException SOAP fault 714 The server is behind a TLS-terminating reverse proxy, so r.TLS is nil and the URL builder emitted http://. Sonos does a HEAD probe to detect the audio MIME type; against an http:// URL that 301s to https://, the probe finds no audio body and bails with 714. The Task 2 code-quality reviewer flagged this exact scenario at the time. Closing it now: honor X-Forwarded-Proto + X-Forwarded-Host before falling back to r.TLS + r.Host. Public URL the speaker fetches now matches the scheme/host the client used to reach the endpoint. --- internal/api/cast_token.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/internal/api/cast_token.go b/internal/api/cast_token.go index 5fdf14f8..37751150 100644 --- a/internal/api/cast_token.go +++ b/internal/api/cast_token.go @@ -56,11 +56,25 @@ func (h *handlers) handleCastStreamToken(w http.ResponseWriter, r *http.Request) exp := time.Now().Add(time.Duration(expSec) * time.Second).Unix() token := SignStreamToken(h.streamSecret, req.TrackID, exp) + // Behind a TLS-terminating reverse proxy, r.TLS is nil even though + // the public-facing URL is https://. UPnP devices (Sonos especially) + // reject SetAVTransportURI with error 714 (IllegalMimeType) when + // they hit an http:// URL that immediately 301s to https:// — the + // MIME probe fails to find an audio body. Honor X-Forwarded-Proto + + // X-Forwarded-Host first so the URL we hand to the speaker reaches + // it on the same scheme/host the client used. Falls back to + // r.TLS-based detection for direct (no-proxy) deployments. scheme := "http" - if r.TLS != nil { + if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { + scheme = proto + } else if r.TLS != nil { scheme = "https" } - url := scheme + "://" + r.Host + "/api/tracks/" + req.TrackID + + host := r.Host + if h := r.Header.Get("X-Forwarded-Host"); h != "" { + host = h + } + url := scheme + "://" + host + "/api/tracks/" + req.TrackID + "/stream?token=" + token + "&exp=" + strconv.FormatInt(exp, 10) writeJSON(w, http.StatusOK, castTokenResponse{Token: token, Exp: exp, URL: url}) -- 2.52.0