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 google_assistant alarm_control_panel #26249

Merged

Conversation

@engrbm87
Copy link
Contributor

commented Aug 28, 2019

Description:

This PR adds the Alarm Control Panel devices to Google Assistant so that we can Arm and Disarm using google voice commands.
The Alarm Panel code needs to be the same as the secure_devices_pin assigned for security devices. If the code_arm_required is False then the system will arm without asking for the PIN otherwise it will ask for the PIN (which is the security CODE).
I have tested this using the manual alarm control panel. This should work for the other alarm system integrations.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10250

Checklist:

  • The code change is tested and works locally.
  • 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:

@MartinHjelmare MartinHjelmare changed the title add alarm_control_panel to google_assistant Add google_assistant alarm_control_panel Aug 31, 2019
@MartinHjelmare MartinHjelmare requested a review from balloob Aug 31, 2019
Dev automation moved this from Needs review to Review in progress Sep 5, 2019
Copy link
Member

left a comment

This is ok. It's a bit weird that we need to have the same code which means it won't work with multiple alarm control panels. However, that might just be fixed in the future if we allow specifying per-device alarm codes.

Please add tests and this is ok to merge.

engrbm87 added 3 commits Sep 18, 2019
{
"level_synonym": [
state.replace("_", " "),
state.split("_")[

This comment has been minimized.

Copy link
@balloob

balloob Sep 25, 2019

Member

What's going on here ? why?

This comment has been minimized.

Copy link
@balloob

balloob Sep 25, 2019

Member

Please add a comment

This comment has been minimized.

Copy link
@engrbm87

engrbm87 Sep 25, 2019

Author Contributor

I wanted to generate the synonyms from the state name itself. so armed_away turns into armed away or away instead of hard coding it.
Do you recommend to keep it this way or just had code the synonyms?

async def execute(self, command, data, params, challenge):
"""Execute an ArmDisarm command."""
if params["arm"] and not params.get("cancel"):
# _LOGGER.debug("%s - %s", self.state.state, params["armLevel"])

This comment has been minimized.

Copy link
@balloob

balloob Sep 25, 2019

Member

Please remove

{
"level_name": "triggered",
"level_values": [
{"level_synonym": ["triggered", "triggered"], "lang": "en"}

This comment has been minimized.

Copy link
@balloob

balloob Sep 25, 2019

Member

these synonyms are duplicate?

This comment has been minimized.

Copy link
@engrbm87

engrbm87 Sep 25, 2019

Author Contributor

The reason is this a duplicate is because for the Trigger state it is one word. and using the logic I did for the synonyms the result will be the same twice.
The alternative would be to hard code the synonym. what do you think?

This comment has been minimized.

Copy link
@balloob

balloob Sep 25, 2019

Member

let's create the synonyms above this dict statement and add an if-check for triggered there.

Dev automation moved this from Review in progress to Reviewer approved Sep 25, 2019
@balloob

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

One small comment needed, for the rest this looks great !

@balloob balloob merged commit d1adb28 into home-assistant:dev Sep 25, 2019
11 checks passed
11 checks passed
CI Build #20190925.13 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch 97.87% of diff hit (target 94.02%)
Details
codecov/project 94.16% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 25, 2019
@lock lock bot locked and limited conversation to collaborators Sep 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
3 participants
You can’t perform that action at this time.