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

Tuya Smart TRV HY369 Thermostatic Radiator Valve #3024

Closed vegetate7 closed 4 years ago

vegetate7 commented 4 years ago

Device

Hd58c50f07f924caab0c6e12045c8091cR

Screenshots

Endpoints and clusters: Screenshot_20200709_193632

Node info panel: Screenshot_20200709_193713

Basic cluster: Screenshot_20200709_193839

Identify cluster: Screenshot_20200709_193919

Smanar commented 4 years ago

Have just added "valve" and "on" based on dp 621. And a new try for "Tuya debug 4".

And for mode I prefer doing something generalist. because in 6 month we can't do rollback without breaking all config already in place.

Mode 'Off': set DP 1130, Data 2 (Force mode on, valve closed)
Mode 'heat': set DP 1130, Data 1 (Force mode on, valve opened)
Mode 'auto': set DP 1130, Data 0 (Force mode off)

You haven't reversed off and auto ? id data = 0 all is stoped no heat ? I haven't see the "holiday" mode on z2m.

For the moment I m seing only 2 solution

If you are agree I have just 2 line to add for the second solution ?

vegetate7 commented 4 years ago

You haven't reversed off and auto ? id data = 0 all is stoped no heat ?

No. not reversed. DP 1130 is for 'Forced' mode, it opens or close valve regardless of all other settings. If 'force mode' disabled (Data=0) then regular modes ("presets") takes control.

  • Put a hack according to the the device name (like that it will not break other future device)

If you are agree I have just 2 line to add for the second solution ?

Yep, I'd like do have second solution. May be in far future "presets" can be added, and this will not prevent.

"Holiday" device mode is DP 1028 , Data 0. Can be enabled by device's buttons. User have to set temperature and amount of days this mode will be in effect. Yes it is not in Z2M, there 'off' instead, but it is wrong. That's why I suggest to differentiate On/Off modes and "presets".

vegetate7 commented 4 years ago

Have just added "valve" and "on" based on dp 621. And a new try for "Tuya debug 4". "Tuya debug 4" now is a very big unsigned integer :)

04:27:38:903 Tuya debug 4: status: 24 transid: 242 dp: 621 fn: 0 data 89307660167413860

"on" works. Reflecting valve position. tested with "forced" modes too. "valve" does not. Forget to create RStateValve item for node?

vegetate7 commented 4 years ago

Hmm. I did not actually realize what I'm sticked to HA entities. From REST-API I able to set any mode. even 'complex' (it displays on device "manual" and "auto" icons together). So I do not think what to say. Worth it to make "presets" and move "mode" to DP 1130? It will make usable HA entity and "full" featured REST-API. Or leave it as is, and REST-API will still have all features, but HA entity "better do not touch" state?

vegetate7 commented 4 years ago

I made a branch from your code with presets.

{
    "config": {
        "heatsetpoint": 2000,
        "mode": "auto",
        "offset": 0,
        "on": true,
        "preset": "comfort",
        "reachable": true,
        "scheduler": null,
        "scheduleron": null
    },
    "ep": 1,
    "etag": "c5ef83b46aec9b637273d29a0012095a",
    "lastseen": "2020-07-21T00:47:53.123",
    "manufacturername": "_TYST11_ckud7u2l",
    "modelid": "kud7u2l",
    "name": "kud7u2l",
    "state": {
        "lastupdated": "none",
        "on": false,
        "temperature": 2600,
        "valve": 0
    },
    "swversion": "20180727",
    "type": "ZHAThermostat",
    "uniqueid": "ec:1b:bd:ff:fe:94:64:5a-01"
}

Will look how it works. tomorrow :)

vegetate7 commented 4 years ago

btw, it works good. From HA, I can set "mode" in HA meaning ("off" to close valve, "heat" to open valve, "auto" to pass control to presets). And from REST-API I can select presets (one of "holiday", "auto", "manual", "confort", "eco", "boost" and "complex").

Could you take a look at https://github.com/vegetate7/deconz-rest-plugin ?

Smanar commented 4 years ago

ha yes, nice, good idea. I have understand now, and I think it s better solution yes.

vegetate7 commented 4 years ago

Should I make pull request? Or you will make your own changes? tho I am not very familiar with GIT :)

Smanar commented 4 years ago

I have copied your change, so I can make the PR. But we have time, version update are made around the 15th, so the next one will be in 20 days. There is 2 3 other tuya device that can use this code on the github. I will ask for them again before.

So if you want to change something else you have time too ^^. And thx for you help, it s the first tuya device full working on deconz, other will be easier.

vegetate7 commented 4 years ago

