feat: abort Send Data commands early after timeout, reduce callback…

… timeout (#6411)
zwave-js · Oct 17, 2023 · b687c16 · b687c16
1 parent 7f8f5b1
commit b687c16
Show file tree

Hide file tree

Showing 5 changed files with 429 additions and 6 deletions.
diff --git a/docs/api/driver.md b/docs/api/driver.md
@@ -689,8 +689,17 @@ interface ZWaveOptions extends ZWaveHostOptions {
 		 */
 		response: number; // [500...60000], default: 10000 ms
 
-		/** How long to wait for a callback from the host for a SendData[Multicast]Request */
-		sendDataCallback: number; // >=10000, default: 65000 ms
+		/**
+		 * How long to wait for a callback from the host for a SendData[Multicast]Request
+		 * before aborting the transmission.
+		 */
+		sendDataAbort: number; // >=5000, <=(sendDataCallback - 5000), default: 20000 ms
+
+		/**
+		 * How long to wait for a callback from the host for a SendData[Multicast]Request
+		 * before considering the controller unresponsive.
+		 */
+		sendDataCallback: number; // >=10000, default: 30000 ms
 
 		/** How much time a node gets to process a request and send a response */
 		report: number; // [500...10000], default: 1000 ms

diff --git a/packages/zwave-js/src/lib/driver/Driver.ts b/packages/zwave-js/src/lib/driver/Driver.ts
@@ -255,7 +255,8 @@ const defaultOptions: ZWaveOptions = {
 		response: 10000,
 		report: 1000, // ReportTime timeout SHOULD be set to CommandTime + 1 second
 		nonce: 5000,
-		sendDataCallback: 65000, // as defined in INS13954
+		sendDataAbort: 20000, // If a controller takes over 15s to reach a node, it's probably not going to happen
+		sendDataCallback: 30000, // INS13954 defines this to be 65000 ms, but waiting that long causes issues with reporting devices
 		sendToSleep: 250, // The default should be enough time for applications to react to devices waking up
 		retryJammed: 1000,
 		refreshValue: 5000, // Default should handle most slow devices until we have a better solution
@@ -340,6 +341,18 @@ function checkOptions(options: ZWaveOptions): void {
 			ZWaveErrorCodes.Driver_InvalidOptions,
 		);
 	}
+	if (
+		options.timeouts.sendDataAbort < 5000
+		|| options.timeouts.sendDataAbort
+			> options.timeouts.sendDataCallback - 5000
+	) {
+		throw new ZWaveError(
+			`The Send Data Abort Callback timeout must be between 5000 and ${
+				options.timeouts.sendDataCallback - 5000
+			} milliseconds!`,
+			ZWaveErrorCodes.Driver_InvalidOptions,
+		);
+	}
 	if (
 		options.timeouts.serialAPIStarted < 1000
 		|| options.timeouts.serialAPIStarted > 30000

diff --git a/packages/zwave-js/src/lib/driver/SerialAPICommandMachine.ts b/packages/zwave-js/src/lib/driver/SerialAPICommandMachine.ts
@@ -139,7 +139,7 @@ export type SerialAPICommandMachineOptions = Partial<
 export type SerialAPICommandMachineParams = {
 	timeouts: Pick<
 		ZWaveOptions["timeouts"],
-		"ack" | "response" | "sendDataCallback"
+		"ack" | "response" | "sendDataAbort" | "sendDataCallback"
 	>;
 	attempts: Pick<ZWaveOptions["attempts"], "controller">;
 };
@@ -312,6 +312,13 @@ export function getSerialAPICommandMachineConfig(
 					],
 				},
 				after: {
+					// Abort Send Data when it takes too long
+					SENDDATA_ABORT_TIMEOUT: {
+						cond: "isSendData",
+						actions: [
+							() => sendDataAbort(),
+						],
+					},
 					CALLBACK_TIMEOUT: {
 						target: "failure",
 						actions: assign({
@@ -446,6 +453,7 @@ export function getSerialAPICommandMachineOptions(
 					|| timeoutConfig.sendDataCallback
 				);
 			},
+			SENDDATA_ABORT_TIMEOUT: timeoutConfig.sendDataAbort,
 			ACK_TIMEOUT: timeoutConfig.ack,
 		},
 	};

diff --git a/packages/zwave-js/src/lib/driver/ZWaveOptions.ts b/packages/zwave-js/src/lib/driver/ZWaveOptions.ts
@@ -20,8 +20,17 @@ export interface ZWaveOptions extends ZWaveHostOptions {
 		 */
 		response: number; // [500...60000], default: 10000 ms
 
-		/** How long to wait for a callback from the host for a SendData[Multicast]Request */
-		sendDataCallback: number; // >=10000, default: 65000 ms
+		/**
+		 * How long to wait for a callback from the host for a SendData[Multicast]Request
+		 * before aborting the transmission.
+		 */
+		sendDataAbort: number; // >=5000, <=(sendDataCallback - 5000), default: 20000 ms
+
+		/**
+		 * How long to wait for a callback from the host for a SendData[Multicast]Request
+		 * before considering the controller unresponsive.
+		 */
+		sendDataCallback: number; // >=10000, default: 30000 ms
 
 		/** How much time a node gets to process a request and send a response */
 		report: number; // [500...10000], default: 1000 ms