Use custom clusters for casting Tuya spell #2287
Conversation
Pull Request Test Coverage Report for Build 4515141014
💛 - Coveralls |
|
Only one side not for implement in the right way it must being sent is between the device have get the network key and before its requesting / getting the updated TC-Link key and the last millisecond is then the device is doing device accouterments after have confirming the valid of the TC-Link key but i think then its too later for device that need it then being in init mode.. |
|
At the moment, there's no way to do this properly and that early in the pairing process. What happens if the spell is cast later? Does the device still "unlock" with the only difference being that the signature might look differently? |
|
It is possible doing with EZSP then tuya is doing it with standard firmware in there ZBGW but that is out of this question then its only Bellows and we must doing it with all radios. With the TS004F is only sending commands from EP1 and nothing from EP 2-4 if being too late and the device have not getting one spell after being retested Many other devices is not need it in INIT mode (only devices that doing "hardware changes" like adding EPs and so on must having it in the INIT mode) and can being done every time but then sniffing tuya ZBGW they is configuring the device network parameters and then sending one leave with rejoin flag sett and the device is rejoining and is being in INIT mode and the ZBGW is first casting the spell and then doing normal other configuration of it that was not done in the first phase. |
So this PR would cast the spell again when reconfiguring the device through the UI. Without re-pairing, would reconfiguring (so casting the spell after it's in the network) make that device magically work? (so sending commands on the other endpoints) |
|
If its casting the spell then doing reconfigure its helping may devices that dont need it in INIT mode and user dont need deleting the device and joining it new the device for getting it working. I can testing it but after the IKEA changes that is waiting or one to 2 week now than i cant having 10 different devices is test mode in 3 different test systems (i have getting my windows 10 WSL not working after some Then i need sniffing the device then joining and resetting it and removing the battery and verifying its that it have loosing the spell and then applying the patch and see its applying it and working well and redoing the same at least 10 times for knowing its working well as it doing today so users is not getting problems in the future and we is hunting problems we have implanting in the code and making some devices not working OK all the time and getting many strange issues from our users. |
|
I was putting the INITs in the HA container and patching the device class to not use the Did i making somthing wrong ?? Used files the device quirk in cunstom quirk and the INITs in the HA container. |
|
From the PR description:
You can use any Tuya When applying all changes from this PR, this is the new class EnchantedDevice(CustomDevice):
TUYA_SPELL = TrueSo it just sets the So in order for the spell to apply, ideally you just inherit from that new class SomeTuyaDevice(CustomDevice):
"""Tuya device."""
TUYA_SPELL = True
signature = {...But ideally, the device quirks all stay the same. All quirks which use a custom Tuya OnOff cluster (all of them should extend |
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## dev #2287 +/- ##
==========================================
+ Coverage 83.91% 83.98% +0.06%
==========================================
Files 260 261 +1
Lines 8214 8242 +28
==========================================
+ Hits 6893 6922 +29
+ Misses 1321 1320 -1
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Hmm, I'll need to have a look at how to deal with devices that use multiple enchantable clusters (so on multiple endpoints). It would all be easier if there's something like a "pre-bind" function that always gets called (even on the |
Hold your horses @TheJulianJES !!I have getting it working and its very good only then adding the For the moment i testing adding it and its getting spell and then resetting the device and taking out the battery before its rejoining for loosing the spell and then resetting it and letting it rejoining without getting spell and later doing reconfigure. Shall i putting the last commitment or is it OK running the test with the 1d36fb4 ? |
|
The latest commit will only "cast the spell" before actually binding the |
|
I have looking and all the times i have getting it working is the spell casted before the device is requesting updating the TC-Link key and it shall being in the safe time window but i testing more for see i can repeating it at last 10 time with successes or i must looking if need more work. |
Yeah. But zha-quirks already replaces the 1 endpoint signature with the 4 endpoint signature, right? It looks like all quirks have an enchantable OnOff cluster on endpoint 1, so that's good. |
|
Its have only one EP 1 without the spell and the quirk is adding 3 new one but the sepll is only being casted one time on EP 1 (all is sniffed with wireshark) so no problems. Shall looking one more time if reconfigure is casting on all or only EP 1. |
|
Reconfigure is only doing EP1 and its very good and no binding of EP 2-4 = OK so not getting strange problems with the device (tuya is not doing any bindings at all on this devices). And it was not casted and the EP 2-4 was not working and after reconfigure its start using them (the last OnOff command is send from EP 3) :-)))))) |
TheJulianJES
force-pushed
the
tjj/tuya_spell_in_clusters_clean
branch
from
6778fae to
abc434e
Compare
|
I is filtering only showing commands sent from the device and its looks like this then joining it: 554 is reading manufacture and model so quirk can being matched and loaded. |
|
Rebased to fix a conflict. I've also added a test to make sure that all quirks which use |
|
I can double check later, but does ZHA also call Edit: Does both, but there's another issue if a cluster changes the |
|
I think the check on EP 1 is very OK then tuya is always making EP 1 with basic and identify and so on (but they can changing) like many other is not doing, Im not 100% sure that some tuya devices is using in cluster then some plugs (like LIDL power strip) can have only in clusters but normally controllers shall having out cluster (but we is doing with tuya devices here). Can you putting Then you have getting all tests working OK give my one sign and i installing the lasts commit and redoing most of the testing but its taking time doing it well for being sure and i think we and @javicalle dont like braking devices for our users so i like being sure its working OK. |
The As long as the If a device does not have I'm still working on some stuff and doing some more testing. I'll need to have a look at when ZHA exactly calls |
|
Uhh, I see why ... did we ever confirm if the remotes actually need the spell? If yes, we have an issue lol ... uh, the EDIT: Nvm, they have a PowerConfiguration cluster. Do we also want to prevent binding for the |
|
You have without spell in #2287 (comment) and was only sending command OnOff from EP1 and after the spell its was start using the new EPs. And i using the |
|
Also not all devices that need the spell is having power config cluster like the LIDL power stripe and other plugs. |
Yeah, I was specifically talking about the remotes which use zha-device-handlers/zhaquirks/tuya/__init__.py Lines 934 to 948 in 8e91d98 For those remotes, we could make the
I'd recommend to use |
|
|
This is little role the dice hwo the system / device is doing the things. Then doing reconfigure its not sending power config to the device but failing after timeout most of the time but also can doing it without errors. I have only updating tuya INIT and the MCU INIT in the container and is having this quirk in the local folder: tu_ts004f.py.txt. My feeling is that all the binding that is not needed that is making the problems with the timing but the good thing is looks like its fixing one device that is not having getting the spell also with reconfigure that was not possible code used before (it can being that redoing the bindings and the device is doing some configure and being in INIT mode but i dont knowing). Im spitted if its 100% and can going in production then its not doing the same thing all the times and i dont like that. I have not getting the continues MAC pull from the device but i looking if i see it then i doing more testing and sniffing it. This device is having the most extreme "personality" i have see !!! Do you need more information or test being done ? |
To confirm, that is the quirk that your device matches, correct?
Do you still get "Executing Tuya spell" and "Executed Tuya spell" in the logs? (Those log entries are printed by the "new Tuya spell". In between those lines, you should see if reading the attributes was successful) You can also try to use the |
|
I only doing testing with TS004FDMS then its the most problematic one and later i can doing the ROK (Smart Rotating Knob). If you looking in the attached quirk i is using the I shall doing more testing later but i only looking in the online sniffs for see have the commands is getting to and from the device with the timing so i can looking in the log then. |
|
Just a side comment, if the timing is critical while casting the Tuya spell, enabling the debug logs add a significant overhead (writing to file is expensive in time) and maybe can impact in the results. |
|
Its one small NanoPi Neo running armbian32 RCP without OTBR and only Zigbeed and 1 IKEA VINDSTYRKA that is sitting in karantene for not getting OTA updates so its very small system but if its working OK with some debug logging its very likely not getting problem then taking it away and also in larger system but cant knowing for 100% until tested. |
|
Do you think we can merge this for the next HA release/beta? It won't break any currently working devices and should be lots better than the current implementation IMO. |
|
I have using this for the DMS: You must say if i have changing my quirk OK or if its somthing wrong !! I think the timing looks good as long the system is not slowed down but then its one external problem with SD-Card or so. Also if the spell is being casted then doing reconfigure 90% of all device is working with it and can being easy fixed for our users. For devices that have getting the spell its shall not being one braking thing then they is keeping it and normally only need it one time then its joining the system. First taking on look on my quirk if i have doing all right and then i doing the same tests with ROK (RotatingKnob) so we is knowing it working well. Your last wish moving LIDL to tuya i think shall waiting after 2023.4 release so not getting all problem at the same time and we need one with the power stripe that can testing it and i have 2 but with the old good working firmware that not need the spell for working OK. |
|
I was copy your last tuya INIT and mcu-INIT to the container and updated the local quirk but i was not liking the power config binding so i was putting in the Then testing the ROK and its looks working very well but is not so tricky as the DMS. If joining one device new its casting the spell OK as long ZHA is doing its parts OK. I think we cant getting it better and i think updating the INITs and the TS004F quirk is OK and its not braking any devices that is working for our users. |
|
I haven't reviewed all the implementation in detail (and I'm very sorry for that), but I have an idea in my head that I want to expose. Maybe is a dumb idea. I was guessing, it would posible to cast the spell from the What do you think? I'm sorry to come here in the last minute when all the discuccion is already setted. I'm impressed for all the work made here I don't want to generate noise now that it seems already done. |
|
Yeah, not a bad idea. I've also thought of that at one point. When I looked at it, I realized that not all (enchanted) devices even expose a Tuya manufacturer cluster (we could "fake one", but I don't know if that's the best idea). I'm not completely sure on this, but I also think that ZHA binds the cluster from id 0 onwards, so the Tuya manufacturer cluster would probably get bound at the end which might "dramatically" delay the spell. This implementation definitely isn't "a final solution", but I think it's the best(?) we can do for now (until ZHA/zigpy/quirks is changed). |
|
Hola JC !! And tuya is using normal zigbee chips with normal MAC so we cant doing like Xiami and fixing MAC range for triggering it then we is casting it on over 50% of all devices then Silicon labs is IKEA and new HUE device and many more. |
|
Now I can see a little more of the full picture. Thanks to both of you! |
|
You is having most experience with strange tuya device from the code side and is knowing how to tricking them. Its one mess and we must living with it then tuya have not implanting all things at all of was forgetting reading the book so in very glad i have most IKEA devices but they is not enough for functionality for all my needs. |
|
This isn't really related to this PR, but for the
Only the And Z2M binds the power configuration cluster for all of these remotes. But you're saying we don't need to do that (for all three devices)? |
|
Any issues with the This one should be ready now. |
|
I have sniffing tuya ZBGW setting it up and they is doing no binding or reporting only normal Zigbee 3 joining then kicking the device then its in the network with leave and rejoin flag set and is casting the spell then its coming back in the network and then is doing nearly nothing. And our experience of the rest TS004F is not doing strange things so you not getting problems like battery draining or device leaving the network. The We have 2 more device that is the same type but is not getting in to PR the single knob and the LIDL that i have not getting (they dont had it in AT so i was not getting any one for testing). plus one more LIDL 4 button remote but its one other TS type but is using the same commands. |
TheJulianJES
added
the
needs final review
|
Was looking little before moving both device back to the large RCP test system and the DMS is updating the battery (was 100% after last paring) it have being 100% for over one year and not is going down little an i think its the normal after having doing so mush testing with it. |
|
Are we good to get this in for now? Should be somewhat "well-tested" |
|
My devices is working OK and have moving them to its normal system now. |
|
Sorry, no exotic devices around here 😅 |
|
´Thanks J & J for the work done !! |
|
Thanks for all the testing too! |
|
Dear all, I have several Tuya based remotes, where I have indeed noticed a battery drain and the habit of reporting a 100% level (right up until the end ... when the battery dies). The 42 and 43 share the ill-behaviour as reported in this thread:
So ... knowing of the done/ongoing work ... can I help in "testing"/"debugging" solutions? Still have a pile of the needed batteries, so not in a big hurry ;-) Cheers, |
|
You can update to Home Assistant Core 2023.4.0 (beta releases today) and then remove and re-pair all battery draining remotes. |
This addresses a part of #2271
This is not a final solution forever though. This should be improved when significant parts of ZHA and zha-quirks change.
IMO, the current "solution" is far worse, as it creates a task to read attributes in a class constructor. So the only time this happens when it's useful is when the device is first added and the quirk is initialized.
Re-pairing or re-configuring do not cast the spell again (as the quirk class is already loaded). (This often causes problems)
Reading attributes during restarting (when the device was added previously) is also broken with the current implementation.
Some tests are also left in a bad state with the current implementation.
This new implementation currently requires all Tuya devices that need the spell to implement a subclass of
TuyaEnchantableClusterand set theTUYA_SPELLattribute toTrueor inheritEnchantedDevice(which just setsTUYA_SPELLtoTrue). This is somewhat to provide backwards compatibility with custom quirks (although those quirks need to implement a subclass ofTuyaEnchantableClusterwhich should be given for most "enchantable" Tuya devices though).This all needs to be checked and tested still, but from what I can see, all devices that are currently an
EnchantedDevicealready implement a subclass ofTuyaEnchantableCluster. So the spell will work properly for them now.(only one remote needed to be changed in this PR)
Test code to "dynamically inject" into clusters that are bound (although the code only does it for
OnOff) is provided in this comment when expanding: #2271 (comment)I wouldn't go with that approach though, as it's way too much of a hack. I think the solution implemented here is the best we can do at the moment. A proper solution will require multiple changes in ZHA, zigpy, and possibly zha-quirks from what I can see.
IMO, I would go with this for now to fix the current implementation.
Another PR should also move the LIDL quirk into Tuya classes and make it use
EnchantedDevice.