search

zigpy / zha-device-handlers

ZHA device handlers bridge the functionality gap created when manufacturers deviate from the ZCL specification, handling deviations and exceptions by parsing custom messages to and from Zigbee devices.
Apache License 2.0
719 stars 664 forks source link

Improve Tuya `EnchantedDevice` implementation #2271

Open TheJulianJES opened 1 year ago

TheJulianJES commented 1 year ago

Description

The implementation for the "Tuya magic spell" is currently implemented in EnchantedDevice like this: https://github.com/zigpy/zha-device-handlers/blob/eed839c4f312a33da0087229a9b2f67185d5d557/zhaquirks/tuya/mcu/__init__.py#L666-L678 Starting to read attribute reads from __init__ isn't ideal, as this theoretically happens every time as ZHA reloads, but those attribute reads seemingly do not actually make it to the network. We also can't really "re-cast" the magic spell (from a reconfiguration for example).

It would be best if this implementation could be improved. One possibility might be to call it from a bind() function from some cluster (that's registered as a channel in ZHA, so the function actually gets called). This makes sure it's called "from a proper place" and also re-casts the device when doing a reconfiguration. Maybe something like a BasicTuyaSpell cluster that casts then spell would work? Edit: See discussion: https://github.com/zigpy/zha-device-handlers/pull/1427 Apparently bind() is not called on the Basic cluster (disabled here).

Some comments from https://github.com/zigpy/zha-device-handlers/issues/2227 include more information on this.

I'll try to look into this soon-ish.

TheJulianJES commented 1 year ago

Apparently we also have a duplicate version of this spell (with two different implementations) for a LIDL/Tuya plug: https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/lidl/ts011f_plug.py

TheJulianJES commented 1 year ago

@MattWestb I've read some discussions on the LIDL power strip that apparently requires the magic spell. It's mentioned that the device only exposes one endpoint if the spell is not cast. This seems to be true, as the signature provided in that quirk only has one endpoint: https://github.com/zigpy/zha-device-handlers/blob/eed839c4f312a33da0087229a9b2f67185d5d557/zhaquirks/lidl/ts011f_plug.py#L135-L141 However, it's replaced with a replacement from the Tuya class that has three endpoints: https://github.com/zigpy/zha-device-handlers/blob/eed839c4f312a33da0087229a9b2f67185d5d557/zhaquirks/tuya/ts011f_plug.py#L359-L394

So what happens if the magic spell isn't cast? ZHA will still create the entities. Do they just not work, or does Tuya do some stuff where all outlets are controlled, even if on/off is only sent to one endpoint?

TheJulianJES commented 1 year ago

So just to see what it could look like, I've made a quick implementation called EnchantDevice that "hijacks" the bind() method of the OnOff cluster (even if it's a CustomCluster already):

Is it bad? Yes. Does it work. Seems to.

It's interesting in case anyone wants to try it. Theoretically, it could be a drop-in replacement for EnchantedDevice, but yeah. This isn't great.

Code to dynamically inject an enchanted On Off cluster ```python class EnchantDevice(CustomDevice): """Device class for Tuya devices which need to have a spell cast to properly work.""" def __init__(self, *args, **kwargs): """Replace cluster with enchanted cluster and initialize.""" if on_off_cluster := self.find_on_off_cluster(self.replacement): # check if OnOff cluster has been replaced if isinstance(on_off_cluster[2], int): new_cluster = self.construct_enchanted_cluster(CustomCluster, OnOff) else: new_cluster = self.construct_enchanted_cluster(on_off_cluster[2]) self.replacement[ENDPOINTS][1][on_off_cluster[1]][ on_off_cluster[0] ] = new_cluster super().__init__(*args, **kwargs) @staticmethod def find_on_off_cluster(replacement): """Find OnOff cluster in cluster list.""" # get 1st endpoint from replacement first_endpoint = replacement.get(ENDPOINTS, {}).get(1) if first_endpoint: # search for OnOff cluster in input clusters cluster_list = first_endpoint.get(INPUT_CLUSTERS, []) possible_result = EnchantDevice.search_through_cluster_list( cluster_list, INPUT_CLUSTERS ) if possible_result: return possible_result # search for OnOff cluster in output clusters cluster_list = first_endpoint.get(OUTPUT_CLUSTERS, []) possible_result = EnchantDevice.search_through_cluster_list( cluster_list, OUTPUT_CLUSTERS ) return possible_result return None @staticmethod def search_through_cluster_list(cluster_list, cluster_type): for index, cluster in enumerate(cluster_list): if isinstance(cluster, int): if cluster == OnOff.cluster_id: return index, cluster_type, cluster elif cluster.cluster_id == OnOff.cluster_id: return index, cluster_type, cluster @staticmethod def construct_enchanted_cluster(*clusters): """Construct a cluster that casts the Tuya spell when binding.""" class EnchantedCluster(*clusters): """Enchanted cluster that casts the Tuya spell when binding.""" def __init__(self, *args, **kwargs): """Initialize EnchantedCluster.""" super().__init__(*args, **kwargs) self.__class__.__name__ = f"{clusters[0].__name__}_Enchanted" async def bind(self): """Bind cluster and start casting the spell.""" result = await super().bind() await self.spell() return result async def spell(self): """Cast spell, so the Tuya device works correctly.""" self.warning( "Casting spell on Tuya device %s", self.endpoint.device.ieee ) attr_to_read = [4, 0, 1, 5, 7, 0xFFFE] basic_cluster = self.endpoint.device.endpoints[1].in_clusters[0] await basic_cluster.read_attributes(attr_to_read) self.warning("Cast spell on Tuya device %s", self.endpoint.device.ieee) return EnchantedCluster ```
TheJulianJES commented 1 year ago

IMO, for now, we should do something like the following. All (currently enchanted) Tuya devices seem to use some variation of the OnOff cluster. We'd just need to make sure that all OnOff clusters in the Tuya base classes inherit from TuyaEnchantableOnOffCluster instead of CustomCluster, OnOff.

This would be:

This should work with Python MRO, while also allowing to just use the TuyaEnchantableOnOffCluster individually. (But this may need to be looked at again, not 100% sure)

Code for TuyaEnchantableOnOffCluster:

class TuyaEnchantableOnOffCluster(CustomCluster, OnOff):
    """Tuya On/Off cluster that casts a magic spell if TUYA_SPELL is set."""

    async def bind(self):
        """Bind cluster and start casting the spell."""
        result = await super().bind()
        if getattr(self.endpoint.device, "TUYA_SPELL", False):
            await self.spell()
        return result

    async def spell(self):
        """Cast spell, so the Tuya device works correctly."""
        self.debug("Casting spell on Tuya device %s", self.endpoint.device.ieee)
        attr_to_read = [4, 0, 1, 5, 7, 0xFFFE]
        basic_cluster = self.endpoint.device.endpoints[1].in_clusters[0]
        await basic_cluster.read_attributes(attr_to_read)
        self.debug("Cast spell on Tuya device %s", self.endpoint.device.ieee)

For a Tuya device to cast the spell, TUYA_SPELL = True just has to be added as a class attribute. Example:

class TuyaSmartRemote0041TOPlusB(CustomDevice, Tuya1ButtonTriggers):
    TUYA_SPELL = True

    signature = {
        MODEL: "TS0041",
    ...

This properly casts the spell on reconfiguring now (and prevents "starting" attribute reads when ZHA is still initializing).

MattWestb commented 1 year ago

The magic spell thing was done with one Hubart / Smart thing user with help of sniff form pairing TS004F with tuya ZBGW and was first implanted in Z2M and ZHA was not getting it then we was having the knowledge and i was getting it working by doing the reading in the device class that later have being moved for getting one nicer implementation.

Its 3 main types of the spell.

Casting the spell then joining is the best / only way and is one must for some devices. For my it shall being implanted in the joining process but the system must knowing its one tuya devices so we cant doing as with Xiaomi devices that is using the MAC address for doing it then tuya is using normal IEEE form the manufactures. tuya ZBGW is reading the manufacture and model ID as one of the first things (after have sending the network key) then joining devices and then casting the spell. Implanting it only on OnOff cluster is braking some very tricky devices like the Temp, Humid, Light sensors with and without LCD display that is also needing it or they is not reporting at all and is not syncing the time.

I have looking on many tusa ZBGW sniffing and after getting more TS004F devices i have doing it with my devises paring with my tuya ZBGW and im knowing how the devices is working with it and how ZHA is sending the spell to the device by sniffing then its very tricky getting it working with some devices and getting it working 100% (was doing new pairing with reset and battery out around 25 times with the DMS for verifying it is working OK).

Z2M is having one config / init section for devices that is used then devices is being joined and doing binding and reporting and they have putting the spell in that section (i can being wrong then im not one code warier).

The LIDL power strip was impalement then we was doing manual attribute reading in the quirks device class and it was working better for the device. Later the EnchantedDevice was implanted and i have moving the TS004F to it and its working OK. For my shall the complete LIDL being moved / integrated in tuya quirks then all is tuya devices and the only difference is that LIDL is using the A version of the devices = is Zigbee certified (not the updated power strip but the original normal working one) like the LIDL TS0501A and the no named is TS0501B.

For my is i importing not to braking the tricky devices and im 110% we shall doing the spell for all quirked tuya devices so they is behaving OK and if not doing it we can getting devices that is working one way in Z2M and then we is trying doing the same in ZHA with the same commands its not working then the device is configured in one different way and we is having problem getting it working and unhappy users that have lost functionality in there devices.

So my suggestion is casting on all tuya devices but doing it in the joining posses / handling and not on cluster level for getting it fast enough for working with tricky devise and not locking it for some device types that we later must redoing.

I have struggling with this problem for 1.5 year and we was getting it working OK and the only thing that have making my happy is that de(F)CONZ was making there PR for it the last week after struggling with there API V2 and old C- code and using our findings and testing (great thanks to the Bulgarian user that was the key for the magic spell !!).

Then im not one code worrier i stop posting in this team and some more then its not productive then i cant helping with the coding and only analyzing the devices is working and having multiple devices under test for days or weeks is taking to mush time for my at the moment.

MattWestb commented 1 year ago

2 devices that is adding extra EP if getting the spell is LIDL power strip and the TS004F DMS and without it they is not using the extra endpoints = the device is not working. And the TS004F DMS is not sending command from EP 2-4 if not being casted and the LIDL is only using EP 1 and is switching all outlets one it (hardware extended by spell).

And i think its some other devices that is doing the same but i dont remember if its was power plugs or wall switches.

I think the first devices that was hidden cluster / Ep was the first gen MCU devices like

Siterwell | GS361A | _TYST11_zivfvd7h | ivfvd7h -- | -- | -- | --

That have the tuya MCU cluster hidden but is working if putting it in the rel´placement list.

MattWestb commented 1 year ago

One more device that need it but is have new signature that need being added. https://github.com/zigpy/zha-device-handlers/issues/2272

github-actions[bot] commented 12 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.