fix: don't ping dead nodes before attempting communication (#6944)

zwave-js · Jun 17, 2024 · 85fda36 · 85fda36
1 parent 159075e
commit 85fda36
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 17 deletions.
diff --git a/packages/zwave-js/src/lib/driver/Driver.ts b/packages/zwave-js/src/lib/driver/Driver.ts
@@ -5382,20 +5382,6 @@ ${handlers.length} left`,
 
 		let node: ZWaveNode | undefined;
 
-		// Don't send messages to dead nodes
-		if (isNodeQuery(msg) || isCommandClassContainer(msg)) {
-			node = this.getNodeUnsafe(msg);
-			if (!messageIsPing(msg) && node?.status === NodeStatus.Dead) {
-				// Instead of throwing immediately, try to ping the node first - if it responds, continue
-				if (!(await node.ping())) {
-					throw new ZWaveError(
-						`The message cannot be sent because node ${node.id} is dead`,
-						ZWaveErrorCodes.Controller_MessageDropped,
-					);
-				}
-			}
-		}
-
 		if (options.priority == undefined) {
 			options.priority = getDefaultPriority(msg);
 		}

diff --git a/packages/zwave-js/src/lib/driver/Transaction.ts b/packages/zwave-js/src/lib/driver/Transaction.ts
@@ -80,6 +80,8 @@ export class Transaction implements Comparable<Transaction> {
 				"creationTimestamp",
 				"changeNodeStatusOnTimeout",
 				"pauseSendThread",
+				"priority",
+				"tag",
 				"requestWakeUpOnDemand",
 			] as const
 		) {

diff --git a/packages/zwave-js/src/lib/test/driver/nodeAsleepNoReject.test.ts b/packages/zwave-js/src/lib/test/driver/nodeAsleepNoReject.test.ts
@@ -63,6 +63,7 @@ integrationTest(
 			const sendQueue = driver["queue"];
 			driver.driverLog.sendQueue(sendQueue);
 			t.is(sendQueue.length, 2);
+			// with priority WakeUp
 			t.is(
 				sendQueue.transactions.get(0)?.priority,
 				MessagePriority.WakeUp,

diff --git a/packages/zwave-js/src/lib/test/driver/nodeDeadReject.test.ts b/packages/zwave-js/src/lib/test/driver/nodeDeadReject.test.ts
@@ -6,7 +6,7 @@ import path from "node:path";
 import { integrationTest } from "../integrationTestSuite";
 
 integrationTest(
-	"When a node does not respond because it is dead, the sendCommand() Promise gets rejected (maxSendAttempts: 1)",
+	"When a node does not respond because it is dead, the sendCommand() Promise and all pending commands get rejected (maxSendAttempts: 1)",
 	{
 		// debug: true,
 		provisioningDirectory: path.join(
@@ -74,7 +74,7 @@ integrationTest(
 );
 
 integrationTest(
-	"When a node does not respond because it is dead, the sendCommand() Promise gets rejected (maxSendAttempts: 2)",
+	"When a node does not respond because it is dead, the sendCommand() Promise and all pending commands get rejected (maxSendAttempts: 2)",
 	{
 		// debug: true,
 		provisioningDirectory: path.join(
@@ -142,7 +142,7 @@ integrationTest(
 );
 
 integrationTest(
-	"When a node does not respond because it is dead, commands sent via the commandClasses API get rejected",
+	"When a node does not respond because it is dead, commands sent via the commandClasses API beforehand get rejected",
 	{
 		// debug: true,
 		provisioningDirectory: path.join(
@@ -194,3 +194,71 @@ integrationTest(
 		},
 	},
 );
+
+integrationTest(
+	"When a node does not respond because it is dead, commands sent via the commandClasses API afterwards are still attempted",
+	{
+		// debug: true,
+		provisioningDirectory: path.join(
+			__dirname,
+			"fixtures/nodeDeadReject",
+		),
+
+		testBody: async (t, driver, node2, mockController, mockNode) => {
+			node2.markAsAlive();
+			mockNode.autoAckControllerFrames = false;
+
+			t.is(node2.status, NodeStatus.Alive);
+
+			const basicSetPromise = node2.commandClasses.Basic.set(99);
+			basicSetPromise.then(() => {
+				driver.driverLog.print("basicSetPromise resolved");
+			}).catch(() => {
+				driver.driverLog.print("basicSetPromise rejected");
+			});
+
+			// The node should have received the first command
+			await wait(50);
+			mockNode.assertReceivedControllerFrame(
+				(frame) =>
+					frame.type === MockZWaveFrameType.Request
+					&& frame.payload instanceof BasicCCSet
+					&& frame.payload.targetValue === 99,
+				{
+					errorMessage: "The first command was not received",
+				},
+			);
+
+			// The command should be rejected
+			await assertZWaveError(t, () => basicSetPromise, {
+				errorCode: ZWaveErrorCodes.Controller_CallbackNOK,
+			});
+			t.is(node2.status, NodeStatus.Dead);
+
+			const basicGetPromise = node2.commandClasses.Basic.get();
+			basicGetPromise.then(() => {
+				driver.driverLog.print("basicGetPromise resolved");
+			}).catch(() => {
+				driver.driverLog.print("basicGetPromise rejected");
+			});
+
+			// The node should have received the second command
+			await wait(50);
+			mockNode.assertReceivedControllerFrame(
+				(frame) =>
+					frame.type === MockZWaveFrameType.Request
+					&& frame.payload instanceof BasicCCGet,
+				{
+					errorMessage: "The second command was not received",
+				},
+			);
+
+			// The second command should be rejected separately
+			await assertZWaveError(t, () => basicGetPromise, {
+				errorCode: ZWaveErrorCodes.Controller_CallbackNOK,
+			});
+			// The node is still dead
+			t.is(node2.status, NodeStatus.Dead);
+		},
+	},
+);