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

Homekit controller garage cover support #19668

Closed
wants to merge 7 commits into from
Closed

Homekit controller garage cover support #19668

wants to merge 7 commits into from

Conversation

rudgr
Copy link

@rudgr rudgr commented Dec 30, 2018

Description:

https://community.home-assistant.io/t/control-homekit-enabled-garage-door-from-homeassistant/87722
https://community.home-assistant.io/t/add-garage-door-opener-device-type-support-to-homekit-controller/87723

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

not applicable

Checklist:

  • The code change is tested and works locally. > tested with Nice Control SPA SpeedGate
  • Local tests pass with tox. Your PR cannot be merged unless tests pass > sorry newbie here, don't know how to run the tests yet :)
  • There is no commented out code in this PR.

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

added Mapping from Homekit type garage-door-opener to component Cover.
Implemented homekit-controller for cover compoment
removed commented code to comply with PR approval checklist
@homeassistant
Copy link
Contributor

Hi @rudgr-duplicate,

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!

@property
def supported_features(self):
"""Flag supported features."""
return SUPPORT_OPEN | SUPPORT_CLOSE | SUPPORT_STOP

Choose a reason for hiding this comment

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

blank line at end of file

@property
def available(self):
"""Return True if entity is available."""
return not self._door_state_current in [STATE_UNKNOWN]

Choose a reason for hiding this comment

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

test for membership should be 'not in'

self.put_characteristics(characteristics)


@property

Choose a reason for hiding this comment

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

too many blank lines (2)


def stop_cover(self, **kwargs):
"""Stop the cover."""
if self._door_state_current not in [STATE_STOPPED, STATE_OPEN, STATE_CLOSED]:

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

if self._door_state_current in [STATE_UNKNOWN]:
return None
return self._door_state_current in [STATE_CLOSED]

Choose a reason for hiding this comment

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

blank line contains whitespace


if ctype == "door-state.current":
self._chars['door-state.current'] = characteristic['iid']
self._door_state_current = MODE_HOMEKIT_TO_HASS.get(characteristic['value'])

Choose a reason for hiding this comment

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

line too long (92 > 79 characters)

accessory = hass.data[KNOWN_ACCESSORIES][discovery_info['serial']]
add_entities([HomeKitCover(accessory, discovery_info)], True)

class HomeKitCover(HomeKitEntity, CoverDevice):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

# Map of hass operation modes to homekit modes
MODE_HASS_TO_HOMEKIT = {v: k for k, v in MODE_HOMEKIT_TO_HASS.items()}

def setup_platform(hass, config, add_entities, discovery_info=None):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

from homeassistant.components.cover import (CoverDevice, SUPPORT_OPEN, SUPPORT_CLOSE, SUPPORT_STOP)
from homeassistant.const import (
STATE_UNKNOWN, STATE_CLOSED, STATE_OPEN, STATE_CLOSING, STATE_OPENING)

Choose a reason for hiding this comment

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

blank line contains whitespace


from homeassistant.components.homekit_controller import (HomeKitEntity,
KNOWN_ACCESSORIES)
from homeassistant.components.cover import (CoverDevice, SUPPORT_OPEN, SUPPORT_CLOSE, SUPPORT_STOP)

Choose a reason for hiding this comment

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

line too long (99 > 79 characters)

removed empty lines
@homeassistant
Copy link
Contributor

Hi @rudgr-duplicate,

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!

test for membership should be 'not in'
@homeassistant
Copy link
Contributor

Hi @rudgr-duplicate,

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!

@rudgr
Copy link
Author

rudgr commented Dec 30, 2018

hmmm, for some reason github created my commits under @rudgr-duplicate instead of @rudgr ... so cannot sign the CLA :(

re-formatted imports
@homeassistant
Copy link
Contributor

Hi @rudgr-duplicate,

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!

@homeassistant
Copy link
Contributor

Hi @rudgr-duplicate,

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!

@fabaff fabaff changed the title Feature/homekit controller garage cover support Homekit controller garage cover support Dec 30, 2018
@frenck
Copy link
Member

frenck commented Dec 31, 2018

Could not find a related documentation PR in our documentation repository, regarding this change.
Adding docs-missing label.

@MartinHjelmare
Copy link
Member

hmmm, for some reason github created my commits under @rudgr-duplicate instead of @rudgr ... so cannot sign the CLA :(

After fixing your git config email and github email, you can rebase your commits with the correct author information. Please turn to google or github support pages for best help, as this is not a problem specific to home assistant.

@rudgr
Copy link
Author

rudgr commented Jan 5, 2019

Tx, seems to be an old duplicate Github account indeed, Github support is working on deleting it, after that, I Will resubmit the PR, including documentation ;-)

@rudgr
Copy link
Author

rudgr commented Jan 9, 2019

github support finally fixed my github account...

I'm happy to update my PR, but seems that @adrum submitted a similar PR that supports more device types: #19866 ?

@MartinHjelmare
Copy link
Member

Maybe you want to review the other PR and help get it merged?

@rudgr
Copy link
Author

rudgr commented Jan 21, 2019

sorry @MartinHjelmare , my HA instance died, so I needed the time to setup everything again, great to see the other PR is merged, can't wait for HA v 0.86 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants