Skip to content

Commit

Permalink
avoid calculating group custom clusters at all if unnecessary
Browse files Browse the repository at this point in the history
  • Loading branch information
@ccutrer
ccutrer committed Feb 16, 2025
1 parent 747bfad commit f66199d
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 30 deletions.
44 changes: 28 additions & 16 deletions src/controller/model/group.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import assert from 'node:assert';

import {logger} from '../../utils/logger';
import * as Zcl from '../../zspec/zcl';
import {CustomClusters} from '../../zspec/zcl/definition/tstype';
import {Cluster, CustomClusters} from '../../zspec/zcl/definition/tstype';
import ZclTransactionSequenceNumber from '../helpers/zclTransactionSequenceNumber';
import {DatabaseEntry, KeyValue} from '../tstype';
import Device from './device';
Expand Down Expand Up @@ -186,7 +186,7 @@ export class Group extends Entity {

public async write(clusterKey: number | string, attributes: KeyValue, options?: Options): Promise<void> {
const optionsWithDefaults = this.getOptionsWithDefaults(options, Zcl.Direction.CLIENT_TO_SERVER);
const cluster = Zcl.Utils.getCluster(clusterKey, undefined, this.customClusters);
const cluster = this.getCluster(clusterKey);
const payload: {attrId: number; dataType: number; attrData: number | string | boolean}[] = [];

for (const [nameOrID, value] of Object.entries(attributes)) {
Expand Down Expand Up @@ -214,7 +214,7 @@ export class Group extends Entity {
'write',
cluster.ID,
payload,
this.customClusters,
this._customClusters ?? {},
optionsWithDefaults.reservedBits,
);

Expand All @@ -230,7 +230,7 @@ export class Group extends Entity {

public async read(clusterKey: number | string, attributes: (string | number)[], options?: Options): Promise<void> {
const optionsWithDefaults = this.getOptionsWithDefaults(options, Zcl.Direction.CLIENT_TO_SERVER);
const cluster = Zcl.Utils.getCluster(clusterKey, undefined, this.customClusters);
const cluster = this.getCluster(clusterKey);
const payload: {attrId: number}[] = [];

for (const attribute of attributes) {
Expand All @@ -246,7 +246,7 @@ export class Group extends Entity {
'read',
cluster.ID,
payload,
this.customClusters,
this._customClusters ?? {},
optionsWithDefaults.reservedBits,
);

Expand All @@ -267,7 +267,7 @@ export class Group extends Entity {

public async command(clusterKey: number | string, commandKey: number | string, payload: KeyValue, options?: Options): Promise<void> {
const optionsWithDefaults = this.getOptionsWithDefaults(options, Zcl.Direction.CLIENT_TO_SERVER);
const cluster = Zcl.Utils.getCluster(clusterKey, undefined, this.customClusters);
const cluster = this.getCluster(clusterKey);
const command = cluster.getCommand(commandKey);

const createLogMessage = (): string => `Command ${this.groupID} ${cluster.name}.${command.name}(${JSON.stringify(payload)})`;
Expand All @@ -283,7 +283,7 @@ export class Group extends Entity {
command.ID,
cluster.ID,
payload,
this.customClusters,
this._customClusters ?? {},
optionsWithDefaults.reservedBits,
);

Expand All @@ -309,15 +309,11 @@ export class Group extends Entity {
}

/**
* Get custom clusters that all members share.
* Calculate, store, and return custom clusters that all members share.
*/
get customClusters(): CustomClusters {
if (this._customClusters) {
return this._customClusters;
}

private calculateCustomClusters(): CustomClusters {
if (this._members.size === 0) {
return {};
return (this._customClusters = {});
}

const membersArray = Array.from(this._members);
Expand All @@ -331,8 +327,24 @@ export class Group extends Entity {
}
}

this._customClusters = commonClusters;
return this._customClusters;
return (this._customClusters = commonClusters);
}

private getCluster(key: string | number): Cluster {
if (this._customClusters) {
return Zcl.Utils.getCluster(key, undefined, this._customClusters);
}

// At first, don't fully calculate custom clusters
const cluster = Zcl.Utils.findCluster(key, undefined, {});

// If no cluster was found, and we haven't calculated custom clusters,
// do so now, and then retry
if (!cluster) {
return Zcl.Utils.getCluster(key, undefined, this.calculateCustomClusters());
}

return cluster;
}
}

Expand Down
47 changes: 33 additions & 14 deletions src/zspec/zcl/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function getClusterDefinition(
key: string | number,
manufacturerCode: number | undefined,
customClusters: CustomClusters,
): {name: string; cluster: ClusterDefinition} {
): {name: string; cluster: ClusterDefinition} | undefined {
let name: string | undefined;

if (typeof key === 'number') {
Expand All @@ -144,25 +144,23 @@ function getClusterDefinition(
name = key;
}

let cluster =
name !== undefined && hasCustomClusters(customClusters)
const hasCustomClustersResult = hasCustomClusters(customClusters);
const cluster =
name !== undefined && hasCustomClustersResult
? {
...Clusters[name as ClusterName],
...customClusters[name], // should override Zcl clusters
}
: Clusters[name as ClusterName];

if (!cluster) {
if (typeof key === 'number') {
name = key.toString();
cluster = {attributes: {}, commands: {}, commandsResponse: {}, manufacturerCode: undefined, ID: key};
} else {
name = undefined;
}
if (!name || !cluster) {
return undefined;
}

if (!name) {
throw new Error(`Cluster with name '${key}' does not exist`);
// If we have customClusters, we have to double check that we didn't end up with
// an empty object due to the use of the spread operator above
if (hasCustomClustersResult) {
for (const k in cluster) return {name, cluster};
return undefined;
}

return {name, cluster};
Expand Down Expand Up @@ -281,7 +279,28 @@ function createCluster(name: string, cluster: ClusterDefinition, manufacturerCod
}

export function getCluster(key: string | number, manufacturerCode: number | undefined, customClusters: CustomClusters): Cluster {
const {name, cluster} = getClusterDefinition(key, manufacturerCode, customClusters);
let nameAndCluster = getClusterDefinition(key, manufacturerCode, customClusters);

if (!nameAndCluster) {
if (typeof key === 'number') {
const name = key.toString();
const cluster = {attributes: {}, commands: {}, commandsResponse: {}, manufacturerCode: undefined, ID: key};
nameAndCluster = {name, cluster};
} else {
throw new Error(`Cluster with name '${key}' does not exist`);
}
}

const {name, cluster} = nameAndCluster;
return createCluster(name, cluster, manufacturerCode);
}

export function findCluster(key: string | number, manufacturerCode: number | undefined, customClusters: CustomClusters): Cluster | undefined {
const nameAndCluster = getClusterDefinition(key, manufacturerCode, customClusters);
if (!nameAndCluster) {
return undefined;
}
const {name, cluster} = nameAndCluster;
return createCluster(name, cluster, manufacturerCode);
}

Expand Down
54 changes: 54 additions & 0 deletions test/controller.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5696,6 +5696,60 @@ describe('Controller', () => {
),
);
expect(mocksendZclFrameToGroup.mock.calls[0][2]).toBeUndefined();
// Do another write, to ensure customClusters was cached
await group.write('myCustomCluster', {superAttribute: 3}, {});
expect(mocksendZclFrameToGroup).toHaveBeenCalledTimes(2);
expect(mocksendZclFrameToGroup.mock.calls[0][0]).toBe(2);
expect(deepClone(mocksendZclFrameToGroup.mock.calls[1][1])).toStrictEqual(
deepClone(
Zcl.Frame.create(
Zcl.FrameType.GLOBAL,
Zcl.Direction.CLIENT_TO_SERVER,
true,
undefined,
12,
'write',
9123,
[{attrData: 3, attrId: 0, dataType: 32}],
device.customClusters,
),
),
);
expect(mocksendZclFrameToGroup.mock.calls[1][2]).toBeUndefined();
});

it('Write to empty group with custom cluster should fail', async () => {
await controller.start();
const group = await controller.createGroup(2);
let error;
try {
await group.write('myCustomCluster', {superAttribute: 5}, {});
} catch (e) {
error = e;
}
expect(error).toStrictEqual(new Error(`Cluster with name 'myCustomCluster' does not exist`));
});

it('Write to group with unsupported custom cluster should fail', async () => {
await controller.start();

await mockAdapterEvents['deviceJoined']({networkAddress: 129, ieeeAddr: '0x129'});
const device = controller.getDeviceByIeeeAddr('0x129')!;
device.addCustomCluster('myCustomCluster', {
ID: 9123,
commands: {},
commandsResponse: {},
attributes: {superAttribute: {ID: 0, type: Zcl.DataType.UINT8}},
});
const group = await controller.createGroup(2);
group.addMember(device.getEndpoint(1)!);
let error;
try {
await group.write('otherCustomCluster', {superAttribute: 5}, {});
} catch (e) {
error = e;
}
expect(error).toStrictEqual(new Error(`Cluster with name 'otherCustomCluster' does not exist`));
});

it('Write to group with unknown attribute should fail', async () => {
Expand Down

0 comments on commit f66199d

Please sign in to comment.