Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Tuya EnchantedDevice implementation #2271

Open
TheJulianJES opened this issue Mar 13, 2023 · 9 comments
Open

Improve Tuya EnchantedDevice implementation #2271

TheJulianJES opened this issue Mar 13, 2023 · 9 comments
Labels
enhancement Improve an existing quirk Tuya Request/PR regarding a Tuya device

Comments

@TheJulianJES
Copy link
Collaborator

TheJulianJES commented Mar 13, 2023

Description

The implementation for the "Tuya magic spell" is currently implemented in EnchantedDevice like this:

class EnchantedDevice(CustomDevice):
"""Class for enchanted Tuya devices which needs to be unlocked by casting a 'spell'."""
def __init__(self, *args, **kwargs):
"""Initialize with task."""
super().__init__(*args, **kwargs)
self._init_device_task = asyncio.create_task(self.spell())
async def spell(self) -> None:
"""Initialize device so that all endpoints become available."""
attr_to_read = [4, 0, 1, 5, 7, 0xFFFE]
basic_cluster = self.endpoints[1].in_clusters[0]
await basic_cluster.read_attributes(attr_to_read)

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: #1427
Apparently bind() is not called on the Basic cluster (disabled here).

Some comments from #2227 include more information on this.

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

@TheJulianJES TheJulianJES added enhancement Improve an existing quirk Tuya Request/PR regarding a Tuya device labels Mar 13, 2023
@TheJulianJES
Copy link
Collaborator Author

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
Copy link
Collaborator Author

@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:

signature = {
MODEL: "TS011F",
ENDPOINTS: {
# <SimpleDescriptor endpoint=1 profile=260 device_type=266
# device_version=1
# input_clusters=[0, 3, 4, 5, 6]
# output_clusters=[10, 25]>

However, it's replaced with a replacement from the Tuya class that has three endpoints:
replacement = {
ENDPOINTS: {
1: {
PROFILE_ID: zha.PROFILE_ID,
DEVICE_TYPE: zha.DeviceType.ON_OFF_PLUG_IN_UNIT,
INPUT_CLUSTERS: [
Basic.cluster_id,
Identify.cluster_id,
Groups.cluster_id,
Scenes.cluster_id,
TuyaZBOnOffAttributeCluster,
],
OUTPUT_CLUSTERS: [Time.cluster_id, Ota.cluster_id],
},
2: {
PROFILE_ID: zha.PROFILE_ID,
DEVICE_TYPE: zha.DeviceType.ON_OFF_PLUG_IN_UNIT,
INPUT_CLUSTERS: [
Identify.cluster_id,
Groups.cluster_id,
Scenes.cluster_id,
TuyaZBOnOffAttributeCluster,
],
OUTPUT_CLUSTERS: [],
},
3: {
PROFILE_ID: zha.PROFILE_ID,
DEVICE_TYPE: zha.DeviceType.ON_OFF_PLUG_IN_UNIT,
INPUT_CLUSTERS: [
Identify.cluster_id,
Groups.cluster_id,
Scenes.cluster_id,
TuyaZBOnOffAttributeCluster,
],
OUTPUT_CLUSTERS: [],
},

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
Copy link
Collaborator Author

TheJulianJES commented Mar 13, 2023

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
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
Copy link
Collaborator Author

TheJulianJES commented Mar 13, 2023

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:

  • class TuyaOnOff(TuyaEnchantableOnOffCluster):
  • class TuyaZBOnOffAttributeCluster(TuyaEnchantableOnOffCluster):
  • class TuyaSmartRemoteOnOffCluster(TuyaEnchantableOnOffCluster, EventableCluster):
  • class TuyaOnOff(TuyaEnchantableOnOffCluster, TuyaLocalCluster): (for mcu/__init__.py)

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
Copy link
Contributor

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.

  • 1 TS004F need it then joining and is in INIT mode (after getting the network key and before its have updating the TC-Link key / sending permit joining 180) and if working the device is adding to endpoint 2, 3 and 4 (adding hardware function). And its keeping the spell until being retested and the battery is going out (only retest or battery is keeping the spell).
    class TuyaSmartRemote004FDMS(EnchantedDevice, CustomDevice):
  • 2 LIDL power strip with new firmware. Its only having one EP but you can casting the spell every time and the device is start using the 3 extra EPs. That is good but it is not having any battery so it can losing the spell (can being implanted in reconfigure for fixing it),
  • 3 Other devices that is not making "hardware changes" like changing commands its using but is not adding cluster and end points.

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
Copy link
Contributor

MattWestb commented Mar 13, 2023

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
Copy link
Contributor

One more device that need it but is have new signature that need being added.
#2272

@github-actions
Copy link

github-actions bot commented Sep 9, 2023

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.

@github-actions github-actions bot added the stale Issue is inactivate and might get closed soon label Sep 9, 2023
@TheJulianJES TheJulianJES added no stale Bot won't close issue/PR even if inactive and removed stale Issue is inactivate and might get closed soon labels Sep 12, 2023
@TheJulianJES
Copy link
Collaborator Author

This should be addressed with #3414. There have been some reports of Tuya devices not working correctly, but according to those logs, the spell was always executed.
Still, we should confirm that there's not been some regression with that PR. I'll leave this issue open for now.

@TheJulianJES TheJulianJES removed the no stale Bot won't close issue/PR even if inactive label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve an existing quirk Tuya Request/PR regarding a Tuya device
Projects
None yet
Development

No branches or pull requests

2 participants