Skip to content

Commit

Permalink
Persist device ID into storage, prevents flip flopping.
Browse files Browse the repository at this point in the history
  • Loading branch information
pwood committed Jun 24, 2024
1 parent 6a738c4 commit 82e410e
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 36 deletions.
27 changes: 13 additions & 14 deletions enumerate_device.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (

type deviceManager interface {
createNextDevice(*node) *device
setDeviceUniqueId(*device, int)
removeDevice(context.Context, IEEEAddressWithSubIdentifier) bool
attachCapabilityToDevice(d *device, c implcaps.ZDACapability)
detachCapabilityFromDevice(d *device, c implcaps.ZDACapability)
Expand Down Expand Up @@ -116,7 +117,7 @@ func (e enumerateDevice) enumerate(pctx context.Context, n *node) {
did := e.updateNodeTable(ctx, n, inventoryDevices)

for _, id := range inventoryDevices {
d := did[id.deviceId]
d := did[id.uniqueId]
errs := e.updateCapabilitiesOnDevice(ctx, d, id)

d.eda.m.Lock()
Expand Down Expand Up @@ -225,7 +226,7 @@ func (e enumerateDevice) runRules(inv inventory) (inventory, error) {
}

type inventoryDevice struct {
deviceId int
uniqueId int
endpoints []endpointDetails
}

Expand All @@ -234,7 +235,7 @@ func (e enumerateDevice) groupInventoryDevices(inv inventory) []inventoryDevice
var endpoints []int

for eid, ep := range inv.endpoints {
invDev := &inventoryDevice{deviceId: int(eid), endpoints: []endpointDetails{ep}}
invDev := &inventoryDevice{uniqueId: int(eid), endpoints: []endpointDetails{ep}}
devices[int(eid)] = invDev
endpoints = append(endpoints, int(eid))
}
Expand All @@ -256,7 +257,7 @@ func (e enumerateDevice) updateNodeTable(ctx context.Context, n *node, inventory
deviceIdMapping := map[int]*device{}
var unsetDevice []*device = nil

/* Look for devices that exist but don't have a deviceId. */
/* Look for devices that exist but don't have a uniqueId. */
n.m.RLock()
for _, d := range n.device {
d.m.RLock()
Expand All @@ -267,16 +268,16 @@ func (e enumerateDevice) updateNodeTable(ctx context.Context, n *node, inventory
}
n.m.RUnlock()

/* Find existing devices that match the deviceId. */
/* Find existing devices that match the uniqueId. */
n.m.RLock()
for _, i := range inventoryDevices {
for _, d := range n.device {
d.m.RLock()
devId := d.deviceId
d.m.RUnlock()

if devId == i.deviceId {
deviceIdMapping[i.deviceId] = d
if devId == i.uniqueId {
deviceIdMapping[i.uniqueId] = d
break
}
}
Expand All @@ -285,7 +286,7 @@ func (e enumerateDevice) updateNodeTable(ctx context.Context, n *node, inventory

/* Create new devices for those that are missing. */
for _, i := range inventoryDevices {
if _, found := deviceIdMapping[i.deviceId]; !found {
if _, found := deviceIdMapping[i.uniqueId]; !found {
var d *device

if len(unsetDevice) > 0 {
Expand All @@ -295,12 +296,10 @@ func (e enumerateDevice) updateNodeTable(ctx context.Context, n *node, inventory
d = e.dm.createNextDevice(n)
}

d.m.Lock()
d.deviceId = i.deviceId
d.deviceIdSet = true
d.m.Unlock()
deviceIdMapping[i.deviceId] = d
e.logger.LogTrace(ctx, "Added new device.", logwrap.Datum("DeviceId", i.deviceId), logwrap.Datum("NewIdentifier", d.Identifier().String()))
e.dm.setDeviceUniqueId(d, i.uniqueId)

deviceIdMapping[i.uniqueId] = d
e.logger.LogTrace(ctx, "Added new device.", logwrap.Datum("DeviceId", i.uniqueId), logwrap.Datum("NewIdentifier", d.Identifier().String()))
}
}

Expand Down
37 changes: 23 additions & 14 deletions enumerate_device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func Test_enumerateDevice_groupInventoryDevices(t *testing.T) {

expected := []inventoryDevice{
{
deviceId: 1,
uniqueId: 1,
endpoints: []endpointDetails{
{
description: zigbee.EndpointDescription{
Expand All @@ -288,7 +288,7 @@ func Test_enumerateDevice_groupInventoryDevices(t *testing.T) {
},
},
{
deviceId: 2,
uniqueId: 2,
endpoints: []endpointDetails{
{
description: zigbee.EndpointDescription{
Expand All @@ -299,7 +299,7 @@ func Test_enumerateDevice_groupInventoryDevices(t *testing.T) {
},
},
{
deviceId: 3,
uniqueId: 3,
endpoints: []endpointDetails{
{
description: zigbee.EndpointDescription{
Expand Down Expand Up @@ -327,6 +327,10 @@ func (m *mockDeviceManager) createNextDevice(n *node) *device {
return args.Get(0).(*device)
}

func (m *mockDeviceManager) setDeviceUniqueId(d *device, id int) {
_ = m.Called(d, id)
}

func (m *mockDeviceManager) removeDevice(ctx context.Context, i IEEEAddressWithSubIdentifier) bool {
args := m.Called(i)
return args.Bool(0)
Expand All @@ -349,20 +353,22 @@ func Test_enumerateDevice_updateNodeTable(t *testing.T) {
n := &node{m: &sync.RWMutex{}}
d := &device{m: &sync.RWMutex{}, capabilities: map[da.Capability]implcaps.ZDACapability{}}

mdm.On("createNextDevice", n).Return(d)

expectedDeviceId := 0x2000

mdm.On("createNextDevice", n).Return(d)
mdm.On("setDeviceUniqueId", d, expectedDeviceId).Run(func(args mock.Arguments) {
d.deviceId = expectedDeviceId
})

id := []inventoryDevice{
{
deviceId: expectedDeviceId,
uniqueId: expectedDeviceId,
},
}

mapping := ed.updateNodeTable(context.Background(), n, id)

assert.Equal(t, d, mapping[expectedDeviceId])
assert.Equal(t, expectedDeviceId, d.deviceId)
})

t.Run("returns an existing on in mapping if present", func(t *testing.T) {
Expand All @@ -377,7 +383,7 @@ func Test_enumerateDevice_updateNodeTable(t *testing.T) {

id := []inventoryDevice{
{
deviceId: existingDeviceId,
uniqueId: existingDeviceId,
},
}

Expand All @@ -387,7 +393,7 @@ func Test_enumerateDevice_updateNodeTable(t *testing.T) {
assert.Equal(t, existingDeviceId, d.deviceId)
})

t.Run("returns an existing an existing device that has its deviceId unset", func(t *testing.T) {
t.Run("returns an existing an existing device that has its uniqueId unset", func(t *testing.T) {
mdm := &mockDeviceManager{}
defer mdm.AssertExpectations(t)

Expand All @@ -399,14 +405,17 @@ func Test_enumerateDevice_updateNodeTable(t *testing.T) {

id := []inventoryDevice{
{
deviceId: existingDeviceId,
uniqueId: existingDeviceId,
},
}

mdm.On("setDeviceUniqueId", d, existingDeviceId).Run(func(args mock.Arguments) {
d.deviceId = existingDeviceId
})

mapping := ed.updateNodeTable(context.Background(), n, id)

assert.Equal(t, d, mapping[existingDeviceId])
assert.Equal(t, existingDeviceId, d.deviceId)
})

t.Run("removes an device that should not be present", func(t *testing.T) {
Expand Down Expand Up @@ -479,7 +488,7 @@ func Test_enumerateDevice_updateCapabilitiesOnDevice(t *testing.T) {
d := &device{m: &sync.RWMutex{}, deviceId: 1, capabilities: map[da.Capability]implcaps.ZDACapability{}}

id := inventoryDevice{
deviceId: 1,
uniqueId: 1,
endpoints: []endpointDetails{
{
rulesOutput: rules.Output{
Expand Down Expand Up @@ -521,7 +530,7 @@ func Test_enumerateDevice_updateCapabilitiesOnDevice(t *testing.T) {
})

id := inventoryDevice{
deviceId: 1,
uniqueId: 1,
endpoints: []endpointDetails{
{
rulesOutput: rules.Output{
Expand Down Expand Up @@ -562,7 +571,7 @@ func Test_enumerateDevice_updateCapabilitiesOnDevice(t *testing.T) {
d.capabilities[capabilities.ProductInformationFlag] = opi

id := inventoryDevice{
deviceId: 1,
uniqueId: 1,
endpoints: []endpointDetails{},
}

Expand Down
2 changes: 1 addition & 1 deletion implcaps/generic/device_workarounds/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (i *Implementation) enumerateZCLReportingKeepAlive(ctx context.Context, m m
return err
}

if err := i.zi.ZCLCommunicator().ConfigureReporting(ctx, ieeeAddress, ack, zcl.BasicId, zigbee.NoManufacturer, localEndpoint, remoteEndpoint, seq, basic.ZCLVersion, zcl.TypeUnsignedInt8, uint16(60), uint16(240), 0); err != nil {
if err := i.zi.ZCLCommunicator().ConfigureReporting(ctx, ieeeAddress, ack, zcl.BasicId, zigbee.NoManufacturer, localEndpoint, remoteEndpoint, seq, basic.ZCLVersion, zcl.TypeUnsignedInt8, uint16(60), uint16(240), uint(0)); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion implcaps/generic/device_workarounds/impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestImplementation_EnableZCLReportingKeepAlive(t *testing.T) {

mnb.On("BindNodeToController", mock.Anything, ieee, zigbee.Endpoint(2), zigbee.Endpoint(3), zcl.BasicId).Return(nil)

mzc.On("ConfigureReporting", mock.Anything, ieee, false, zcl.BasicId, zigbee.NoManufacturer, zigbee.Endpoint(2), zigbee.Endpoint(3), uint8(4), basic.ZCLVersion, zcl.TypeUnsignedInt8, uint16(60), uint16(240), 0).Return(nil)
mzc.On("ConfigureReporting", mock.Anything, ieee, false, zcl.BasicId, zigbee.NoManufacturer, zigbee.Endpoint(2), zigbee.Endpoint(3), uint8(4), basic.ZCLVersion, zcl.TypeUnsignedInt8, uint16(60), uint16(240), uint(0)).Return(nil)

s := memory.New()
i := NewDeviceWorkaround(mzi)
Expand Down
9 changes: 8 additions & 1 deletion provider_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func (z *ZDA) providerLoadNode(pctx context.Context, i zigbee.IEEEAddress) {
defer end()

n, _ := z.createNode(i)

for _, d := range z.deviceListFromPersistence(i) {
z.providerLoadDevice(ctx, n, d)
}
Expand All @@ -32,7 +33,13 @@ func (z *ZDA) providerLoadDevice(pctx context.Context, n *node, i IEEEAddressWit

d := z.createSpecificDevice(n, i.SubIdentifier)

capSection := z.sectionForDevice(i).Section("Capability")
devSection := z.sectionForDevice(i)

if id, found := devSection.Int("UniqueId"); found {
z.setDeviceUniqueId(d, int(id))
}

capSection := devSection.Section("Capability")

for _, cName := range capSection.SectionKeys() {
cctx, cend := z.logger.Segment(ctx, "Loading capability data.", logwrap.Datum("capability", cName))
Expand Down
4 changes: 4 additions & 0 deletions provider_load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ func Test_gateway_providerLoad(t *testing.T) {
id := IEEEAddressWithSubIdentifier{IEEEAddress: zigbee.GenerateLocalAdministeredIEEEAddress(), SubIdentifier: 1}
dS := g.sectionForDevice(id)

dS.Set("UniqueId", 5)

cS := dS.Section("Capability", "ProductInformation")
cS.Set("Implementation", "GenericProductInformation")

Expand All @@ -32,6 +34,8 @@ func Test_gateway_providerLoad(t *testing.T) {

d := g.getDevice(id)

assert.Equal(t, d.deviceId, 5)

c := d.Capability(capabilities.ProductInformationFlag)
assert.NotNil(t, c)

Expand Down
14 changes: 12 additions & 2 deletions table.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ func (z *ZDA) createNextDevice(n *node) *device {
return z.createSpecificDevice(n, subId)
}

func (z *ZDA) setDeviceUniqueId(d *device, id int) {
d.m.Lock()
defer d.m.Unlock()

d.deviceId = id
d.deviceIdSet = true

z.sectionForDevice(d.address).Set("UniqueId", id)
}

func (z *ZDA) removeDevice(ctx context.Context, addr IEEEAddressWithSubIdentifier) bool {
n := z.getNode(addr.IEEEAddress)

Expand Down Expand Up @@ -178,15 +188,15 @@ func (z *ZDA) attachCapabilityToDevice(d *device, c implcaps.ZDACapability) {
cF := c.Capability()

d.capabilities[cF] = c
z.sectionForDevice(d.address).Section("Device", capabilities.StandardNames[cF])
z.sectionForDevice(d.address).Section("Capability", capabilities.StandardNames[cF])
z.sendEvent(da.CapabilityAdded{Device: d, Capability: cF})
}

func (z *ZDA) detachCapabilityFromDevice(d *device, c implcaps.ZDACapability) {
cF := c.Capability()
if _, found := d.capabilities[cF]; found {
z.sendEvent(da.CapabilityRemoved{Device: d, Capability: cF})
z.sectionForDevice(d.address).Section("Device").SectionDelete(capabilities.StandardNames[cF])
z.sectionForDevice(d.address).Section("Capability").SectionDelete(capabilities.StandardNames[cF])
delete(d.capabilities, cF)
}
}
27 changes: 24 additions & 3 deletions table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func Test_gateway_attachCapabilityToDevice(t *testing.T) {
assert.Len(t, events, 1)
assert.IsType(t, da.CapabilityAdded{}, events[0])

assert.True(t, g.sectionForDevice(d.address).Section("Device").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))
assert.True(t, g.sectionForDevice(d.address).Section("Capability").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))
})
}

Expand All @@ -272,7 +272,7 @@ func Test_gateway_detachCapabilityFromDevice(t *testing.T) {
c := product_information.NewProductInformation()
g.attachCapabilityToDevice(d, c)

assert.True(t, g.sectionForDevice(d.address).Section("Device").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))
assert.True(t, g.sectionForDevice(d.address).Section("Capability").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))

_ = drainEvents(g)

Expand All @@ -284,7 +284,7 @@ func Test_gateway_detachCapabilityFromDevice(t *testing.T) {
assert.Len(t, events, 1)
assert.IsType(t, da.CapabilityRemoved{}, events[0])

assert.False(t, g.sectionForDevice(d.address).Section("Device").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))
assert.False(t, g.sectionForDevice(d.address).Section("Capability").SectionExists(capabilities.StandardNames[capabilities.ProductInformationFlag]))
})

t.Run("does nothing if called for unattached capability", func(t *testing.T) {
Expand All @@ -305,3 +305,24 @@ func Test_gateway_detachCapabilityFromDevice(t *testing.T) {
assert.Len(t, g.events, 0)
})
}

func TestZDA_setDeviceUniqueId(t *testing.T) {
g := New(context.Background(), memory.New(), nil, nil)
g.events = make(chan any, 0xffff)

addr := zigbee.GenerateLocalAdministeredIEEEAddress()

n, _ := g.createNode(addr)
d := g.createNextDevice(n)

_ = drainEvents(g)

g.setDeviceUniqueId(d, 4)

assert.Equal(t, 4, d.deviceId)
assert.True(t, d.deviceIdSet)

id, found := g.sectionForDevice(d.address).Int("UniqueId")
assert.True(t, found)
assert.Equal(t, int64(4), id)
}

0 comments on commit 82e410e

Please sign in to comment.