node-zwave-js icon indicating copy to clipboard operation
node-zwave-js copied to clipboard

Proof of concept: let device config files override the interview for specific CCs

Open j9brown opened this issue 3 years ago • 2 comments

This is a proof of concept of a general purpose mechanism to allow device config files to work around invalid information supplied by devices. It introduces a new config flag "commandClasses.interview" that when present causes the node to skip the interview for a given CC and endpoint and to store the supplied values into the valueDB.

This set of patches includes a configuration file that has a rather absurd number of bugs including off-by-one errors in the thermostat setpoint bitmap and some quirks with the setpoint scale. These changes completely resolve #4588.

Let me know what you think!

fixes: https://github.com/zwave-js/node-zwave-js/issues/1625

j9brown avatar May 11 '22 03:05 j9brown

Incidentally, another approach I considered instead of using compat flags to set internal values was to add a compat flag to rewrite requests or responses from the nodes. e.g. When preparing to send command XYZ to the node, rewrite it as ABC. Or, instead of actually sending XYZ, use this simulated response DEF.

The problem with rewriting messages is that we'd have to come up with some syntax for expressing messages and rewrite rules in device config files. Messages could be expressed as hex strings but that's not very user friendly or flexible. Rules could be expressed using a template processing language but that might be overkill. And we can't just reuse the existing CC serialization with reflection to make JSON objects to compare and rewrite because there are side-effects in the CC constructors (especially reports).

So at least for now, it seems better to solve the problem in a different way. shrug

(I guess we do still have the option of adding a thermostat specific compat flag with an option to override the scale value and disable learning and an option to apply a workaround for off-by-on errors in the setpoint type bitmap encoding.)

j9brown avatar May 15 '22 04:05 j9brown

The problem with rewriting messages is that we'd have to come up with some syntax for expressing messages and rewrite rules in device config files. Messages could be expressed as hex strings but that's not very user friendly or flexible. Rules could be expressed using a template processing language but that might be overkill.

A "nicer" way would probably be hooking into the API calls via compat flags, e.g. by stating what a call to ThermostatSetpointCCAPI.getSupported returns. But that still has the downside that some of the information is stored as a side effect of the constructor, which would need to be refactored to be done in a different location.

Side note: I'm currently in the process of a huge refactor which will remove the hard coupling between CC / Message classes and the driver instance. Once that is done, this approach will be much simpler to implement.

I'm fine with either approach. The one followed in this PR can be more fine-grained, but is more verbose. Replacing the API calls is currently blocked by https://github.com/zwave-js/node-zwave-js/pull/4614 (or probably another PR I'll cherry-pick out of that soon)

AlCalzone avatar May 17 '22 06:05 AlCalzone

@j9brown I finally went forward with the approach I mentioned in my last comment here --> https://github.com/zwave-js/node-zwave-js/pull/5987

The device-specific fixes you did here are now tracked in https://github.com/zwave-js/node-zwave-js/issues/5990

AlCalzone avatar Jul 10 '23 11:07 AlCalzone