refactor: handle Basic CC interview separately, be very explicit when…

… the CC is considered supported (#6984)

packages/cc/src/cc/BasicCC.ts

@@ -257,17 +257,20 @@ export class BasicCC extends CommandClass {
 			direction: "none",
 		});
 
+		// Assume that the endpoint supports Basic CC, so the values get persisted correctly.
+		endpoint.addCC(CommandClasses.Basic, { isSupported: true });
+
 		// try to query the current state
 		await this.refreshValues(applHost);
 
-		// Remove Basic CC support when there was no response
+		// Remove Basic CC support again when there was no response
 		if (
 			this.getValue(applHost, BasicCCValues.currentValue) == undefined
 		) {
 			applHost.controllerLog.logNode(node.id, {
 				endpoint: this.endpointIndex,
 				message:
-					"No response to Basic Get command, assuming the node does not support Basic CC...",
+					"No response to Basic Get command, assuming Basic CC is unsupported...",
 			});
 			// 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.
@@ -333,7 +336,8 @@ remaining duration: ${basicResponse.duration?.toString() ?? "undefined"}`;
 			ret.push(BasicCCValues.compatEvent.endpoint(endpoint.index));
 		} else if (
 			!endpoint.supportsCC(CommandClasses.Basic) && (
-				endpoint.controlsCC(CommandClasses.Basic)
+				(endpoint.controlsCC(CommandClasses.Basic)
+					&& compat?.mapBasicSet !== "Binary Sensor")
 				|| compat?.mapBasicReport === false
 				|| compat?.mapBasicSet === "report"
 			)

packages/cc/src/cc/VersionCC.ts

@@ -471,10 +471,20 @@ export class VersionCC extends CommandClass {
 				// Remember which CC version this endpoint supports
 				let logMessage: string;
 				if (supportedVersion > 0) {
-					endpoint.addCC(cc, {
-						isSupported: true,
-						version: supportedVersion,
-					});
+					// Basic CC has special rules for when it is considered supported
+					// Therefore we mark all other CCs as supported, but not Basic CC,
+					// for which support is determined later.
+					if (cc === CommandClasses.Basic) {
+						endpoint.addCC(cc, {
+							isControlled: true,
+							version: supportedVersion,
+						});
+					} else {
+						endpoint.addCC(cc, {
+							isSupported: true,
+							version: supportedVersion,
+						});
+					}
 					logMessage = `  supports CC ${CommandClasses[cc]} (${
 						num2hex(cc)
 					}) in version ${supportedVersion}`;

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

@@ -176,59 +176,21 @@ export class Endpoint implements IZWaveEndpoint {
 		return !!this._implementedCommandClasses.get(cc)?.isControlled;
 	}
 
-	/** Checks if this device type is allowed to support Basic CC per the specification */
+	/**
+	 * Checks if this endpoint is allowed to support Basic CC per the specification.
+	 * This depends on the device type and the other supported CCs
+	 */
 	public maySupportBasicCC(): boolean {
+		// Basic CC must not be offered if any other actuator CC is supported
+		if (actuatorCCs.some((cc) => this.supportsCC(cc))) {
+			return false;
+		}
+		// ...or the device class forbids it
 		return this.deviceClass?.specific.maySupportBasicCC
 			?? this.deviceClass?.generic.maySupportBasicCC
 			?? true;
 	}
 
-	/** Adds Basic CC to the supported CCs if no other actuator CCs are supported */
-	public maybeAddBasicCCAsFallback(): void {
-		if (
-			!this.supportsCC(CommandClasses.Basic)
-			&& this.maySupportBasicCC()
-			&& !actuatorCCs.some((cc) => this.supportsCC(cc))
-		) {
-			this.addCC(CommandClasses.Basic, { isSupported: true });
-		}
-	}
-
-	/** Removes the BasicCC from the supported CCs if the device type forbids it */
-	public removeBasicCCSupportIfForbidden(): void {
-		if (
-			this.supportsCC(CommandClasses.Basic)
-			&& !this.maySupportBasicCC()
-		) {
-			// We assume that the device reports support for this CC in error, and that it actually controls it.
-			// TODO: Consider if we should check additional sources, like the issued commands in AGI CC
-			this.addCC(CommandClasses.Basic, {
-				isSupported: false,
-				isControlled: true,
-			});
-		}
-	}
-
-	/** Removes the BasicCC from the supported CCs if any other actuator CCs are supported */
-	public hideBasicCCInFavorOfActuatorCCs(): void {
-		// This behavior is defined in SDS14223
-		if (
-			this.supportsCC(CommandClasses.Basic)
-			&& actuatorCCs.some((cc) => this.supportsCC(cc))
-		) {
-			// 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 entirely
-			if (
-				!this.supportsCC(CommandClasses.Basic)
-				&& !this.controlsCC(CommandClasses.Basic)
-			) {
-				this.removeCC(CommandClasses.Basic);
-			}
-		}
-	}
-
 	/** Determines if support for a CC was force-removed via config file */
 	public wasCCRemovedViaConfig(cc: CommandClasses): boolean {
 		if (this.supportsCC(cc)) return false;

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

@@ -1852,13 +1852,6 @@ export class ZWaveNode extends Endpoint
 			if (this.interviewStage === InterviewStage.NodeInfo) {
 				// Only advance the interview if it was completed, otherwise abort
 				if (await this.interviewCCs()) {
-					// After interviewing the CCs, we may need to clean up the Basic CC values.
-					// Some device types are not allowed to support it, but there are devices that do.
-					// If a device type is forbidden to support Basic CC, remove the "support" portion of it
-					for (const endpoint of this.getAllEndpoints()) {
-						endpoint.removeBasicCCSupportIfForbidden();
-					}
-
 					this.setInterviewStage(InterviewStage.CommandClasses);
 				} else {
 					return false;
@@ -2353,10 +2346,6 @@ protocol version:      ${this.protocolVersion}`;
 			if (typeof action === "boolean") return action;
 		}
 
