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

Use voluptuous for input_slider, input_boolean, input_select #3256

Merged
merged 9 commits into from
Sep 23, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Use voluptuous for input slider
  • Loading branch information
kellerza committed Sep 20, 2016
commit 7936db647394259d8fde2938b6b55d46944f9b43
11 changes: 7 additions & 4 deletions homeassistant/components/input_boolean.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@
vol.Optional(ATTR_ENTITY_ID): cv.entity_ids,
})

PLATFORM_SCHEMA = vol.Schema({
cv.slug: {
vol.Optional(CONF_NAME): cv.string,
vol.Optional(CONF_INITIAL, default=False): cv.boolean,
vol.Optional(CONF_ICON): cv.icon,
Copy link
Member

Choose a reason for hiding this comment

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

CONF_NAME and CONF_ICON are present in const.py.

}}, required=True)


def is_on(hass, entity_id):
"""Test if input_boolean is True."""
Expand All @@ -53,10 +60,6 @@ def toggle(hass, entity_id):

def setup(hass, config):
"""Set up input boolean."""
if not isinstance(config.get(DOMAIN), dict):
_LOGGER.error('Expected %s config to be a dictionary', DOMAIN)
return False

component = EntityComponent(_LOGGER, DOMAIN, hass)

entities = []
Expand Down
33 changes: 13 additions & 20 deletions homeassistant/components/input_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.entity_component import EntityComponent
from homeassistant.util import slugify

DOMAIN = 'input_select'
ENTITY_ID_FORMAT = DOMAIN + '.{}'
Expand All @@ -33,6 +32,14 @@
vol.Required(ATTR_OPTION): cv.string,
})

PLATFORM_SCHEMA = vol.Schema({
cv.slug: {
vol.Optional(CONF_NAME): cv.string,
vol.Required(CONF_OPTIONS): vol.All(cv.ensure_list, [cv.string]),
Copy link
Member

Choose a reason for hiding this comment

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

We no longer validate that there is at least 1 option. Add vol.Length(1)

vol.Optional(CONF_INITIAL): cv.string,
vol.Optional(CONF_ICON): cv.icon,
}}, required=True)


def select_option(hass, entity_id, option):
"""Set input_select to False."""
Expand All @@ -44,36 +51,22 @@ def select_option(hass, entity_id, option):

def setup(hass, config):
"""Setup input select."""
if not isinstance(config.get(DOMAIN), dict):
_LOGGER.error('Expected %s config to be a dictionary', DOMAIN)
return False

component = EntityComponent(_LOGGER, DOMAIN, hass)

entities = []

for object_id, cfg in config[DOMAIN].items():
if object_id != slugify(object_id):
_LOGGER.warning("Found invalid key for boolean input: %s. "
"Use %s instead", object_id, slugify(object_id))
continue
if not cfg:
_LOGGER.warning("No configuration specified for %s", object_id)
continue

name = cfg.get(CONF_NAME)
options = cfg.get(CONF_OPTIONS)

if not isinstance(options, list) or len(options) == 0:
_LOGGER.warning('Key %s should be a list of options', CONF_OPTIONS)
continue

options = [str(val) for val in options]

state = cfg.get(CONF_INITIAL)

if state not in options:
print(options)
print(cfg)
Copy link
Member

Choose a reason for hiding this comment

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

Leftovers

state = options[0]
if state is not None:
_LOGGER.warning('Initial state %s is not in the options %s',
state, ','.join(options))

icon = cfg.get(CONF_ICON)

Expand Down
26 changes: 12 additions & 14 deletions homeassistant/components/input_slider.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.entity_component import EntityComponent
from homeassistant.util import slugify

DOMAIN = 'input_slider'
ENTITY_ID_FORMAT = DOMAIN + '.{}'
Expand All @@ -37,6 +36,17 @@
vol.Required(ATTR_VALUE): vol.Coerce(float),
})

PLATFORM_SCHEMA = vol.Schema({
cv.slug: {
vol.Optional(CONF_NAME): cv.string,
vol.Optional(CONF_MIN): vol.Coerce(int),
vol.Optional(CONF_MAX): vol.Coerce(int),
vol.Optional(CONF_INITIAL): vol.Coerce(int),
vol.Optional(CONF_STEP, default=1): vol.Coerce(int),
Copy link
Member

Choose a reason for hiding this comment

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

I guess that we can go with cv.positive_int here.

Copy link
Member Author

Choose a reason for hiding this comment

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

On second glance, they must all be float

vol.Optional(CONF_ICON): cv.icon,
vol.Optional(ATTR_UNIT_OF_MEASUREMENT): cv.string
}}, required=True)


def select_value(hass, entity_id, value):
"""Set input_slider to value."""
Expand All @@ -48,28 +58,16 @@ def select_value(hass, entity_id, value):

def setup(hass, config):
"""Set up input slider."""
if not isinstance(config.get(DOMAIN), dict):
_LOGGER.error('Expected %s config to be a dictionary', DOMAIN)
return False

component = EntityComponent(_LOGGER, DOMAIN, hass)

entities = []

for object_id, cfg in config[DOMAIN].items():
if object_id != slugify(object_id):
_LOGGER.warning("Found invalid key for boolean input: %s. "
"Use %s instead", object_id, slugify(object_id))
continue
if not cfg:
_LOGGER.warning("No configuration specified for %s", object_id)
continue

name = cfg.get(CONF_NAME)
minimum = cfg.get(CONF_MIN)
maximum = cfg.get(CONF_MAX)
state = cfg.get(CONF_INITIAL, minimum)
step = cfg.get(CONF_STEP, 1)
step = cfg.get(CONF_STEP)
icon = cfg.get(CONF_ICON)
unit = cfg.get(ATTR_UNIT_OF_MEASUREMENT)

