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 yet another TS0043 variant #2074

Merged
merged 1 commit into from
Jan 10, 2023
Merged

Conversation

fingolfin
Copy link
Contributor

This one reports four on/off switches , just like the TuyaSmartRemote0042TOPlusA does, and also in general the profile looks just like that (except with one more physical button). Thus I named this quirk TuyaSmartRemote0043TOPlusA.

However, I did deviate in the modelling of the replacement endpoints, and instead of copying them from the TS0042 variant, I decided to model them more closely on the other TS0043 variants. (I have no clue if that's a good or bad idea, advice is highly welcome, I am new to this).

I got it from AliExpress.

Tuya-ZigBee-Wireless-Wand-Schalter-1-2-3Gang-Wifi-Automatisierung-Push-Taste-Batterie-Powered-Zigbee-Gateway jpg_640x640

The profile looks like what was reported on issue #1699 so perhaps this PR fixes that, too, although there we have "manufacturer": "_TZ3000_uyjmm0et", while for mine the id is _TZ3000_w3c7ouru.

Likewise, issue #1683 with _TZ3000_kkahwiyu probably is resolved by this.

Finally, I think issue #1557 is related, and a similar quirk ought to be added for TS0041, but I don't have such a device to test.

@fingolfin
Copy link
Contributor Author

Oh and also #1990 (or at least the second part of it, it seems to talk about two devices?))

@coveralls
Copy link

coveralls commented Jan 7, 2023

Pull Request Test Coverage Report for Build 3881673625

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 83.533%

Totals Coverage Status
Change from base Build 3873395484: 0.008%
Covered Lines: 6757
Relevant Lines: 8089

💛 - Coveralls

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2023

Codecov Report

Base: 83.48% // Head: 83.53% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (d7836c0) compared to base (2296895).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2074      +/-   ##
==========================================
+ Coverage   83.48%   83.53%   +0.04%     
==========================================
  Files         249      251       +2     
  Lines        8065     8089      +24     
==========================================
+ Hits         6733     6757      +24     
  Misses       1332     1332              
Impacted Files Coverage Δ
zhaquirks/tuya/ts0043.py 100.00% <100.00%> (ø)
zhaquirks/tuya/ts0041.py 100.00% <0.00%> (ø)
zhaquirks/feibit/switch.py 100.00% <0.00%> (ø)
zhaquirks/feibit/__init__.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution.

one small thing:

@MattWestb
Copy link
Contributor

Missing the SimpleDescriptor for all endpoints that making it easier for devs and user finding the right quirk then testing new devices.

@TheJulianJES
Copy link
Collaborator

Missing the SimpleDescriptor for all endpoints that making it easier for devs and user finding the right quirk then testing new devices.

I've never used that comment tbh. It's also not there for many other Tuya remotes or quirks in general.
But you're correct that it is present for all other remotes in that file.

There's an experimental quirk generator/matcher here: https://github.com/zigpy/zha-device-handlers/tree/dm/quirk-stub-generator/quirk_generator (that doesn't need comments)

@fingolfin Maybe add the comment with the SimpleDescriptor. It should be the following if I'm correct (add that before model – see other quirks in that same class for reference):

        # SizePrefixedSimpleDescriptor(endpoint=1, profile=260, device_type=0, device_version=1, input_clusters=[0, 1, 6, 57344], output_clusters=[10, 25]))
        # SizePrefixedSimpleDescriptor(endpoint=2, profile=260, device_type=0, device_version=1, input_clusters=[1, 6], output_clusters=[])
        # SizePrefixedSimpleDescriptor(endpoint=3, profile=260, device_type=0, device_version=1, input_clusters=[1, 6], output_clusters=[])
        # SizePrefixedSimpleDescriptor(endpoint=4, profile=260, device_type=0, device_version=1, input_clusters=[1, 6], output_clusters=[])

@MattWestb
Copy link
Contributor

MattWestb commented Jan 9, 2023

Its good then its in place and is right but some have only copy the last device class and making changes on the endpoints and not updating the simple discretion and that is very bad then trying !fast patching" testing new devices for users (still must recalculating Dec / Hex).

Edit:

Great work done @fingolfin !!!

@fingolfin
Copy link
Contributor Author

I'll look into it later today

This one reports four switches, just like the `TuyaSmartRemote0042TOPlusA`
does, and also in general looks just like that (except with one more button).
Thus I named this quirk `TuyaSmartRemote0043TOPlusA`.

However, I did deviate in the replacement endpoints, and instead of copying
them from the TS0042 variant, I decided to model them more closely on
the other TS0043 variants.
@fingolfin
Copy link
Contributor Author

Added the SizePrefixedSimpleDescriptor now.

Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

Still looks good! Should be good to go

(FYI, commits are automatically squashed when merged)

@dmulcahey dmulcahey merged commit cec721e into zigpy:dev Jan 10, 2023
@fingolfin fingolfin deleted the mh/_TZ3000_w3c7ouru branch January 10, 2023 22:29
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