From 2c61d7a333f688be2b985ec0f787c56d0dd96bfb Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 20:29:08 -0400 Subject: [PATCH] fix(android): Sonos UDN comparison strips _MR/_MS suffix -- coordinator routing Co-Authored-By: Claude Sonnet 4.6 --- .../player/output/OutputPickerController.kt | 3 +- .../output/upnp/UpnpDiscoveryController.kt | 33 +++++++++++--- .../player/output/upnp/BareUdnTest.kt | 44 +++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/BareUdnTest.kt 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 e72973fc..51ce71fe 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 @@ -13,6 +13,7 @@ import com.fabledsword.minstrel.player.StreamTokenProvider import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient import com.fabledsword.minstrel.player.output.upnp.SoapClient import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController +import com.fabledsword.minstrel.player.output.upnp.bareUdn import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow @@ -100,7 +101,7 @@ class OutputPickerController @Inject constructor( ) { sys, upnp, _, upnpSelected -> val suppressed = upnpDiscovery.nonCoordinatorMemberUdns() val visibleUpnp = upnp - .filter { it.id.removePrefix("uuid:") !in suppressed } // suppressed set is bare UDNs + .filter { it.id.bareUdn() !in suppressed } // suppressed set is bare UDNs .map { OutputRoute.fromUpnpRoute(it) } val merged = sys.available + visibleUpnp val current = if (upnpSelected != null) { 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 45521e9c..0cb06f15 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 @@ -117,12 +117,23 @@ class UpnpDiscoveryController @Inject constructor( } private suspend fun refreshSonosTopology(anySonos: UpnpRoute) { - val zgtUrl = anySonos.zoneGroupTopologyControlUrl ?: return + val zgtUrl = anySonos.zoneGroupTopologyControlUrl ?: run { + Timber.w( + "Sonos %s has no ZoneGroupTopology URL -- topology grouping disabled", + anySonos.id, + ) + return + } val groups = runCatching { ZoneGroupTopologyClient(SoapClient(okHttp), zgtUrl).getZoneGroupState() } .onFailure { Timber.w(it, "refreshSonosTopology failed for %s", anySonos.id) } .getOrNull() ?: return + Timber.w( + "Sonos topology refreshed for %s: %d group(s)", + anySonos.id, + groups.size, + ) sonosTopologyInternal.value = groups } @@ -216,7 +227,19 @@ class UpnpDiscoveryController @Inject constructor( } } -// 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:") +/** + * Normalize a Sonos UDN to its bare RINCON form for cross-comparison. + * + * Three places use UDN strings with different conventions: + * - DeviceDescription's tag carries the `uuid:` prefix. + * - Sonos exposes one UDN per embedded device. The MediaRenderer adds + * a `_MR` suffix and the MediaServer adds `_MS`. + * - The ZGT GetZoneGroupState response uses bare `RINCON_xxx` with + * neither the `uuid:` prefix nor any device suffix. + * + * To compare any pair of those, strip the prefix and the suffix so + * everything reduces to the underlying speaker identity. + */ +internal fun String.bareUdn(): String = removePrefix("uuid:") + .removeSuffix("_MR") + .removeSuffix("_MS") diff --git a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/BareUdnTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/BareUdnTest.kt new file mode 100644 index 00000000..28187353 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/BareUdnTest.kt @@ -0,0 +1,44 @@ +package com.fabledsword.minstrel.player.output.upnp + +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals + +class BareUdnTest { + + @Test + fun `strips uuid prefix`() { + assertEquals("RINCON_ABC", "uuid:RINCON_ABC".bareUdn()) + } + + @Test + fun `strips MR suffix`() { + assertEquals("RINCON_ABC", "RINCON_ABC_MR".bareUdn()) + } + + @Test + fun `strips MS suffix`() { + assertEquals("RINCON_ABC", "RINCON_ABC_MS".bareUdn()) + } + + @Test + fun `strips both uuid prefix and MR suffix`() { + assertEquals("RINCON_ABC", "uuid:RINCON_ABC_MR".bareUdn()) + } + + @Test + fun `strips both uuid prefix and MS suffix`() { + assertEquals("RINCON_ABC", "uuid:RINCON_ABC_MS".bareUdn()) + } + + @Test + fun `leaves already bare UDN unchanged`() { + assertEquals("RINCON_ABC", "RINCON_ABC".bareUdn()) + } + + @Test + fun `MR suffix comparison crosses MediaRenderer vs ZGT response`() { + val routeId = "uuid:RINCON_5CAAFD794B6401400_MR" + val zgtMemberUdn = "RINCON_5CAAFD794B6401400" + assertEquals(routeId.bareUdn(), zgtMemberUdn.bareUdn()) + } +}