mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
fix(health): skip fetchHealth re-render for non-resolving quick fixes
Add a refresh policy to the fix registry so the UI can avoid the
unnecessary "Loading…" flash when a quick fix does not change any
check state.
- Registry stores fixEntry{fn, refresh} instead of bare FixFunc.
- RegisterFix (existing callers) keeps refresh=true: resolved
findings disappear from the list after the fix runs.
- New RegisterFixNoRefresh sets refresh=false: used for persistent
operator affordances whose success leaves the finding unchanged.
- RunFix now returns (string, bool, error); the bool propagates to
the healthFixResponse JSON as "refresh".
- play_ding registered via RegisterFixNoRefresh — pressing it never
resolves the finding, so no re-fetch is needed.
- runQuickFix in script.js gates setTimeout(fetchHealth, 400) on
data.refresh !== false; absent or true keeps the existing behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
db33f7f22e
commit
04f7388051
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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}`;
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
`<ContentItem source="INTERNET_RADIO" type="stationurl" location="%s" sourceAccount="" isPresetable="false"><itemName>AfterTouch ding</itemName></ContentItem>`,
|
||||
`<ContentItem source="LOCAL_INTERNET_RADIO" type="stationurl" location="%s" sourceAccount="" isPresetable="true"><itemName>AfterTouch ding</itemName></ContentItem>`,
|
||||
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'",
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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: "<checkID>/<fixID>"
|
||||
fixes map[string]fixEntry // key: "<checkID>/<fixID>"
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user