fix: reopen serial port if controller is missing ACKs after soft-reset (

#6477)

packages/serial/src/SerialPortBindingMock.ts

@@ -151,6 +151,7 @@ export const MockBinding: MockBindingInterface = {
 
 interface MockPortBindingEvents {
 	write: (data: Buffer) => void;
+	close: () => void;
 }
 
 /**
@@ -221,6 +222,8 @@ export class MockPortBinding extends TypedEventEmitter<MockPortBindingEvents>
 		if (this.pendingRead) {
 			this.pendingRead(new CanceledError("port is closed"));
 		}
+
+		this.emit("close");
 	}
 
 	async read(

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

@@ -513,7 +513,8 @@ type PrefixedNodeEvents = {
 
 const enum ControllerRecoveryPhase {
 	None,
-	NoACK,
+	ACKTimeout,
+	ACKTimeoutAfterReset,
 	CallbackTimeout,
 	CallbackTimeoutAfterReset,
 }
@@ -3529,46 +3530,73 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
 	): boolean {
 		if (!this._controller) return false;
 
-		const fail = (message: string) => {
-			this.rejectTransaction(transaction, error);
-			void this.destroyWithMessage(message);
+		const recoverByReopeningSerialport = async () => {
+			if (!this.serial) return;
+			this.driverLog.print(
+				"Attempting to recover unresponsive controller by reopening the serial port...",
+				"warn",
+			);
+			if (this.serial.isOpen) await this.serial.close();
+			await wait(1000);
+			await this.openSerialport();
+
+			this.driverLog.print(
+				"Serial port reopened. Returning to normal operation and hoping for the best...",
+				"warn",
+			);
+
+			// We don't know if this worked
+			// Go back to normal operation and hope for the best.
+			this._controller?.setStatus(ControllerStatus.Ready);
+			this._recoveryPhase = ControllerRecoveryPhase.None;
 		};
 
-		if (this._controller.status !== ControllerStatus.Unresponsive) {
+		if (
+			(this._controller.status !== ControllerStatus.Unresponsive
+				&& !this.maySoftReset())
+			|| this._recoveryPhase
+				=== ControllerRecoveryPhase.ACKTimeoutAfterReset
+		) {
+			// Either we can/could not do a soft reset or the controller is still timing out afterwards
+			void recoverByReopeningSerialport().catch(noop);
+
+			return true;
+		} else if (this._controller.status !== ControllerStatus.Unresponsive) {
 			// The controller was responsive before this transaction failed.
 			// Mark it as unresponsive and try to soft-reset it.
 			this.controller.setStatus(
 				ControllerStatus.Unresponsive,
 			);
 
-			this._recoveryPhase = ControllerRecoveryPhase.NoACK;
+			this._recoveryPhase = ControllerRecoveryPhase.ACKTimeout;
 
 			this.driverLog.print(
-				"Attempting to recover unresponsive controller...",
+				"Attempting to recover unresponsive controller by restarting it...",
 				"warn",
 			);
 
-			// Re-queue the transaction.
-			// Its message generator may have finished, so reset that too.
-			transaction.reset();
-			this.queue.add(transaction.clone());
-
 			// Execute the soft-reset asynchronously
 			void this.softReset().then(() => {
 				// The controller responded. It is no longer unresponsive
+
+				// Re-queue the transaction, so it can get handled next.
+				// Its message generator may have finished, so reset that too.
+				transaction.reset();
+				this.queue.add(transaction.clone());
+
 				this._controller?.setStatus(ControllerStatus.Ready);
 				this._recoveryPhase = ControllerRecoveryPhase.None;
 			}).catch(() => {
-				// Soft-reset failed. Reject the original transaction and try restarting the driver.
-				fail(
-					"Recovering unresponsive controller failed. Restarting the driver...",
-				);
+				// Soft-reset failed. Reject the transaction
+				this.rejectTransaction(transaction, error);
+
+				// and reopen the serial port
+				return recoverByReopeningSerialport();
 			});
 
 			return true;
 		} else {
-			// We already attempted to recover from an unresponsive controller.
-			// Ending up here means the soft-reset also failed and the driver is about to be destroyed
+			// Not sure what to do here
 			return false;
 		}
 	}
@@ -3661,20 +3689,20 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
 					"warn",
 				);
 
-				// Re-queue the transaction.
-				// Its message generator may have finished, so reset that too.
-				transaction.reset();
-				this.queue.add(transaction.clone());
-
 				// Execute the soft-reset asynchronously
 				void this.softReset().then(() => {
-					// The controller responded. It is no longer unresponsive
-					this._controller?.setStatus(ControllerStatus.Ready);
+					// The controller responded. It is no longer unresponsive.
 
+					// Re-queue the transaction, so it can get handled next.
+					// Its message generator may have finished, so reset that too.
+					transaction.reset();
+					this.queue.add(transaction.clone());
+
+					this._controller?.setStatus(ControllerStatus.Ready);
 					this._recoveryPhase =
 						ControllerRecoveryPhase.CallbackTimeoutAfterReset;
 				}).catch(() => {
-					// Soft-reset failed. Just reject the original transaction.
+					// Soft-reset failed. Just reject the transaction
 					this.rejectTransaction(transaction, error);
 
 					this.driverLog.print(

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

@@ -55,10 +55,20 @@ integrationTest(
 );
 
 integrationTest(
-	"When the controller is still unresponsive after soft reset, destroy the driver",
+	"When the controller is still unresponsive after soft reset, re-open the serial port",
 	{
 		// debug: true,
 
+		additionalDriverOptions: {
+			testingHooks: {
+				skipNodeInterview: true,
+			},
+			attempts: {
+				// Spend less time waiting
+				controller: 1,
+			},
+		},
+
 		async customSetup(driver, mockController, mockNode) {
 			const doNotRespond: MockControllerBehavior = {
 				onHostMessage(host, controller, msg) {
@@ -74,8 +84,13 @@ integrationTest(
 			shouldRespond = false;
 			mockController.autoAckHostMessages = false;
 
-			const errorSpy = Sinon.spy();
-			driver.on("error", errorSpy);
+			const serialPortCloseSpy = Sinon.stub().callsFake(() => {
+				shouldRespond = true;
+				mockController.autoAckHostMessages = true;
+			});
+			mockController.serial.on("close", serialPortCloseSpy);
+
+			await wait(1000);
 
 			await assertZWaveError(
 				t,
@@ -90,11 +105,24 @@ integrationTest(
 				},
 			);
 
-			// The driver should have been destroyed
+			// The serial port should have been closed and reopened
 			await wait(100);
-			assertZWaveError(t, errorSpy.getCall(0).args[0], {
-				errorCode: ZWaveErrorCodes.Driver_Failed,
-			});
+			t.true(serialPortCloseSpy.called);
+
+			// FIXME: When closing the serial port, we lose the connection between the mock port instance and the controller
+			// Fix it at some point, then enable the below test.
+
+			// await wait(1000);
+
+			// // Sending a command should work again, assuming the controller is responsive again
+			// await t.notThrowsAsync(() =>
+			// 	driver.sendMessage<GetControllerIdResponse>(
+			// 		new GetControllerIdRequest(driver),
+			// 		{ supportCheck: false },
+			// 	)
+			// );
+
+			// driver.driverLog.print("TEST PASSED");
 		},
 	},
 );