Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 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
320 changes: 320 additions & 0 deletions homeassistant/components/sensor/filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,320 @@
"""
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 logging
import statistics
from collections import deque, Counter

import voluptuous as vol

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,
ATTR_ICON, STATE_UNKNOWN, STATE_UNAVAILABLE)
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'
FILTER_NAME_THROTTLE = 'throttle'

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'

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),
})

FILTER_THROTTLE_SCHEMA = FILTER_SCHEMA.extend({
vol.Required(CONF_FILTER_NAME): FILTER_NAME_THROTTLE,
})

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,
FILTER_THROTTLE_SCHEMA)])
})


async def async_setup_platform(hass, config, async_add_devices,
discovery_info=None):
"""Set up the template 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,
entity=entity_id,
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,
entity=entity_id,
time_constant=time_constant))
elif _filter[CONF_FILTER_NAME] == FILTER_NAME_THROTTLE:
filters.append(ThrottleFilter(window_size=window_size,
precision=precision,
entity=entity_id))

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
self._icon = ICON

async 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._icon = new_state.attributes.get(ATTR_ICON, self._icon)

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.


if self._state in [STATE_UNKNOWN, STATE_UNAVAILABLE]:
return

for filt in self._filters:
try:
filtered_state = filt.filter_state(self._state)
_LOGGER.debug("%s(%s, %s) -> %s", filt.name,
self._entity,
self._state,
"skip" if filt.skip_processing else
filtered_state)
if filt.skip_processing:
return

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 the name "skip", I expected this line to be continue and thus skip this filter. With return you break the filter chain ?

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

Its supposed to break the chain.

If we want to skip that value, it should not be of interest to any other filter in the chain.

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 see. Can we clarify the name by making it skip_processing ?

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.

OK

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)
finally:
self._state = filtered_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.

Why is this in the finally ? Do you still want this to happen when a TypeError (or whatever) is raised? Probably should be moved below the try…except…finally.

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.

I figured if the filter doesn't work, it could be helpful for a normal user to see the state which blew up the filter.

Please note that due to chaining this state is not necessarily the state that came from the raw sensor.

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 think that when an error occurs, we need to fail hard. Otherwise people might start relying on failed state or not even realize that a failed state has occurred.

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.

I see your point, do you suggest moving from warning to error and raise the error ?

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 wouldn't raise the error, as that does nothing but log an exception. Log the error (as that will show up in the issue list) and just make sure that nothing is updated.

To do so, make sure you keep track of the new state in a local variable and only assign to self._state after all filters have run through successfully.


self.async_schedule_update_ha_state()

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 self._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

for filt_stat_key, filt_stat_value in filt.stats.items():
filt_stat = "{}_{}".format(filt.name, filt_stat_key)
_LOGGER.debug("stats(%s): %s: %s", self._entity,

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.

This is a lot of debug data inside a double for loop.

@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's true... but its VERY important for debug, and it is also the reason I did not use the code you propose in the next comment (it was my 1st version before I required debug info)

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.

Why is it very important for debugging ? You're just constructing a dictionary here.

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.

Logging is not a free operation, even if it will be ignored because logging for that level has been disabled.

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.

filter can be "working" without actually doing any thing useful, example: number of outliers might be too high/low. It is important to fine tune the parameters of the filter.

filt_stat, filt_stat_value)
state_attr.update({

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.

This is so expensive and inefficient. Create a dict with 1 value to then pass to the update method after which it is discarded? 🤔 You can do state_atr[filt_stat] = filt_stat_value but really, just go for the define it at once declaration I suggested earlier or explain to me why it is so important to log every attribute individually?

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.

@robmarkcole and @OttoWinter in the previous PR made a case for having stats of the filters.

Personally I think logging is enough to tune the filters, logging can even be done in the filter itself saving the dict.

If I don't get feedback from you guys, I'm taking the stats off from this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I say leave the stats of for now, since I coudn't even say what stats I'd request. Once I've been using the component a while then will be in a position to do some analysis and make a suggestion, cheers

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 we should do stat reporting through attributes (if at all). I personally hate it when entities put completely unnecessary stuff in their attributes that just clutter the front end (and state machine)

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.

Then it's settled! :) uff ... one less feature to creep this PR

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.

Databases all over the world are cheering 😉

filt_stat: filt_stat_value
})

return state_attr


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

Args:
window_size (int): size of the sliding window that holds previous
values
precision (int): round filtered value to precision value
entity (string): used for debugging only
"""

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

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

@property
def skip_processing(self):
"""Return wether the current filter_state should be skipped."""
return self._skip_processing

@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 not None:
filtered = round(filtered, self.precision)
self.states.append(filtered)
return filtered


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

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

Args:
radius (float): band radius
"""

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

def _filter_state(self, new_state):
"""Implement the outlier filter."""
self._stats_internal['total_filtered'] += 1
new_state = float(new_state)

self._stats['erasures'] = "{0:.2f}%".format(
100 * self._stats_internal['erasures']
/ self._stats_internal['total_filtered']
)

if (len(self.states) > 1 and
abs(new_state - statistics.median(self.states))
> self._radius):

self._stats_internal['erasures'] += 1

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


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

Args:
time_constant (int): time constant.
"""

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

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

if not self.states:
return new_state

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

return filtered


class ThrottleFilter(Filter):
"""Throttle Filter.

One sample per window.
"""

def __init__(self, window_size, precision, entity):
"""Initialize Filter."""
super().__init__(FILTER_NAME_THROTTLE, window_size, precision, entity)

def _filter_state(self, new_state):
"""Implement the throttle filter."""
if not self.states or len(self.states) == self.states.maxlen:
self.states.clear()
self._skip_processing = False
else:
self._skip_processing = True

return new_state
Loading