So if you want to change something else you have time too ^^.

Ok, I've added 'config/offset' support (DP 556, Temperature calibration). May be will add Locked and WindowOpen. But. Where should I stop?

Smanar commented 4 years ago

lol Depend of your use, if you think you can use it, add it. I haven't used the "window open" on my previous valve for exemple, easier to do that with the home automation application.

I think it s possible you make change on my branch, they will be integrated automaticaly with your name, else, don't worry I will update the code myself.

vegetate7 commented 4 years ago

I can not update you branch directly. So I made PR.

remote: Permission to Smanar/deconz-rest-plugin.git denied to vegetate7.
fatal: unable to access 'https://github.com/Smanar/deconz-rest-plugin.git/': The requested URL returned error: 403
SwoopX commented 4 years ago

Just fyi, I introduced resource item "windowopen" with .79 for the Danfoss Ally.

vegetate7 commented 4 years ago

Just fyi, I introduced resource item "windowopen" with .79 for the Danfoss Ally.

Yep, I saw it. Unfortunately I can not verify which DP this device using for WindowOpen reporting, so will not add it. BTW added 'config/locked' (ChildLock) to the PR. And I think to stop here. I agree with @Smanar , most of features can be done with Home Automation software.

Smanar commented 4 years ago

I go the PR. Updated a little.

Perhaps we can add too a switch this WE > https://github.com/dresden-elektronik/deconz-rest-plugin/issues/3042

Still no ideas for the display bug for data value ...

vegetate7 commented 4 years ago

There is also more important issue. SensorNode for device is not persistent, and need re-pairing after each deconz restart.

SwoopX commented 4 years ago

That screams for an amendment of database.cpp.

vegetate7 commented 4 years ago

Updated a little

Just now sinking my fork and noticed. 'config/offset' (temperature calibration) is signed int32. So, -10 ( -1*) will be '\xFF,\xFF,\xFF,\xF6' byte array.

vegetate7 commented 4 years ago

Yep, I saw it. Unfortunately I can not verify which DP this device using for WindowOpen reporting

DP 371 is for reporting "WindowOpen". One byte, 0 - "closed" , 1 - "open". Just FYI. This mode controls valve directly, and not very useful in HA environment. Also mode settings reseted when battery changed (and other settings too :)

Smanar commented 4 years ago

Ha good catch But I think the problem will be more important, because we use in code

item = sensorNode.addItem(DataTypeInt16, RConfigOffset);

or

rItemDescriptors.emplace_back(ResourceItemDescriptor(DataTypeInt16, RConfigOffset, INT16_MIN, INT16_MAX));

I think it will be be a little more delicate.

Smanar commented 4 years ago

Can you try the last code pls ? It will perhaps solve too the display bug. I have rollback to use your code but with signed int 32 as you have said.

vegetate7 commented 4 years ago

Compiled last code. Offset set an get good. but display decimal value still no.

01:13:49:074 Tuya : debug 8
01:13:49:074 Tuya : debug 1 : size 10
01:13:49:074 Tuya debug 4: status: 33 transid: 179 dp: 515 fn: 0 payload 21b30302000400000109
01:13:49:074 Tuya debug 4: decimal value  7785940903444611337
01:13:49:388 Tuya : debug 8
01:13:49:388 Tuya : debug 1 : size 10
01:13:49:388 Tuya debug 4: status: 33 transid: 180 dp: 556 fn: 0 payload 21b42c020004fffffff6
01:13:49:388 Tuya debug 4: decimal value  7785940907739578358
01:13:49:717 Tuya : debug 8
01:13:49:717 Tuya : debug 1 : size 7
01:13:49:717 Tuya debug 4: status: 33 transid: 181 dp: 366 fn: 0 payload 21b56e01000100
01:13:49:717 Tuya debug 4: decimal value  7785940903444611072
Smanar commented 4 years ago

Ok, so the better part is working, but I realy don't understand

7785940903444611337 - 7785940903444611072 = 265 00 00 01 09 - 01 00 01 00 = -16777207

???

But I wrong, data is signed so i need to use

DBG_Printf(DBG_INFO, "Tuya debug 4: decimal value %lld\n" , data );

vegetate7 commented 4 years ago

btw, why you trying to get LongLong (8 bytes) from Long (4 bytes)? I think it's arch / compiler depended behavior. Changed to "DBG_Printf(DBG_INFO, "Tuya debug 4: decimal value %ld\n" , data);" and all works now:

03:28:22:921 Tuya : debug 8
03:28:22:921 Tuya : debug 1 : size 10
03:28:22:921 Tuya debug 4: status: 34 transid: 53 dp: 626 fn: 0 payload 22357202000400000011
03:28:22:921 Tuya debug 4: decimal value  17

Negative too:

03:33:33:409 Tuya : debug 8
03:33:33:409 Tuya : debug 1 : size 10
03:33:33:409 Tuya debug 4: status: 34 transid: 69 dp: 556 fn: 0 payload 22452c020004fffffff6
03:33:33:409 Tuya debug 4: decimal value  -10
vegetate7 commented 4 years ago

Heh. Just received second Tuya TRV. Little different. It reports 0xef00 cluter too. but different MAC prefix and model id. deconz-gui show it, even with 0xEF00 cluster. But no nodes created. This is one called Tuya TRV HY369RT in z2m. Interesting thing - tryin to add both TRV's resulting deconz crash :) Should I open new device request issue, or we can continue here?

Product name: Tuya Smart TRV HY369RT Manufacturer: _TZE200_ckud7u2l Model identifier: TS0601 Device type: [ X ] Thermostat

Device: Screenshot_20200725_043311

Node info: Screenshot_20200725_041215

Basic cluster: Screenshot_20200725_041345

0xEF00 cluster: Screenshot_20200725_041406

I can send commands to 0xef00 cluster from deconz-gui. But can not quess 'data' format.

vegetate7 commented 4 years ago

Should I open new device request issue, or we can continue here?

Added 'modelId=="TS0601"' along with 'kud7u2l'. It works! At least I tested setheatpoint and force mode switch.

{
    "config": {
        "heatsetpoint": 1550,
        "locked": null,
        "mode": "heat",
        "offset": -100,
        "on": true,
        "preset": "manual",
        "reachable": true,
        "scheduler": null,
        "scheduleron": null
    },
    "ep": 1,
    "etag": "5e3a05c65ca2e0258fa8ec56947f5f95",
    "lastseen": "2020-07-25T00:55:00.079",
    "manufacturername": "Heiman",
    "modelid": "TS0601",
    "name": "TS0601",
    "state": {
        "lastupdated": "none",
        "on": null,
        "temperature": 2650,
        "valve": null
    },
    "type": "ZHAThermostat",
    "uniqueid": "bc:33:ac:ff:fe:6e:ba:40-01"
}
SwoopX commented 4 years ago

Please add a new request for the device.

Smanar commented 4 years ago

btw, why you trying to get LongLong (8 bytes) from Long (4 bytes)? I think it's arch / compiler depended behavior. Changed to "DBG_Printf(DBG_INFO, "Tuya debug 4: decimal value %ld\n" , data);" and all works now:

Right, data is a qint32.

@SwoopX the 2 devices are using exactly the same code, why we can't merge them ?

BTW the crash is not realy good, but I don't understand it too. The device is not in whitelist (before you adding it) so deconz ignore it in normal working mode.

vegetate7 commented 4 years ago

After whitelisting HY369RT crashes is gone. Both devices can be added. May be reason were attempts to add this device as "Smart Plug" - in the HA left traces of 3 such attempts, equal of count of crashes. But hot it can depend of paired HY369 I do not know.

Anyway, added new PR. May be you will need to re-style the code.

Smanar commented 4 years ago

Ha yes, you are right, it s reconised as plug, If I have time I will check if I find something suspicious on code, but yes, I think there is a weak in deconz code somewhere.

You haven't usefull log, from deconz ou HA during your tests ?

PS: Code merged.

SwoopX commented 4 years ago

It is a different device and also has a different model ID. We should keep the devices as clean as possible, imho.

Smanar commented 4 years ago

Agree, but there is realy nothing say about this device (the second one), all have been say for previous one. And if an user use the search engine to find information, it will be almost same.

But the crash worry me, I think it appear because the device is reconised as plug but haven't on/off cluster, but I don't see something in code that can provoke a crash.

vegetate7 commented 4 years ago

It is a different device and also has a different model ID. We should keep the devices as clean as possible, imho.

I've added new DeviceRequest issue https://github.com/dresden-elektronik/deconz-rest-plugin/issues/3078 You can just link it to this issue as "the same"

vegetate7 commented 4 years ago

You haven't usefull log, from deconz ou HA during your tests ?

I haven't old logs. I run deconz in docker container, so after deconz crashed, container closed, and all logs disappearing. I tried to make new (with HY369RT enabled in the code). Deleted HY369RT, restart deconz. Re-join HY369 (old). Join HY369RT (new). No crash this time. Also new device creates SensorNode after first join. Not need second one. Do not think anything interesting in new logs, but for the case.. deconz.newtuya.log

