diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 469b4f0..30f3047 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -463,6 +463,7 @@ func setupRouter(server *handlers.Server, pyProxy *httputil.ReverseProxy) *chi.M r.Route("/setup", func(r chi.Router) { r.Get("/devices", server.HandleListDiscoveredDevices) r.Post("/devices", server.HandleAddManualDevice) + r.Delete("/devices/{deviceId}", server.HandleRemoveDevice) r.Post("/discover", server.HandleTriggerDiscovery) r.Get("/discovery-status", server.HandleGetDiscoveryStatus) r.Get("/settings", server.HandleGetSettings) diff --git a/pkg/models/models.go b/pkg/models/models.go index 7d0bb31..83846e6 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -185,6 +185,7 @@ type ServiceDeviceInfo struct { IPAddress string `json:"ip_address" xml:"ipAddress"` Name string `json:"name" xml:"name"` DiscoveryMethod string `json:"discovery_method,omitempty"` + AccountID string `json:"account_id,omitempty"` } // CustomerSupportDevice represents device information for customer support purposes. diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index f94eae0..7bb62aa 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -134,6 +134,7 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) { accDevices := ds.listDevicesInAccount(dir, acc.Name()) for i := range accDevices { info := accDevices[i] + info.AccountID = acc.Name() key := info.DeviceID if key == "" { diff --git a/pkg/service/handlers/handlers_setup.go b/pkg/service/handlers/handlers_setup.go index 5b8e74b..ec0d751 100644 --- a/pkg/service/handlers/handlers_setup.go +++ b/pkg/service/handlers/handlers_setup.go @@ -64,6 +64,7 @@ func (s *Server) HandleAddManualDevice(w http.ResponseWriter, r *http.Request) { } s.handleDiscoveredDevice(d) + s.mergeOverlappingDevices() w.Header().Set("Content-Type", "application/json") @@ -92,6 +93,53 @@ func (s *Server) HandleGetDiscoveryStatus(w http.ResponseWriter, _ *http.Request } } +// HandleRemoveDevice removes a device from the datastore. +func (s *Server) HandleRemoveDevice(w http.ResponseWriter, r *http.Request) { + deviceId := chi.URLParam(r, "deviceId") + if deviceId == "" { + http.Error(w, "Device ID is required", http.StatusBadRequest) + return + } + + // For now we assume a default account if not specified, + // or we might need to find which account this device belongs to. + // Looking at DataStore.ListAllDevices, it returns models.ServiceDeviceInfo which has DeviceID. + + devices, err := s.ds.ListAllDevices() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + var found bool + + for i := range devices { + if devices[i].DeviceID == deviceId { + err = s.ds.RemoveDevice(devices[i].AccountID, devices[i].DeviceID) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + found = true + + break + } + } + + if !found { + http.Error(w, "Device not found", http.StatusNotFound) + return + } + + w.Header().Set("Content-Type", "application/json") + + if err := json.NewEncoder(w).Encode(map[string]bool{"ok": true}); err != nil { + http.Error(w, "Failed to encode response", http.StatusInternalServerError) + return + } +} + // HandleGetSettings returns the current service settings. func (s *Server) HandleGetSettings(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/pkg/service/handlers/handlers_setup_test.go b/pkg/service/handlers/handlers_setup_test.go index 8a691d8..c9d6534 100644 --- a/pkg/service/handlers/handlers_setup_test.go +++ b/pkg/service/handlers/handlers_setup_test.go @@ -249,6 +249,95 @@ func TestMigrationAndCA(t *testing.T) { } } +func TestRemoveDevice(t *testing.T) { + tempDir, err := os.MkdirTemp("", "remove-device-test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + // Setup a dummy device in the datastore + account := "test-account" + deviceID := "TEST-DEVICE-ID" + deviceDir := filepath.Join(tempDir, account, "devices", deviceID) + if err := os.MkdirAll(deviceDir, 0755); err != nil { + t.Fatalf("Failed to create device dir: %v", err) + } + + infoFile := filepath.Join(deviceDir, "DeviceInfo.xml") + infoXML := `Test DeviceSoundTouch 10` + if err := os.WriteFile(infoFile, []byte(infoXML), 0644); err != nil { + t.Fatalf("Failed to create device info file: %v", err) + } + + r, _ := setupRouter("http://localhost:8001", ds) + ts := httptest.NewServer(r) + defer ts.Close() + + // 1. Verify device exists + res, err := http.Get(ts.URL + "/setup/devices") + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + + var devices []map[string]interface{} + if err := json.NewDecoder(res.Body).Decode(&devices); err != nil { + t.Fatalf("Failed to decode devices: %v", err) + } + + found := false + for _, d := range devices { + if d["device_id"] == deviceID { + found = true + break + } + } + if !found { + t.Fatalf("Device not found in list before removal") + } + + // 2. Remove device + req, err := http.NewRequest(http.MethodDelete, ts.URL+"/setup/devices/"+deviceID, nil) + if err != nil { + t.Fatal(err) + } + res, err = http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + t.Errorf("Expected status OK, got %v", res.Status) + } + + // 3. Verify device is gone + res, err = http.Get(ts.URL + "/setup/devices") + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + + if err := json.NewDecoder(res.Body).Decode(&devices); err != nil { + t.Fatalf("Failed to decode devices after removal: %v", err) + } + + for _, d := range devices { + if d["device_id"] == deviceID { + t.Errorf("Device still exists in list after removal") + } + } + + // 4. Verify directory is gone + if _, err := os.Stat(deviceDir); !os.IsNotExist(err) { + t.Errorf("Device directory still exists after removal") + } +} + type mockSSH struct{} func (m *mockSSH) Run(command string) (string, error) { diff --git a/pkg/service/handlers/main_test.go b/pkg/service/handlers/main_test.go index 38ebecf..75c7d5d 100644 --- a/pkg/service/handlers/main_test.go +++ b/pkg/service/handlers/main_test.go @@ -47,6 +47,8 @@ func setupRouter(targetURL string, ds *datastore.DataStore) (*chi.Mux, *Server) // Setup Setup for tests r.Route("/setup", func(r chi.Router) { + r.Get("/devices", server.HandleListDiscoveredDevices) + r.Delete("/devices/{deviceId}", server.HandleRemoveDevice) r.Get("/settings", server.HandleGetSettings) r.Post("/settings", server.HandleUpdateSettings) r.Get("/proxy-settings", server.HandleGetProxySettings) diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index 1bc6ea6..9df66b4 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -141,12 +141,15 @@ func (s *Server) DiscoverDevices(ctx context.Context) { for _, d := range devices { s.handleDiscoveredDevice(*d) } + + // Post-discovery cleanup: merge overlapping IP/Serial entries + s.mergeOverlappingDevices() } func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) { log.Printf("Discovered Bose device: %s at %s (Serial: %s)", d.Name, d.Host, d.SerialNo) - // 1. Check if we already have this device by serial number (best identifier) + // 1. Check if we already have this device existingID := s.findExistingDeviceID(d) // Use SerialNo if available, otherwise fallback to IP for the datastore directory name @@ -187,15 +190,83 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) { } } +func (s *Server) mergeOverlappingDevices() { + allDevices, err := s.ds.ListAllDevices() + if err != nil { + return + } + + // Group devices by IP + byIP := make(map[string][]models.ServiceDeviceInfo) + + for i := range allDevices { + dev := allDevices[i] + if dev.IPAddress != "" { + byIP[dev.IPAddress] = append(byIP[dev.IPAddress], dev) + } + } + + for ip, devices := range byIP { + if len(devices) <= 1 { + continue + } + + // We have multiple entries for the same IP. + // Try to find one with a Serial Number to be the master. + var master *models.ServiceDeviceInfo + + for i := range devices { + if devices[i].DeviceSerialNumber != "" || devices[i].DeviceID != "" { + master = &devices[i] + break + } + } + + if master == nil { + // None have serials, just keep the first one (already handled by ListAllDevices unique check usually, + // but ListAllDevices might see different AccountIDs or directories) + continue + } + + masterID := master.DeviceID + if masterID == "" { + masterID = master.DeviceSerialNumber + } + + for i := range devices { + dev := devices[i] + devID := dev.DeviceID + + if devID == "" { + devID = dev.IPAddress + } + + if devID != masterID && dev.IPAddress == ip { + log.Printf("Merging overlapping device entry %s into %s (IP: %s)", devID, masterID, ip) + _ = s.ds.RemoveDevice(dev.AccountID, devID) + } + } + } +} + func (s *Server) findExistingDeviceID(d models.DiscoveredDevice) string { allDevices, _ := s.ds.ListAllDevices() for i := range allDevices { known := allDevices[i] + // Match by Serial if d.SerialNo != "" && (known.DeviceID == d.SerialNo || known.DeviceSerialNumber == d.SerialNo) { if known.DeviceID != "" { return known.DeviceID } + return known.IPAddress + } + // Match by IP + if d.Host != "" && known.IPAddress == d.Host { + if known.DeviceID != "" { + return known.DeviceID + } + return known.IPAddress } } diff --git a/pkg/service/handlers/server_merge_test.go b/pkg/service/handlers/server_merge_test.go new file mode 100644 index 0000000..53fd913 --- /dev/null +++ b/pkg/service/handlers/server_merge_test.go @@ -0,0 +1,120 @@ +package handlers + +import ( + "os" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/models" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" +) + +func TestMergeOverlappingDevices(t *testing.T) { + tempDir, err := os.MkdirTemp("", "merge-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + s := &Server{ds: ds} + + // Case 1: IP-only entry and Serial-based entry for the same IP + ip := "192.168.1.100" + serial := "SERIAL123" + + // 1. Save IP-based entry + infoIP := &models.ServiceDeviceInfo{ + Name: "Speaker IP", + IPAddress: ip, + } + err = ds.SaveDeviceInfo("default", ip, infoIP) + if err != nil { + t.Fatalf("Failed to save IP info: %v", err) + } + + // 2. Save Serial-based entry + infoSerial := &models.ServiceDeviceInfo{ + DeviceID: serial, + DeviceSerialNumber: serial, + Name: "Speaker Serial", + IPAddress: ip, + } + err = ds.SaveDeviceInfo("default", serial, infoSerial) + if err != nil { + t.Fatalf("Failed to save Serial info: %v", err) + } + + // Verify both exist + devices, _ := ds.ListAllDevices() + if len(devices) != 2 { + t.Fatalf("Expected 2 devices before merge, got %d", len(devices)) + } + + // Run merge + s.mergeOverlappingDevices() + + // Verify merge + devices, _ = ds.ListAllDevices() + if len(devices) != 1 { + t.Fatalf("Expected 1 device after merge, got %d", len(devices)) + } + + if devices[0].DeviceID != serial { + t.Errorf("Expected remaining device to be Serial-based (%s), got %s", serial, devices[0].DeviceID) + } +} + +func TestFindExistingDeviceID(t *testing.T) { + tempDir, _ := os.MkdirTemp("", "find-test-*") + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + s := &Server{ds: ds} + + ip := "192.168.1.101" + serial := "SERIAL456" + + // Save IP-based + ds.SaveDeviceInfo("default", ip, &models.ServiceDeviceInfo{ + IPAddress: ip, + Name: "IP Speaker", + }) + + // Test finding by IP + foundID := s.findExistingDeviceID(models.DiscoveredDevice{ + Host: ip, + }) + if foundID != ip { + t.Errorf("Expected to find by IP, got %s", foundID) + } + + // Save Serial-based for SAME IP + ds.SaveDeviceInfo("default", serial, &models.ServiceDeviceInfo{ + DeviceID: serial, + DeviceSerialNumber: serial, + IPAddress: ip, + Name: "Serial Speaker", + }) + + // Test finding by IP should now return Serial (if Serial is known) + // Actually findExistingDeviceID returns the first match it finds in allDevices. + // Since we haven't merged yet, it could be either. + + // Test finding by Serial + foundID = s.findExistingDeviceID(models.DiscoveredDevice{ + Host: ip, + SerialNo: serial, + }) + if foundID != serial && foundID != ip { + t.Errorf("Expected to find by Serial or IP, got %s", foundID) + } + + // Merge and check again + s.mergeOverlappingDevices() + foundID = s.findExistingDeviceID(models.DiscoveredDevice{ + Host: ip, + }) + if foundID != serial { + t.Errorf("After merge, expected to find Serial ID for IP, got %s", foundID) + } +} diff --git a/pkg/service/handlers/web/css/style.css b/pkg/service/handlers/web/css/style.css index 328f5b8..eddc047 100644 --- a/pkg/service/handlers/web/css/style.css +++ b/pkg/service/handlers/web/css/style.css @@ -66,3 +66,13 @@ pre { background-color: #eee; padding: 10px; overflow-x: auto; font-size: 12px; .info-box a:hover { text-decoration: none; } + +.btn-danger { + background-color: #f44336; + color: white; + border: none; + padding: 5px 10px; +} +.btn-danger:hover { + background-color: #d32f2f; +} diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 6ca59e1..c5e1191 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -113,6 +113,7 @@ async function fetchDevices() { + `; @@ -265,6 +266,27 @@ async function addManualDevice() { } } +async function removeDevice(deviceId, name) { + if (!confirm(`Are you sure you want to remove device "${name}"?`)) { + return; + } + + try { + const response = await fetch(`/setup/devices/${deviceId}`, { + method: 'DELETE' + }); + + if (response.ok) { + fetchDevices(); + } else { + const err = await response.text(); + alert('Failed to remove device: ' + err); + } + } catch (error) { + alert('Error removing device: ' + error.message); + } +} + async function triggerDiscovery() { const indicator = document.getElementById('discovery-indicator'); if (indicator) indicator.style.display = 'inline';