fix: recovery from missing SendData callback if command may be retr…

…ied (#6343)
zwave-js · Sep 29, 2023 · f365296 · f365296
1 parent a20a90c
commit f365296
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 10 deletions.
diff --git a/packages/core/src/error/ZWaveError.ts b/packages/core/src/error/ZWaveError.ts
@@ -268,7 +268,11 @@ export function isTransmissionError(e: unknown): e is ZWaveError & {
  *
  * This explicitly does not include transmission errors.
  */
-export function isRecoverableZWaveError(e: unknown): e is ZWaveError {
+export function isRecoverableZWaveError(e: unknown): e is ZWaveError & {
+	code:
+		| ZWaveErrorCodes.Controller_InterviewRestarted
+		| ZWaveErrorCodes.Controller_NodeRemoved;
+} {
 	if (!isZWaveError(e)) return false;
 	switch (e.code) {
 		case ZWaveErrorCodes.Controller_InterviewRestarted:
@@ -280,15 +284,21 @@ export function isRecoverableZWaveError(e: unknown): e is ZWaveError {
 
 export function isMissingControllerACK(
 	e: unknown,
-): e is ZWaveError {
+): e is ZWaveError & {
+	code: ZWaveErrorCodes.Controller_Timeout;
+	context: "ACK";
+} {
 	return isZWaveError(e)
 		&& e.code === ZWaveErrorCodes.Controller_Timeout
 		&& e.context === "ACK";
 }
 
 export function isMissingControllerCallback(
 	e: unknown,
-): e is ZWaveError {
+): e is ZWaveError & {
+	code: ZWaveErrorCodes.Controller_Timeout;
+	context: "callback";
+} {
 	return isZWaveError(e)
 		&& e.code === ZWaveErrorCodes.Controller_Timeout
 		&& e.context === "callback";

diff --git a/packages/zwave-js/src/lib/driver/Driver.ts b/packages/zwave-js/src/lib/driver/Driver.ts
@@ -4673,7 +4673,6 @@ ${handlers.length} left`,
 					// We got a result - it will be passed to the next iteration
 					break attemptMessage;
 				} catch (e: any) {
-					let delay = 0;
 					let zwError: ZWaveError;
 
 					if (!isZWaveError(e)) {
@@ -4691,14 +4690,12 @@ ${handlers.length} left`,
 							});
 							return;
 						} else if (
-							isSendData(msg)
-							&& e.code === ZWaveErrorCodes.Controller_Timeout
-							&& e.context === "callback"
+							isSendData(msg) && isMissingControllerCallback(e)
 						) {
 							// If the callback to SendData times out, we need to issue a SendDataAbort
 							await this.abortSendData();
-							// Wait a short amount of time so everything can settle
-							delay = 50;
+							// Reject the transaction - this will trigger the recovery mechanism and retry the command afterwards
+							throw e;
 						} else if (isMissingControllerACK(e)) {
 							// The controller is unresponsive. Reject the transaction, so we can attempt to recover
 							throw e;
@@ -4712,7 +4709,6 @@ ${handlers.length} left`,
 							)
 						) {
 							// Retry the command
-							if (delay) await wait(delay, true);
 							continue attemptMessage;
 						}
 

diff --git a/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts b/packages/zwave-js/src/lib/test/driver/sendDataMissingCallbackAbort.test.ts
@@ -213,3 +213,108 @@ integrationTest(
 		},
 	},
 );
+
+integrationTest(
+	"Missing callback recovery works if the command can be retried",
+	{
+		// debug: true,
+
+		// provisioningDirectory: path.join(
+		// 	__dirname,
+		// 	"__fixtures/supervision_binary_switch",
+		// ),
+
+		additionalDriverOptions: {
+			testingHooks: {
+				skipNodeInterview: true,
+			},
+		},
+
+		customSetup: async (driver, mockController, mockNode) => {
+			// This is almost a 1:1 copy of the default behavior, except that the callback never gets 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",
+							);
+						}
+
+						// Put the controller into sending state
+						controller.state.set(
+							MockControllerStateKeys.CommunicationState,
+							MockControllerCommunicationState.Sending,
+						);
+
+						// Notify the host that the message was sent
+						const res = new SendDataResponse(host, {
+							wasSent: true,
+						});
+						await controller.sendToHost(res.serialize());
+
+						return true;
+					} else if (msg instanceof SendDataAbort) {
+						// Put the controller into idle state
+						controller.state.set(
+							MockControllerStateKeys.CommunicationState,
+							MockControllerCommunicationState.Idle,
+						);
+
+						return true;
+					}
+				},
+			};
+			mockController.defineBehavior(handleBrokenSendData);
+
+			const handleSoftReset: MockControllerBehavior = {
+				onHostMessage(host, controller, msg) {
+					// Soft reset should restore normal operation
+					if (msg instanceof SoftResetRequest) {
+						shouldTimeOut = false;
+						return true;
+					}
+				},
+			};
+			mockController.defineBehavior(handleSoftReset);
+		},
+		testBody: async (t, driver, node, mockController, mockNode) => {
+			// Circumvent the options validation so the test doesn't take forever
+			driver.options.timeouts.sendDataCallback = 1500;
+
+			shouldTimeOut = true;
+
+			const firstCommand = node.commandClasses.Basic.set(99);
+			const followupCommand = node.commandClasses.Basic.set(0);
+
+			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,
+			);
+
+			// The ping and the followup command should eventually succeed
+			await firstCommand;
+			await followupCommand;
+
+			t.pass();
+		},
+	},
+);