-		// Basic CC MUST only be used/interviewed when no other actuator CC is supported. If Basic CC is not in the NIF
-		// or list of supported CCs, we need to add it here manually, so its version can get queried.
-		this.maybeAddBasicCCAsFallback();
-
 		if (this.supportsCC(CommandClasses.Version)) {
 			this.driver.controllerLog.logNode(
 				this.id,
@@ -2411,21 +2400,25 @@ protocol version:      ${this.protocolVersion}`;
 		// We determine the correct interview order of the remaining CCs by topologically sorting two dependency graph
 		// In order to avoid emitting unnecessary value events for the root endpoint,
 		// we defer the application CC interview until after the other endpoints have been interviewed
-		const priorityCCs = [
+		// The following CCs are interviewed "manually" outside of the automatic interview sequence,
+		// because there are special rules around them.
+		const specialCCs = [
 			CommandClasses.Security,
 			CommandClasses["Security 2"],
 			CommandClasses["Manufacturer Specific"],
 			CommandClasses.Version,
 			CommandClasses["Wake Up"],
+			// Basic CC is interviewed last
+			CommandClasses.Basic,
 		];
 		const rootInterviewGraphBeforeEndpoints = this.buildCCInterviewGraph([
-			...priorityCCs,
+			...specialCCs,
 			...applicationCCs,
 		]);
 		let rootInterviewOrderBeforeEndpoints: CommandClasses[];
 
 		const rootInterviewGraphAfterEndpoints = this.buildCCInterviewGraph([
-			...priorityCCs,
+			...specialCCs,
 			...nonApplicationCCs,
 		]);
 		let rootInterviewOrderAfterEndpoints: CommandClasses[];
@@ -2655,10 +2648,6 @@ protocol version:      ${this.protocolVersion}`;
 				}
 			}
 
-			// Basic CC MUST only be used/interviewed when no other actuator CC is supported. If Basic CC is not in the NIF
-			// or list of supported CCs, we need to add it here manually, so its version can get queried.
-			this.maybeAddBasicCCAsFallback();
-
 			// This intentionally checks for Version CC support on the root device.
 			// Endpoints SHOULD not support this CC, but we still need to query their
 			// CCs that the root device may or may not support
@@ -2707,6 +2696,7 @@ protocol version:      ${this.protocolVersion}`;
 				CommandClasses.Security,
 				CommandClasses["Security 2"],
 				CommandClasses.Version,
+				CommandClasses.Basic,
 			]);
 			let endpointInterviewOrder: CommandClasses[];
 			try {
@@ -2762,6 +2752,62 @@ protocol version:      ${this.protocolVersion}`;
 			else if (typeof action === "boolean") return action;
 		}
 
