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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embedded platforms and the road to packaged components #124

Closed
balloob opened this Issue Dec 20, 2018 · 13 comments

Comments

Projects
None yet
8 participants
@balloob
Copy link
Member

balloob commented Dec 20, 2018

So @rytilahti had a good point in his long (and mainly off-topic 馃槈) comment to my proposal to change how we deal with discovery. But I felt that a) it should be it's own issue and b) it should go further. And credit where credit is due, it's also something @amelchio has said before, and probably others too.

@rytilahti proposed to make each platform a folder with their own metadata file. I've always shot this idea down, because it means that all our code has to be written twice, once for things based as a component and once for things based as a platform. That would be a maintenance hell and thus hinder progress.

However, thinking about this, I realized that we can embed the platforms in components. That way we forget about the idea of "platforms" altogether. Instead, an entity component can load embedded platforms from other components.

File structure wise, we would go from:

hue/__init__.py
light/hue.py

to:

hue/__init__.py
hue/light.py

The content of the file stays exactly the same, but now we have all code for a single component in one folder.

This means a bunch of things, among others:

  • Only one code path, it's now all components all the way down
  • We can implement this backwards compatible, by just looking at both spots. Implementation wise, we just change loader.py to be defined as:
    def get_platform(hass, domain, platform):
        return (
            get_component(hass, "{}.{}".format(platform, domain) or
            get_component(hass, "{}.{}".format(domain, platform))
  • We can start having config entries for "platforms", because it's a component now.
  • We can now start defining a metadata file for components per #121 (comment) (but discussing that is out of scope for this issue)
  • Custom components will now be easier to distribute, which leads to鈥
  • Packaged components. If we have a single folder for a component and all it's platforms, we can make sure we have a file with metadata to specify requirements (so we can install requirements before loading the Python file and thus import stuff at the top of our python files), versioning, code owners, logo, name. Now this can be independently distributed and updated. Not just by us, but by others too, like Hass.io add-on repositories including updates etc. Just like Hass.io add-ons, it will ease maintenance as no longer the core team is the bottleneck of all PR reviews (this is just a future vision, not to be discussed in this issue!)
  • Entity components can be cleaned up and broken up into multiple files, like adding a media_player/const.py for the constants. This will help with circular dependencies like we're currently facing in home-assistant/home-assistant#18700

Downsides:

The only one I can think of is that once all built-in platforms are embedded and we deprecate the old pattern of light/hue.py, we could break some custom components that didn't update yet.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 20, 2018

How are we thinking about services here? Will platform specific services be registered under the platform domain, eg light, or under the component domain, eg hue? Do we change anything about that with the new structure?

@balloob

This comment has been minimized.

Copy link
Member Author

balloob commented Dec 20, 2018

They should be published under the component domain. Hue already does this (hue.hue_activate_scene). Sonos does not (yet): media_player.sonos_X, but those should become sonos.X

@pvizeli

This comment has been minimized.

Copy link
Member

pvizeli commented Dec 20, 2018

I like that a lot because with config flow it was going to be weird. They need to be a component, and for a platform only piece before it was strange. This close this behaves.

@armills

This comment has been minimized.

Copy link
Member

armills commented Dec 20, 2018

IMO this is a slam dunk.

Should we also consider separating entity components, and integration components? I think that if we get rid of the notion of having entity platforms hosted underneath our base components, it makes sense to separate those from our integrations, since those are more of a core concern.

@iantrich

This comment has been minimized.

Copy link

iantrich commented Dec 20, 2018

Can we elaborate on services? Are we saying that if I currently have the device light.bed but it is a x branded light, its service will be x.x_turn_on_light or am I missing something obvious?

@elupus

This comment has been minimized.

Copy link

elupus commented Dec 20, 2018

To limit issues with circular dependencies the init.py must be mostly empty. Since importing component/x/const.py will automatically load init.py. thus we don't want that to pull in a bunch of stuff.

PS. Im from the referenced wip branch above where I ran into problems with the constants for climate being defined inside climate platform, but needed from helpers.

@elupus

This comment has been minimized.

Copy link

elupus commented Dec 20, 2018

@iantrich, I'm pretty sure that only applies to custom services. Not the default supplied by platform.

@balloob

This comment has been minimized.

Copy link
Member Author

balloob commented Dec 20, 2018

Should we also consider separating entity components, and integration components?

I'm not ready for that yet. I want to keep it such that we have a single component interface on top of the core.

@elupus

This comment has been minimized.

Copy link

elupus commented Dec 21, 2018

This will be lovely for custom components too btw. Since it allow:

cd custom_components
git clone component_a
git clone component_b

Ie multiple custom components hosted by somebody else. Now it's messy since they need to populate same directories.

Edit: Realised it was already mentioned.

@petri

This comment has been minimized.

Copy link

petri commented Dec 22, 2018

How about using Python entry points as a plugin / component package discovery mechanism?

@balloob

This comment has been minimized.

Copy link
Member Author

balloob commented Dec 22, 2018

@petri yes, probably. But as mentioned in the original issue, discussing how to exactly implement packaged components is out of scope for this PR.

@rytilahti

This comment has been minimized.

Copy link

rytilahti commented Dec 22, 2018

What are the remaining points that are to be discussed under this issue, assuming we are agreeing that the proposal sounds good enough to start tinkering with the code? I suppose the only (sofar) listed downside is not a major issue.

Should a new issue discussing the implementation be opened? The remark from @petri would be on-topic there (and IMO that is the way to go for the implementation, making (custom) components pypi installable is a nice side-effect of using an existing 'metadata' possibilities of setuptools).

@balloob

This comment has been minimized.

Copy link
Member Author

balloob commented Dec 24, 2018

You can open an issue and discuss MVP, only represent functionality we have today in a manifest. Discovery is beyond MVP, as we first need to migrate all of Home Assistant integrations to follow this issue and then a potential packaged component proposal, before we can even think about implementing it. This is the last comment about packaged components on this issue, all others will be removed.

@balloob balloob referenced this issue Jan 11, 2019

Merged

Allow embedded platforms #19948

4 of 4 tasks complete

@wafflebot wafflebot bot added the in progress label Jan 11, 2019

@dgomes dgomes referenced this issue Feb 13, 2019

Open

Somfy open api #19548

8 of 8 tasks complete

tetienne added a commit to tetienne/home-assistant that referenced this issue Feb 14, 2019

mad-ady added a commit to mad-ady/home-assistant-customizations that referenced this issue Mar 4, 2019

@andrewsayre andrewsayre referenced this issue Mar 9, 2019

Open

Add HEOS media player platform #21721

8 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.