dresden-elektronik / deconz-rest-plugin

deCONZ REST-API plugin to control ZigBee devices
BSD 3-Clause "New" or "Revised" License
1.89k stars 496 forks source link

v2.05.09 Xiaomi magic side effects #458

Closed ebaauw closed 6 years ago

ebaauw commented 6 years ago

I'm seeing some funny things as result of the changes for the Xiaomi "magic":

  1. I now see a state.voltage and a state.temperature attribute for each of my CLIP sensors. Looks like they're initialised to 3015 and 1900. For the CLIPTemperature, the original state.temperature value seems to have been overwritten.
  2. For the Xiaomi sensors, the state.voltage and state.temperature attributes only seem to appear on the first report.
  3. It would seem that the two-attribute report (when pressing the reset) is not recognised.
manup commented 6 years ago
  1. I now see a state.voltage and a state.temperature attribute for each of my CLIP sensors. Looks like they're initialised to 3015 and 1900. For the CLIPTemperature, the original state.temperature value seems to have been overwritten.

Damn I think they slipped through the filter when the indication was received with nwk source address.

  1. For the Xiaomi sensors, the state.voltage and state.temperature attributes only seem to appear on the first report.

I'll need to wait for some reports but it should work normally with latest commit.

  1. It would seem that the two-attribute report (when pressing the reset) is not recognised.

Yes that's the case the first attribute needs to be skipped, I need to check some data first so that the given string.lengt of the first attribute is correct and can be skipped without side effects.

ebaauw commented 6 years ago

Damn I think they slipped through the filter when the indication was received with nwk source address.

Yeah I think so too. Better check that the sensor found is indeed a Xiaomi device (modelId starts with lumi. or something) - oh I see you've done that already ;-)

I'll need to wait for some reports but it should work normally with latest commit.

I think the resource items should be added from addSensorNode() and sqliteLoadAllSensorsCallback() instead of from handleZclAttributeReportIndicationXiaomiSpecial()

I need to check some data first so that the given string.lengt of the first attribute is correct and can be skipped without side effects.

From what I've seen: yes. I assume the 0x0005 attribute will be handled regularly anyways, so you'd just need to skip length bytes and check again for attibute 0xff01 and ZclCharacterString. It might be prudent to read length from the stream only after confirming the data type is ZclCharacterString and when the stream is not empty.

What about updating state.lastupdated?

I really prefer to use config.battery over state.voltage (or state.batteryvoltage). It might be enough simply to push back a Power Configuration cluster to the sensor's fingerprint to add config.battery to the Xioami sensors.

Oh, and there's a typo in one of log messages Xiomi instead Xiaomi.

manup commented 6 years ago

I think the resource items should be added from addSensorNode() and sqliteLoadAllSensorsCallback() instead of from handleZclAttributeReportIndicationXiaomiSpecial()

Agree, the idea was to add it for already paired sensors on the fly, but addSensorNode() and sqliteLoadAllSensorsCallback() is sure the saner and more logic way — I'll fix that [1].

From what I've seen: yes. I assume the 0x0005 attribute will be handled regularly anyways, so you'd just need to skip length bytes and check again for attibute 0xff01 and ZclCharacterString. It might be prudent to read length from the stream only after confirming the data type is ZclCharacterString and when the stream is not empty.

Agree, will be added to the next version. If I remember correctly when pairing the sensors they also send such combined messages, good to have the state very fast.

What about updating state.lastupdated?

See https://github.com/dresden-elektronik/deconz-rest-plugin/commit/e5a3891ecb9fc1a1eb1ceab06dafeb517cb64b14#commitcomment-27912513

I really prefer to use config.battery over state.voltage (or state.batteryvoltage). It might be enough simply to push back a Power Configuration cluster to the sensor's fingerprint to add config.battery to the Xioami sensors.

With the suggested linear mapping to 0–100 we can just update the config.battery state which is added prior in [1], with that I'm ok to remove the state.voltage from state albeit it's quite interesting to see the voltage :) The fingerprint doesn't need to be changed for that.

Oh, and there's a typo in one of log messages Xiomi instead Xiaomi.

fixed now

wvuyk commented 6 years ago

I am here seeing the state.temperature on the Xiaomi cubes as well now. Is the cube reporting temperatures? The value changed once so far from 2100 to 2200 in the REST API. Am wondering what this is really?

ebaauw commented 6 years ago

Yes, all (?) Xiaomi devices have a crude temperature sensor. state.temperature is in 0.01 ˚C, so 2100 means 21˚C. The resolution of the crude sensor is 1˚C, so the last two digits will always be 0.

Note that state.voltage and state.temperature (for the Xiaomi sensors) has been changed to config.battery and config.temperature in v2.05.10. Of course, state.voltage remains for power measurement, and state.temperature for "real" temperature sensors (incl. the Xiaomi Aqara weather and Xiaomi Mi temperature/humidity sensors).

wvuyk commented 6 years ago

I saw this changeindeed after installing 2.05.10. Why I was asking is that for most temperature readings ZHATemperature sensors were created. With this change I will just ignore the confg.temperature as it is not very usefull I guess... config.battery is very usefull on the other hand

ebaauw commented 6 years ago

That’s what I’ve done in homebridge-hue, at least for now. I added config.offset to ZHATemperature, because the Hue motion sensors are not very accurate either, but this assumes there’s a fixed offset between the reported and the actual temperature.

wvuyk commented 6 years ago

I did the same, having a few oregon temperature devices and philips motion sensors and seeing different temperatures on every device, even next to each other on the table after a few hours. Even the oregon devices differ on the smae models. Makes me wonder where to find a reliable measurements for households.

I am not using config.offset yet, handling it now in the plugin’s settings, but might move it to config.offset so other apps might take advantage of it too.

I have the xiaomi plug on order here, will test the voltage and consumption also. Might come in handy as Homeseer already has an energy database in place for Zwave. Might be good to merge in there for Zigbee devices also.