hue: add support for username configuration option

[craigcabrey]

Description:
Allow a Hue bridge username to be specified in configuration.yaml. This removes the external dependency on the Phue configuration. The motivation here is to keep secrets all in secrets.yaml.

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

Example entry for configuration.yaml (if applicable):

light:
  platform: hue
  host: {{bridge address}}
  allow_unreachable: false
  username: !secret hue_username

Checklist:

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

• Documentation added/updated in home-assistant.io

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

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
• New dependencies have been added to the REQUIREMENTS variable (example).
• New dependencies are only imported inside functions that use them (example).
• New 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:

• 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.

This option will remove the need to specify an external configuration
(though this change doesn't stop you from doing that if you still like).

[mention-bot]

@craigcabrey, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @balloob and @rhooper to be potential reviewers.

[kocsenc]

Could be beneficial to add username in here? So it can give a more fruitful message for the user to fix the problem. 👍

[balloob]

Problem with outputting the username that it is actually an auth token (user+pass in 1), there is no password.

[pvizeli]

Add username option also to voluptuous

[pvizeli]

use component_setup from bootstrap

[craigcabrey]

component_setup() is scoped inside of from_config_dict() in homeassistant.bootstrap. Did you mean setup_component()?

[pvizeli]

👍

[pvizeli]

use component_setup from bootstrap

[pvizeli]

use component_setup from bootstrap

[pvizeli]

use component_setup from bootstrap

[pvizeli]

use component_setup from bootstrap

[pvizeli]

Have you test you code in your env? This code work only with you unittest but will not work on a real config in first state who had open the PR.

[craigcabrey]

@pvizeli I'm not sure what you mean. I'm running with these changes right now. username is None if it isn't specified in the config, which will just be passed to phue.Bridge (which is fine with a NoneType).

[craigcabrey]

@pvizeli can you provide any clarification? I'm eager to get this in mergable (mergeable?) shape 😄

[balloob]

Whilte testing your PR I realized that this one should actually be vol.Optional. Could you change that ?

[craigcabrey]

👍

[balloob]

Tested locally, code works with config files.

Check Travis logs, it actually found a bug 👍 Line 199 calls setup_bridge.

You can disable the pylint warning for too many arguments.

[craigcabrey]

Is setup_bridge not supposed to be called? It looks correct. Perhaps I'm misunderstanding the problem?

[balloob]

@craigcabrey The call to setup_bridge in line 199 does not incorporate the newly added username parameter.

[pvizeli]

Don't use hue.setup_platform in unitest. Use bootstrap.setup_component with a config dict. See on other tests how that work...

[craigcabrey]

@pvizeli I'd argue that those tests are not ideal. I'm looking at bootstrap.setup_component and there's far too much going on for it to be used as an entry point for a unit test.

It would essentially invalidate the unit test results -- there would be too much noise for very little signal. Unless you're thinking of these as integration tests, I'd strongly recommend keeping the tests scoped to as little code as possible.

However, if you and @balloob insist on this, then I'll be happy to oblige.

[pvizeli]

    def test_template(self):
        """Test template."""
        with assert_setup_component(1):
            assert setup_component(self.hass, 'sensor', {
                'sensor': {
                    'platform': 'template',
                    'sensors': {
                        'test_template_sensor': {
                            'value_template':
                                "It {{ states.sensor.test_state.state }}."
                        }
                    }
                }
            })

        state = self.hass.states.get('sensor.test_template_sensor')
        assert state.state == 'It .'

        self.hass.states.set('sensor.test_state', 'Works')
        self.hass.block_till_done()
        state = self.hass.states.get('sensor.test_template_sensor')
        assert state.state == 'It Works.'

    def test_template_syntax_error(self):
        """Test templating syntax error."""
        with assert_setup_component(0):
            assert setup_component(self.hass, 'sensor', {
                'sensor': {
                    'platform': 'template',
                    'sensors': {
                        'test_template_sensor': {
                            'value_template':
                                "{% if rubbish %}"
                        }
                    }
                }
            })
        assert self.hass.states.all() == []

[balloob]

The reason we have been migrating tests to use setup_comonent is that it verifies that the setup works correctly together with PLATFORM_SCHEMA. I'm also fine with adding 1 extra test that uses setup_component.

It also seems like you have 2 lint issues left. PyLint is a bit too strict here so you can disable them by adding this comment # pylint: disable=too-many-arguments

************* Module homeassistant.components.light.hue
R:111, 0: Too many arguments (6/5) (too-many-arguments)
R:184, 0: Too many arguments (6/5) (too-many-arguments)

[balloob]

This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it.