Fixed silent speaker grouping failures

IntelliNest.xcodeproj/project.pbxproj

@@ -93,6 +93,7 @@
 		FE0000000000000000000005 /* MusicTrackControls.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD0000000000000000000005 /* MusicTrackControls.swift */; };
 		EE000011000000000000000B /* MusicViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF000011000000000000000B /* MusicViewModelTests.swift */; };
 		EE00001100000000000000C1 /* MusicViewModelGroupingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF00001100000000000000C1 /* MusicViewModelGroupingTests.swift */; };
+		EE00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift */; };
 		EE000012000000000000000C /* MusicModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF000012000000000000000C /* MusicModelTests.swift */; };
 		F62CD2E0286D950A00462092 /* LightEntity.swift in Sources */ = {isa = PBXBuildFile; fileRef = F62CD2DF286D950A00462092 /* LightEntity.swift */; };
 		F62CD2E328770D5500462092 /* LightsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F62CD2E228770D5500462092 /* LightsViewModel.swift */; };
@@ -340,6 +341,7 @@
 		FD0000000000000000000005 /* MusicTrackControls.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicTrackControls.swift; sourceTree = "<group>"; };
 		EF000011000000000000000B /* MusicViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelTests.swift; sourceTree = "<group>"; };
 		EF00001100000000000000C1 /* MusicViewModelGroupingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelGroupingTests.swift; sourceTree = "<group>"; };
+		EF00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelGroupingRulesTests.swift; sourceTree = "<group>"; };
 		EF000012000000000000000C /* MusicModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicModelTests.swift; sourceTree = "<group>"; };
 		F62CD2E228770D5500462092 /* LightsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LightsViewModel.swift; sourceTree = "<group>"; };
 		F62CD2E428770F3B00462092 /* RestAPIService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RestAPIService.swift; sourceTree = "<group>"; };
@@ -627,6 +629,7 @@
 			AA000000C00000000000000C /* HeaterEntityTests.swift */,
 				EF000011000000000000000B /* MusicViewModelTests.swift */,
 				EF00001100000000000000C1 /* MusicViewModelGroupingTests.swift */,
+				EF00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift */,
 				EF000012000000000000000C /* MusicModelTests.swift */,
 				EF0000150000000000000022 /* SpotifyApiServiceTests.swift */,
 				EF0000160000000000000023 /* MusicViewModelSpotifyTests.swift */,
@@ -1313,6 +1316,7 @@
 			DD0000001000000000000001 /* LynkViewModelTests.swift in Sources */,
 				EE000011000000000000000B /* MusicViewModelTests.swift in Sources */,
 				EE00001100000000000000C1 /* MusicViewModelGroupingTests.swift in Sources */,
+				EE00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift in Sources */,
 				EE000012000000000000000C /* MusicModelTests.swift in Sources */,
 				EE0000150000000000000022 /* SpotifyApiServiceTests.swift in Sources */,
 				EE0000160000000000000023 /* MusicViewModelSpotifyTests.swift in Sources */,

IntelliNest/Model/MediaPlayerEntity.swift

