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

Allow Config Entries to be discovered #121

Closed
balloob opened this issue Dec 18, 2018 · 4 comments
Closed

Allow Config Entries to be discovered #121

balloob opened this issue Dec 18, 2018 · 4 comments

Comments

@balloob
Copy link
Member

balloob commented Dec 18, 2018

Right now the netdisco package is used for discovery. Routing all discovery through netdisco has the problem that we now have a centralized place that has to support an ever growing list of devices that can be discovered, some of them with custom discovery protocol implementations. We also discover things that Home Assistant doesn't support. Result is that netdisco is getting bloated. I've already stopped approving new discovery protocols and plan to remove all non-standard discovery protocols.

But this will leave a discovery gap. And I do think that discovery is important to get users up and running during onboarding and when they want to set up things later.

Proposal

We freeze adding any custom protocols to netdisco (as per home-assistant-libs/netdisco#230)

We will no longer discover every X minutes. Instead we discover once during onboarding and when the user navigates to the integrations page in the UI. We will initially allow a discovery legacy mode to keep running netdisco each X minutes.

Config Entries will be able to register how they can be discovered: upnp/SSDP, mDNS/zeroconf/bonjour or with their own custom method. Our new discovery mechanism will loop over all config entries and see how they can be discovered.

If integration defines a custom discovery, we will install the component requirements and will call that method, the config entry can now leverage the lib for discovery. Installing requirements seem like a lot of useless installation and loading modules, but it's not that bad: on Hass.io/Docker the packages are already installed. We also will only run discovery during onboarding and whenever the user navigates to the integration page. So during normal operation, these packages won't be loaded.

We will keep a list of popular config entries that will be included when doing discovery. This list contains all that rely on standard protocols and a couple popular integrations that use a custom discovery (ie Philips Hue nupnp). A user can have the option to do an "extended" discovery that will include all integrations with custom discovery.

Eventually I want to deprecate the netdisco package. The code from netdisco that we depend on will be included in Home Assistant as part of the discovery component.

Some code to illustrate

class HueDiscovery(config_entries.ConfigDiscovery):

    async def async_ssdp_discovery(self, ssdp_results):
        entries = ssdp_results.find_by_device_description({
            "manufacturer": "Royal Philips Electronics",
            "manufacturerURL": "http://www.philips.com",
            "modelNumber": ["929000226503", "BSB002"]
        })

        for entry in entries:
            self.hass.async_create_task(self.hass.config_entries.flow.async_init(
                DOMAIN, context={'source': config_entries.SOURCE_DISCOVERY},
                data={
                    'host': entry['host'],
                    'path': entry['path'],
                }
            ))


@config_entries.HANDLERS.register(DOMAIN)
class HueFlowHandler(config_entries.ConfigFlow):

    @staticmethod
    @callback
    def async_discovery():
        return HueDiscovery()

Advantages

  • No more 1 place (netdisco) to contain config for an ever growing list of dependencies
  • No more code duplication between netdisco and integrations
  • Only do discovery when it's necessary (onboarding, visit to integrations page)
  • Better mapping between discoveries and the devices that an integration can handle.
  • Code for an integration in 1 place.
  • Allow possibility to have integration discovery code be able to detect when one of their devices has changed IP and update the config entry.

Disadvantages

  • Only supports config entries when legacy mode dropped from discovery
@MartinHjelmare
Copy link
Member

What do you mean with the last point?

Only supports config entries when legacy mode dropped from discovery

@balloob
Copy link
Member Author

balloob commented Dec 18, 2018

When we drop legacy discovery mode based on Netdisco, we would no longer discover integrations that rely on passing discovery_info to async_setup_platform. These: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/discovery.py#L58

@rytilahti
Copy link
Member

rytilahti commented Dec 19, 2018

So it's been a while since I pondered about making the discovery nicer, so I'm definitely 👍 for getting this cleaned.
As this topic has been on my thoughts almost as long as I have used homeassistant, I will give my 0.02e on the topic.

In contrast to @balloob's proposal, I think that instead of making it harder to make devices discoverable, we should empower everyone to make their components/platforms more discoverable (adding discoverability to IQS?). Note that I'm not against sunsetting netdisco nor against the idea of enabling the component/platform developers to implement their custom discovery handling where it is supposed to be (that is, near the component/platform code), merely pointing out that we can do better for the regular developers with no special needs.

As this comment snowballed into way bigger than I expected, here's a short overview of what I propose (on top of @balloob's proposal):

  • Move from polling not to "poll only on configure integration and onboarding", but to continuous discovery (the devices announce themselves commonly over UPnP/mDNS when they join the network). Notify the user in these cases and let them decide how to proceed and which component(s)/platform(s) they want to configure for the new device.

  • Add a new, component/platform-specific metadata file containing information on what kind of devices it supports. This file will be used to see which platforms are interested in handling a newly connected device. It will also greatly reduce the boilerplate code currently located in netdisco (most of the discoverables are simply checking a field or two in the responses, see the listing at the end).

  • Plant the idea, that having such a metadata file (and have platforms in their own sub-directories) can also be useful for other use-cases.

