From 748e001485db872fa9373920085b6a6c1ed8f118 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Sat, 17 Aug 2024 08:35:12 +0200 Subject: [PATCH] fix: Ember: fix GP proxied messages handling (#1151) --- src/adapter/ember/adapter/emberAdapter.ts | 57 +---------------------- src/adapter/ember/ezsp/ezsp.ts | 50 ++++++++++++++------ src/zspec/consts.ts | 2 + src/zspec/zcl/buffaloZcl.ts | 2 +- test/adapter/ember/emberAdapter.test.ts | 44 ++++++++--------- 5 files changed, 61 insertions(+), 94 deletions(-) diff --git a/src/adapter/ember/adapter/emberAdapter.ts b/src/adapter/ember/adapter/emberAdapter.ts index 62aaf3b312..ef29f2c8c6 100644 --- a/src/adapter/ember/adapter/emberAdapter.ts +++ b/src/adapter/ember/adapter/emberAdapter.ts @@ -314,7 +314,6 @@ export class EmberAdapter extends Adapter { this.ezsp.on('stackStatus', this.onStackStatus.bind(this)); this.ezsp.on('trustCenterJoin', this.onTrustCenterJoin.bind(this)); this.ezsp.on('messageSent', this.onMessageSent.bind(this)); - this.ezsp.on('greenpowerMessage', this.onGreenpowerMessage.bind(this)); this.ezsp.once('ncpNeedsResetAndInit', this.onNcpNeedsResetAndInit.bind(this)); } @@ -545,7 +544,7 @@ export class EmberAdapter extends Adapter { } /** - * Emitted from @see Ezsp.ezspIncomingMessageHandler + * Emitted from @see Ezsp.ezspIncomingMessageHandler @see Ezsp.ezspGpepIncomingMessageHandler * * @param type * @param apsFrame @@ -609,60 +608,6 @@ export class EmberAdapter extends Adapter { this.emit('zclPayload', payload); } - /** - * Emitted from @see Ezsp.ezspGpepIncomingMessageHandler - * - * @param sequenceNumber - * @param commandIdentifier - * @param sourceId - * @param frameCounter - * @param gpdCommandId - * @param gpdCommandPayload - * @param gpdLink - */ - private async onGreenpowerMessage( - sequenceNumber: number, - commandIdentifier: number, - sourceId: number, - frameCounter: number, - gpdCommandId: number, - gpdCommandPayload: Buffer, - gpdLink: number, - ): Promise { - try { - const gpdHeader = Buffer.alloc(15); - gpdHeader.writeUInt8(0b00000001, 0); // frameControl: FrameType.SPECIFIC + Direction.CLIENT_TO_SERVER + disableDefaultResponse=false - gpdHeader.writeUInt8(sequenceNumber, 1); // transactionSequenceNumber - gpdHeader.writeUInt8(commandIdentifier, 2); // commandIdentifier - gpdHeader.writeUInt16LE(0, 3); // options XXX: bypassed, same as deconz https://github.com/Koenkk/zigbee-herdsman/pull/536 - gpdHeader.writeUInt32LE(sourceId, 5); // srcID - // omitted: gpdIEEEAddr ieeeAddr - // omitted: gpdEndpoint uint8 - gpdHeader.writeUInt32LE(frameCounter, 9); // frameCounter - gpdHeader.writeUInt8(gpdCommandId, 13); // commandID - gpdHeader.writeUInt8(gpdCommandPayload.length, 14); // payloadSize - - const data = Buffer.concat([gpdHeader, gpdCommandPayload]); - const payload: ZclPayload = { - header: Zcl.Header.fromBuffer(data), - data, - clusterID: Zcl.Clusters.greenPower.ID, - address: sourceId, - endpoint: ZSpec.GP_ENDPOINT, - linkquality: gpdLink, - groupID: this.greenPowerGroup, - wasBroadcast: true, - destinationEndpoint: ZSpec.GP_ENDPOINT, - }; - - this.oneWaitress.resolveZCL(payload); - this.emit('zclPayload', payload); - } catch (err) { - /* istanbul ignore next */ - logger.error(`<~x~ [GP] Failed creating ZCL payload. Skipping. ${err}`, NS); - } - } - /** * Emitted from @see Ezsp.ezspTrustCenterJoinHandler * Also from @see Ezsp.ezspIdConflictHandler as a DEVICE_LEFT diff --git a/src/adapter/ember/ezsp/ezsp.ts b/src/adapter/ember/ezsp/ezsp.ts index d1f9290acc..4f7c992136 100644 --- a/src/adapter/ember/ezsp/ezsp.ts +++ b/src/adapter/ember/ezsp/ezsp.ts @@ -5,6 +5,7 @@ import {Queue} from '../../../utils'; import {logger} from '../../../utils/logger'; import * as ZSpec from '../../../zspec'; import {EUI64, ExtendedPanId, NodeId, PanId} from '../../../zspec/tstypes'; +import * as Zcl from '../../../zspec/zcl'; import {Clusters} from '../../../zspec/zcl/definition/cluster'; import * as Zdo from '../../../zspec/zdo'; import {SerialPortOptions} from '../../tstype'; @@ -5244,10 +5245,12 @@ export class Ezsp extends EventEmitter { if (apsFrame.profileId === Zdo.ZDO_PROFILE_ID) { this.emit('zdoResponse', apsFrame, packetInfo.senderShortId, messageContents); - } else if (apsFrame.profileId === ZSpec.HA_PROFILE_ID || apsFrame.profileId === ZSpec.WILDCARD_PROFILE_ID) { + } else if ( + apsFrame.profileId === ZSpec.HA_PROFILE_ID || + apsFrame.profileId === ZSpec.WILDCARD_PROFILE_ID || + (apsFrame.profileId === ZSpec.GP_PROFILE_ID && type !== EmberIncomingMessageType.BROADCAST_LOOPBACK) + ) { this.emit('incomingMessage', type, apsFrame, packetInfo.lastHopLqi, packetInfo.senderShortId, messageContents); - } else if (apsFrame.profileId === ZSpec.GP_PROFILE_ID) { - // only broadcast loopback in here } } @@ -8506,7 +8509,7 @@ export class Ezsp extends EventEmitter { gpdCommandPayload: Buffer, ): void { logger.debug( - `ezspGpepIncomingMessageHandler(): callback called with: [status=${EmberGPStatus[status]}], [gpdLink=${gpdLink}], ` + + `ezspGpepIncomingMessageHandler(): callback called with: [status=${EmberGPStatus[status] ?? status}], [gpdLink=${gpdLink}], ` + `[sequenceNumber=${sequenceNumber}], [addr=${JSON.stringify(addr)}], [gpdfSecurityLevel=${EmberGpSecurityLevel[gpdfSecurityLevel]}], ` + `[gpdfSecurityKeyType=${EmberGpKeyType[gpdfSecurityKeyType]}], [autoCommissioning=${autoCommissioning}], ` + `[bidirectionalInfo=${bidirectionalInfo}], [gpdSecurityFrameCounter=${gpdSecurityFrameCounter}], [gpdCommandId=${gpdCommandId}], ` + @@ -8516,7 +8519,7 @@ export class Ezsp extends EventEmitter { if (addr.applicationId === EmberGpApplicationId.IEEE_ADDRESS) { // XXX: don't bother parsing for upstream for now, since it will be rejected - logger.error(`<=== [GP] Received IEEE address type in message. Support not implemented upstream. Dropping.`, NS); + logger.error(`<=x= [GP] Received IEEE address type in message. Support not implemented upstream. Dropping.`, NS); return; } @@ -8526,22 +8529,39 @@ export class Ezsp extends EventEmitter { if (!gpdCommandPayload.length) { // XXX: seem to be receiving duplicate commissioningNotification from some devices, second one with empty payload? // this will mess with the process no doubt, so dropping them + logger.debug(`<=x= [GP] Received commissioning notification with empty payload. Dropping.`, NS); return; } commandIdentifier = Clusters.greenPower.commands.commissioningNotification.ID; } - this.emit( - 'greenpowerMessage', - sequenceNumber, - commandIdentifier, - addr.sourceId, - gpdSecurityFrameCounter, - gpdCommandId, - gpdCommandPayload, - gpdLink, - ); + const apsFrame: EmberApsFrame = { + profileId: ZSpec.GP_PROFILE_ID, + clusterId: Zcl.Clusters.greenPower.ID, + sourceEndpoint: ZSpec.GP_ENDPOINT, + destinationEndpoint: ZSpec.GP_ENDPOINT, + options: 0, // not used + groupId: ZSpec.GP_GROUP_ID, + sequence: 0, // not used + }; + // this stuff is already parsed by EmberZNet stack, but Z2M expects the full buffer, so combine it back + const gpdHeader = Buffer.alloc(15); // addr.applicationId === EmberGpApplicationId.IEEE_ADDRESS ? 20 : 15 + gpdHeader.writeUInt8(0b00000001, 0); // frameControl: FrameType.SPECIFIC + Direction.CLIENT_TO_SERVER + disableDefaultResponse=false + gpdHeader.writeUInt8(sequenceNumber, 1); + gpdHeader.writeUInt8(commandIdentifier, 2); // commandIdentifier + gpdHeader.writeUInt16LE(0, 3); // options, only srcID present + gpdHeader.writeUInt32LE(addr.sourceId, 5); + // omitted: gpdIEEEAddr (ieeeAddr) + // omitted: gpdEndpoint (uint8) + gpdHeader.writeUInt32LE(gpdSecurityFrameCounter, 9); + gpdHeader.writeUInt8(gpdCommandId, 13); + gpdHeader.writeUInt8(gpdCommandPayload.length, 14); + + const messageContents = Buffer.concat([gpdHeader, gpdCommandPayload]); // omitted: gppNwkAddr (uint16), gppGddLink (uint8) + + // XXX: BROADCAST currently hardcoded to match upstream codepath + this.emit('incomingMessage', EmberIncomingMessageType.BROADCAST, apsFrame, gpdLink, addr.sourceId & 0xffff, messageContents); } /** diff --git a/src/zspec/consts.ts b/src/zspec/consts.ts index d7d8974e34..9c47c0c29b 100644 --- a/src/zspec/consts.ts +++ b/src/zspec/consts.ts @@ -22,6 +22,8 @@ export const HA_ENDPOINT = 0x01; /** The GP endpoint, as defined in the ZigBee spec. */ export const GP_ENDPOINT = 0xf2; +export const GP_GROUP_ID = 0x0b84; + /** The maximum 802.15.4 channel number is 26. */ export const MAX_802_15_4_CHANNEL_NUMBER = 26; /** The minimum 2.4GHz 802.15.4 channel number is 11. */ diff --git a/src/zspec/zcl/buffaloZcl.ts b/src/zspec/zcl/buffaloZcl.ts index 395b1359b7..7dfa828d98 100644 --- a/src/zspec/zcl/buffaloZcl.ts +++ b/src/zspec/zcl/buffaloZcl.ts @@ -574,7 +574,7 @@ export class BuffaloZcl extends Buffalo { }; // Manufacturer-specific Attribute Reporting } else if (options.payload?.commandID == 0xa1) { - if (options.payload.payloadSize == null) { + if (options.payload.payloadSize == undefined) { throw new Error('Cannot read GDP_FRAME with commandID=0xA1 without payloadSize options specified'); } diff --git a/test/adapter/ember/emberAdapter.test.ts b/test/adapter/ember/emberAdapter.test.ts index 0a2a7c4bd6..0196fece23 100644 --- a/test/adapter/ember/emberAdapter.test.ts +++ b/test/adapter/ember/emberAdapter.test.ts @@ -1453,7 +1453,8 @@ describe('Ember Adapter Layer', () => { ); const spyEmit = jest.spyOn(adapter, 'emit'); const sourceId: number = 1234; - const gpdLink = 123; + const nwkAddress: NodeId = sourceId & 0xffff; + const gpdLink: number = 123; const sequenceNumber: number = 1; const commandIdentifier: number = Zcl.Clusters.greenPower.commands.commissioningNotification.ID; const frameCounter: number = 102; @@ -1472,19 +1473,6 @@ describe('Ember Adapter Layer', () => { 0x00, 0x00 /* outgoingCounter */, ]); - - mockEzspEmitter.emit( - 'greenpowerMessage', - sequenceNumber, - commandIdentifier, - sourceId, - frameCounter, - gpdCommandId, - gpdCommandPayload, - gpdLink, - ); - await flushPromises(); - const gpdHeader = Buffer.alloc(15); gpdHeader.writeUInt8(0b00000001, 0); gpdHeader.writeUInt8(sequenceNumber, 1); @@ -1494,18 +1482,30 @@ describe('Ember Adapter Layer', () => { gpdHeader.writeUInt32LE(frameCounter, 9); gpdHeader.writeUInt8(gpdCommandId, 13); gpdHeader.writeUInt8(gpdCommandPayload.length, 14); + const messageContents = Buffer.concat([gpdHeader, gpdCommandPayload]); + const apsFrame: EmberApsFrame = { + profileId: ZSpec.GP_PROFILE_ID, + clusterId: Zcl.Clusters.greenPower.ID, + sourceEndpoint: ZSpec.GP_ENDPOINT, + destinationEndpoint: ZSpec.GP_ENDPOINT, + options: 0, // not used + groupId: ZSpec.GP_GROUP_ID, + sequence: 0, // not used + }; + + mockEzspEmitter.emit('incomingMessage', EmberIncomingMessageType.BROADCAST, apsFrame, gpdLink, nwkAddress, messageContents); + await flushPromises(); - const data = Buffer.concat([gpdHeader, gpdCommandPayload]); const payload: ZclPayload = { - header: Zcl.Header.fromBuffer(data), - data, - clusterID: Zcl.Clusters.greenPower.ID, - address: sourceId, - endpoint: ZSpec.GP_ENDPOINT, + header: Zcl.Header.fromBuffer(messageContents), + data: messageContents, + clusterID: apsFrame.clusterId, + address: nwkAddress, + endpoint: apsFrame.sourceEndpoint, linkquality: gpdLink, - groupID: 0x0b84, // TODO: this should be moved out of Adapter class and into ZSpec consts + groupID: apsFrame.groupId, wasBroadcast: true, - destinationEndpoint: ZSpec.GP_ENDPOINT, + destinationEndpoint: apsFrame.destinationEndpoint, }; expect(spyResolveZCL).toHaveBeenCalledTimes(1);