fix: treat no error as successful command, update currentValue immedi…

…ately (#1522)

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

@@ -7,24 +7,26 @@ import {
 	ZWaveError,
 	ZWaveErrorCodes,
 } from "@zwave-js/core";
-import { getEnumMemberName } from "@zwave-js/shared";
+import { getEnumMemberName, ObjectKeyMap } from "@zwave-js/shared";
 import { isArray } from "alcalzone-shared/typeguards";
 import type { Driver, SendCommandOptions } from "../driver/Driver";
 import type { Endpoint } from "../node/Endpoint";
 import { VirtualEndpoint } from "../node/VirtualEndpoint";
 import { getCommandClass } from "./CommandClass";
 
+export type ValueIDProperties = Pick<ValueID, "property" | "propertyKey">;
+
 /** Used to identify the method on the CC API class that handles setting values on nodes directly */
 export const SET_VALUE: unique symbol = Symbol.for("CCAPI_SET_VALUE");
 export type SetValueImplementation = (
-	property: Pick<ValueID, "property" | "propertyKey">,
+	property: ValueIDProperties,
 	value: unknown,
 ) => Promise<void>;
 
 /** Used to identify the method on the CC API class that handles polling values from nodes */
 export const POLL_VALUE: unique symbol = Symbol.for("CCAPI_POLL_VALUE");
 export type PollValueImplementation<T extends unknown = unknown> = (
-	property: Pick<ValueID, "property" | "propertyKey">,
+	property: ValueIDProperties,
 ) => Promise<T | undefined>;
 
 // Since the setValue API is called from a point with very generic parameters,
@@ -82,6 +84,7 @@ export class CCAPI {
 		protected readonly endpoint: Endpoint | VirtualEndpoint,
 	) {
 		this.ccId = getCommandClass(this);
+		this.scheduledPolls = new ObjectKeyMap();
 	}
 
 	/**
@@ -105,7 +108,45 @@ export class CCAPI {
 	 */
 	public get pollValue(): PollValueImplementation | undefined {
 		// wotan-disable-next-line no-restricted-property-access
-		return this[POLL_VALUE];
+		const implementation = this[POLL_VALUE]?.bind(this);
+		if (!implementation) return;
+		// Polling manually should cancel scheduled polls to avoid polling too often
+		// Therefore return a wrapper which takes care of that
+		return (property) => {
+			// Cancel any scheduled polls
+			if (this.scheduledPolls.has(property)) {
+				clearTimeout(this.scheduledPolls.get(property)!);
+				this.scheduledPolls.delete(property);
+			}
+			// Call the implementation
+			return implementation(property);
+		};
+	}
+
+	protected scheduledPolls: ObjectKeyMap<ValueIDProperties, NodeJS.Timeout>;
+	/**
+	 * Schedules a value to be polled after a given time. Schedules are deduplicated on a per-property basis.
+	 * @returns `true` if the poll was scheduled, `false` otherwise
+	 */
+	protected schedulePoll(
+		property: ValueIDProperties,
+		timeoutMs: number = this.driver.options.timeouts.refreshValue,
+	): boolean {
+		if (this.scheduledPolls.has(property)) return false;
+		// wotan-disable-next-line no-restricted-property-access
+		if (!this[POLL_VALUE]) return false;
+
+		this.scheduledPolls.set(
+			property,
+			setTimeout(async () => {
+				try {
+					await this.pollValue!(property);
+				} catch {
+					/* ignore */
+				}
+			}, timeoutMs).unref(),
+		);
+		return true;
 	}
 
 	/**

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

@@ -89,8 +89,23 @@ export class BasicCCAPI extends CCAPI {
 			throwWrongValueType(this.ccId, property, "number", typeof value);
 		}
 		await this.set(value);
+
+		// If the command did not fail, assume that it succeeded and update the currentValue accordingly
+		// so UIs have immediate feedback
+		if (this.isSinglecast()) {
+			const valueDB = this.endpoint.getNodeUnsafe()?.valueDB;
+			valueDB?.setValue(
+				getCurrentValueValueId(this.endpoint.index),
+				value,
+			);
+
+			// and verify the current value after a delay
+			this.schedulePoll({ property });
+		}
 	};
 
+	private refreshTimeout: NodeJS.Timeout | undefined;
+
 	protected [POLL_VALUE]: PollValueImplementation = async ({
 		property,
 	}): Promise<unknown> => {
@@ -121,8 +136,6 @@ export class BasicCCAPI extends CCAPI {
 		}
 	}
 
-	private refreshTimeout: NodeJS.Timeout | undefined;
-
 	public async set(targetValue: number): Promise<void> {
 		this.assertSupportsCommand(BasicCommand, BasicCommand.Set);
 
@@ -131,30 +144,7 @@ export class BasicCCAPI extends CCAPI {
 			endpoint: this.endpoint.index,
 			targetValue,
 		});
-		if (this.isSinglecast()) {
-			// remember the value in case the device does not respond with a target value
-			this.endpoint
-				.getNodeUnsafe()
-				?.valueDB.setValue(
-					getTargetValueValueId(this.endpoint.index),
-					targetValue,
-					{ noEvent: true },
-				);
-		}
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value after a delay
-			if (this.refreshTimeout) clearTimeout(this.refreshTimeout);
-			setTimeout(async () => {
-				this.refreshTimeout = undefined;
-				try {
-					await this.get();
-				} catch {
-					/* ignore */
-				}
-			}, this.driver.options.timeouts.refreshValue).unref();
-		}
 	}
 }
 

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

