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

Add Tuya TS011F plug variant, add Tuya 0x1888 cluster class #2592

Merged
merged 5 commits into from
Sep 24, 2023

Conversation

TheJulianJES
Copy link
Collaborator

@TheJulianJES TheJulianJES commented Sep 17, 2023

Proposed change

Adds yet another variant of the Tuya TS011F plug.
Not sure if TuyaZBMeteringClusterWithUnit is required or if TuyaZBMeteringCluster is enough. (Couldn't we even merge those two classes?)

It also adds a class for the Tuya 0x1888 cluster (which is outside the manufacturer specific range, thus tests fail unless it has a custom cluster class).

Additional information

Fixes #2573
Waiting for confirmation in that issue that the custom quirk works

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

@TheJulianJES TheJulianJES added the Tuya Request/PR regarding a Tuya device label Sep 17, 2023
@TheJulianJES TheJulianJES changed the title Add Tuya TS011F plug variant Add Tuya TS011F plug variant, add Tuya 0x1888 cluster class Sep 17, 2023
@TheJulianJES TheJulianJES marked this pull request as ready for review September 17, 2023 04:48
@TheJulianJES
Copy link
Collaborator Author

TheJulianJES commented Sep 17, 2023

Python 3.12 test failure is expected for now, as zigpy hasn't pushed a version compatible with Python 3.12 rc2.
All other tests pass / code test coverage is basically unchanged.

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (7745c83) 86.54% compared to head (a061fa6) 86.55%.
Report is 2 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2592      +/-   ##
==========================================
+ Coverage   86.54%   86.55%   +0.01%     
==========================================
  Files         276      276              
  Lines        8502     8511       +9     
==========================================
+ Hits         7358     7367       +9     
  Misses       1144     1144              
Files Changed Coverage Δ
zhaquirks/tuya/__init__.py 76.48% <100.00%> (+0.19%) ⬆️
zhaquirks/tuya/ts011f_plug.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


name = "Tuya Manufacturer Specific 1"
cluster_id = TUYA_CLUSTER_1888_ID
ep_attribute = "tuya_manufacturer_specific_1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not happy with that name, but don't have any nice suggestion. Maybe ep_attribute = "tuya_manufacturer_specific_1888"? or "tuya_manufacturer_specific_6280"?

Copy link
Collaborator Author

@TheJulianJES TheJulianJES Sep 24, 2023

Choose a reason for hiding this comment

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

Changed it to append the decimal representation of the cluster ID now: a061fa6
.. or would hex be better? (I wonder if we already do something similar anything else?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't recall any similar 'labeling'.
Any approach would be fine to me, maybe the hex would direct match to the device signature, isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll leave it in decimal for now. For hex, it might be confusing if we don't have 0x in front(?)
Doesn't really matter and we can always change it later, should we ever need to use the ep_attribute of those two clusters.

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.

LGTM
Just an aesthetic comment without nice adjustment 🤷🏻‍♂️

cluster_id = TUYA_CLUSTER_E000_ID
ep_attribute = "tuya_is_pita_0"
Copy link
Contributor

Choose a reason for hiding this comment

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

This naming is made of @Adminiuga but i can being good getting somthing better as the old PITA ;-)

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.

LGTM

@TheJulianJES TheJulianJES merged commit bbee055 into zigpy:dev Sep 24, 2023
14 checks passed
@Virusmater
Copy link

Hello, I see that _TZ3000_0zfrhq4i variant was added with this commit.
I am using the current version of Home Assistant 2023.9.3

The plug works, but showing big amounts of energy in Summation Delivered. Will it be fixed by this commit and I just need to wait for HA update?

elupus pushed a commit to elupus/zha-device-handlers that referenced this pull request Jan 17, 2024
…2592)

* Add Tuya TS011F plug variant

* Use correct cluster: `TuyaZBE000Cluster`

* Add `TuyaZB1888Cluster` cluster class due to it being outside manufacturer range

* Use `TuyaZB1888Cluster` for new Tuya plug quirk

* Change `ep_attribute` of Tuya clusters
lgraf pushed a commit to lgraf/zha-device-handlers that referenced this pull request May 6, 2024
…2592)

* Add Tuya TS011F plug variant

* Use correct cluster: `TuyaZBE000Cluster`

* Add `TuyaZB1888Cluster` cluster class due to it being outside manufacturer range

* Use `TuyaZB1888Cluster` for new Tuya plug quirk

* Change `ep_attribute` of Tuya clusters
@deezid
Copy link

deezid commented Aug 16, 2024

I still have the same problem. The 20A version of the plug (TZ3000_0zfrhq4i) reports the correct summation values while the 16A version (TZ3000_cehuw1lw) reports values that are like 50x higher.

HA 2024.8.1

@juup
Copy link

juup commented Sep 30, 2024

I also have problems with TZ3000_cehuw1lw. The summation is far from realistic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready PR should be ready to merge Tuya Request/PR regarding a Tuya device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Device Support Request] TS011F Smart Plug (_TZ3000_cehuw1lw)
6 participants