zha-device-handlers icon indicating copy to clipboard operation
zha-device-handlers copied to clipboard

Create ts0601_valve_zgv1.py

Open zoic21 opened this issue 3 years ago • 6 comments

zoic21 avatar Mar 31 '22 12:03 zoic21

Pull Request Test Coverage Report for Build 2096257919

  • 29 of 61 (47.54%) changed or added relevant lines in 1 file are covered.
  • 21 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.3%) to 80.009%

Changes Missing Coverage Covered Lines Changed/Added Lines %
zhaquirks/tuya/ts0601_valve_zgv1.py 29 61 47.54%
<!-- Total: 29 61
Files with Coverage Reduction New Missed Lines %
zhaquirks/tuya/ts0211.py 1 84.21%
zhaquirks/xiaomi/aqara/roller_curtain_e1.py 20 59.7%
<!-- Total: 21
Totals Coverage Status
Change from base Build 2066732976: -0.3%
Covered Lines: 5399
Relevant Lines: 6748

💛 - Coveralls

coveralls avatar Mar 31 '22 13:03 coveralls

Hi, I intend to migrate the TuyaNewManufCluster implementation to a new implementation and this case would be a good candidate for testing on a TRV device.

@zoic21 would you be willing to try with another implementation and do some testing? If yes, I'll try to have a new ASAP implementation so I can validate it. If so, I'll try to make the changes over your implementation ASAP so you can validate it.

The classes implemented with the 'new' approach are located here:

  • https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/tuya/mcu/init.py

Some devices with this implementation are:

  • https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/tuya/ts0601_switch.py
  • https://github.com/zigpy/zha-device-handlers/blob/dev/zhaquirks/tuya/ts0601_dimmer.py

Thanks in advanced.

javicalle avatar Mar 31 '22 18:03 javicalle

Hello,

