Skip to content

Commit

Permalink
fix: Ember: Reworked multicast registration on coordinator (#959)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nerivec
Nerivec committed Mar 8, 2024
1 parent 4a55f23 commit 1f9ada9
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 151 deletions.
138 changes: 13 additions & 125 deletions src/adapter/ember/adapter/emberAdapter.ts
View file
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
View file
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
View file
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
View file
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

0 comments on commit 1f9ada9

Please sign in to comment.