Skip to content

Adding source attribute to geo location platforms#17339

Merged
balloob merged 3 commits intohome-assistant:devfrom
exxamalte:geo-location-source-attribute
Oct 12, 2018
Merged

Adding source attribute to geo location platforms#17339
balloob merged 3 commits intohome-assistant:devfrom
exxamalte:geo-location-source-attribute

Conversation

@exxamalte
Copy link
Copy Markdown
Contributor

@exxamalte exxamalte commented Oct 11, 2018

Description:

This adds a source attribute to the existing geo_location platforms. The purpose of this change is to make entities generated by these platforms identifiable for the new geo_location trigger.
Each new platform will need to override the source method in its own sub-class of GeoLocationEvent, so that the value is automatically added by the state_attributes method.

Related issue (if applicable): prerequisite for #16967

Pull request in home-assistant.io with documentation (if applicable): n/a

Example entry for configuration.yaml (if applicable): n/a

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

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

ATTR_OPTION = 'option'

# For entities which have a source they want to specify.
ATTR_SOURCE = 'source'
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 keep this constant local

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.

OK

@property
def source(self) -> Optional[str]:
"""Return source value of this external event."""
return None
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 make this mandatory by raising NotImplementedError

I don't want the automation trigger to not be able to work on all geo platforms.

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.

OK

@balloob balloob merged commit 1f86383 into home-assistant:dev Oct 12, 2018
@ghost ghost removed the in progress label Oct 12, 2018
@exxamalte exxamalte deleted the geo-location-source-attribute branch October 13, 2018 01:28
exxamalte added a commit to exxamalte/home-assistant that referenced this pull request Oct 13, 2018
@exxamalte exxamalte mentioned this pull request Oct 13, 2018
7 tasks
MartinHjelmare pushed a commit that referenced this pull request Oct 14, 2018
* initial integration with nsw rural fire service feed

* improved test coverage

* updated requirements

* grouped imports

* removed debug print statement

* moved manager's startup code into separate call

* simplified feed update code

* simplified feed update code

* simplified device state attribute code

* added source to conform with pr #17339

* fixed lint

* refactored how entities are managed

* fixed pylint

* simplified signalling
@balloob balloob mentioned this pull request Oct 26, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
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.

4 participants