Add template extension coordinates by eifinger · Pull Request #30019 · home-assistant/core

eifinger · 2019-12-17T09:38:15Z

Description:

This template extension will allow for a clean and reusable way to enable dynamic travel_time sensors. The issue was discussed several times in the forum here and here as well as in the PRs #27237 #28716.

Using this template the following config is possible:

input_select:
  here_destination_preset:
    options:
      - zone.home
      - zone.office
      - zone.somewheredefault

sensor:
  - platform: here_travel_time
    api_key: my_api_key
    name: Reistijd
    origin_template: {{ coordinates('device_tracker.myphone') }}
    destination_template: {{ coordinates('input_select.here_destination_preset') }}

automation:    
  - alias: 'set'
    trigger:
      - platform: homeassistant
        event: start
      - platform: time
        at: 00:00
    action:
       service: input_select.select_option
       data_template:
        entity_id: input_select.here_destination_preset
        option: "{% if is_state('device_tracker.myphone','home') %}zone.office{% elif is_state('device_tracker.myphone','office') %}zone.home{% else %}zone.somewheredefault{% endif %}"

The simple case {{ coordinates('device_tracker.myphone') }} can already be accomplished with existing methods but now the user does not have to know that a device_tracker stores latitude and longitude as attributes.

The nested case {{ coordinates('input_select.here_destination_preset') }} is currently only possible by creating a complex structure of dependent template_sensors using complex templates.

Related issue (if applicable): fixes #TBD

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

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

See above.

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 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:

The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
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:

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

Mariusthvdb · 2019-12-17T10:01:42Z

this is very nice indeed! thanks for working on this, highly anticipated functionality!.

might I suggest 2 further improvements ?

why prescribe the suffix _template. Would be much user friendlier if the component were to autodetect the fact a template was entered in the fields for destination and origin.
further more, if we are on the subject of auto-detection, why not make it even better, and have the component auto sense the gps coordinates, if a config would be using a device_tracker? It expects/excepts lat/lon only, so have it look for that when a device_tracker was entered as origin/destination?
same would go for the input_select. Why not simply accept {{states('input_select.here_destination_preset'}} and auto find the coordinates of that zone?

cheers, thanks for considering.

eifinger · 2019-12-17T10:59:04Z

@Mariusthvdb to your suggestions:

Without the _template suffix it would be indeed much less verbose to implement in a travel_time integration. But I could not find any instance in the documentation where a user was allowed to supply a template to a field and it was not named with the suffix _template. Did I overlook it?
I am not sure I understand you correctly. You want to get rid of the template I implement here and move the functionality directly to the integration.

This was not done for several reasons:

The integration (e.g. here_travel_time) should not be responsible for homeassistant internal stuff, which this functionality is
While working on the PR for the new travel_time integration I initially put in in there but realized this is some functionality I would want in other integrations as well.
I could have put it into some helper method but then it would be up to each developer of each integration to implement this functionality. By releasing this functionality as a template all a developer has to do is enable the usage of templates. He doesn't have to look through all the functions in the homeassistant source and think of ways the user would want to interact with his integration. If the template compiles all is good.

Mariusthvdb · 2019-12-17T11:26:42Z

yes on both accounts.
also yes on understanding your considerations ;-) I was merely trying to make it easier for the user, not having to think about all demands the various integrations put forward.

about needing _template or not: not really sure tbh if there are other integrations. I do know several custom Lovelace cards simply accept templates or fixed values regardless. I know that is not the same, but one easily gets used to that friendly configuration..

about the other question, you might be right on track! Suppose it would be best if HA system wide would simply accept templates on all config fields... as long as that is not the case, your solution might be wisest.
thanks for taking the time to explain, appreciated

stale · 2020-03-21T16:27:43Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

eifinger · 2020-03-21T16:29:17Z

Keep open

stale · 2020-05-15T05:40:19Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

eifinger · 2020-05-15T05:52:41Z

Still open

dgomes

Looks good, please fix the conflict

pvizeli · 2020-05-26T08:26:41Z

+    return None
+
+
+def _entity_state_is_valid_coordinate_set(state: str) -> bool:


This function is only used once, you can also use:

try: cv.gps(state.split(",")) except vol.Invalid: _LOGGER.error(...) else: return entity.state

Removed the function

eifinger · 2020-05-26T09:08:38Z

Done

