Skip to content

Add google_assistant alarm_control_panel#26249

Merged
balloob merged 7 commits into
home-assistant:devfrom
engrbm87:alarm_control_panel_google_assistant
Sep 25, 2019
Merged

Add google_assistant alarm_control_panel#26249
balloob merged 7 commits into
home-assistant:devfrom
engrbm87:alarm_control_panel_google_assistant

Conversation

@engrbm87
Copy link
Copy Markdown
Contributor

@engrbm87 engrbm87 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 August 31, 2019 10:44
Copy link
Copy Markdown
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 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.

{
"level_synonym": [
state.replace("_", " "),
state.split("_")[
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.

What's going on here ? why?

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.

Please add a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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"])
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.

Please remove

{
"level_name": "triggered",
"level_values": [
{"level_synonym": ["triggered", "triggered"], "lang": "en"}
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.

these synonyms are duplicate?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

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.

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

@balloob
Copy link
Copy Markdown
Member

balloob 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
@lock lock Bot locked and limited conversation to collaborators Sep 26, 2019
@engrbm87 engrbm87 deleted the alarm_control_panel_google_assistant branch September 16, 2022 09:07
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.

3 participants