Skip to content

Commit

Permalink
fix: add additional 1s delay before verifying a transitioned value (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
AlCalzone authored Oct 25, 2021
1 parent 4ad4f6d commit 27bb4ee
Show file tree
Hide file tree
Showing 13 changed files with 54 additions and 29 deletions.
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

0 comments on commit 27bb4ee

Please sign in to comment.