diff --git a/packages/cc/src/cc/BasicCC.ts b/packages/cc/src/cc/BasicCC.ts index fc037cd047a1..f3b01217c89c 100644 --- a/packages/cc/src/cc/BasicCC.ts +++ b/packages/cc/src/cc/BasicCC.ts @@ -7,6 +7,7 @@ import { MessagePriority, type MessageRecord, type SupervisionResult, + type ValueID, ValueMetadata, maybeUnknownToString, parseMaybeNumber, @@ -70,7 +71,6 @@ export const BasicCCValues = Object.freeze({ }, }), - // TODO: This should really not be a static CC value, but depend on compat flags: ...V.staticPropertyWithName( "compatEvent", "event", @@ -80,9 +80,7 @@ export const BasicCCValues = Object.freeze({ } as const, { stateful: false, - autoCreate: (applHost, endpoint) => - !!applHost.getDeviceConfig?.(endpoint.nodeId)?.compat - ?.treatBasicSetAsEvent, + autoCreate: false, }, ), }), @@ -262,12 +260,9 @@ export class BasicCC extends CommandClass { // try to query the current state await this.refreshValues(applHost); - // Remove Basic CC support when there was no response, - // but only if the compat event shouldn't be used. + // Remove Basic CC support when there was no response if ( - !applHost.getDeviceConfig?.(node.id)?.compat - ?.treatBasicSetAsEvent - && this.getValue(applHost, BasicCCValues.currentValue) == undefined + this.getValue(applHost, BasicCCValues.currentValue) == undefined ) { applHost.controllerLog.logNode(node.id, { endpoint: this.endpointIndex, @@ -276,7 +271,10 @@ export class BasicCC extends CommandClass { }); // SDS14223: A controlling node MUST conclude that the Basic Command Class is not supported by a node (or // endpoint) if no Basic Report is returned. - endpoint.removeCC(CommandClasses.Basic); + endpoint.addCC(CommandClasses.Basic, { isSupported: false }); + if (!endpoint.controlsCC(CommandClasses.Basic)) { + endpoint.removeCC(CommandClasses.Basic); + } } // Remember that the interview is complete @@ -317,6 +315,28 @@ remaining duration: ${basicResponse.duration?.toString() ?? "undefined"}`; }); } } + + public override getDefinedValueIDs( + applHost: ZWaveApplicationHost, + ): ValueID[] { + const ret: ValueID[] = []; + + // Defer to the base implementation if Basic CC is supported + const endpoint = this.getEndpoint(applHost)!; + if (endpoint.supportsCC(this.ccId)) { + ret.push(...super.getDefinedValueIDs(applHost)); + } + + // Add the compat event value if it should be exposed + if ( + !!applHost.getDeviceConfig?.(endpoint.nodeId)?.compat + ?.treatBasicSetAsEvent + ) { + ret.push(BasicCCValues.compatEvent.endpoint(endpoint.index)); + } + + return ret; + } } // @publicAPI diff --git a/packages/zwave-js/src/lib/node/Endpoint.ts b/packages/zwave-js/src/lib/node/Endpoint.ts index 18f8558aa662..6aaa6f75f7c3 100644 --- a/packages/zwave-js/src/lib/node/Endpoint.ts +++ b/packages/zwave-js/src/lib/node/Endpoint.ts @@ -243,8 +243,9 @@ export class Endpoint implements IZWaveEndpoint { this.supportsCC(CommandClasses.Basic) && actuatorCCs.some((cc) => this.supportsCC(cc)) ) { - // We still want to know if BasicCC is controlled, so only mark it as not supported + // Mark the CC as not supported, but remember if it is controlled this.addCC(CommandClasses.Basic, { isSupported: false }); + // If the record is now only a dummy, remove the CC if ( !this.supportsCC(CommandClasses.Basic) diff --git a/packages/zwave-js/src/lib/node/Node.ts b/packages/zwave-js/src/lib/node/Node.ts index baf78ebfb267..d9a12cf74ba8 100644 --- a/packages/zwave-js/src/lib/node/Node.ts +++ b/packages/zwave-js/src/lib/node/Node.ts @@ -2919,17 +2919,13 @@ protocol version: ${this.protocolVersion}`; private modifySupportedCCBeforeInterview(endpoint: Endpoint): void { const compat = this._deviceConfig?.compat; - // Don't offer or interview the Basic CC if any actuator CC is supported - except if the config files forbid us - // to map the Basic CC to other CCs or expose Basic Set as an event - if (compat?.treatBasicSetAsEvent) { - if (endpoint.index === 0) { - // To create the compat event value, we need to force a Basic CC interview - endpoint.addCC(CommandClasses.Basic, { - isSupported: true, - version: 1, - }); - } - } else if (!compat?.disableBasicMapping) { + // If the config file instructs us to expose Basic Set as an event, mark the CC as controlled + if (compat?.treatBasicSetAsEvent && endpoint.index === 0) { + endpoint.addCC(CommandClasses.Basic, { isControlled: true }); + } + + // Don't offer or interview the Basic CC if any actuator CC is supported + if (!compat?.disableBasicMapping) { endpoint.hideBasicCCInFavorOfActuatorCCs(); } diff --git a/packages/zwave-js/src/lib/node/mockCCBehaviors/Basic.ts b/packages/zwave-js/src/lib/node/mockCCBehaviors/Basic.ts index 8c9bb44fb4ad..66ce55ecfff5 100644 --- a/packages/zwave-js/src/lib/node/mockCCBehaviors/Basic.ts +++ b/packages/zwave-js/src/lib/node/mockCCBehaviors/Basic.ts @@ -18,7 +18,9 @@ const respondToBasicGet: MockNodeBehavior = { && frame.payload instanceof BasicCCGet ) { // Do not respond if BasicCC is not explicitly listed as supported - if (!self.implementedCCs.has(CommandClasses.Basic)) return false; + if (!self.implementedCCs.get(CommandClasses.Basic)?.isSupported) { + return false; + } const cc = new BasicCCReport(self.host, { nodeId: controller.host.ownNodeId, diff --git a/packages/zwave-js/src/lib/node/utils.ts b/packages/zwave-js/src/lib/node/utils.ts index 154eb35886d9..5dbb3c0665cb 100644 --- a/packages/zwave-js/src/lib/node/utils.ts +++ b/packages/zwave-js/src/lib/node/utils.ts @@ -301,6 +301,7 @@ export function getDefinedValueIDs( ): TranslatedValueID[] { let ret: ValueID[] = []; const allowControlled: CommandClasses[] = [ + CommandClasses.Basic, CommandClasses["Scene Activation"], ]; for (const endpoint of getAllEndpoints(applHost, node)) { diff --git a/packages/zwave-js/src/lib/test/cc/BasicCC.integration.test.ts b/packages/zwave-js/src/lib/test/cc/BasicCC.integration.test.ts deleted file mode 100644 index 411f5ed1be1c..000000000000 --- a/packages/zwave-js/src/lib/test/cc/BasicCC.integration.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { BasicCCValues } from "@zwave-js/cc"; -import { CommandClasses } from "@zwave-js/core"; -import type { ThrowingMap } from "@zwave-js/shared"; -import { MockController } from "@zwave-js/testing"; -import ava, { type TestFn } from "ava"; -import { createDefaultMockControllerBehaviors } from "../../../Utils"; -import type { Driver } from "../../driver/Driver"; -import { createAndStartTestingDriver } from "../../driver/DriverMock"; -import { ZWaveNode } from "../../node/Node"; - -interface TestContext { - driver: Driver; - node2: ZWaveNode; - controller: MockController; -} - -const test = ava as TestFn; - -test.before(async (t) => { - t.timeout(30000); - - const { driver } = await createAndStartTestingDriver({ - skipNodeInterview: true, - loadConfiguration: false, - beforeStartup(mockPort) { - const controller = new MockController({ serial: mockPort }); - controller.defineBehavior( - ...createDefaultMockControllerBehaviors(), - ); - t.context.controller = controller; - }, - }); - t.context.driver = driver; - - const node2 = new ZWaveNode(2, driver); - (driver.controller.nodes as ThrowingMap).set( - node2.id, - node2, - ); - node2.addCC(CommandClasses.Basic, { - isSupported: true, - version: 1, - }); - t.context.node2 = node2; -}); - -test.after.always(async (t) => { - const { driver } = t.context; - await driver.destroy(); - driver.removeAllListeners(); -}); - -test.serial("should NOT include the compat event value", (t) => { - const { node2 } = t.context; - const valueIDs = node2.getDefinedValueIDs(); - t.false( - valueIDs - .map(({ property }) => property) - .includes(BasicCCValues.compatEvent.id.property), - ); -}); - -test.serial("except when the corresponding compat flag is set", (t) => { - const { node2 } = t.context; - // @ts-expect-error - node2["_deviceConfig"] = { - compat: { - treatBasicSetAsEvent: true, - }, - }; - - const valueIDs = node2.getDefinedValueIDs(); - t.true( - valueIDs - .map(({ property }) => property) - .includes(BasicCCValues.compatEvent.id.property), - ); -}); diff --git a/packages/zwave-js/src/lib/test/compat/fixtures/basicEventNoSupport/deviceConfig.json b/packages/zwave-js/src/lib/test/compat/fixtures/basicEventNoSupport/deviceConfig.json new file mode 100644 index 000000000000..e584b52cf509 --- /dev/null +++ b/packages/zwave-js/src/lib/test/compat/fixtures/basicEventNoSupport/deviceConfig.json @@ -0,0 +1,19 @@ +{ + "manufacturer": "Test Manufacturer", + "manufacturerId": "0xdead", + "label": "Test Device", + "description": "With Basic Event", + "devices": [ + { + "productType": "0xbeef", + "productId": "0xcafe" + } + ], + "firmwareVersion": { + "min": "0.0", + "max": "255.255" + }, + "compat": { + "treatBasicSetAsEvent": true + } +} diff --git a/packages/zwave-js/src/lib/test/compat/treatBasicSetAsEvent.test.ts b/packages/zwave-js/src/lib/test/compat/treatBasicSetAsEvent.test.ts new file mode 100644 index 000000000000..50de561b29fd --- /dev/null +++ b/packages/zwave-js/src/lib/test/compat/treatBasicSetAsEvent.test.ts @@ -0,0 +1,168 @@ +import { BasicCCValues } from "@zwave-js/cc"; +import { CommandClasses } from "@zwave-js/core"; +import path from "node:path"; +import { integrationTest } from "../integrationTestSuite"; + +integrationTest( + "Basic CC values should only include the compatEvent value if Basic CC is not supported, but the compat flag is set", + { + // debug: true, + + nodeCapabilities: { + manufacturerId: 0xdead, + productType: 0xbeef, + productId: 0xcafe, + + commandClasses: [ + CommandClasses.Version, + CommandClasses["Manufacturer Specific"], + // Basic CC is only controlled + { + ccId: CommandClasses.Basic, + isSupported: false, + isControlled: true, + version: 1, + }, + ], + }, + + additionalDriverOptions: { + storage: { + deviceConfigPriorityDir: path.join( + __dirname, + "fixtures/basicEventNoSupport", + ), + }, + }, + + async testBody(t, driver, node, mockController, mockNode) { + // Make sure the custom config is loaded + const treatBasicSetAsEvent = node.deviceConfig?.compat + ?.treatBasicSetAsEvent; + t.is(treatBasicSetAsEvent, true); + + const valueIDs = node.getDefinedValueIDs(); + t.false( + valueIDs.some((v) => BasicCCValues.currentValue.is(v)), + "Found Basic CC currentValue although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.targetValue.is(v)), + "Found Basic CC targetValue although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.duration.is(v)), + "Found Basic CC duration although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.restorePrevious.is(v)), + "Found Basic CC restorePrevious although it shouldn't be exposed", + ); + + t.true( + valueIDs.some((v) => BasicCCValues.compatEvent.is(v)), + "Did not find Basic CC compatEvent although it should be exposed", + ); + }, + }, +); + +integrationTest( + "Basic CC values should only include the compatEvent value if Basic CC should be hidden in favor of another CC, but the compat flag is set", + { + // debug: true, + + nodeCapabilities: { + manufacturerId: 0xdead, + productType: 0xbeef, + productId: 0xcafe, + + commandClasses: [ + CommandClasses.Version, + CommandClasses["Manufacturer Specific"], + // Basic CC is supported, but should be hidden + CommandClasses.Basic, + CommandClasses["Binary Switch"], + ], + }, + + additionalDriverOptions: { + storage: { + deviceConfigPriorityDir: path.join( + __dirname, + "fixtures/basicEventNoSupport", + ), + }, + }, + + async testBody(t, driver, node, mockController, mockNode) { + // Make sure the custom config is loaded + const treatBasicSetAsEvent = node.deviceConfig?.compat + ?.treatBasicSetAsEvent; + t.is(treatBasicSetAsEvent, true); + + const valueIDs = node.getDefinedValueIDs(); + t.false( + valueIDs.some((v) => BasicCCValues.currentValue.is(v)), + "Found Basic CC currentValue although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.targetValue.is(v)), + "Found Basic CC targetValue although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.duration.is(v)), + "Found Basic CC duration although it shouldn't be exposed", + ); + t.false( + valueIDs.some((v) => BasicCCValues.restorePrevious.is(v)), + "Found Basic CC restorePrevious although it shouldn't be exposed", + ); + + t.true( + valueIDs.some((v) => BasicCCValues.compatEvent.is(v)), + "Did not find Basic CC compatEvent although it should be exposed", + ); + }, + }, +); + +integrationTest( + "Basic CC values should NOT include the compatEvent value if Basic CC is supported, and the compat flag is not set", + { + // debug: true, + + nodeCapabilities: { + commandClasses: [ + CommandClasses.Version, + CommandClasses.Basic, + // No other actuator CC is supported + ], + }, + + async testBody(t, driver, node, mockController, mockNode) { + const valueIDs = node.getDefinedValueIDs(); + t.true( + valueIDs.some((v) => BasicCCValues.currentValue.is(v)), + "Did not find Basic CC currentValue although it should be exposed", + ); + t.true( + valueIDs.some((v) => BasicCCValues.targetValue.is(v)), + "Did not find Basic CC targetValue although it should be exposed", + ); + t.true( + valueIDs.some((v) => BasicCCValues.duration.is(v)), + "Did not find Basic CC duration although it should be exposed", + ); + t.true( + valueIDs.some((v) => BasicCCValues.restorePrevious.is(v)), + "Did not find Basic CC restorePrevious although it should be exposed", + ); + + t.false( + valueIDs.some((v) => BasicCCValues.compatEvent.is(v)), + "Found Basic CC compatEvent although it shouldn't be exposed", + ); + }, + }, +);