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

Leviton DZMX1 dimmer needs a delay before automagic refresh after being manually tapped #1695

Closed
3 of 10 tasks
mdoggydog opened this issue Feb 10, 2021 · 5 comments · Fixed by #1716
Closed
3 of 10 tasks
Labels
device compatibility Work that needs to be done to support non-compliant or legacy devices help wanted Extra attention is needed
Projects

Comments

@mdoggydog
Copy link
Contributor

Describe the bug

When the physical button on the Levition DZMX1 dimmer switch is tapped, the device emits an ApplicationUpdateRequest. zwave-js correctly interprets this as a state-change notification (logging "Node does not send unsolicited updates, refreshing actuator and sensor values...") and immediately performs a refreshValues() on the node, sending a MultilevelSwitchCCGet command. But, the device immediately responds with an "Application Status" command indicating "busy, try again later", which is dropped as unimplemented by zwave-js. Ten seconds later, the outstanding Get command times out.

The consequences of all this is:

  • The state of the device is not automatically updated after a physical button tap.
  • Attempts to communicate with the device after a physical button tap (e.g., to manually poll the value, or change the value via zwave) will be delayed for 10 seconds, waiting for the automatic Get command to timeout.

I figure the "busy, try again later" response from the device was a technique to avoid violating the old Lutron patent. A refreshValues() on the node does succeed if it is delayed at least 950ms after receiving the ApplicationUpdateRequest. So, it looks like Leviton implemented "Well, let's just require a 1 second delay, so that no one can possibly call this an 'instant update'."

To handle this, I propose inserting a device-specific delay before refreshValues() is called in this block

// This is not the handler for wakeup notifications, but some legacy devices send this
// message whenever there's an update
if (this.requiresManualValueRefresh()) {
this.driver.controllerLog.logNode(this.nodeId, {
message: `Node does not send unsolicited updates, refreshing actuator and sensor values...`,
});
void this.refreshValues();
}
The delay is trivial, but what about the device-specific aspect? Would it be reasonable to just add a new parameter to the schema for device configuration files?

(Another possibility would be to implement handling the "Application Status, busy, try again later" message, and automatically retrying the request. But... even that would require knowing how long to wait before the retry, hence a device-specific parameter.)

Device information

Which device(s) is/are affected (make/model)? Leviton DZMX1
What are the node IDs? Node 2

Last Known Working Configuration

  • New device
  • Previously working device (node-zwave-js)
  • Previously working device (other platform)

Installation information
How did you install node-zwave-js?

  • zwavejs2mqtt (latest) docker image
  • zwavejs2mqtt (dev) docker image
  • ioBroker.zwave2 adapter
  • Pkg (npm install @zwave-js/server)
  • Manual Docker build
    • node-zwave-js branch:
    • zwavejs2mqtt branch:
  • Manually built (to experiment with delays before refreshValues())
  • Other:

To Reproduce
Steps to reproduce the behavior:

  1. Have DZMX1 in network.
  2. Tap its button.
  3. Within the next 10 seconds, try to do something with it over RF.
  4. Look at logs.

Additional context
none

Logfile:

This logfile is from stock zwave-js (without any delays hacked in). The timeline is:

  • Light is off, zwave-js-server started up.
  • Human taps physical button.
  • Device calls for help, zwave-js responds, device says "Naw, I am busy", zwave-js ignores that.
  • 3-4 seconds later, human runs script asking zwave-js to poll the current level value and then set the value to zero.
  • 10 seconds after the button tap, when zwave-js's original automatic Get request times out, the requested poll/set finally occur.

zwave-175924.log

@mdoggydog mdoggydog added the bug Something isn't working label Feb 10, 2021
@zwave-js-bot zwave-js-bot added this to Needs triage in Triage Feb 10, 2021
@AlCalzone
Copy link
Member

  1. I don't think waiting 1 second before polling is terrible here. After all - that is a workaround for legacy devices.
  2. The 10s delay should no longer be a problem once Measure command time, time out responses accordingly #1518 is solved.

@AlCalzone AlCalzone added device compatibility Work that needs to be done to support non-compliant or legacy devices help wanted Extra attention is needed and removed bug Something isn't working labels Feb 10, 2021
@mdoggydog
Copy link
Contributor Author

  1. I don't think waiting 1 second before polling is terrible here. After all - that is a workaround for legacy devices.

I think I could come up with a 1-line PR for a fixed-duration delay. :^) But...

...I wouldn't want to introduce a regression. If I have legacy devices that do not require the delay and they happily notify me of status changes within 10's of ms with the current code, and then after an update there is a weird inexplicable 1s delay, I would be confused at first and then peeved.

Looking back at #398 (the issue that led to this code in the first place), I wonder if @RoboPhred's switch requires a delayed refresh or not. I'd guess "no", otherwise he/she would have reported that the value still wasn't getting updated.

  1. The 10s delay should no longer be a problem once Measure command time, time out responses accordingly #1518 is solved.

(But with that alone, the refreshValues() would still timeout (albeit faster) and there would still be no update, right?)

@AlCalzone
Copy link
Member

Hm you got a point there. I'm not sure how frequently this will come up, but I guess we could add a compat flag manualValueRefreshDelay.

@mdoggydog
Copy link
Contributor Author

I can take a stab at adding this.

Any hints/clues as to where to start (a filename, a keyword --- some crate or barrel to form the basis of my cargo cult) would be appreciated.

@AlCalzone
Copy link
Member

There are a few parts to this.

  1. define the compat flat:
    1.1 JSON schema: maintenance\schemas\device-config.json, somewhere in the properties key, line 223
    1.2 parsing: packages\config\src\CompatConfig.ts, class CompatConfig
    1.3 Documentation: docs\development\config-files.md
  2. use the compat flag (you know where), you can access it with this.deviceConfig.compat?.manualValueRefreshDelay.

mdoggydog added a commit to mdoggydog/node-zwave-js that referenced this issue Feb 11, 2021
See zwave-js#1695.  Some devices (e.g., Leviton DZMX1) will not only surreptitiously
signal a locally-caused state change via a spontaneous NIF, they also
require the update request to be delayed in order to handle it successfully.
This compat option allows for specifying such a delay in milliseconds.
mdoggydog added a commit to mdoggydog/node-zwave-js that referenced this issue Feb 11, 2021
Empirical testing with a Leviton DZMX1 (firmware version 0.7)
revealed that it would respond with "busy, try later" messages for
delays less than ~940ms.  So, set this value to 1000ms.

Fixes zwave-js#1695.
Triage automation moved this from Needs triage to Closed Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
device compatibility Work that needs to be done to support non-compliant or legacy devices help wanted Extra attention is needed
Projects
No open projects
Triage
Closed
2 participants