feat(marge): seed SPOTIFY/SpotifyConnectUserName placeholder + bind presets by exact (type, account) - #303
Draft
gesellix wants to merge 3 commits into
Draft
feat(marge): seed SPOTIFY/SpotifyConnectUserName placeholder + bind presets by exact (type, account)#303gesellix wants to merge 3 commits into
gesellix wants to merge 3 commits into
Conversation
…resets by exact (type, account) When Spotify Connect is the active source on the speaker, a storePreset sent to marge carries source="SPOTIFY" sourceAccount="SpotifyConnectUserName" — the firmware-internal slug for "the user pushed playback from the Spotify app, no OAuth involved". marge.UpdatePreset's old fallback matched by SourceKeyType alone and collapsed every SPOTIFY preset onto whichever OAuth-brokered entry happened to exist (account=gesellix), rewriting the sourceAccount on save and quietly redirecting recall through the OAuth token path instead of the firmware's Connect path. UpdatePreset now: - parses Source and SourceAccount from the incoming XML; - prefers an exact (SourceKeyType, SourceKeyAccount) match before falling back to the type-only legacy path. Direct ID match still wins first. New marge.EnsurePlaceholderSources seeds the SPOTIFY/SpotifyConnectUserName placeholder (no credentials, Status=UNAVAILABLE, deterministic ID PLACEHOLDER_SPOTIFY_SpotifyConnectUserName). knownPlaceholderSources() collects future placeholders (UPnP, StoredMusic) in one slice. registerSpotifySourceForDevice in handlers/server.go calls EnsurePlaceholderSources after a successful AddSource so every priming run guarantees both the OAuth entry and the Connect placeholder exist. Tests in pkg/service/marge/placeholder_sources_test.go: - placeholder is seeded on first call, no-op on second (idempotent); - placeholder coexists with an OAuth source under the same account; - a Connect-style storePreset binds to the placeholder, not the OAuth source (regression); - an OAuth-style storePreset still binds to the OAuth source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… priming EnsurePlaceholderSources was previously called from registerSpotifySourceForDevice (inside PrimeDeviceWithSpotify), which is gated on a Spotify service being configured AND an account being linked. That meant a user who only ever uses Spotify Connect (push from the Spotify app, no OAuth-managed account in AfterTouch) never got the SPOTIFY/SpotifyConnectUserName placeholder, so Connect-initiated presets still collapsed back to "invalid SourceID". Move the call to handleDiscoveredDevice (and its fallback) where it belongs semantically: the placeholder represents a firmware-managed slot on the speaker, so it should exist for every known device, period — no music-service config required. Idempotent, so re-discovery is safe. Remove the duplicate call from registerSpotifySourceForDevice; the discovery hook is now the single source of truth. New test in placeholder_discovery_test.go asserts that handleDiscoveredDevice seeds the placeholder for a fresh device when NO Spotify service is configured (srv.spotifyService == nil). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…clo to spec After adding the (SourceKeyType, SourceKeyAccount) tier to the storePreset matcher, UpdatePreset's cyclomatic complexity climbed to 23 (gocyclo threshold is 20). The three match tiers (ID → type+account → type-only legacy) read more clearly as named branches in a helper anyway, so pull them into findMatchingSourceForUpdatePreset and an isWellKnownProviderID predicate. UpdatePreset now just parses, logs, calls the matcher, and proceeds. Also adds the doc comment revive was missing on UpdatePreset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When Spotify Connect is the active source on the speaker, a storePreset
sent to marge carries source="SPOTIFY" sourceAccount="SpotifyConnectUserName"
— the firmware-internal slug for "the user pushed playback from the Spotify
app, no OAuth involved". marge.UpdatePreset's old fallback matched by
SourceKeyType alone and collapsed every SPOTIFY preset onto whichever
OAuth-brokered entry happened to exist (account=gesellix), rewriting the
sourceAccount on save and quietly redirecting recall through the OAuth
token path instead of the firmware's Connect path.
UpdatePreset now:
back to the type-only legacy path. Direct ID match still wins first.
New marge.EnsurePlaceholderSources seeds the SPOTIFY/SpotifyConnectUserName
placeholder (no credentials, Status=UNAVAILABLE, deterministic ID
PLACEHOLDER_SPOTIFY_SpotifyConnectUserName). knownPlaceholderSources()
collects future placeholders (UPnP, StoredMusic) in one slice.
registerSpotifySourceForDevice in handlers/server.go calls
EnsurePlaceholderSources after a successful AddSource so every priming run
guarantees both the OAuth entry and the Connect placeholder exist.
Tests in pkg/service/marge/placeholder_sources_test.go:
source (regression);
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com