Skip to content

Add supported_features to cover component#6082

Merged
balloob merged 1 commit into
home-assistant:devfrom
emlove:cover-supported-features
Feb 19, 2017
Merged

Add supported_features to cover component#6082
balloob merged 1 commit into
home-assistant:devfrom
emlove:cover-supported-features

Conversation

@emlove
Copy link
Copy Markdown
Contributor

@emlove emlove commented Feb 18, 2017

Description:
This PR adds supported_features to the cover component. Currently the use case is for zwave garage doors, which don't support the cover_stop service. This allows entities to declare their supported features so the frontend can display the appropriate controls.

See also:
home-assistant/frontend#215

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@armills, thanks for your PR! By analyzing the history of the files in this pull request, we identified @turbokongen, @balloob and @pvizeli to be potential reviewers.

]

SUPPORT_OPEN = 1
SUPPORT_CLOSE = 2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would we separate open and close? I don't think there will ever be a use case that has no open and close.

Even if a device does not have open and close specific but has set position, they could mimick open and close by setting position to fully open or fully closed. So I think that we can remove those.

Copy link
Copy Markdown
Contributor Author

@emlove emlove Feb 18, 2017

Choose a reason for hiding this comment

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

I do think that we need to keep the open/close separate from the set position, because although set position can emulate open/close, the opposite isn't true. It's very possible that there is a device that tracks position, but only has binary open/close commands.

As far as open/close being separate, I don't have any example of a device, but it certainly could be possible. I wouldn't have predicted a media player that does support next track but doesn't support previous, and then Pandora came around.

What about a compromise, where we have one flag SUPPORT_OPENCLOSE = 3, that way they could be broken out in the future if necessary, but don't clutter the current code? It's not necessary for my case, but I'd like to leave options open for the future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, you convinced me. I'm fine with it.

SUPPORT_SET_POSITION = 4
SUPPORT_STOP = 8
SUPPORT_OPEN_TILT = 16
SUPPORT_CLOSE_TILT = 32
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here, can be merged with open tilt ?

@balloob balloob merged commit 5f095b5 into home-assistant:dev Feb 19, 2017
@emlove emlove deleted the cover-supported-features branch February 19, 2017 02:25
@home-assistant home-assistant locked and limited conversation to collaborators Jun 2, 2017
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.

4 participants