From 5e89385694154acc3e58dc787bf89debcbd84b16 Mon Sep 17 00:00:00 2001 From: AlCalzone Date: Fri, 15 Sep 2023 17:39:15 +0200 Subject: [PATCH] fix: handle notification enum behavior "replace" correctly (#6282) --- packages/cc/src/cc/NotificationCC.ts | 9 +- packages/zwave-js/src/lib/node/Node.ts | 21 ++++- .../cc-specific/notificationEnums.test.ts | 91 ++++++++++++++++--- 3 files changed, 98 insertions(+), 23 deletions(-) diff --git a/packages/cc/src/cc/NotificationCC.ts b/packages/cc/src/cc/NotificationCC.ts index fe291c0205d3..9b0077ea364f 100644 --- a/packages/cc/src/cc/NotificationCC.ts +++ b/packages/cc/src/cc/NotificationCC.ts @@ -387,7 +387,7 @@ export class NotificationCCAPI extends PhysicalCCAPI { } } -function getNotificationEnumBehavior( +export function getNotificationEnumBehavior( notificationConfig: Notification, valueConfig: NotificationValueDefinition & { type: "state" }, ): "none" | "extend" | "replace" { @@ -444,9 +444,10 @@ export function getNotificationValueMetadata( } if (valueConfig.parameter instanceof NotificationParameterWithEnum) { for (const [value, label] of valueConfig.parameter.values) { - metadata.states![ - getNotificationStateValueWithEnum(valueConfig.value, value) - ] = label; + const stateKey = enumBehavior === "replace" + ? value + : getNotificationStateValueWithEnum(valueConfig.value, value); + metadata.states![stateKey] = label; } } diff --git a/packages/zwave-js/src/lib/node/Node.ts b/packages/zwave-js/src/lib/node/Node.ts index af841e738990..f6c37678dfa8 100644 --- a/packages/zwave-js/src/lib/node/Node.ts +++ b/packages/zwave-js/src/lib/node/Node.ts @@ -89,6 +89,7 @@ import { NodeNamingAndLocationCCValues } from "@zwave-js/cc/NodeNamingCC"; import { NotificationCCReport, NotificationCCValues, + getNotificationEnumBehavior, getNotificationStateValueWithEnum, getNotificationValueMetadata, } from "@zwave-js/cc/NotificationCC"; @@ -4367,12 +4368,22 @@ protocol version: ${this.protocolVersion}`; } } if (typeof command.eventParameters === "number") { - // This notification contains an enum value. We set "fake" values for these to distinguish them + // This notification contains an enum value. Depending on how the enum behaves, + // we may need to set "fake" values for these to distinguish them // from states without enum values - const valueWithEnum = getNotificationStateValueWithEnum( - value, - command.eventParameters, - ); + const enumBehavior = valueConfig + ? getNotificationEnumBehavior( + notificationConfig, + valueConfig, + ) + : "extend"; + + const valueWithEnum = enumBehavior === "replace" + ? command.eventParameters + : getNotificationStateValueWithEnum( + value, + command.eventParameters, + ); this.valueDB.setValue(valueId, valueWithEnum); } else { this.valueDB.setValue(valueId, value); diff --git a/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts b/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts index 67f16890cb5a..fb4eb842d8fb 100644 --- a/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts +++ b/packages/zwave-js/src/lib/test/cc-specific/notificationEnums.test.ts @@ -7,10 +7,10 @@ import { createMockZWaveRequestFrame } from "@zwave-js/testing"; import { wait } from "alcalzone-shared/async"; import { integrationTest } from "../integrationTestSuite"; -integrationTest( +integrationTest.only( "Notifications with enum event parameters are evaluated correctly", { - // debug: true, + debug: true, nodeCapabilities: { commandClasses: [ @@ -44,8 +44,8 @@ integrationTest( // For the valve operation status variable, the embedded enum replaces its possible states // since there is only one meaningless state, so it doesn't make sense to preserve it // This is different from the "Door state" value which has multiple states AND enums - [0x0100]: "Off / Closed", - [0x0101]: "On / Open", + [0x00]: "Off / Closed", + [0x01]: "On / Open", }); // Send notifications to the node @@ -64,7 +64,7 @@ integrationTest( await wait(100); let value = node.getValue(valveOperationStatusId); - t.is(value, 0x0100); + t.is(value, 0x00); cc = new NotificationCCReport(mockNode.host, { nodeId: mockController.host.ownNodeId, @@ -80,13 +80,13 @@ integrationTest( await wait(100); value = node.getValue(valveOperationStatusId); - t.is(value, 0x0101); + t.is(value, 0x01); }, }, ); integrationTest( - "Notification types multiple states and optional enums merge/extend states for all of them", + "Notification types with multiple states and optional enums merge/extend states for all of them", { // debug: true, @@ -104,16 +104,15 @@ integrationTest( ], }, - testBody: async (t, driver, node, _mockController, _mockNode) => { + testBody: async (t, driver, node, mockController, mockNode) => { await node.commandClasses.Notification.getSupportedEvents(0x06); + const doorStateValueId = NotificationCCValues.notificationVariable( + "Access Control", + "Door state", + ).id; const states = ( - node.getValueMetadata( - NotificationCCValues.notificationVariable( - "Access Control", - "Door state", - ).id, - ) as ValueMetadataNumeric + node.getValueMetadata(doorStateValueId) as ValueMetadataNumeric ).states; t.deepEqual(states, { [0x16]: "Window/door is open", @@ -123,6 +122,70 @@ integrationTest( [0x1600]: "Window/door is open in regular position", [0x1601]: "Window/door is open in tilt position", }); + + // Send notifications to the node + let cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, + eventParameters: Buffer.from([0x00]), // open in regular position + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + // wait a bit for the value to be updated + await wait(100); + + let value = node.getValue(doorStateValueId); + t.is(value, 0x1600); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, + eventParameters: Buffer.from([0x01]), // open in tilt position + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x1601); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x16, // open + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x16); + + cc = new NotificationCCReport(mockNode.host, { + nodeId: mockController.host.ownNodeId, + notificationType: 0x06, + notificationEvent: 0x17, // closed + }); + await mockNode.sendToController( + createMockZWaveRequestFrame(cc, { + ackRequested: false, + }), + ); + await wait(100); + + value = node.getValue(doorStateValueId); + t.is(value, 0x17); }, }, );