I'll start by defining a couple of user stories to show how my proposed changes would improve the situation even further.

User stories

  1. User adds a new (supported) device to the network
    1.1. A notification is displayed to guide the user to decide if he wants to configure the device:
    1.1.1 If unwanted. Either dismiss (will be shown again later) or disable (no future notifications from this device shall be shown).
    1.1.2. If wanted, click on Configure leads to the configuration page, where the all potential platforms/components supporting the device are shown, and can be activated and configured.

  2. User wants to configure integrations
    2.1. Discovery process is done as a whole, discovering all (even the disabled) devices and providing the opportunity to configure them.
    2.2. If other platforms support the already configured device, the user is given a choice to configure those platforms to be activated for the device.

  3. User wants to remove a configured device
    3.1. Go to configure integrations, and simply removes the configuration as it is done already.

Developer stories

  1. Developer wants to make the new platform discoverable
    1.1. The developer adds a single file defining how the platform can be discovered using mDNS or UPnP.

    • No need to modify netdisco and/or core (discovery component) and/or convert the platform into a full component.
  2. Developer wants to use a custom discovery protocol
    2.1. Developer has to use config entry and implement the custom discovery protocol.

Implementation

In order to making the implementation as straightforward as possible, a new meta-data file called metadata.<FILEFORMAT> shall be stored alongside the component/platform. This metadata.<FILEFORMAT> file MAY be used by developers to define which mDNS service types or UPnP search targets (ST) they claim to support.

The components/platforms who have registered their wish to support the new device are loaded on demand and their respective setup_{component,platform} is called with the full discovery_info as delivered by the discovery module.
The developer can read the wanted information directly out from the provided data structure and decide if is capable of supporting the given device and inform the user about it.

The biggest change besides the new metadata file is that the platforms are to be moved into their own directories.
For example, light/yeelight.py shall be renamed to light/yeelight/yeelight.py and light/yeelight/metadata.<FILEFORMAT> can be created to enable discovering of Yeelight bulbs.

NOTE: Nothing here is set in stone, these are just to give an idea how different platforms could define what they support. The available configuration keys for the metadata are based on checking the current netdisco implementations, there is a list at the end of what types of checks the implemented discoveries do.

Reasoning for a new file and structure

Adding a new file and modifying the source tree structure should not be taken lightly, so I'm listing here some advantages and disadvantages of this change.

Some of these are completely unrelated to discoverability, but, in my opinion worthy reasons to make the change for the future's sake.

