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

Create ts0601_valve_zgv1.py #1450

Closed
wants to merge 3 commits into from
Closed

Create ts0601_valve_zgv1.py #1450

wants to merge 3 commits into from

Conversation

zoic21
Copy link
Contributor

@zoic21 zoic21 commented Mar 31, 2022

No description provided.

@coveralls
Copy link

coveralls commented Mar 31, 2022

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%
Files with Coverage Reduction New Missed Lines %
zhaquirks/tuya/ts0211.py 1 84.21%
zhaquirks/xiaomi/aqara/roller_curtain_e1.py 20 59.7%
Totals Coverage Status
Change from base Build 2066732976: -0.3%
Covered Lines: 5399
Relevant Lines: 6748

💛 - Coveralls

Copy link
Collaborator

@javicalle javicalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code formats.
lambda x: x is not needed and can be ommited

zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
zhaquirks/tuya/ts0601_valve_zgv1.py Outdated Show resolved Hide resolved
@javicalle
Copy link
Collaborator

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:

Some devices with this implementation are:

Thanks in advanced.

@zoic21
Copy link
Contributor Author

zoic21 commented Apr 5, 2022

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.

@codecov-commenter
Copy link

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%) ⬆️
zhaquirks/xiaomi/aqara/roller_curtain_e1.py 59.70% <0.00%> (+1.88%) ⬆️

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.

@javicalle
Copy link
Collaborator

javicalle commented Apr 5, 2022

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.

@ScratMan
Copy link

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

@dmulcahey
Copy link
Collaborator

Superseded by #1560

@dmulcahey dmulcahey closed this Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants