Skip to content

Commit

Permalink
fix: only expose currentValue on devices that use Basic CC for reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
AlCalzone committed Nov 30, 2023
1 parent a58ec5d commit bcfb1d0
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 7 deletions.
6 changes: 6 additions & 0 deletions maintenance/schemas/deviceClasses.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
"controlledCCs": {
"$ref": "#/definitions/CCs"
},
"maySupportBasicCC": {
"const": false
},
"specific": {
"type": "object",
"patternProperties": {
Expand All @@ -45,6 +48,9 @@
},
"controlledCCs": {
"$ref": "#/definitions/CCs"
},
"maySupportBasicCC": {
"const": false
}
},
"required": ["label"],
Expand Down
5 changes: 4 additions & 1 deletion packages/cc/src/cc/BasicCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,15 @@ remaining duration: ${basicResponse.duration?.toString() ?? "undefined"}`;
ret.push(...super.getDefinedValueIDs(applHost));
}

// Add the compat event value if it should be exposed
if (
!!applHost.getDeviceConfig?.(endpoint.nodeId)?.compat
?.treatBasicSetAsEvent
) {
// 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
ret.push(BasicCCValues.currentValue.endpoint(endpoint.index));
}

return ret;
Expand Down
2 changes: 2 additions & 0 deletions packages/cc/src/cc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,8 @@ export type {
MultilevelSensorCCGetOptions,
MultilevelSensorCCGetSupportedScaleOptions,
MultilevelSensorCCReportOptions,
MultilevelSensorCCSupportedScaleReportOptions,
MultilevelSensorCCSupportedSensorReportOptions,
} from "./MultilevelSensorCC";
export {
MultilevelSensorCC,
Expand Down
18 changes: 13 additions & 5 deletions packages/config/config/deviceClasses.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"0x01": {
"label": "Remote Controller",
"controlledCCs": ["Basic"],
"maySupportBasicCC": false,
"specific": {
"0x01": {
"label": "Portable Remote Controller",
Expand Down Expand Up @@ -164,6 +165,7 @@
"0x07": {
"label": "Gateway",
"zwavePlusDeviceType": "Gateway",
"maySupportBasicCC": false,
"supportedCCs": [
"Manufacturer Specific",
// "Security", (some devices only support S2)
Expand Down Expand Up @@ -252,7 +254,8 @@
"specific": {
"0x01": {
"label": "Notification Sensor",
"zwavePlusDeviceType": "Sensor - Notification"
"zwavePlusDeviceType": "Sensor - Notification",
"maySupportBasicCC": false
}
}
},
Expand Down Expand Up @@ -327,11 +330,13 @@
"specific": {
"0x01": {
"label": "Repeater Slave",
"zwavePlusDeviceType": "Repeater"
"zwavePlusDeviceType": "Repeater",
"maySupportBasicCC": false
},
"0x03": {
"label": "IR Repeater",
"zwavePlusDeviceType": "IR Repeater",
"maySupportBasicCC": false,
"supportedCCs": [
"Association", // V2+
"Association Group Information",
Expand Down Expand Up @@ -624,7 +629,8 @@
"specific": {
"0x01": {
"label": "Basic Wall Controller",
"zwavePlusDeviceType": "Wall Controller"
"zwavePlusDeviceType": "Wall Controller",
"maySupportBasicCC": false
}
}
},
Expand All @@ -643,7 +649,8 @@
"specific": {
"0x01": {
"label": "Routing Multilevel Sensor",
"zwavePlusDeviceType": "Sensor - Multilevel"
"zwavePlusDeviceType": "Sensor - Multilevel",
"maySupportBasicCC": false
}
}
},
Expand All @@ -653,7 +660,7 @@
},
"0x31": {
"label": "Meter",
"supportedCCs": ["Basic"],
"maySupportBasicCC": false,
"specific": {
"0x01": {
"label": "Simple Meter",
Expand Down Expand Up @@ -843,6 +850,7 @@
"label": "Secure Keypad",
"zwavePlusDeviceType": "Entry Control Keypad",
"requiresSecurity": true,
"maySupportBasicCC": false,
"supportedCCs": [
// "Device Reset Locally", (we can't know if the device can be reset)
"Entry Control",
Expand Down
32 changes: 32 additions & 0 deletions packages/config/src/DeviceClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,21 @@ export class GenericDeviceClass {
this.controlledCCs = [];
}

if (definition.maySupportBasicCC != undefined) {
if (definition.maySupportBasicCC !== false) {
throwInvalidConfig(
"device classes",
`maySupportBasicCC in device class ${this.label} (${
num2hex(this.key)
}) must be false or omitted (= true)!`,
);
} else {
this.maySupportBasicCC = false;
}
} else {
this.maySupportBasicCC = true;
}

const specific = new Map<number, SpecificDeviceClass>();
if (isObject(definition.specific)) {
for (
Expand Down Expand Up @@ -190,6 +205,7 @@ export class GenericDeviceClass {
public readonly requiresSecurity?: boolean;
public readonly supportedCCs: readonly CommandClasses[];
public readonly controlledCCs: readonly CommandClasses[];
public readonly maySupportBasicCC: boolean;
public readonly specific: ReadonlyMap<number, SpecificDeviceClass>;
}

Expand Down Expand Up @@ -312,6 +328,21 @@ export class SpecificDeviceClass {
...generic.controlledCCs,
...this.controlledCCs,
]);

if (definition.maySupportBasicCC != undefined) {
if (definition.maySupportBasicCC !== false) {
throwInvalidConfig(
"device classes",
`maySupportBasicCC in device class ${generic.label} -> ${this.label} (${
num2hex(this.key)
}) must be false or omitted (= true)!`,
);
} else {
this.maySupportBasicCC = false;
}
} else {
this.maySupportBasicCC = true;
}
}

public readonly key: number;
Expand All @@ -320,4 +351,5 @@ export class SpecificDeviceClass {
public readonly requiresSecurity?: boolean;
public readonly supportedCCs: readonly CommandClasses[];
public readonly controlledCCs: readonly CommandClasses[];
public readonly maySupportBasicCC: boolean;
}
25 changes: 24 additions & 1 deletion packages/zwave-js/src/lib/node/Endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,16 +226,39 @@ 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 */
public maySupportBasicCC(): boolean {
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
Expand All @@ -246,7 +269,7 @@ export class Endpoint implements IZWaveEndpoint {
// 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 the record is now only a dummy, remove the CC entirely
if (
!this.supportsCC(CommandClasses.Basic)
&& !this.controlsCC(CommandClasses.Basic)
Expand Down
7 changes: 7 additions & 0 deletions packages/zwave-js/src/lib/node/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1816,6 +1816,13 @@ 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { BasicCCValues } from "@zwave-js/cc";
import { CommandClasses } from "@zwave-js/core";
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",
{
debug: true,

nodeCapabilities: {
// Routing Multilevel Sensor, MUST not support Basic CC
genericDeviceClass: 0x21,
specificDeviceClass: 0x01,
commandClasses: [
CommandClasses.Version,
// But it reports support if asked
CommandClasses.Basic,
],
},

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.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",
);
},
},
);

0 comments on commit bcfb1d0

Please sign in to comment.