diff --git a/pkg/service/ding/ding.go b/pkg/service/ding/ding.go index 50697c6..bda0e8c 100644 --- a/pkg/service/ding/ding.go +++ b/pkg/service/ding/ding.go @@ -173,6 +173,7 @@ func Render(opts Options) []byte { // instance is audible even if the first is missed. if opts.Repeat > 1 { repeatGapN := int(math.Round(float64(opts.SampleRate) * opts.RepeatGapDuration)) + baseLeft := append([]float64{}, left...) baseRight := append([]float64{}, right...) silence := make([]float64, repeatGapN) diff --git a/pkg/service/handlers/handlers_health_checks.go b/pkg/service/handlers/handlers_health_checks.go index c44399d..71017a5 100644 --- a/pkg/service/handlers/handlers_health_checks.go +++ b/pkg/service/handlers/handlers_health_checks.go @@ -27,6 +27,10 @@ type healthFixRequest struct { type healthFixResponse struct { OK bool `json:"ok"` Message string `json:"message,omitempty"` + // Refresh tells the UI whether to re-fetch health after this fix. + // false for persistent affordances (e.g. play_ding) that don't + // change any check state, so no "Loading…" flash occurs. + Refresh bool `json:"refresh"` } // HandleHealthChecks runs every registered health check and @@ -73,7 +77,7 @@ func (s *Server) HandleHealthFix(w http.ResponseWriter, r *http.Request) { return } - msg, err := s.healthRegistry.RunFix(req.CheckID, req.FixID, req.Target) + msg, refresh, err := s.healthRegistry.RunFix(req.CheckID, req.FixID, req.Target) if err != nil { if errors.Is(err, health.ErrFixNotFound) { writeJSONError(w, http.StatusNotFound, err.Error()) @@ -87,7 +91,7 @@ func (s *Server) HandleHealthFix(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(healthFixResponse{OK: true, Message: msg}); err != nil { + if err := json.NewEncoder(w).Encode(healthFixResponse{OK: true, Message: msg, Refresh: refresh}); err != nil { http.Error(w, "Failed to encode response", http.StatusInternalServerError) return } diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 7331633..8bcb6e7 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -4368,8 +4368,10 @@ async function runQuickFix(checkId, fixId, target, confirmMsg, button) { status.textContent = data.message || "Done."; status.style.color = "#2e7d32"; } - // Refresh to drop the resolved finding. - setTimeout(fetchHealth, 400); + // Re-fetch health so resolved findings disappear from the list. + // Skipped when the server signals refresh:false (persistent + // affordances like play_ding that don't change check state). + if (data.refresh !== false) setTimeout(fetchHealth, 400); } catch (e) { if (status) { status.textContent = `Failed: ${e.message || e}`; diff --git a/pkg/service/health/checks_sources_test.go b/pkg/service/health/checks_sources_test.go index 2459e39..9ec4ab1 100644 --- a/pkg/service/health/checks_sources_test.go +++ b/pkg/service/health/checks_sources_test.go @@ -77,7 +77,7 @@ func TestSourcesXMLPresent_QuickFix_MaterialisesDefaults(t *testing.T) { r := NewRegistry() RegisterSourcesXMLPresent(r, ds) - msg, err := r.RunFix(CheckIDSourcesXMLPresent, FixIDCreateDefaultSources, Target{ + msg, _, err := r.RunFix(CheckIDSourcesXMLPresent, FixIDCreateDefaultSources, Target{ Account: account, Device: device, }) @@ -143,7 +143,7 @@ func TestSourcesXMLPresent_FixRejectsEmptyTarget(t *testing.T) { r := NewRegistry() RegisterSourcesXMLPresent(r, ds) - if _, err := r.RunFix(CheckIDSourcesXMLPresent, FixIDCreateDefaultSources, Target{}); err == nil { + if _, _, err := r.RunFix(CheckIDSourcesXMLPresent, FixIDCreateDefaultSources, Target{}); err == nil { t.Errorf("expected error for empty target, got nil") } } diff --git a/pkg/service/health/checks_test_playback.go b/pkg/service/health/checks_test_playback.go index afadcfe..19ebf8f 100644 --- a/pkg/service/health/checks_test_playback.go +++ b/pkg/service/health/checks_test_playback.go @@ -3,6 +3,7 @@ package health import ( "bytes" "context" + "encoding/base64" "fmt" "io" "net/http" @@ -28,6 +29,23 @@ const FixIDPlayDing = "play_ding" // pkg/service/handlers (see static/media embed). const DingMediaPath = "/media/aftertouch-ding.wav" +// DingCustomPath is the AfterTouch custom-playback prefix. The speaker +// fetches this URL, AfterTouch responds with a BMX JSON payload, and the +// speaker plays via LOCAL_INTERNET_RADIO — avoiding the INTERNET_RADIO +// FLAC-parser path that causes UNKNOWN_SOURCE_ERROR (1005) on some firmware +// versions (see issue #345). +const DingCustomPath = "/custom/v1/playback/" + +// dingCustomURL builds the LOCAL_INTERNET_RADIO proxy URL for the ding WAV. +// The WAV URL is base64url-encoded into the path; the name query param sets +// the display name on the speaker. +func dingCustomURL(serverURL string) string { + mediaURL := serverURL + DingMediaPath + encoded := base64.URLEncoding.EncodeToString([]byte(mediaURL)) + + return serverURL + DingCustomPath + encoded + "?name=AfterTouch+ding" +} + // RegisterTestPlaybackCheck registers the playback_test check and // its play_ding quick fix. serverURLFn returns the externally // reachable URL of this service — the speaker fetches the audio @@ -42,7 +60,10 @@ func RegisterTestPlaybackCheck(r *Registry, ds *datastore.DataStore, serverURLFn }, }) - r.RegisterFix(CheckIDTestPlayback, FixIDPlayDing, func(target Target) (string, error) { + // play_ding is a persistent operator affordance, not a resolvable + // finding — success doesn't change any check state, so the UI + // should not re-fetch health afterwards (no "Loading…" flash). + r.RegisterFixNoRefresh(CheckIDTestPlayback, FixIDPlayDing, func(target Target) (string, error) { return playDingOnDevice(ds, serverURLFn(), target) }) } @@ -80,7 +101,7 @@ func runTestPlaybackCheck(ds *datastore.DataStore, serverURL string) []Finding { Severity: SeverityInfo, Target: Target{Account: dev.AccountID, Device: dev.DeviceID}, Message: fmt.Sprintf("Play the AfterTouch ding on %s.", displayName(dev.Name, dev.DeviceID)), - Details: fmt.Sprintf("Pushes %s%s to the speaker via a custom-radio ContentItem. Confirms migration is healthy end-to-end without depending on TuneIn or any external service.", serverURL, DingMediaPath), + Details: fmt.Sprintf("Pushes the ding WAV to the speaker via LOCAL_INTERNET_RADIO (custom-playback proxy at %s%s). Confirms migration is healthy end-to-end without depending on TuneIn or any external service.", serverURL, DingCustomPath), QuickFixes: []QuickFix{{ ID: FixIDPlayDing, Label: "Play ding", @@ -114,8 +135,8 @@ func playDingOnDevice(ds *datastore.DataStore, serverURL string, target Target) return "", fmt.Errorf("device %s has no IP address recorded", target.Device) } - mediaURL := serverURL + DingMediaPath - contentItem := buildDingContentItem(mediaURL) + customURL := dingCustomURL(serverURL) + contentItem := buildDingContentItem(customURL) selectURL := fmt.Sprintf("http://%s:8090/select", dev.IPAddress) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) @@ -143,18 +164,23 @@ func playDingOnDevice(ds *datastore.DataStore, serverURL string, target Target) return fmt.Sprintf("Pushed ding URL to %s. You should hear it within a second.", displayName(dev.Name, target.Device)), nil } -func buildDingContentItem(mediaURL string) string { - escaped := xmlAttrEscape(mediaURL) +// buildDingContentItem returns the XML ContentItem that pushes the ding to a +// speaker. Uses LOCAL_INTERNET_RADIO with the AfterTouch custom-playback +// proxy URL so the speaker fetches a BMX JSON response and plays via the +// LOCAL_INTERNET_RADIO code path — avoiding the INTERNET_RADIO FLAC-parser +// issue that causes UNKNOWN_SOURCE_ERROR (1005) on some firmware versions +// (issue #345). +func buildDingContentItem(customURL string) string { + escaped := xmlAttrEscape(customURL) return fmt.Sprintf( - `AfterTouch ding`, + `AfterTouch ding`, escaped, ) } func dingCurlCommand(speakerIP, serverURL string) string { - mediaURL := serverURL + DingMediaPath - body := buildDingContentItem(mediaURL) + body := buildDingContentItem(dingCustomURL(serverURL)) return fmt.Sprintf( "curl -sS -X POST 'http://%s:8090/select' -H 'Content-Type: application/xml' -d '%s'", diff --git a/pkg/service/health/checks_test_playback_test.go b/pkg/service/health/checks_test_playback_test.go index 7d71a2c..ce08e64 100644 --- a/pkg/service/health/checks_test_playback_test.go +++ b/pkg/service/health/checks_test_playback_test.go @@ -118,24 +118,31 @@ func TestPlayDing_PostsContentItemToSelectEndpoint(t *testing.T) { // reuse it directly with httptest. Test the building blocks // (ContentItem rendering + the curl-command form) here, and // leave the full POST plumbing for a manual smoke test. - mediaURL := "http://aftertouch.local" + DingMediaPath - contentItem := buildDingContentItem(mediaURL) + const serverBase = "http://aftertouch.local" + customURL := dingCustomURL(serverBase) + contentItem := buildDingContentItem(customURL) - if !strings.Contains(contentItem, "source=\"INTERNET_RADIO\"") { - t.Errorf("ContentItem missing INTERNET_RADIO source, got %q", contentItem) + if !strings.Contains(contentItem, "source=\"LOCAL_INTERNET_RADIO\"") { + t.Errorf("ContentItem missing LOCAL_INTERNET_RADIO source, got %q", contentItem) } - if !strings.Contains(contentItem, mediaURL) { - t.Errorf("ContentItem missing media URL, got %q", contentItem) + if !strings.Contains(contentItem, DingCustomPath) { + t.Errorf("ContentItem missing custom-playback path, got %q", contentItem) } - cmd := dingCurlCommand("192.0.2.10", "http://aftertouch.local") + // The WAV URL is base64-encoded inside the custom URL — verify the + // custom URL itself is present in the ContentItem. + if !strings.Contains(contentItem, customURL) { + t.Errorf("ContentItem missing custom URL, got %q", contentItem) + } + + cmd := dingCurlCommand("192.0.2.10", serverBase) if !strings.Contains(cmd, "192.0.2.10:8090/select") { t.Errorf("curl command should target speaker /select, got %q", cmd) } - if !strings.Contains(cmd, "/media/aftertouch-ding.wav") { - t.Errorf("curl command should include the ding URL, got %q", cmd) + if !strings.Contains(cmd, DingCustomPath) { + t.Errorf("curl command should include the custom-playback path, got %q", cmd) } } diff --git a/pkg/service/health/health.go b/pkg/service/health/health.go index cd9bd36..bbf5017 100644 --- a/pkg/service/health/health.go +++ b/pkg/service/health/health.go @@ -114,18 +114,29 @@ type CheckResult struct { // registered under the (checkID, fixID) pair. var ErrFixNotFound = errors.New("quick fix not registered") +// fixEntry pairs a FixFunc with its refresh policy. refresh=true +// means the UI should re-run fetchHealth after the fix succeeds so +// resolved findings disappear from the list. refresh=false is used +// for persistent affordances (e.g. play_ding) that never change check +// state — no re-render is needed and the brief "Loading…" flash is +// avoided. +type fixEntry struct { + fn FixFunc + refresh bool +} + // Registry owns the set of checks and fixes for one Server // instance. The default zero value is not usable; construct via // NewRegistry. type Registry struct { mu sync.RWMutex checks []Check - fixes map[string]FixFunc // key: "/" + fixes map[string]fixEntry // key: "/" } // NewRegistry returns an empty Registry. func NewRegistry() *Registry { - return &Registry{fixes: map[string]FixFunc{}} + return &Registry{fixes: map[string]fixEntry{}} } // Register adds a check to the registry. Duplicate IDs replace @@ -147,12 +158,25 @@ func (r *Registry) Register(c Check) { // RegisterFix associates a FixFunc with the given (checkID, fixID) // pair. A QuickFix with that ID can be advertised by any Finding -// emitted by the matching check. +// emitted by the matching check. After a successful run the UI will +// re-fetch health so resolved findings disappear from the list. func (r *Registry) RegisterFix(checkID, fixID string, fn FixFunc) { r.mu.Lock() defer r.mu.Unlock() - r.fixes[fixKey(checkID, fixID)] = fn + r.fixes[fixKey(checkID, fixID)] = fixEntry{fn: fn, refresh: true} +} + +// RegisterFixNoRefresh is like RegisterFix but signals the UI that +// re-fetching health after the fix runs is unnecessary. Use this for +// persistent operator affordances (e.g. play_ding) whose success +// doesn't change any check state — skipping the re-fetch avoids a +// distracting "Loading…" flash with no benefit. +func (r *Registry) RegisterFixNoRefresh(checkID, fixID string, fn FixFunc) { + r.mu.Lock() + defer r.mu.Unlock() + + r.fixes[fixKey(checkID, fixID)] = fixEntry{fn: fn, refresh: false} } // RunAll executes every registered check and returns the results @@ -183,20 +207,22 @@ func (r *Registry) RunAll() []CheckResult { return out } -// RunFix dispatches to the FixFunc registered for (checkID, -// fixID). The returned string is forwarded as the user-facing -// success message. ErrFixNotFound is returned when no fix is -// registered. -func (r *Registry) RunFix(checkID, fixID string, target Target) (string, error) { +// RunFix dispatches to the FixFunc registered for (checkID, fixID). +// Returns the user-facing success message, whether the UI should +// re-fetch health afterwards, and any execution error. +// ErrFixNotFound is returned when no fix is registered. +func (r *Registry) RunFix(checkID, fixID string, target Target) (string, bool, error) { r.mu.RLock() - fn, ok := r.fixes[fixKey(checkID, fixID)] + entry, ok := r.fixes[fixKey(checkID, fixID)] r.mu.RUnlock() if !ok { - return "", fmt.Errorf("%w: %s/%s", ErrFixNotFound, checkID, fixID) + return "", false, fmt.Errorf("%w: %s/%s", ErrFixNotFound, checkID, fixID) } - return fn(target) + msg, err := entry.fn(target) + + return msg, entry.refresh, err } func fixKey(checkID, fixID string) string { diff --git a/pkg/service/health/health_test.go b/pkg/service/health/health_test.go index 9a9ba52..63bde3a 100644 --- a/pkg/service/health/health_test.go +++ b/pkg/service/health/health_test.go @@ -58,7 +58,7 @@ func TestRegistry_RunFix_Dispatch(t *testing.T) { return "applied", nil }) - msg, err := r.RunFix("c1", "f1", Target{Account: "A", Device: "D"}) + msg, refresh, err := r.RunFix("c1", "f1", Target{Account: "A", Device: "D"}) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -67,6 +67,10 @@ func TestRegistry_RunFix_Dispatch(t *testing.T) { t.Errorf("unexpected message: %q", msg) } + if !refresh { + t.Errorf("expected refresh=true for a fix registered via RegisterFix") + } + if captured.Account != "A" || captured.Device != "D" { t.Errorf("target not propagated to fix: %+v", captured) } @@ -75,7 +79,7 @@ func TestRegistry_RunFix_Dispatch(t *testing.T) { func TestRegistry_RunFix_NotFound(t *testing.T) { r := NewRegistry() - _, err := r.RunFix("nope", "also-nope", Target{}) + _, _, err := r.RunFix("nope", "also-nope", Target{}) if !errors.Is(err, ErrFixNotFound) { t.Errorf("expected ErrFixNotFound, got %v", err) }