Template support for alarm_control_panel by alistairg · Pull Request #24997 · home-assistant/core

alistairg · 2019-07-06T20:41:24Z

Description:

Template support for alarm control panel - templated state, icon, entity picture. Scripted actions for arm away, home, stay and disarm.

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

TBD

Example entry for `configuration.yaml` (if applicable):

alarm_control_panel:
  - platform: template
    panels:
      alarmdecoder:
        friendly_name: "Virtual Alarm Panel"
        code_arm_required: false
        value_template: "{{ states('alarm_control_panel.alarm_panel') }}"
        disarm:
          service: alarm_control_panel.alarm_disarm
          data:
            entity_id: alarm_control_panel.alarm_panel
            code: !secret alarm_code
        arm_away:
          service: alarm_control_panel.alarm_arm_away
          data:
            entity_id: alarm_control_panel.alarm_panel
            code: !secret alarm_code
        arm_home:
          service: alarm_control_panel.alarm_arm_home
          data:
            entity_id: alarm_control_panel.alarm_panel
            code: !secret alarm_code
        arm_night:
          service: alarm_control_panel.alarm_arm_night
          data:
            entity_id: alarm_control_panel.alarm_panel
            code: !secret alarm_code

Checklist:

[ X] The code change is tested and works locally.
[X ] Local tests pass with tox. Your PR cannot be merged unless tests pass
[ X] There is no commented out code in this PR.
[ X] I have followed the development checklist

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

Documentation added/updated in home-assistant.io

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

[NA ] The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
[NA] New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

If the code does not interact with devices:

[ X] Tests have been added to verify that the new code works.

ghost · 2019-07-06T20:41:30Z

Hey there @PhracturedBlue, mind taking a look at this pull request as its been labeled with a integration (template) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

alistairg · 2019-07-06T21:32:05Z

Docs in home-assistant/home-assistant.io#9801

MartinHjelmare · 2019-07-07T12:43:18Z

What functionality here isn't supported already via the manual alarm control panel + automations?

alistairg · 2019-07-07T14:59:11Z

@MartinHjelmare - fair question. They're fairly different, though.

The manual panel is best for creating a new panel from scratch, where one doesn't already exit. The logic and state machine is managed by Home Assistant.

The template panel is best for augmenting the behavior of an existing panel, or series of security devices, whose state machine is managed externally. For example - template panel supports templating for rendering state.

My own use case here is adjusting the behaviour of our actual alarm panel when exposed via Home Assistant and Siri. I want our panel to only be "quick armable" via Homekit if one of us is home, and "disarmable" under other complex rules. I also need to be able to augment the Alarmdecoder component, which doesn't support "Night" mode properly (as the underlying AlarmDecoder API doesn't, and my PR to fix that natively within the component was rejected as a result).

MartinHjelmare · 2019-07-10T00:03:42Z

@balloob do you think we should add this next to the manual alarm control panel?

alistairg · 2019-07-10T00:31:10Z

@MartinHjelmare - I honestly think that these are two really different use cases. For me, the “template” series is about adjusting the behaviour of existing components, or merging several together to present something more logical in HASS (or onward tools, like Alexa, Google, Siri). It’s also incredibly powerful for adjusting the logic of existing devices when controlled via the UI or other apps (e.g. “if the light brightness is manually controlled turn off the flux component”, or “if the alarm is sounding or no-one is home, refuse to disarm it via the UI”).

The Manual Alarm Control Panel is a complex beast that’s about creating a full-fledged security system within Home Assistant. For the UI-adjustment use cases that’ll lead end users to have to create a lot of configuration code, and try and synchronize two different state machines.... that’s likely to lead to poor user experiences, and other fun bugs (e.g. what happens when HASS is restarted?...)

MartinHjelmare · 2019-07-10T08:00:24Z

I want a second opinion before I go ahead and review.

balloob · 2019-07-10T22:45:47Z

I think that this would be a useful contribution. @alistairg has a good point with state being managed outside of this integration versus the manual alarm, which manages it itself.

MartinHjelmare

I haven't looked at the tests yet.

MartinHjelmare · 2019-09-03T08:25:55Z

+    vol.Optional(CONF_ARM_HOME_ACTION): cv.SCRIPT_SCHEMA,
+    vol.Optional(CONF_ARM_NIGHT_ACTION): cv.SCRIPT_SCHEMA,
+    vol.Optional(CONF_ICON_TEMPLATE): cv.template,
+    vol.Optional(CONF_ENTITY_PICTURE_TEMPLATE): cv.template,


Do we need entity picture and icon template?

MartinHjelmare · 2019-09-03T08:29:17Z

+    alarm_control_panels = []
+
+    for device, device_config in config[CONF_ALARM_CONTROL_PANELS].items():
+        friendly_name = device_config.get(CONF_FRIENDLY_NAME, device)


Please use dict[key] for required config keys and keys with default config schema values.

MartinHjelmare · 2019-09-03T08:31:34Z

+                code_arm_required, entity_ids)
+        )
+
+    if not alarm_control_panels:


We should require at least one alarm panel in the config schema.

MartinHjelmare · 2019-09-03T08:32:04Z

+        return False
+
+    async_add_entities(alarm_control_panels)
+    return True


Nothing is checking this return value. We can remove this statement.

MartinHjelmare · 2019-09-03T08:33:10Z

+        self._state = None
+        self._icon = None
+        self._entity_picture = None
+        self._brightness = None


Brightness isn't used.

MartinHjelmare · 2019-09-03T08:34:47Z

+        optimistic_set = False
+
+        if self._template is None:
+            _LOGGER.info("Optimistically setting arm state to Away")


MartinHjelmare · 2019-09-03T08:35:35Z

+                ATTR_CODE: code
+            }, context=self._context)
+        else:
+            _LOGGER.warning("No script action defined for Arm Away.")


I'd make this an error.

We don't end logging messages with period.

frenck · 2019-11-20T08:28:41Z

A couple of months have passed now, without activity on this PR.
I'm going to close the PR for now because it is running stale.

Nevertheless, @alistairg, I hope you are able to re-open and pick up again since this contribution would be very much appreciated 👍

Completed tests

df9eff0

homeassistant added cla-signed has-tests integration: template new-platform labels Jul 6, 2019

alistairg added 2 commits July 6, 2019 13:45

Shortened lines

0f54e1f

Added additional tests

c55e695

alistairg mentioned this pull request Jul 6, 2019

Documentation for template alarm_control_panel home-assistant/home-assistant.io#9801

Merged

2 tasks

Increased code test coverage

f6ebc1d

Added default friendly name per docs

c71b289

MartinHjelmare requested changes Sep 3, 2019

View reviewed changes

frenck closed this Nov 20, 2019

lock Bot locked and limited conversation to collaborators Nov 21, 2019

Uh oh!

Conversation

alistairg commented Jul 6, 2019 • edited by frenck Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

ghost commented Jul 6, 2019

Uh oh!

alistairg commented Jul 6, 2019

Uh oh!

MartinHjelmare commented Jul 7, 2019

Uh oh!

alistairg commented Jul 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinHjelmare commented Jul 10, 2019

Uh oh!

alistairg commented Jul 10, 2019

Uh oh!

MartinHjelmare commented Jul 10, 2019

Uh oh!

balloob commented Jul 10, 2019

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frenck commented Nov 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alistairg commented Jul 6, 2019 •

edited by frenck

Loading

Example entry for `configuration.yaml` (if applicable):

alistairg commented Jul 7, 2019 •

edited

Loading