Skip to content

Commit

Permalink
fix: only mark Basic CC as controlled if compat flags are set (#7001)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlCalzone committed Jul 11, 2024
1 parent 8f60e3f commit ea4467f
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 32 deletions.
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
}
],
"dprint.experimentalLsp": true,
"dprint.path": "./node_modules/dprint/dprint",
"editor.defaultFormatter": "dprint.dprint",
"[jsonc]": {
"editor.formatOnSave": true,
Expand Down
23 changes: 8 additions & 15 deletions packages/cc/src/cc/BasicCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,27 +323,20 @@ remaining duration: ${basicResponse.duration?.toString() ?? "undefined"}`;
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));
}

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.supportsCC(CommandClasses.Basic) && (
(endpoint.controlsCC(CommandClasses.Basic)
&& compat?.mapBasicSet !== "Binary Sensor")
|| 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
}

if (endpoint.supportsCC(this.ccId)) {
// Defer to the base implementation if Basic CC is supported.
// This implies that no other actuator CC is supported.
ret.push(...super.getDefinedValueIDs(applHost));
} else if (endpoint.controlsCC(CommandClasses.Basic)) {
// During the interview, we mark Basic CC as controlled only if we want to expose currentValue
ret.push(BasicCCValues.currentValue.endpoint(endpoint.index));
}

Expand Down
1 change: 0 additions & 1 deletion packages/cc/src/cc/VersionCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ export class VersionCC extends CommandClass {
// for which support is determined later.
if (cc === CommandClasses.Basic) {
endpoint.addCC(cc, {
isControlled: true,
version: supportedVersion,
});
} else {
Expand Down
44 changes: 30 additions & 14 deletions packages/zwave-js/src/lib/node/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2393,8 +2393,6 @@ protocol version: ${this.protocolVersion}`;
if (typeof action === "boolean") return action;
}

// 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
this.modifySupportedCCBeforeInterview(this);

// We determine the correct interview order of the remaining CCs by topologically sorting two dependency graph
Expand Down Expand Up @@ -2754,7 +2752,11 @@ protocol version: ${this.protocolVersion}`;

// At the very end, figure out if Basic CC is supposed to be supported
// First on the root device
if (!this.wasCCRemovedViaConfig(CommandClasses.Basic)) {
const compat = this.deviceConfig?.compat;
if (
!this.wasCCRemovedViaConfig(CommandClasses.Basic)
&& this.getCCVersion(CommandClasses.Basic) > 0
) {
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
Expand All @@ -2767,14 +2769,18 @@ protocol version: ${this.protocolVersion}`;
const action = await interviewEndpoint(
this,
CommandClasses.Basic,
true,
);
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...",
});
// Consider the device to control Basic CC, but only if we want to expose the currentValue
if (
compat?.mapBasicReport === false
|| compat?.mapBasicSet === "report"
) {
// TODO: Figure out if we need to consider mapBasicSet === "auto" in the case where it falls back to Basic CC currentValue
this.addCC(CommandClasses.Basic, { isControlled: true });
}
}
}

Expand All @@ -2783,6 +2789,7 @@ protocol version: ${this.protocolVersion}`;
const endpoint = this.getEndpoint(endpointIndex);
if (!endpoint) continue;
if (endpoint.wasCCRemovedViaConfig(CommandClasses.Basic)) continue;
if (endpoint.getCCVersion(CommandClasses.Basic) === 0) continue;

if (endpoint.maySupportBasicCC()) {
// The endpoint probably supports Basic CC and is allowed to.
Expand All @@ -2796,15 +2803,20 @@ protocol version: ${this.protocolVersion}`;
const action = await interviewEndpoint(
endpoint,
CommandClasses.Basic,
true,
);
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...",
});
// Consider the device to control Basic CC, but only if we want to expose the currentValue
if (
compat?.mapBasicReport === false
|| compat?.mapBasicSet === "report"
) {
// TODO: Figure out if we need to consider mapBasicSet === "auto" in the case where it falls back to Basic CC currentValue
endpoint.addCC(CommandClasses.Basic, {
isControlled: true,
});
}
}
}

Expand All @@ -2818,6 +2830,10 @@ protocol version: ${this.protocolVersion}`;
public updateNodeInfo(nodeInfo: NodeUpdatePayload): void {
if (this.interviewStage < InterviewStage.NodeInfo) {
for (const cc of nodeInfo.supportedCCs) {
if (cc === CommandClasses.Basic) {
// Basic CC MUST not be in the NIF and we have special rules to determine support
continue;
}
this.addCC(cc, { isSupported: true });
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,35 @@ import path from "node:path";
import { integrationTest } from "../integrationTestSuite";

integrationTest(
"On devices that MUST not support Basic CC, but use Basic Set to report status, ONLY currentValue should be exposed",
"On devices that MUST not support Basic CC, and treat Basic Set as a report, ONLY currentValue 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/mapBasicSetReport",
),
},
},

async testBody(t, driver, node, mockController, mockNode) {
const valueIDs = node.getDefinedValueIDs();
t.true(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"manufacturer": "Test Manufacturer",
"manufacturerId": "0xdead",
"label": "Test Device",
"description": "With Basic Event",
"description": "With Basic Set as Binary Sensor",
"devices": [
{
"productType": "0xbeef",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"manufacturer": "Test Manufacturer",
"manufacturerId": "0xdead",
"label": "Test Device",
"description": "With Basic Set as Report",
"devices": [
{
"productType": "0xbeef",
"productId": "0xcafe"
}
],
"firmwareVersion": {
"min": "0.0",
"max": "255.255"
},
"compat": {
"mapBasicSet": "report"
}
}

0 comments on commit ea4467f

Please sign in to comment.