dresden-elektronik / deconz-rest-plugin

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

DDF clone and fixes for Tuya Smart Knobs (_TZ3000_* TS004F) #7904

Open chishm opened 1 month ago

chishm commented 1 month ago

This follows on from the work of @manup in #6242, to add device clones of the Tuya Smart Knob and to extend the current capabilities of the DDF.

Each way of pressing and turning the knob is now indicated by a different button code. There are 2 modes ("command" and "event") and two distinct rotations in command mode (just rotate vs press down and rotate). E.g. rotating clockwise in command mode will give button code 1030, pressing and rotating in command mode gives code 2030, and rotating in event mode gives code 3030 (there is no press and rotate for event mode).

There is now a resources item (config/mode) to discover the current mode ("command" or "event").

The state/angle value is now a relative angle for the current rotation event. It didn't make sense to have a single absolute angle when the device can be twisted in multiple ways.

The device will now send commands to the correct group address. This is achieved using the Add Group command in the Group cluster. Having to do so is a quirk of this device (and possibly other Tuya devices).

NB: The group add command is performed using a script in the DDF. To support this, the C++ code was modified to allocate "auto" groups before calling any write function in the DDF.

Extending the implementation detail of @manup's work, the state/ct item is used to parse the Color Control cluster for pressed-down rotations. The item is set to "public": false and is not exposed to the REST-API.

TODO: The battery level is not being reported for some reason.

A lot of this work drew upon knowledge provided by others. I'd like to thank:

github-actions[bot] commented 1 month ago

Hey @chishm, thanks for your pull request!

[!TIP] Modified bundles can be downloaded here. Relative expire date

DDB changes

Modified

Validation

[!TIP] Everything is fine !

:clock630: Updated for commit c1b67eb49a5bb4570c33d1c6215206fc36bb6728

Smanar commented 1 month ago

Nice, so much work on thoses device, I realy like your JS scripts with all comments, but I have a doubt, how the JS core will handle them, it will ignore them, or use memory for nothing ? On my side I make shorter code possible, but less instructive/reusable than yours.

TODO: The battery level is not being reported for some reason.

Yeah, good question, I don't see the problem too, still nothing after a time ? if the binding don't work, you can have the poll. If you try to read the attribute in deconz it update the value in the API (take care you can test that only 1 time ^^)

    {
      "bind": "unicast",
      "src.ep": 1,
      "dst.ep": 1,
      "cl": "0x0000"
    },

You are sure this part is usefull ?

BTW the bot that validate DDF need to take some holidays, embittered bot ....

manup commented 1 month ago

Nice, so much work on thoses device, I realy like your JS scripts with all comments, but I have a doubt, how the JS core will handle them, it will ignore them, or use memory for nothing ?

Currently the comments are read and "processed" by DucktapeJS, but that can be changed in future so they are ignored when loading the JS files and not even processed then compiling the Javascript byte code.

So imho I think it's fine to keep them, especially if they help other devs to understand the script.

chishm commented 1 month ago

Nice, so much work on thoses device, I realy like your JS scripts with all comments, but I have a doubt, how the JS core will handle them, it will ignore them, or use memory for nothing ? On my side I make shorter code possible, but less instructive/reusable than yours.

Thank you. I found the comments necessary for my own understanding. With all the magic constants used it was becoming hard to follow what I'd written a few days earlier. As @manup said, it'd be better if the comments could remain but be ignored during loading of the scripts.

    {
      "bind": "unicast",
      "src.ep": 1,
      "dst.ep": 1,
      "cl": "0x0000"
    },

You are sure this part is usefull ?

No, it's not necessary. I'd left it in after trying to debug a problem I had with retrieving the firmware version number (turned out I'd forgotten to include tuya_swversion.js in my DDF). I've removed it now, tested, and everything seems fine.

TODO: The battery level is not being reported for some reason.

Yeah, good question, I don't see the problem too, still nothing after a time ? if the binding don't work, you can have the poll. If you try to read the attribute in deconz it update the value in the API (take care you can test that only 1 time ^^)

BTW the bot that validate DDF need to take some holidays, embittered bot ....

Turns out the bot was being helpful after all. It'd put " eval" instead of "eval" for the parse script of config/battery, and this was causing my battery reporting problem. I've also removed the polling, as the device reports its battery level correctly.

Smanar commented 1 month ago

From the bot you miss

        {
          "name": "cap/color/ct/max"
        },
        {
          "name": "cap/color/ct/min"
        },

But here I don't understand the problem, on my side I will remove all this part.

Error: Unrecognized key(s) in object: 'buttons', 'buttonevents'

Zehir commented 1 month ago

Error: Unrecognized key(s) in object: 'buttons', 'buttonevents'

They should be on the subdevice no the root level of the DDF

https://github.com/dresden-elektronik/deconz-rest-plugin/pull/7904/files#diff-683a795de9d78e2e89d35aa871936863fc4cf25618c96426688c1c88c8659d52R24-R84

chishm commented 1 month ago

Thanks, @Smanar and @Zehir for your help with getting the DDF syntax right. I couldn't find devcap1.schema.json in the repo, so I've been using existing DDF files as examples, and obviously got a few things wrong there.

chishm commented 2 weeks ago

Hi @Smanar and @manup,

Is there anything else I need to do before this can be merged?

Smanar commented 1 week ago

For me there is no problem, and you have the device to make test, perhaps the c++ part need a special Milestone ?

manup commented 4 days ago

Good work :+1: I think the PR is fine now. One question since I don't understand every detail of it, is this a breaking change REST-API wise how the modified devices work?

chishm commented 2 days ago

@Smanar: That makes sense that the maintainers might be waiting for the next release to merge this.

@manup: It is a breaking change for two items: the state/angle value is now relative, and state/buttonevent has some more values reported for the different ways that the knob can be pressed and turned.