-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
MQTT sensors handling of publishing NaN values #7768
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #7768 +/- ##
==========================================
+ Coverage 53.70% 53.80% +0.09%
==========================================
Files 50 50
Lines 9408 9813 +405
Branches 1654 1353 -301
==========================================
+ Hits 5053 5280 +227
- Misses 4056 4207 +151
- Partials 299 326 +27 ☔ View full report in Codecov by Sentry. |
@jesserockz, @clydebarrow, wdyt? Similarly the way the API handles NaN to None state |
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.
I originally put this on the beta milestone, but just realised this could be a breaking change for people not using HA.
I think this needs a configuration option (something like publish_nan_as_none
) for either the global mqtt config, or on the mqtt sensor schema that defaults to false to keep existing functionality.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Thanks @jesserockz for your review and guidance. After further consideration and as per your suggestion I've updated the PR to have a global config for MQTT to avoid breaking things for non HA users. Also updated docs esphome/esphome-docs#4452 Cheers! |
Updated Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Hi, @jesserockz thanks for the suggestions is there anything else you need from my side for this PR? |
What does this implement/fix?
Publishing "None" instead of "NaN" values for sensors in MQTT. This is to align with HASS's MQTT changes as it no longer seem to like them NaN values in sensor states and it's spamming the logs each time nan is published e.g.
Types of changes
Related issue or feature (if applicable):
N/A
Pull request in esphome-docs with documentation (if applicable):
Test Environment
Example entry for
config.yaml
:Checklist:
tests/
folder).If user exposed functionality or configuration variables are added/changed: