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 ZHA cover tilt #102072

Merged
merged 5 commits into from
Oct 24, 2023
Merged

Conversation

tomasbedrich
Copy link
Contributor

Proposed change

This PR adds support to control cover tilt as defined in Zigbee Cluster Library Specification (rev 8) 7.4.2.1.2.9.
There are multiple devices waiting for this support:

This PR is a duplicate of #93301, which is currently stale despite our numerous attempts to contact the author @josef109.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @dmulcahey, @Adminiuga, @puddly, mind taking a look at this pull request as it has been labeled with an integration (zha) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of zha can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign zha Removes the current integration label and assignees on the pull request, add the integration domain after the command.

@tomasbedrich tomasbedrich marked this pull request as ready for review October 15, 2023 19:43
homeassistant/components/zha/cover.py Outdated Show resolved Hide resolved
homeassistant/components/zha/cover.py Outdated Show resolved Hide resolved
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant bot marked this pull request as draft October 16, 2023 15:40
@tomasbedrich tomasbedrich marked this pull request as ready for review October 16, 2023 20:01
@home-assistant home-assistant bot requested a review from puddly October 16, 2023 20:01
@cremor cremor mentioned this pull request Oct 17, 2023
12 tasks
@puddly
Copy link
Contributor

puddly commented Oct 20, 2023

This PR looks good, I think it would be good to get into the beta.

So that CI is unblocked, could you add a few unit tests for async_update and async_restore_last_state?

@MartinHjelmare MartinHjelmare changed the title ZHA: Add support for cover tilt Add ZHA cover tilt Oct 20, 2023
@tomasbedrich
Copy link
Contributor Author

There is a test at least for restore_state. I’m wondering why it is not executed / why codecov reports miss.

@tomasbedrich
Copy link
Contributor Author

The changes to increase the coverage are hopefully done. While implementing them, perhaps I discovered a dead code in zha/cover.py which I removed. Please review thoroughly.

In case the dead async_update removal is valid, there is a similar snippet in zha/lock.py and maybe other files. But this is out of scope for this PR.

@tomasbedrich
Copy link
Contributor Author

One more warning: currently I am unable to test the code on a physically connected tilting cover device. If anyone here has an opportunity to test this branch if it still works good (compared to #93301), it would be nice.

@m67hoff
Copy link

m67hoff commented Oct 23, 2023

@tomasbedrich i’m testing your version since last week and it works fine.
Thank you very much.
I will update to the lates commit today and give you a feedback.

@m67hoff
Copy link

m67hoff commented Oct 24, 2023

IMHO it works awesome - I tested all cover.xxx services https://www.home-assistant.io/integrations/cover#services with the nexentro (needs no quirk) and to me it looks good
I also tried from the dashboard. Anything else I should test ?

@tomasbedrich
Copy link
Contributor Author

Specifically I'm interested if this code change didn't break anything, but I'm not sure how to test it.

@m67hoff
Copy link

m67hoff commented Oct 24, 2023

I'm also not sure - but maybe this should fixed here #99646 - since it has nothing to do with the tilt support. And the #99646 is anyway fixing states.

If I understand this right it's for updating the cover state. This seems working (also with your last change).
I set it to close 40% and till 60% and it is shown the same way "opening" as it was with no custom-ZHA

image

@puddly puddly merged commit d25b4aa into home-assistant:dev Oct 24, 2023
23 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants