Skip to content

Commit

Permalink
fix: implement workaround for some 500 series controllers replying wi…
Browse files Browse the repository at this point in the history
…th invalid callbacks in case of SUC route assignment failure (#6370)
  • Loading branch information
AlCalzone authored Oct 5, 2023
1 parent 2da6d9e commit ae9a87b
Show file tree
Hide file tree
Showing 15 changed files with 466 additions and 57 deletions.
6 changes: 6 additions & 0 deletions docs/config-files/file-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ Several command classes are refreshed regularly (every couple of hours) if they

By default, received `Basic CC::Report` commands are mapped to a more appropriate CC. Setting `disableBasicMapping` to `true` disables this feature.

### `disableCallbackFunctionTypeCheck`

By default, responses or callbacks for Serial API commands must have the same function type (command identifier) in order to be recognized. However, in some situations, certain controllers send a callback with an invalid function type. In this case, the faulty commands may be listed in the `disableCallbackFunctionTypeCheck` array to disable the check for a matching function type.

> [!NOTE] This compat flag requires command-specific support and is not a generic escape hatch.
### `disableStrictEntryControlDataValidation`

The specifications mandate strict rules for the data and sequence numbers in `Entry Control CC Notifications`, which some devices do not follow, causing the notifications to get dropped. Setting `disableStrictEntryControlDataValidation` to `true` disables these strict checks.
Expand Down
8 changes: 8 additions & 0 deletions maintenance/schemas/device-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,14 @@
"disableBasicMapping": {
"const": true
},
"disableCallbackFunctionTypeCheck": {
"type": "array",
"items": {
"type": "integer",
"minimum": 1
},
"minItems": 1
},
"disableStrictEntryControlDataValidation": {
"const": true
},
Expand Down
8 changes: 8 additions & 0 deletions packages/config/config/devices/0x0000/husbzb-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,13 @@
"firmwareVersion": {
"min": "0.0",
"max": "255.255"
},
"compat": {
// In some situations, AssignSUCReturnRoute and DeleteSUCReturnRoute get answered with a wrong callback function type,
// triggering Z-Wave JS's unresponsive controller check.
"disableCallbackFunctionTypeCheck": [
81, // AssignSUCReturnRoute
85 // DeleteSUCReturnRoute
]
}
}
8 changes: 8 additions & 0 deletions packages/config/config/devices/0x0086/zw090.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,13 @@
"metadata": {
"reset": "Use this procedure only in the event that the primary controller is missing or otherwise inoperable.\n\nPress and hold the Action Button on Z-Stick for 20 seconds and then release",
"manual": "https://products.z-wavealliance.org/ProductManual/File?folder=&filename=MarketCertificationFiles/1345/Z%20Stick%20Gen5%20manual%201.pdf"
},
"compat": {
// In some situations, AssignSUCReturnRoute and DeleteSUCReturnRoute get answered with a wrong callback function type,
// triggering Z-Wave JS's unresponsive controller check.
"disableCallbackFunctionTypeCheck": [
81, // AssignSUCReturnRoute
85 // DeleteSUCReturnRoute
]
}
}
19 changes: 19 additions & 0 deletions packages/config/src/devices/CompatConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,23 @@ compat option disableBasicMapping must be true or omitted`,
this.disableBasicMapping = definition.disableBasicMapping;
}

if (definition.disableCallbackFunctionTypeCheck != undefined) {
if (
!isArray(definition.disableCallbackFunctionTypeCheck)
|| !definition.disableCallbackFunctionTypeCheck.every(
(d: any) => typeof d === "number" && d % 1 === 0 && d > 0,
)
) {
throwInvalidConfig(
"devices",
`config/devices/${filename}:
when present, compat option disableCallbackFunctionTypeCheck msut be an array of positive integers`,
);
}
this.disableCallbackFunctionTypeCheck =
definition.disableCallbackFunctionTypeCheck;
}

if (definition.disableStrictEntryControlDataValidation != undefined) {
if (definition.disableStrictEntryControlDataValidation !== true) {
throwInvalidConfig(
Expand Down Expand Up @@ -568,6 +585,7 @@ compat option overrideQueries must be an object!`,
public readonly disableBasicMapping?: boolean;
public readonly disableStrictEntryControlDataValidation?: boolean;
public readonly disableStrictMeasurementValidation?: boolean;
public readonly disableCallbackFunctionTypeCheck?: number[];
public readonly enableBasicSetMapping?: boolean;
public readonly forceNotificationIdleReset?: boolean;
public readonly forceSceneControllerGroupCount?: number;
Expand Down Expand Up @@ -608,6 +626,7 @@ compat option overrideQueries must be an object!`,
"removeCCs",
"disableAutoRefresh",
"disableBasicMapping",
"disableCallbackFunctionTypeCheck",
"disableStrictEntryControlDataValidation",
"disableStrictMeasurementValidation",
"enableBasicSetMapping",
Expand Down
20 changes: 13 additions & 7 deletions packages/serial/src/message/Message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export type MessageOptions =
*/
export class Message {
public constructor(
protected host: ZWaveHost,
public readonly host: ZWaveHost,
options: MessageOptions = {},
) {
// decide which implementation we follow
Expand Down Expand Up @@ -355,12 +355,18 @@ export class Message {
/** Checks if a message is an expected callback for this message */
public isExpectedCallback(msg: Message): boolean {
if (msg.type !== MessageType.Request) return false;
// If a received request included a callback id, enforce that the response contains the same
if (
this.hasCallbackId()
&& (!msg.hasCallbackId() || this._callbackId !== msg._callbackId)
) {
return false;

// Some controllers have a bug causing them to send a callback with a function type of 0 and no callback ID
// To prevent this from triggering the unresponsive controller detection we need to forward these messages as if they were correct
if (msg.functionType !== 0 as any) {
// If a received request included a callback id, enforce that the response contains the same
if (
this.hasCallbackId()
&& (!msg.hasCallbackId()
|| this._callbackId !== msg._callbackId)
) {
return false;
}
}

return this.testMessage(msg, this.expectedCallback);
Expand Down
26 changes: 15 additions & 11 deletions packages/testing/src/MockControllerCapabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,27 @@ export interface MockControllerCapabilities {
watchdogEnabled: boolean;
}

export function getDefaultSupportedFunctionTypes(): FunctionType[] {
return [
FunctionType.GetSerialApiInitData,
FunctionType.GetControllerCapabilities,
FunctionType.SendData,
FunctionType.SendDataMulticast,
FunctionType.GetControllerVersion,
FunctionType.GetControllerId,
FunctionType.GetNodeProtocolInfo,
FunctionType.RequestNodeInfo,
FunctionType.AssignSUCReturnRoute,
];
}

export function getDefaultMockControllerCapabilities(): MockControllerCapabilities {
return {
firmwareVersion: "1.0",
manufacturerId: 0xffff,
productType: 0xffff,
productId: 0xfffe,
supportedFunctionTypes: [
FunctionType.GetSerialApiInitData,
FunctionType.GetControllerCapabilities,
FunctionType.SendData,
FunctionType.SendDataMulticast,
FunctionType.GetControllerVersion,
FunctionType.GetControllerId,
FunctionType.GetNodeProtocolInfo,
FunctionType.RequestNodeInfo,
FunctionType.AssignSUCReturnRoute,
],
supportedFunctionTypes: getDefaultSupportedFunctionTypes(),

controllerType: ZWaveLibraryTypes["Static Controller"],
libraryVersion: "Z-Wave 7.17.99",
Expand Down
1 change: 1 addition & 0 deletions packages/testing/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from "./CCSpecificCapabilities";
export * from "./MockController";
export { getDefaultSupportedFunctionTypes } from "./MockControllerCapabilities";
export * from "./MockNode";
export { ccCaps } from "./MockNodeCapabilities";
export type { PartialCCCapabilities } from "./MockNodeCapabilities";
Expand Down
34 changes: 26 additions & 8 deletions packages/zwave-js/src/lib/controller/Controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,15 @@ import {
} from "../serialapi/network-mgmt/AssignReturnRouteMessages";
import {
AssignSUCReturnRouteRequest,
type AssignSUCReturnRouteRequestTransmitReport,
AssignSUCReturnRouteRequestTransmitReport,
} from "../serialapi/network-mgmt/AssignSUCReturnRouteMessages";
import {
DeleteReturnRouteRequest,
type DeleteReturnRouteRequestTransmitReport,
} from "../serialapi/network-mgmt/DeleteReturnRouteMessages";
import {
DeleteSUCReturnRouteRequest,
type DeleteSUCReturnRouteRequestTransmitReport,
DeleteSUCReturnRouteRequestTransmitReport,
} from "../serialapi/network-mgmt/DeleteSUCReturnRouteMessages";
import {
GetPriorityRouteRequest,
Expand Down Expand Up @@ -4319,14 +4319,23 @@ ${associatedNodes.join(", ")}`,
await this.deleteSUCReturnRoutes(nodeId);

