[ember] Improve errors & checks + GreenPower support

src/adapter/ember/adapter/emberAdapter.ts

@@ -649,64 +649,46 @@ export class EmberAdapter extends Adapter {
     /**
      * Emitted from @see Ezsp.ezspGpepIncomingMessageHandler
      * 
-     * @param sender uint32_t or EmberEUI64 depending on `EmberGpApplicationId`. See emitter
+     * @param sequenceNumber 
+     * @param commandIdentifier 
+     * @param sourceId 
+     * @param frameCounter 
      * @param gpdCommandId 
+     * @param gpdCommandPayload 
      * @param gpdLink 
-     * @param sequenceNumber 
-     * @param deviceId 
-     * @param options 
-     * @param key 
-     * @param counter 
-     */
-    private async onGreenpowerMessage(sender: number | EmberEUI64, gpdCommandId: number, gpdLink: number, sequenceNumber: number, deviceId?: number,
-        options?: number, key?: EmberKeyData, counter?: number): Promise<void> {
-        // TODO: all this stuff needs triple-checking, also with upstream (not really multi-adapter at first glance?)
-        //       more params avail in EZSP handler
-        switch (gpdCommandId) {
-        case 0xE0: {
-            if (!key) {
-                return;
-            }
-
-            // commissioning notification
-            const gpdMessage = {
-                // gppNwkAddr: ?,// XXX
-                commandID: gpdCommandId,
-                commandFrame: {options: options, securityKey: key.contents, deviceID: deviceId, outgoingCounter: counter},
-                // XXX: Z2M seems to want only sourceId, but it isn't always present..? @see ezspGpepIncomingMessageHandler
-                srcID: sender,
-            };
-            const zclFrame = ZclFrame.create(FrameType.SPECIFIC, Direction.CLIENT_TO_SERVER, true, null, sequenceNumber, 'commissioningNotification',
-                Cluster.greenPower.ID, gpdMessage);
-            const payload: ZclDataPayload = {
-                frame: zclFrame,
-                address: sender,
-                endpoint: GP_ENDPOINT,
-                linkquality: gpdLink,
-                groupID: null,
-                wasBroadcast: true,
-                destinationEndpoint: GP_ENDPOINT,
-            };
-
-            this.emit(Events.zclData, payload);
-        }
-        default:{// XXX: all the rest in one basket?
-            const gpdMessage = {commandID: gpdCommandId, srcID: sender};// same as above about `srcID`
-            const zclFrame = ZclFrame.create(FrameType.SPECIFIC, Direction.CLIENT_TO_SERVER, true, null, sequenceNumber, 'notification',
-                Cluster.greenPower.ID, gpdMessage);
-
+     */
+    private async onGreenpowerMessage(sequenceNumber: number, commandIdentifier: number, sourceId: number, frameCounter: number,
+        gpdCommandId: number, gpdCommandPayload: Buffer, gpdLink: number) : Promise<void> {
+        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 gpFrame = ZclFrame.fromBuffer(Cluster.greenPower.ID, Buffer.concat([gpdHeader, gpdCommandPayload]));
             const payload: ZclDataPayload = {
-                frame: zclFrame,
-                address: sender,
+                frame: gpFrame,
+                address: sourceId,
                 endpoint: GP_ENDPOINT,
                 linkquality: gpdLink,
-                groupID: null,
-                wasBroadcast: true,
+                groupID: this.greenPowerGroup,
+                // XXX: upstream sends to `gppNwkAddr` if `wasBroadcast` is false, even if `gppNwkAddr` is null
+                wasBroadcast: (gpFrame.Payload.gppNwkAddr != null) ? false : true,
                 destinationEndpoint: GP_ENDPOINT,
             };
 
+            this.oneWaitress.resolveZCL(payload);
             this.emit(Events.zclData, payload);
-        }
+        } catch (err) {
+            console.error(`<~x~ [GP] Failed creating ZCL payload. Skipping. ${err}`);
+            return;
         }
     }
 
@@ -1047,13 +1029,12 @@ export class EmberAdapter extends Adapter {
                     + `with status=${EzspStatus[status]}.`);
             }
 
-            status = (await this.emberSetEzspPolicy(
-                EzspPolicyId.APP_KEY_REQUEST_POLICY,
-                STACK_CONFIGS[this.stackConfig].KEY_TABLE_SIZE ? EzspDecisionId.ALLOW_APP_KEY_REQUESTS : EzspDecisionId.DENY_APP_KEY_REQUESTS,
-            ));
+            const appKeyPolicy = STACK_CONFIGS[this.stackConfig].KEY_TABLE_SIZE
+                ? EzspDecisionId.ALLOW_APP_KEY_REQUESTS : EzspDecisionId.DENY_APP_KEY_REQUESTS;
+            status = (await this.emberSetEzspPolicy(EzspPolicyId.APP_KEY_REQUEST_POLICY, appKeyPolicy));
 
             if (status !== EzspStatus.SUCCESS) {
-                throw new Error(`[INIT TC] Failed to set EzspPolicyId APP_KEY_REQUEST_POLICY to DENY_APP_KEY_REQUESTS `
+                throw new Error(`[INIT TC] Failed to set EzspPolicyId APP_KEY_REQUEST_POLICY to ${EzspDecisionId[appKeyPolicy]} `
                     + `with status=${EzspStatus[status]}.`);
             }
 
@@ -1091,6 +1072,7 @@ export class EmberAdapter extends Adapter {
             debug(`[INIT TC] Current network config=${JSON.stringify(this.networkOptions)}`);
             debug(`[INIT TC] Current NCP network: nodeType=${EmberNodeType[nodeType]} params=${JSON.stringify(netParams)}`);
 
+            // XXX: should not force a form when it's only a channel change, just change the channel, wait a sec, then continue the logic
             if ((npStatus === EmberStatus.SUCCESS) && (nodeType === EmberNodeType.COORDINATOR) && (this.networkOptions.panID === netParams.panId)
                 && (equals(this.networkOptions.extendedPanID, netParams.extendedPanId))
                 && (this.networkOptions.channelList.includes(netParams.radioChannel))) {

src/adapter/ember/ezsp/ezsp.ts

@@ -242,7 +242,7 @@ export enum EzspEvents {
     MESSAGE_SENT_DELIVERY_FAILED = 'MESSAGE_SENT_DELIVERY_FAILED',
 
     //-- ezspGpepIncomingMessageHandler
-    /** params => sender: number | EmberEUI64, gpdCommandId: number, gpdLink: number, sequenceNumber: number, deviceId?: number, options?: number, key?: EmberKeyData, counter?: number */
+    /** params => sequenceNumber: number, commandIdentifier: number, sourceId: number, frameCounter: number, gpdCommandId: number, gpdCommandPayload: Buffer, gpdLink: number */
     GREENPOWER_MESSAGE = 'GREENPOWER_MESSAGE',
 }
 /* eslint-enable max-len */
@@ -7535,59 +7535,39 @@ export class Ezsp extends EventEmitter {
         gpdfSecurityLevel: EmberGpSecurityLevel, gpdfSecurityKeyType: EmberGpKeyType, autoCommissioning: boolean, bidirectionalInfo: number,
         gpdSecurityFrameCounter: number, gpdCommandId: number, mic: number, proxyTableIndex: number, gpdCommandPayload: Buffer): void {
         debug(`ezspGpepIncomingMessageHandler(): callback called with: [status=${EmberStatus[status]}], [gpdLink=${gpdLink}], `
-            + `[sequenceNumber=${sequenceNumber}], [addr=${addr}], [gpdfSecurityLevel=${gpdfSecurityLevel}], `
+            + `[sequenceNumber=${sequenceNumber}], [addr=${JSON.stringify(addr)}], [gpdfSecurityLevel=${gpdfSecurityLevel}], `
             + `[gpdfSecurityKeyType=${gpdfSecurityKeyType}], [autoCommissioning=${autoCommissioning}], [bidirectionalInfo=${bidirectionalInfo}], `
             + `[gpdSecurityFrameCounter=${gpdSecurityFrameCounter}], [gpdCommandId=${gpdCommandId}], [mic=${mic}], `
-            + `[proxyTableIndex=${proxyTableIndex}], [gpdCommandPayload=${gpdCommandPayload}]`);
-
-        // TODO: triple-checking required here
-        if (gpdCommandPayload.length) {
-            const gpdBuffalo = new EzspBuffalo(gpdCommandPayload, 0);
-
-            switch (gpdCommandId) {
-            case 0xE0: {
-                // commissioning notification
-                // eslint-disable-next-line @typescript-eslint/no-unused-vars
-                const st = gpdBuffalo.readUInt8();
-                const deviceId = gpdBuffalo.readUInt8();
-                const options = gpdBuffalo.readUInt8();
-                // eslint-disable-next-line @typescript-eslint/no-unused-vars
-                const extOptions = gpdBuffalo.readUInt8();
-                const key = gpdBuffalo.readEmberKeyData();
-                // eslint-disable-next-line @typescript-eslint/no-unused-vars
-                const mic = gpdBuffalo.readUInt32();
-                const counter = gpdBuffalo.readUInt32();
-
-                this.emit(
-                    EzspEvents.GREENPOWER_MESSAGE,
-                    (addr.applicationId === EmberGpApplicationId.SOURCE_ID) ? addr.sourceId : addr.gpdIeeeAddress,
-                    gpdCommandId,
-                    gpdLink,
-                    sequenceNumber,
-                    deviceId,
-                    options,
-                    key,
-                    counter
-                );
-                break;
-            }
-            default: {
-                // notification
-                this.emit(
-                    EzspEvents.GREENPOWER_MESSAGE,
-                    (addr.applicationId === EmberGpApplicationId.SOURCE_ID) ? addr.sourceId : addr.gpdIeeeAddress,
-                    gpdCommandId,
-                    gpdLink,
-                    sequenceNumber,
-                    null,
-                    null,
-                    null,
-                    null
-                );
-                break;
-            }
+            + `[proxyTableIndex=${proxyTableIndex}], [gpdCommandPayload=${gpdCommandPayload.toString('hex')}]`);
+
+        if (addr.applicationId === EmberGpApplicationId.IEEE_ADDRESS) {
+            // XXX: don't bother parsing for upstream for now, since it will be rejected
+            console.error(`<=== [GP] Received IEEE address type in message. Support not implemented upstream. Dropping.`);
+            return;
+        }
+
+        let commandIdentifier = Cluster.greenPower.commands.notification.ID;
+
+        if (gpdCommandId === 0xE0) {
+            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
+                return;
             }
+
+            commandIdentifier = Cluster.greenPower.commands.commissioningNotification.ID;
         }
+
+        this.emit(
+            EzspEvents.GREENPOWER_MESSAGE,
+            sequenceNumber,
+            commandIdentifier,
+            addr.sourceId,
+            gpdSecurityFrameCounter,
+            gpdCommandId,
+            gpdCommandPayload,
+            gpdLink,
+        );
     }
 
     /**

src/adapter/ember/uart/ash.ts

@@ -261,7 +261,15 @@ export class UartAsh extends EventEmitter {
 
     public counters: UartAshCounters;
 
+    /**
+     * Errors reported by the NCP.
+     * The `NcpFailedCode` from the frame reporting this is logged before this is set to make it clear where it failed:
+     * - The NCP sent an ERROR frame during the initial reset sequence (before CONNECTED state)
+     * - The NCP sent an ERROR frame
+     * - The NCP sent an unexpected RSTACK
+     */
     private ncpError: EzspStatus;
+    /** Errors reported by the Host. */
     private hostError: EzspStatus;
     /** sendExec() state variable */
     private sendState: SendState;
@@ -588,13 +596,20 @@ export class UartAsh extends EventEmitter {
             if (this.flags & Flag.CONNECTED) {
                 this.counters.rxCancelled += 1;
 
-                console.warn(`Frame(s) in progress cancelled. ${buffer.subarray(0, iCAN).toString('hex')}`);
+                console.warn(`Frame(s) in progress cancelled in [${buffer.toString('hex')}]`);
             }
 
             // get rid of everything up to the CAN flag and start reading frame from there, no need to loop through bytes in vain
             buffer = buffer.subarray(iCAN + 1);
         }
 
+        if (!buffer.length) {
+            // skip any CANCEL that results in empty frame (have yet to see one, but just in case...)
+            // shouldn't happen for any other reason, unless receiving bad stuff from port?
+            debug(`Received empty frame. Skipping.`);
+            return;
+        }
+
         const status = this.receiveFrame(buffer);
 
         if (status === EzspStatus.SUCCESS) {
@@ -1068,7 +1083,8 @@ export class UartAsh extends EventEmitter {
 
                 return EzspStatus.SUCCESS;
             } else if (frameType === AshFrameType.ERROR) {
-                return this.ncpDisconnect(this.rxSHBuffer[2]);
+                console.error(`Received ERROR from NCP while connecting, with code=${NcpFailedCode[this.rxSHBuffer[2]]}.`);
+                return this.ncpDisconnect(EzspStatus.ASH_NCP_FATAL_ERROR);
             }
 
             return EzspStatus.ASH_IN_PROGRESS;
@@ -1177,12 +1193,14 @@ export class UartAsh extends EventEmitter {
             break;
         case AshFrameType.RSTACK:
             // unexpected ncp reset
-            this.ncpError = this.rxSHBuffer[2];
+            console.error(`Received unexpected reset from NCP, with reason=${NcpFailedCode[this.rxSHBuffer[2]]}.`);
+            this.ncpError = EzspStatus.ASH_NCP_FATAL_ERROR;
 
             return this.hostDisconnect(EzspStatus.ASH_ERROR_NCP_RESET);
         case AshFrameType.ERROR:
             // ncp error
-            return this.ncpDisconnect(this.rxSHBuffer[2]);
+            console.error(`Received ERROR from NCP, with code=${NcpFailedCode[this.rxSHBuffer[2]]}.`);
+            return this.ncpDisconnect(EzspStatus.ASH_NCP_FATAL_ERROR);
         case AshFrameType.INVALID:
             // reject invalid frames
             debug(`<-x- [FRAME type=${frameTypeStr}] Rejecting. ${this.rxSHBuffer.toString('hex')}`);
@@ -1210,7 +1228,7 @@ export class UartAsh extends EventEmitter {
      * @returns 
      */
     private readFrame(buffer: Buffer): EzspStatus {
-        let status: EzspStatus;
+        let status: EzspStatus = EzspStatus.ERROR_INVALID_CALL;// no actual data to read, something's very wrong
         let index: number = 0;
         // let inByte: number = 0x00;
         let outByte: number = 0x00;
@@ -1388,7 +1406,7 @@ export class UartAsh extends EventEmitter {
         this.flags = 0;
         this.ncpError = error;
 
-        console.error(`ASH disconnected: ${EzspStatus[error]} | NCP status: ${EzspStatus[this.ncpError]}`);
+        console.error(`ASH disconnected | NCP status: ${EzspStatus[this.ncpError]}`);
 
         this.emit(AshEvents.ncpError, error);