history_average sensor - time-weighted average of state values from history #10717

kmwoley · 2017-11-21T05:59:12Z

Description:

A sensor which reports the weighted average of a sensor's recorded value over a specified period of time.

Motivation - history_stats and statistics didn't exactly meet the needs I had for a sensor - I wanted a sensor that could average my bathroom's humidity over the last day, so I could use it in an automation to kick on an exhaust fan when the relative humidity was a few % points above the recent average.

Unlike the statistics sensor which samples a sensor's values to produce a mean (along with other stats), this sensor uses the history recorded in the database to calculate it's results. That allows the value to be useful across reboots, can leverage history to produce results over a much longer period of time, and doesn't rely upon picking a correct sampling interval for the data being averaged.

Unlike history_stats sensor, this sensor doesn't look to count a specific discrete state, but averages the values of non-discrete states. Much of the inspiration, code for this sensor started with history_stats

Note:
I did look at just incorporating this into history_stats, but that sensor requires a state attribute. Incorporating this functionality into that sensor would require making that attribute optional, which didn't feel right. I'd be happy to combine this sensor with another if folks felt that was a better path.

Related issue (if applicable): fixes #
n/a

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
TBD - assuming folks like this as a stand alone sensor, I'll write up the docs.

Example entry for `configuration.yaml` (if applicable):

- platform: history_average
  name: Master Bathroom Average Relative Humidity
  entity_id: sensor.master_bathroom_multisensor_relative_humidity
  unit_of_measurement: '%'
  end: '{{ as_timestamp(now()) }}'
  duration:
    hours: 12

Checklist:

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

Documentation added/updated in home-assistant.github.io

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.

kmwoley · 2017-11-21T06:02:59Z

This is an updated version of this older pull request that went stale:
#9260

I believe I've addressed the feedback in that pull request; happy to write the docs if you all are happy with the work here. From the comments on the other pull request, it sounds like some renaming may be in order if folks aren't happy with 'history_average'. I'm not wed to anything.

kmwoley · 2017-11-21T07:08:37Z

In my local testing, I had one heck of a time getting the in-memory recorder to work correctly. Apparently I didn't succeed given the tox results here. I would appreciate any pointers that can be given as to how to setup my tests correctly so that I can leverage the history / recorder.

I should mention that the tests pass locally when running py.test *test_history_average.py - I just don't know what's going on when I run tox and haven't had any luck debugging it.

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/const.py

 MINOR_VERSION = 59
+=======
+MINOR_VERSION = 53
+>>>>>>> actually fixing merge conflicts. sigh.


missing whitespace around operator

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/const.py

 MINOR_VERSION = 59
+=======


missing whitespace around operator

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/const.py

@@ -1,7 +1,11 @@
 # coding: utf-8
 """Constants used by Home Assistant components."""
 MAJOR_VERSION = 0
+<<<<<<< HEAD


SyntaxError: invalid syntax
missing whitespace around bitwise or shift operator
missing whitespace around operator

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/components/notify/pushbullet.py

            self._push_data(message, title, data, self.pushbullet)
+=======
+            self._push_data(filepath, message, title, url, self.pushbullet)
+>>>>>>> actually fixing merge conflicts. sigh.


missing whitespace around operator

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/components/notify/pushbullet.py

            self._push_data(message, title, data, self.pushbullet)
+=======
+            self._push_data(filepath, message, title, url, self.pushbullet)


unexpected indentation

houndci-bot · 2017-11-22T06:02:38Z

homeassistant/components/notify/pushbullet.py

            self._push_data(message, title, data, self.pushbullet)
+=======


missing whitespace around operator

houndci-bot · 2017-11-22T06:02:39Z

homeassistant/components/notify/pushbullet.py

@@ -86,7 +86,11 @@ def send_message(self, message=None, **kwargs):

        if not targets:
            # Backward compatibility, notify all devices in own account
+<<<<<<< HEAD


expected 2 blank lines after class or function definition, found 0
expected an indented block
IndentationError: expected an indented block
missing whitespace around bitwise or shift operator
missing whitespace around operator

balloob

Realized I had a bunch of pending comments for this PR. Not sure if I fully reviewed it.

balloob · 2017-11-24T00:40:12Z

homeassistant/components/sensor/history_average.py

+            _LOGGER.warning(ex)
+            return
+        _LOGGER.error("Error parsing template for field %s", field)
+        _LOGGER.error(ex)


Don' do this. If you want to print the stack trace use _LOGGER.exception("your message")

Actually, looking at this again, this is how the template.py implements the same check/warning:

except TemplateError as ex: if ex.args and ex.args[0].startswith( "UndefinedError: 'None' has no attribute"): # Common during HA startup - so just a warning _LOGGER.warning('Could not render template %s,' ' the state is unknown.', self._name) return self._state = None _LOGGER.error('Could not render template %s: %s', self._name, ex)

Do you still recommend _LOGGER.exception("message") - just trying for consistency; best practice.

balloob · 2017-11-24T00:40:51Z

homeassistant/core.py

@@ -608,6 +608,10 @@ def __eq__(self, other):
                self.state == other.state and
                self.attributes == other.attributes)

+    def __lt__(self, other):


Please remove this as it is incorrect. Not all states can be ordered, what if they are from different entities. It's unpredictable. If you want to sort a bunch of states, just pass in a lambda to use as key: sorted(my_states, key state: state.last_changed)

My primary operation is to insert an item into an already sorted list - but bisect doesn't support passing in a lambda to use as a key AFAIK.

It felt wrong to put the item on the end and then sort the list, but I can do that to avoid needing the __lt__ on the State.

balloob · 2017-11-24T00:41:29Z

tests/components/sensor/test_history_average.py

+
+        setup_component(self.hass, 'sensor', config)
+        self.assertEqual(self.hass.states.get('sensor.test'), None)
+        self.assertRaises(TypeError,


We should never raise a TypeError on invalid config.

This was a pattern I borrowed from the existing History Stats sensor tests - but I think I'm realizing that there's a lot in that test and that Sensor that may not be good to follow.

The TypeError is raised from template parsing - can you point me to a recommendation / example of how to handle bad template parsing, and associated tests?

Thanks.

You can remove this test case. It can never happen because we have a CONFIG_SCHEMA that protects from it.

Got it. Done.

You write "Done" but I still see this test case?

Sorry - I missed a test case that was looking for a TypeError. Just pushed the removal of another that one. Are there other test cases you were talking about removing?

balloob · 2017-11-24T00:42:59Z

homeassistant/components/sensor/history_average.py

+
+
+class HistoryAverageHelper:
+    """Static methods to make the HistoryAverageSensor code lighter."""


Instead of static methods, just define them in the root of your module.

Will do. Thanks. Another place where I was following the existing History Stats sensor... I agree, this is ugly.

houndci-bot · 2017-11-28T08:15:05Z

homeassistant/components/sensor/history_average.py

+https://home-assistant.io/components/sensor.history_average/
+"""
+import asyncio
+from operator import attrgetter 


trailing whitespace

- help to save database space, writes that are somewhat useless (would be nice to keep them for sensor display w/o storing them as attributes in the DB, but that's not possible AFAIK)

+ no longer average values between sensor readings + remove datetime, homeassistant.util.dt + timestamps no longer rounded + bug fix on incorrect hass object reference + removed buggy "get first item" code that wasn't right at all...

- update to core.py to ordering of States in a list via bisect - known issue: tests don't trigger the async update of the sensor (reading from history is fine)

- move the starting of hass under test to a better location - add logging to testing - remove print() statements - pass pylint, flake8, etc.

- things pass locally; pushing to see if it fixes the CI will pass - patched recorder / history calls to avoid random, not-understood db errors that happen under test - removed various patching of datetime calls that are now not needed

balloob · 2018-02-18T06:40:21Z

homeassistant/components/sensor/history_average.py

+            async_track_state_change(
+                self._hass, self._entity_id, state_listener)
+
+            self._hass.async_add_job(self.async_update_ha_state(True))


Why only do this here and not in async_added_to_hass?

I'm a little new to asyncio - I was attempting to follow patterns I've seen elsewhere in HASS code, so it's possible I wasn't following a great example. Are you saying I should be calling self.async_schedule_update_ha_state(True) in three places?

sensor_startup

state_listener

async_added_to_hass

Or that it could be pulled out of sensor_startup and have it two places: async_added_to_hass and state_listener?

balloob · 2018-02-18T06:41:06Z

homeassistant/components/sensor/history_average.py

+        """Return the polling state."""
+        # Since the values can/do change with time, not just with
+        # state changes, the sensor should poll
+        return True


I don't like this 😢 I understand why this is, but it is spamming our state machine. Wonder if we should approach this problem differently.

I'm open to suggestions. 🤷‍♂️

balloob · 2018-02-18T06:41:32Z

homeassistant/components/sensor/history_average.py

+    @property
+    def device_state_attributes(self):
+        """Return the state attributes of the sensor."""
+        def period_in_seconds(period):


Very expensive to keep defining these functions, just move them out.

balloob · 2018-02-18T06:42:39Z

homeassistant/components/sensor/history_average.py

+                return 0.0
+            return (period[1] - period[0]).total_seconds()
+
+        def pretty_duration(period):


We should not have a pretty duration. Attributes should be machine readable. If pretty printend is necessary, the frontend should add this.

I agree. However, I'm not sure what data type to return here to represent a length in time. I tried timedelta, but it's not JSON serializable. Do you have a suggestion?

balloob · 2018-02-18T06:43:14Z

homeassistant/components/sensor/history_average.py

+        yield from self.asysnc_trim_history()
+        start, end = self._period
+
+        _LOGGER.info(" - period start: %s", start)


This is not info log worthy. Max debug. Also limit to 1 statement instead of 2.

robmarkcole · 2018-02-22T06:10:33Z

The functionality of this component is almost identical to what @dgomes wants to achieve with filters. I feel there should be a single platform for filters/smoothing that handles prepping the data for filtering, and each filter then implemented as a component.

balloob · 2018-02-22T07:46:31Z

@robmarkcole the difference is that the filter would operate on state changes captured during operation. This sensor would include states fetched from the history. You're right, this could be added to the filter component and would probably be better. That way we can capture every use case.

dgomes · 2018-02-22T09:51:42Z

I've also seen this commit, and share your comments.

Since I wouldn't like to bloat the existing PR with additional features, I'm leaving a possible merge for further ahead :)

balloob · 2018-02-26T08:28:57Z

So what should we do? Should we wait for #12650 to get merged and add it as a filter to that platform?

B1tMaster · 2018-03-05T01:38:19Z

this is great PR. I really need this functionality.

Question: did you implement your own capturing and keeping history of previous values of the sensor in memory and use this for computing a rolling average? From this it sounds as you just get data from database " sensor uses the history recorded in the database to calculate it's results" . How did you deal with the fact that SqlAlchemy does not flush data to DB for some time. Would your sensor work for periods of last 15 - 20 mins? (before SQLalchemy flushed data to database)

I attempted to do something this with new SQL sensor and AVG() method over a specified period of time, but found that sqlalchemy does not flush data ot MariaDB often enough..

balloob · 2018-03-09T03:29:55Z

I think that we should close this PR and add weighted average to the new filter sensor.

kmwoley · 2018-03-10T13:38:32Z

Having not been following the filter sensor development, I don't have a strong opinion either way. Is the development far enough along on the filter sensor to start adding this functionality?

dgomes · 2018-03-10T14:01:18Z

@kmwoley it's ready to go! (already released in 0.65)

kmwoley · 2018-03-22T03:25:28Z

@dgomes @balloob The filters sensor doesn't exactly meet the need that this pull request accomplishes - i.e. relying on the history so that values are correct @ boot.

If you all think incorporating history into the filters sensor is a good thing to pursue, I'd be happy to work on extending the filters sensor - but it's not clear that your intent with filters is to draw from history. What do you all think?

balloob · 2018-03-22T03:36:04Z

History support is being added by @dgomes as we speak: #13075

kmwoley · 2018-03-22T04:12:14Z

Sweet! Thanks.

kmwoley requested a review from a team as a code owner November 21, 2017 05:59

homeassistant added new-platform platform: sensor.history_average cla-signed labels Nov 21, 2017

balloobbot added core platform labels Nov 21, 2017

kmwoley changed the title ~~History average~~ history_average sensor - weighted average of a state values from history Nov 21, 2017

kmwoley mentioned this pull request Nov 21, 2017

history_average sensor - weighted average of a state values from history #9260

Closed

3 tasks

homeassistant added the cla-signed label Nov 21, 2017

kmwoley force-pushed the history_average branch from 894987a to 4132233 Compare November 22, 2017 06:02

homeassistant added the cla-signed label Nov 22, 2017

houndci-bot reviewed Nov 22, 2017

View reviewed changes

kmwoley force-pushed the history_average branch from f04fc15 to 811245f Compare November 22, 2017 08:57

homeassistant added the cla-signed label Nov 22, 2017

kmwoley changed the title ~~history_average sensor - weighted average of a state values from history~~ history_average sensor - time-weighted average of state values from history Nov 22, 2017

balloob requested changes Nov 28, 2017

View reviewed changes

homeassistant added the cla-signed label Nov 28, 2017

houndci-bot reviewed Nov 28, 2017

View reviewed changes

kmwoley added 10 commits December 5, 2017 03:41

initial commit

fb81007

remove start, end attributes

39da696

- help to save database space, writes that are somewhat useless (would be nice to keep them for sensor display w/o storing them as attributes in the DB, but that's not possible AFAIK)

fix flake8, linting issues

45cef4a

minor changes, cleanup

f5fb375

adding tests, lots of bug fixes

67ee2e6

+ no longer average values between sensor readings + remove datetime, homeassistant.util.dt + timestamps no longer rounded + bug fix on incorrect hass object reference + removed buggy "get first item" code that wasn't right at all...

fix indent issue in test

a0ebd70

rewrite as async sensor, move state tracking in memory (reduce db reads)

6e6ac6d

- update to core.py to ordering of States in a list via bisect - known issue: tests don't trigger the async update of the sensor (reading from history is fine)

fix state change tests, linting, flake8

3b1655f

- move the starting of hass under test to a better location - add logging to testing - remove print() statements - pass pylint, flake8, etc.

various test fixes to get CI to pass

2c93a55

- things pass locally; pushing to see if it fixes the CI will pass - patched recorder / history calls to avoid random, not-understood db errors that happen under test - removed various patching of datetime calls that are now not needed

make sure hass is started on each test

27c6442

balloob reviewed Feb 18, 2018

View reviewed changes

balloob mentioned this pull request Feb 18, 2018

How to deal with history sensors home-assistant/architecture#11

Closed

kmwoley added 4 commits February 21, 2018 19:16

Merge remote-tracking branch 'upstream/dev' into history_average

ac800e4

code review feedback

1470a45

fix flake8 error

0a98437

removed test cases that are not needed

b133de7

Merge remote-tracking branch 'upstream/dev' into history_average

23c786b

homeassistant added the cla-signed label Feb 26, 2018

removed another test case looking for a type_error

baa3da1

dgomes mentioned this pull request Mar 4, 2018

SQL Sensor Component UnboundLocalError when no data returned. #12856

Closed

B1tMaster mentioned this pull request Mar 5, 2018

Investigate improving Wind Alerts reporting by using stats and history sensors B1tMaster/haas-config#79

Open

kmwoley closed this Mar 22, 2018

home-assistant locked and limited conversation to collaborators Jul 26, 2018

ghost added integration: history_average and removed platform: sensor.history_average labels Mar 21, 2019

		self._push_data(message, title, data, self.pushbullet)
		=======



		class HistoryAverageHelper:
		"""Static methods to make the HistoryAverageSensor code lighter."""

history_average sensor - time-weighted average of state values from history #10717

history_average sensor - time-weighted average of state values from history #10717

Conversation

kmwoley commented Nov 21, 2017

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

kmwoley commented Nov 21, 2017

kmwoley commented Nov 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmwoley Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robmarkcole commented Feb 22, 2018

balloob commented Feb 22, 2018

dgomes commented Feb 22, 2018

balloob commented Feb 26, 2018

B1tMaster commented Mar 5, 2018

balloob commented Mar 9, 2018

kmwoley commented Mar 10, 2018

dgomes commented Mar 10, 2018

kmwoley commented Mar 22, 2018

balloob commented Mar 22, 2018

kmwoley commented Mar 22, 2018

Example entry for `configuration.yaml` (if applicable):

kmwoley commented Nov 21, 2017 •

edited

Loading

kmwoley Nov 28, 2017 •

edited

Loading