balloob · 2020-06-22T23:25:12Z

+        return _get_location_from_attributes(entity)
+
+    # Check if device is in a zone
+    zone_entity = _resolve_state(hass, f"zone.{entity.state}")


Suggested change

zone_entity = _resolve_state(hass, f"zone.{entity.state}")

zone_entity = hass.states.get(entity.state)

Setting your state to the entity ID of a zone is not an official supported "thing", we shouldn't encode it in our template helper unless we want to adopt this.

Let's remove this support.

balloob

The goal of this PR is to put logic in the template helper to make passing a parameter to a sensor easier. That doesn't make sense.

If this is something that the sensor wants to support, they should just implement it.

eifinger · 2020-06-23T07:01:47Z

As I explained in the description this functionality was discussed several times already.
I first wanted it to introduce directly to the here_travel_time sensor with this pr: #27237 but was told it wasn't the right place.
We then discussed on discord and on #28716 that it should also not be put into the helpers or a travel_time integration.
Then I opened this PR.

My preference would still be to implement it directly into here_travel_time.

balloob · 2020-06-23T19:17:22Z

I think it's fine to add a helper to helpers/location.py that can be used by travel time integrations.

It's not fine to hijack templates for a one-off function.

eifinger · 2020-06-29T20:19:04Z

I opened a new PR #37234 to add the function under helpers/location.py

eifinger requested a review from a team as a code owner December 17, 2019 09:38

homeassistant added the cla-signed label Dec 17, 2019

probot-home-assistant Bot added the core label Dec 17, 2019

eifinger mentioned this pull request Dec 17, 2019

Add template extension coordinates home-assistant/home-assistant.io#11482

Closed

2 tasks

eifinger mentioned this pull request Jan 20, 2020

Add travel_time integration #28716

Closed

9 tasks

stale Bot added the stale label Mar 21, 2020

stale Bot removed the stale label Mar 21, 2020

MartinHjelmare added the second-opinion-wanted Add this label when a reviewer needs a second opinion from another member. label Apr 15, 2020

stale Bot added the stale label May 15, 2020

stale Bot removed the stale label May 15, 2020

dgomes requested changes May 25, 2020

View reviewed changes

eifinger added 2 commits May 26, 2020 09:51

Add template extension coordinates

866a5ae

fix mypy

4c015b7

eifinger force-pushed the add_template_get_coords branch from fafb720 to 4c015b7 Compare May 26, 2020 07:51

run pyupgrade

ce48187

pvizeli reviewed May 26, 2020

View reviewed changes

Remove function

2e657cd

balloob reviewed Jun 22, 2020

View reviewed changes

balloob requested changes Jun 22, 2020

View reviewed changes

eifinger mentioned this pull request Jun 29, 2020

Add helpers.location.coordinates #37234

Merged

20 tasks

eifinger closed this Jun 29, 2020

		return None


		def _entity_state_is_valid_coordinate_set(state: str) -> bool:

	zone_entity = _resolve_state(hass, f"zone.{entity.state}")
	zone_entity = hass.states.get(entity.state)

Uh oh!

Conversation

eifinger commented Dec 17, 2019 • edited 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!

Mariusthvdb commented Dec 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eifinger commented Dec 17, 2019

Uh oh!

Mariusthvdb commented Dec 17, 2019

Uh oh!

stale Bot commented Mar 21, 2020

Uh oh!

eifinger commented Mar 21, 2020

Uh oh!

stale Bot commented May 15, 2020

Uh oh!

eifinger commented May 15, 2020

Uh oh!

dgomes left a comment

Choose a reason for hiding this comment

Uh oh!

pvizeli May 26, 2020

Choose a reason for hiding this comment

Uh oh!

eifinger May 26, 2020

Choose a reason for hiding this comment

Uh oh!

eifinger commented May 26, 2020

Uh oh!

balloob Jun 22, 2020

Choose a reason for hiding this comment

Uh oh!

balloob Jun 22, 2020

Choose a reason for hiding this comment

Uh oh!

balloob left a comment

Choose a reason for hiding this comment

Uh oh!

eifinger commented Jun 23, 2020

Uh oh!

balloob commented Jun 23, 2020

Uh oh!

eifinger commented Jun 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eifinger commented Dec 17, 2019 •

edited

Loading

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

Mariusthvdb commented Dec 17, 2019 •

edited

Loading