Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
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
265 changes: 265 additions & 0 deletions homeassistant/components/sensor/filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
"""
Allows the creation of a sensor that filters state property.

For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.filter/
"""
import asyncio
import logging
import statistics
from collections import deque

import voluptuous as vol

from homeassistant.util import slugify
from homeassistant.core import callback
from homeassistant.components.sensor import PLATFORM_SCHEMA
from homeassistant.const import (
CONF_NAME, CONF_ENTITY_ID, ATTR_UNIT_OF_MEASUREMENT, ATTR_ENTITY_ID)
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.event import async_track_state_change

_LOGGER = logging.getLogger(__name__)

FILTER_NAME_LOWPASS = 'lowpass'
FILTER_NAME_OUTLIER = 'outlier'

CONF_FILTERS = 'filters'
CONF_FILTER_NAME = 'filter'
CONF_FILTER_WINDOW_SIZE = 'window_size'
CONF_FILTER_PRECISION = 'precision'
CONF_FILTER_RADIUS = 'radius'
CONF_FILTER_TIME_CONSTANT = 'time_constant'

DEFAULT_FILTER_RADIUS = 2.0
DEFAULT_FILTER_TIME_CONSTANT = 10

NAME_TEMPLATE = "{} filter"
ICON = 'mdi: chart-line-variant'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think the space belongs here (between mdi: and chart-[...]) :)

Also: Wouldn't it be better if we copied the icon from the base entity? I mean for me, the use case for this platform would be to improve some shitty raw sensor values that deviate way too much. For example, for the following sensor I'd like to have some smoothing. It would therefore be nice not to have to manually set the icon for every single filter platform.

screen shot 2018-02-25 at 21 20 18

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point, will get the icon from the "parent"


FILTER_SCHEMA = vol.Schema({
vol.Optional(CONF_FILTER_WINDOW_SIZE): vol.Coerce(int),
vol.Optional(CONF_FILTER_PRECISION): vol.Coerce(int),
})

FILTER_OUTLIER_SCHEMA = FILTER_SCHEMA.extend({
vol.Required(CONF_FILTER_NAME): FILTER_NAME_OUTLIER,
vol.Optional(CONF_FILTER_RADIUS,
default=DEFAULT_FILTER_RADIUS): vol.Coerce(float),
})

FILTER_LOWPASS_SCHEMA = FILTER_SCHEMA.extend({
vol.Required(CONF_FILTER_NAME): FILTER_NAME_LOWPASS,
vol.Optional(CONF_FILTER_TIME_CONSTANT,
default=DEFAULT_FILTER_TIME_CONSTANT): vol.Coerce(int),
})

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_ENTITY_ID): cv.entity_id,
vol.Optional(CONF_NAME): cv.string,
vol.Required(CONF_FILTERS): vol.All(cv.ensure_list,
[vol.Any(FILTER_OUTLIER_SCHEMA,
FILTER_LOWPASS_SCHEMA)])
})