try {
const result = await this.driver.sendMessage<
AssignSUCReturnRouteRequestTransmitReport
>(
const result = await this.driver.sendMessage(
new AssignSUCReturnRouteRequest(this.driver, {
nodeId,
}),
);

if (
!(result instanceof AssignSUCReturnRouteRequestTransmitReport)
) {
this.driver.controllerLog.logNode(
nodeId,
`Assigning SUC return route failed: Invalid callback received`,
"error",
);
return false;
}

const success = this.handleRouteAssignmentTransmitReport(
result,
nodeId,
Expand Down Expand Up @@ -4492,14 +4501,23 @@ ${associatedNodes.join(", ")}`,
});

try {
const result = await this.driver.sendMessage<
DeleteSUCReturnRouteRequestTransmitReport
>(
const result = await this.driver.sendMessage(
new DeleteSUCReturnRouteRequest(this.driver, {
nodeId,
}),
);

if (
!(result instanceof DeleteSUCReturnRouteRequestTransmitReport)
) {
this.driver.controllerLog.logNode(
nodeId,
`Deleting SUC return route failed: Invalid callback received`,
"error",
);
return false;
}

const success = this.handleRouteAssignmentTransmitReport(
result,
nodeId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,25 @@ export interface AssignSUCReturnRouteRequestOptions extends MessageBaseOptions {
nodeId: number;
}

function testAssignSUCReturnRouteCallback(
sent: AssignSUCReturnRouteRequest,
callback: Message,
): boolean {
// Some controllers have a bug where they incorrectly respond with DeleteSUCReturnRoute
if (
callback.host
.getDeviceConfig?.(callback.host.ownNodeId)
?.compat
?.disableCallbackFunctionTypeCheck
?.includes(FunctionType.AssignSUCReturnRoute)
) {
return true;
}
return callback.functionType === FunctionType.AssignSUCReturnRoute;
}

@expectedResponse(FunctionType.AssignSUCReturnRoute)
@expectedCallback(FunctionType.AssignSUCReturnRoute)
@expectedCallback(testAssignSUCReturnRouteCallback)
export class AssignSUCReturnRouteRequest extends AssignSUCReturnRouteRequestBase
implements INodeQuery
{
Expand Down
Loading

0 comments on commit ae9a87b

Please sign in to comment.