refactor: cleanup, prevent managing routes for LR, highlight some FIXMEs

packages/core/src/capabilities/NodeInfo.ts

@@ -212,6 +212,7 @@ export type NodeInformationFrame =
 	& NodeProtocolInfoAndDeviceClass
 	& ApplicationNodeInformation;
 
+// FIXME: Split these methods into two, one each for long range and one each for classic Z-Wave
 export function parseNodeProtocolInfo(
 	buffer: Buffer,
 	offset: number,

packages/core/src/capabilities/Protocols.ts

@@ -106,3 +106,7 @@ export function longRangeChannelToString(channel: LongRangeChannel): string {
 	}
 	return `Unknown (${num2hex(channel)})`;
 }
+
+export function isLongRangeNodeId(nodeId: number): boolean {
+	return nodeId > 255;
+}

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

@@ -44,6 +44,7 @@ import {
 	NodeType,
 	type ProtocolDataRate,
 	ProtocolType,
+	Protocols,
 	RFRegion,
 	type RSSI,
 	type Route,
@@ -66,6 +67,7 @@ import {
 	encodeX25519KeyDERSPKI,
 	indexDBsByNode,
 	isEmptyRoute,
+	isLongRangeNodeId,
 	isValidDSK,
 	isZWaveError,
 	nwiHomeIdFromDSK,
@@ -1368,10 +1370,8 @@ export class ZWaveController
 			);
 			nodeIds.unshift(this._ownNodeId!);
 		}
-
-		// BUGBUG: do nodes need to implicitly know that they were a long range node? Or are long range nodes determined 100% by their nodeID values being >= 256?
-		// The controller is the odd-man out, as it's both. Let's assume that apart from explicit exclusion we don't need to know for now.
 		nodeIds.push(...lrNodeIds);
+
 		for (const nodeId of nodeIds) {
 			this._nodes.set(
 				nodeId,
@@ -1470,6 +1470,7 @@ export class ZWaveController
 	}
 
 	private isLongRange(): boolean {
+		// FIXME: Rely on the SerialAPIStarted command, make sure the controller is soft-reset before we need to know this
 		return !!this._supportsLongRange;
 	}
 
@@ -2341,9 +2342,11 @@ supported CCs: ${
 						"no initiate command received, bootstrapping node...",
 					);
 
-					// Assign SUC return route to make sure the node knows where to get its routes from
-					newNode.hasSUCReturnRoute = await this
-						.assignSUCReturnRoutes(newNode.id);
+					if (newNode.protocol == Protocols.ZWave) {
+						// Assign SUC return route to make sure the node knows where to get its routes from
+						newNode.hasSUCReturnRoute = await this
+							.assignSUCReturnRoutes(newNode.id);
+					}
 
 					// Include using the default inclusion strategy:
 					// * Use S2 if possible,
@@ -3496,10 +3499,13 @@ supported CCs: ${
 				// If it is actually a sleeping device, it will be marked as such later
 				newNode.markAsAlive();
 
-				// Assign SUC return route to make sure the node knows where to get its routes from
-				newNode.hasSUCReturnRoute = await this.assignSUCReturnRoutes(
-					newNode.id,
-				);
+				if (newNode.protocol == Protocols.ZWave) {
+					// Assign SUC return route to make sure the node knows where to get its routes from
+					newNode.hasSUCReturnRoute = await this
+						.assignSUCReturnRoutes(
+							newNode.id,
+						);
+				}
 
 				const opts = this._inclusionOptions;
 
@@ -3767,9 +3773,11 @@ supported CCs: ${
 					// If it is actually a sleeping device, it will be marked as such later
 					newNode.markAsAlive();
 
-					// Assign SUC return route to make sure the node knows where to get its routes from
-					newNode.hasSUCReturnRoute = await this
-						.assignSUCReturnRoutes(newNode.id);
+					if (newNode.protocol == Protocols.ZWave) {
+						// Assign SUC return route to make sure the node knows where to get its routes from
+						newNode.hasSUCReturnRoute = await this
+							.assignSUCReturnRoutes(newNode.id);
+					}
 
 					// Try perform the security bootstrap process. When replacing a node, we don't know any supported CCs
 					// yet, so we need to trust the chosen inclusion strategy.
@@ -4046,6 +4054,8 @@ supported CCs: ${
 		const todoSleeping: number[] = [];
 
 		const addTodo = (nodeId: number) => {
+			if (isLongRangeNodeId(nodeId)) return;
+
 			if (pendingNodes.has(nodeId)) {
 				pendingNodes.delete(nodeId);
 				const node = this.nodes.getOrThrow(nodeId);
@@ -4177,6 +4187,13 @@ supported CCs: ${
 		}
 
 		const node = this.nodes.getOrThrow(nodeId);
+		// Z-Wave Long Range does not route
+		if (node.protocol == Protocols.ZWaveLongRange) {
+			throw new ZWaveError(
+				`Cannot rebuild routes for nodes using Z-Wave Long Range!`,
+				ZWaveErrorCodes.Argument_Invalid,
+			);
+		}
 
 		// Don't start the process twice
 		if (this._isRebuildingRoutes) {
@@ -4413,12 +4430,13 @@ ${associatedNodes.join(", ")}`,
 	 * This will assign up to 4 routes, depending on the network topology (that the controller knows about).
 	 */
 	public async assignSUCReturnRoutes(nodeId: number): Promise<boolean> {
-		if (nodeId >= 0x100) {
-			this.driver.controllerLog.logNode(nodeId, {
-				message: `Skipping SUC return route because isLR...`,
-				direction: "outbound",
-			});
-			return true;
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
 		}
 
 		this.driver.controllerLog.logNode(nodeId, {
@@ -4508,6 +4526,15 @@ ${associatedNodes.join(", ")}`,
 		routes: Route[],
 		priorityRoute?: Route,
 	): Promise<boolean> {
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		}
+
 		this.driver.controllerLog.logNode(nodeId, {
 			message: `Assigning custom SUC return routes...`,
 			direction: "outbound",
@@ -4607,6 +4634,15 @@ ${associatedNodes.join(", ")}`,
 	 * This will assign up to 4 routes, depending on the network topology (that the controller knows about).
 	 */
 	public async deleteSUCReturnRoutes(nodeId: number): Promise<boolean> {
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		}
+
 		this.driver.controllerLog.logNode(nodeId, {
 			message: `Deleting SUC return route...`,
 			direction: "outbound",
@@ -4693,6 +4729,22 @@ ${associatedNodes.join(", ")}`,
 		nodeId: number,
 		destinationNodeId: number,
 	): Promise<boolean> {
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		} else if (isLongRangeNodeId(destinationNodeId)) {
+			this.driver.controllerLog.logNode(
+				destinationNodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		}
+
 		// Make sure this is not misused by passing the controller's node ID
 		if (destinationNodeId === this.ownNodeId) {
 			throw new ZWaveError(
@@ -4765,6 +4817,22 @@ ${associatedNodes.join(", ")}`,
 		routes: Route[],
 		priorityRoute?: Route,
 	): Promise<boolean> {
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		} else if (isLongRangeNodeId(destinationNodeId)) {
+			this.driver.controllerLog.logNode(
+				destinationNodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		}
+
 		// Make sure this is not misused by passing the controller's node ID
 		if (destinationNodeId === this.ownNodeId) {
 			throw new ZWaveError(
@@ -4889,6 +4957,15 @@ ${associatedNodes.join(", ")}`,
 	 * other end nodes, including the priority return routes.
 	 */
 	public async deleteReturnRoutes(nodeId: number): Promise<boolean> {
+		if (isLongRangeNodeId(nodeId)) {
+			this.driver.controllerLog.logNode(
+				nodeId,
+				`Cannot manage routes for nodes using Z-Wave Long Range!`,
+				"error",
+			);
+			return false;
+		}
+
 		this.driver.controllerLog.logNode(nodeId, {
 			message: `Deleting all return routes...`,
 			direction: "outbound",

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

@@ -137,11 +137,6 @@ export type InclusionOptions =
 		 * This is not recommended due to the overhead caused by S0.
 		 */
 		forceSecurity?: boolean;
-
-		/**
-		 * Force long range. If not provided, will default to long range iff the controller supports it, and not otherwise.
-		 */
-		isLongRange?: boolean;
 	}
 	| {
 		strategy: InclusionStrategy.Security_S2;
@@ -168,11 +163,6 @@ export type InclusionOptions =
 		strategy:
 			| InclusionStrategy.Insecure
 			| InclusionStrategy.Security_S0;
-
-		/**
-		 * Force long range. If not provided, will default to long range iff the controller supports it, and not otherwise.
-		 */
-		isLongRange?: boolean;
 	};
 
 /**
@@ -236,6 +226,11 @@ export interface PlannedProvisioningEntry {
 
 	/** Which protocol to use for inclusion. Default: Z-Wave Classic */
 	protocol?: Protocols;
+	/**
+	 * The protocols that are **supported** by the device.
+	 * When this is not set, applications should default to Z-Wave classic.
+	 */
+	supportedProtocols?: readonly Protocols[];
 
 	/** The security classes that have been **granted** by the user */
 	securityClasses: SecurityClass[];

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

@@ -1,5 +1,6 @@
 import {
 	type MaybeNotKnown,
+	Protocols,
 	SecurityClass,
 	ZWaveError,
 	ZWaveErrorCodes,
@@ -61,6 +62,25 @@ export function assertProvisioningEntry(
 			}
 		}
 	}
+
+	if (
+		arg.protocol != undefined
+		&& (typeof arg.protocol !== "number" || !(arg.protocol in Protocols))
+	) {
+		throw fail("protocol is not a valid");
+	}
+
+	if (arg.supportedProtocols != undefined) {
+		if (!isArray(arg.supportedProtocols)) {
+			throw fail("supportedProtocols must be an array");
+		} else if (
+			!arg.supportedProtocols.every(
+				(p: any) => typeof p === "number" && p in Protocols,
+			)
+		) {
+			throw fail("supportedProtocols contains invalid entries");
+		}
+	}
 }
 
 /** Checks if the SDK version is greater than the given one */

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

@@ -1428,6 +1428,8 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
 			// No need to initialize databases if skipInterview is true, because it is only used in some
 			// Driver unit tests that don't need access to them
 
+			// FIXME: Setting the node ID type, opening the cache and querying the controller ID should be done AFTER soft-resetting
+
 			// Identify the controller and determine if it supports soft reset
 			await this.controller.identify();
 			await this.initNetworkCache(this.controller.homeId!);
@@ -1445,6 +1447,11 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
 				await this.softResetInternal(false);
 			}
 
+			// FIXME: We should now know if the controller supports ZWLR or not
+			// Also, set the node ID type to 16-bit here only if ZWLR is supported.
+
+			// FIXME: This block is unnecessary when setting the node ID type explicitly
+
 			// There are situations where a controller claims it has the ID 0,
 			// which isn't valid. In this case try again after having soft-reset the stick
 			// TODO: Check if this is still necessary now that we support 16-bit node IDs

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

@@ -178,6 +178,15 @@ function tryParseProvisioningList(
 					&& entry.requestedSecurityClasses.every((s) =>
 						isSerializedSecurityClass(s)
 					)))
+			// protocol and supportedProtocols are stored as strings, not the enum values
+			&& (entry.protocol == undefined
+				|| isSerializedProtocol(entry.protocol))
+			&& (entry.supportedProtocols == undefined || (
+				isArray(entry.supportedProtocols)
+				&& entry.supportedProtocols.every((s) =>
+					isSerializedProtocol(s)
+				)
+			))
 			&& (entry.status == undefined
 				|| isSerializedProvisioningEntryStatus(entry.status))
 		) {
@@ -208,6 +217,13 @@ function tryParseProvisioningList(
 				parsed.protocol =
 					Protocols[entry.protocol as any] as any as Protocols;
 			}
+			if (entry.supportedProtocols) {
+				parsed.supportedProtocols = (
+					entry.supportedProtocols as any[]
+				)
+					.map((s) => Protocols[s] as any as Protocols)
+					.filter((s): s is Protocols => s !== undefined);
+			}
 			ret.push(parsed);
 		} else {
 			return;
@@ -270,6 +286,16 @@ function isSerializedProvisioningEntryStatus(
 	);
 }
 
+function isSerializedProtocol(
+	s: unknown,
+): s is keyof typeof Protocols {
+	return (
+		typeof s === "string"
+		&& s in Protocols
+		&& typeof Protocols[s as any] === "number"
+	);
+}
+
 function tryParseDate(value: unknown): Date | undefined {
 	// Dates are stored as timestamps
 	if (typeof value === "number") {
@@ -463,6 +489,12 @@ export function serializeNetworkCacheValue(
 						entry.protocol,
 					);
 				}
+				if (entry.supportedProtocols != undefined) {
+					serialized.supportedProtocols = entry.supportedProtocols
+						.map(
+							(p) => getEnumMemberName(Protocols, p),
+						);
+				}
 				ret.push(serialized);
 			}
 			return ret;