@@ -37,11 +37,11 @@ import {
 	implementedVersion,
 } from "./CommandClass";
 
-function getTargetValueValueId(endpoint?: number): ValueID {
+function getCurrentValueValueId(endpoint?: number): ValueID {
 	return {
 		commandClass: CommandClasses["Binary Switch"],
 		endpoint,
-		property: "targetValue",
+		property: "currentValue",
 	};
 }
 
@@ -108,30 +108,7 @@ export class BinarySwitchCCAPI extends CCAPI {
 			targetValue,
 			duration,
 		});
-		if (this.isSinglecast()) {
-			// remember the value in case the device does not respond with a target value
-			this.endpoint
-				.getNodeUnsafe()
-				?.valueDB.setValue(
-					getTargetValueValueId(this.endpoint.index),
-					targetValue,
-					{ noEvent: true },
-				);
-		}
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value after a delay
-			if (this.refreshTimeout) clearTimeout(this.refreshTimeout);
-			setTimeout(async () => {
-				this.refreshTimeout = undefined;
-				try {
-					await this.get();
-				} catch {
-					/* ignore */
-				}
-			}, duration?.toMilliseconds() ?? 1000).unref();
-		}
 	}
 
 	protected [SET_VALUE]: SetValueImplementation = async (
@@ -145,6 +122,21 @@ export class BinarySwitchCCAPI extends CCAPI {
 			throwWrongValueType(this.ccId, property, "boolean", typeof value);
 		}
 		await this.set(value);
+
+		// If the command did not fail, assume that it succeeded and update the currentValue accordingly
+		// so UIs have immediate feedback
+		if (this.isSinglecast()) {
+			const valueDB = this.endpoint.getNodeUnsafe()?.valueDB;
+			valueDB?.setValue(
+				getCurrentValueValueId(this.endpoint.index),
+				value,
+			);
+
+			// Verify the current value after a delay
+			// TODO: #1321
+			const duration = undefined as Duration | undefined;
+			this.schedulePoll({ property }, duration?.toMilliseconds() ?? 1000);
+		}
 	};
 
 	protected [POLL_VALUE]: PollValueImplementation = async ({

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

@@ -146,11 +146,6 @@ export class CentralSceneCCAPI extends CCAPI {
 			slowRefresh,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			await this.getConfiguration();
-		}
 	}
 
 	protected [SET_VALUE]: SetValueImplementation = async (

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

@@ -85,11 +85,6 @@ export class ClimateControlScheduleCCAPI extends CCAPI {
 			switchPoints,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			await this.get(weekday);
-		}
 	}
 
 	public async get(
@@ -168,11 +163,6 @@ export class ClimateControlScheduleCCAPI extends CCAPI {
 			overrideState: state,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			await this.getOverride();
-		}
 	}
 }
 

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

