refactor: rework SendThreadMachine to eliminate nonce and update step…

…s and handle sequenced messages (zwave-js#3691)

.vscode/extensions.json

@@ -4,7 +4,7 @@
 		"dbaeumer.vscode-eslint",
 		"esbenp.prettier-vscode",
 		"EditorConfig.EditorConfig",
-		"eamodio.toggle-excluded-files",
+		"amodio.toggle-excluded-files",
 		"arcanis.vscode-zipfs"
 	]
 }

package.json

@@ -53,7 +53,7 @@
     "@zwave-js/config": "workspace:*",
     "@zwave-js/core": "workspace:*",
     "@zwave-js/shared": "workspace:*",
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "ansi-colors": "^4.1.1",
     "comment-json": "^4.1.1",
     "commitizen": "^4.2.4",

packages/config/package.json

@@ -42,7 +42,7 @@
   "dependencies": {
     "@zwave-js/core": "8.7.4",
     "@zwave-js/shared": "8.7.3",
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "ansi-colors": "^4.1.1",
     "fs-extra": "^10.0.0",
     "json-logic-js": "^2.0.1",

packages/core/package.json

@@ -39,7 +39,7 @@
     "@alcalzone/jsonl-db": "^2.2.0",
     "@zwave-js/shared": "8.7.3",
     "@zwave-js/winston-daily-rotate-file": "^4.5.6-0",
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "ansi-colors": "^4.1.1",
     "dayjs": "^1.10.7",
     "logform": "*",

packages/serial/package.json

@@ -39,7 +39,7 @@
     "@sentry/node": "^6.14.3",
     "@zwave-js/core": "8.7.4",
     "@zwave-js/shared": "8.7.3",
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "serialport": "^9.2.5",
     "winston": "^3.3.3"
   },

packages/shared/package.json

