fix: expose Basic CC currentValue when compat flag says so (#6964)

packages/cc/src/cc/BasicCC.ts

@@ -327,14 +327,19 @@ remaining duration: ${basicResponse.duration?.toString() ?? "undefined"}`;
 			ret.push(...super.getDefinedValueIDs(applHost));
 		}
 
-		if (
-			applHost.getDeviceConfig?.(endpoint.nodeId)?.compat?.mapBasicSet
-				=== "event"
-		) {
+		const compat = applHost.getDeviceConfig?.(endpoint.nodeId)?.compat;
+		if (compat?.mapBasicSet === "event") {
 			// Add the compat event value if it should be exposed
 			ret.push(BasicCCValues.compatEvent.endpoint(endpoint.index));
-		} else if (endpoint.controlsCC(CommandClasses.Basic)) {
-			// Otherwise, only expose currentValue on devices that only control Basic CC
+		} else if (
+			!endpoint.supportsCC(CommandClasses.Basic) && (
+				endpoint.controlsCC(CommandClasses.Basic)
+				|| compat?.mapBasicReport === false
+				|| compat?.mapBasicSet === "report"
+			)
+		) {
+			// Otherwise, only expose currentValue on devices that only control Basic CC,
+			// or devices where a compat flag indicates that currentValue is meant to be exposed
 			ret.push(BasicCCValues.currentValue.endpoint(endpoint.index));
 		}
 
@@ -431,6 +436,49 @@ export class BasicCCReport extends BasicCC {
 	@ccValue(BasicCCValues.duration)
 	public readonly duration: Duration | undefined;
 
+	public persistValues(applHost: ZWaveApplicationHost): boolean {
+		// Basic CC Report persists its values itself, since there are some
+		// specific rules when which value may be persisted.
+		// These rules are essentially encoded in the getDefinedValueIDs overload,
+		// so we simply reuse that here.
+
+		// Figure out which values may be persisted.
+		const definedValueIDs = this.getDefinedValueIDs(applHost);
+		const shouldPersistCurrentValue = definedValueIDs.some((vid) =>
+			BasicCCValues.currentValue.is(vid)
+		);
+		const shouldPersistTargetValue = definedValueIDs.some((vid) =>
+			BasicCCValues.targetValue.is(vid)
+		);
+		const shouldPersistDuration = definedValueIDs.some((vid) =>
+			BasicCCValues.duration.is(vid)
+		);
+
+		if (this.currentValue !== undefined && shouldPersistCurrentValue) {
+			this.setValue(
+				applHost,
+				BasicCCValues.currentValue,
+				this.currentValue,
+			);
+		}
+		if (this.targetValue !== undefined && shouldPersistTargetValue) {
+			this.setValue(
+				applHost,
+				BasicCCValues.targetValue,
+				this.targetValue,
+			);
+		}
+		if (this.duration !== undefined && shouldPersistDuration) {
+			this.setValue(
+				applHost,
+				BasicCCValues.duration,
+				this.duration,
+			);
+		}
+
+		return true;
+	}
+
 	public serialize(): Buffer {
 		this.payload = Buffer.from([
 			this.currentValue ?? 0xfe,

packages/cc/src/cc/VersionCC.ts

@@ -472,6 +472,7 @@ export class VersionCC extends CommandClass {
 				let logMessage: string;
 				if (supportedVersion > 0) {
 					endpoint.addCC(cc, {
+						isSupported: true,
 						version: supportedVersion,
 					});
 					logMessage = `  supports CC ${CommandClasses[cc]} (${
@@ -591,6 +592,8 @@ export class VersionCC extends CommandClass {
 		for (const [cc] of endpoint.getCCs()) {
 			// We already queried the Version CC version at the start of this interview
 			if (cc === CommandClasses.Version) continue;
+			// And we queried Basic CC just before this
+			if (cc === CommandClasses.Basic) continue;
 			// Skip the query of endpoint CCs that are also supported by the root device
 			if (this.endpointIndex > 0 && node.getCCVersion(cc) > 0) continue;
 			await queryCCVersion(cc);

packages/zwave-js/src/lib/node/MockNodeBehaviors.ts

@@ -52,6 +52,9 @@ const respondToRequestNodeInfo: MockNodeBehavior = {
 				nodeId: self.id,
 				...self.capabilities,
 				supportedCCs: [...self.implementedCCs]
+					// Basic CC must not be included in the NIF
+					.filter(([ccId]) => ccId !== CommandClasses.Basic)
+					// Only include supported CCs
 					.filter(([, info]) => info.isSupported)
 					.map(([ccId]) => ccId),
 			});

packages/zwave-js/src/lib/node/Node.ts

@@ -2970,11 +2970,8 @@ protocol version:      ${this.protocolVersion}`;
 			endpoint.addCC(CommandClasses.Basic, { isControlled: true });
 		}
 
-		// Don't offer or interview the Basic CC if any actuator CC is supported, unless the config file tells us
-		// not to map Basic CC reports
-		if (compat?.mapBasicReport !== false) {
-			endpoint.hideBasicCCInFavorOfActuatorCCs();
-		}
+		// Mark Basic CC as not supported if any other actuator CC is supported
+		endpoint.hideBasicCCInFavorOfActuatorCCs();
 
 		// Window Covering CC:
 		// CL:006A.01.51.01.2: A controlling node MUST NOT interview and provide controlling functionalities for the
@@ -6242,14 +6239,14 @@ protocol version:      ${this.protocolVersion}`;
 
 		// Remove the Basic CC if it should be hidden
 		// TODO: Do this as part of loadDeviceConfig
-		const compat = this._deviceConfig?.compat;
-		if (
-			compat?.mapBasicReport !== false && compat?.mapBasicSet !== "event"
-		) {
-			for (const endpoint of this.getAllEndpoints()) {
-				endpoint.hideBasicCCInFavorOfActuatorCCs();
-			}
+		// const compat = this._deviceConfig?.compat;
+		// if (
+		// 	compat?.mapBasicReport !== false && compat?.mapBasicSet !== "event"
+		// ) {
+		for (const endpoint of this.getAllEndpoints()) {
+			endpoint.hideBasicCCInFavorOfActuatorCCs();
 		}
+		// }
 
 		// Mark already-interviewed nodes as potentially ready
 		if (this.interviewStage === InterviewStage.Complete) {

packages/zwave-js/src/lib/node/utils.ts

@@ -313,14 +313,18 @@ export function getDefinedValueIDsInternal(
 ): TranslatedValueID[] {
 	let ret: ValueID[] = [];
 	const allowControlled: CommandClasses[] = [
-		CommandClasses.Basic,
 		CommandClasses["Scene Activation"],
 	];
 	for (const endpoint of getAllEndpoints(applHost, node)) {
 		for (const cc of allCCs) {
 			if (
+				// Create values only for supported CCs
 				endpoint.supportsCC(cc)
+				// ...and some controlled CCs
 				|| (endpoint.controlsCC(cc) && allowControlled.includes(cc))
+				// ...and possibly Basic CC, which has some extra checks to know
+				// whether values should be exposed
+				|| cc === CommandClasses.Basic
 			) {
 				const ccInstance = CommandClass.createInstanceUnchecked(
 					applHost,

packages/zwave-js/src/lib/test/compat/basicCCReportForThermostat.test.ts

@@ -0,0 +1,65 @@
+import { BasicCCValues } from "@zwave-js/cc";
+import { CommandClasses } from "@zwave-js/core";
+import path from "node:path";
+import { integrationTest } from "../integrationTestSuite";
+
+integrationTest(
+	"A Thermostat with compat flag mapBasicReport=false exposes currentValue",
+	{
+		// debug: true,
+
+		nodeCapabilities: {
+			manufacturerId: 0xdead,
+			productType: 0xbeef,
+			productId: 0xcafe,
+
+			// General Thermostat V2
+			genericDeviceClass: 0x08,
+			specificDeviceClass: 0x06,
+			commandClasses: [
+				CommandClasses["Z-Wave Plus Info"],
+				// CommandClasses["Multilevel Sensor"],
+				CommandClasses["Version"],
+				{
+					ccId: CommandClasses["Thermostat Setpoint"],
+					version: 3,
+				},
+				{
+					ccId: CommandClasses["Thermostat Mode"],
+					version: 3,
+				},
+				CommandClasses["Manufacturer Specific"],
+				// CommandClasses["Meter"],
+				{
+					ccId: CommandClasses.Basic,
+					version: 1,
+				},
+			],
+		},
+
+		additionalDriverOptions: {
+			storage: {
+				deviceConfigPriorityDir: path.join(
+					__dirname,
+					"fixtures/mapBasicReportFalse",
+				),
+			},
+		},
+
+		async testBody(t, driver, node, mockController, mockNode) {
+			// Despite the compat flag, Basic CC should not be considered supported
+			t.false(node.supportsCC(CommandClasses.Basic));
+
+			// But currentValue should be exposed - otherwise it makes no sense to not map it
+			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.false(
+				valueIDs.some((v) => BasicCCValues.targetValue.is(v)),
+				"Found Basic CC targetValue although it shouldn't be exposed",
+			);
+		},
+	},
+);

packages/zwave-js/src/lib/test/compat/fixtures/mapBasicReportFalse/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": {
+		"mapBasicReport": false
+	}
+}

packages/zwave-js/src/lib/test/driver/fixtures/s2CollisionsSupervised/7e570001.jsonl

@@ -10,7 +10,6 @@
 {"k":"node.1.deviceClass","v":{"basic":2,"generic":2,"specific":7}}
 {"k":"node.1.endpoint.0.commandClass.0x72","v":{"isSupported":true,"isControlled":false,"secure":false,"version":0}}
 {"k":"node.1.endpoint.0.commandClass.0x86","v":{"isSupported":true,"isControlled":false,"secure":false,"version":0}}
-{"k":"node.1.endpoint.0.commandClass.0x20","v":{"isSupported":false,"isControlled":true,"secure":false,"version":0}}
 {"k":"node.1.endpoint.0.commandClass.0x60","v":{"isSupported":false,"isControlled":true,"secure":false,"version":0}}
 {"k":"node.1.interviewStage","v":"Complete"}
 {"k":"node.2.isListening","v":true}
@@ -23,7 +22,6 @@
 {"k":"node.2.supportsBeaming","v":true}
 {"k":"node.2.deviceClass","v":{"basic":4,"generic":6,"specific":1}}
 {"k":"node.2.interviewStage","v":"Complete"}
-{"k":"node.2.endpoint.0.commandClass.0x20","v":{"isSupported":true,"isControlled":false,"secure":true,"version":1}}
 {"k":"node.2.endpoint.0.commandClass.0x25","v":{"isSupported":true,"isControlled":false,"secure":true,"version":1}}
 {"k":"node.2.endpoint.0.commandClass.0x6c","v":{"isSupported":true,"isControlled":false,"secure":true,"version":1}}
 {"k":"node.2.endpoint.0.commandClass.0x9f","v":{"isSupported":true,"isControlled":false,"secure":true,"version":1}}

packages/zwave-js/src/lib/test/driver/s2Collisions.test.ts

@@ -2,6 +2,7 @@ import {
 	BasicCCReport,
 	BasicCCValues,
 	BinarySwitchCCReport,
+	BinarySwitchCCValues,
 	type CommandClass,
 	InvalidCC,
 	Security2CC,
@@ -175,8 +176,8 @@ integrationTest(
 		},
 
 		testBody: async (t, driver, node, mockController, mockNode) => {
-			// Send a secure Basic SET to sync the SPAN
-			await node.commandClasses.Basic.set(1);
+			// Send a secure Binary Switch SET to sync the SPAN
+			await node.commandClasses["Binary Switch"].set(false);
 
 			driver.driverLog.print("----------");
 			driver.driverLog.print("START TEST");
@@ -187,28 +188,30 @@ integrationTest(
 			// Now create a collision by having both parties send at the same time
 			const nodeToHost = Security2CC.encapsulate(
 				mockNode.host,
-				new BasicCCReport(mockNode.host, {
+				new BinarySwitchCCReport(mockNode.host, {
 					nodeId: mockController.host.ownNodeId,
-					currentValue: 99,
+					currentValue: true,
 				}),
 			);
 			const p1 = mockNode.sendToController(
 				createMockZWaveRequestFrame(nodeToHost, {
 					ackRequested: true,
 				}),
 			);
-			const p2 = node.commandClasses.Basic.set(0);
+			const p2 = node.commandClasses["Binary Switch"].set(false);
 
 			const [, p2result] = await Promise.all([p1, p2]);
 
 			// Give the node a chance to respond
 			await wait(250);
 
 			// If the collision was handled gracefully, we should now have the value reported by the node
-			const currentValue = node.getValue(BasicCCValues.currentValue.id);
-			t.is(currentValue, 99);
+			const currentValue = node.getValue(
+				BinarySwitchCCValues.currentValue.id,
+			);
+			t.is(currentValue, true);
 
-			// Ensure the Basic Set causing a collision eventually gets resolved
+			// Ensure the Binary Switch Set causing a collision eventually gets resolved
 			t.like(p2result, {
 				status: SupervisionStatus.Success,
 			});