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

Implemented event_data_template (new) #11057

Merged
merged 11 commits into from Jan 19, 2018

Conversation

@tschmidty69
Copy link
Contributor

tschmidty69 commented Dec 10, 2017

Description:

Implemented event_data_template within scripts. This can be used to create custom triggers or
pass data to other scripts. A good use case (mine) would be to pass custom data to an appdaemon app.

Related issue (if applicable): fixes #N/A

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4180

Example entry for configuration.yaml (if applicable):

script:
  set_timer:
    sequence:
      - event: APP_SET_TIMER
        event_data_template:
          name: '{{ name }}'
          hours: '{{ hours }}'
          minutes: '{{ minutes }}'
          seconds: '{{ seconds }}'

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.
@tschmidty69 tschmidty69 requested a review from home-assistant/core as a code owner Dec 10, 2017
@balloobbot balloobbot added the core label Dec 10, 2017
homeassistant/helpers/script.py Outdated
"""Recursive template creator helper function."""
if isinstance(value, list):
return [_data_template_creator(item)
for item in value]

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 10, 2017

continuation line over-indented for visual indent

@tschmidty69 tschmidty69 mentioned this pull request Dec 10, 2017
0 of 2 tasks complete
@tschmidty69 tschmidty69 mentioned this pull request Dec 10, 2017
0 of 2 tasks complete
tests/helpers/test_script.py Outdated
'event': event,
'event_data_template': {
'hello': """
{% if True %}

This comment has been minimized.

Copy link
@balloob

balloob Dec 10, 2017

Member

Please use a passed in variable.

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 11, 2017

Author Contributor

Again I copied this from service data template, but it bothered me that there was no variable in that test either, so I added it in both places.

homeassistant/helpers/script.py Outdated
event_data = dict(action.get(CONF_EVENT_DATA, {}))
if CONF_EVENT_DATA_TEMPLATE in action:
try:
def _data_template_creator(value):

This comment has been minimized.

Copy link
@balloob

balloob Dec 10, 2017

Member

This should be extracted as a method and moved to the template helper: render(hass, obj)

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 11, 2017

Author Contributor

Because I am a hack (sysadmin, not a professional coder), I had basically just copied this from service.py/async_call_from_config (notice I did add the try/except there since it was annoying that template errors cause exceptions there).

But I very much like the idea of moving this to the template render since it would make it a lot easier to add templating to other functions. I'll give it a whack.

More of a future question, but how opposed would you be to having all data* structures go through templating and not explicitly require data: or data_template:?. The only real downside I can see would be some additional overhead and maybe latency on calls going through the render but it would be fairly minimal I would think. Just a thought, it's not a big deal, but could open up a lot of creative possibilities.

This comment has been minimized.

Copy link
@balloob

balloob Dec 11, 2017

Member

The problem with running things through templates is that it ends up as a string.

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 11, 2017

Author Contributor

Ah, so things like entity_id are preserved as objects, etc. But aren't they passed as strings anyway if they are included as part of a data_template since they go through the template render process? Don't answer, Lol I don't have time to take enough of a stab so I'll just work on one thing at a time.

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 11, 2017

Author Contributor

Looks like this whole construct was for this issue: #2985

So this was put in to support lists and dicts as an object that contains template strings, which up until now was only supported in scripts.py (where I stole the code).

But you want to move this functionality into the render() function directly so we resolve the entire list or dict there along with single elements as it currently does, right?

I'll keep hacking at it but wanted to make double sure that was the direction you wanted.

This comment has been minimized.

Copy link
@balloob

balloob Dec 12, 2017

Member

No no, we should not move it into an existing function. Instead create a new one that can handle complex data types.

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 13, 2017

Author Contributor

I have banged on this for more time than I'd like to admit today and have to admit I am not capable of reworking this. Not saying I couldn't given enough time, but cutting my losses. You can reject this if you want; I'll implement the workaround of passing attributes through a sensor or something

This comment has been minimized.

Copy link
@tschmidty69

tschmidty69 Dec 14, 2017

Author Contributor

@balloob I don't know what notifications you get on these, so figured I would repost. Sorry, was frustrated banging on this in between work and a lack of sleep (which will be you soon, congrats!). So this is implemented as a function in template.py, and cleaned up in script.py and service.py. Updated unit tests for all 3. Looks good to me at this point.

@monster1025

This comment has been minimized.

Copy link

monster1025 commented Dec 13, 2017

Thx for this functionality, I'll be waiting for it in release =)

@tschmidty69

This comment has been minimized.

Copy link
Contributor Author

tschmidty69 commented Dec 14, 2017

@balloob Sorry, I got over my frustration and moved this into template.py. Added tests for lists and dicts as items and referencing specific elements so this should be all set.

@balloobbot balloobbot removed the component label Dec 14, 2017
tschmidty69 added 2 commits Dec 14, 2017
…istant into event-data-template
@@ -43,6 +43,16 @@ def attach(hass, obj):
elif isinstance(obj, Template):
obj.hass = hass

def render_complex(value, variables=None):

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 14, 2017

expected 2 blank lines, found 1

@@ -43,6 +43,16 @@ def attach(hass, obj):
elif isinstance(obj, Template):
obj.hass = hass

def render_complex(value, variables=None):

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 14, 2017

expected 2 blank lines, found 1

tschmidty69 added 3 commits Dec 14, 2017
@tschmidty69 tschmidty69 reopened this Dec 14, 2017
@tschmidty69

This comment has been minimized.

Copy link
Contributor Author

tschmidty69 commented Jan 2, 2018

@balloob @rytilahti @andrey-git Any chance of getting this reviewed? Using this fairly extensively to pass data to appdaemon without having to use the workaround of setting a sensor state to pass transient data.

homeassistant/components/snips.py Outdated

yield from hass.components.mqtt.async_subscribe(
INTENT_TOPIC, message_received)

return True

def resolve_slot_values(slot):

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

expected 2 blank lines, found 1

tests/components/test_snips.py Outdated
"slots": []
}
"""
# unsub = mqtt.subscribe(self.hass, 'hermes/dialogueManager/endSession',

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

line too long (80 > 79 characters)

tests/components/test_snips.py Outdated
fire_mqtt_message(self.hass,
'hermes/intent/CallServiceIntent', payload)
self.hass.block_till_done()
#print("test_handle:", test_handle.output[1])

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

tests/components/test_snips.py Outdated
#from unittest import mock

from homeassistant.core import callback
#from homeassistant import setup

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

tests/components/test_snips.py Outdated
"slots": [
"""The tests for the Dialogflow component."""
import unittest
#from unittest import mock

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

tests/components/test_snips.py Outdated
# 'hermes/dialogueManager/endSession',
# self.record_calls)
#print("self.calls:", self.calls)
#self.assertEqual('hermes/dialogueManager/endSession',

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

tests/components/test_snips.py Outdated
#unsub = mqtt.subscribe(self.hass,
# 'hermes/dialogueManager/endSession',
# self.record_calls)
#print("self.calls:", self.calls)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

tests/components/test_snips.py Outdated
"slots": []
}
"""
#unsub = mqtt.subscribe(self.hass,

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 4, 2018

block comment should start with '# '

@tschmidty69 tschmidty69 force-pushed the tschmidty69:event-data-template branch to b01ec45 Jan 4, 2018
@tschmidty69 tschmidty69 changed the title Implemented event_data_template Implemented event_data_template (new) Jan 9, 2018
@balloob balloob merged commit 48619c9 into home-assistant:dev Jan 19, 2018
4 checks passed
4 checks passed
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 93.926%
Details
hound No violations found. Woof!
@balloob balloob mentioned this pull request Jan 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.