Advantages

  1. Cleaner directory structure and separation, all light.yeelight platform files are under light/yeelight/ and not under light/ among other platforms.
  • Developers can, if they wish so, split their huge platform files into smaller chunks.
  1. No need to have a central file for supported discoveries, nor need modify the core code to add new ones (https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/discovery.py#L58).

  2. The file could also be re-used in the future for various other purposes:

  • Defining REQUIREMENTS and DEPENDENCIES, meaning no more need to load the module to look these up.
  • Defining CODEOWNERS, no need to keep updating /CODEOWNERS, clearer ownership (and thus, PR/bug notifications)
  • Defining CONFIGURATION and SERVICES
  • Defining SUPPORTED_FEATURES (allows for easier IQS, effectively disallowing developers to change features on the fly)
  1. The metadata file will be easier to parse than python code, and can easily be exploited for documentation purposes (generating documentation blocks for services and configuration, ..)

Disadvantages

  • More directories? The current platforms shall be kept working for the near future as they are.

Backwards compatibility & required changes from developers

The existing implementations SHALL continue working without any changes for some later determined time period, and the conversion shall be made straightforward.
The already available discovery_info argument will be re-used to pass the data as provided by the discovery protocol (mDNS, UPnP).

Component developers

  • A metadata file needs to be created.
  • If special handling was implemented in netdisco, this has to be moved into setup_component.

Platform developers

  • A new, platform-specific directory for storing the metadata file must.
  • If special handling was implemented in netdisco, this has to be moved into setup_platform.

Implementation details

SSDP

  • The available filter keys for developers are all of those (not just Required, but also Recommended and Allowed) defined in "UPnP Device Architecture 2.0" (http://upnp.org/specs/arch/UPnP-arch-DeviceArchitecture-v2.0.pdf).
  • Exact strings, regular expressions and wildcards are allowed.
  • Specifying multiple keys are interpreted as OR (meaning a single platform/component can advertise to support both MediaRenderer and MediaServer).
  • Different keys are always interpreted as AND.

Note, that the file format is not important here, these examples just try to give an idea how such configuration would look like. Instead of ini files this could be YAML or JSON.

Generic media renderer (dlna_dmr)

  • A regexp matching on ST
[upnp]
st=urn:schemas-upnp-org:device:MediaRenderer:[1-3]

Huawei

  • Matching ST + manufacturer
[upnp]
manufacturer=Huawei Technologies Co., Ltd.
st=urn:schemas-upnp-org:device:InternetGatewayDevice:1

Deconz

  • Matching Recommended modelDescription and Allowed manufacturerURL
[upnp]
modelDescription=dresden elektronik Wireless Light Control
manufacturerURL=http://www.dresden-elektronik.de

Hue

  • A slightly more complicated, but doable with a simple regexp.
[upnp]
manufacturer=Royal Philips Electronics
manufacturerURL=http://www.philips.com
modelNumber=(929000226503|BSB002)

mDNS

  • Only service type and (host)name can be set.
  • Exact strings, regular expressions and wildcards are allowed.
  • Multiple instances of the same key can be defined and are then interpreted as OR (for readability).
  • Different keys are always interpreted as AND.

Yeelight bulbs

  • Device name starts with 'yeelink-light-'
[mdns]
service=_miio._udp.local.
name=^yeelink-light-

HP printers

  • Device name starts with 'HP '
[mdns]
service=_printer._tcp.local.
name='HP *'

Custom discovery protocols

  • Developers are required to do this with a configuration flow (as shown in the original post).
  • The discovery is can only be activated manually from "configure integrations".
  • During onboarding we could show a list of devices requiring a custom protocol (defined in the metadata, again?), to let them install the required dependencies and to perform the discovery process.

Appendix 1. Common discovery protocols

  • There are two widely used discovery protocols: mDNS and UPnP (SSDP). I'll describe briefly below how they work.
  • We support these out of the box. The component and platform developers do not need to write extra code (except in edge cases, which they can do that inside their own setup_platform())
  • Instead of having differing keys for the discover_info, the data received from the device (as delivered by the upstream lib) is handed in to the supporting platforms.
    • This will reduce the boilerplate code, while still making it possible for platform developers to do extra checks.
    • Makes us rely on upstream developers for the used interfaces, but it should be a no-brainer as this SHALL provide merely already decoded discovery data from the wire.

UPnP/SSDP

How does it work

The gruntwork here is (to be) done in the backend library, but I think describing how these work will help to understand the big picture better.

  1. UDP multicast on port 1900 to search for wanted devices (by defining the wanted ST, or simply requesting all of them)
  • Devices respond with information about their offered services (including device type) and the URL to the service description URL
  • In order to get more information about the device (manufacturer, model, ..), it is necessary to perform a HTTP GET request to obtain the given service description file.
  1. Live updates from devices (ssdp:alive, ssdp:goodbye)

Implementation

  • Potential upstream lib: async_upnp_client (@StevenLooman is the author and a homeassistant contributor, Apache 2.0 licensed)
  • Provides asynchronous discovery, interfaces to access the service description as raw XML
  • Requires adding a way to listen for multicast announcements for "new device" notifications

mDNS (multicast DNS)

How does it work

  1. UDP multicast on 5353 to request information about specific service type (or a list of all available service types)
  • Devices respond with their types, which can then be queried to obtain more information.
  1. Devices MUST announce when they come online
  • Can be used to listen for new devices, similarly to UPnP.

Implementation

  • python-aiozeroconf (based on netdisco's current upstream lib, LGPL-2.1)
  • Possibility to fetch all available service types
  • Does not seem to support defining a listener for all potential types, but should also be doable
    • We want to avoid creating listeners for specific types, and simply receive all available devices, parse them and compare to supported ones.

Appendix 2. current netdisco discoverables

  • This is a list of all currently (20181219) supported discoverables with the information they are looking for, including peculiarities.

mDNS

  • Apple TV: _appletv-v2._tcp.local.

    • Replaces \xa0 in name, needs to be moved into platform
  • Arduino: _arduino._tcp.local.

  • Axis: _axis-video._tcp.local.

    • Custom utf8 decoding, needs to be moved into platform
    • Custom host resolving, should be done by new discovery automatically
  • Bluesound: _musc._tcp.local.

  • Bose Soundtouch: _soundtouch._tcp.local.

  • ESPHome: _esphomelib._tcp.local.

  • Freebox: _fbx-api._tcp.local.

  • Google Cast: _googlecast._tcp.local.

  • HASS iOS: _hass-ios._tcp.local.

  • HASS: _home-assistant._tcp.local.

  • HomeKit: _hap._tcp.local.

    • Parses name out from hostname, needs to be moved to the platform
  • HP printers: _printer._tcp.local.

    • Device name starts with HP
  • Ikea Tradfri: _coap._udp.local.

  • Kodi: _http._tcp.local.

    • Device name starts with Kodi
  • LG Smart devices: _lg-smart-device._tcp.local.

  • Lutron: _lutron._tcp.local.

  • Nanoleaf Aurora: _nanoleafapi._tcp.local.

  • SABnzbd: _http._tcp.local.

    • Device name starts with SABnzbd on
  • Spotify connect: _spotify-connect._tcp.local.

  • TiVO DVR: _tivo-remote._tcp.local.

  • Volumio: _Volumio._tcp.local.

  • Xiaomi Gateway: _miio._udp.local.

    • Device name starts with lumi-gateway-
    • Custom attribute parsing due to broken protocol? Needs to be moved into the component
  • Yeelight: _miio._udp.local.

    • Device name starts with yeelink-light-
    • Custom parsing, needs to be moved to the platform

SSDP

  • Asus Router

    • deviceType: urn:schemas-upnp-org:device:InternetGatewayDevice:1
    • manufacturer: ASUSTeK Computer Inc.
  • Belkin Wemo: Belkin International Inc.

    • manufacturer: Belkin International Inc.
    • Reads mac address separately from entry, needs to be moved into platform
  • Cambridge Audio StreamMagic

    • deviceType: urn:schemas-upnp-org:device:MediaRenderer:1
    • manufacturer: Cambridge Audio
  • Canon printer

    • deviceType: urn:schemas-cipa-jp:device:DPSPrinter:1
    • manufacturer: CANON INC.
  • Deconz

  • DenonAVR

    • manufacturer: Denon
    • deviceType: urn:schemas-upnp-org:device:MediaRenderer:1
    • Separate parsing of presentationURL for hostname, should be done by the new discoverer automatically
  • DirectTV

    • manufacturer: DIRECTV
    • deviceType: urn:schemas-upnp-org:device:MediaServer:1
  • DLNA DMR

    • ST: urn:schemas-upnp-org:device:MediaRenderer:{1,3}
  • DLNA DMS

    • ST: urn:schemas-upnp-org:device:MediaServer:{1,4}
  • AVM Fritzbox

    • ST: urn:schemas-upnp-org:device:fritzbox:1
  • Frontier Silicon

    • ST: contains both fsapi and urn:schemas-frontier-silicon-com
  • Harmony

    • manufacturer: Logitech
    • deviceType: urn:myharmony-com:device:harmony:1
  • Huawei routers

    • manufacturer: Huawei Technologies Co., Ltd.
    • urn:schemas-upnp-org:device:InternetGatewayDevice:1
  • IGD

    • ST: urn:schemas-upnp-org:device:InternetGatewayDevice:{1,2}
  • Konnected Security devices

    • ST: urn:schemas-konnected-io:device:Security:1
  • Netgear router

    • manufacturer: NETGEAR, Inc.
    • deviceType: urn:schemas-upnp-org:device:InternetGatewayDevice:1
  • OctoPrint

    • manufacturer: The OctoPrint Project
  • OpenHome

    • ST: urn:av-openhome-org:service:Product:2
  • Panasonic Viera

    • ST: urn:panasonic-com:service:p00NetworkControl:1
  • Philips Hue

    • manufacturer: Royal Philips Electronics
    • manufacturerURL: http://www.philips.com
    • modelNumber: ["929000226503", "BSB002"]
  • Samsung printers

    • manufacturer: Samsung Electronics
    • deviceType: urn:schemas-upnp-org:device:Printer:1
  • Samsung TVs

    • ST: urn:samsung.com:device:RemoteControlReceiver:1
    • Manual [TV] prefix removal, needs to be moved to the platform
  • Songpal

    • ST: urn:schemas-sony-com:service:ScalarWebAPI:1
    • Checks for 'modelNumber' to ignore Bravia TVs
    • Parses custom elements from the service description XML file, a bit tricky.
  • Sonos

    • ST: urn:schemas-upnp-org:device:ZonePlayer:1
  • LG WebOS TV

    • deviceType: urn:schemas-upnp-org:device:Basic:1
    • modelName: LG Smart TV
  • Wink

    • ST: urn:wink-com:device:hub2:2, urn:wink-com:device:hub:2 and urn:wink-com:device:relay:2
  • Yamaha receivers

    • manufacturer: Yamaha Corporation
    • deviceType: urn:schemas-upnp-org:device:MediaRenderer:1
    • modelNumber: not 'N301'
    • Parses custom elements from the service description XML, a bit tricky.
  • Ziggo Mediabox XL

    • modelDescription: UPC Hzn Gateway
    • deviceType: urn:schemas-upnp-org:device:RemoteUIServer:2

Custom

  • Daikin
  • Logitech media server
  • Plex mediaserver
  • Tellstick
  • Xbox smartglass (should be detectable over SSDP, being a Microsoft protocol after all, move to custom discovery in component)

@fbradyirl
Copy link

fbradyirl commented Mar 7, 2019

Just wondering if a decision was ever finalised around this? Are PRs to Netdisco no longer allowed? I have a very standard PR open to discover Enigma2 based devices. Please let me know if this needs to be done in a different fashion. Thanks.

@tboyce021 tboyce021 mentioned this issue Mar 12, 2019
1 task
@balloob balloob closed this as completed Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants