OpenZWave / open-zwave

a C++ library to control Z-Wave Networks via a USB Z-Wave Controller.
http://www.openzwave.net/
GNU Lesser General Public License v3.0
1.05k stars 912 forks source link

Invalid thermostat set point on Secure SRT323 (or Horstmann HRT4-ZW) #1239

Open dsoulayrol opened 7 years ago

dsoulayrol commented 7 years ago

Hi,

I observe that, sometimes, changing the thermostat set point on Secure SRT323 results in the minimal value being displayed on the object (5°C) and a negative value returned on the network.

My application exposes the thermostat to the user as an object with plus, minus and set buttons. The first two augment or diminish the current set point by 0.5 steps, the last one sets a value provided by the user. Hence, in the attached logs, it is possible to see a lonely ThermostatSetpointCmd_Set command queued when the set button is used, or a series of them when plus or minus are pressed multiple times in a row.

horstmann_setpoint.txt

In the logs, the object has the ID 89. Its Wake Up Interval is set to 15 minutes. I have first successfully changed the set point to 28, and then pressed the minus button until I got 23. open-zwave accurately sends all the successive values until the last one:

2017-06-02 10:52:33.239 Info, Node089, Sending (WakeUp) message (Callback ID=0x61, Expected Reply=0x13) - ThermostatSetpointCmd_Set (Node=89): 0x01, 0x0c, 0x00, 0x13, 0x59, 0x05, 0x43, 0x01, 0x01, 0x21, 0xe6, 0x25, 0x61, 0x7c

But the returned value is -2.6:

2017-06-02 10:52:33.300 Detail, Node089, Received: 0x01, 0x0c, 0x00, 0x04, 0x00, 0x59, 0x06, 0x43, 0x03, 0x01, 0x22, 0xff, 0xe6, 0xd2 2017-06-02 10:52:33.300 Detail, 2017-06-02 10:52:33.300 Info, Node089, Response RTT 35 Average Response RTT 35 2017-06-02 10:52:33.300 Detail, Node089, Refreshed Value: old value=28.0, new value=-2.6, type=decimal

I then reproduced once the problem and, later, I used the set button to fix the same value. This time, the value is encoded differently in the request:

2017-06-02 11:27:31.997 Info, Node089, Sending (WakeUp) message (Callback ID=0x70, Expected Reply=0x13) - ThermostatSetpointCmd_Set (Node=89): 0x01, 0x0c, 0x00, 0x13, 0x59, 0x05, 0x43, 0x01, 0x01, 0x01, 0x17, 0x25, 0x70, 0xbc

And the process is successful:

2017-06-02 11:27:32.060 Detail, Node089, Received: 0x01, 0x0c, 0x00, 0x04, 0x00, 0x59, 0x06, 0x43, 0x03, 0x01, 0x22, 0x00, 0xe6, 0x2d 2017-06-02 11:27:32.060 Detail, 2017-06-02 11:27:32.060 Info, Node089, Response RTT 35 Average Response RTT 35 2017-06-02 11:27:32.060 Detail, Node089, Refreshed Value: old value=-2.6, new value=23.0, type=decimal

As long as I understand the Z-Wave specifications, both encodings are valid. (But how does open-Zwave chooses the number of bytes?). I can't decide if the problem could be triggered by the encoding of the value in the request, the possibly long series of ThermostatSetpointCmd_Set requests, a bug in the device... Or did I miss something?

dsoulayrol commented 7 years ago

More analysis seems to indicate that the problem is triggered when the value received by open-zwave is a float (that is, when there is at least one decimal, even if it is '0'), and can be encoded on one byte. For example: 25.0 is 0xFA with precision=1. The specifications and a review of the CommandClass::ValueToInteger show this is valid. So the problem must be on the object side.

I guess this ticket can be closed. However, I'd like to take advantage of this thread to know if this is a real bug, or if I should have known that the device is limited in some way? Did I miss something in the protocol that tells the device does only support integers for example?

Thanks.

dsoulayrol commented 7 years ago

Hi.

I have today observed a surprisingly similar behaviour on a Danfoss Room Sensor. Setting, for example, 21.5 returned the value configured in Min/Override Setpoint, while setting integer values, like 21, works as expected.

I have configured the Min/Override Setpoint and Max/Override Setpoint values so that they don't get in the way, but the error remains. The traces from open-zwave show once again that an erroneous value is returned by the object in the ThermostatSetpointCmd_Get packet.

I still don't believe there is a problem with the implementation of the ThermostatSetpointCmd_Set command on open-zwave, but I'm clueless to understand why those two thermostat objects behave like this.

Fishwaldo commented 7 years ago

this is probably related to #819 or #820

dsoulayrol commented 7 years ago

Thanks for your response.

Sorry I did not find those issues earlier, as it seems my devices behave exactly like the Qubino thermostat in #776, referenced by #820.

I understand you are waiting for a test of the ThermostatSetpointCmd_Set with the parameter "override_precision" set in the device configuration file as following?

<CommandClass id="67" base="0" override_precision="2" />
haimgel commented 7 years ago

Not sure if I should open a new ticket with this, but I think I'm experiencing the same (or related) issue, with a different thermostat.

On "Honeywell TH8320ZW1000 Touchscreen Thermostat", setting the setpoint to any whole number works as expected, and setting the setpoint to any fractional number (like 23.5), results in thermostat going down to 10C (the lowest value). Thermostat supports setting the temperature in 0.5C increments.

I've verified this, setting the temperature in OpenZWave Control Panel, so this is definitely an issue in OpenZWave, and not in upper level stacks.

Update: Added override_precision="2" to CommandClass in my configuration file, and it solved the issue!

angrycamel commented 5 years ago

I had the same issue for the HRT-ZW4 / SRT323 and added the override_precision="2" to the zwcfg xml file for HomeAssistant and it worked well. Is it something that can be made a default precision for these devices? Because it seems like everyone who uses it with ozw will need to do this override, as things stand

glward commented 5 years ago

I'm seeing the same thing with the "Honeywell T6 Pro TH6320ZW2003"; using the override_precision flag fixes it here as well.

That said, I believe this is a bug in OpenZWave's encoding of the setpoint value field. According to the most recent Z-Wave spec found here, section 4.108.2, the value field of THERMOSTAT_SETPOINT_SET:

This field MUST be encoded using signed representation and comply with Table 10, Signed field encoding (two’s complement representation).

My interpretation of this is that any number in this field must be encoded as two's compliment, even positive integers. This means that any common setpoint in Celsius with precision 1 would require 2 bytes, since the effective range would only be -12.8 to 12.7C. The thermostat is probably correct in interpreting, say, "0xF5" (245) as "-11.0" instead of "24.5" based on this interpretation of the spec, and it handles it by instead setting it to the lowest supported value.

The check for value encoding in CommandClass::ValueToInteger does not account for this or any other Value fields which "MUST" (per the spec) be encoded as two's compliment signed integers, so for any value which encodes to 128..255, it will incorrectly encode them in one byte instead of two.

automaton82 commented 4 years ago

Honeywell Zwave TH8320ZW1000 also experiences this problem. I think at this point OZW is not implementing the spec correctly.