Expand Down
22 changes: 8 additions & 14 deletions tests/components/test_input_boolean.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# pylint: disable=too-many-public-methods,protected-access
import unittest

import voluptuous as vol

from homeassistant.components import input_boolean
from homeassistant.const import (
STATE_ON, STATE_OFF, ATTR_ICON, ATTR_FRIENDLY_NAME)
Expand All @@ -22,20 +24,12 @@ def tearDown(self): # pylint: disable=invalid-name

def test_config(self):
"""Test config."""
self.assertFalse(input_boolean.setup(self.hass, {
'input_boolean': None
}))

self.assertFalse(input_boolean.setup(self.hass, {
'input_boolean': {
}
}))

self.assertFalse(input_boolean.setup(self.hass, {
'input_boolean': {
'name with space': None
}
}))
with self.assertRaises(vol.Invalid):
input_boolean.PLATFORM_SCHEMA(None)
Copy link
Member

Choose a reason for hiding this comment

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

I prefer testing this using bootstrap._setup_component to follow the natural flow of discovering the validations and running them.

with self.assertRaises(vol.Invalid):
input_boolean.PLATFORM_SCHEMA({})
with self.assertRaises(vol.Invalid):
input_boolean.PLATFORM_SCHEMA({'name with space': None})

def test_methods(self):
"""Test is_on, turn_on, turn_off methods."""
Expand Down
45 changes: 15 additions & 30 deletions tests/components/test_input_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# pylint: disable=too-many-public-methods,protected-access
import unittest

import voluptuous as vol

from homeassistant.components import input_select
from homeassistant.const import (
ATTR_ICON, ATTR_FRIENDLY_NAME)
Expand All @@ -22,34 +24,18 @@ def tearDown(self): # pylint: disable=invalid-name

def test_config(self):
"""Test config."""
self.assertFalse(input_select.setup(self.hass, {
'input_select': None
}))

self.assertFalse(input_select.setup(self.hass, {
'input_select': {
}
}))

self.assertFalse(input_select.setup(self.hass, {
'input_select': {
'name with space': None
}
}))

self.assertFalse(input_select.setup(self.hass, {
'input_select': {
with self.assertRaises(vol.Invalid):
input_select.PLATFORM_SCHEMA(None)
with self.assertRaises(vol.Invalid):
input_select.PLATFORM_SCHEMA({})
with self.assertRaises(vol.Invalid):
input_select.PLATFORM_SCHEMA({'name with space': None})
with self.assertRaises(vol.Invalid):
input_select.PLATFORM_SCHEMA({
'hello': {
'options': None
}
}
}))

self.assertFalse(input_select.setup(self.hass, {
'input_select': {
'hello': None
}
}))
})

def test_select_option(self):
"""Test select_option methods."""
Expand Down Expand Up @@ -91,7 +77,7 @@ def test_config_options(self):
]

self.assertTrue(input_select.setup(self.hass, {
Copy link
Member

Choose a reason for hiding this comment

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

Also all these tests have to go through bootstrap._setup_component because validators can change the config object (ie via cv.ensure_list)

'input_select': {
'input_select': input_select.PLATFORM_SCHEMA({
'test_1': {
'options': [
1,
Expand All @@ -100,11 +86,11 @@ def test_config_options(self):
},
'test_2': {
'name': 'Hello World',
'icon': 'work',
'icon': 'mdi:work',
'options': test_2_options,
'initial': 'Better Option',
},
},
})
}))

self.assertEqual(count_start + 2, len(self.hass.states.entity_ids()))
Expand All @@ -119,11 +105,10 @@ def test_config_options(self):
self.assertEqual(['1', '2'],
state_1.attributes.get(input_select.ATTR_OPTIONS))
self.assertNotIn(ATTR_ICON, state_1.attributes)
self.assertNotIn(ATTR_FRIENDLY_NAME, state_1.attributes)

self.assertEqual('Better Option', state_2.state)
self.assertEqual(test_2_options,
state_2.attributes.get(input_select.ATTR_OPTIONS))
self.assertEqual('Hello World',
state_2.attributes.get(ATTR_FRIENDLY_NAME))
self.assertEqual('work', state_2.attributes.get(ATTR_ICON))
self.assertEqual('mdi:work', state_2.attributes.get(ATTR_ICON))
27 changes: 10 additions & 17 deletions tests/components/test_input_slider.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
# pylint: disable=too-many-public-methods,protected-access
import unittest

from homeassistant.components import input_slider
import voluptuous as vol

from homeassistant.components import input_slider
from tests.common import get_test_home_assistant


Expand All @@ -20,31 +21,23 @@ def tearDown(self): # pylint: disable=invalid-name

def test_config(self):
"""Test config."""
self.assertFalse(input_slider.setup(self.hass, {
'input_slider': None
}))

self.assertFalse(input_slider.setup(self.hass, {
'input_slider': {
}
}))

self.assertFalse(input_slider.setup(self.hass, {
'input_slider': {
'name with space': None
}
}))
with self.assertRaises(vol.Invalid):
input_slider.PLATFORM_SCHEMA(None)
with self.assertRaises(vol.Invalid):
input_slider.PLATFORM_SCHEMA({})
with self.assertRaises(vol.Invalid):
input_slider.PLATFORM_SCHEMA({'name with space': None})

def test_select_value(self):
"""Test select_value method."""
self.assertTrue(input_slider.setup(self.hass, {
'input_slider': {
'input_slider': input_slider.PLATFORM_SCHEMA({
'test_1': {
'initial': 50,
'min': 0,
'max': 100,
},
}
})
}))
entity_id = 'input_slider.test_1'

Expand Down