vegetate7 commented 4 years ago

Ha. After both SensorNode created i restarted deconz again. SensorNodes gone, as expected. But new device appears now as 3 smart plugs in deconz-gui: Screenshot_20200726_001207

Re-joined devices, and all Ok again: Screenshot_20200726_001817

Logs. deconz.newtuya2.log deconz.newtuya3.log

Smanar commented 4 years ago

Ha yes, it s my fault.

Im too on a tuya device, a switch with 3 button, but only 1 endpoint so I m creating 2fake andpoint, and this switch is reconised as plug too

I need to correct that, to take a llok just search "Make 2 fakes device for tuya stuff" in code

vegetate7 commented 4 years ago

Yep, I see. I do not know how to really distinguish devices, as they have same ModelID == "TS0601". May be by "Manufacturer Name" ? Or even by combination of properties (like SupportedDevices).

Smanar commented 4 years ago
   if (sd && (sd->deviceId() == DEV_ID_SMART_PLUG) && (node->simpleDescriptors().size() < 2) && ((node->address() & 0xffffff0000000000ULL ) == silabs3MacPrefix))

The 2 devices haven't the same Mac adress, I think it will work (for the moment)

vegetate7 commented 4 years ago

'node->address()' returns class. Need something like '(node->address().ext() & macPrefixMask ) == silabs3MacPrefix)' - this compiling, but not tested yet. May be worth to check if 'ext' exists.

Smanar commented 4 years ago

Yep, sorry, not tested on my side, need to use .ext()

if (sd && (sd->deviceId() == DEV_ID_SMART_PLUG) && (node->simpleDescriptors().size() < 2) && ((node->address().ext() & 0xffffff0000000000ULL ) == silabs3MacPrefix))

vegetate7 commented 4 years ago

Built new code. Disconnected device, deleted TS0601 node, restarted deconz then enabled join, and joined device. The node created from first time, ZHAThermostat. Logs: deconz.joinTS0601.log

Restarted deconz again. And now have 3 SmartPlugs again. Screenshot_20200728_013241

Logs: deconz.restart.log

Btw, no "Tuya debug 13" in the logs. So may be those 3 SmartPlugs stored in DB. I do not know how to clean it.

Smanar commented 4 years ago

Exactly, so now, it s not my fault ^^.

And you haven't a plug device entry too (I think no, because of missing cluster 00006) ?

01:25:39:872 LightNode 34: Smart plug 34 added

But nothing on log about this device (according to mac adress) ...

If you delete it in deconz, for me it s enought to delete the deconz entry in db. And when you include it, you have only 1 endpoint displayed, the the 3 endpoint display bug is provoked at restart.

Wich one debug parameter are you using ?

And how many device are created in the API ? If Phoscon is still in permit join, you will have 3 entries, 1 by endpoint if deconz detect 3 of them.

vegetate7 commented 4 years ago

And you haven't a plug device entry too (I think no, because of missing cluster 00006) ?

LightNode created. In the API and in the Phoscon webapp.

{
    "etag": "b1da9c12e599352fefb72ac1b8b4556b",
    "hascolor": false,
    "lastannounced": "2020-07-27T19:25:57Z",
    "lastseen": "2020-07-27T19:57:06Z",
    "manufacturername": "_TZE200_ckud7u2l",
    "modelid": "TS0601",
    "name": "Smart plug 34",
    "state": {
        "alert": "none",
        "on": false,
        "reachable": true
    },
    "swversion": null,
    "type": "Smart plug",
    "uniqueid": "bc:33:ac:ff:fe:6e:ba:40-01"
}

Only one. but GUI show 3 endpoints. After re-joining device LightNode is gone, and SensorNode added in API. I'll try to delete nodes in GUI and Webapp too.

Wich one debug parameter are you using ?

dbg_info=1 Hmm. deconz crashed some times, after I disconnected TS0601. Stopped to crash when I deleted "Smart Plug 34" from Webapp.

But entry is still in DB, just with "deleted" state: bc:33:ac:ff:fe:6e:ba:40-01|34|deleted|Smart plug 34||1|TS0601|_TZE200_ckud7u2l||{"attr/lastannounced":"2020-07-27T19:25:57Z","attr/lastseen":"2020-07-27T19:57:06Z","attr/manufacturername":"_TZE200_ckud7u2l","attr/modelid":"TS0601","attr/name":"Smart plug 34","attr/swversion":null,"attr/type":"Smart plug","attr/uniqueid":"bc:33:ac:ff:fe:6e:ba:40-01","state/alert":null,"state/on":false,"state/reachable":true}

