From 9002cf5559816c2a53753480b234a4b3288a3e7f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 17:05:31 -0400 Subject: [PATCH] refactor(android): UPnP discovery cleanup -- bareUdn helper, Timber, suppress, kdoc Co-Authored-By: Claude Sonnet 4.6 --- .../output/upnp/UpnpDiscoveryController.kt | 23 ++++++++++++------- .../minstrel/player/output/upnp/UpnpRoute.kt | 8 +++---- .../output/upnp/DeviceDescriptionTest.kt | 1 + 3 files changed, 20 insertions(+), 12 deletions(-) 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 ff54cc4b..12f02b2c 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 @@ -1,3 +1,4 @@ +@file:Suppress("TooManyFunctions") // discovery + Sonos topology + transport-lookup density package com.fabledsword.minstrel.player.output.upnp import android.content.Context @@ -15,6 +16,7 @@ import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.OkHttpClient import okhttp3.Request +import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -113,7 +115,9 @@ class UpnpDiscoveryController @Inject constructor( val zgtUrl = anySonos.zoneGroupTopologyControlUrl ?: return val groups = runCatching { ZoneGroupTopologyClient(SoapClient(okHttp), zgtUrl).getZoneGroupState() - }.getOrNull() ?: return + } + .onFailure { Timber.w(it, "refreshSonosTopology failed for %s", anySonos.id) } + .getOrNull() ?: return sonosTopologyInternal.value = groups } @@ -124,13 +128,11 @@ class UpnpDiscoveryController @Inject constructor( * carries it but Sonos's ZoneGroupState Coordinator attr does not. */ fun coordinatorRouteFor(udn: String): UpnpRoute? { - val bare = udn.removePrefix("uuid:") + val bare = udn.bareUdn() val coord = sonosTopologyInternal.value - .firstOrNull { g -> g.members.any { it.udn.removePrefix("uuid:") == bare } } + .firstOrNull { g -> g.members.any { it.udn.bareUdn() == bare } } ?.coordinatorUdn ?: return null - return routesInternal.value.firstOrNull { - it.id.removePrefix("uuid:") == coord.removePrefix("uuid:") - } + return routesInternal.value.firstOrNull { it.id.bareUdn() == coord.bareUdn() } } /** @@ -139,8 +141,8 @@ class UpnpDiscoveryController @Inject constructor( */ fun nonCoordinatorMemberUdns(): Set { return sonosTopologyInternal.value.flatMap { g -> - g.members.map { it.udn.removePrefix("uuid:") } - .filter { it != g.coordinatorUdn.removePrefix("uuid:") } + g.members.map { it.udn.bareUdn() } + .filter { it != g.coordinatorUdn.bareUdn() } }.toSet() } @@ -208,3 +210,8 @@ class UpnpDiscoveryController @Inject constructor( val IP_SUFFIX_REGEX = Regex("""\s*\(\d+\.\d+\.\d+\.\d+\)$""") } } + +// uuid: prefix normalization — DeviceDescription carries it; Sonos +// ZoneGroupState Coordinator/UUID attrs don't. Helper centralizes the strip +// so a future change to the convention isn't a silent compare-always-false bug. +private fun String.bareUdn(): String = removePrefix("uuid:") diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt index 63022a1b..b1ffbbba 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpRoute.kt @@ -7,10 +7,10 @@ import okhttp3.HttpUrl * Bose SoundTouch, generic DLNA renderers). Lifted out of the SOAP / * SSDP details so the picker UI consumes a narrow domain shape. * - * Generic UPnP only for THIS slice — Sonos-specific grouping value-adds - * (group join/leave, zone topology) live in a separate Sonos extension - * scoped in - * docs/superpowers/specs/2026-06-03-android-output-picker-upnp-scope.md. + * Sonos devices additionally populate [zoneGroupTopologyControlUrl] from the + * ZoneGroupTopology service in their device description; non-Sonos devices + * leave it null. The discovery controller uses that URL to aggregate stereo + * pairs and multi-speaker groups into single picker rows. * * [id] is the device UDN (e.g. `uuid:RINCON_ABC...`). [name] is the * raw `` straight from the device description — callers diff --git a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt index 8b05d772..2668d066 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/DeviceDescriptionTest.kt @@ -58,6 +58,7 @@ class DeviceDescriptionTest { "http://192.168.1.50:1400/MediaRenderer/RenderingControl/Control", desc.renderingControlUrl?.toString(), ) + assertNull(desc.zoneGroupTopologyControlUrl) } @Test