fix: reduce resopnse timeout back to 10s, add test for recovery

docs/api/driver.md

@@ -684,10 +684,10 @@ interface ZWaveOptions extends ZWaveHostOptions {
 		byte: number; // >=1, default: 150 ms
 
 		/**
-		 * How long to wait for a controller response. Usually this timeout should never elapse,
-		 * so this is merely a safeguard against the driver stalling.
+		 * How long to wait for a controller response. Usually this should never elapse, but when it does,
+		 * the driver will abort the transmission and try to recover the controller if it is unresponsive.
 		 */
-		response: number; // [500...60000], default: 30000 ms
+		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

packages/zwave-js/src/lib/driver/Driver.ts

@@ -251,8 +251,8 @@ const defaultOptions: ZWaveOptions = {
 		ack: 1000,
 		byte: 150,
 		// Ideally we'd want to have this as low as possible, but some
-		// 500 series controllers can take upwards of 10 seconds to respond sometimes.
-		response: 30000,
+		// 500 series controllers can take several seconds to respond sometimes.
+		response: 10000,
 		report: 1000, // ReportTime timeout SHOULD be set to CommandTime + 1 second
 		nonce: 5000,
 		sendDataCallback: 65000, // as defined in INS13954
@@ -5040,13 +5040,14 @@ ${handlers.length} left`,
 			}
 		});
 
-		this.serialAPIInterpreter.onTransition((state) => {
-			if (state.changed) {
-				this.driverLog.print(
-					`CMDMACHINE: ${JSON.stringify(state.toStrings())}`,
-				);
-			}
-		});
+		// Uncomment this for debugging state machine transitions
+		// this.serialAPIInterpreter.onTransition((state) => {
+		// 	if (state.changed) {
+		// 		this.driverLog.print(
+		// 			`CMDMACHINE: ${JSON.stringify(state.toStrings())}`,
+		// 		);
+		// 	}
+		// });
 
 		this.serialAPIInterpreter.start();
 

packages/zwave-js/src/lib/driver/ZWaveOptions.ts

@@ -15,10 +15,10 @@ export interface ZWaveOptions extends ZWaveHostOptions {
 		byte: number; // >=1, default: 150 ms
 
 		/**
-		 * How long to wait for a controller response. Usually this timeout should never elapse,
-		 * so this is merely a safeguard against the driver stalling.
+		 * How long to wait for a controller response. Usually this should never elapse, but when it does,
+		 * the driver will abort the transmission and try to recover the controller if it is unresponsive.
 		 */
-		response: number; // [500...60000], default: 30000 ms
+		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

packages/zwave-js/src/lib/test/driver/sendDataMissingResponse.test.ts

@@ -26,7 +26,7 @@ let lastCallbackId: number;
 integrationTest(
 	"Abort transmission and wait for callback if SendData is missing the response",
 	{
-		debug: true,
+		// debug: true,
 
 		additionalDriverOptions: {
 			testingHooks: {
@@ -117,6 +117,81 @@ integrationTest(
 	},
 );
 
+integrationTest(
+	"Recover controller if callback times out after timed out SendData response",
+	{
+		// debug: true,
+
+		additionalDriverOptions: {
+			testingHooks: {
+				skipNodeInterview: true,
+			},
+		},
+
+		customSetup: async (driver, mockController, mockNode) => {
+			// This is almost a 1:1 copy of the default behavior, except that the response and callback never get sent
+			const handleBrokenSendData: MockControllerBehavior = {
+				async onHostMessage(host, controller, msg) {
+					// If the controller is operating normally, defer to the default behavior
+					if (!shouldTimeOut) return false;
+
+					if (msg instanceof SendDataRequest) {
+						// Check if this command is legal right now
+						const state = controller.state.get(
+							MockControllerStateKeys.CommunicationState,
+						) as MockControllerCommunicationState | undefined;
+						if (
+							state != undefined
+							&& state !== MockControllerCommunicationState.Idle
+						) {
+							throw new Error(
+								"Received SendDataRequest while not idle",
+							);
+						}
+
+						lastCallbackId = msg.callbackId;
+
+						// Don't send the response or the callback
+
+						return true;
+					} else if (msg instanceof SendDataAbort) {
+						// Return to normal operation
+						shouldTimeOut = false;
+					}
+				},
+			};
+			mockController.defineBehavior(handleBrokenSendData);
+		},
+		testBody: async (t, driver, node, mockController, mockNode) => {
+			// Circumvent the options validation so the test doesn't take forever
+			driver.options.timeouts.response = 500;
+			driver.options.timeouts.sendDataCallback = 1500;
+
+			node.markAsAlive();
+			shouldTimeOut = true;
+
+			const basicSetPromise = node.commandClasses.Basic.set(99);
+
+			await wait(2000);
+
+			mockController.assertReceivedHostMessage(
+				(msg) => msg.functionType === FunctionType.SendDataAbort,
+			);
+			mockController.clearReceivedHostMessages();
+
+			// The stick should have been soft-reset
+			await wait(1000);
+			mockController.assertReceivedHostMessage(
+				(msg) => msg.functionType === FunctionType.SoftReset,
+			);
+
+			// And the command should eventually succeed
+			await basicSetPromise;
+			t.pass();
+		},
+	},
+);
+
 // integrationTest(
 // 	"Mark node as dead if SendData is still missing the callback after soft-reset",
 // 	{