@@ -89,11 +89,6 @@ export class ClockCCAPI extends CCAPI {
 			weekday: weekday ?? Weekday.Unknown,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			await this.get();
-		}
 	}
 }
 

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

@@ -224,7 +224,9 @@ export class ColorSwitchCCAPI extends CCAPI {
 
 		await this.driver.sendCommand(cc, this.commandOptions);
 
-		// Assume it worked and update currentColor
+		// If the command did not fail, assume that it succeeded and update the values accordingly
+		// TODO: The API methods should not modify the value DB directly, but to do so
+		// this requires a nicer way of synchronizing hexColor with the others
 		if (this.isSinglecast()) {
 			const valueDB = this.endpoint.getNodeUnsafe()?.valueDB;
 			if (valueDB) {
@@ -330,8 +332,13 @@ export class ColorSwitchCCAPI extends CCAPI {
 			await this.set({ [propertyKey]: value });
 
 			if (this.isSinglecast()) {
-				// Refresh the current value
-				await this.get(propertyKey);
+				// Verify the current value after a delay
+				// TODO: #1321
+				const duration = undefined as Duration | undefined;
+				this.schedulePoll(
+					{ property, propertyKey },
+					duration?.toMilliseconds(),
+				);
 			}
 		} else if (property === "hexColor") {
 			// No property key, this is the hex color #rrggbb

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

@@ -221,10 +221,8 @@ export class ConfigurationCCAPI extends PhysicalCCAPI {
 
 		await this.set(property, targetValue, valueSize as any);
 
-		// Refresh the current value and ignore potential timeouts
-		void this.get(property).catch(() => {
-			/* ignore */
-		});
+		// Verify the current value after a delay
+		this.schedulePoll({ property, propertyKey }, 1000);
 	};
 
 	protected [POLL_VALUE]: PollValueImplementation = async ({

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

@@ -130,6 +130,9 @@ export class DoorLockCCAPI extends PhysicalCCAPI {
 				);
 			}
 			await this.set(value);
+
+			// Verify the current value after a delay
+			this.schedulePoll({ property });
 		} else if (
 			typeof property === "string" &&
 			configurationSetParameters.includes(property as any)
@@ -164,6 +167,10 @@ export class DoorLockCCAPI extends PhysicalCCAPI {
 			}
 
 			await this.setConfiguration(config);
+
+			// Refresh the current value
+			// TODO: #1321, #1521
+			await this.getConfiguration();
 		} else {
 			throwUnsupportedProperty(this.ccId, property);
 		}
@@ -275,17 +282,6 @@ export class DoorLockCCAPI extends PhysicalCCAPI {
 			mode,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		// Refresh the current value after a delay
-		if (this.refreshTimeout) clearTimeout(this.refreshTimeout);
-		setTimeout(async () => {
-			this.refreshTimeout = undefined;
-			try {
-				await this.get();
-			} catch {
-				/* ignore */
-			}
-		}, this.driver.options.timeouts.refreshValue).unref();
 	}
 
 	public async setConfiguration(
@@ -302,9 +298,6 @@ export class DoorLockCCAPI extends PhysicalCCAPI {
 			...configuration,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		// Refresh the current value
-		await this.getConfiguration();
 	}
 
 	// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types

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

@@ -275,15 +275,6 @@ export class IndicatorCCAPI extends CCAPI {
 			...(typeof value === "number" ? { value } : { values: value }),
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			if (typeof value === "number") {
-				await this.get();
-			} else {
-				await this.get(value[0]?.indicatorId);
-			}
-		}
 	}
 
 	public async getSupported(

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

@@ -71,11 +71,6 @@ export class LanguageCCAPI extends CCAPI {
 			country,
 		});
 		await this.driver.sendCommand(cc, this.commandOptions);
-
-		if (this.isSinglecast()) {
-			// Refresh the current value
-			await this.get();
-		}
 	}
 }