Change DHW comfort switch to select#883
Open
bouwew wants to merge 47 commits into
Open
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 23 minutes and 27 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 We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughRefactors DHW comfort mode control from a switch-based model (dhw_cm_switch) to a measurement-based select-like mode. Updates constants to add dhw_mode type field and remove legacy toggle mapping; adds helper methods to collect and map DHW modes with XML→comfort/off translation; removes dhw_cm_switch from switch models; applies conditional gateway entity field migration; updates all test fixtures to represent DHW state as dhw_modes array plus select_dhw_mode or water_heater_mode; adjusts test assertions and bumps version. ChangesDHW Mode Selection for Heater Central Devices
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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugwise/helper.py (1)
35-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused import.
The
TOGGLESconstant is imported but never used in this file.🧹 Proposed fix
- TOGGLES,🤖 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 `@plugwise/helper.py` at line 35, Remove the unused import TOGGLES from the import list in this module: locate the import statement that includes TOGGLES in plugwise/helper.py and delete TOGGLES (or the whole import if it becomes empty) so the file no longer imports an unused symbol.Source: Pipeline failures
tests/test_adam.py (1)
120-123:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winTest will fail:
dhw_cm_switchremoved from fixture.According to the layer description, the
adam_plus_anna_newfixture (loaded at line 35) has haddhw_cm_switchentries removed from switches objects. This test attempts to tinker withdhw_cm_switchon the heater_central device and expects success (line 123assert switch_change), but the switch no longer exists in the fixture data.🔧 Suggested fix options
Option 1: Remove the obsolete switch test
- switch_change = await self.tinker_switch( - api, - "056ee145a816487eaa69243c3280f8bf", - model="dhw_cm_switch", - ) - assert switch_changeOption 2: Update test to verify the new select entity
Replace the switch test with a test for the new
select_dhw_modefunctionality, if a corresponding test helper exists.🤖 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/test_adam.py` around lines 120 - 123, The test fails because the adam_plus_anna_new fixture no longer includes the dhw_cm_switch entity; update tests/test_adam.py by removing the obsolete switch assertion or replacing it with a check for the new select entity (e.g., change the call that targets model="dhw_cm_switch" on device "056ee145a816487eaa69243c3280f8bf" to instead target the new select entity name such as "select_dhw_mode" and assert its behavior), ensuring you update the invoked test helper call and the final assertion (currently "assert switch_change") to match the new entity and expected return value.
🧹 Nitpick comments (1)
plugwise/helper.py (1)
467-480: ⚡ Quick winClarify
old_measurementinitialization and scope.The variable
old_measurementis assigned on line 468 only whennew_nameexists, but it's referenced unconditionally on line 477. While this appears safe in practice (bothdomestic_hot_water_modeanddomestic_hot_water_comfort_modehave DATA mappings with names), the logic is fragile and not immediately obvious.♻️ Proposed refactor for clarity
def _appliance_measurements( self, appliance: etree.Element, data: GwEntityData, measurements: dict[str, DATA | UOM], ) -> None: """Helper-function for _get_measurement_data() - collect appliance measurement data.""" for measurement, attrs in measurements.items(): p_locator = f'.//logs/point_log[type="{measurement}"]/period/measurement' if (appl_p_loc := appliance.find(p_locator)) is not None: if skip_obsolete_measurements(appliance, measurement): continue + old_measurement = measurement if new_name := getattr(attrs, ATTR_NAME, None): - old_measurement = measurement measurement = new_name match measurement: case "elga_status_code": data["elga_status_code"] = int(appl_p_loc.text) case "select_dhw_mode": if self._dhw_allowed_modes: data["select_dhw_mode"] = appl_p_loc.text if old_measurement == "domestic_hot_water_comfort_mode": data["select_dhw_mode"] = ( "comfort" if appl_p_loc.text == "on" else "off" )This ensures
old_measurementis always defined before the match statement.🤖 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 `@plugwise/helper.py` around lines 467 - 480, old_measurement is only set when new_name exists but is referenced later unconditionally in the match branch for "select_dhw_mode"; initialize old_measurement to the current measurement before the conditional that may rename it so it always has a value (ensure the block using ATTR_NAME still updates measurement when new_name is present and preserves old_measurement for later checks), referencing the existing symbols measurement, old_measurement, ATTR_NAME, appl_p_loc, and self._dhw_allowed_modes to locate and adjust the logic.
🤖 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 `@plugwise/helper.py`:
- Around line 267-278: _collect_dhw_modes calls _get_toggle_state with None for
the data parameter which violates the declared GwEntityData (non-Optional) type;
either make _get_toggle_state accept Optional[GwEntityData] (update its
signature and handle None inside the function) or change the call in
_collect_dhw_modes to pass an empty dict/appropriate GwEntityData-shaped empty
value instead of None for the "domestic_hot_water_comfort_mode" /
"dhw_cm_switch" call so the types remain consistent.
---
Outside diff comments:
In `@plugwise/helper.py`:
- Line 35: Remove the unused import TOGGLES from the import list in this module:
locate the import statement that includes TOGGLES in plugwise/helper.py and
delete TOGGLES (or the whole import if it becomes empty) so the file no longer
imports an unused symbol.
In `@tests/test_adam.py`:
- Around line 120-123: The test fails because the adam_plus_anna_new fixture no
longer includes the dhw_cm_switch entity; update tests/test_adam.py by removing
the obsolete switch assertion or replacing it with a check for the new select
entity (e.g., change the call that targets model="dhw_cm_switch" on device
"056ee145a816487eaa69243c3280f8bf" to instead target the new select entity name
such as "select_dhw_mode" and assert its behavior), ensuring you update the
invoked test helper call and the final assertion (currently "assert
switch_change") to match the new entity and expected return value.
---
Nitpick comments:
In `@plugwise/helper.py`:
- Around line 467-480: old_measurement is only set when new_name exists but is
referenced later unconditionally in the match branch for "select_dhw_mode";
initialize old_measurement to the current measurement before the conditional
that may rename it so it always has a value (ensure the block using ATTR_NAME
still updates measurement when new_name is present and preserves old_measurement
for later checks), referencing the existing symbols measurement,
old_measurement, ATTR_NAME, appl_p_loc, and self._dhw_allowed_modes to locate
and adjust the logic.
🪄 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: c3649d18-c70a-480a-8b38-f2124773ded9
📒 Files selected for processing (10)
plugwise/constants.pyplugwise/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/test_adam.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #883 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3464 3475 +11
=========================================
+ Hits 3464 3475 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
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 `@tests/test_anna.py`:
- Line 443: The assertion at tests/test_anna.py that compares self.entity_items
to a hardcoded value (assert self.entity_items == 67) is invalid because the
fixtures' schema doesn't match that top-level count; change the test to derive
the expected entity count using the same enumeration logic used elsewhere in the
file (reuse the same iteration/filtering that computes self.entity_items in the
other checks around lines ~443/508/531/554) instead of a hardcoded literal, or
compute expected by walking each fixture's entries (including
dhw_modes/select_dhw_mode) with the same function/method that populates
self.entity_items so the assertion compares like-for-like; update only the
assertion site in tests/test_anna.py to use that computed expected value.
🪄 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: 60e13c73-fc5e-4b72-9d24-5d7fc876c892
📒 Files selected for processing (50)
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/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_p1/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/helper.pyplugwise/smile.pytests/data/adam/adam_heatpump_cooling.jsontests/data/anna/anna_elga_2.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_p1.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 (2)
- plugwise/smile.py
- plugwise/constants.py
✅ Files skipped from review due to trivial changes (5)
- fixtures/anna_v4/data.json
- tests/data/anna/anna_elga_2_cooling.json
- fixtures/anna_heatpump_cooling/data.json
- fixtures/m_anna_heatpump_idle/data.json
- fixtures/adam_onoff_cooling_fake_firmware/data.json
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/data/adam/adam_heatpump_cooling.json
- plugwise/helper.py
CoMPaTech
approved these changes
Jun 13, 2026
Contributor
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 (1)
tests/data/anna/anna_v4_dhw.json (1)
54-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winData inconsistency between DHW state sensors and water_heater_mode.
The fixture shows
dhw_state: trueandflame_state: true(lines 55-56), indicating the water heater is actively heating DHW, butwater_heater_modeis set to"off"(line 77). These values are contradictory. The filename suffix_dhwfurther suggests this fixture should represent an active DHW scenario, not an off state.Expected:
water_heater_mode: "comfort"to match the active DHW state.🔧 Proposed fix
- "water_heater_mode": "off", + "water_heater_mode": "comfort",🤖 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_v4_dhw.json` around lines 54 - 77, The fixture has inconsistent DHW sensor fields: dhw_state and flame_state are true but water_heater_mode is "off"; update the water_heater_mode field in the JSON (the "water_heater_mode" property) to "comfort" so it matches the active DHW scenario (dhw_state/flame_state true and dhw_modes includes "comfort").
🧹 Nitpick comments (2)
plugwise/smile.py (1)
231-246: 💤 Low valueDeferred location validation may reduce error clarity.
The signature change to
loc_id: str | Nonecorrectly reflects that DHW mode selection (lines 236-237) doesn't require a location ID, while schedule and zone profile operations do. However, the validation for requiredloc_idis now deferred to the called methods (set_schedule_stateandset_zone_profile) rather than validated at theset_selectentry point.This means callers who forget to pass
loc_idfor schedule/profile operations will get an error message from the inner method rather than from the top-level handler, which may be less clear.Consider adding early validation in
set_select:case "select_schedule" | "select_zone_profile": if loc_id is None: raise PlugwiseError(f"Plugwise: location required for {key}.")🤖 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 `@plugwise/smile.py` around lines 231 - 246, The set_select handler defers loc_id validation to inner methods (set_schedule_state, set_zone_profile), so add an early check in set_select for the cases that require a location: in the match arm for "select_schedule" and "select_zone_profile" validate if loc_id is None and raise PlugwiseError(f"Plugwise: location required for {key}.") before calling set_schedule_state or set_zone_profile; leave the other branches (set_dhw_mode, set_gateway_mode, set_regulation_mode) unchanged since they do not need a loc_id.plugwise/constants.py (1)
183-184: Clarify DHW mode mapping:domestic_hot_water_modeanddomestic_hot_water_comfort_modeintentionally feed the sameselect_dhw_modeBoth entries in
HEATER_CENTRAL_MEASUREMENTSmap to the same payload key ("select_dhw_mode"), and that’s used as a compatibility normalization:domestic_hot_water_comfort_modeis specifically converted from toggle values (on/off) intocomfort/off, and it’s also used as the fallback when actuatordomestic_hot_water_mode_control_functionalitydoesn’t expose allowed modes. Add a short comment near the constants to document that these two measurements are alternate sources for the same DHW-mode concept.🤖 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 `@plugwise/constants.py` around lines 183 - 184, Add a short clarifying comment next to the HEATER_CENTRAL_MEASUREMENTS constant explaining that "domestic_hot_water_mode" and "domestic_hot_water_comfort_mode" intentionally map to the same payload key "select_dhw_mode" as alternate sources for the same DHW-mode concept; note that domestic_hot_water_comfort_mode is derived from toggle values (on/off → comfort/off) and that it serves as a fallback when the actuator key domestic_hot_water_mode_control_functionality does not expose allowed modes.
🤖 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 `@plugwise/data.py`:
- Around line 81-86: The remapping currently gated on "max_dhw_temperature"
should instead trigger when the DHW mode info is present; change the condition
that reads if "max_dhw_temperature" in entity: to check for DHW mode capability
such as if "select_dhw_mode" in entity (or if entity.get("select_dhw_mode") is
not None / or presence of "dhw_modes"), so the existing block that pops
"select_dhw_mode" and sets "water_heater_mode" runs whenever select_dhw_mode/DHW
mode support is present.
In `@plugwise/smile.py`:
- Around line 340-345: The validation is unreachable because the combined
condition checks "or loc_id is None" with new_state, causing the generic
PlugwiseError to be raised before the specific loc_id check; change the logic so
you first validate new_state against (STATE_OFF, STATE_ON) and raise
PlugwiseError if invalid, then separately check loc_id is None and raise the
more specific PlugwiseError message; update the condition that currently uses
"if new_state not in (STATE_OFF, STATE_ON) or loc_id is None:" to only test
new_state, and keep the existing loc_id None branch as the second independent
check.
---
Outside diff comments:
In `@tests/data/anna/anna_v4_dhw.json`:
- Around line 54-77: The fixture has inconsistent DHW sensor fields: dhw_state
and flame_state are true but water_heater_mode is "off"; update the
water_heater_mode field in the JSON (the "water_heater_mode" property) to
"comfort" so it matches the active DHW scenario (dhw_state/flame_state true and
dhw_modes includes "comfort").
---
Nitpick comments:
In `@plugwise/constants.py`:
- Around line 183-184: Add a short clarifying comment next to the
HEATER_CENTRAL_MEASUREMENTS constant explaining that "domestic_hot_water_mode"
and "domestic_hot_water_comfort_mode" intentionally map to the same payload key
"select_dhw_mode" as alternate sources for the same DHW-mode concept; note that
domestic_hot_water_comfort_mode is derived from toggle values (on/off →
comfort/off) and that it serves as a fallback when the actuator key
domestic_hot_water_mode_control_functionality does not expose allowed modes.
In `@plugwise/smile.py`:
- Around line 231-246: The set_select handler defers loc_id validation to inner
methods (set_schedule_state, set_zone_profile), so add an early check in
set_select for the cases that require a location: in the match arm for
"select_schedule" and "select_zone_profile" validate if loc_id is None and raise
PlugwiseError(f"Plugwise: location required for {key}.") before calling
set_schedule_state or set_zone_profile; leave the other branches (set_dhw_mode,
set_gateway_mode, set_regulation_mode) unchanged since they do not need a
loc_id.
🪄 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: 31c1a149-0f03-4f9c-9e0b-cb9bb8d0f368
📒 Files selected for processing (35)
CHANGELOG.mdfixtures/adam_bad_thermostat/data.jsonfixtures/adam_heatpump_cooling/data.jsonfixtures/adam_jip/data.jsonfixtures/adam_onoff_cooling_fake_firmware/data.jsonfixtures/anna_elga_no_cooling/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_jip/data.jsonfixtures/m_anna_heatpump_cooling/data.jsonfixtures/m_anna_heatpump_idle/data.jsonplugwise/__init__.pyplugwise/constants.pyplugwise/data.pyplugwise/helper.pyplugwise/smile.pypyproject.tomltests/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/anna/anna_elga_no_cooling.jsontests/data/anna/anna_heatpump_heating.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.json
✅ Files skipped from review due to trivial changes (11)
- pyproject.toml
- CHANGELOG.md
- tests/data/anna/anna_loria_cooling_active.json
- tests/data/adam/adam_jip.json
- fixtures/m_anna_heatpump_cooling/data.json
- plugwise/init.py
- tests/data/adam/adam_heatpump_cooling.json
- fixtures/anna_v4_no_tag/data.json
- fixtures/anna_loria_heating_idle/data.json
- tests/data/adam/adam_onoff_cooling_fake_firmware.json
- fixtures/anna_heatpump_heating/data.json
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/data/adam/adam_bad_thermostat.json
- plugwise/helper.py
Comment on lines
+81
to
+86
| # Replace select_dhw_mode with water_heater_mode when applicable | ||
| if "max_dhw_temperature" in entity: | ||
| mode = entity.get("select_dhw_mode") | ||
| if mode is not None: | ||
| entity.pop("select_dhw_mode") | ||
| entity["water_heater_mode"] = mode |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if all heater_central entities with DHW state also have max_dhw_temperature
# Find heater_central entities in test fixtures
rg -A5 '"dev_class": "heater_central"' fixtures/ --json | \
jq -r 'select(.type=="match") | .data.path.text' | sort -u | while read fixture; do
echo "=== $fixture ==="
jq 'to_entries[] | select(.value.dev_class == "heater_central") |
{id: .key, has_dhw_state: (.value.binary_sensors.dhw_state != null), has_max_dhw: (.value.max_dhw_temperature != null), has_dhw_modes: (.value.dhw_modes != null), has_water_heater_mode: (.value.water_heater_mode != null)}' "$fixture"
doneRepository: plugwise/python-plugwise
Length of output: 6729
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) See if fixtures contain select_dhw_mode anywhere
echo "=== Files containing select_dhw_mode ==="
rg -n '"select_dhw_mode"' fixtures/ | head -n 50 || true
# 2) For the specific fixture JSONs that had dhw_modes but no max_dhw and no water_heater_mode,
# check whether select_dhw_mode exists.
targets=(
"fixtures/adam_plus_anna/data.json"
"fixtures/adam_plus_anna_new/data.json"
"fixtures/adam_plus_anna_new_regulation_off/data.json"
"fixtures/anna_elga_2/data.json"
"fixtures/anna_p1/data.json"
)
for f in "${targets[@]}"; do
echo
echo "=== Inspecting $f ==="
jq -r '
. as $root
| to_entries[]
| select(.value.dev_class == "heater_central")
| {
id: .key,
has_max_dhw: (.value.max_dhw_temperature != null),
has_select_dhw_mode: (.value.select_dhw_mode != null),
has_water_heater_mode: (.value.water_heater_mode != null),
has_dhw_modes: (.value.dhw_modes != null)
}' "$f"
doneRepository: plugwise/python-plugwise
Length of output: 2082
Fix DHW mode remapping discriminator in plugwise/data.py (lines 81-86).
The remap select_dhw_mode → water_heater_mode only runs when "max_dhw_temperature" in entity, but fixture data shows DHW mode support exists without max_dhw_temperature (e.g., fixtures/adam_plus_anna*/data.json, fixtures/anna_elga_2/data.json, fixtures/anna_p1/data.json: select_dhw_mode + dhw_modes present while water_heater_mode is missing). Update the condition to key off select_dhw_mode/DHW mode capability instead of max_dhw_temperature.
🤖 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 `@plugwise/data.py` around lines 81 - 86, The remapping currently gated on
"max_dhw_temperature" should instead trigger when the DHW mode info is present;
change the condition that reads if "max_dhw_temperature" in entity: to check for
DHW mode capability such as if "select_dhw_mode" in entity (or if
entity.get("select_dhw_mode") is not None / or presence of "dhw_modes"), so the
existing block that pops "select_dhw_mode" and sets "water_heater_mode" runs
whenever select_dhw_mode/DHW mode support is present.
Contributor
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 `@plugwise/data.py`:
- Line 81: Update the comment that describes the DHW field remapping to use the
correct target field name. The comment currently references water_heater_mode
but should reference dhw_mode to accurately reflect the actual field mapping
being performed in the code. Change the comment text to state that
select_dhw_mode is mapped to dhw_mode.
🪄 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: cc4aa1f8-50e3-4c1a-a252-bc8a1e08c13a
📒 Files selected for processing (60)
CHANGELOG.mdfixtures/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/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_p1/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/__init__.pyplugwise/constants.pyplugwise/data.pyplugwise/helper.pyplugwise/smile.pypyproject.tomltests/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.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_p1.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/smile.py
✅ Files skipped from review due to trivial changes (17)
- tests/data/anna/anna_elga_no_cooling.json
- fixtures/anna_elga_2_cooling/data.json
- tests/data/anna/anna_v4_UPDATED_DATA.json
- fixtures/anna_heatpump_heating/data.json
- tests/data/anna/anna_elga_2.json
- tests/data/anna/anna_loria_heating_idle.json
- tests/data/anna/anna_v4.json
- tests/data/adam/adam_plus_anna_new_regulation_off.json
- tests/data/anna/anna_heatpump_heating_UPDATED_DATA.json
- tests/data/anna/anna_heatpump_cooling.json
- pyproject.toml
- tests/data/anna/anna_elga_2_cooling.json
- tests/data/anna/anna_elga_2_schedule_off.json
- fixtures/anna_v4/data.json
- fixtures/anna_v4_no_tag/data.json
- fixtures/adam_plus_anna/data.json
- fixtures/anna_loria_heating_idle/data.json
🚧 Files skipped from review as they are similar to previous changes (21)
- CHANGELOG.md
- fixtures/m_anna_heatpump_idle/data.json
- fixtures/m_anna_heatpump_cooling/data.json
- fixtures/adam_jip/data.json
- tests/data/adam/adam_plus_anna.json
- fixtures/anna_p1/data.json
- plugwise/init.py
- fixtures/m_adam_heating_off_schedule/data.json
- fixtures/anna_elga_2_schedule_off/data.json
- fixtures/anna_elga_2/data.json
- tests/data/adam/adam_plus_anna_new.json
- tests/data/adam/adam_onoff_cooling_fake_firmware.json
- tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json
- tests/data/anna/anna_p1.json
- fixtures/adam_plus_anna_new/data.json
- fixtures/anna_heatpump_cooling/data.json
- tests/data/anna/anna_heatpump_cooling_fake_firmware.json
- fixtures/anna_loria_driessens/data.json
- tests/test_adam.py
- tests/test_anna.py
- plugwise/helper.py
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation