Update TimeSMAFilter based on time #53053
Conversation
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
probot-home-assistant
bot
added
bugfix
integration: filter
small-pr
| @@ -454,7 +462,7 @@ def filter_state(self, new_state): | |||
|
|
|||
|
|
|||
| @FILTERS.register(FILTER_NAME_RANGE) | |||
| class RangeFilter(Filter, SensorEntity): | |||
| class RangeFilter(Filter): | |||
There was a problem hiding this comment.
I might have misunderstood something, but I see no reason for the filters to inherit SensorEntity
There was a problem hiding this comment.
Not that I can recall :/
since tests didn't break and coverage is high, I'm ok with this
|
This idea was discussed in the past (unfortunately can't find original discussion) As I recall the issue put forward was that this would create fake states. Since no update from the source sensor triggers the new filter state, the filter sensor can produce completely unrelated values to the source. |
|
For reference, TimeSMAfilter follows http://www.eckner.com/papers/Algorithms%20for%20Unevenly%20Spaced%20Time%20Series.pdf |
|
I see your point. An example where the source sensor is low for a long period, then a constant high value for a period. Then the filtered value is not updated before the source sensor drops back to the low value. If you already have discussed this and concluded that the current behavior is intended, I guess we can close this pr? |
|
I think the original PR already discussed this issue: what I never did, was to submit the SMA_linear |
|
Hmm, not sure I see any arguments about fake states? Is that the main argument? |
|
Fake states was another discussion, haven't traced yet where that occurred. Personally I'm fine in creating a configuration option or a new filter with your intended behaviour. |
Will do that 👍 |
Signed-off-by: Daniel Hjelseth Høyer <[email protected]>
| @@ -72,6 +73,8 @@ | |||
| NAME_TEMPLATE = "{} filter" | |||
| ICON = "mdi:chart-line-variant" | |||
|
|
|||
| SCAN_INTERVAL = timedelta(minutes=3) | |||
There was a problem hiding this comment.
Any reasoning for choosing 3min ?
|
Also documentation needs to be updated |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Signed-off-by: Daniel Hjelseth Høyer [email protected]
Breaking change
Proposed change
Update TimeSMAFilter based on time
A time-based filter should not only update when the source value is updated.
Make a new configuration to allow this.
Needs to update documentation and add a test.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: