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 websock command to query device for triggers #24044

Merged
merged 11 commits into from
Jun 10, 2019

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented May 22, 2019

Description:

Add websock command to query device for triggers.

Triggers can be provided by the integration that provided the device or the entity integrations that the device has entities with.

As an example of the latter, this PR also adds turn_on and turn_off triggers to light entities.
TODO: Add an example of the former.

This PR is intended as initial step towards home-assistant/architecture#178

To test, add to configuration.yaml:

device_automation:

Example websock exchange:

# Ask for triggers for device:
{"id": 1, "type": "automation/device_automation/list_triggers", "device_id": "46d461813d264be185bad875ea907166"}
# Two triggers returned:
{"id": 1, "type": "result", "success": true, "result": {"triggers": [{"platform": "device", "domain": "light", "type": "turn_on", "entity_id": "light.brown_light"}, {"platform": "device", "domain": "light", "type": "turn_off", "entity_id": "light.brown_light"}]}}

The returned trigger can be used in an automation:

trigger:
 - platform: 'device'
   domain: 'light'
   type: turn_off
   entity_id: light.disco_room
action:
   service: homeassistant.turn_on
   entity_id: light.sonoff

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

If the code does not interact with devices:

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

@emontnemery emontnemery requested a review from a team as a code owner May 22, 2019 19:48
}))


async def async_trigger(hass, config, action, automation_info):
Copy link
Member

Choose a reason for hiding this comment

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

Name it async_attach_trigger. What is config vs automation_info ? Shouldn't we just have hass, trigger_config, action ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, but I leave it for now so the new trigger can be tested without any changes to the existing automation framework.

def _domain_validator(config):
"""Validate it is a valid domain or platform."""
try:
platform = importlib.import_module(
Copy link
Member

Choose a reason for hiding this comment

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

This is really ugly, and it won't work with custom integrations either. We should use the async_get_integration method instead. However, that's async. Maybe just drop this validator ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the problem was that async_get_integration is async. I removed the validator for now.

try:
integration = await async_get_integration(hass, domain)
except IntegrationNotFound:
_LOGGER.exception('Integration %s not found', domain)
Copy link
Member

Choose a reason for hiding this comment

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

We should never use exception unless it's an unexpected exception. This one we know why it happens (but shouldn't). I suggest we log a warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


async def async_get_device_automation_triggers(hass, device_id):
"""List device triggers."""
device_registry = await hass.helpers.device_registry.async_get_registry()
Copy link
Member

Choose a reason for hiding this comment

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

Run both at the same time: dev_reg, ent_reg = await asyncio.gather(…)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

for entity in entities:
domains.add(split_entity_id(entity.entity_id)[0])

for domain in domains:
Copy link
Member

Choose a reason for hiding this comment

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

Let's run all of these tasks at the same time using asyncio.gather

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

# Trigger when light is turned on
CONF_PLATFORM: 'device',
CONF_DOMAIN: DOMAIN,
CONF_TYPE: 'turn_on',
Copy link
Member

Choose a reason for hiding this comment

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

let's create constants for turn_on and turn_off

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

vol.Required(CONF_PLATFORM): 'device',
vol.Optional(CONF_DEVICE_ID): str,
vol.Required(CONF_DOMAIN): DOMAIN,
vol.Required(CONF_ENTITY_ID): cv.entity_ids,
Copy link
Member

Choose a reason for hiding this comment

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

This should be single?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

state_config = {
state.CONF_ENTITY_ID: config[CONF_ENTITY_ID],
state.CONF_TO: to_state
}
Copy link
Member

Choose a reason for hiding this comment

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

Include a FROM or else a color update will trigger it too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@balloob
Copy link
Member

balloob commented Jun 3, 2019

I think that overall this looks great.

For translations, would we do something like device_automation.<domain>.<type> ?

@emontnemery
Copy link
Contributor Author

@balloob thanks for reviewing! All fixed now + tests added.

@balloob balloob changed the title WIP: Add websock command to query device for triggers Add websock command to query device for triggers Jun 10, 2019
@balloob balloob merged commit 935240f into home-assistant:dev Jun 10, 2019
@balloob balloob mentioned this pull request Jun 26, 2019
@emontnemery emontnemery mentioned this pull request Aug 22, 2019
6 tasks
@emontnemery emontnemery deleted the device_automations branch September 6, 2019 10:02
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Oct 12, 2019
* Add websock command to query device for triggers

* Lint

* Refactor

* Add support for domain automations

* Make device automation an automation platform

* lint

* Support device_id in light trigger

* Review comments

* Add tests

* Add tests

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants