-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make pulse_meter
PULSE filter report the pulse as soon as it can
#6014
Conversation
Hey there @cstaahl, @stevebaxter, mind taking a look at this pull request as it has been labeled with an integration ( |
25359f8
to
fcdaee3
Compare
…d simplify the pulse filter
c76a604
to
74de981
Compare
Should be a more complete fix for #5813 as described in that PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #6014 +/- ##
==========================================
+ Coverage 53.70% 54.05% +0.34%
==========================================
Files 50 50
Lines 9408 9554 +146
Branches 1654 1687 +33
==========================================
+ Hits 5053 5164 +111
- Misses 4056 4066 +10
- Partials 299 324 +25 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I appreciate you putting the effort into this file (It looks great btw), we do not want "documentation" throughout the esphome codebase in adhoc files. This is what https://esphome/esphome-docs is for.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @cstaahl, @stevebaxter, mind taking a look at this pull request as it has been labeled with an integration ( |
What does this implement/fix?
See #5813 (comment)
The pulse meter filter would only publish the pulses when the next pulse came through when in PULSE mode.
This caused some strange behaviour when the pulses were slow as features like timeout would happen at unexpected times as even though a pulse had happened since the next pulse hadn't yet it was not counted.
This change allows the main loop to "peek" the filter state to see if it can count the incomplete pulse and repay that pulse count later when the interrupt catches up.
This also resulted in some refactoring to the pulse filter to enable this which simplified it significantly in the process.
I've also added a markdown file to help describe the pulse filter so that myself and others are not as confused by it in the future.
Since almost all of my devices use higher frequencies and the EDGE filter mode, if someone who uses some of these slower pulse rate devices could test this that would be appreciated.
Types of changes
Related issue or feature (if applicable): fixes
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#
Test Environment
Example entry for
config.yaml
:# Example config.yaml
Checklist:
tests/
folder).If user exposed functionality or configuration variables are added/changed: