-
-
Notifications
You must be signed in to change notification settings - Fork 36.3k
Fixing state_class for ecowitt rain sensors #159974
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
base: dev
Are you sure you want to change the base?
Fixing state_class for ecowitt rain sensors #159974
Conversation
|
Hey there @pvizeli, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
How does this PR differ / be a better option / compare to the existing open PR? ../Frenck |
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.
Pull request overview
This PR fixes a regression where rain sensors lost their state_class attribute, which is needed for long-term statistics in Home Assistant. The previous change (PR #155812) removed state_class from all accumulating rain sensors because moving-window sensors (like "last 24h") should not have this attribute. However, this over-corrected by also removing it from non-moving-window sensors like daily, weekly, monthly, and yearly totals that reset at specific time boundaries.
Key changes:
- Adds a new constant
RAIN_TOTAL_INCREASING_SENSOR_KEYScontaining 14 rain sensor keys (both imperial and metric units) - Replaces hardcoded tuple check with the new constant to determine which sensors get
TOTAL_INCREASINGstate class - Expands the list from 2 sensors (totalrainin, totalrainmm) to 14 sensors including event, hourly, daily, weekly, monthly, and yearly variants
Comments suppressed due to low confidence (1)
homeassistant/components/ecowitt/sensor.py:310
- This integration lacks test coverage for sensor functionality. Consider adding tests to verify that the correct state_class is applied to rain sensors, especially given the recent changes to distinguish between moving-window and non-moving-window sensors. Tests should verify that sensors in RAIN_TOTAL_INCREASING_SENSOR_KEYS receive TOTAL_INCREASING state class while other rain sensors do not.
if sensor.key in RAIN_TOTAL_INCREASING_SENSOR_KEYS:
description = dataclasses.replace(
description,
state_class=SensorStateClass.TOTAL_INCREASING,
)
as the other pr is not linked to the issue, I have to admit that I simply missed the other PR. |
Co-authored-by: Copilot <[email protected]>
Proposed change
Fixes #159188
PR #155812 removed state class from accumulating rain sensor (weekly, hourly, last 24h, ....) because they are moving windows. However the truth is that not all rain sensor are moving windows - "last 24h" is, but not others - daily, hourly, yearly, etc.
This causes the error described in the issue.
This PR extends the list of sensor keys that are considered as "TOTAL_INCREASING", as they are not moving windows.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: