Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Entity device-class declarations Integrations/ESPHome/Core.yaml |
Device classes were added to the DPS310 pressure offset and pressure sensors, ESP temperature, Wi-Fi RSSI, CO₂, SEN55 measurements, and MICS-4514 gas sensors. The AQI device-class value was quoted. SEN55 accuracy settings and temperature filtering remain unchanged. |
Priority:
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Bug fix
Suggested reviewers: bharvey88
Merge Risk: 
1dda1
-
No merge-blocking risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding device classes to multiple entities in Core.yaml. Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each sensor’s name,
And adds its class beside the same.
The pressure, gases, warmth, and air,
Now list their types with tidy care.
The AQI quote is set in place,
While SEN55 keeps its settings’ trace.
Comment @coderabbitai help to get the list of available commands.
Most importantly, set atmospheric_pressure on pressure, because it was failing to graph together with other atmospheric_pressure entities. Somehow, it ended up as pressure, despite not having a device class set. Put aqi's device class in quotes, per the esphome docs.
Most importantly, set atmospheric_pressure on pressure, because it was failing to graph together with other atmospheric_pressure entities. Somehow, it ended up as pressure, despite not having a device class set.
Put aqi's device class in quotes, per the esphome docs.
Version: 26.10.16.1
(I don't understand the point of this question; really the branch should be based on recent beta, and if anything the question is which beta commit it's been tested against.)
What does this implement/fix?
The config does not set device classes for a number of entities, and HA best practice is to do so. In particular, the pressure sensor defaults to pressure, which is for things other than atmospheric pressure. This PR sets the device class to atmospheric_pressure.
My expectation is that users will not notice this, except for "why is the DSP310 pressure graphed separately from my other barometer sensors" will turn into "they are graphed together, of course, so I didn't notice or think about this".
Types of changes
Checklist:
If user-visible functionality or configuration variables are added/modified:
Summary by CodeRabbit