Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ember] Reworked multicast registration on coordinator #959

Merged
merged 1 commit into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 13 additions & 125 deletions src/adapter/ember/adapter/emberAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ const STACK_CONFIGS = {
/** <0-255> (Default: 0) @see EzspConfigId.SOURCE_ROUTE_TABLE_SIZE */
SOURCE_ROUTE_TABLE_SIZE: 200,// Z3GatewayGPCombo: 100, darkxst: 200
/** <1-250> (Default: 8) @see EzspConfigId.MULTICAST_TABLE_SIZE */
MULTICAST_TABLE_SIZE: 16,// darkxst: 16
MULTICAST_TABLE_SIZE: 16,// darkxst: 16, NOTE: should always be at least enough to register FIXED_ENDPOINTS multicastIds
},
"zigbeed": {
ADDRESS_TABLE_SIZE: 128,
Expand Down Expand Up @@ -436,12 +436,6 @@ export class EmberAdapter extends Adapter {

private defaultApsOptions: EmberApsOption;

/**
* Mirrors the NCP multicast table. null === not in use.
* Index 0 is Green Power and must always remain there.
*/
private multicastTable: EmberMulticastTableEntry[];

constructor(networkOptions: TsType.NetworkOptions, serialPortOptions: TsType.SerialPortOptions, backupPath: string,
adapterOptions: TsType.AdapterOptions, logger?: LoggerStub) {
super(networkOptions, serialPortOptions, backupPath, adapterOptions, logger);
Expand Down Expand Up @@ -759,9 +753,6 @@ export class EmberAdapter extends Adapter {

this.defaultApsOptions = (EmberApsOption.RETRY | EmberApsOption.ENABLE_ROUTE_DISCOVERY | EmberApsOption.ENABLE_ADDRESS_DISCOVERY);

// always at least length==1 because of allowed MULTICAST_TABLE_SIZE range
this.multicastTable = new Array<EmberMulticastTableEntry>(STACK_CONFIGS[this.stackConfig].MULTICAST_TABLE_SIZE).fill(null);

this.ezsp.once(EzspEvents.ncpNeedsResetAndInit, this.onNcpNeedsResetAndInit.bind(this));
}

Expand Down Expand Up @@ -961,6 +952,8 @@ export class EmberAdapter extends Adapter {
* Register fixed endpoints and set any related multicast entries that need to be.
*/
private async registerFixedEndpoints(): Promise<void> {
let mcTableIdx = 0;

for (const ep of FIXED_ENDPOINTS) {
if (ep.networkIndex !== 0x00) {
debug(`Multi-network not currently supported. Skipping endpoint ${JSON.stringify(ep)}.`);
Expand All @@ -978,8 +971,8 @@ export class EmberAdapter extends Adapter {
ep.profileId,
ep.deviceId,
ep.deviceVersion,
ep.inClusterList,
ep.outClusterList,
ep.inClusterList.slice(),// copy
ep.outClusterList.slice(),// copy
));

if (status === EzspStatus.SUCCESS) {
Expand All @@ -991,22 +984,20 @@ export class EmberAdapter extends Adapter {
debug(`Endpoint "${ep.endpoint}" already registered.`);
}

if (ep.endpoint === GP_ENDPOINT) {
const gpMulticastEntry: EmberMulticastTableEntry = {
multicastId: this.greenPowerGroup,
for (const multicastId of ep.multicastIds) {
const multicastEntry: EmberMulticastTableEntry = {
multicastId,
endpoint: ep.endpoint,
networkIndex: ep.networkIndex,
};

const status = (await this.ezsp.ezspSetMulticastTableEntry(0, gpMulticastEntry));
const status = (await this.ezsp.ezspSetMulticastTableEntry(mcTableIdx++, multicastEntry));

if (status !== EmberStatus.SUCCESS) {
throw new Error(`Failed to register group "Green Power" in multicast table with status=${EmberStatus[status]}.`);
throw new Error(`Failed to register group "${multicastId}" in multicast table with status=${EmberStatus[status]}.`);
}

// NOTE: ensure GP is always added first in the table
this.multicastTable[0] = gpMulticastEntry;
debug(`Registered multicast table entry: ${JSON.stringify(gpMulticastEntry)}.`);
debug(`Registered multicast table entry: ${JSON.stringify(multicastEntry)}.`);
}
}
}
Expand Down Expand Up @@ -1503,104 +1494,6 @@ export class EmberAdapter extends Adapter {
this.watchdogCountersHandle = setInterval(this.watchdogCounters.bind(this), WATCHDOG_COUNTERS_FEED_INTERVAL);
}

/**
* Handle changes in groups that needs to be propagated to the NCP multicast table.
*
* XXX: Since Z2M doesn't explicitly check-in downstream when groups are created/removed, we look at outgoing genGroups commands.
* If the NCP doesn't know about groups, it can miss messages from some devices (remotes for example), so we add it...
*
* @param commandId
* @param groupId
*/
private async onGroupChange(commandId: number, groupId?: number): Promise<void> {
switch (commandId) {
case Cluster.genGroups.commands.add.ID: {
// check if group already in multicast table, should not happen...
const existingIndex = this.multicastTable.findIndex((e) => ((e != null) && (e.multicastId === groupId)));

if (existingIndex == -1) {
// find first unused index
const newEntryIndex = this.multicastTable.findIndex((e) => (!e));

if (newEntryIndex != -1) {
const newEntry: EmberMulticastTableEntry = {
multicastId: groupId,
endpoint: FIXED_ENDPOINTS[0].endpoint,
networkIndex: FIXED_ENDPOINTS[0].networkIndex,
};
const status = (await this.ezsp.ezspSetMulticastTableEntry(newEntryIndex, newEntry));

if (status !== EmberStatus.SUCCESS) {
console.error(
`Failed to register group "${groupId}" in multicast table at index "${newEntryIndex}" with status=${EmberStatus[status]}.`
);
} else {
debug(`Registered multicast table entry: ${JSON.stringify(newEntry)}.`);
}

// always assume "it worked" to keep sync with Z2M first, NCP second, otherwise trouble might arise... should always work anyway
this.multicastTable[newEntryIndex] = newEntry;
} else {
console.warn(`Coordinator multicast table is full (max: ${STACK_CONFIGS[this.stackConfig].MULTICAST_TABLE_SIZE}). `
+ `Some devices in new groups may not work properly, including in group "${groupId}". `
+ `If that happens, please remove groups to be below the limit. `
+ `Removed groups are only removed from coordinator after a Zigbee2MQTT restart.`);
}
} else {
debug(`Added group "${groupId}", but local table says it is already registered at index "${existingIndex}". Skipping.`);
}
break;
}
// NOTE: Can't remove groups, since we watch from command exec to group members, that would trigger from any removed member,
// even though the group might still exist...
// Leaving this here (since it's done...), just in case we get better notifications for groups from upstream.
// case Cluster.genGroups.commands.remove.ID: {
// const entryIndex = this.multicastTable.findIndex((e) => ((e != null) && (e.multicastId === groupId)));

// // just in case, never remove GP at i zero, should never be the case...
// if (entryIndex > 0) {
// const entry = this.multicastTable[entryIndex];
// entry.endpoint = 0;// signals "not in use" in the stack
// const status = (await this.ezsp.ezspSetMulticastTableEntry(entryIndex, entry));

// if (status !== EmberStatus.SUCCESS) {
// console.error(`Failed to remove multicast table entry at index "${entryIndex}" for group "${groupId}".`);
// } else {
// debug(`Removed multicast table entry at index "${entryIndex}".`);
// }

// // always assume "it worked" to keep sync with Z2M first, NCP second, otherwise trouble might arise... should always work anyway
// this.multicastTable[entryIndex] = null;
// } else {
// debug(`Removed group "${groupId}", but local table did not have a reference to it.`);
// }
// break;
// }
// case Cluster.genGroups.commands.removeAll.ID: {
// // this can create quite a few NCP calls, but hopefully shouldn't happen often
// // always skip green power at i==0
// for (let i = 1; i < this.multicastTable.length; i++) {
// const entry = this.multicastTable[i];

// if (entry != null) {
// entry.endpoint = 0;// signals "not in use" in the stack
// const status = (await this.ezsp.ezspSetMulticastTableEntry(i, entry));

// if (status !== EmberStatus.SUCCESS) {
// console.error(`Failed to remove multicast entry at index "${i}" with status=${EmberStatus[status]}.`);
// } else {
// debug(`Removed multicast table entry at index "${i}".`);
// }
// }

// this.multicastTable[i] = null;
// }

// break;
// }
}
}

//---- START Events

//---- END Events
Expand Down Expand Up @@ -2740,8 +2633,8 @@ export class EmberAdapter extends Adapter {
profileID: ep.profileId,
ID: ep.endpoint,
deviceID: ep.deviceId,
inputClusters: ep.inClusterList,
outputClusters: ep.outClusterList,
inputClusters: ep.inClusterList.slice(),// copy
outputClusters: ep.outClusterList.slice(),// copy
};
}),
});
Expand Down Expand Up @@ -3623,11 +3516,6 @@ export class EmberAdapter extends Adapter {
}
}

// track group changes in NCP multicast table
if (apsFrame.clusterId === Cluster.genGroups.ID) {
await this.onGroupChange(command.ID, zclFrame.Payload.groupid);
}

debug(`~~~> [ZCL to=${networkAddress} apsFrame=${JSON.stringify(apsFrame)} header=${JSON.stringify(zclFrame.Header)}]`);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [status, messageTag] = (await this.ezsp.send(
Expand Down
12 changes: 9 additions & 3 deletions src/adapter/ember/adapter/endpoints.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Cluster from '../../../zcl/definition/cluster';
import {GP_ENDPOINT, GP_PROFILE_ID, HA_PROFILE_ID} from '../consts';
import {ClusterId, ProfileId} from '../types';
import {ClusterId, EmberMulticastId, ProfileId} from '../types';


type FixedEndpointInfo = {
Expand All @@ -13,11 +13,13 @@ type FixedEndpointInfo = {
/** Version of the device. uint8_t */
deviceVersion: number,
/** List of server clusters. */
inClusterList: ClusterId[],
inClusterList: readonly ClusterId[],
/** List of client clusters. */
outClusterList: ClusterId[],
outClusterList: readonly ClusterId[],
/** Network index for this endpoint. uint8_t */
networkIndex: number,
/** Multicast group IDs to register in the multicast table */
multicastIds: readonly EmberMulticastId[],
};


Expand Down Expand Up @@ -63,6 +65,9 @@ export const FIXED_ENDPOINTS: readonly FixedEndpointInfo[] = [
Cluster.touchlink.ID,// 0x1000, // touchlink
],
networkIndex: 0x00,
// Cluster spec 3.7.2.4.1: group identifier 0x0000 is reserved for the global scene used by the OnOff cluster.
// - IKEA sending state updates via MULTICAST(0x0000) https://github.com/Koenkk/zigbee-herdsman/issues/954
multicastIds: [0],
},
{// green power
endpoint: GP_ENDPOINT,
Expand All @@ -76,5 +81,6 @@ export const FIXED_ENDPOINTS: readonly FixedEndpointInfo[] = [
Cluster.greenPower.ID,// 0x0021,// Green Power
],
networkIndex: 0x00,
multicastIds: [0x0B84],
},
];
26 changes: 12 additions & 14 deletions src/adapter/ember/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1884,13 +1884,15 @@ export enum EmberBindingType {
UNUSED_BINDING = 0,
/** A unicast binding whose 64-bit identifier is the destination EUI64. */
UNICAST_BINDING = 1,
/** A unicast binding whose 64-bit identifier is the many-to-one
* destination EUI64. Route discovery should be disabled when sending
* unicasts via many-to-one bindings. */
/**
* A unicast binding whose 64-bit identifier is the many-to-one destination EUI64.
* Route discovery should be disabled when sending unicasts via many-to-one bindings.
*/
MANY_TO_ONE_BINDING = 2,
/** A multicast binding whose 64-bit identifier is the group address. This
* binding can be used to send messages to the group and to receive
* messages sent to the group. */
/**
* A multicast binding whose 64-bit identifier is the group address.
* This binding can be used to send messages to the group and to receive messages sent to the group.
*/
MULTICAST_BINDING = 3,
};

Expand All @@ -1902,17 +1904,13 @@ export enum EmberOutgoingMessageType {
VIA_ADDRESS_TABLE,
/** Unicast sent using an entry in the binding table. */
VIA_BINDING,
/** Multicast message. This value is passed to emberMessageSentHandler() only.
* It may not be passed to emberSendUnicast(). */
/** Multicast message. This value is passed to emberMessageSentHandler() only. It may not be passed to emberSendUnicast(). */
MULTICAST,
/** An aliased multicast message. This value is passed to emberMessageSentHandler() only.
* It may not be passed to emberSendUnicast(). */
/** An aliased multicast message. This value is passed to emberMessageSentHandler() only. It may not be passed to emberSendUnicast(). */
MULTICAST_WITH_ALIAS,
/** An aliased Broadcast message. This value is passed to emberMessageSentHandler() only.
* It may not be passed to emberSendUnicast(). */
/** An aliased Broadcast message. This value is passed to emberMessageSentHandler() only. It may not be passed to emberSendUnicast(). */
BROADCAST_WITH_ALIAS,
/** A broadcast message. This value is passed to emberMessageSentHandler() only.
* It may not be passed to emberSendUnicast(). */
/** A broadcast message. This value is passed to emberMessageSentHandler() only. It may not be passed to emberSendUnicast(). */
BROADCAST
};

Expand Down
18 changes: 9 additions & 9 deletions src/adapter/ember/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,21 @@ export type EmberBindingTableEntry = {
type: EmberBindingType ,
/** The endpoint on the local node. uint8_t */
local: number,
/** A cluster ID that matches one from the local endpoint's simple descriptor.
* This cluster ID is set by the provisioning application to indicate which
* part an endpoint's functionality is bound to this particular remote node
* and is used to distinguish between unicast and multicast bindings. Note
* that a binding can be used to to send messages with any cluster ID, not
* just that listed in the binding.
/**
* A cluster ID that matches one from the local endpoint's simple descriptor.
* This cluster ID is set by the provisioning application to indicate which part an endpoint's functionality is bound
* to this particular remote node and is used to distinguish between unicast and multicast bindings.
* Note that a binding can be used to to send messages with any cluster ID, not just that listed in the binding.
* uint16_t
*/
clusterId: number,
/** The endpoint on the remote node (specified by \c identifier). uint8_t */
/** The endpoint on the remote node (specified by identifier). uint8_t */
remote: number,
/** A 64-bit identifier. This is either:
/**
* A 64-bit identifier. This is either:
* - The destination EUI64, for unicasts.
* - A 16-bit multicast group address, for multicasts.
*/
*/
identifier: EmberEUI64,
/** The index of the network the binding belongs to. uint8_t */
networkIndex: number,
Expand Down