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

RFC: Config Manager #11947

Closed
balloob opened this issue Jan 27, 2018 · 28 comments · Fixed by #12079
Closed

RFC: Config Manager #11947

balloob opened this issue Jan 27, 2018 · 28 comments · Fixed by #12079

Comments

@balloob
Copy link
Member

balloob commented Jan 27, 2018

Sorry, long RFC

Goal: allow people to configure components from the frontend without having to touch any text file.
Subgoal: have discovery use this system so people can disable certain things from being discovered again or configure discovered items.

TL;DR: New helper config_manager. Components can declare config handlers that help in creation of config data. The helper will store the config data outside of configuration.yaml. HASS will setup known config pieces before processing normal config.


So I've been thinking about a config manager to power a config UI for Home Assistant. For a long time I thought that a config manager for Home Assistant should be build on top of configuration.yaml. But after thinking a bit more, I realized that it's going to make things too complicated and messy. Especially as configuration.yaml has been overly complicated with packages, !include etc etc.

I've approached the design by looking at the following component configuration use cases:

  1. The Nest component requires a 2 step configuration. First step is to add client_id, client_secret. Second step requires auth linking by following a link to the Nest.com website and copying a pin code into Home Assistant.
  2. Hue bridge is automatically discovered. After discovery, the user has to press a button on the bridge to authorize Home Assistant to create a user in the next 30 seconds.
  3. Chromecast is automatically discovered and does not require authorization.

