Skip to content

Data Filter Sensor (was: Filter helper decorator)#12506

Closed
dgomes wants to merge 55 commits intohome-assistant:devfrom
dgomes:filter
Closed

Data Filter Sensor (was: Filter helper decorator)#12506
dgomes wants to merge 55 commits intohome-assistant:devfrom
dgomes:filter

Conversation

@dgomes
Copy link
Copy Markdown
Contributor

@dgomes dgomes commented Feb 18, 2018

Description:

When dealing with physical sensors (those who read real life data) it is common to get errors and noise (due to the quality of the sensors or undesirable events). Good devices implement internally filters that remove outliers and smooth data. But many other devices do not (especially DIY devices/hardware and cheap devices).

This Pull request introduces a helper decorator Filter that can be added to the state() method or to any other @Property method.

Two files are provided:

  • helpers/filter.py : implements the decorator and filter algorithms
  • components/sensor/filter.py : implements a meta sensor that uses the fore mentioned decorator and can be used regardless of components being updated with the Filter decorator

Filters have been implemented as decorators since I believe they are of broader use then to this single meta filter component (I personally use it for two different custom_components that handle sensor information coming from DIY devices). Furthermore this implementation makes it trivial to add filtering to existing and future components.

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: data_filter
    entity_id: sensor.relative_humidity
    name: lowpass
    options:
      time_constant: 10

Checklist:

  • The code change is tested and works locally.
  • Documentation added/updated in home-assistant.github.io
  • 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.


async_add_devices([
DataFilterSensor(hass, name, entity_id, filter_name, wsize,
config.get(CONF_FILTER_OPTIONS, dict()))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

@dgomes dgomes changed the title Filter helper decorator Data Filter Sensor (was: Filter helper decorator) Feb 22, 2018
@robmarkcole
Copy link
Copy Markdown
Contributor

robmarkcole commented Feb 23, 2018

@dgomes you example config in the description needs to be updated:

sensor:
  - platform: data_filter
    entity_id: sensor.relative_humidity
    name: lowpass_relative_humidity
    filter: lowpass
    options:
      time_constant: 10

It's working well, could add some graph images to the docs:
image

Another idea, could a real time demo of the sensor be embedded in the docs @balloob ?

@dgomes
Copy link
Copy Markdown
Contributor Author

dgomes commented Feb 23, 2018

@robmarkcole guess your comment belongs in #4725 ;)

The example in the documentation is just like the one you propose... what am I missing ?

The image is a nice idea :)

@dgomes dgomes closed this Feb 23, 2018
@dgomes dgomes reopened this Feb 23, 2018
@robmarkcole
Copy link
Copy Markdown
Contributor

@dgomes your docs are correct but the config at top of this page requires platform: data_filter

Re graphs, a picture tells a thousand words :)

@OttoWinter
Copy link
Copy Markdown
Member

OttoWinter commented Feb 23, 2018

I have a suggestion for the config schema: Why not do something like:

sensor:
  - platform: data_filter
    name: lowpass_relative_humidity
    filters:
      - filter: lowpass
        time_constant: 10
      - filter: mean  # doesn't exist yet; just to show the point
        last_values: 42

For me, I think that would have several benefits:

  • Readability: Now the filter and it's options sit "closer" to one another in the config, thus increasing readability. Having a separate options: key seems like a bit of a hack to me.
  • Easy chaining of filters: If I understand correctly, with your current config you'd have to create multiple filter platforms to apply multiple data_filters to a raw data stream. What if I want to have my humidity sensor do lowpass detection & do some weighed history average? With filters: this would be a lot nicer.
  • Less pressure on the state machine by only publishing the "end result" and not having intermediary sensor states.

@dgomes
Copy link
Copy Markdown
Contributor Author

dgomes commented Feb 23, 2018

Thanks for the feedback @OttoWinter !

Let me start by saying how much I'm enjoying working in this project, and the all interesting discussions within the developers community.

I think the data_filter platform has become a Frankenstein after all this discussion...

The platform was supposed to be a demonstrator of the Filter decorators, the platform was a way to demo the decorators without interfering with any other platform, it was not an end in itself (heck! it could have never existed and we could still have filters!). The choice of very simple filter (BASIC) was to not draw attention to the filters but to the helper. Yet, everyone focused on the platform....

My vision was that developers would have incorporated the decorators with options tuned for each sensor (no need for end-user to change any filter option value, the sensor platform developer would tune according to his use case/hardware and would expose a high level option at most!). Filtering would have been enabled/disabled per sensor platform through a configuration option in each sensor platform (not in the data_filter platform or helper).
It was never supposed to be done through the data_filter component like is currently the case.

I accepted the core team decision not to have Filters as helpers, but as a platform. I've moved the decorators away into the data_filter platform (yes... very hackish, but was my way of still being able to import the decorators into other components while keeping the filters in the official repository and exposing the possibility to other developers).

This brings us to today:

The current PR pushes filters towards the usage pattern of the template_sensor which I hate very dearly as it pollutes the state machine with extra entities, but from which I cannot escape...

I'm no longer proud of this PR... :(

This component does not address my goals and the changes requested, although very appropriate have created / will create something very different from what was my initial goal. With that the code has become very weird. The current data_filter doesn't support chaining as suggested by @OttoWinter, adding it in the current code would make things even more weird...

I could have ended this very long and sorrow comment with a "So Long, and Thanks for All the Fish", but I like this community too much.

If people here agree that the template_sensor pattern is the way to go, I will start working a new platform from scratch (lets call it filter_sensor), that doesn't need to have idempotent filter algorithms (which currently adds complexity but was need to implement the filter decorator pattern).

Final bitching remark: I'm keeping the decorators in my custom_components dir :P

@dgomes dgomes mentioned this pull request Feb 25, 2018
4 tasks
@dgomes dgomes closed this Feb 25, 2018
@dgomes dgomes deleted the filter branch March 18, 2018 19:41
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants