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

Implementations for NO_MANUFACTURER_ID dimmers #1489

Merged
merged 6 commits into from
Apr 19, 2022

Conversation

javicalle
Copy link
Collaborator

@javicalle javicalle commented Apr 13, 2022

This PR adds device definitions for dimmers that require send commands with the NO_MANUFACTURER_ID (or what is the same manufacturer_specific=False).

A few comments:

  • tuya/mcu/TuyaLevelControl now handles move_to_level_with_on_off command, sending an on_off command before the current_level command.
  • the TuyaLevelControlManufCluster handles DPs up to 3 gangs.
  • the new NoManufacturerCluster will force the NO_MANUFACTURER_ID in commands. This is needed for recent dimmers
  • at some point I wanted to migrate the remaining classes to the new implementation and I have done now. TuyaSingleSwitchDimmer devices will use the same implementation as the rest
  • some definitions are using the NoManufacturerCluster implementation and others no. It can be important (or not) when adding new devices to the existing implementations

Related issues:

This PR is good to go in next minor version.

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2022

Codecov Report

Merging #1489 (5739fe8) into dev (c379b94) will increase coverage by 0.03%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##              dev    #1489      +/-   ##
==========================================
+ Coverage   80.36%   80.40%   +0.03%     
==========================================
  Files         227      227              
  Lines        6702     6725      +23     
==========================================
+ Hits         5386     5407      +21     
- Misses       1316     1318       +2     
Impacted Files Coverage Δ
zhaquirks/__init__.py 77.89% <ø> (-0.24%) ⬇️
zhaquirks/adeo/color_controller.py 82.50% <ø> (ø)
zhaquirks/aduro/adurolightncc.py 100.00% <ø> (ø)
zhaquirks/aurora/aurora_dimmer.py 82.60% <ø> (ø)
zhaquirks/ikea/dimmer.py 100.00% <ø> (ø)
zhaquirks/ikea/fivebtnremote.py 100.00% <ø> (ø)
zhaquirks/ikea/fivebtnremotezha.py 100.00% <ø> (ø)
zhaquirks/ikea/fourbtnremote.py 100.00% <ø> (ø)
zhaquirks/ikea/shortcutbtn.py 100.00% <ø> (ø)
zhaquirks/ikea/symfonisk.py 100.00% <ø> (ø)
... and 9 more

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 5272b4c...5739fe8. Read the comment docs.

@coveralls
Copy link

coveralls commented Apr 13, 2022

Pull Request Test Coverage Report for Build 2163027951

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.01%) to 80.38%

Files with Coverage Reduction New Missed Lines %
zhaquirks/tuya/mcu/init.py 2 98.04%
zhaquirks/tuya/ts0601_dimmer.py 2 93.75%
Totals Coverage Status
Change from base Build 2150069164: 0.01%
Covered Lines: 5408
Relevant Lines: 6728

💛 - Coveralls

@javicalle javicalle marked this pull request as ready for review April 13, 2022 17:52
@javicalle javicalle added the needs final review PR is ready for a final review from a maintainer label Apr 13, 2022
@dmulcahey dmulcahey merged commit 504c4bc into zigpy:dev Apr 19, 2022
@yftj
Copy link

yftj commented Apr 19, 2022

Hey, I recently had my device support request closed via this thread. I assume a fix has been implemented? Any work I need to do to add the fix or will an update to ZHA via Homeassistant fix this. Thank you so much for your work!

@javicalle
Copy link
Collaborator Author

Hi @yftj,
the fix has been added to the library mainstream and (probably) will be included in the next library version.
And this new version will be included in future versions of HA. It could be in the next fix or in the future v2022.05.

However, there is no simple criteria to define when the new version of the library will be generated or when it will be included in HA, it depends on multiple factors.

@yftj
Copy link

yftj commented May 5, 2022

Just tested today with the new update to HAOS, I have full control of my dimmers now (dimmer switch TS0601 _TZE200_1agwnems)! Thank you very much! There is however a small problem, changing the brightness slider causes the light to momentarily turn off almost like a reset for a fraction of a second (time it takes to blink almost) and then from full brightness goes down to the selected brightness. Expected result would be for the dimmer to change from eg. 40% to 50% instead it goes from 40% to off to 100% and slowly dims to 50%. Sorry if this isn't the right place to report a bug.

@javicalle
Copy link
Collaborator Author

@yftj Please, open a new issue with your device info and attach the logs from your problem. The logs will tell us if the behaviour comes from HA, from ZHA or if is a device issue.

@javicalle javicalle deleted the nomanuf_dimmers branch May 28, 2022 09:46
javicalle added a commit to javicalle/zha-device-handlers that referenced this pull request Sep 14, 2022
Implement the `on_off` call in the `move_to_level_with_on_off` command for the MCU Tuya devices.
The previous implementation was removed in zigpy#1748. Originally introduced in zigpy#1489
dmulcahey pushed a commit that referenced this pull request Sep 25, 2022
* Implement the `move_to_level_with_on_off` command 

Implement the `on_off` call in the `move_to_level_with_on_off` command for the MCU Tuya devices.
The previous implementation was removed in #1748. Originally introduced in #1489

* Fix black coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment