Merge pull request #404 from AlCalzone/todo-killer

Address and fix a bunch of TODOs

src/lib/commandclass/AssociationCC.ts

@@ -42,7 +42,6 @@ export function getNodeIdsValueId(groupId: number): ValueID {
 export function getGroupCountValueId(): ValueID {
 	return {
 		commandClass: CommandClasses.Association,
-		// TODO: endpoint?
 		propertyName: "groupCount",
 	};
 }

src/lib/commandclass/CRC16.ts

@@ -10,6 +10,8 @@ import {
 	CommandClass,
 	commandClass,
 	CommandClassDeserializationOptions,
+	DynamicCCResponse,
+	expectedCCResponse,
 	gotDeserializationOptions,
 	implementedVersion,
 } from "./CommandClass";
@@ -76,8 +78,12 @@ interface CRC16CCCommandEncapsulationOptions extends CCCommandOptions {
 	encapsulatedCC: CommandClass;
 }
 
+// This indirection is necessary to expect the same message in response
+const getResponseForCommandEncapsulation: DynamicCCResponse = () =>
+	CRC16CCCommandEncapsulation;
+
 @CCCommand(CRC16Command.CommandEncapsulation)
-// TODO: Infer the expected response from the encapsulated CC
+@expectedCCResponse(getResponseForCommandEncapsulation)
 export class CRC16CCCommandEncapsulation extends CRC16CC {
 	public constructor(
 		driver: IDriver,

src/lib/commandclass/MultiChannelAssociationCC.ts

@@ -51,7 +51,6 @@ export function getEndpointsValueId(groupId: number): ValueID {
 export function getGroupCountValueId(): ValueID {
 	return {
 		commandClass: CommandClasses["Multi Channel Association"],
-		// TODO: endpoint?
 		propertyName: "groupCount",
 	};
 }

src/lib/commandclass/NotificationCC.ts

@@ -224,8 +224,6 @@ export class NotificationCC extends CommandClass {
 			direction: "none",
 		});
 
-		// TODO: Require the association and AGI interview to be done first (GH#198)
-
 		if (this.version >= 2) {
 			let supportedNotificationTypes: readonly NotificationType[];
 			let supportedNotificationNames: string[];

src/lib/commandclass/TimeCC.ts

@@ -160,10 +160,12 @@ export class TimeCCTimeReport extends TimeCC {
 		super(driver, options);
 		if (gotDeserializationOptions(options)) {
 			validatePayload(this.payload.length >= 3);
-			// TODO: Verify ranges
-			this.hour = this.payload[0] & 0b11111; // Range 0..23
-			this.minute = this.payload[1]; // Range 0..59
-			this.second = this.payload[2]; // Range 0..59
+			this.hour = this.payload[0] & 0b11111;
+			validatePayload(this.hour >= 0, this.hour <= 23);
+			this.minute = this.payload[1];
+			validatePayload(this.minute >= 0, this.minute <= 59);
+			this.second = this.payload[2];
+			validatePayload(this.second >= 0, this.second <= 59);
 		} else {
 			this.hour = options.hour;
 			this.minute = options.minute;

src/lib/commandclass/TimeParametersCC.ts

@@ -236,6 +236,13 @@ export class TimeParametersCCSet extends TimeParametersCC {
 				minute: this.payload[5],
 				second: this.payload[6],
 			};
+			validatePayload(
+				dateSegments.month >= 1 && dateSegments.month <= 12,
+				dateSegments.day >= 1 && dateSegments.day <= 31,
+				dateSegments.hour >= 0 && dateSegments.hour <= 23,
+				dateSegments.minute >= 0 && dateSegments.minute <= 59,
+				dateSegments.second >= 0 && dateSegments.second <= 59,
+			);
 			this.dateAndTime = segmentsToDate(
 				dateSegments,
 				shouldUseLocalTime(

src/lib/commandclass/WakeUpCC.ts

@@ -68,8 +68,7 @@ export class WakeUpCCAPI extends CCAPI {
 				typeof value,
 			);
 		}
-		// TODO: Use optional chaining when possible
-		await this.setInterval(value, this.driver.controller.ownNodeId || 1);
+		await this.setInterval(value, this.driver.controller.ownNodeId ?? 1);
 		// Refresh the current value
 		await this.getInterval();
 	};

src/lib/controller/Controller.ts

@@ -9,17 +9,8 @@ import { CommandClasses } from "../commandclass/CommandClasses";
 import { Driver, RequestHandler } from "../driver/Driver";
 import { ZWaveError, ZWaveErrorCodes } from "../error/ZWaveError";
 import log from "../log";
-import {
-	FunctionType,
-	MessagePriority,
-	MessageType,
-} from "../message/Constants";
-import { Message } from "../message/Message";
-import {
-	BasicDeviceClasses,
-	DeviceClass,
-	GenericDeviceClasses,
-} from "../node/DeviceClass";
+import { FunctionType } from "../message/Constants";
+import { BasicDeviceClasses, DeviceClass } from "../node/DeviceClass";
 import { ZWaveNode } from "../node/Node";
 import { JSONObject } from "../util/misc";
 import { num2hex } from "../util/strings";
@@ -373,54 +364,54 @@ export class ZWaveController extends EventEmitter {
 			);
 		}
 
-		// Tell the Z-Wave stick what kind of application this is
+		// TODO: Tell the Z-Wave stick what kind of application this is
 		//   The Z-Wave Application Layer MUST use the \ref ApplicationNodeInformation
 		//   function to generate the Node Information frame and to save information about
 		//   node capabilities. All Z Wave application related fields of the Node Information
 		//   structure MUST be initialized by this function.
 
-		// TODO: Afterwards, a hard reset is required, so we need to move this into another method
-		if (
-			this.isFunctionSupported(
-				FunctionType.FUNC_ID_SERIAL_API_APPL_NODE_INFORMATION,
-			)
-		) {
-			log.controller.print(`sending application info...`);
-
-			// TODO: Generate this list dynamically
-			// A list of all CCs the controller will respond to
-			const supportedCCs = [CommandClasses.Time];
-			// Turn the CCs into buffers and concat them
-			const supportedCCBuffer = Buffer.concat(
-				supportedCCs.map(cc =>
-					cc >= 0xf1
-						? // extended CC
-						  Buffer.from([cc >>> 8, cc & 0xff])
-						: // normal CC
-						  Buffer.from([cc]),
-				),
-			);
-
-			const appInfoMsg = new Message(this.driver, {
-				type: MessageType.Request,
-				functionType:
-					FunctionType.FUNC_ID_SERIAL_API_APPL_NODE_INFORMATION,
-				payload: Buffer.concat([
-					Buffer.from([
-						0x01, // APPLICATION_NODEINFO_LISTENING
-						GenericDeviceClasses["Static Controller"],
-						0x01, // specific static PC controller
-						supportedCCBuffer.length, // length of supported CC list
-					]),
-					// List of supported CCs
-					supportedCCBuffer,
-				]),
-			});
-			await this.driver.sendMessage(appInfoMsg, {
-				priority: MessagePriority.Controller,
-				supportCheck: false,
-			});
-		}
+		// Afterwards, a hard reset is required, so we need to move this into another method
+		// if (
+		// 	this.isFunctionSupported(
+		// 		FunctionType.FUNC_ID_SERIAL_API_APPL_NODE_INFORMATION,
+		// 	)
+		// ) {
+		// 	log.controller.print(`sending application info...`);
+
+		// 	// TODO: Generate this list dynamically
+		// 	// A list of all CCs the controller will respond to
+		// 	const supportedCCs = [CommandClasses.Time];
+		// 	// Turn the CCs into buffers and concat them
+		// 	const supportedCCBuffer = Buffer.concat(
+		// 		supportedCCs.map(cc =>
+		// 			cc >= 0xf1
+		// 				? // extended CC
+		// 				  Buffer.from([cc >>> 8, cc & 0xff])
+		// 				: // normal CC
+		// 				  Buffer.from([cc]),
+		// 		),
+		// 	);
+
+		// 	const appInfoMsg = new Message(this.driver, {
+		// 		type: MessageType.Request,
+		// 		functionType:
+		// 			FunctionType.FUNC_ID_SERIAL_API_APPL_NODE_INFORMATION,
+		// 		payload: Buffer.concat([
+		// 			Buffer.from([
+		// 				0x01, // APPLICATION_NODEINFO_LISTENING
+		// 				GenericDeviceClasses["Static Controller"],
+		// 				0x01, // specific static PC controller
+		// 				supportedCCBuffer.length, // length of supported CC list
+		// 			]),
+		// 			// List of supported CCs
+		// 			supportedCCBuffer,
+		// 		]),
+		// 	});
+		// 	await this.driver.sendMessage(appInfoMsg, {
+		// 		priority: MessagePriority.Controller,
+		// 		supportCheck: false,
+		// 	});
+		// }
 
 		log.controller.print("Interview completed");
 	}

src/lib/driver/Driver.ts

@@ -68,7 +68,7 @@ const defaultOptions: ZWaveOptions = {
 	timeouts: {
 		ack: 1000,
 		byte: 150,
-		report: 1000, // TODO: SDS11846 - ReportTime timeout SHOULD be set to CommandTime + 1 second
+		report: 1000,
 	},
 	skipInterview: false,
 };
@@ -175,11 +175,6 @@ export class Driver extends EventEmitter implements IDriver {
 	private sendQueue = new SortedList<Transaction>();
 	/** A map of handlers for all sorts of requests */
 	private requestHandlers = new Map<FunctionType, RequestHandlerEntry[]>();
-	// /** A map of handlers specifically for send data requests */
-	// private sendDataRequestHandlers = new Map<
-	// 	CommandClasses,
-	// 	RequestHandlerEntry<SendDataRequest>[]
-	// >();
 
 	public readonly cacheDir = path.resolve(__dirname, "../../..", "cache");
 
@@ -802,7 +797,6 @@ export class Driver extends EventEmitter implements IDriver {
 						this.currentTransaction.timeoutInstance = setTimeout(
 							() => {
 								if (!this.currentTransaction) return;
-								// TODO: Do we need more information here?
 								log.driver.print(
 									"The transaction timed out",
 									"warn",
@@ -950,12 +944,6 @@ export class Driver extends EventEmitter implements IDriver {
 		handler: RequestHandler,
 		oneTime: boolean = false,
 	): void {
-		// if (fnType === FunctionType.SendData) {
-		// 	throw new Error(
-		// 		"Cannot register a generic request handler for SendData requests. " +
-		// 			"Use `registerSendDataRequestHandler()` instead!",
-		// 	);
-		// }
 		const handlers = this.requestHandlers.has(fnType)
 			? this.requestHandlers.get(fnType)!
 			: [];
@@ -979,12 +967,6 @@ ${handlers.length} registered`,
 		fnType: FunctionType,
 		handler: RequestHandler,
 	): void {
-		// if (fnType === FunctionType.SendData) {
-		// 	throw new Error(
-		// 		"Cannot unregister a generic request handler for SendData requests. " +
-		// 			"Use `unregisterSendDataRequestHandler()` instead!",
-		// 	);
-		// }
 		const handlers = this.requestHandlers.has(fnType)
 			? this.requestHandlers.get(fnType)!
 			: [];
@@ -1002,60 +984,6 @@ ${handlers.length} left`,
 		this.requestHandlers.set(fnType, handlers);
 	}
 
-	// /**
-	//  * Registers a handler for SendData request messages
-	//  * @param cc The command class to register the handler for
-	//  * @param handler The request handler callback
-	//  */
-	// public registerSendDataRequestHandler(
-	// 	cc: CommandClasses,
-	// 	handler: RequestHandler<SendDataRequest>,
-	// 	oneTime: boolean = false,
-	// ): void {
-	// 	const handlers = this.sendDataRequestHandlers.has(cc)
-	// 		? this.sendDataRequestHandlers.get(cc)!
-	// 		: [];
-	// 	const entry: RequestHandlerEntry = { invoke: handler, oneTime };
-	// 	handlers.push(entry);
-	// 	log(
-	// 		"driver",
-	// 		`added${oneTime ? " one-time" : ""} send data request handler for ${
-	// 			CommandClasses[cc]
-	// 		} (${cc})... ${handlers.length} registered`,
-	// 		"debug",
-	// 	);
-	// 	this.sendDataRequestHandlers.set(cc, handlers);
-	// }
-
-	// /**
-	//  * Unregisters a handler for SendData request messages
-	//  * @param cc The command class to unregister the handler for
-	//  * @param handler The previously registered request handler callback
-	//  */
-	// public unregisterSendDataRequestHandler(
-	// 	cc: CommandClasses,
-	// 	handler: RequestHandler<SendDataRequest>,
-	// ): void {
-	// 	const handlers = this.sendDataRequestHandlers.has(cc)
-	// 		? this.sendDataRequestHandlers.get(cc)!
-	// 		: [];
-	// 	for (let i = 0, entry = handlers[i]; i < handlers.length; i++) {
-	// 		// remove the handler if it was found
-	// 		if (entry.invoke === handler) {
-	// 			handlers.splice(i, 1);
-	// 			break;
-	// 		}
-	// 	}
-	// 	log(
-	// 		"driver",
-	// 		`removed send data request handler for ${
-	// 			CommandClasses[cc]
-	// 		} (${cc})... ${handlers.length} left`,
-	// 		"debug",
-	// 	);
-	// 	this.sendDataRequestHandlers.set(cc, handlers);
-	// }
-
 	/**
 	 * Is called when a Request-type message was received
 	 */
@@ -1119,18 +1047,6 @@ ${handlers.length} left`,
 					return;
 				}
 			}
-			// } else if (msg instanceof SendDataRequest && msg.command.ccId) {
-			// 	// TODO: Find out if this actually happens
-			// 	// we handle SendDataRequests differently because their handlers are organized by the command class
-			// 	const ccId = msg.command.ccId;
-			// 	log(
-			// 		"driver",
-			// 		`handling send data request ${CommandClasses[ccId]} (${num2hex(
-			// 			ccId,
-			// 		)}) for node ${msg.command.nodeId}`,
-			// 		"debug",
-			// 	);
-			// 	handlers = this.sendDataRequestHandlers.get(ccId);
 		} else {
 			// TODO: This deserves a nicer formatting
 			log.driver.print(

src/lib/node/Node.ts

@@ -870,7 +870,6 @@ version:               ${this.version}`;
 			for (const cc of nodeInfo.controlledCCs)
 				this.addCC(cc, { isControlled: true });
 			this.nodeInfoReceived = true;
-			// TODO: Trigger a cache save
 		}
 
 		// As the NIF is sent on wakeup, treat this as a sign that the node is awake
@@ -1242,31 +1241,6 @@ version:               ${this.version}`;
 		}
 	}
 
-	// /**
-	//  * Requests the state for the CCs of this node
-	//  * @param kind The kind of state to be requested
-	//  * @param commandClasses The command classes to request the state for. Defaults to all
-	//  */
-	// public async requestState(
-	// 	kind: StateKind,
-	// 	commandClasses: CommandClasses[] = [
-	// 		...this.implementedCommandClasses.keys(),
-	// 	],
-	// ): Promise<void> {
-	// 	// TODO: Support multiple endpoints
-	// 	const factories = commandClasses
-	// 		// This assertion is not nice, but I see no better way
-	// 		.map(
-	// 			cc =>
-	// 				(getCCConstructor(cc) as unknown) as
-	// 					| (typeof CommandClass)
-	// 					| undefined,
-	// 		)
-	// 		.filter(cc => !!cc)
-	// 		.map(cc => () => cc!.requestState(this.driver, this, kind));
-	// 	await promiseSequence(factories);
-	// }
-
 	/**
 	 * @internal
 	 * Serializes this node in order to store static data in a cache

test/run.ts

@@ -4,7 +4,6 @@ import { Driver } from "../";
 // @ts-ignore
 const driver = new Driver("COM5").once("driver ready", async () => {
 	// console.log(`sending application info...`);
-	// // TODO: Generate this list dynamically
 	// // A list of all CCs the controller will respond to
 	// const supportedCCs = [CommandClasses.Time];
 	// // Turn the CCs into buffers and concat them