I think I'll stop deconz and manually clean the db.

vegetate7 commented 4 years ago

BTW interesting. SensorNode for old device is in the DB. But node not created after deconz restarts:

sqlite> select * from sensors where uniqueid="ec:1b:bd:ff:fe:94:64:5a-01";
29|kud7u2l|ZHAThermostat|kud7u2l|_TYST11_ckud7u2l|ec:1b:bd:ff:fe:94:64:5a-01|20180727|{"lastupdated":null,"on":null,"temperature":null,"valve":null}|{"heatsetpoint":null,"locked":null,"mode":null,"offset":0,"on":true,"preset":null,"reachable":true,"scheduler":null,"scheduleron":null}|{"d":0,"ep":1,"in":[61184],"p":260}|normal|1

Ok. I've removed from DB all entries related to TS0601. Started deconz, joined device. One "Smart Plug 34" LightNode created. one endpoint, all ok. Re-joined it, LightNode still in API, SensorNode added. Logs: deconz.cleandb.log Restarted deconz. And again, no SensorNode, only one LightNode, in the GUI one endpoint. Aand! deconz crashed shortly. Log from one of restarts between crashes. Nothing interesting: deconz.cleandb_restart_crash.log

Then I was fast, and re-joined device before deconz crashed, and crashes gone. That time I've enabled "gbg_info=2" option, so DB transactions visible now. May be helpful for work on persistence of SensorNodes. deconz.restart_info2.log

PS: Just wonder, can it be if device reporting something while not joined as thermostat, and that "something" resulting crash if processed as LightNode?

vegetate7 commented 4 years ago

PS: Just wonder, can it be if device reporting something while not joined as thermostat, and that "something" resulting crash if processed as LightNode?

in the 'void DeRestPluginPrivate::handleTuyaClusterIndication' - while processing thermostat DPs, existence of SensorNode not checked. Only at the top of function, but 'OR'-ed with LightNode check. So if LightNode exists, then indications can be processed, and there calls to non-existing node: ResourceItem *item = sensorNode->item(RConfigHeatSetpoint);

SwoopX commented 4 years ago

Yep, sorry, not tested on my side, need to use .ext()

if (sd && (sd->deviceId() == DEV_ID_SMART_PLUG) && (node->simpleDescriptors().size() < 2) && ((node->address().ext() & 0xffffff0000000000ULL ) == silabs3MacPrefix))

You can use (node->address().ext() & macPrefixMask) 😉

Smanar commented 4 years ago

PS: Just wonder, can it be if device reporting something while not joined as thermostat, and that "something" resulting crash if processed as LightNode?

in the 'void DeRestPluginPrivate::handleTuyaClusterIndication' - while processing thermostat DPs, existence of SensorNode not checked. Only at the top of function, but 'OR'-ed with LightNode check. So if LightNode exists, then indications can be processed, and there calls to non-existing node: ResourceItem *item = sensorNode->item(RConfigHeatSetpoint);

Yeah, I realy like you for debugging ^^, good debuging logic. I will take a look on this part and try to correct another problem.

In fact all problem are from the 3 button tuya switch (and ATM I have no answer about it) I m setting the hasServerOnOff to true if I detect the tuya cluster because this device haven't usefull cluster. But your device have no usefull cluster good point, but have the tuya cluster, so I set the hasServerOnOff to true too, bad point.

vegetate7 commented 4 years ago

For the sake of true, I understood may be a half :) But again IMHO, may be worth to make special function or functions for Tuya devices for cases like that? and then use it in smthg like: else if (i->inClusters()[c].id() == TUYA_CLUSTER_ID) { hasServerOnOff = getTuyaDeviceCapabilities(SERVER_ON_OFF); }

Smanar commented 4 years ago

Yep I m using this kind of fonction

       else if ((i->inClusters()[c].id() == TUYA_CLUSTER_ID) && (node->macCapabilities() & deCONZ::MacDeviceIsFFD) ) { hasServerOnOff = true; }

So for the switch, is router, is FFD > hasServerOnOff = true But for the thermostat, end device > hasServerOnOff = off

I m trying to check the crash error, but I don't see in log "Tuya debug 4" so not sure the crash was in this part, but you are right, I m putting security check.

vegetate7 commented 4 years ago

I m trying to check the crash error, but I don't see in log "Tuya debug 4"

I think deconz logs is somehow buffered. When I looking into it with "follow" option (docker logs -f --tail 0 addon_core_deconz) - logs are spitting out once per 2-3 minutes. So if "debug 4" was immediately before crash - it may not go to log.