Scope of this RFC:

  • Allow adding configuration for components
  • Allow removing configuration for components
  • Allow discovering configuration for components
  • Allow disabling configuration (so that a re-discovery won't add the config again)
  • Allow removing configuration
  • Allow support for future migrations
  • Entities will contain the config entry ID that they are part of.

Out of scope for MVP:

  • Editing config
  • Any config that is not required to get a component up and running (because no editing yet).

My idea is to achieve this with a new helper: homeassistant.helpers.config_manager. It will allow components to register config handlers by extending the ConfigBase class.

Hass will:

  • Will load stored configs before processing configuration.yaml. This will allow things like entity IDs to exist before automations etc are loaded. Setting up components with stored configs is done with two new methods:
async def async_setup_config(hass, config_info):
    pass

# Optional. Otherwise note to user that removing an config requires a restart.
async def async_unload_config(hass, config_info):
    pass

Not 100% yet what else we want to pass to these methods. Maybe a component config (also stored in config manager, not from configuration.yaml)

homeassistant.helpers.config_manager is responsible for:

  • Storing config data in a file
  • Maintaining a registry of configh andlers
  • Routing user requests to the right handlers.

Config handlers (like HueConfigHandler) are responsible for:

  • Returning config pages to show to the user. Pages consist of a title, introduction (markdown), voluptuous schema for expected data, current step, total number of steps.
  • Handling user input for a config page
  • Handling discovery input

When handling the user input, the config handler can choose to:

  • return the same page again with error messages
  • return the next page
  • create config and finish the process

Time for some code

I've been tweaking design by writing code and see if it would make sense. Here is code that would work for Nest, Hue and Chromecast.

1. Nest

The Nest component requires a 2 step configuration. First step is to add client_id, client_secret. Second step requires auth linking by following a link to the Nest.com website and copying a pin code into Home Assistant.

import voluptuous as vol
from homeassistant.helpers import config_manager

@config_manager.config_handler('nest')
class NestConfig(config_manager.ConfigBase):
    # Future versions of config might require a migration
    VERSION = 1

    def __init__(self):
        # Data from step 0 that we'll store.
        self.client_data = None

    async def async_handle(self, step_number, user_input):
        """
        Handle a step in the add config process.
        step_number: the step that we're on.
        user_input: input received from user for the step.
        
        If user_input is None, return form for that step.
        """
        if step_number == 0:
            if user_input is not None:
                self.client_data = user_input
                return (await self.async_handle(1))

            return self.show_form(
                step_number=0,
                total_steps=2,
                title='Client information',
                introduction='Some Markdown introduction',
                schema=vol.Schema({
                    # Translation keys are:
                    #   components.<comp name>.create_config.<step>.<key name>.caption
                    #   components.<comp name>.create_config.<step>.<key name>.description
                    # In this case: components.nest.create_config.0.client_id.caption
                    vol.Required('client_id'): cv.string,
                    vol.Required('client_secret'): cv.string,
                })
            )

        if step_number == 1:
            if user_input is not None:
                import nest
                # Validate auth code and get access token
                token_info = await nest.async_get_credentials(
                    self.client_data['client_id'],
                    self.client_data['client_secret'],
                    user_input['auth_code'])
                user = await nest.async_get_user(token_info['access_token'])
                return self.create_config(
                    title=user['name'],
                    data=token_info
                )

            return self.show_form(
                step_number=1,
                total_steps=2,
                title='Authorize account',
                introduction='Markdown link to oauth flow',
                schema=Vol.Schema({
                    vol.Required('auth_code')
                })
            )

2. Hue

Hue bridge is automatically discovered. After discovery, the user has to press a button on the bridge to authorize Home Assistant to create a user in the next 30 seconds.

@config_manager.config_handler('hue')
class HueConfig(config_manager.ConfigBase):
    VERSION = 1

    def __init__(self):
        self.host = None
        self.via_discovery = False

    async def async_handle(self, step_number, user_input):
        if step_number == 'DISCOVERY':
            self.via_discovery = True

            # Now user_input = discovery info.
            self.host = user_input['host']

            # Get current Hue accounts to see if we know it
            current_accounts = self.async_get_accounts(include_disabled=True)

            existing = any(account.data['host'] == self.host
                           for account in current_accounts)

            # Do nothing
            if existing:
                return None

            # Show form for step 1
            return (await self.async_handle(1))

        # Manually started add Hue account
        if step_number == 0:
            if user_input is not None:
                self.host = user_input['host']

                # Show form for step 1
                return (await self.async_handle(1))

            hosts = await async_get_hue_hosts()

            return self.show_form(
                step_number=0,
                total_steps=2,
                title='Pair with the Hue Bridge',
                introduction='Enter IP address of your Hue host.',
                schema=vol.Schema({
                    vol.Required('host'): vol.In(hosts)
                })
            )

        if step_number == 1:
            errors = {}
            if user_input is not None:
                import phue

                bridge = await async_get_auth(self.host)

                # Auth succeeded
                if bridge is not None:
                    return self.create_config(
                        title=bridge['name'],
                        source='discovery' if self.via_discovery else 'user',
                        data=bridge
                    )

                # auth not succeeded, we will show the form again.
                errors['base'] = 'Unable to authenticate. Try again.'

            return self.show_form(
                step_number=0,
                total_steps=1 if self.via_discovery else 2,
                title='Pair with the Hue Bridge',
                introduction='Press the button on your bridge, then press submit',
                errors=errors,
            )

3. Chromecast

Chromecast is automatically discovered and does not require authorization.

@config_manager.config_handler('cast')
class CastConfig(config_manager.ConfigBase):
    VERSION = 1
    INVOKE_MANUAL = False

    async def async_handle(self, step_number, user_input):
        # Always triggered via discovery.
        if step_number != 'discovery':
            return None

        # Get current Cast accounts to see if we know it
        current_accounts = self.async_get_accounts(include_disabled=True)

        existing = any(account.data['serial'] == user_input['serial']
                        for account in current_accounts)
        
        if existing:
            return None

        return self.create_config(
            title=user_input['cast_name'],
            source='discovery',
            data=user_input
        )

Migrations

Migration support would be easy to add. Each config handler has a VERSION const that we store together with the config entry. When we are going to setup components with stored config, we can see if the stored version is the same as the current config handler version.

If not, call a migration method on the config handler:

def async_migrate_config(self, config_version, config_info):
    if config_version < 2:
        config_info['host'] = config_info.pop('ip')

    return config_info

Discovery

Discovery entries will go to the config handler. If the config handler generates a form to show, the frontend will be notified (TBD).

When an config entry is created from discovery, we should set the source to discovery. That way we can indicate to the user that it was automatically discovered and that it should disable, not remove the config entry if it doesn't want to see it in Home Assistant anymore. By disabling, it will exist but not be loaded. If deleted, a new run of discovery will just add it again.

Notes

  • To generate a form from a voluptuous schema we're going to use voluptuous_form
  • For things like Z-Wave we can attach some "device finder" logic to easily populate a dropdown with available USB devices

Full stack:

Some interaction examples from user interaction to interacting with the config handler inside a component.

All interactions are done via a new section in the config panel. It will list all current configuration entries and has an "Add Integration" button. Because entities contain the config entry that they are part of as part of their attributes, we can show the entities that are part of each config entry.

Adding an config entry

  • Config panel in frontend will query API for available components to configure: /api/config/types
  • User clicks "Add Nest" button
  • Config panel will fetch first step of config entry creation by doing POST /api/config/entry. This will create a temporary entry id. Under the hood the config manager will instantiate an instance of NestConfigHandler and calls NestConfigHandler.async_handle(0). This instance is kept for the duration of the config process so that the backend can track state.
  • User is presented with the first page of the Nest config wizard. Fills in data and clicks SUBMIT. This info is posted to /api/config/entry/<id>
  • The user input is passed to NestConfigHandler.async_handle(0, user_input) which returns a new form to show.
  • User fills in second form, clicks SUBMIT. This info is posted to /api/config/entries/<id>
  • The user input is passed to NestConfigHandler.async_handle(1, user_input) which returns self.create_config. The config manager will store this config entry, call homeassistant.components.nest.async_setup_config(hass, config_info) and return "form done" command.
  • UI shows a success message

Removing a config entry

  • Config panel in frontend will query API for component configurations: /api/config/entries/ and show it as list to the user.
  • User clicks delete on an entry. This info is then submitted to API: DELETE /api/config/entries<id>
  • Config manager will remove the config entry and call homeassistant.components.nest.async_unload_config(hass, config_info) if available.
  • User is shown success message. If async_unload_config did not exist, show a message that the user has to restart Home Assistant for the integration to be unloaded.
@tschmidty69
Copy link
Contributor

tschmidty69 commented Jan 27, 2018

This is definitely needed to get hassio to reach it's goals. A couple comments and questions:

  • Account as a name seems a bit confusing to me. No good suggestions but maybe autoconfig or config_manager_entity? (it's late)

  • File this data is stored in should still remain human editable, maybe config_manger/platform_entity_id.yaml for each instance? Of course could be deliminated in the yaml file itself, but the goal of keeping it editable is a worthy one and I like file separation when it makes sense.

  • Api should be fleshed out enough that configuration can be done (almost) completely through REST calls. Particularly for Hassio an addon should be able to create entities, ie. a dark_sky addon could create weather or sensor component, AWS TTS addon could configure AWS TTS, etc. This would be huge for allowing Hassio addons to "just work"

  • I am not a UI person, and I don't think we need to make it hand-holdy, but this would need to be a pretty prominent part of the UI and fairly well thought out.

  • Where do we have these handlers live? To me it seems like the best place is in each individual component. Could return a 'not implemented' if a handler function isn't defined, but in general this should be generalized to the point where every component handles it's own config.

I am definitely up for helping with the grunt work of adding handlers to components. Per my last point, this would be a lot of modifications (ala lazy service loading) but on a bigger scale. Hmm, on second thought let's make @amelchio do it!

@balloob
Copy link
Member Author

balloob commented Jan 27, 2018

My bad on the account part, that is indeed confusing. I started out naming it account manager but then at the end decided it should be a more generic "Config Manager". I forgot to update most examples. Will do so now.

File will be stored as a single YAML file and kept in memory afterwards.

This is not targeted at Hass.io. This is meant for components inside Home Assistant. Hass.io needs to figure out their own patterns. Discussion here

I am a UI person. The idea is to be able to generate forms from voluptuous schemas.

Handlers will live in each component. We don't have to "upgrade" everything. We can just upgrade as we go. Once we have done a couple different variations, we can see if the API is solid or not. I wouldn't want to scan for handlers in the source, as it requires each file to be loaded into memory and Python doesn't forget. Instead I was thinking about initially just hardcoding the list (Hue, Cast, Wemo) and later we can migrate to something similar to script/gen_requirements_all

@Kane610
Copy link
Member

Kane610 commented Jan 27, 2018

This looks great! Will you allow modification of already configured component or would it require a remove + add?

@balloob
Copy link
Member Author

balloob commented Jan 27, 2018

Initially remove + add. Edit can be added later.

@DubhAd
Copy link
Contributor

DubhAd commented Jan 27, 2018

From a file system perspective, it would be good to create a sub-directory for all these configuration files to live in. This will stop the top level folder becoming (even more) horribly cluttered with lots of component configuration files.

@point-4ward
Copy link
Contributor

^would also make the .gitignore file a little easier to make sure you don't upload the faff.

Just be careful to warn people in case they already have a folder by that name so they can rename it before hand.

@balloob balloob mentioned this issue Jan 28, 2018
6 tasks
@abmantis
Copy link
Contributor

I'm glad to see this being worked on! This is a great advance for HA!
Any plans on how to deal with "secret" stuff like passwords, which we usually place on secrets.yaml?

@balloob
Copy link
Member Author

balloob commented Jan 30, 2018

Initially we would focus on storing account info in the config manager. In that case it is all secrets.

What people like to share is automations, which for now will not be part of this.

@emlove
Copy link
Contributor

emlove commented Jan 31, 2018

Finally got a chance to read this, some notes:

  1. In the workflow, step 0 is first called to get the initial form for the user. When the user data is returned to the backend the next call should be step 1 instead of step 0. async_handle(1, user_input), so we're not reusing the step numbers for different purposes. The return (await self.async_handle(1)) is telling that something's up. Rewrote the nest example as this would exist below.
  2. I imagine this isn't the final design, but rather than step_number='DISCOVERY', we should have a discovery_info parameter that defaults to None.
  3. For the translation keys, the next thing that's still on the translation roadmap is a home assistant registration / API for the backend to send platform-specific translation keys to the frontend. We shouldn't add these configuration strings to the polymer translation files, since that wouldn't be great for the final design. We might want to try and get that infrastructure in place first though, since we then wouldn't have any legacy configuration code to worry about that doesn't support localization.
  4. We should optionally allow errors to be associated with one of the fields on the form. This would let the frontend style the fields / show the message in an appropriate location. It would at least be nice to keep this in mind so we have room in the protocol to add this later.
  5. Is self.create_config returning an object that represents the instruction to create the config, or is the config object the result of calling create_config? If the former we might want to just call it save_config to make the function name more flexible for when editing is introduced.
  6. What's the decorator on these objects? Are they not namespaced by their component domains?
  7. We should probably think about component search in the config panel from the start. We already know we have an unmanageable number of integrations for just scrolling. Even if it's a dead simple search API that could be improved later.
  8. I'd prefer description instead of introduction. Once you get a few steps deep it's not an introduction any more. 😄
import voluptuous as vol
from homeassistant.helpers import config_manager

@config_manager.config_handler('nest')
class NestConfig(config_manager.ConfigBase):
    # Future versions of config might require a migration
    VERSION = 1

    async def async_handle(self, step_number, user_input):
        """
        Handle a step in the add config process.
        step_number: the step that we're on.
        user_input: input received from user for the step.
        
        If user_input is None, return form for that step.
        """
        if step_number == 0:
            return self.show_form(
                step_number=0,
                total_steps=2,
                title='Client information',
                introduction='Some Markdown introduction',
                schema=vol.Schema({
                    # Translation keys are:
                    #   components.<comp name>.create_config.<step>.<key name>.caption
                    #   components.<comp name>.create_config.<step>.<key name>.description
                    # In this case: components.nest.create_config.0.client_id.caption
                    vol.Required('client_id'): cv.string,
                    vol.Required('client_secret'): cv.string,
                })
            )

        if step_number == 1:
            self.client_data = user_input
            return self.show_form(
                step_number=1,
                total_steps=2,
                title='Authorize account',
                introduction='Markdown link to oauth flow',
                schema=Vol.Schema({
                    vol.Required('auth_code')
                })
            )

        if step_number == 2:
            import nest
            # Validate auth code and get access token
            token_info = await nest.async_get_credentials(
                self.client_data['client_id'],
                self.client_data['client_secret'],
                user_input['auth_code'])
            user = await nest.async_get_user(token_info['access_token'])
            return self.create_config(
                title=user['name'],
                data=token_info
            )

@emlove
Copy link
Contributor

emlove commented Jan 31, 2018

Also, another use case to consider. The Automatic device tracker. Currently, after specifying the client id / client secret, we direct the user to an OAuth URL. When the user is authenticated, Automatic redirects them back to home assistant, and the required key is posted with that redirect and saved internally by hass, requiring no further action from the user, as opposed to the nest case where a key needs to be copied/pasted.

@tschmidty69
Copy link
Contributor

tschmidty69 commented Jan 31, 2018

All great points of course from armills. Couple thoughts on steps.

Rather than directly declaring step_numbers this would seem to best be abstracted by const declarations and a next_step and maybe last_step which would allow us to more easily add steps.

Within the context we know what steps we need. But if an app required/supported oauth for instance, we could query what steps are needed at the beginning and show appropriate text at the start of the config, ie. don't let them keep failing oauth if they don't have the correct config to support CONFIG_MGR_OAUTH_INIT. Would also give some definition to what each step was meant to accomplish within the code.

As to armills point on translation, these could probably be used as a basis for a generic translation for form introduction/description. With field specific descriptions/errors this would seem to cover most of that need.

CONSTS

CONFIG_MGR_DISCOVERY = 0
CONFIG_MGR_INIT = 1 # this and discovery being two different cases
CONFIG_MGR_DISCOVERY = 2
CONFIG_MGR_TIMED_HUE_AUTH = 3 
CONFIG_MGR_OATH_INIT = 3 
CONFIG_MGR_OATH_RESP = 4
CONFIG_MGR_PRESS_A_BUTTON = 5
CONFIG_MGR_COMPLETE = 6
CONFIG_MGR_SOME_OTHER_STEP = 7

NEST

    async def async_handle(self, step, user_input):
        if step == CONFIG_INIT:
            return self.show_form(
                next_step=CONFIG_MGR_OAUTH_INIT,
                last_step=CONFIG_MGR_OAUTH_RESP

        if step == CONFIG_MGR_OAUTH_INIT:
            self.client_data = user_input
            return self.show_form(
                next_step=CONFIG_MGR_OAUTH_RESP,

        if step == CONFIG_MGR_OAUTH_RESP:
             if error: 
                  next_step = CONFIG_MGR_OAUTH_INIT
                  self.async_handle()
             if step = last_step
             finish()

HUE

    async def async_handle(self, step, user_input):
        if step == CONFIG_MGR_DISCOVERY:
            next_step = CONFIG_MGR_TIMED_HUE_AUTH
            last_step = CONFIG_MGR_COMPLETE
            discover()

        if stepr == CONFIG_MGR_TIMED_HUE_AUTH:
             next_steps=CONFIG_MGR_COMPLETE

        if stepr == CONFIG_MGR_COMPLETE:
             if step == last_step:
             finish()

CAST

    async def async_handle(self, step, user_input):
        if step == CONFIG_DISCOVERY:
        finish()

@balloob
Copy link
Member Author

balloob commented Jan 31, 2018

Good point about the steps. Your code is for sure cleaner.

Let me try to clarify my reasoning a bit. The return (await self.async_handle('auth_account')) should be seen as a redirect: we are done with the data, now show this step. We want to know from which step the data is coming from, as there might be multiple entry points to "start a configure flow". (cloud, oauth2_client, discovery, user). Also, passing in data might still mean that we don't go to the next step if an error occurs (ie invalid password).

The step_number parameter should have been named step_id instead.

Besides DISCOVERY I also expect CLOUD to be a possible "step" for configuration. Take your Automatic example:

  • User is logged into Cloud
  • Clicks on "link Automatic account" and starts oauth2 flow with Cloud client id
  • OAuth flow finishes and redirects to Cloud which triggers the message to be send via the cloud to the instance
  • Instance handles the incoming auth and finishes the config flow

Another potential step_id could be a generic oauth2 client that one could integrate into HASS and handle redirect at the instance.


Yes about errors, translations.


create_config would return an object that represents the action. It's just a helper to make sure we include flow_id etc. The actual processing of the action happens in ConfigManager class, not ConfigHandler.


Decorator is to register the ConfigHandler so we know which ConfigHandlers are available. Otherwise all components will need to call it the same thing (as we do for async_setup etc).

I also thought we would use the decorator for discoverability but realized that it won't show up unless that file is loaded, and we're not going to load all components. Instead will have to end up generating a list of components that support the Config Manager.


Yes to search and description.

@balloob
Copy link
Member Author

balloob commented Jan 31, 2018

Actually wonder if description is even necessary? It's going to be replaced by translation anyway. (not sure how we should do that with markdown though, in case we want to include link/image)

@tschmidty69
Copy link
Contributor

Ugh let me lazily retype this after some errant keystroke turned this into a blank page...

So I think functions should accept an error class that can contain the step and fields causing errors (since vol.form can't handle all cases). Show form already accepts it but this would allow us to retry the form.

  async def async_handle(self, step_id, user_input, *error):
     if step_id == CONFIG_MGR_OAUTH_INIT:
        if error. step_id == ERROR_CONFIG_MGR_OAUTH_INIT:
          return self.show_form(params, error=error) ### Error would contain a form/step specific error as
                                                      well as fields
        continue_func...

I think with how much we have to do with translations, we should just make sure we aren't backed into a corner but shouldn't worry beyond that (much). Abstracting these errors and steps to a meaningful const will allow us to translate it at the frontend. Adding beyond that means fundamentally adding a lang global and all that. With this we could have the frontend display hass.translation['ERROR_CONFIG_MGR_OAUTH_INIT'] down the road.

@emlove
Copy link
Contributor

emlove commented Jan 31, 2018

Ah, I think I see where you're going with the step_number/step_id. What if we have show_form include an id parameter for the step, which lets us bake the routing in to the form, without requiring an extra hop for platform authors. This should retain the necessary flexibility.

import voluptuous as vol
from homeassistant.helpers import config_manager

STEP_CLIENT_INFO = 'client_info'
STEP_OAUTH = 'oauth'

@config_manager.config_handler('nest')
class NestConfig(config_manager.ConfigBase):
    # Future versions of config might require a migration
    VERSION = 1

    def __init__(self):
        # Data from step 0 that we'll store.
        self.client_data = None

    async def async_handle(self, source_step_id, user_input):
        """
        Handle a step in the add config process.
        source_step_id: the step that originated the request.
        user_input: input received from user for the step.
        
        If user_input is None, return form for that step.
        """
        if source_step_id is None:  # Or whatever core constant gets used
            return self.show_form(
                step_id=STEP_CLIENT_INFO
                step_number=1,
                total_steps=2,
                title='Client information',
                introduction='Some Markdown introduction',
                schema=vol.Schema({
                    # Translation keys are:
                    #   components.<comp name>.create_config.<step>.<key name>.caption
                    #   components.<comp name>.create_config.<step>.<key name>.description
                    # In this case: components.nest.create_config.0.client_id.caption
                    vol.Required('client_id'): cv.string,
                    vol.Required('client_secret'): cv.string,
                })
            )

        if source_step_id is STEP_CLIENT_INFO:
            self.client_data = user_input
            return self.show_form(
                step_id=STEP_OAUTH,
                step_number=2,
                total_steps=2,
                title='Authorize account',
                introduction='Markdown link to oauth flow',
                schema=Vol.Schema({
                    vol.Required('auth_code')
                })
            )

        if source_step_id is STEP_OAUTH:
            import nest
            # Validate auth code and get access token
            token_info = await nest.async_get_credentials(
                self.client_data['client_id'],
                self.client_data['client_secret'],
                user_input['auth_code'])
            user = await nest.async_get_user(token_info['access_token'])
            return self.create_config(
                title=user['name'],
                data=token_info
            )

@balloob
Copy link
Member Author

balloob commented Jan 31, 2018

@armills how would we do error handling in that case?

if source_step_id is STEP_CLIENT_INFO:
    is_valid = yield from validate_client_info(user_input)
    if not is_valid:
        # We want to show form for STEP_CLIENT_INFO

I think that getting the data back to the step that rendered the form is the most clear. It makes no assumptions and the current step can decide what is next (with steps based on the just given input).


Btw just had a thought about translations. For a lot of things we should be able to get standardized translations (username, password, email etc)

@balloob
Copy link
Member Author

balloob commented Jan 31, 2018

Just had an insight.

What if each step got its own method, dynamically called.

@asyncio.coroutine
def async_handle_client_info(self, user_input=None):

@asyncio.coroutine
def async_handle_discovery(self, input):

@asyncio.coroutine
def async_handle_oauth(self, input):

We would still do the redirect stuff of returning another step.

@balloob
Copy link
Member Author

balloob commented Jan 31, 2018

I put out a prototype of this RFC up in #12079. Have been hacking at it for a couple of days. Have taken most comments into consideration. Nothing is set in stone yet but since there are so many moving pieces, I wanted to code some things together so I can start looking at it from the frontend point.

@emlove
Copy link
Contributor

emlove commented Jan 31, 2018

I think the routing would certainly be easier to read if they were named methods. In either case platforms could always create their own internal methods to return the show_form objects for each stage so they could better delegate them internally.

Here's another idea for brainstorming: What if these configuration methods were coroutines that returned the user input once it's received? This would let us take advantage of async magic to write the routing rules using normal flow control patterns, which might be easier to grok for most contributors. Example:

import voluptuous as vol
from homeassistant.helpers import config_manager

@config_manager.config_handler('nest')
class NestConfig(config_manager.ConfigBase):
    # Future versions of config might require a migration
    VERSION = 1

    async def async_handle(self, discovery_data=None):
        """
        Handle a step in the add config process.
        discovery_data: Additional information if this was initiated by discovery
        """
        client_data = None
        while client_data is None:
            user_input = await self.show_form(
                step_id=STEP_CLIENT_INFO
                step_number=1,
                total_steps=2,
                title='Client information',
                introduction='Some Markdown introduction',
                schema=vol.Schema({
                    # Translation keys are:
                    #   components.<comp name>.create_config.<step>.<key name>.caption
                    #   components.<comp name>.create_config.<step>.<key name>.description
                    # In this case: components.nest.create_config.0.client_id.caption
                    vol.Required('client_id'): cv.string,
                    vol.Required('client_secret'): cv.string,
                })
            )

            if is_valid(user_input):
                client_data = user_input

        user = None
        while auth_code is None:
            user_input = await self.show_form(
                step_id=STEP_OAUTH,
                step_number=2,
                total_steps=2,
                title='Authorize account',
                introduction='Markdown link to oauth flow',
                schema=Vol.Schema({
                    vol.Required('auth_code')
                })
            )

            import nest
            # Validate auth code and get access token
            token_info = await nest.async_get_credentials(
                client_data['client_id'],
                client_data['client_secret'],
                user_input['auth_code'])
            user = await nest.async_get_user(token_info['access_token'])

        # This could possibly also be a coroutine if we wanted to allow final
        # cleanup afterwards, although it's probably best to enforce
        # create_config as the termination of async_handle
        return self.create_config(
            title=user['name'],
            data=token_info
        )

@balloob
Copy link
Member Author

balloob commented Feb 2, 2018

I thought more about putting in 1 method but decided I don't like it. Main downside is that based on a step, we might want to show different config options. It gets pretty confusing code if that is all tied together with if…else commands.

@tschmidty69
Copy link
Contributor

One other thought I had while riding, is adding a property in the config manager for 'created_by' or 'owner' with values like discovery, user, component_x. It is a bit down the road, but the thought being that if a component created several entities in the config_manager and was subsequently removed it would allow us to track other entities that depended on it.

@balloob
Copy link
Member Author

balloob commented Feb 3, 2018

Demo video of the config manager in action

Hue works completely: create entry -> loads Hue. Restart will load entry again.
Nest only saves entry, doesn't use entry yet for setup.

Backend PR: #12079
Frontend PR: home-assistant/frontend#861

@balloob
Copy link
Member Author

balloob commented Feb 6, 2018

The code is finished now for both frontend and backend PR (wrapping up tests as we speak)

I've added an experimental and temporary component config_entry_example.py. We can remove this file when we start adding real config flows. For now it's a great example how it will work.

The functionality will not be exposed to users yet. To experiment with it, update your config:

config:
  config_entries: true

The reason I am this cautious is that we need more experimentation to see if the API needs more tweaking.

@balloob
Copy link
Member Author

balloob commented Feb 6, 2018

Here is an architecture diagram of how it all works together:

home assistant config manager architecture

https://docs.google.com/drawings/d/10ESEPcVplCzcBg7_Ir4bpgka5YjVbtBRnsebetYkpxA/edit?usp=sharing

@c727
Copy link
Contributor

c727 commented Feb 6, 2018

In the openhab "config manager" you can also manage your lamps. not only the hub, that way. Not sure if this is also planned here but I think this is the better concept as the user has a consistent entity manager and no separation between hub-based and independent gadgets.

oh

@balloob
Copy link
Member Author

balloob commented Feb 6, 2018

Here is a video of the config manager in action https://youtu.be/_qpKe-FW9Yk

@balloob
Copy link
Member Author

balloob commented Feb 6, 2018

@c727 the idea is that entities will know to which config entry they belong. So then we can show those together.

@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants