From 70f5b9cf07ceb4c4bada88a378d60666419078bb Mon Sep 17 00:00:00 2001 From: Tobias Laross Date: Mon, 24 Aug 2026 13:24:33 +0200 Subject: [PATCH 1/2] Fixed silent speaker grouping failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Home Assistant answers media_player.join with a 200 before the membership is live, and Music Assistant drops a join it can't act on without reporting anything. The single confirming reload could not tell those apart, so a group change that never happened looked like a successful tap. Grouping now reloads up to three times before deciding, and raises the error banner when the membership never changes. The usual cause is the leader playing a source Music Assistant doesn't own — Spotify Connect straight to the Sonos reports no active_queue — so MediaPlayerEntity decodes that attribute and the banner names it, since starting playback from the app is the fix. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VFiUyeVCBWhZoxkKK9uBEA --- IntelliNest.xcodeproj/project.pbxproj | 4 ++ IntelliNest/Model/MediaPlayerEntity.swift | 17 ++++++++ .../ViewModels/MusicViewModel+Grouping.swift | 40 ++++++++++++++---- IntelliNest/ViewModels/MusicViewModel.swift | 9 ++++ .../MusicViewModelGroupingRulesTests.swift | 42 +++++++++++++++++++ IntelliNestTests/MusicViewModelTests.swift | 12 ++++-- 6 files changed, 113 insertions(+), 11 deletions(-) create mode 100644 IntelliNestTests/MusicViewModelGroupingRulesTests.swift diff --git a/IntelliNest.xcodeproj/project.pbxproj b/IntelliNest.xcodeproj/project.pbxproj index aadf923..baad97f 100644 --- a/IntelliNest.xcodeproj/project.pbxproj +++ b/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 = ""; }; EF000011000000000000000B /* MusicViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelTests.swift; sourceTree = ""; }; EF00001100000000000000C1 /* MusicViewModelGroupingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelGroupingTests.swift; sourceTree = ""; }; + EF00001100000000000000D1 /* MusicViewModelGroupingRulesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicViewModelGroupingRulesTests.swift; sourceTree = ""; }; EF000012000000000000000C /* MusicModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MusicModelTests.swift; sourceTree = ""; }; F62CD2E228770D5500462092 /* LightsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LightsViewModel.swift; sourceTree = ""; }; F62CD2E428770F3B00462092 /* RestAPIService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RestAPIService.swift; sourceTree = ""; }; @@ -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 */, diff --git a/IntelliNest/Model/MediaPlayerEntity.swift b/IntelliNest/Model/MediaPlayerEntity.swift index 41519d8..c6943ec 100644 --- a/IntelliNest/Model/MediaPlayerEntity.swift +++ b/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 diff --git a/IntelliNest/ViewModels/MusicViewModel+Grouping.swift b/IntelliNest/ViewModels/MusicViewModel+Grouping.swift index 6df816c..2e9f028 100644 --- a/IntelliNest/ViewModels/MusicViewModel+Grouping.swift +++ b/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 diff --git a/IntelliNest/ViewModels/MusicViewModel.swift b/IntelliNest/ViewModels/MusicViewModel.swift index 5731fcd..3cd01bc 100644 --- a/IntelliNest/ViewModels/MusicViewModel.swift +++ b/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 { diff --git a/IntelliNestTests/MusicViewModelGroupingRulesTests.swift b/IntelliNestTests/MusicViewModelGroupingRulesTests.swift new file mode 100644 index 0000000..e96336c --- /dev/null +++ b/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") + } +} diff --git a/IntelliNestTests/MusicViewModelTests.swift b/IntelliNestTests/MusicViewModelTests.swift index d1476cf..8413bbd 100644 --- a/IntelliNestTests/MusicViewModelTests.swift +++ b/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) } From f03cd863f9db2022a94d1aef50f44b32d7ccf433 Mon Sep 17 00:00:00 2001 From: Tobias Laross Date: Mon, 24 Aug 2026 13:40:50 +0200 Subject: [PATCH 2/2] Fixed group confirmation reading the wrong leader Confirmation called isGrouped, which reads whichever speaker is selected when the reloads resume. Selecting another speaker mid-confirmation made a landed join report failure, and could judge an unjoin against a group the request never touched. Membership is now read from the leader the request was sent to, and the external-source check reads that same leader. Also compares activeQueueID in MediaPlayerEntity equality, which otherwise hides a transition into or out of external playback from any future consumer. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VFiUyeVCBWhZoxkKK9uBEA --- IntelliNest/Model/MediaPlayerEntity.swift | 3 +- .../ViewModels/MusicViewModel+Grouping.swift | 25 +++++++----- IntelliNest/ViewModels/MusicViewModel.swift | 4 +- .../MusicViewModelGroupingRulesTests.swift | 38 +++++++++++++++++++ IntelliNestTests/MusicViewModelTests.swift | 6 ++- 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/IntelliNest/Model/MediaPlayerEntity.swift b/IntelliNest/Model/MediaPlayerEntity.swift index c6943ec..6f79f30 100644 --- a/IntelliNest/Model/MediaPlayerEntity.swift +++ b/IntelliNest/Model/MediaPlayerEntity.swift @@ -259,7 +259,8 @@ struct MediaPlayerEntity: EntityProtocol, Decodable { lhs.repeatMode == rhs.repeatMode && lhs.mediaPosition == rhs.mediaPosition && lhs.mediaDuration == rhs.mediaDuration && - lhs.mediaPositionUpdatedAt == rhs.mediaPositionUpdatedAt + lhs.mediaPositionUpdatedAt == rhs.mediaPositionUpdatedAt && + lhs.activeQueueID == rhs.activeQueueID } } diff --git a/IntelliNest/ViewModels/MusicViewModel+Grouping.swift b/IntelliNest/ViewModels/MusicViewModel+Grouping.swift index 2e9f028..ea51550 100644 --- a/IntelliNest/ViewModels/MusicViewModel+Grouping.swift +++ b/IntelliNest/ViewModels/MusicViewModel+Grouping.swift @@ -76,7 +76,7 @@ extension MusicViewModel { defer { pendingGroupingSpeakers.remove(speakerID) } if wasGrouped { let success = await restAPIService.unjoinSpeaker(memberID: speakerID) - if success, await confirmGroupChange(speakerID, shouldBeGrouped: false) { + if success, await confirmGroupChange(speakerID, with: activeSpeakerID, shouldBeGrouped: false) { return } setErrorBannerText("Kunde inte dela upp högtalare", "Det gick inte att ta bort \(speakerName) från gruppen") @@ -90,12 +90,12 @@ extension MusicViewModel { let success = await restAPIService.joinSpeakers(leaderID: activeSpeakerID, memberIDs: [speakerID], unjoinFirst: isInOtherGroup) - if success, await confirmGroupChange(speakerID, shouldBeGrouped: true) { + if success, await confirmGroupChange(speakerID, with: activeSpeakerID, 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 { + if speakers[activeSpeakerID]?.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") @@ -105,14 +105,21 @@ extension MusicViewModel { } } - /// 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 { + /// Reloads the speakers until `speakerID`'s membership in `leaderID`'s group + /// matches what the 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. Membership is read against the leader the request was sent to + /// rather than `isGrouped`, since the user can select a different speaker while + /// these reloads are in flight. + private func confirmGroupChange(_ speakerID: EntityId, + with leaderID: EntityId, + shouldBeGrouped: Bool, + attempts: Int = 3) async -> Bool { for attempt in 1 ... attempts { await reloadSpeakers() - if isGrouped(speakerID) == shouldBeGrouped { + let isMember = speakers[leaderID]?.groupMembers.contains(speakerID) == true + if isMember == shouldBeGrouped { return true } if attempt < attempts { diff --git a/IntelliNest/ViewModels/MusicViewModel.swift b/IntelliNest/ViewModels/MusicViewModel.swift index 3cd01bc..187431f 100644 --- a/IntelliNest/ViewModels/MusicViewModel.swift +++ b/IntelliNest/ViewModels/MusicViewModel.swift @@ -191,7 +191,7 @@ class MusicViewModel: ObservableObject, Reloadable { /// 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 + let waitBeforeGroupRecheck: @MainActor () 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 @@ -234,7 +234,7 @@ class MusicViewModel: ObservableObject, Reloadable { saveLastSpeaker: @escaping @MainActor (EntityId) -> Void = { UserDefaults.shared.set($0.rawValue, forKey: StorageKeys.lastMusicSpeaker.rawValue) }, - waitBeforeGroupRecheck: @escaping @Sendable () async -> Void = { + waitBeforeGroupRecheck: @escaping @MainActor () async -> Void = { try? await Task.sleep(for: .seconds(1)) }) { self.restAPIService = restAPIService diff --git a/IntelliNestTests/MusicViewModelGroupingRulesTests.swift b/IntelliNestTests/MusicViewModelGroupingRulesTests.swift index e96336c..69405fc 100644 --- a/IntelliNestTests/MusicViewModelGroupingRulesTests.swift +++ b/IntelliNestTests/MusicViewModelGroupingRulesTests.swift @@ -13,6 +13,7 @@ extension MusicViewModelTests { data: speakerJSON(entityID: .mediaPlayerKitchen, state: "playing", friendlyName: "Köket", activeQueue: "RINCON_38420B10EC2801400")) await viewModel.reload() + XCTAssertFalse(viewModel.isGrouped(.mediaPlayerSpa)) stubPostService(path: "/api/services/media_player/join") await viewModel.toggleGroupMember(.mediaPlayerSpa) XCTAssertFalse(viewModel.isGrouped(.mediaPlayerSpa)) @@ -31,6 +32,7 @@ extension MusicViewModelTests { friendlyName: "Köket", title: "Kite", artist: "Benjamin Ingrosso")) await viewModel.reload() XCTAssertEqual(viewModel.activeSpeakerID, .mediaPlayerKitchen) + XCTAssertFalse(viewModel.isGrouped(.mediaPlayerSpa)) stubPostService(path: "/api/services/media_player/join") await viewModel.toggleGroupMember(.mediaPlayerSpa) @@ -39,4 +41,40 @@ extension MusicViewModelTests { XCTAssertEqual(bannerMessages.last, "Köket spelar från en annan app. Starta musiken härifrån för att spela på flera högtalare") } + + func testJoinConfirmedAgainstTheLeaderItWasSentTo() async { + // Home Assistant applies the group a beat late, and the user picks another + // speaker while the confirmation reloads are still running. The join still + // has to be judged against Köket, the leader it was sent to — not against + // whatever is selected by the time the reloads finish. + stubAllSpeakers(playing: .mediaPlayerKitchen) + stubSpeaker(.mediaPlayerKitchen, + data: speakerJSON(entityID: .mediaPlayerKitchen, state: "playing", + friendlyName: "Köket", activeQueue: "RINCON_38420B10EC2801400")) + await viewModel.reload() + XCTAssertEqual(viewModel.activeSpeakerID, .mediaPlayerKitchen) + XCTAssertFalse(viewModel.isGrouped(.mediaPlayerSpa)) + + stubPostService(path: "/api/services/media_player/join") + let group = [EntityId.mediaPlayerKitchen.rawValue, EntityId.mediaPlayerSpa.rawValue] + onGroupRecheckWait = { [weak self] in + guard let self else { + return + } + stubSpeaker(.mediaPlayerKitchen, + data: speakerJSON(entityID: .mediaPlayerKitchen, state: "playing", + friendlyName: "Köket", groupMembers: group, + activeQueue: "RINCON_38420B10EC2801400")) + stubSpeaker(.mediaPlayerSpa, + data: speakerJSON(entityID: .mediaPlayerSpa, state: "playing", + friendlyName: "Spa", groupMembers: group)) + viewModel.selectSpeaker(.mediaPlayerLivingRoom) + } + + await viewModel.toggleGroupMember(.mediaPlayerSpa) + + XCTAssertEqual(viewModel.speakers[.mediaPlayerKitchen]?.groupMembers, + [.mediaPlayerKitchen, .mediaPlayerSpa]) + XCTAssertTrue(bannerTitles.isEmpty) + } } diff --git a/IntelliNestTests/MusicViewModelTests.swift b/IntelliNestTests/MusicViewModelTests.swift index 8413bbd..7d89d74 100644 --- a/IntelliNestTests/MusicViewModelTests.swift +++ b/IntelliNestTests/MusicViewModelTests.swift @@ -8,6 +8,9 @@ class MusicViewModelTests: XCTestCase { var urlCreator: URLCreator! var bannerTitles: [String] = [] var bannerMessages: [String] = [] + /// Runs when the view model pauses between confirmation reloads, so a test can + /// change the world mid-flight (re-stub speakers, select another speaker). + var onGroupRecheckWait: (() -> Void)? /// In-memory backing for the last-used-speaker persistence so tests stay /// deterministic instead of touching shared `UserDefaults`. var storedLastSpeaker: EntityId? @@ -20,6 +23,7 @@ class MusicViewModelTests: XCTestCase { override func setUp() async throws { bannerTitles = [] bannerMessages = [] + onGroupRecheckWait = nil storedLastSpeaker = nil URLProtocolStub.startInterceptingRequests() let stubbedSession = URLProtocolStub.createStubbedURLSession() @@ -38,7 +42,7 @@ class MusicViewModelTests: XCTestCase { }, loadLastSpeaker: { [weak self] in self?.storedLastSpeaker }, saveLastSpeaker: { [weak self] in self?.storedLastSpeaker = $0 }, - waitBeforeGroupRecheck: {}) + waitBeforeGroupRecheck: { [weak self] in self?.onGroupRecheckWait?() }) } override func tearDown() async throws {