@@ -31,7 +31,7 @@
     "node": ">=v12.22.2"
   },
   "dependencies": {
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "fs-extra": "^10.0.0"
   },
   "scripts": {

packages/zwave-js/package.json

@@ -61,7 +61,7 @@
     "@zwave-js/core": "8.7.4",
     "@zwave-js/serial": "8.7.4",
     "@zwave-js/shared": "8.7.3",
-    "alcalzone-shared": "^4.0.0",
+    "alcalzone-shared": "^4.0.1",
     "ansi-colors": "^4.1.1",
     "axios": "^0.24.0",
     "execa": "^5.1.1",

packages/zwave-js/src/lib/commandclass/CommandClass.ts

@@ -964,20 +964,6 @@ export class CommandClass {
 		return propertyKey.toString();
 	}
 
-	/** Whether this CC needs to exchange one or more messages before it can be sent */
-	public requiresPreTransmitHandshake(): boolean {
-		return false; // By default it doesn't
-	}
-
-	/**
-	 * Perform a handshake before the actual message will be transmitted.
-	 */
-	public preTransmitHandshake(): Promise<void> {
-		return Promise.resolve();
-		// By default do nothing
-		// If handshake messages should be sent, they need the highest priority
-	}
-
 	/** Returns the number of bytes that are added to the payload by this CC */
 	protected computeEncapsulationOverhead(): number {
 		// Default is ccId (+ ccCommand):

packages/zwave-js/src/lib/commandclass/Security2CC.ts

@@ -207,37 +207,6 @@ export class Security2CCAPI extends CCAPI {
 		return true;
 	}
 
-	/**
-	 * Requests a nonce from the target node
-	 */
-	public async requestNonce(): Promise<void> {
-		this.assertSupportsCommand(Security2Command, Security2Command.NonceGet);
-
-		this.assertPhysicalEndpoint(this.endpoint);
-
-		if (!this.driver.securityManager2) {
-			throw new ZWaveError(
-				`Nonces can only be sent if secure communication is set up!`,
-				ZWaveErrorCodes.Driver_NoSecurity,
-			);
-		}
-
-		const cc = new Security2CCNonceGet(this.driver, {
-			nodeId: this.endpoint.nodeId,
-			endpoint: this.endpoint.index,
-		});
-
-		await this.driver.sendCommand(cc, {
-			...this.commandOptions,
-			priority: MessagePriority.PreTransmitHandshake,
-			// Only try getting a nonce once
-			maxSendAttempts: 1,
-			// We don't want failures causing us to treat the node as asleep or dead
-			// The "real" transaction will do that for us
-			changeNodeStatusOnMissingACK: false,
-		});
-	}
-
 	/**
 	 * Queries the securely supported commands for the current security class
 	 * @param securityClass Can be used to overwrite the security class to use. If this doesn't match the current one, new nonces will need to be exchanged.
@@ -604,18 +573,33 @@ interface Security2CCMessageEncapsulationOptions extends CCCommandOptions {
 	encapsulated: CommandClass;
 }
 
+// An S2 encapsulated command may result in a NonceReport to be sent by the node if it couldn't decrypt the message
 function getCCResponseForMessageEncapsulation(
 	sent: Security2CCMessageEncapsulation,
 ) {
 	if (sent.encapsulated?.expectsCCResponse()) {
-		return Security2CCMessageEncapsulation;
+		return [
+			Security2CCMessageEncapsulation as any,
+			Security2CCNonceReport as any,
+		];
+	}
+}
+
+function testCCResponseForMessageEncapsulation(
+	sent: Security2CCMessageEncapsulation,
+	received: Security2CCMessageEncapsulation | Security2CCNonceReport,
+) {
+	if (received instanceof Security2CCMessageEncapsulation) {
+		return "checkEncapsulated";
+	} else {
+		return received.SOS && !!received.receiverEI;
 	}
 }
 
 @CCCommand(Security2Command.MessageEncapsulation)
 @expectedCCResponse(
 	getCCResponseForMessageEncapsulation,
-	() => "checkEncapsulated",
+	testCCResponseForMessageEncapsulation,
 )
 export class Security2CCMessageEncapsulation extends Security2CC {
 	// Define the securityManager and controller.ownNodeId as existing
@@ -881,20 +865,7 @@ export class Security2CCMessageEncapsulation extends Security2CC {
 	public encapsulated?: CommandClass;
 	public extensions: Security2Extension[];
 
-	private _wasRetriedAfterDecodeFailure: boolean = false;
-	/** Indicates whether sending this command was retried after the target node failed to decode it  */
-	public get wasRetriedAfterDecodeFailure(): boolean {
-		return this._wasRetriedAfterDecodeFailure;
-	}
-
-	public prepareRetryAfterDecodeFailure(): void {
-		if (this._wasRetriedAfterDecodeFailure) {
-			throw new ZWaveError(
-				`S2 encapsulated messages may only be retried once after the target failed to decode them!`,
-				ZWaveErrorCodes.Controller_MessageDropped,
-			);
-		}
-		this._wasRetriedAfterDecodeFailure = true;
+	public unsetSequenceNumber(): void {
 		this._sequenceNumber = undefined;
 	}
 
@@ -927,23 +898,6 @@ export class Security2CCMessageEncapsulation extends Security2CC {
 		return spanExtension?.senderEI;
 	}
 
-	public requiresPreTransmitHandshake(): boolean {
-		// We need the receiver's EI to be able to send an encrypted command
-		const secMan = this.driver.securityManager2;
-		const peerNodeId = this.nodeId as number;
-		const spanState = secMan.getSPANState(peerNodeId);
-		return (
-			spanState.type === SPANState.None ||
-			spanState.type === SPANState.LocalEI
-		);
-	}
-
-	public async preTransmitHandshake(): Promise<void> {
-		// Request a nonce
-		return this.getNode()!.commandClasses["Security 2"].requestNonce();
-		// Yeah, that's it :)
-	}
-
 	public serialize(): Buffer {
 		// TODO: Support Multicast
 		const node = this.getNode()!;

packages/zwave-js/src/lib/commandclass/SecurityCC.ts

@@ -116,20 +116,11 @@ export class SecurityCCAPI extends PhysicalCCAPI {
 	}
 
 	/**
-	 * Requests a new nonce for Security CC encapsulation
+	 * Requests a new nonce for Security CC encapsulation which is not directly linked to a specific command.
 	 */
-	public async getNonce(
-		options: {
-			/** Whether the command should be sent as a standalone transaction. Default: false */
-			standalone?: boolean;
-			/** Whether the received nonce should be stored as "free". Default: false */
-			storeAsFreeNonce?: boolean;
-		} = {},
-	): Promise<Buffer | undefined> {
+	public async getNonce(): Promise<Buffer | undefined> {
 		this.assertSupportsCommand(SecurityCommand, SecurityCommand.NonceGet);
 
-		const { standalone = false, storeAsFreeNonce = false } = options;
-
 		const cc = new SecurityCCNonceGet(this.driver, {
 			nodeId: this.endpoint.nodeId,
 			endpoint: this.endpoint.index,
@@ -138,32 +129,24 @@ export class SecurityCCAPI extends PhysicalCCAPI {
 			cc,
 			{
 				...this.commandOptions,
-				// Standalone nonce requests must be handled immediately
-				priority: standalone
-					? MessagePriority.Normal
-					: MessagePriority.PreTransmitHandshake,
+				// Nonce requests must be handled immediately
+				priority: MessagePriority.Handshake,
 				// Only try getting a nonce once
 				maxSendAttempts: 1,
-				// We don't want failures causing us to treat the node as asleep or dead
-				// The "real" transaction will do that for us
-				changeNodeStatusOnMissingACK: standalone,
 			},
 		);
-
 		if (!response) return;
 
 		const nonce = response.nonce;
-		if (storeAsFreeNonce) {
-			const secMan = this.driver.securityManager!;
-			secMan.setNonce(
-				{
-					issuer: this.endpoint.nodeId,
-					nonceId: secMan.getNonceId(nonce),
-				},
-				{ nonce, receiver: this.driver.controller.ownNodeId! },
-				{ free: true },
-			);
-		}
+		const secMan = this.driver.securityManager!;
+		secMan.setNonce(
+			{
+				issuer: this.endpoint.nodeId,
+				nonceId: secMan.getNonceId(nonce),
+			},
+			{ nonce, receiver: this.driver.controller.ownNodeId! },
+			{ free: true },
+		);
 		return nonce;
 	}
 
@@ -615,52 +598,6 @@ export class SecurityCCCommandEncapsulation extends SecurityCC {
 		});
 	}
 
-	public requiresPreTransmitHandshake(): boolean {
-		// We require a new nonce if there is no free one,
-		// we don't have one yet or if the old one has expired
-		const secMan = this.driver.securityManager;
-
-		// If the nonce is already known we don't need a handshake
-		if (
-			this.nonceId != undefined &&
-			secMan.hasNonce({
-				issuer: this.nodeId,
-				nonceId: this.nonceId,
-			})
-		) {
-			return false;
-		}
-
-		// Try to get a free nonce before requesting a new one
-		const freeNonce = secMan.getFreeNonce(this.nodeId);
-		if (freeNonce) {
-			this.nonceId = secMan.getNonceId(freeNonce);
-			return false;
-		}
-
-		return true;
-	}
-
-	public async preTransmitHandshake(): Promise<void> {
-		// Request a nonce
-		const nonce = await this.getNode()!.commandClasses.Security.getNonce();
-		// TODO: Handle this more intelligent
-		if (nonce) {
-			// and store it
-			const secMan = this.driver.securityManager;
-			this.nonceId = secMan.getNonceId(nonce);
-			secMan.setNonce(
-				{
-					issuer: this.nodeId,
-					nonceId: this.nonceId,
-				},
-				{ nonce, receiver: this.driver.controller.ownNodeId },
-				// The nonce is reserved for this command
-				{ free: false },
-			);
-		}
-	}
-
 	public serialize(): Buffer {
 		function throwNoNonce(): never {
 			throw new ZWaveError(

packages/zwave-js/src/lib/controller/Controller.ts

@@ -1589,10 +1589,7 @@ export class ZWaveController extends TypedEventEmitter<ControllerEventCallbacks>
 			await api.getSecurityScheme(); // ignore the result
 
 			// Request nonce separately, so we can impose a timeout
-			await api.getNonce({
-				standalone: true,
-				storeAsFreeNonce: true,
-			});
+			await api.getNonce();
 
 			// send the network key
 			await api.setNetworkKey(this.driver.securityManager.networkKey);

packages/zwave-js/src/lib/controller/SendDataBridgeMessages.ts

@@ -30,6 +30,8 @@ import {
 	priority,
 } from "../message/Message";
 import type { SuccessIndicator } from "../message/SuccessIndicator";
+import { ApplicationCommandRequest } from "./ApplicationCommandRequest";
+import { BridgeApplicationCommandRequest } from "./BridgeApplicationCommandRequest";
 import { MAX_SEND_ATTEMPTS } from "./SendDataMessages";
 import { TransmitOptions, TransmitStatus } from "./SendDataShared";
 
@@ -163,6 +165,18 @@ export class SendDataBridgeRequest<CCType extends CommandClass = CommandClass>
 		if (this.transmitOptions & TransmitOptions.AutoRoute) return 48;
 		return 54;
 	}
+
+	public expectsNodeUpdate(): boolean {
+		return this.command.expectsCCResponse();
+	}
+
+	public isExpectedNodeUpdate(msg: Message): boolean {
+		return (
+			(msg instanceof ApplicationCommandRequest ||
+				msg instanceof BridgeApplicationCommandRequest) &&
+			this.command.isExpectedCCResponse(msg.command)
+		);
+	}
 }
 
 interface SendDataBridgeRequestTransmitReportOptions

packages/zwave-js/src/lib/controller/SendDataMessages.ts

@@ -30,6 +30,8 @@ import {
 	priority,
 } from "../message/Message";
 import type { SuccessIndicator } from "../message/SuccessIndicator";
+import { ApplicationCommandRequest } from "./ApplicationCommandRequest";
+import { BridgeApplicationCommandRequest } from "./BridgeApplicationCommandRequest";
 import { TransmitOptions, TransmitStatus } from "./SendDataShared";
 
 export const MAX_SEND_ATTEMPTS = 5;
@@ -131,6 +133,18 @@ export class SendDataRequest<CCType extends CommandClass = CommandClass>
 		if (this.transmitOptions & TransmitOptions.AutoRoute) return 48;
 		return 54;
 	}
+
+	public expectsNodeUpdate(): boolean {
+		return this.command.expectsCCResponse();
+	}
+
+	public isExpectedNodeUpdate(msg: Message): boolean {
+		return (
+			(msg instanceof ApplicationCommandRequest ||
+				msg instanceof BridgeApplicationCommandRequest) &&
+			this.command.isExpectedCCResponse(msg.command)
+		);
+	}
 }
 
 interface SendDataRequestTransmitReportOptions extends MessageBaseOptions {