mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
fix(health): consistency report — cut noise, fix Audio leak, group unsynced
First operator run of the new consistency check surfaced both real bugs
and a lot of noise. This commit refines the report so the remaining
findings are actionable.
Real bugs fixed:
- loadServiceView now resolves preset/recent Source via SourceID lookup
against Sources.xml, instead of trusting the persisted Source field.
syncPresets / syncRecents in sync.go currently writes the upstream
FullResponseSource.Type ("Audio") into ServicePreset.Source, which
made every cross-side mismatch finding read "service source='Audio'".
Underlying syncPresets/Recents misfeature is a separate fix; the
consistency check stops being fooled by it.
- Duplicate-source dedup keyed by type+account, not just type.
SpotifyConnectUserName + SpotifyAlexaUserName, QPlay1UserName +
QPlay2UserName are legitimate sub-accounts of the same source type
and used to falsely trip duplicate_source warnings.
Noise removed:
- Cross-side source_mismatch comparison dropped. Speaker /sources
enumerates local I/O sources (AUX, BLUETOOTH, AIRPLAY, QPLAY, …),
service Sources.xml tracks credentialed streaming sources (TUNEIN,
INTERNET_RADIO, …). They legitimately don't overlap on most types,
so the asymmetry was pure noise.
- Internal-consistency check restricted to the service side. Streaming
sources are never in the speaker's /sources by design (they're
proxied through BMX), so a TUNEIN preset on the speaker always
looked "dangling" against speaker /sources.
- Service-only / speaker-only recent cascade collapsed into one
summary line when 5+ speaker recents are missing from service.
- New short-circuit: when the service has nothing (presets, recents,
sources all empty) for a device the speaker clearly has state for,
emit one "this device looks unsynced, click Sync" warning instead
of dozens of per-slot mismatches.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
c47cf81a93
commit
ce2935a4bd
@@ -111,6 +111,10 @@ func checkOneDeviceConsistency(ds *datastore.DataStore, account, deviceID, ipAdd
|
||||
|
||||
var findings []Finding
|
||||
|
||||
// Internal consistency is meaningful only on the service side —
|
||||
// the speaker manages its own preset/source coherence locally,
|
||||
// and its /sources list deliberately omits streaming sources
|
||||
// (which would always trigger spurious "dangling" findings).
|
||||
findings = append(findings, issuesToFindings(target, CheckInternalConsistency(serviceView), SeverityWarning)...)
|
||||
|
||||
if ipAddress == "" {
|
||||
@@ -136,12 +140,33 @@ func checkOneDeviceConsistency(ds *datastore.DataStore, account, deviceID, ipAdd
|
||||
return findings
|
||||
}
|
||||
|
||||
findings = append(findings, issuesToFindings(target, CheckInternalConsistency(speakerView), SeverityWarning)...)
|
||||
// "Unsynced device" short-circuit: when the service has no
|
||||
// presets / recents / sources for a device that the speaker
|
||||
// clearly does have all three for, emit one consolidated
|
||||
// finding instead of a torrent of per-slot mismatches.
|
||||
if isServiceUnsynced(serviceView) && !isSpeakerEmpty(speakerView) {
|
||||
findings = append(findings, Finding{
|
||||
Severity: SeverityWarning,
|
||||
Target: target,
|
||||
Message: "Device has speaker state (presets, recents, sources) but service has nothing for it — looks like a missed pair/sync. Click \"Sync\" in the device tab or factory-reset and re-pair.",
|
||||
})
|
||||
|
||||
return findings
|
||||
}
|
||||
|
||||
findings = append(findings, issuesToFindings(target, CheckCrossSide(speakerView, serviceView), SeverityWarning)...)
|
||||
|
||||
return findings
|
||||
}
|
||||
|
||||
func isServiceUnsynced(v ConsistencyView) bool {
|
||||
return len(v.Presets) == 0 && len(v.Recents) == 0 && len(v.Sources) == 0
|
||||
}
|
||||
|
||||
func isSpeakerEmpty(v ConsistencyView) bool {
|
||||
return len(v.Presets) == 0 && len(v.Recents) == 0 && len(v.Sources) == 0
|
||||
}
|
||||
|
||||
func issuesToFindings(target Target, issues []ConsistencyIssue, severity Severity) []Finding {
|
||||
if len(issues) == 0 {
|
||||
return nil
|
||||
@@ -177,10 +202,35 @@ func loadServiceView(ds *datastore.DataStore, account, deviceID string) (Consist
|
||||
|
||||
view := ConsistencyView{Label: "service"}
|
||||
|
||||
// Build sourceID -> SourceKeyType index up front so we can resolve
|
||||
// the *effective* source for each preset/recent. syncPresets and
|
||||
// syncRecents in sync.go currently persist the upstream
|
||||
// FullResponseSource.Type ("Audio") into ServicePreset.Source,
|
||||
// which is meaningless next to the speaker's actual source name.
|
||||
// Resolve via SourceID so the cross-side diff compares like with
|
||||
// like; fall back to the persisted Source string when the SourceID
|
||||
// is empty or doesn't resolve.
|
||||
sourceByID := make(map[string]string, len(sources))
|
||||
for i := range sources {
|
||||
if sources[i].ID != "" && sources[i].SourceKeyType != "" {
|
||||
sourceByID[sources[i].ID] = sources[i].SourceKeyType
|
||||
}
|
||||
}
|
||||
|
||||
resolveSource := func(persisted, sourceID string) string {
|
||||
if sourceID != "" {
|
||||
if resolved := sourceByID[sourceID]; resolved != "" {
|
||||
return resolved
|
||||
}
|
||||
}
|
||||
|
||||
return persisted
|
||||
}
|
||||
|
||||
for i := range presets {
|
||||
view.Presets = append(view.Presets, ConsistencyPreset{
|
||||
Slot: presets[i].ButtonNumber,
|
||||
Source: presets[i].Source,
|
||||
Source: resolveSource(presets[i].Source, presets[i].SourceID),
|
||||
SourceID: presets[i].SourceID,
|
||||
Location: presets[i].Location,
|
||||
Name: presets[i].Name,
|
||||
@@ -190,7 +240,7 @@ func loadServiceView(ds *datastore.DataStore, account, deviceID string) (Consist
|
||||
for i := range recents {
|
||||
view.Recents = append(view.Recents, ConsistencyRecent{
|
||||
ID: recents[i].ID,
|
||||
Source: recents[i].Source,
|
||||
Source: resolveSource(recents[i].Source, recents[i].SourceID),
|
||||
SourceID: recents[i].SourceID,
|
||||
Location: recents[i].Location,
|
||||
Name: recents[i].Name,
|
||||
|
||||
@@ -71,13 +71,11 @@ type ConsistencyIssueKind string
|
||||
|
||||
// Recognised ConsistencyIssueKind values.
|
||||
const (
|
||||
IssuePresetMismatch ConsistencyIssueKind = "preset_mismatch"
|
||||
IssueRecentMismatch ConsistencyIssueKind = "recent_mismatch"
|
||||
IssueSourceMismatch ConsistencyIssueKind = "source_mismatch"
|
||||
IssueDanglingPreset ConsistencyIssueKind = "dangling_preset"
|
||||
IssueDanglingRecent ConsistencyIssueKind = "dangling_recent"
|
||||
IssueDuplicateSource ConsistencyIssueKind = "duplicate_source"
|
||||
IssueMissingSpeakerSide ConsistencyIssueKind = "missing_speaker_side"
|
||||
IssuePresetMismatch ConsistencyIssueKind = "preset_mismatch"
|
||||
IssueRecentMismatch ConsistencyIssueKind = "recent_mismatch"
|
||||
IssueDanglingPreset ConsistencyIssueKind = "dangling_preset"
|
||||
IssueDanglingRecent ConsistencyIssueKind = "dangling_recent"
|
||||
IssueDuplicateSource ConsistencyIssueKind = "duplicate_source"
|
||||
)
|
||||
|
||||
// CheckCrossSide compares speaker and service views for the same device
|
||||
@@ -155,38 +153,33 @@ func CheckCrossSide(speaker, service ConsistencyView) []ConsistencyIssue {
|
||||
}
|
||||
}
|
||||
|
||||
for id, sv := range serviceRecents {
|
||||
if _, ok := speakerRecents[id]; !ok {
|
||||
// Service-only recents are common (the speaker drops old
|
||||
// entries faster than we do), so only flag if there are
|
||||
// disproportionately many.
|
||||
_ = sv
|
||||
speakerOnly := 0
|
||||
|
||||
for id := range speakerRecents {
|
||||
if _, ok := serviceRecents[id]; !ok {
|
||||
speakerOnly++
|
||||
}
|
||||
}
|
||||
|
||||
// Sources: compare by type (speaker lists by type, not by numeric ID).
|
||||
speakerSourceTypes := sourceTypesFromView(speaker)
|
||||
serviceSourceTypes := sourceTypesFromView(service)
|
||||
|
||||
for t := range speakerSourceTypes {
|
||||
if !serviceSourceTypes[t] {
|
||||
issues = append(issues, ConsistencyIssue{
|
||||
Kind: IssueSourceMismatch,
|
||||
Side: "speaker",
|
||||
Detail: "source type " + safeQuote(t) + " present on speaker but not in service Sources.xml — service /full will not include presets referencing this source",
|
||||
})
|
||||
}
|
||||
// Speaker-only recents are normal: the speaker keeps a longer
|
||||
// history than the service ingests. Surface a single summary line
|
||||
// when the gap is large enough to look like a missed sync, instead
|
||||
// of one finding per missing id (which used to drown the report).
|
||||
if speakerOnly >= 5 {
|
||||
issues = append(issues, ConsistencyIssue{
|
||||
Kind: IssueRecentMismatch,
|
||||
Side: "speaker",
|
||||
Detail: strconv.Itoa(speakerOnly) + " recent(s) present on speaker but missing from service — usually means service hasn't ingested the latest /recents notification; harmless unless the gap keeps growing",
|
||||
})
|
||||
}
|
||||
|
||||
for t := range serviceSourceTypes {
|
||||
if !speakerSourceTypes[t] {
|
||||
issues = append(issues, ConsistencyIssue{
|
||||
Kind: IssueSourceMismatch,
|
||||
Side: "service",
|
||||
Detail: "source type " + safeQuote(t) + " in service Sources.xml but not advertised by speaker /sources — preset references to this source may fail at play time",
|
||||
})
|
||||
}
|
||||
}
|
||||
// Note: we deliberately do *not* compare speaker /sources types
|
||||
// against service Sources.xml. Those two lists answer different
|
||||
// questions: speaker /sources enumerates local I/O sources (AUX,
|
||||
// BLUETOOTH, AIRPLAY, QPLAY, …) plus active-account ones (SPOTIFY),
|
||||
// while service Sources.xml tracks credentialed streaming sources
|
||||
// (TUNEIN, INTERNET_RADIO, …). They legitimately don't overlap on
|
||||
// most types, so flagging the asymmetry was pure noise.
|
||||
|
||||
sort.SliceStable(issues, func(i, j int) bool {
|
||||
return issues[i].Detail < issues[j].Detail
|
||||
@@ -201,6 +194,16 @@ func CheckCrossSide(speaker, service ConsistencyView) []ConsistencyIssue {
|
||||
// 10004" class (GH-269 NorbertBauer's restore case after a partial backup
|
||||
// import).
|
||||
//
|
||||
// Only meaningful on the service side. The speaker's /sources lists local
|
||||
// I/O sources (AUX, BLUETOOTH, AIRPLAY, ALEXA, QPLAY, UPNP, …) plus
|
||||
// active-account ones; it does *not* enumerate streaming sources
|
||||
// (TUNEIN, INTERNET_RADIO, …) — those are proxied through BMX. So a
|
||||
// preset with source="TUNEIN" on the speaker side legitimately has no
|
||||
// matching entry in speaker /sources, and flagging it as "dangling"
|
||||
// would be a false positive. Callers should pass a service-side view
|
||||
// here; speaker-side internal consistency is enforced by the firmware
|
||||
// itself.
|
||||
//
|
||||
// The side string is "speaker" or "service" — included verbatim in each
|
||||
// finding so the operator can tell which side is internally inconsistent.
|
||||
func CheckInternalConsistency(view ConsistencyView) []ConsistencyIssue {
|
||||
@@ -208,7 +211,8 @@ func CheckInternalConsistency(view ConsistencyView) []ConsistencyIssue {
|
||||
|
||||
sourceIDs := map[string]bool{}
|
||||
sourceTypes := map[string]bool{}
|
||||
dupCount := map[string]int{}
|
||||
dupKey := map[string]int{}
|
||||
dupLabel := map[string]string{}
|
||||
|
||||
for _, s := range view.Sources {
|
||||
if s.ID != "" {
|
||||
@@ -217,16 +221,24 @@ func CheckInternalConsistency(view ConsistencyView) []ConsistencyIssue {
|
||||
|
||||
if s.Type != "" {
|
||||
sourceTypes[s.Type] = true
|
||||
dupCount[s.Type]++
|
||||
|
||||
// Multiple <sourceItem> entries with the same source type
|
||||
// but different sourceAccount are normal (e.g. Spotify
|
||||
// Connect + Spotify Alexa, QPlay1 + QPlay2). Key dedup by
|
||||
// type+account so we only flag *true* duplicates that
|
||||
// would shadow each other in mapPresetsToFullResponse.
|
||||
key := s.Type + "\x00" + s.Account
|
||||
dupKey[key]++
|
||||
dupLabel[key] = s.Type + accountSuffix(s.Account)
|
||||
}
|
||||
}
|
||||
|
||||
for t, n := range dupCount {
|
||||
for key, n := range dupKey {
|
||||
if n > 1 {
|
||||
issues = append(issues, ConsistencyIssue{
|
||||
Kind: IssueDuplicateSource,
|
||||
Side: view.Label,
|
||||
Detail: view.Label + ": source type " + safeQuote(t) + " has " + plural(n, "entry", "entries") + " in Sources.xml — mapPresetsToFullResponse picks the first match, the rest are inert",
|
||||
Detail: view.Label + ": source " + safeQuote(dupLabel[key]) + " has " + plural(n, "entry", "entries") + " in Sources.xml — mapPresetsToFullResponse picks the first match, the rest are inert",
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -268,6 +280,14 @@ func CheckInternalConsistency(view ConsistencyView) []ConsistencyIssue {
|
||||
return issues
|
||||
}
|
||||
|
||||
func accountSuffix(account string) string {
|
||||
if account == "" {
|
||||
return ""
|
||||
}
|
||||
|
||||
return " (account " + account + ")"
|
||||
}
|
||||
|
||||
func indexPresetsBySlot(in []ConsistencyPreset) map[string]ConsistencyPreset {
|
||||
out := make(map[string]ConsistencyPreset, len(in))
|
||||
for i := range in {
|
||||
@@ -290,18 +310,6 @@ func indexRecentsByID(in []ConsistencyRecent) map[string]ConsistencyRecent {
|
||||
return out
|
||||
}
|
||||
|
||||
func sourceTypesFromView(v ConsistencyView) map[string]bool {
|
||||
out := map[string]bool{}
|
||||
|
||||
for _, s := range v.Sources {
|
||||
if s.Type != "" {
|
||||
out[s.Type] = true
|
||||
}
|
||||
}
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
// presetFieldsCompatible is strict on Source attribute (GH-343 footprint
|
||||
// reproduces here as a cross-side mismatch). Location is compared only
|
||||
// when both sides provide it; the speaker's /presets always does, the
|
||||
|
||||
@@ -68,6 +68,7 @@ func TestCheckInternalConsistency_DuplicateSourceType(t *testing.T) {
|
||||
view := ConsistencyView{
|
||||
Label: "service",
|
||||
Sources: []ConsistencySource{
|
||||
// Same type + same (empty) account — true shadow duplicate.
|
||||
{ID: "10004", Type: "TUNEIN"},
|
||||
{ID: "14774275", Type: "TUNEIN"},
|
||||
},
|
||||
@@ -88,6 +89,29 @@ func TestCheckInternalConsistency_DuplicateSourceType(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckInternalConsistency_MultipleAccountsAreNotDuplicates locks in
|
||||
// the dedup-by-type+account policy: two Spotify entries with different
|
||||
// sourceAccount (Connect, Alexa) are legitimate, not shadow duplicates.
|
||||
func TestCheckInternalConsistency_MultipleAccountsAreNotDuplicates(t *testing.T) {
|
||||
view := ConsistencyView{
|
||||
Label: "speaker",
|
||||
Sources: []ConsistencySource{
|
||||
{Type: "SPOTIFY", Account: "SpotifyConnectUserName"},
|
||||
{Type: "SPOTIFY", Account: "SpotifyAlexaUserName"},
|
||||
{Type: "QPLAY", Account: "QPlay1UserName"},
|
||||
{Type: "QPLAY", Account: "QPlay2UserName"},
|
||||
},
|
||||
}
|
||||
|
||||
got := CheckInternalConsistency(view)
|
||||
|
||||
for _, iss := range got {
|
||||
if iss.Kind == IssueDuplicateSource {
|
||||
t.Errorf("did not expect duplicate-source finding for distinct accounts; got: %s", iss.Detail)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckCrossSide_PresetSourceMismatch_GH343(t *testing.T) {
|
||||
// Speaker reports TUNEIN; service reports RADIOPLAYER — the
|
||||
// GH-343 footprint, where /full rebinding silently rewrote the
|
||||
|
||||
Reference in New Issue
Block a user