Further improvement on HA water_heater related output data#888
Conversation
|
Warning Review limit reached
More reviews will be available in 4 minutes and 45 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (51)
📝 WalkthroughWalkthroughReplaces the flat ChangesBoiler/DHW Temperature Schema Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #888 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3484 3502 +18
=========================================
+ Hits 3484 3502 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
fixtures/m_adam_heating_off_schedule/data.json (1)
9-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMigrate remaining legacy DHW fields in this fixture as well.
Line 22 still contains
max_dhw_temperatureand Line 33 still exposessensors.water_temperature, so this fixture does not fully match the consolidated actuator contract.Suggested fixture shape update
- "max_dhw_temperature": { - "lower_bound": 40.0, - "resolution": 0.01, - "setpoint": 60.0, - "upper_bound": 60.0 - }, + "dhw_temperature": { + "current": 37.0, + "lower_bound": 40.0, + "resolution": 0.01, + "setpoint": 60.0, + "upper_bound": 60.0 + }, @@ - "water_temperature": 37.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fixtures/m_adam_heating_off_schedule/data.json` around lines 9 - 34, The fixture data.json file still contains legacy DHW fields that need to be removed to match the consolidated actuator contract. Remove the max_dhw_temperature object (which contains lower_bound, resolution, setpoint, and upper_bound properties) and remove the water_temperature field from the sensors object. Keep only the required fields that are part of the current actuator contract for the heater_central device class.fixtures/m_adam_heating/data.json (1)
9-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winComplete the DHW fixture migration to the new schema.
Line 22 still uses legacy
max_dhw_temperatureand Line 33 still keepssensors.water_temperature. This leaves the fixture in a mixed old/new contract state after introducingboiler_temperature.Suggested fixture shape update
- "max_dhw_temperature": { - "lower_bound": 40.0, - "resolution": 0.01, - "setpoint": 60.0, - "upper_bound": 60.0 - }, + "dhw_temperature": { + "current": 37.0, + "lower_bound": 40.0, + "resolution": 0.01, + "setpoint": 60.0, + "upper_bound": 60.0 + }, @@ - "water_temperature": 37.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fixtures/m_adam_heating/data.json` around lines 9 - 34, The fixture file fixtures/m_adam_heating/data.json is in a mixed state between old and new schema. Remove the legacy `max_dhw_temperature` object (which spans multiple lines around the setpoint and bounds fields) and remove the `sensors.water_temperature` property from the sensors object to complete the migration to the new schema that now includes boiler_temperature. This will ensure the fixture uses only the new contract structure without mixing old and new field definitions.fixtures/m_anna_heatpump_cooling/data.json (1)
54-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove duplicate temperature sensors after actuator migration.
Line 55 (
sensors.dhw_temperature) and Line 61 (sensors.water_temperature) still expose values that are now represented asdhw_temperature.currentandboiler_temperature.current. This fixture is out of sync with the consolidated schema used in the other updated fixtures and can create conflicting test expectations.Suggested fixture adjustment
"sensors": { - "dhw_temperature": 41.5, "intended_boiler_temperature": 0.0, "modulation_level": 40, "outdoor_air_temperature": 28.0, "return_temperature": 23.8, - "water_pressure": 1.57, - "water_temperature": 22.7 + "water_pressure": 1.57 },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fixtures/m_anna_heatpump_cooling/data.json` around lines 54 - 62, The sensors object in the fixtures contains duplicate temperature readings that conflict with the actuator migration schema. Remove the dhw_temperature property (currently 41.5) and the water_temperature property (currently 22.7) from the sensors object, as these values are now properly represented through the consolidated actuator schema as dhw_temperature.current and boiler_temperature.current respectively. This will bring the fixture in line with the updated schema used in other fixtures.fixtures/m_anna_heatpump_idle/data.json (1)
44-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove duplicated legacy scalar temperature sensors after promoting to top-level objects.
Line 44 introduces
dhw_temperatureas the new structured object, but Lines 55 and 61 still keep legacy scalarsensors.dhw_temperatureandsensors.water_temperature. This leaves this fixture in a mixed schema state versus the rest of the cohort and can skew downstream entity extraction/counts.Suggested fixture correction
"sensors": { - "dhw_temperature": 46.3, "intended_boiler_temperature": 18.0, "modulation_level": 0, "outdoor_air_temperature": 28.2, "return_temperature": 22.0, - "water_pressure": 1.57, - "water_temperature": 19.1 + "water_pressure": 1.57 },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fixtures/m_anna_heatpump_idle/data.json` around lines 44 - 62, The fixture contains duplicate temperature sensor definitions in a mixed schema state. The dhw_temperature and water_temperature sensors are defined both as structured top-level objects (with properties like current, setpoint, bounds) and as legacy scalar values nested under the sensors object. Remove the duplicate scalar sensor entries dhw_temperature (currently 46.3) and water_temperature (currently 19.1) from the sensors object to maintain consistency with the rest of the fixture cohort and prevent schema confusion in downstream entity extraction.tests/data/anna/anna_heatpump_cooling.json (1)
47-50:⚠️ Potential issue | 🟠 MajorComplete the DHW schema migration for this fixture.
The
anna_heatpump_cooling.jsonfixture still contains legacy DHW sensor fields (dhw_temperatureanddomestic_hot_water_setpointin sensors) while the equivalentanna_heatpump_heating.jsonhas been fully migrated to the structureddhw_temperatureobject format. Both fixtures reference the same entity (1cbf783bb11e4a7c8a6843dee3a86927) and are actively tested. Update lines 48-49 and remove these legacy sensor fields to complete the migration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/data/anna/anna_heatpump_cooling.json` around lines 47 - 50, The anna_heatpump_cooling.json fixture contains legacy DHW sensor fields (dhw_temperature and domestic_hot_water_setpoint) in the sensors object that should be removed to match the schema migration already completed in anna_heatpump_heating.json. Remove the dhw_temperature and domestic_hot_water_setpoint fields from the sensors object in anna_heatpump_cooling.json to align with the structured dhw_temperature object format used in the heating fixture, ensuring both fixtures maintain consistent schema structure since they reference the same entity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fixtures/m_adam_cooling/data.json`:
- Around line 10-16: The fixture contains conflicting temperature values:
boiler_temperature.current is set to 43.0 while sensors.water_temperature exists
separately at 19.0. To complete the schema migration, remove the standalone
sensors.water_temperature key entirely, as boiler_temperature.current is now the
consolidated representation for temperature. This ensures the fixture maintains
a single source of truth for the heater temperature value.
In `@plugwise/helper.py`:
- Around line 589-604: The set_number() function in plugwise/smile.py needs to
update its setter key remapping to handle the new actuator names being emitted
from helper.py. Currently it only remaps max_dhw_temperature, but needs to also
include remapping for the new keys dhw_temperature and boiler_temperature to
ensure proper thermostat-functionality lookup. Update the remapping logic in
set_number() to add these new actuator name mappings alongside the existing key
remappings.
---
Outside diff comments:
In `@fixtures/m_adam_heating_off_schedule/data.json`:
- Around line 9-34: The fixture data.json file still contains legacy DHW fields
that need to be removed to match the consolidated actuator contract. Remove the
max_dhw_temperature object (which contains lower_bound, resolution, setpoint,
and upper_bound properties) and remove the water_temperature field from the
sensors object. Keep only the required fields that are part of the current
actuator contract for the heater_central device class.
In `@fixtures/m_adam_heating/data.json`:
- Around line 9-34: The fixture file fixtures/m_adam_heating/data.json is in a
mixed state between old and new schema. Remove the legacy `max_dhw_temperature`
object (which spans multiple lines around the setpoint and bounds fields) and
remove the `sensors.water_temperature` property from the sensors object to
complete the migration to the new schema that now includes boiler_temperature.
This will ensure the fixture uses only the new contract structure without mixing
old and new field definitions.
In `@fixtures/m_anna_heatpump_cooling/data.json`:
- Around line 54-62: The sensors object in the fixtures contains duplicate
temperature readings that conflict with the actuator migration schema. Remove
the dhw_temperature property (currently 41.5) and the water_temperature property
(currently 22.7) from the sensors object, as these values are now properly
represented through the consolidated actuator schema as dhw_temperature.current
and boiler_temperature.current respectively. This will bring the fixture in line
with the updated schema used in other fixtures.
In `@fixtures/m_anna_heatpump_idle/data.json`:
- Around line 44-62: The fixture contains duplicate temperature sensor
definitions in a mixed schema state. The dhw_temperature and water_temperature
sensors are defined both as structured top-level objects (with properties like
current, setpoint, bounds) and as legacy scalar values nested under the sensors
object. Remove the duplicate scalar sensor entries dhw_temperature (currently
46.3) and water_temperature (currently 19.1) from the sensors object to maintain
consistency with the rest of the fixture cohort and prevent schema confusion in
downstream entity extraction.
In `@tests/data/anna/anna_heatpump_cooling.json`:
- Around line 47-50: The anna_heatpump_cooling.json fixture contains legacy DHW
sensor fields (dhw_temperature and domestic_hot_water_setpoint) in the sensors
object that should be removed to match the schema migration already completed in
anna_heatpump_heating.json. Remove the dhw_temperature and
domestic_hot_water_setpoint fields from the sensors object in
anna_heatpump_cooling.json to align with the structured dhw_temperature object
format used in the heating fixture, ensuring both fixtures maintain consistent
schema structure since they reference the same entity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00dae7c1-4649-48a9-a235-3d078ff131ce
📒 Files selected for processing (53)
fixtures/adam_bad_thermostat/data.jsonfixtures/adam_heatpump_cooling/data.jsonfixtures/adam_jip/data.jsonfixtures/adam_onoff_cooling_fake_firmware/data.jsonfixtures/adam_plus_anna/data.jsonfixtures/adam_plus_anna_new/data.jsonfixtures/adam_plus_anna_new_regulation_off/data.jsonfixtures/anna_elga_2_cooling/data.jsonfixtures/anna_elga_2_schedule_off/data.jsonfixtures/anna_elga_no_cooling/data.jsonfixtures/anna_heatpump_cooling/data.jsonfixtures/anna_heatpump_cooling_fake_firmware/data.jsonfixtures/anna_heatpump_heating/data.jsonfixtures/anna_loria_cooling_active/data.jsonfixtures/anna_loria_driessens/data.jsonfixtures/anna_loria_heating_idle/data.jsonfixtures/anna_v4/data.jsonfixtures/anna_v4_dhw/data.jsonfixtures/anna_v4_no_tag/data.jsonfixtures/m_adam_cooling/data.jsonfixtures/m_adam_heating/data.jsonfixtures/m_adam_heating_off_schedule/data.jsonfixtures/m_adam_jip/data.jsonfixtures/m_anna_heatpump_cooling/data.jsonfixtures/m_anna_heatpump_idle/data.jsonplugwise/constants.pyplugwise/data.pyplugwise/helper.pyplugwise/legacy/helper.pytests/data/adam/adam_bad_thermostat.jsontests/data/adam/adam_heatpump_cooling.jsontests/data/adam/adam_jip.jsontests/data/adam/adam_onoff_cooling_fake_firmware.jsontests/data/adam/adam_plus_anna.jsontests/data/adam/adam_plus_anna_new.jsontests/data/adam/adam_plus_anna_new_regulation_off.jsontests/data/anna/anna_elga_2_cooling.jsontests/data/anna/anna_elga_2_cooling_UPDATED_DATA.jsontests/data/anna/anna_elga_2_schedule_off.jsontests/data/anna/anna_elga_no_cooling.jsontests/data/anna/anna_heatpump_cooling.jsontests/data/anna/anna_heatpump_cooling_fake_firmware.jsontests/data/anna/anna_heatpump_heating.jsontests/data/anna/anna_heatpump_heating_UPDATED_DATA.jsontests/data/anna/anna_loria_cooling_active.jsontests/data/anna/anna_loria_driessens.jsontests/data/anna/anna_loria_heating_idle.jsontests/data/anna/anna_v4.jsontests/data/anna/anna_v4_UPDATED_DATA.jsontests/data/anna/anna_v4_dhw.jsontests/data/anna/anna_v4_no_tag.jsontests/test_adam.pytests/test_anna.py
💤 Files with no reviewable changes (1)
- plugwise/legacy/helper.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fixtures/m_adam_heating/data.json`:
- Around line 21-27: The out-of-bounds DHW temperature values exist in four
fixture files (m_adam_heating/data.json, m_adam_heating_off_schedule/data.json,
m_adam_jip/data.json, and adam_jip/data.json) where current values fall below
lower_bound. Address this by either: (1) implementing runtime validation in the
ActuatorData class to enforce that lower_bound ≤ current ≤ upper_bound
constraints, or (2) adding documentation to these fixture files explaining that
the current values intentionally represent device warmup/coldstart states where
temperature may temporarily fall outside nominal bounds. Choose one approach and
apply it consistently across all affected fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6ca3a1f-9710-4dc4-b9e2-24bbf2ba9342
📒 Files selected for processing (7)
fixtures/m_adam_cooling/data.jsonfixtures/m_adam_heating/data.jsonfixtures/m_adam_heating_off_schedule/data.jsonfixtures/m_anna_heatpump_cooling/data.jsonfixtures/m_anna_heatpump_idle/data.jsonplugwise/helper.pyscripts/manual_fixtures.py
🚧 Files skipped from review as they are similar to previous changes (3)
- fixtures/m_anna_heatpump_cooling/data.json
- fixtures/m_anna_heatpump_idle/data.json
- plugwise/helper.py
|
1 participant
Summary by CodeRabbit
Refactor
currentplus configuration bounds.Tests