@asyncio.coroutine
def async_setup_platform(hass, config, async_add_devices, discovery_info=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We no longer use coroutine decorator. Instead use the async keyword. Throughout this PR, remove @asyncio.coroutine and make it async def async_setup_platform(. They are functionally equivalent.

"""Set up the template sensors."""
sensors = []

name = config.get(CONF_NAME)
entity_id = config.get(CONF_ENTITY_ID)
filters = []

for _filter in config[CONF_FILTERS]:
window_size = _filter.get(CONF_FILTER_WINDOW_SIZE)
precision = _filter.get(CONF_FILTER_PRECISION)

if _filter[CONF_FILTER_NAME] == FILTER_NAME_OUTLIER:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can do this easier. If you use the Registry decorator, you would have all classes by type in the registry. Then the config is already valid from the schema and maps to the keywords.

FILTERS = Registry()

@FILTERS.register(FILTER_NAME_LOWPASS)
class LowPassFilter(Filter):
    …

for filter_args in filters:
    filter_args = filter_args.copy()
    filter_args['entity'] = entity_id
    filter_class = FILTERS[filter_args.pop(CONF_FILTER_NAME)]
    filters.append(filter_class(**filter_args))

radius = _filter.get(CONF_FILTER_RADIUS)
filters.append(OutlierFilter(window_size=window_size,
precision=precision,
radius=radius))
elif _filter[CONF_FILTER_NAME] == FILTER_NAME_LOWPASS:
time_constant = _filter.get(CONF_FILTER_TIME_CONSTANT)
filters.append(LowPassFilter(window_size=window_size,
precision=precision,
time_constant=time_constant))

sensors.append(SensorFilter(name, entity_id, filters))

async_add_devices(sensors)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a style suggestion: Why not just async_add_devices([SensorFilter(name, entity_id, filters)])?



class SensorFilter(Entity):
"""Representation of a Filter Sensor."""

def __init__(self, name, entity_id, filters):
"""Initialize the sensor."""
self._name = name
self._entity = entity_id
self._unit_of_measurement = None
self._state = None
self._filters = filters

@asyncio.coroutine
def async_added_to_hass(self):
"""Register callbacks."""
@callback
def filter_sensor_state_listener(entity, old_state, new_state):
"""Handle device state changes."""
self._unit_of_measurement = new_state.attributes.get(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this one fall back to the old unit if there is none in the current state? (just like icon)

@dgomes dgomes Mar 1, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

filters have no units... they are pure math :)

while pure math might have an icon, makes no sense to have a default unit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With old unit I meant the unit of measurement of the previous state that was processed. For history it's important that all states of an entity have the same unit.

What happens now is that a state will have unit set to None if the source state becomes unavailable/unknown. We should probably move setting this instance variable when we know we're going to adopt a filtered version of the state.

ATTR_UNIT_OF_MEASUREMENT)

self._state = new_state.state

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find this a very weird assignment as it is never used. I would prefer a local variable that is assigned to self._state at the end. That way if something unexpected blows up, you are not left with a temp variable as state.

@dgomes dgomes Mar 1, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

its not temp, its the state of the previous filter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

new_state.state is the state that triggered the state change. It has not been filtered yet. Since we have at least 1 filter, this assignment will never last when we update Home Assistant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would expect only to write values to self._state that you are intending to publish to Home Assistant. Class instance variables should not be used for temporary variables inside the scope.

for filt in self._filters:
try:
filtered_state = filt.filter_state(self._state)
_LOGGER.debug("%s(%s) -> %s", filt.name, self._state,
filtered_state)
self._state = filtered_state
filt.states.append(filtered_state)
except ValueError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to move the conversion to number above the for loop so we do it exactly once?

@dgomes dgomes Feb 26, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It all depends on the filters configured by the user. And some filters are not processing numbers (e.g. the ThrottleFilter will work with strings).
Therefore conversion should be filter dependent

_LOGGER.warning("Could not convert state: %s to number",
self._state)

self.async_schedule_update_ha_state(True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't pass True as you don't want Home Assistant to call update (as you don't have one)


async_track_state_change(
self.hass, self._entity, filter_sensor_state_listener)

@property
def name(self):
"""Return the name of the sensor."""
return self._name

@property
def state(self):
"""Return the state of the sensor."""
return self._state

@property
def icon(self):
"""Return the icon to use in the frontend, if any."""
return ICON

@property
def unit_of_measurement(self):
"""Return the unit_of_measurement of the device."""
return self._unit_of_measurement

@property
def should_poll(self):
"""No polling needed."""
return False

@property
def device_state_attributes(self):
"""Return the state attributes of the sensor."""
state_attr = {
ATTR_ENTITY_ID: self._entity
}
for filt in self._filters:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't you just want something like this?

attr = {
    "{}_{}".format(filt.name, filt_stat_key): filt_stat_value
    for filt_stat_key, filt_stat_value in filt.stats.items()
    for filt in self._filters
}

attr[ATTR_ENTITY_ID] = self._entity
return attr

If we know that stats() calls are always used for device_state_attributes, why not prefix it while being generated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adding the filt.name to all the stats where they are being generated would pollute each filter code.

An option would be in Filter.stats() property, but here it wouldn't be much different then the current case, as the double for would still be present.

Personally I would not expose the stats, but others have pointed out use-case where they want to snag one of the stats attributes.

Maybe a configuration option? Would that be ok ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MVP has really blown up here he. All of a sudden we have filter chaining, 10s of stats attributes. Duplicate attribute names (if a double filter used) that will override each other

state_attr.update({
slugify("{} stats".format(filt.name)): filt.stats

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How will the nested dict that we create for state attributes look in the frontend?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this information is for fine tuning the filter options only. Should not be presented in the frontend.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

State attributes are shown in a table under the history graph in the more info card of sensors.

@dgomes dgomes Feb 25, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Only alternative I see is to slugify the nested dict():

outlier_erasures: 123
lowpass_median: 50

comments ?

@MartinHjelmare MartinHjelmare Feb 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably easiest to flatten the dicts into one dict, by concatenating + slugify the key names for the inner dict with the holding key name. I think that's what you mean too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes :)

})

return state_attr


class Filter(object):
"""Filter skeleton.

Args:
window_size (int): size of the sliding window that holds previous
values
"""

def __init__(self, name, window_size=1, precision=None):
"""Initialize common attributes."""
self.states = deque(maxlen=window_size)
self.precision = precision
self._stats = {}
self._name = name

@property
def name(self):
"""Return filter name."""
return self._name

@property
def stats(self):
"""Return statistics of the filter."""
return self._stats

def _filter_state(self, new_state):
"""Implement filter."""
raise NotImplementedError()

def filter_state(self, new_state):
"""Implement a common interface for filters."""
filtered = self._filter_state(new_state)
if self.precision is None:
return filtered
return round(filtered, self.precision)


class OutlierFilter(Filter):
"""BASIC outlier filter.

Determines if new state is in a band around the median.

Args:
radius (float): band radius
window_size (int): see Filter()
"""

def __init__(self, window_size, precision, radius):
"""Initialize Filter."""
super().__init__(FILTER_NAME_OUTLIER, window_size, precision)
self._radius = radius

def _filter_state(self, new_state):
"""Implement the outlier filter."""
new_state = float(new_state)
if (len(self.states) > 1 and
abs(new_state - statistics.median(self.states))
> self._radius):

erasures = self._stats.get('erasures', 0)
self._stats['erasures'] = erasures+1

_LOGGER.debug("Outlier in %s: %s", self._name, new_state)
return self.states[-1]
return new_state


class LowPassFilter(Filter):
"""BASIC Low Pass Filter.

Args:
time_constant (int): time constant.
window_size (int): see Filter()
"""

def __init__(self, window_size, precision, time_constant):
"""Initialize Filter."""
super().__init__(FILTER_NAME_LOWPASS, window_size, precision)
self._time_constant = time_constant

def _filter_state(self, new_state):
"""Implement the low pass filter."""
new_state = float(new_state)

try:
new_weight = 1.0 / self._time_constant
prev_weight = 1.0 - new_weight
filtered = prev_weight * self.states[-1] + new_weight * new_state
except IndexError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Prefer to just catch an empty states list instead of going for try…except.

if not self.states
    return new_state

new_weight =

# if we don't have enough states to run the filter
# just accept the new value
filtered = new_state

return filtered
83 changes: 83 additions & 0 deletions tests/components/sensor/test_filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
"""The test for the data filter sensor platform."""
import unittest

from homeassistant.setup import setup_component
from tests.common import get_test_home_assistant, assert_setup_component


class TestFilterSensor(unittest.TestCase):
"""Test the Data Filter sensor."""

def setup_method(self, method):
"""Setup things to be run when tests are started."""
self.hass = get_test_home_assistant()
self.values = [20, 19, 18, 21, 22, 0]

def teardown_method(self, method):
"""Stop everything that was started."""
self.hass.stop()

def test_setup_fail(self):
"""Test if filter doesn't exist."""
config = {
'sensor': {
'platform': 'filter',
'entity_id': 'sensor.test_monitored',
'filters': [{'filter': 'nonexisting'}]
}
}
with assert_setup_component(0):
assert setup_component(self.hass, 'sensor', config)

def test_outlier(self):
"""Test if filter outlier works."""
config = {
'sensor': {
'platform': 'filter',
'name': 'test',
'entity_id': 'sensor.test_monitored',
'filters': [{
'filter': 'outlier',
'radius': 4.0
}]
}
}
with assert_setup_component(1):
assert setup_component(self.hass, 'sensor', config)

self.hass.start()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as Martin's comment: you don't need to call hass.start()

self.hass.block_till_done()

for value in self.values:
self.hass.states.set(config['sensor']['entity_id'], value)
self.hass.block_till_done()

state = self.hass.states.get('sensor.test')
self.assertEqual('22.0', state.state)

def test_lowpass(self):
"""Test if filter lowpass works."""
config = {
'sensor': {
'platform': 'filter',
'name': 'test',
'entity_id': 'sensor.test_monitored',
'filters': [{
'filter': 'lowpass',
'time_constant': 10,
'precision': 2
}]
}
}
with assert_setup_component(1):
assert setup_component(self.hass, 'sensor', config)

self.hass.start()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

self.hass.block_till_done()

for value in self.values:
self.hass.states.set(config['sensor']['entity_id'], value)
self.hass.block_till_done()

state = self.hass.states.get('sensor.test')
self.assertEqual(18.05, round(float(state.state), 2))