Implement device merging logic and web-based device removal

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)

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.

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 == "" {

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")

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 := `<?xml version="1.0" encoding="UTF-8" ?><info deviceID="TEST-DEVICE-ID"><name>Test Device</name><type>SoundTouch 10</type></info>`
+	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) {

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)

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
 		}
 	}

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)
+	}
+}

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;
+}

pkg/service/handlers/web/js/script.js

@@ -113,6 +113,7 @@ async function fetchDevices() {
                         <td>
                             <button onclick="prepareSync('${d.ip_address}')">Sync Data</button>
                             <button onclick="prepareMigration('${d.ip_address}')">Migrate</button>
+                            <button class="btn-danger" onclick="removeDevice('${d.device_id}', '${d.name}')">Remove</button>
                         </td>
                     </tr>
                 `;
@@ -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';