@@ -42,6 +42,21 @@ struct MediaPlayerEntity: EntityProtocol, Decodable {
     /// When `mediaPosition` was sampled (`media_position_updated_at`), the anchor
     /// for extrapolating the live position while playing.
     var mediaPositionUpdatedAt: Date?
+    /// The Music Assistant queue driving this player (`active_queue`), or nil when
+    /// nothing MA controls is playing on it. A Sonos streaming from the Spotify app
+    /// over Spotify Connect reports no active queue, because Music Assistant is not
+    /// in the path at all.
+    var activeQueueID: String?
+
+    /// Whether the speaker is playing something Music Assistant doesn't control —
+    /// Spotify Connect straight to the Sonos, an AirPlay session, the TV. Grouping
+    /// another speaker onto this one can't work in that state: Home Assistant
+    /// accepts the `join` with a 200 and Music Assistant drops it, having no stream
+    /// of its own to extend. Playback has to be (re)started through Music Assistant
+    /// first.
+    var isPlayingExternalSource: Bool {
+        hasLiveAudio && activeQueueID == nil
+    }
 
     var isActive: Bool {
         state == "playing"
@@ -180,6 +195,7 @@ struct MediaPlayerEntity: EntityProtocol, Decodable {
         case mediaPosition = "media_position"
         case mediaDuration = "media_duration"
         case mediaPositionUpdatedAt = "media_position_updated_at"
+        case activeQueue = "active_queue"
     }
 
     init(entityId: EntityId, state: String = "Loading", friendlyName: String = "") {
@@ -214,6 +230,7 @@ struct MediaPlayerEntity: EntityProtocol, Decodable {
             if let updatedAtString = try attributes.decodeIfPresent(String.self, forKey: .mediaPositionUpdatedAt) {
                 mediaPositionUpdatedAt = Entity.utcDateFormatter.date(from: updatedAtString)
             }
+            activeQueueID = try attributes.decodeIfPresent(String.self, forKey: .activeQueue)
         } else {
             friendlyName = ""
             volumeLevel = 0

IntelliNest/ViewModels/MusicViewModel+Grouping.swift

@@ -70,16 +70,16 @@ extension MusicViewModel {
         guard let activeSpeakerID, speakerID != activeSpeakerID else {
             return
         }
+        let speakerName = speakers[speakerID]?.friendlyName ?? speakerID.rawValue
+        let wasGrouped = isGrouped(speakerID)
         pendingGroupingSpeakers.insert(speakerID)
         defer { pendingGroupingSpeakers.remove(speakerID) }
-        let speakerName = speakers[speakerID]?.friendlyName ?? speakerID.rawValue
-        if isGrouped(speakerID) {
+        if wasGrouped {
             let success = await restAPIService.unjoinSpeaker(memberID: speakerID)
-            if success {
-                await reloadSpeakers()
-            } else {
-                setErrorBannerText("Kunde inte dela upp högtalare", "Det gick inte att ta bort \(speakerName) från gruppen")
+            if success, await confirmGroupChange(speakerID, shouldBeGrouped: false) {
+                return
             }
+            setErrorBannerText("Kunde inte dela upp högtalare", "Det gick inte att ta bort \(speakerName) från gruppen")
         } else {
             // A speaker already synced into a different group (e.g. Spa paired with
             // Matbord-ute) can't be moved by a plain join, so unjoin it from its
@@ -90,14 +90,38 @@ extension MusicViewModel {
             let success = await restAPIService.joinSpeakers(leaderID: activeSpeakerID,
                                                             memberIDs: [speakerID],
                                                             unjoinFirst: isInOtherGroup)
-            if success {
-                await reloadSpeakers()
+            if success, await confirmGroupChange(speakerID, shouldBeGrouped: true) {
+                return
+            }
+            // A leader playing a source Music Assistant doesn't own is the usual
+            // reason a join lands nowhere, and it's the one the user can act on.
+            if activeSpeaker?.isPlayingExternalSource == true {
+                let leaderName = speakers[activeSpeakerID]?.friendlyName ?? activeSpeakerID.rawValue
+                setErrorBannerText("Kunde inte gruppera högtalare",
+                                   "\(leaderName) spelar från en annan app. Starta musiken härifrån för att spela på flera högtalare")
             } else {
                 setErrorBannerText("Kunde inte gruppera högtalare", "Det gick inte att lägga till \(speakerName) i gruppen")
             }
         }
     }
 
+    /// Reloads the speakers until `speakerID`'s membership matches what the group
+    /// change asked for, and reports whether it ever did. Home Assistant returns 200
+    /// from `join`/`unjoin` before the membership is live, so a single reload can't
+    /// tell "not applied yet" from a group Music Assistant quietly refused to build.
+    private func confirmGroupChange(_ speakerID: EntityId, shouldBeGrouped: Bool, attempts: Int = 3) async -> Bool {
+        for attempt in 1 ... attempts {
+            await reloadSpeakers()
+            if isGrouped(speakerID) == shouldBeGrouped {
+                return true
+            }
+            if attempt < attempts {
+                await waitBeforeGroupRecheck()
+            }
+        }
+        return false
+    }
+
     /// Promotes a grouped speaker to primary — the one shown and controlled as the
     /// group's main speaker. Playback still routes through the live Music Assistant
     /// group leader (`playbackTargetID`), so switching the primary never interrupts

IntelliNest/ViewModels/MusicViewModel.swift

@@ -188,6 +188,11 @@ class MusicViewModel: ObservableObject, Reloadable {
     /// owner's name). Injected as a closure so tests don't depend on shared
     /// `UserDefaults`.
     let currentUser: @MainActor () -> User
+    /// Pauses between the reloads that confirm a group change landed. Home Assistant
+    /// applies the membership a beat after the service call returns, so it has to be
+    /// re-read rather than trusted once. Injected so tests confirm without wall time.
+    let waitBeforeGroupRecheck: @Sendable () async -> Void
+
     /// Reads/writes the last speaker the user controlled, so it can be
     /// pre-selected when the music view next opens. Injected as closures so tests
     /// don't depend on shared `UserDefaults`.
@@ -228,6 +233,9 @@ class MusicViewModel: ObservableObject, Reloadable {
          },
          saveLastSpeaker: @escaping @MainActor (EntityId) -> Void = {
              UserDefaults.shared.set($0.rawValue, forKey: StorageKeys.lastMusicSpeaker.rawValue)
+         },
+         waitBeforeGroupRecheck: @escaping @Sendable () async -> Void = {
+             try? await Task.sleep(for: .seconds(1))
          }) {
         self.restAPIService = restAPIService
         self.setErrorBannerText = setErrorBannerText
@@ -238,6 +246,7 @@ class MusicViewModel: ObservableObject, Reloadable {
         self.currentUser = currentUser
         self.loadLastSpeaker = loadLastSpeaker
         self.saveLastSpeaker = saveLastSpeaker
+        self.waitBeforeGroupRecheck = waitBeforeGroupRecheck
         isSpotifyAuthorized = spotify.isAuthorized
         var initialSpeakers: [EntityId: MediaPlayerEntity] = [:]
         for speakerID in Self.speakerIDs {

IntelliNestTests/MusicViewModelGroupingRulesTests.swift

@@ -0,0 +1,42 @@
+@testable import IntelliNest
+import XCTest
+
+// MARK: - Group changes Home Assistant accepts but never applies
+
+@MainActor
+extension MusicViewModelTests {
+    func testJoinAcceptedButNeverAppliedShowsBanner() async {
+        // Home Assistant answers 200 and the membership never changes. The tap has
+        // to report that instead of leaving the speaker looking like it joined.
+        stubAllSpeakers(playing: .mediaPlayerKitchen)
+        stubSpeaker(.mediaPlayerKitchen,
+                    data: speakerJSON(entityID: .mediaPlayerKitchen, state: "playing",
+                                      friendlyName: "Köket", activeQueue: "RINCON_38420B10EC2801400"))
+        await viewModel.reload()
+        stubPostService(path: "/api/services/media_player/join")
+        await viewModel.toggleGroupMember(.mediaPlayerSpa)
+        XCTAssertFalse(viewModel.isGrouped(.mediaPlayerSpa))
+        XCTAssertTrue(bannerTitles.contains("Kunde inte gruppera högtalare"))
+        XCTAssertEqual(bannerMessages.last, "Det gick inte att lägga till \(EntityId.mediaPlayerSpa.rawValue) i gruppen")
+        XCTAssertTrue(viewModel.pendingGroupingSpeakers.isEmpty)
+    }
+
+    func testJoinOntoAnExternalSourceExplainsWhyItFailed() async {
+        // Köket is playing over Spotify Connect, so it reports no active_queue and
+        // Music Assistant has no stream to extend to Spa. The banner has to say that
+        // rather than blame Spa, since starting playback from the app is the fix.
+        stubAllSpeakers()
+        stubSpeaker(.mediaPlayerKitchen,
+                    data: speakerJSON(entityID: .mediaPlayerKitchen, state: "playing",
+                                      friendlyName: "Köket", title: "Kite", artist: "Benjamin Ingrosso"))
+        await viewModel.reload()
+        XCTAssertEqual(viewModel.activeSpeakerID, .mediaPlayerKitchen)
+
+        stubPostService(path: "/api/services/media_player/join")
+        await viewModel.toggleGroupMember(.mediaPlayerSpa)
+
+        XCTAssertTrue(bannerTitles.contains("Kunde inte gruppera högtalare"))
+        XCTAssertEqual(bannerMessages.last,
+                       "Köket spelar från en annan app. Starta musiken härifrån för att spela på flera högtalare")
+    }
+}

IntelliNestTests/MusicViewModelTests.swift

@@ -7,6 +7,7 @@ class MusicViewModelTests: XCTestCase {
     var restAPIService: RestAPIService!
     var urlCreator: URLCreator!
     var bannerTitles: [String] = []
+    var bannerMessages: [String] = []
     /// In-memory backing for the last-used-speaker persistence so tests stay
     /// deterministic instead of touching shared `UserDefaults`.
     var storedLastSpeaker: EntityId?
@@ -18,6 +19,7 @@ class MusicViewModelTests: XCTestCase {
 
     override func setUp() async throws {
         bannerTitles = []
+        bannerMessages = []
         storedLastSpeaker = nil
         URLProtocolStub.startInterceptingRequests()
         let stubbedSession = URLProtocolStub.createStubbedURLSession()
@@ -30,11 +32,13 @@ class MusicViewModelTests: XCTestCase {
             repeatReloadAction: { _ in }
         )
         viewModel = MusicViewModel(restAPIService: restAPIService,
-                                   setErrorBannerText: { [weak self] title, _ in
+                                   setErrorBannerText: { [weak self] title, message in
                                        self?.bannerTitles.append(title)
+                                       self?.bannerMessages.append(message)
                                    },
                                    loadLastSpeaker: { [weak self] in self?.storedLastSpeaker },
-                                   saveLastSpeaker: { [weak self] in self?.storedLastSpeaker = $0 })
+                                   saveLastSpeaker: { [weak self] in self?.storedLastSpeaker = $0 },
+                                   waitBeforeGroupRecheck: {})
     }
 
     override func tearDown() async throws {
@@ -63,7 +67,8 @@ class MusicViewModelTests: XCTestCase {
                      entityPicture: String? = nil,
                      groupMembers: [String] = [],
                      shuffle: Bool = false,
-                     repeatMode: String = "off") -> Data {
+                     repeatMode: String = "off",
+                     activeQueue: String? = nil) -> Data {
         var attributes: [String: Any] = [
             "friendly_name": friendlyName,
             "volume_level": volume,
@@ -76,6 +81,7 @@ class MusicViewModelTests: XCTestCase {
         if let album { attributes["media_album_name"] = album }
         if let contentID { attributes["media_content_id"] = contentID }
         if let entityPicture { attributes["entity_picture"] = entityPicture }
+        if let activeQueue { attributes["active_queue"] = activeQueue }
         return makeEntityJSON(entityId: entityID.rawValue, state: state, attributes: attributes)
     }