Skip to content

Embed geofency platform into component#20083

Merged
MartinHjelmare merged 2 commits into
home-assistant:devfrom
rohankapoorcom:embed-geofency
Jan 16, 2019
Merged

Embed geofency platform into component#20083
MartinHjelmare merged 2 commits into
home-assistant:devfrom
rohankapoorcom:embed-geofency

Conversation

@rohankapoorcom
Copy link
Copy Markdown
Member

@rohankapoorcom rohankapoorcom commented Jan 14, 2019

Description:

This PR leverages #19948 to embed the Geofency device tracker platform under the component.

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.

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated 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:

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

@ghost ghost added the in progress label Jan 14, 2019
Comment thread homeassistant/components/geofency/device_tracker.py Outdated
@MartinHjelmare MartinHjelmare changed the title Embded geofency platform into component Embed geofency platform into component Jan 15, 2019
@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Jan 16, 2019

Just noticed that we don't use async_setup_entry in the geofency device tracker platform. Also noticed that there's no async_unload_entry in the device tracker base component.

It doesn't matter for the set up, but if trying to unload the entry, nothing will happen and the dispatch signal won't be removed. We should probably do something about this in the future.

@MartinHjelmare MartinHjelmare merged commit 11c78d5 into home-assistant:dev Jan 16, 2019
@ghost ghost removed the in progress label Jan 16, 2019
@rohankapoorcom rohankapoorcom deleted the embed-geofency branch January 17, 2019 05:12
@rohankapoorcom
Copy link
Copy Markdown
Member Author

rohankapoorcom commented Jan 18, 2019

Also noticed that there's no async_unload_entry in the device tracker base component.

@MartinHjelmare does this mean the base component needs to switch to using EntityComponent?

@MartinHjelmare
Copy link
Copy Markdown
Member

Not necessarily, although I would like that. Switching to EntityComponent would be a big refactor. For now, we could just add a class that collects the necessary setup and unload methods and has access to the DeviceTracker instance in the device tracker base component.

@rohankapoorcom
Copy link
Copy Markdown
Member Author

Sounds good. I'll push something up once I get it working.

@rohankapoorcom
Copy link
Copy Markdown
Member Author

Pushed up in #20237.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants