Alarm.com ADC-T2000 Configuration Parameter 5

klyoku commented 2 years ago

Is your problem within Home Assistant (Core or Z-Wave JS Integration)?

NO, my problem is NOT within Home Assistant or the ZWave JS integration

Is your problem within ZWaveJS2MQTT?

NO, my problem is NOT within ZWaveJS2MQTT

Checklist

[X] I have checked the troubleshooting section and my problem is not described there.
[X] I have read the changelog and my problem was not mentioned there.

Describe the bug

The configuration for parameter 5 has some form of mask/offset, so that the valid range is not actually -100 to 100. For example, 704643072 in the parameter means 0 offset. As a result, zwavejs gives an error when trying to set this configuration parameter:

Here is how it should be set written by the developer of the device handler from smartthings: https://community.smartthings.com/t/release-alarm-com-z-wave-thermostat-adc-t2000/74833/28

Device information

Manufacturer: 0x0190 Model name: adc-t_2000

How are you using `node-zwave-js`?

[ ] zwavejs2mqtt Docker image (latest)
[ ] zwavejs2mqtt Docker image (dev)
[ ] zwavejs2mqtt Docker manually built (please specify branches)
[ ] ioBroker.zwave2 adapter (please specify version)
[X] HomeAssistant zwave_js integration (please specify version)
[ ] pkg
[ ] node-red-contrib-zwave-js (please specify version, double click node to find out)
[ ] Manually built from GitHub (please specify branch)
[ ] Other (please describe)

Which branches or versions?

8.11.6

Did you change anything?

no

If yes, what did you change?

No response

Did this work before?

No, it never worked anywhere

If yes, where did it work?

No response

Attach Driver Logfile

to be attached when required.

AlCalzone commented 2 years ago

I wonder if this is the same as for the ADC-T3000, that was added here: https://github.com/zwave-js/node-zwave-js/pull/4131 I guess parts of the new config from there need to be carried over. @EiNSTeiN- you don't have a T2000 by any chance? :)

zwave-js-assistant[bot] commented 2 years ago

Because of the large amount of Z-Wave devices, we cannot write all configuration files ourselves. Help from the community is required, so we can focus our time on improving Z-Wave JS itself. 🙏🏻

This issue has been labeled "Help wanted", meaning we kindly ask anyone who wants to help out for support. Here are a few resources to get you started - don't hesitate to ask if you are having problems:

We may get around to doing it ourselves at some point, but community support will speed up this process immensely.

Thanks!

EiNSTeiN- commented 2 years ago

I have 2 of those but not connected to HA right now... Last time I connected one, I did notice the zwave float values and I made a note to open a PR when I get ready to transfer my T2000 thermostats to HA.

If someone else wants to tackle this first, it should be a matter of adding a 0xffff00 bitmask like this on every configuration parameter that represents a temperature in the adc-t2000 config.

blhoward2 commented 2 years ago

I don't think that you're trying to set a valid value for your device. Our parameter matches the zwave alliance DB, OpenHAB, and the SmartThings handler that you referenced. Just set the value you want, -100 to 100.

At SmartThings, the parameter is the same as ours:

<Value type="byte" byteSize="4" index="5" label="Calibration Temperature" min="-100" max="100" value="0" setting_type="zwave" fw="" disabled="false">
 <Help>
Calibration Temperature Range (in deg. F) Precision is tenths of a degree.
Range: -100 to 100
Default: 0
</Help>
</Value>

klyoku commented 2 years ago

I've tried to set it with -100 to 100 in the beginning. It says the parameter has been updated, but the thermostat doesn't reflect the change, and if I refresh the Z-Wave device configuration page, I see the number is reverted. See below screenshots:

This is when I set it to 20. As you can see, no error is shown:

The thermostat reading is not reflected after the change:

And if I go back to the device configuration page, the number has been reverted. (note this used to say 704643072 when I initially opened this issue. It has recently changed to truncate the digits)

Let me know how I can submit a more detailed report/log if you suspect I've done something wrong instead of the bitmask issue as suggested by EiNSTeiN

blhoward2 commented 2 years ago

What do you expect this to do? It says precision not offset. I'd expect this to be a dead band setting that lets the temperature exceed or be under the set temp before the thermostat switches. I do this with mine (using a different thermostat) so it kicks in at 72 and runs until it's 74 before turning off.

This wouldn't be the first device to report back an improper value. We'd need to see the zwavejs logs at a silly level.

EiNSTeiN- commented 2 years ago

Keep in mind the value is encoded as a zwave float. The top 8 bits are the size/scale/precision, the bottom 8 bits are unused, the middle 16 bits are the actual value. Everything except the middle 16 bits are read only so all you need to do is shift the value you want to set by 8 bits (multiply by 256). Instead of 100, enter 25600 and that should set that zwave float value to 10.0.

blhoward2 commented 2 years ago

I'm not sure that's true. Not per the Smartthings handler. This isn't a partial parameter anywhere. If it's wrong, it's wrong everywhere, which is possible.

Someone is going to have to do it and test it, or back that up with logs, before we're going to deviate from everywhere else.

If this should be partials like the other one, then we're missing a bunch but we aren't adding those blind.

EiNSTeiN- commented 2 years ago

Yes it is. What else does 704643072 decode to if not a zwave float. It is 0x2A000000, where 0x2A is the correct precision/size/encoding for tenths of a degree, Fahrenheit, 2 byte value.

blhoward2 commented 2 years ago

We've seen lots of devices before that report back something completely erroneous. All I'm saying is so far we have a screenshot with one value being set. The fact it's reporting that back doesn't mean that is the correct format to set it. Manufacturers make all sorts of mistakes.

So, again, someone needs to make the change you propose, test it, and provide logs validating that it works.

You can set any value in zwavejs2mqtt. So testing this shouldn't be that hard and doesn't even require changing the file. Let's see someone set a value encoded and get back the same encoded value.

EiNSTeiN- commented 2 years ago

Gotcha, I agree I haven't tested. Can do it tomorrow as I've recently connected my T2000 to HA

zwave-js-assistant[bot] commented 2 years ago

This issue has not seen any recent activity and was marked as "stale 💤". Closing for housekeeping purposes... 🧹

Feel free to reopen if the issue persists.

AlCalzone commented 2 years ago

@EiNSTeiN- any new insights?

EiNSTeiN- commented 2 years ago

Sorry for the delay

I had to increase maxValue for one of the parameter to test this. I changed parameter 5 from 704643072 (0x2A000000) to the new value 704645632 (0x2A000A00) which should set the calibration temp to 10 (in 0.1F increments, so 1 degree).

I tried setting the value to 704668672/0x2A006400 (or how I expect the value 100 would be encoded) for parameter 5, this works and the value sticks, but anything above 100 (e.g. 101 would be encoded as 0x2A006500) does not stick and reverts back, which points to what I would expect if the value is encoded into the second and third bytes like for the ADC-3000. Since parameter 5 has a -100 to 100 range, setting values outside this range is rejected by the device.

I also tested setting the value back to 0x2A000000 and then to 2A0064FF (i.e. 100 for the value, and all bits set for the lower byte). This sets the value to 704668672/0x2A006400 as we would expect if the bottom byte is unused or readonly.

zwave-js / node-zwave-js