8000 Support splitting of input_boolean config by Danielhiversen · Pull Request #3564 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support splitting of input_boolean config #3564

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

Closed
wants to merge 6 commits into from

Conversation

Danielhiversen
Copy link
Member

Description:
Allow splitting of input_boolean config.
To day the example config only loads the input_boolean.test1 component, ant the input_boolean.test2 is not loaded. There is no error message in the log to tell that something is wrong.

I will do the same with input_select and input_slider, if this solution is accepted.

Related issue (if applicable): fixes #3065

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

Example entry for configuration.yaml (if applicable):

input_boolean:
  test1:
    name: Test1

input_boolean 2:
  test2:
    name: Test2

Checklist:
the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

for object_id, cfg in config[DOMAIN].items():
if not cfg:
cfg = {}
for _, p_config in config_per_platform(config, DOMAIN):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of config_per_platform, use extract_domain_configs (also in helpers package)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@kellerza
Copy link
Member
kellerza commented Oct 3, 2016

How do you handle duplicate entitie_ids?

@Danielhiversen
Copy link
Member Author

@kellerza Solved with generate_entity_id

@Danielhiversen
Copy link
Member Author

@balloob Any more comments on this?

@balloob
Copy link
Member
balloob commented Oct 5, 2016

I just realised that now we a) enforce that the DOMAIN is at least present and b) we only validate the config under DOMAIN key.

And so the problem with this is that we are going to get into weird territory, as this component is diverging from the usual config path: a component either has all config under it's DOMAIN or it supports multiple entries but it is also platform based.

This component fits neither structure now, hence we cannot leverage the automatic config validation.

We could hack around this by changing our CONFIG_SCHEMA to be a PLATFORM_SCHEMA. However this means that our config will now come in as a list of dictionaries under DOMAIN, ie:

{
  input_boolean: [
    { 'notify_home': { 'initial': True },
    { 'bla_di_bla': { 'initial': True },
  ]
}

So to continue with this PR:

  • Add a test with invalid config under a key that is not DOMAIN
  • Transform CONFIG_SCHEMA into PLATFORM_SCHEMA:
PLATFORM_SCHEMA = vol.Schema({
     cv.slug: vol.Any({
         vol.Optional(CONF_NAME): cv.string,
         vol.Optional(CONF_INITIAL, default=False): cv.boolean,
         vol.Optional(CONF_ICON): cv.icon,
     }, None)})
  • Update the code to be able to handle the above mentioned config format.

@kellerza
Copy link
Member
kellerza commented Oct 5, 2016

That is exactly what this commit did for the input_* components

It also raised a warning on duplicates, which is probably better than generating unique IDs, since input_* entities will always be used in automations and creating a random name will only hide config errors

@balloob
Copy link
Member
balloob commented Oct 5, 2016

Oh @kellerza you're so right. My bad.

I'm btw still not convinced that the value we gain is more than the complexity we add.

@kellerza
Copy link
Member
kellerza commented Oct 5, 2016

That is exactly what swayed me not to push the argument in #3256 - it does become a bit complex 😄

@balloob
Copy link
Member
balloob commented Oct 13, 2016

I'm closing this as it's not worth the possible confusion for the user.

@balloob balloob closed this Oct 13, 2016
@kellerza kellerza mentioned this pull request Oct 14, 2016
8 tasks
@Danielhiversen Danielhiversen deleted the input_boolean_fix branch November 2, 2016 16:48
@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
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 this pull request may close these issues.

Failing to load second input_boolean components
3 participants
0