Skip to content

Commit

Permalink
fix: ensure the normal Basic CC values are only exposed if they shoul…
Browse files Browse the repository at this point in the history
…d be, even with the compat `event` enabled (#6485)
  • Loading branch information
AlCalzone authored Oct 31, 2023
1 parent 6c33fdb commit 997d607
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 101 deletions.
40 changes: 30 additions & 10 deletions packages/cc/src/cc/BasicCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
MessagePriority,
type MessageRecord,
type SupervisionResult,
type ValueID,
ValueMetadata,
maybeUnknownToString,
parseMaybeNumber,
Expand Down Expand Up @@ -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",
Expand All @@ -80,9 +80,7 @@ export const BasicCCValues = Object.freeze({
} as const,
{
stateful: false,
autoCreate: (applHost, endpoint) =>
!!applHost.getDeviceConfig?.(endpoint.nodeId)?.compat
?.treatBasicSetAsEvent,
autoCreate: false,
},
),
}),
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion packages/zwave-js/src/lib/node/Endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 7 additions & 11 deletions packages/zwave-js/src/lib/node/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
4 changes: 3 additions & 1 deletion packages/zwave-js/src/lib/node/mockCCBehaviors/Basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/zwave-js/src/lib/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
78 changes: 0 additions & 78 deletions packages/zwave-js/src/lib/test/cc/BasicCC.integration.test.ts

This file was deleted.

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 Event",
"devices": [
{
"productType": "0xbeef",
"productId": "0xcafe"
}
],
"firmwareVersion": {
"min": "0.0",
"max": "255.255"
},
"compat": {
"treatBasicSetAsEvent": true
}
}
Loading

0 comments on commit 997d607

Please sign in to comment.