Skip to content

Add support to Hunter Douglas for Silhouette Type 23 Tilting#70775

Merged
bdraco merged 8 commits intohome-assistant:devfrom
trullock:hunterdouglas
May 9, 2022
Merged

Add support to Hunter Douglas for Silhouette Type 23 Tilting#70775
bdraco merged 8 commits intohome-assistant:devfrom
trullock:hunterdouglas

Conversation

@trullock
Copy link
Contributor

@trullock trullock commented Apr 26, 2022

Proposed change

Add support to Hunter Douglas for Silhouette Type 23 Tilting

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)
  • 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
  • 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.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@trullock trullock requested a review from bdraco as a code owner April 26, 2022 10:13
@homeassistant
Copy link
Contributor

Hi @trullock,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@probot-home-assistant
Copy link

Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (hunterdouglas_powerview) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@trullock trullock changed the title Add support for Silhouette Type 23 Tilting Add support to Hunter Douglas for Silhouette Type 23 Tilting May 1, 2022
@trullock
Copy link
Contributor Author

trullock commented May 1, 2022

@bdraco I've pushed a refactored version

@bdraco
Copy link
Member

bdraco commented May 1, 2022

@trullock If you cherry-pick this commit it will resolve some of the comments above bdraco@291ca66

@bdraco
Copy link
Member

bdraco commented May 1, 2022

Thank you for your contribution thus far! 🎖 Since this is a significant contribution, we would appreciate you'd added yourself to the list of code owners for this integration. ❤️

Please, add your GitHub username to the manifest.json of this integration.

For more information about "code owners", see: Architecture Decision Record 0008: Code owners.

@trullock
Copy link
Contributor Author

trullock commented May 1, 2022

@trullock If you cherry-pick this commit it will resolve some of the comments above bdraco@291ca66

you want me to stick this on top of my PR, right?

@trullock
Copy link
Contributor Author

trullock commented May 2, 2022

I've done everything except adding myself to code owners, doing so breaks the hassfest pre-commit hook in ways I dont understand, all manner of unhelpful errors occur :/

I want to give the current code a final testing tomorrow hopefully so don't merge it yet, but PR feedback welcome

@bdraco
Copy link
Member

bdraco commented May 2, 2022

I've done everything except adding myself to code owners, doing so breaks the hassfest pre-commit hook in ways I dont understand, all manner of unhelpful errors occur :/

You can run python3 -m script.hassfest manually instead

@trullock
Copy link
Contributor Author

trullock commented May 4, 2022

@bdraco pushed the final set of changes and physically tested it, all working well.

I still cant get hassfest to run (needed to add myself as a contributor)

image

If you can add me and commit that that would be great, or can otherwise help me solve the above

Otherwise this PR is ready i think :)

@bdraco
Copy link
Member

bdraco commented May 9, 2022

If you can add me and commit that that would be great, or can otherwise help me solve the above

I'm not sure what is causing the encoding error on your system, but I went ahead and took care of running hassfest

@bdraco
Copy link
Member

bdraco commented May 9, 2022

Tested. Didn't see any breakage with my shades

@bdraco bdraco merged commit 9ef5c23 into home-assistant:dev May 9, 2022
@trullock
Copy link
Contributor Author

trullock commented May 9, 2022

If you can add me and commit that that would be great, or can otherwise help me solve the above

I'm not sure what is causing the encoding error on your system, but I went ahead and took care of running hassfest

Awesome thanks.

Just using it via msysgit on windows, never had an issue before. I'll take it to the forums


RESYNC_DELAY = 60

POSKIND_NONE = 0
Copy link
Member

Choose a reason for hiding this comment

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

Please separate words with underscore in variable names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in the next PR to extend this functionality further

@kingy444
Copy link
Contributor

kingy444 commented May 9, 2022

Tested. Didn't see any breakage with my shades

@bdraco @trullock I think there is something in this commit that breaks the is_closed
It is returning True but the UI is not treating it as such

The button close button is not disabling as you would expect it to - i rolled back to an old version and it worked as expected

i am investigating and will update here if i can, and/or include the fix in my pull

@kingy444
Copy link
Contributor

kingy444 commented May 9, 2022

Tested. Didn't see any breakage with my shades

@bdraco @trullock I think there is something in this commit that breaks the is_closed It is returning True but the UI is not treating it as such

The button close button is not disabling as you would expect it to - i rolled back to an old version and it worked as expected

i am investigating and will update here if i can, and/or include the fix in my pull

@bdraco found the cause. Wasnt this one but a commit you made a couple back where #70195 implemented _attr_assumed_state = True

As a result shades always have this display even when closed
image
Where as we actually expect a closed shade to look like this
image

With all the 'FORCE_RESYNC' we are sending now is this necessary?

@bdraco
Copy link
Member

bdraco commented May 9, 2022

Yes. The assumed state is still needed until we solve #70043 via #70043 (comment)

@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2022
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.

Hunter Douglas integration doesnt support tilt

5 participants