Yes I can try the new implementation. I have also another tuya device to do (_TZE200_nnrfa68v.TS0601 it's temperature and humidity device). So I can also try new implementation on the next device.

zoic21 avatar Apr 05 '22 12:04 zoic21

Codecov Report

Merging #1450 (867ceb9) into dev (70fbb4a) will decrease coverage by 0.28%. The diff coverage is 47.54%.

@@            Coverage Diff             @@
##              dev    #1450      +/-   ##
==========================================
- Coverage   80.29%   80.00%   -0.29%     
==========================================
  Files         227      228       +1     
  Lines        6683     6748      +65     
==========================================
+ Hits         5366     5399      +33     
- Misses       1317     1349      +32     
Impacted Files Coverage Δ
zhaquirks/tuya/ts0601_valve_zgv1.py 47.54% <47.54%> (ø)
zhaquirks/tuya/ts0211.py 84.21% <0.00%> (+0.87%) :arrow_up:
zhaquirks/xiaomi/aqara/roller_curtain_e1.py 59.70% <0.00%> (+1.88%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 70fbb4a...867ceb9. Read the comment docs.

codecov-commenter avatar Apr 05 '22 12:04 codecov-commenter

Hi @zoic21, I have a proposal, but there are a few changes I want to highlight:

  • I have renamed some classes to give them a more generic character (they will not be specific to ZGV1). Those classes are candidates to be brought into tuya.mcu.__init__.py class
  • the ZGV1Timer class loses control over read-only attributes (the Unauthorize write attribute... part). I can review this, but it's still normal behavior on other zigbee attributes: those that are read-only won't do anything (or so I hope that will happen here)
  • I have not analyzed in detail the writing of the attributes 0x000B and 0x000C part. At this point I am optimistic and hope that if there are any problems it can be handled with the converters
  • I have replaced the ZGV1OnOff class with the TuyaOnOff class. I have doubts about it because my interpretation is that the value is not manipulated when reading but to execute the command it seems that the value is inverted, is that right? I have tried to implement it with dp_converter=lambda x: (not x) but this part is quite experimental
ts0601_valve_zbv1.py
"""Tuya ZGV1."""

from typing import Dict

from zigpy.profiles import zha
from zigpy.quirks import CustomDevice
import zigpy.types as t
from zigpy.zcl.clusters.general import (
    Basic,
    Groups,
    Ota,
    PowerConfiguration,
    Scenes,
    Time,
)
from zigpy.zcl.clusters.measurement import FlowMeasurement

from zhaquirks.const import (
    DEVICE_TYPE,
    ENDPOINTS,
    INPUT_CLUSTERS,
    MODELS_INFO,
    OUTPUT_CLUSTERS,
    PROFILE_ID,
)
from zhaquirks.tuya import TuyaLocalCluster
from zhaquirks.tuya.mcu import (
    DPToAttributeMapping,
    TuyaAttributesCluster,
    TuyaDPType,
    TuyaMCUCluster,
    TuyaOnOff,
)


class TuyaMCUFlowMeasurement(FlowMeasurement, TuyaLocalCluster):
    """Tuya Water consumed cluster."""


class TuyaMCUPowerConfiguration(PowerConfiguration, TuyaLocalCluster):
    """Tuya PowerConfiguration."""


class ZGV1Timer(TuyaAttributesCluster):
    """Tuya Timer cluster."""

    cluster_id = 0x043E
    name = "Timer"
    ep_attribute = "timer"

    attributes = {
        0x000C: ("state", t.uint16_t),
        0x000B: ("time_left", t.uint16_t),
        0x000F: ("last_valve_open_duration", t.uint16_t),
    }

    server_commands = {}
    client_commands = {}


class TuyaZGV1ManufCluster(TuyaMCUCluster):
    """Tuya with ZGV1."""

    dp_to_attribute: Dict[int, DPToAttributeMapping] = {
        1: DPToAttributeMapping(
            TuyaOnOff.ep_attribute,
            "on_off",
            dp_type=TuyaDPType.BOOL,
            dp_converter=lambda x: (not x),
        ),
        5: DPToAttributeMapping(
            TuyaMCUFlowMeasurement.ep_attribute,
            "measured_value",
            dp_type=TuyaDPType.VALUE,
            converter=lambda x: x / 33.8140226,
        ),
        7: DPToAttributeMapping(
            TuyaMCUPowerConfiguration.ep_attribute,
            "battery_percentage_remaining",
            dp_type=TuyaDPType.VALUE,
        ),
        11: DPToAttributeMapping(
            ZGV1Timer.ep_attribute,
            "time_left",
            dp_type=TuyaDPType.VALUE,
            converter=lambda x: x / 60,
        ),
        12: DPToAttributeMapping(
            ZGV1Timer.ep_attribute,
            "state",
            dp_type=TuyaDPType.ENUM,
        ),
        15: DPToAttributeMapping(
            ZGV1Timer.ep_attribute,
            "last_valve_open_duration",
            dp_type=TuyaDPType.VALUE,
            converter=lambda x: x / 60,
        ),
    }

    data_point_handlers = {
        1: "_dp_2_attr_update",
        5: "_dp_2_attr_update",
        7: "_dp_2_attr_update",
        11: "_dp_2_attr_update",
        12: "_dp_2_attr_update",
        15: "_dp_2_attr_update",
    }


class TuyaZGV1(CustomDevice):
    """Tuya Air quality device."""

    signature = {
        # NodeDescriptor(logical_type=<LogicalType.Router: 1>, complex_descriptor_available=0, user_descriptor_available=0, reserved=0, aps_flags=0, frequency_band=<FrequencyBand.Freq2400MHz: 8>, mac_capability_flags=<MACCapabilityFlags.AllocateAddress|RxOnWhenIdle|MainsPowered|FullFunctionDevice: 142>, manufacturer_code=4098, maximum_buffer_size=82, maximum_incoming_transfer_size=82, server_mask=11264, maximum_outgoing_transfer_size=82, descriptor_capability_field=<DescriptorCapability.0: 0>, *allocate_address=True, *is_alternate_pan_coordinator=False, *is_coordinator=False, *is_end_device=False, *is_full_function_device=True, *is_mains_powered=True, *is_receiver_on_when_idle=True, *is_router=True, *is_security_capable=False)]
        # device_version=1
        # SizePrefixedSimpleDescriptor(endpoint=1, profile=260, device_type=81, device_version=1,
        # input_clusters=[0, 4, 5, 61184],
        # output_clusters=[25, 10])
        MODELS_INFO: [("_TZE200_akjefhj5", "TS0601")],
        ENDPOINTS: {
            1: {
                PROFILE_ID: zha.PROFILE_ID,
                DEVICE_TYPE: zha.DeviceType.SMART_PLUG,
                INPUT_CLUSTERS: [
                    Basic.cluster_id,
                    Groups.cluster_id,
                    Scenes.cluster_id,
                    TuyaZGV1ManufCluster.cluster_id,
                ],
                OUTPUT_CLUSTERS: [Time.cluster_id, Ota.cluster_id],
            }
        },
    }

    replacement = {
        ENDPOINTS: {
            1: {
                DEVICE_TYPE: zha.DeviceType.ON_OFF_LIGHT,
                INPUT_CLUSTERS: [
                    Basic.cluster_id,
                    Groups.cluster_id,
                    Scenes.cluster_id,
                    TuyaZGV1ManufCluster,
                    TuyaMCUFlowMeasurement,
                    TuyaOnOff,
                    TuyaMCUPowerConfiguration,
                    ZGV1Timer,
                ],
                OUTPUT_CLUSTERS: [Time.cluster_id, Ota.cluster_id],
            }
        }
    }

Of course take my proposal as a suggestion and accept only what you feel comfortable with. I remain at your disposal for any questions or clarification you need in this regard. I'll be happy to help you in any way I can.

Regards.

javicalle avatar Apr 05 '22 19:04 javicalle

This is interesting, I found this pull request while trying to manage my SASWELL SAS980SWT-7-Z01 garden watering valve with ZHA (see #1660 ). My valve has exactly the same hardware appearance, and shares the same model and vendor IDs but the signature looks different :

{
  "node_descriptor": "NodeDescriptor(logical_type=<LogicalType.EndDevice: 2>, complex_descriptor_available=0, user_descriptor_available=0, reserved=0, aps_flags=0, frequency_band=<FrequencyBand.Freq2400MHz: 8>, mac_capability_flags=<MACCapabilityFlags.AllocateAddress: 128>, manufacturer_code=4098, maximum_buffer_size=82, maximum_incoming_transfer_size=82, server_mask=11264, maximum_outgoing_transfer_size=82, descriptor_capability_field=<DescriptorCapability.NONE: 0>, *allocate_address=True, *is_alternate_pan_coordinator=False, *is_coordinator=False, *is_end_device=True, *is_full_function_device=False, *is_mains_powered=False, *is_receiver_on_when_idle=False, *is_router=False, *is_security_capable=False)",
  "endpoints": {
    "1": {
      "profile_id": 260,
      "device_type": "0x0051",
      "in_clusters": [
        "0x0000",
        "0x0001",
        "0x0004",
        "0x0005",
        "0x0006",
        "0x0702",
        "0xef00"
      ],
      "out_clusters": [
        "0x000a",
        "0x0019"
      ]
    }
  },
  "manufacturer": "_TZE200_akjefhj5",
  "model": "TS0601",
  "class": "ts0601_TZE200_akjefhj5_valve.TuyaValve"
}

So the clusters inside are the following : "0x0000": Basic "0x0001": PowerConfiguration "0x0004": Groups "0x0005": Scenes "0x0006": OnOff "0x0702": Smart Energy - Metering "0xef00": TuyaManufCluster

I don't have the 0x043E cluster, and when I try to use your quirk with my valve, it doesn't work. I used another quirk made for another vendor ID which looks closer to what I see in my device's signature, and I can get the on/off working with write and read of the timer and (strange) volume delivered value, but battery level is not working (I need some help to understand how to get battery level from the device reporting and expose it in HA).

Why is there two models sharing the same product vendor and model ? Won't it lead to issues when one of them is added to a ZHA release ? if I (manage to) build my own quirk for my valve, would ZHA use it instead of this quirk if it's merged later on ?

Thanks

ScratMan avatar Jul 27 '22 10:07 ScratMan

Superseded by #1560

dmulcahey avatar Aug 30 '22 11:08 dmulcahey