Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add additional 1s delay before verifying a transitioned value #3592

Merged
merged 1 commit into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions packages/zwave-js/src/lib/commandclass/API.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ValueChangeOptions, ValueID } from "@zwave-js/core";
import type { Duration, ValueChangeOptions, ValueID } from "@zwave-js/core";
import {
CommandClasses,
Maybe,
Expand Down Expand Up @@ -82,6 +82,11 @@ export function throwWrongValueType(
);
}

export interface SchedulePollOptions {
duration?: Duration;
transition?: "fast" | "slow";
}

/**
* The base class for all CC APIs exposed via `Node.commandClasses.<CCName>`
* @publicAPI
Expand Down Expand Up @@ -128,8 +133,17 @@ export class CCAPI {
*/
protected schedulePoll(
property: ValueIDProperties,
timeoutMs: number = this.driver.options.timeouts.refreshValue,
{ duration, transition = "slow" }: SchedulePollOptions = {},
): boolean {
// Figure out the delay. If a non-zero duration was given or this is a "fast" transition,
// use/add the short delay. Otherwise, default to the long delay.
const durationMs = duration?.toMilliseconds() ?? 0;
const additionalDelay =
!!durationMs || transition === "fast"
? this.driver.options.timeouts.refreshValueAfterTransition
: this.driver.options.timeouts.refreshValue;
const timeoutMs = durationMs + additionalDelay;

if (this.isSinglecast()) {
const node = this.endpoint.getNodeUnsafe();
if (!node) return false;
Expand Down
9 changes: 8 additions & 1 deletion packages/zwave-js/src/lib/commandclass/BinarySwitchCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,14 @@ export class BinarySwitchCCAPI extends CCAPI {
// Verify the current value after a delay
// We query currentValue instead of targetValue to make sure that unsolicited updates cancel the scheduled poll
if (property === "targetValue") property = "currentValue";
this.schedulePoll({ property }, duration?.toMilliseconds() ?? 1000);
this.schedulePoll(
{ property },
{
duration,
// on/off "transitions" are usually fast
transition: "fast",
},
);
} else if (this.isMulticast()) {
if (!this.driver.options.disableOptimisticValueUpdate) {
// Figure out which nodes were affected by this command
Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/ColorSwitchCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,10 @@ export class ColorSwitchCCAPI extends CCAPI {
await this.set({ [propertyKey]: value, duration });

if (this.isSinglecast()) {
// Verify the current value after a delay
// Verify the current value after a (short) delay
this.schedulePoll(
{ property, propertyKey },
duration?.toMilliseconds(),
{ duration, transition: "fast" },
);
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/zwave-js/src/lib/commandclass/ConfigurationCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,8 @@ export class ConfigurationCCAPI extends CCAPI {
// Verify the current value after a delay
(this as ConfigurationCCAPI).schedulePoll(
{ property, propertyKey },
1000,
// Configuration changes are instant
{ transition: "fast" },
);
}
};
Expand Down
13 changes: 3 additions & 10 deletions packages/zwave-js/src/lib/commandclass/MultilevelSwitchCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,8 @@ export class MultilevelSwitchCCAPI extends CCAPI {
// We query currentValue instead of targetValue to make sure that unsolicited updates cancel the scheduled poll
if (property === "targetValue")
property = "currentValue";
this.schedulePoll(
{ property },
duration?.toMilliseconds(),
);

this.schedulePoll({ property }, { duration });
}
} else if (this.isMulticast()) {
// Only update currentValue for valid target values
Expand Down Expand Up @@ -382,12 +380,7 @@ export class MultilevelSwitchCCAPI extends CCAPI {
// We query currentValue instead of targetValue to make sure that unsolicited updates cancel the scheduled poll
if (property === "targetValue")
property = "currentValue";
// TODO: #1321
const duration = undefined as Duration | undefined;
this.schedulePoll(
{ property },
duration?.toMilliseconds(),
);
this.schedulePoll({ property }, { duration });
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/SoundSwitchCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ export class SoundSwitchCCAPI extends CCAPI {
await this.stopPlaying();
}
if (this.isSinglecast()) {
// Verify the current value after a delay
this.schedulePoll({ property });
// Verify the current value after a (short) delay
this.schedulePoll({ property }, { transition: "fast" });
}
} else {
throwUnsupportedProperty(this.ccId, property);
Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/ThermostatFanModeCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ export class ThermostatFanModeCCAPI extends CCAPI {
}

if (this.isSinglecast()) {
// Verify the current value after a delay
this.schedulePoll({ property });
// Verify the current value after a (short) delay
this.schedulePoll({ property }, { transition: "fast" });
}
};

Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/ThermostatModeCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ export class ThermostatModeCCAPI extends CCAPI {
await this.set(value);

if (this.isSinglecast()) {
// Verify the current value after a delay
this.schedulePoll({ property });
// Verify the current value after a (short) delay
this.schedulePoll({ property }, { transition: "fast" });
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,11 @@ export class ThermostatSetpointCCAPI extends CCAPI {
await this.set(propertyKey, value, preferredScale ?? 0);

if (this.isSinglecast()) {
// Verify the current value after a delay
this.schedulePoll({ property, propertyKey });
// Verify the current value after a (short) delay
this.schedulePoll(
{ property, propertyKey },
{ transition: "fast" },
);
}
};

Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/UserCodeCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ export class UserCodeCCAPI extends PhysicalCCAPI {
throwUnsupportedProperty(this.ccId, property);
}

// Verify the current value after a delay
this.schedulePoll({ property, propertyKey });
// Verify the current value after a (short) delay
this.schedulePoll({ property, propertyKey }, { transition: "fast" });
};

protected [POLL_VALUE]: PollValueImplementation = async ({
Expand Down
4 changes: 2 additions & 2 deletions packages/zwave-js/src/lib/commandclass/WakeUpCC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export class WakeUpCCAPI extends CCAPI {
await this.setInterval(value, this.driver.controller.ownNodeId ?? 1);

if (this.isSinglecast()) {
// Verify the current value after a delay
this.schedulePoll({ property });
// Verify the current value after a (short) delay
this.schedulePoll({ property }, { transition: "fast" });
}
};

Expand Down
1 change: 1 addition & 0 deletions packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ const defaultOptions: ZWaveOptions = {
nonce: 5000,
sendDataCallback: 65000, // as defined in INS13954
refreshValue: 5000, // Default should handle most slow devices until we have a better solution
refreshValueAfterTransition: 1000, // To account for delays in the device
serialAPIStarted: 5000,
},
attempts: {
Expand Down
8 changes: 7 additions & 1 deletion packages/zwave-js/src/lib/driver/ZWaveOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,16 @@ export interface ZWaveOptions {

/**
* @internal
* How long to wait for a poll after setting a value
* How long to wait for a poll after setting a value without transition duration
*/
refreshValue: number;

/**
* @internal
* How long to wait for a poll after setting a value with transition duration. This doubles as the "fast" delay.
*/
refreshValueAfterTransition: number;

/**
* How long to wait for the Serial API Started command after a soft-reset before resorting
* to polling the API for the responsiveness check.
Expand Down