Skip to content

Add HomeKit Door accessory type#80741

Merged
bdraco merged 24 commits into
home-assistant:devfrom
Dexwell:homekit_door_accessory
Mar 7, 2023
Merged

Add HomeKit Door accessory type#80741
bdraco merged 24 commits into
home-assistant:devfrom
Dexwell:homekit_door_accessory

Conversation

@Dexwell
Copy link
Copy Markdown
Contributor

@Dexwell Dexwell commented Oct 21, 2022

Proposed change

This PR aims to make cover templates with the device_class set to door appear as actual doors in HomeKit. See #79363

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
  • 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
Copy Markdown
Contributor

Hi Dexwell

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!

@home-assistant
Copy link
Copy Markdown
Contributor

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

Code owner commands

Code owners of homekit can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant unassign homekit Removes the current integration label and assignees on the issue, add the integration domain after the command.

@Dexwell
Copy link
Copy Markdown
Contributor Author

Dexwell commented Oct 21, 2022

I took a stab at implementing this myself—please bear with me as this is my first contribution to this project. I have not been able to test whether this achieves the desired result in HomeKit as I don't know how to test this in a development environment, but in a different project making these similar changes produced the correct result. Hopefully this is a good first start and someone else can get this PR across the finish line.

Comment thread homeassistant/components/homekit/accessories.py
bdraco
bdraco previously requested changes Oct 23, 2022
Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

All HomeKit accessories need 100% test coverage before we can merge them. Please add tests for this new accessory type. Thanks!

@Dexwell
Copy link
Copy Markdown
Contributor Author

Dexwell commented Oct 24, 2022

I've never written tests—can anyone else pick this up?

@Quentame Quentame added the help-wanted Wanna help? Jump in! label Dec 2, 2022
@jredd
Copy link
Copy Markdown
Contributor

jredd commented Jan 11, 2023

@Dexwell If you based your code off of the garage integration you could just mimic the test coverage for the garage door. Take a look in the tests directory, it has a similar folder layout to the homeassistant directory.

@jredd
Copy link
Copy Markdown
Contributor

jredd commented Jan 14, 2023

If I would be allowed to make changes to this I have the testing code made.

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Jan 14, 2023

@jredd Either @Dexwell can invite you to collaborate on his repo, or you might be better off opening a fresh PR

@Dexwell
Copy link
Copy Markdown
Contributor Author

Dexwell commented Jan 14, 2023

@jredd That's awesome! Absolutely, I believe you can do so by creating a pull request to dexwell:homekit_door_accessory, correct?

I invited you as a collaborator on my repo :)

@jredd
Copy link
Copy Markdown
Contributor

jredd commented Jan 14, 2023

@Dexwell It took me a min to figure out how to do it with Github, I'm use to gitlab now and things are a bit weird for me. I have created a pr into your fork I believe, let me know how it looks and if I need to create more changes.

feat: adding testing for the door type cover
@home-assistant
Copy link
Copy Markdown
Contributor

Hi jredd

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!

@bdraco bdraco self-requested a review February 4, 2023 17:25
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Feb 20, 2023

Should be good to go once a docs PR is opened and linked to update https://www.home-assistant.io/integrations/homekit/#supported-components

Dexwell added a commit to Dexwell/home-assistant.io that referenced this pull request Feb 21, 2023
@Dexwell
Copy link
Copy Markdown
Contributor Author

Dexwell commented Feb 23, 2023

Should be good to go once a docs PR is opened and linked to update https://www.home-assistant.io/integrations/homekit/#supported-components

Done!

@bdraco bdraco self-requested a review February 23, 2023 20:13
@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 7, 2023

Screenshot 2023-03-06 at 5 34 46 PM

Screenshot 2023-03-06 at 5 33 24 PM

I changed one of my shades to display as a door and reloaded the bridge.

Its working as expected

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 7, 2023

Thanks @Dexwell

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 7, 2023

We still need a test for the accessories file

 Check warning on line 155 in homeassistant/components/homekit/accessories.py

Codecov
/ codecov/patch
homeassistant/components/homekit/accessories.py#L155
Added line #L155 was not covered by tests

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Mar 7, 2023

I added the missing coverage.

@bdraco bdraco merged commit 755c44d into home-assistant:dev Mar 7, 2023
bdraco pushed a commit to home-assistant/home-assistant.io that referenced this pull request Mar 7, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 8, 2023
tetele pushed a commit to tetele/home-assistant.io that referenced this pull request Mar 8, 2023
2023.3: Beta release notes

2023.3: Update KNX change description (home-assistant#26347)

2023.3: Process community feedback on beta release notes

2023.3: Add Z-Wave JS breaking change details

Clarify the example is below the sentence (home-assistant#26349)

Update Insteon documentation (home-assistant#26399)

2023.3: Finalize restarting Home Assistant

2023.3: Finalize Python 3.11 section

2023.3: Finalize sensor precision

2023.3: Finalize new integrations

2023.3: Finalize breaking change + dev blog links

2023.3: Finalize other noteworthy changes

2023.3: Add missing todoist breaking change

2023.3: Finalize new automation dialog

2023.3: Finalize thread and matter

2023.3: Finalize intro

2023.3: Finalize new light dialog

2023.3: Title, date, description, links

2023.3: Small tweaks

2023.3: Small tweaks

Add Heltun integration (home-assistant#26411)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Revert "Update derivative.markdown" (home-assistant#26402)

Remove reference to non-implemented attributes (home-assistant#26408)

Apply suggestions from code review

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

2023.3: Update changelog

Fix cover templates breaking change text

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

Use brand logos if ha_brand in search results (home-assistant#26427)

Clarify Thread border router credential sync (home-assistant#26423)

2023.3.1 (home-assistant#26430)

* 2023.3.1

* Update source/_posts/2023-03-01-release-20233.markdown

* Update source/_posts/2023-03-01-release-20233.markdown

add reinstall instructions (home-assistant#26439)

Bump rack from 2.2.6.2 to 2.2.6.3 (home-assistant#26440)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Add scene.delete service

See home-assistant/core#89090

Add climate devices documentation for LIVISI integration (home-assistant#25957)

Add Camera to Prosegur (home-assistant#25419)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Adding Obihai codeowner (home-assistant#26386)

HELTUN partnership announcement blog post (home-assistant#26415)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Update renault documentation - Add button section (home-assistant#26260)

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Document energy_storage and volume_storage device classes (home-assistant#26394)

Co-authored-by: Franck Nijhof <git@frenck.dev>

Add Config Flow to Obihai (home-assistant#26354)

Add reolink button entities (home-assistant#26355)

device_class and state_class (home-assistant#25742)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Reolink add switch entities (home-assistant#26353)

Co-authored-by: Franck Nijhof <git@frenck.dev>

Add Reolink siren documentation (home-assistant#26282)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Franck Nijhof <git@frenck.dev>

Add Reolink select entities (home-assistant#26341)

Co-authored-by: Franck Nijhof <git@frenck.dev>
Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Add new Reolink number entities (home-assistant#26283)

Add reolink light entities (home-assistant#26338)

Add Obihai Reboot button (home-assistant#26385)

Add clarification for reolink entities (home-assistant#26469)

Co-authored-by: Franck Nijhof <frenck@frenck.nl>

Add Door cover documentation (home-assistant#26331)

Resolves home-assistant/core#80741
@frenck frenck added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed Hacktoberfest hacktoberfest-accepted integration: homekit new-feature noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) Quality Scale: No score

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HomeKit: Cover of device_class 'door' should appear as door in the Home app

6 participants