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 SecurityPanelController for alarm_control_panel to alexa #27081

Merged
merged 13 commits into from
Oct 4, 2019

Conversation

ochlocracy
Copy link
Member

@ochlocracy ochlocracy commented Sep 30, 2019

Description:

Implements the Alexa.SecurityPanelController for alarm_control_panel to alexa. This will expose entities in the alarm_control_panel domain during alexa discovery.

Utterances:

  • Alexa, arm my home in away mode.
  • Alexa, arm my home.
  • Alexa, disarm my home.
  • Alexa, is my home armed?

Caveats to consider:

  • User must opt-in to disarm by voice feature in the Alexa app. Alexa will require a 4 digit voice PIN for disarming. User can configure a 4 digit PIN in the Alexa app, or using the existing code configured for the entity. If entity code_format attribute is set to FORMAT_NUMBER The Alexa app will offer the ability to use the existing code. Otherwise user must configure a voice PIN in the Alexa app.
  • The code attribute is a private property depending on supported integrations, therefore it can not be validated against the PIN provided in the Alexa API request. The PIN provided in the API request will be passed directly to the alarm_disarm service.
    • If a PIN code is provided by the Alexa API request and disarm fails, it is reported to Alexa as an UNAUTHORIZED PIN code failure.
    • Unable to verify configured code attribute is 4 digits. Alexa API only supports FOUR_DIGIT_PIN.
  • alarm_control_panel state must be disarmed before arming. User can not change from one armed state to another armed state per Alexa API guidelines.
  • Alexa does not support arming with voice PIN at this time. Therefore if code_arm_required is set the entity will not be exposed during discovery.
  • armed_custom_bypass isn't supported by Alexa API and assumed armed_home

Related issue (if applicable):
Checks off part of #24579

Example entry for configuration.yaml:

Will include all entities in the alarm_control_panel domain during discovery, depending on filtering.

alexa:
  smart_home:
    filter:
      include_entities:
        - alarm_control_panel.entity_id
      include_domains:
        - alarm_control_panel

Checklist:

  • The code change is tested and works locally.
    Tested with alarmdotcom and manual alarm_control_panel, using custom smart home skill, with haaska.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]

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

If the code does not interact with devices:

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

@frenck
Copy link
Member

frenck commented Sep 30, 2019

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.

@frenck
Copy link
Member

frenck commented Sep 30, 2019

@ochlocracy run python3 -m script.hassfest from the project root, to update derived files from the manifest after you've changed it. 👍

@ochlocracy ochlocracy force-pushed the alexa-security-panel-contoller branch 2 times, most recently from 68b1c7f to cb9530d Compare October 3, 2019 14:00

def properties_proactively_reported(self):
"""Return True if properties asynchronously reported."""
return True
Copy link
Member

Choose a reason for hiding this comment

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

For another PR, but I realize that we should probably make this return alexa_config.should_report_state. Not everyone could have state reporting enabled.


def interfaces(self):
"""Yield the supported interfaces."""
if not self.entity.attributes.get("code_arm_required"):
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could still expose it even if this is required, but have arming it fail. That way users could still ask if their security panel is armed.

Dev automation moved this from Needs review to Reviewer approved Oct 3, 2019
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

This is great! Both comments can be addressed in a future PR. Awesome job! 🐬 🎉

@balloob
Copy link
Member

balloob commented Oct 3, 2019

This can be merged once merge conflicts resolved

@ochlocracy ochlocracy force-pushed the alexa-security-panel-contoller branch from 0e543bf to ebed412 Compare October 4, 2019 01:18
@ochlocracy
Copy link
Member Author

Resolved conflicts. There are some changes to alarm_control_panel that could be made to integrate this better overall, and resolve your comments. I'll start looking into that, and issue a PR.

@balloob balloob merged commit 9a5c1fb into home-assistant:dev Oct 4, 2019
Dev automation moved this from Reviewer approved to Done Oct 4, 2019
@lock lock bot locked and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants