From e678f2a31d8ac1fef7597ddc967bc828a081b88e Mon Sep 17 00:00:00 2001
From: Darell Tan <darell.tan@gmail.com>
Date: Fri, 22 Nov 2024 03:40:27 +0800
Subject: [PATCH] Only process EndDevices during add and more verbose errors

If a Coordinator is present in the device list, AddDevicesFromJSON will
fail catastrophically, which shouldn't happen. Therefore, make sure only
EndDevices are considered during add. Also updated the tests to check
for this.

Added a device descriptor for failed adds. This will help with
identifying which device failed (and perhaps, why).
---
 bridge.go      | 11 +++++++++--
 bridge_test.go | 43 +++++++++++++++++++++++++++++++------------
 z2m.go         | 15 ++++++++-------
 3 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/bridge.go b/bridge.go
index 424aaf9..1f8af6c 100644
--- a/bridge.go
+++ b/bridge.go
@@ -442,6 +442,12 @@ func (br *Bridge) accessories() []*accessory.A {
 	return acc
 }
 
+func deviceJsonDescriptor(d Device) []byte {
+	d.Definition = nil
+	j, _ := json.Marshal(d)
+	return j
+}
+
 // Creates and calls AddDevice() based on the JSON definitions from zigbee2mqtt/bridge/devices.
 func (br *Bridge) AddDevicesFromJSON(devJson []byte) error {
 	var devices []Device
@@ -458,12 +464,13 @@ func (br *Bridge) AddDevicesFromJSON(devJson []byte) error {
 			if err == ErrDeviceSkipped || err == ErrUnknownDeviceType {
 				continue
 			}
-			return err
+			return fmt.Errorf("createAccessory failed: %+v %s", err, deviceJsonDescriptor(dev))
+
 		}
 
 		err = br.AddDevice(&dev, acc, exp)
 		if err != nil {
-			return err
+			return fmt.Errorf("AddDevice failed: %+v %s", err, deviceJsonDescriptor(dev))
 		}
 	}
 
diff --git a/bridge_test.go b/bridge_test.go
index 294562b..d3de96d 100644
--- a/bridge_test.go
+++ b/bridge_test.go
@@ -17,6 +17,7 @@ const ContactSensorTemplate = `
         "interviewing": false,
 		"disabled": false,
         "supported": true,
+		"type": "EndDevice",
 		"definition": {
 			"exposes": [
                 {
@@ -41,28 +42,24 @@ func fileSize(path string) (int64, error) {
 }
 
 func TestBridgePersistState(t *testing.T) {
-	dir, err := os.MkdirTemp("", "hapz2m-bridge*")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(dir)
-
-	var m sync.Map
-	ctx := context.Background()
-
-	b := NewBridge(ctx, dir)
+	dir := t.TempDir()
+	b := NewBridge(context.Background(), dir)
 
 	// devices
 	s1 := fmt.Appendf(nil, ContactSensorTemplate, 10)
 	s2 := fmt.Appendf(nil, ContactSensorTemplate, 20)
 
-	err = b.AddDevicesFromJSON(fmt.Appendf(nil, "[%s, %s]", s1, s2))
+	err := b.AddDevicesFromJSON(fmt.Appendf(nil, "[%s, %s]", s1, s2))
 	if err != nil {
 		t.Fatalf("cannot add devices: %v", err)
 	}
+	if len(b.devices) != 2 {
+		t.Fatalf("devices not added to bridge!")
+	}
 
 	// empty state, should have no errors
 	t.Logf("loading from empty db")
+	var m sync.Map
 	if err := b.loadZ2MState(&m); err != nil {
 		t.Errorf("empty state load should not error: %v", err)
 	}
@@ -95,7 +92,7 @@ func TestBridgePersistState(t *testing.T) {
 	}
 
 	// re-create with less devices
-	b2 := NewBridge(ctx, dir)
+	b2 := NewBridge(context.Background(), t.TempDir())
 	b2.AddDevicesFromJSON(fmt.Appendf(nil, "[%s]", s1))
 
 	t.Logf("loading from initial db")
@@ -115,3 +112,25 @@ func TestBridgePersistState(t *testing.T) {
 	}
 
 }
+
+func TestBridgeAddCoordinator(t *testing.T) {
+	b := NewBridge(context.Background(), t.TempDir())
+
+	err := b.AddDevicesFromJSON([]byte(`
+		[{
+		"definition": null,
+		"disabled": false,
+		"endpoints": [{"foo": true}],
+		"friendly_name": "Coordinator",
+		"ieee_address": "0x0022222200000000",
+		"interview_completed": true,
+		"interviewing": false,
+		"network_address": 0,
+		"supported": true,
+		"type": "Coordinator"
+		}]
+	`))
+	if err != nil {
+		t.Fatalf("cannot add coordinator: %v", err)
+	}
+}
diff --git a/z2m.go b/z2m.go
index 694f3c5..a4b031d 100644
--- a/z2m.go
+++ b/z2m.go
@@ -76,7 +76,8 @@ func initExposeMappings(exposes ...*ExposeMapping) error {
 func createAccessory(dev *Device) (*accessory.A, []*ExposeMapping, error) {
 	if dev.Disabled ||
 		!dev.Supported ||
-		!dev.InterviewCompleted {
+		!dev.InterviewCompleted ||
+		dev.Type != "EndDevice" {
 		return nil, nil, ErrDeviceSkipped
 	}
 
@@ -278,14 +279,14 @@ type Device struct {
 	IEEEAddress        string `json:"ieee_address"`
 	InterviewCompleted bool   `json:"interview_completed"`
 	Interviewing       bool   `json:"interviewing"`
-	Manufacturer       string `json:"manufacturer"`
-	ModelId            string `json:"model_id"`
+	Manufacturer       string `json:"manufacturer,omitempty"`
+	ModelId            string `json:"model_id,omitempty"`
 	NetworkAddress     int    `json:"network_address"`
-	PowerSource        string `json:"power_source"`
-	SoftwareBuildId    string `json:"software_build_id"`
-	DateCode           string `json:"date_code"`
+	PowerSource        string `json:"power_source,omitempty"`
+	SoftwareBuildId    string `json:"software_build_id,omitempty"`
+	DateCode           string `json:"date_code,omitempty"`
 
-	Definition *DevDefinition `json:"definition"`
+	Definition *DevDefinition `json:"definition,omitempty"`
 
 	Disabled  bool   `json:"disabled"`
 	Supported bool   `json:"supported"`