+		// At the very end, figure out if Basic CC is supposed to be supported
+		// First on the root device
+		if (!this.wasCCRemovedViaConfig(CommandClasses.Basic)) {
+			if (this.maySupportBasicCC()) {
+				// The device probably supports Basic CC and is allowed to.
+				// Interview the Basic CC to figure out if it actually supports it
+				this.driver.controllerLog.logNode(
+					this.id,
+					`Root device interview: ${getCCName(CommandClasses.Basic)}`,
+					"silly",
+				);
+
+				const action = await interviewEndpoint(
+					this,
+					CommandClasses.Basic,
+				);
+				if (typeof action === "boolean") return action;
+			} else {
+				// The device is supposed to only control Basic CC
+				this.driver.controllerLog.logNode(this.id, {
+					message:
+						"Node implements Basic CC but is not supposed to support it. Assuming it only controls Basic CC...",
+				});
+			}
+		}
+
+		// Then on all endpoints
+		for (const endpointIndex of this.getEndpointIndizes()) {
+			const endpoint = this.getEndpoint(endpointIndex);
+			if (!endpoint) continue;
+			if (endpoint.wasCCRemovedViaConfig(CommandClasses.Basic)) continue;
+
+			if (endpoint.maySupportBasicCC()) {
+				// The endpoint probably supports Basic CC and is allowed to.
+				// Interview the Basic CC to figure out if it actually supports it
+				this.driver.controllerLog.logNode(this.id, {
+					endpoint: endpoint.index,
+					message: `Endpoint ${endpoint.index} interview: Basic CC`,
+					level: "silly",
+				});
+
+				const action = await interviewEndpoint(
+					endpoint,
+					CommandClasses.Basic,
+				);
+				if (typeof action === "boolean") return action;
+			} else {
+				// The device is supposed to only control Basic CC
+				this.driver.controllerLog.logNode(this.id, {
+					endpoint: endpoint.index,
+					message:
+						"Endpoint implements Basic CC but is not supposed to support it. Assuming it only controls Basic CC...",
+				});
+			}
+		}
+
 		return true;
 	}
 
@@ -2989,16 +3035,6 @@ protocol version:      ${this.protocolVersion}`;
 	 * and certification requirements
 	 */
 	private modifySupportedCCBeforeInterview(endpoint: Endpoint): void {
-		const compat = this._deviceConfig?.compat;
-
-		// If the config file instructs us to expose Basic Set as an event, mark the CC as controlled
-		if (compat?.mapBasicSet === "event" && endpoint.index === 0) {
-			endpoint.addCC(CommandClasses.Basic, { isControlled: true });
-		}
-
-		// 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
 		// Multilevel Switch Command Class for a node (or endpoint) supporting the Window Covering CC, as it is a fully
@@ -3800,11 +3836,6 @@ protocol version:      ${this.protocolVersion}`;
 				if (!didSetMappedValue) {
 					// Store the value in the value DB now
 					command.persistValues(this.driver);
-
-					// Since the node sent us a Basic report, we are sure that it is at least supported
-					// If this is the only supported actuator CC, add it to the support list,
-					// so the information lands in the network cache
-					sourceEndpoint.maybeAddBasicCCAsFallback();
 				}
 			}
 		} else if (command instanceof BasicCCSet) {
@@ -6302,17 +6333,6 @@ protocol version:      ${this.protocolVersion}`;
 		// Restore the device config
 		await this.loadDeviceConfig();
 
-		// 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();
-		}
-		// }
-
 		// Mark already-interviewed nodes as potentially ready
 		if (this.interviewStage === InterviewStage.Complete) {
 			this.readyMachine.send("RESTART_FROM_CACHE");

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

@@ -1,5 +1,6 @@
 import { BasicCCValues } from "@zwave-js/cc";
 import { CommandClasses } from "@zwave-js/core";
+import path from "node:path";
 import { integrationTest } from "../integrationTestSuite";
 
 integrationTest(
@@ -44,3 +45,60 @@ integrationTest(
 		},
 	},
 );
+
+integrationTest(
+	"On devices that MUST not support Basic CC, and map Basic Set to a different CC, NO Basic CC values should be exposed",
+	{
+		// debug: true,
+
+		nodeCapabilities: {
+			manufacturerId: 0xdead,
+			productType: 0xbeef,
+			productId: 0xcafe,
+
+			// Routing Multilevel Sensor, MUST not support Basic CC
+			genericDeviceClass: 0x21,
+			specificDeviceClass: 0x01,
+			commandClasses: [
+				CommandClasses["Manufacturer Specific"],
+				CommandClasses.Version,
+				// But it reports support if asked
+				CommandClasses.Basic,
+			],
+		},
+
+		additionalDriverOptions: {
+			storage: {
+				deviceConfigPriorityDir: path.join(
+					__dirname,
+					"fixtures/mapBasicSetBinarySensor",
+				),
+			},
+		},
+
+		async testBody(t, driver, node, mockController, mockNode) {
+			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.false(
+				valueIDs.some((v) => BasicCCValues.compatEvent.is(v)),
+				"Found Basic CC compatEvent although it shouldn't be exposed",
+			);
+		},
+	},
+);

packages/zwave-js/src/lib/test/compat/fixtures/mapBasicSetBinarySensor/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": {
+		"mapBasicSet": "Binary Sensor"
+	}
+}