Skip to content

Commit

Permalink
fix: do not persist or map values form force-removed CCs, do not pers…
Browse files Browse the repository at this point in the history
…ist values from mapped CCs (#6760)
  • Loading branch information
AlCalzone authored Apr 11, 2024
1 parent 07fe7d6 commit b924bb5
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 9 deletions.
37 changes: 36 additions & 1 deletion packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import {
deserializeCacheValue,
getCCName,
highResTimestamp,
isEncapsulationCC,
isLongRangeNodeId,
isMissingControllerACK,
isMissingControllerCallback,
Expand Down Expand Up @@ -4710,9 +4711,43 @@ ${handlers.length} left`,
}
}

private shouldPersistCCValues(cc: CommandClass): boolean {
// Always persist encapsulation CCs, otherwise interviews don't work.
if (isEncapsulationCC(cc.ccId)) return true;

// Do not persist values for a node or endpoint that does not exist
const endpoint = this.tryGetEndpoint(cc);
const node = endpoint?.getNodeUnsafe();
if (!node) return false;

// Do not persist values for a CC that was force-removed via config
if (endpoint?.wasCCRemovedViaConfig(cc.ccId)) return false;

// Do not persist values for a CC that's being mapped to another endpoint.
// FIXME: This duplicates logic in Node.ts -> handleCommand
const compatConfig = node?.deviceConfig?.compat;
if (
cc.endpointIndex === 0
&& cc.constructor.name.endsWith("Report")
&& node.getEndpointCount() >= 1
// Only map reports from the root device to an endpoint if we know which one
&& compatConfig?.mapRootReportsToEndpoint != undefined
) {
const targetEndpoint = node.getEndpoint(
compatConfig.mapRootReportsToEndpoint,
);
if (targetEndpoint?.supportsCC(cc.ccId)) return false;
}

return true;
}

/** Persists the values contained in a Command Class in the corresponding nodes's value DB */
private persistCCValues(cc: CommandClass) {
cc.persistValues(this);
if (this.shouldPersistCCValues(cc)) {
cc.persistValues(this);
}

if (isEncapsulatingCommandClass(cc)) {
this.persistCCValues(cc.encapsulated);
} else if (isMultiEncapsulatingCommandClass(cc)) {
Expand Down
13 changes: 13 additions & 0 deletions packages/zwave-js/src/lib/node/Endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,19 @@ export class Endpoint implements IZWaveEndpoint {
}
}

/** Determines if support for a CC was force-removed via config file */
public wasCCRemovedViaConfig(cc: CommandClasses): boolean {
if (this.supportsCC(cc)) return false;

const compatConfig = this.getNodeUnsafe()?.deviceConfig?.compat;
if (!compatConfig) return false;

const removedEndpoints = compatConfig.removeCCs?.get(cc);
if (!removedEndpoints) return false;

return removedEndpoints == "*" || removedEndpoints.includes(this.index);
}

/**
* Retrieves the version of the given CommandClass this endpoint implements.
* Returns 0 if the CC is not supported.
Expand Down
23 changes: 17 additions & 6 deletions packages/zwave-js/src/lib/node/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3042,6 +3042,21 @@ protocol version: ${this.protocolVersion}`;
this.markAsAwake();
}

// If the received CC was force-removed via config file, ignore it completely
const endpoint = this.getEndpoint(command.endpointIndex);
if (endpoint?.wasCCRemovedViaConfig(command.ccId)) {
this.driver.controllerLog.logNode(
this.id,
{
endpoint: endpoint.index,
direction: "inbound",
message:
`Ignoring ${command.constructor.name} because CC support was removed via config file`,
},
);
return;
}

if (command instanceof BasicCC) {
return this.handleBasicCommand(command);
} else if (command instanceof MultilevelSwitchCC) {
Expand Down Expand Up @@ -3662,11 +3677,7 @@ protocol version: ${this.protocolVersion}`;
// 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
if (!actuatorCCs.some((cc) => sourceEndpoint.supportsCC(cc))) {
sourceEndpoint.addCC(CommandClasses.Basic, {
isControlled: true,
});
}
sourceEndpoint.maybeAddBasicCCAsFallback();
}
} else if (command instanceof BasicCCSet) {
// Treat BasicCCSet as value events if desired
Expand Down Expand Up @@ -3708,7 +3719,7 @@ protocol version: ${this.protocolVersion}`;
),
command.targetValue,
);
// Since the node sent us a Basic command, we are sure that it is at least controlled
// Since the node sent us a Basic Set, we are sure that it is at least controlled
// Add it to the support list, so the information lands in the network cache
if (!sourceEndpoint.controlsCC(CommandClasses.Basic)) {
sourceEndpoint.addCC(CommandClasses.Basic, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ test.beforeEach(async (t) => {
removeAllListeners: () => {},
} as any;

driver.controller.nodes.getOrThrow = (nodeId: number) => {
const node = driver.controller.nodes.get(nodeId);
if (!node) throw new Error(`Node ${nodeId} not found`);
return node;
};

t.context = { driver, serialport };
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ integrationTest(
},
);

integrationTest.only(
integrationTest(
"The unresponsive controller recovery does not kick in when it was enabled via config",
{
debug: true,
// debug: true,

additionalDriverOptions: {
attempts: {
Expand Down

0 comments on commit b924bb5

Please sign in to comment.