Feature request: add setting of Adam thermostat zone profile #814
Conversation
|
Warning Rate limit exceeded@bouwew has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds zone-profile support: new fixture/test fields ( Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant API as SmileAPI
participant Data as SmileData
participant Helper as SmileHelper
Note over Test,API: Set zone profile via public API
Test->>API: set_select("select_zone_profile", loc_id, profile)
API->>API: set_zone_profile(loc_id, profile)
API->>API: validate profile ∈ ALLOWED_ZONE_PROFILES
API->>API: build XML payload
API->>API: POST /thermostat/<loc_id>
Note over Test,Data: Data parsing flow on fetch
Test->>Data: _get_location_data(raw)
Data->>Helper: _regulation_control(raw)
Note right of Helper: extract regulation_control -> set `select_zone_profile` & `zone_profiles`
Data->>Helper: _control_state(raw)
Note right of Helper: read control_state and continue parsing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 #814 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3390 3439 +49
=========================================
+ Hits 3390 3439 +49 ☔ View full report in Codecov by Sentry. 🚀 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: 1
🧹 Nitpick comments (2)
plugwise/data.py (1)
162-167: Approve with optional refactor suggestion.The updated
_control_stateinvocation correctly removes theloc_idparameter to match the new signature. The condition properly handles thestr | boolreturn type.On Line 167, the
str()cast is redundant sincectrl_stateis guaranteed to be a string at this point (it passed the membership test on line 162). Consider simplifying:- data["control_state"] = str(ctrl_state) + data["control_state"] = ctrl_stateplugwise/helper.py (1)
848-852: Keep_countaccounting explicit for the new keys.Right now we only bump
_countonce after adding bothselect_regulation_controlandregulation_control_modes, and we rely on the earlier increment from the actuator dict (before thepop) to balance things out. That coupling makes the tally fragile if a future firmware ever skips the nested key but still expects these selects to be surfaced. Please subtract one when popping from the actuator payload and then increment once per new top-level key so the counter reflects the actual items we expose.
Proposed adjustment:- if (reg_control := data["thermostat"].get("regulation_control")) is not None: - data["select_regulation_control"] = reg_control - data["regulation_control_modes"] = ["active", "off", "passive"] - data["thermostat"].pop("regulation_control") - self._count += 1 + if (reg_control := data["thermostat"].get("regulation_control")) is not None: + data["thermostat"].pop("regulation_control") + self._count -= 1 + data["select_regulation_control"] = reg_control + self._count += 1 + data["regulation_control_modes"] = ["active", "off", "passive"] + self._count += 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
fixtures/adam_heatpump_cooling/data.json(10 hunks)fixtures/adam_jip/data.json(4 hunks)fixtures/adam_onoff_cooling_fake_firmware/data.json(1 hunks)fixtures/adam_plus_anna_new/data.json(2 hunks)fixtures/adam_plus_anna_new_regulation_off/data.json(2 hunks)fixtures/m_adam_cooling/data.json(2 hunks)fixtures/m_adam_heating/data.json(2 hunks)fixtures/m_adam_jip/data.json(4 hunks)plugwise/constants.py(4 hunks)plugwise/data.py(1 hunks)plugwise/helper.py(3 hunks)scripts/tests_and_coverage.sh(1 hunks)tests/data/adam/adam_heatpump_cooling.json(10 hunks)tests/data/adam/adam_jip.json(4 hunks)tests/data/adam/adam_onoff_cooling_fake_firmware.json(1 hunks)tests/data/adam/adam_plus_anna_new.json(2 hunks)tests/data/adam/adam_plus_anna_new_regulation_off.json(2 hunks)tests/test_adam.py(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
Applied to files:
fixtures/m_adam_heating/data.jsontests/data/adam/adam_plus_anna_new_regulation_off.jsontests/data/adam/adam_jip.jsonfixtures/m_adam_jip/data.jsonfixtures/adam_plus_anna_new/data.jsonfixtures/adam_heatpump_cooling/data.jsontests/data/adam/adam_heatpump_cooling.jsonfixtures/adam_onoff_cooling_fake_firmware/data.jsontests/data/adam/adam_plus_anna_new.jsonfixtures/adam_jip/data.jsonfixtures/m_adam_cooling/data.jsontests/data/adam/adam_onoff_cooling_fake_firmware.jsonfixtures/adam_plus_anna_new_regulation_off/data.json
📚 Learning: 2024-12-13T11:26:00.100Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 671
File: plugwise/legacy/data.py:97-106
Timestamp: 2024-12-13T11:26:00.100Z
Learning: In `plugwise/legacy/data.py`, the legacy thermostat does not support cooling, so handling of the cooling state is unnecessary in this code.
Applied to files:
plugwise/helper.py
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
Applied to files:
plugwise/helper.py
🧬 Code graph analysis (2)
plugwise/helper.py (2)
plugwise/util.py (1)
format_measure(196-214)plugwise/constants.py (1)
GwEntityData(519-581)
plugwise/data.py (1)
plugwise/helper.py (2)
_regulation_control(843-852)_control_state(798-824)
🔇 Additional comments (24)
fixtures/adam_plus_anna_new/data.json (1)
282-283: LGTM! Consistent regulation control field additions.The new
regulation_control_modesandselect_regulation_controlfields are consistently added to both ThermoZone entries with appropriate values.Also applies to: 321-322
scripts/tests_and_coverage.sh (1)
61-62: Verify the removal of coverage reporting.The pytest invocation has been changed to remove coverage options. This means code coverage will no longer be tracked or reported. Please confirm this change is intentional, as coverage metrics are typically important for maintaining code quality.
If coverage was removed temporarily for development purposes, consider re-enabling it before merging this PR.
fixtures/m_adam_cooling/data.json (1)
198-199: LGTM! Consistent with other fixture updates.The regulation control fields are properly added to both ThermoZone entries in this cooling mode fixture.
Also applies to: 237-238
fixtures/adam_jip/data.json (1)
11-12: LGTM! All ThermoZone entries properly updated.The regulation control fields are consistently added to all four ThermoZone entries in this fixture.
Also applies to: 38-39, 255-256, 306-307
fixtures/m_adam_jip/data.json (1)
10-11: LGTM! Consistent regulation control additions.All ThermoZone entries are properly updated with regulation control fields.
Also applies to: 37-38, 254-255, 305-306
fixtures/m_adam_heating/data.json (1)
197-198: LGTM! Proper regulation control configuration.Both ThermoZone entries are correctly updated with regulation control fields, with appropriate mode selections per zone.
Also applies to: 236-237
fixtures/adam_onoff_cooling_fake_firmware/data.json (1)
94-95: LGTM! Regulation control fields properly added.The single ThermoZone entry in this fixture is correctly updated.
tests/test_adam.py (4)
37-37: LGTM! Entity count correctly updated.The entity count increase from 216 to 220 correctly reflects the addition of 2 regulation control fields for each of the 2 ThermoZone entries in this fixture (2 zones × 2 fields = 4 new entities).
324-324: LGTM! Entity count correctly updated.The entity count increase from 518 to 538 correctly reflects the addition of regulation control fields across multiple ThermoZone entries in the heatpump cooling fixture.
347-347: LGTM! Entity count correctly updated.The entity count increase from 67 to 69 correctly reflects the addition of 2 regulation control fields for the single ThermoZone entry in this fixture.
411-411: LGTM! Entity count correctly updated.The entity count increase from 261 to 269 correctly reflects the addition of 2 regulation control fields for each of the 4 ThermoZone entries in the adam_jip fixture (4 zones × 2 fields = 8 new entities).
tests/data/adam/adam_onoff_cooling_fake_firmware.json (1)
95-96: LGTM!The addition of
regulation_control_modesandselect_regulation_controlfields to the ThermoZone entry is consistent with the PR's objective to add regulation control support. The values are appropriate for the data schema.tests/data/adam/adam_jip.json (1)
12-13: LGTM!The regulation control fields are consistently added to all ThermoZone entries. The uniform values across all zones (all set to "active") align with the expected data structure for this test fixture.
Also applies to: 39-40, 256-257, 307-308
tests/data/adam/adam_plus_anna_new_regulation_off.json (1)
236-237: LGTM!The different
select_regulation_controlvalues between zones (Living room: "active", Bathroom: "passive") appropriately test various regulation control configurations, which is valuable for comprehensive test coverage.Also applies to: 271-272
tests/data/adam/adam_heatpump_cooling.json (1)
17-18: LGTM!The regulation control fields are consistently added across all ThermoZone entries. The mix of "active" and "passive" modes across different zones provides good test coverage for various regulation control scenarios in a heatpump cooling configuration.
Also applies to: 137-138, 253-254, 397-398, 431-432, 484-485, 538-539, 572-573, 704-705, 808-809
tests/data/adam/adam_plus_anna_new.json (1)
283-284: LGTM!The regulation control configuration matches the expected pattern, with different modes between zones (Living room: "active", Bathroom: "passive") providing appropriate test coverage.
Also applies to: 322-323
fixtures/adam_plus_anna_new_regulation_off/data.json (1)
235-236: LGTM!The fixture data correctly includes the regulation control fields with appropriate values matching the test data expectations.
Also applies to: 270-271
fixtures/adam_heatpump_cooling/data.json (1)
16-17: LGTM!The fixture data is consistent with the corresponding test data, properly including regulation control fields across all ThermoZone entries with appropriate values for heatpump cooling scenarios.
Also applies to: 136-137, 252-253, 396-397, 430-431, 483-484, 537-538, 571-572, 703-704, 807-808
plugwise/data.py (1)
158-158: LGTM!The addition of
self._regulation_control(data)correctly extracts regulation control data before processing control state, which is the appropriate order of operations.plugwise/helper.py (2)
523-528: Nice defensive fallback.Catching
ValueErrorhere keeps the actuator payload flowing even when the gateway returns a non-numeric string, which should spare us from upstream crashes.
806-809: Control-state extraction looks solid.Pulling
control_statestraight fromdata["thermostat"]keeps the XML walking contained to a single pass, and the counter bookkeeping stays balanced.plugwise/constants.py (3)
254-257: Good addition to ActuatorDataType.Including
control_stateandregulation_controlkeeps the Literal aligned with what we now parse.
509-512: TypedDict update matches the new actuator fields.Surface-level schema now matches what helpers emit.
560-561: Gateway entity schema extension looks good.Regulation control select/modes slots are now available to downstream consumers.
bouwew
force-pushed
the
regulation_control
branch
from
aedcb7f to
f3307a5
Compare
bouwew
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugwise/helper.py (2)
523-528: Defensive handling for malformed actuator values.The try-except block gracefully handles cases where
format_measureraisesValueError, falling back to the raw string value. This prevents crashes when encountering unexpected data formats.However, consider logging a warning when the fallback occurs to aid debugging:
try: temp_dict[act_key] = format_measure( pw_function.text, TEMP_CELSIUS ) except ValueError: + LOGGER.debug( + "Failed to format actuator value '%s' for key '%s', using raw value", + pw_function.text, act_key + ) temp_dict[act_key] = str(pw_function.text)
843-853: New _regulation_control method extracts zone profile data.This method populates
select_zone_profileandzone_profilesfrom theregulation_controlvalue indata["thermostat"]. The hardcodedzone_profilesarray["active", "off", "passive"]matches the fixture data.Consider making zone_profiles configurable.
The hardcoded
["active", "off", "passive"]array may need adjustment if future firmware versions support different profiles. Consider extracting this as a constant or deriving it from device capabilities if available in the XML data.# plugwise/constants.py ZONE_PROFILES = ["active", "off", "passive"] # plugwise/helper.py def _regulation_control(self, data: GwEntityData) -> None: """Helper-function for smile.py: _get_location_data(). Adam: collect the thermostat regulation_mode of a location. """ if (reg_control := data["thermostat"].get("regulation_control")) is not None: data["select_zone_profile"] = reg_control data["zone_profiles"] = ZONE_PROFILES data["thermostat"].pop("regulation_control") self._count += 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
fixtures/adam_heatpump_cooling/data.json(20 hunks)fixtures/adam_jip/data.json(8 hunks)fixtures/adam_onoff_cooling_fake_firmware/data.json(2 hunks)fixtures/adam_plus_anna_new/data.json(4 hunks)fixtures/adam_plus_anna_new_regulation_off/data.json(4 hunks)fixtures/m_adam_cooling/data.json(4 hunks)fixtures/m_adam_heating/data.json(4 hunks)fixtures/m_adam_jip/data.json(8 hunks)plugwise/constants.py(4 hunks)plugwise/data.py(1 hunks)plugwise/helper.py(3 hunks)tests/data/adam/adam_heatpump_cooling.json(20 hunks)tests/data/adam/adam_jip.json(8 hunks)tests/data/adam/adam_onoff_cooling_fake_firmware.json(2 hunks)tests/data/adam/adam_plus_anna_new.json(4 hunks)tests/data/adam/adam_plus_anna_new_regulation_off.json(4 hunks)tests/test_adam.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- fixtures/adam_jip/data.json
- fixtures/adam_plus_anna_new/data.json
- tests/data/adam/adam_onoff_cooling_fake_firmware.json
- tests/data/adam/adam_heatpump_cooling.json
- tests/data/adam/adam_jip.json
- tests/data/adam/adam_plus_anna_new.json
- fixtures/adam_onoff_cooling_fake_firmware/data.json
- plugwise/constants.py
- plugwise/data.py
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
Applied to files:
tests/data/adam/adam_plus_anna_new_regulation_off.jsonfixtures/adam_heatpump_cooling/data.jsonfixtures/m_adam_jip/data.jsonfixtures/m_adam_heating/data.jsonfixtures/adam_plus_anna_new_regulation_off/data.jsonfixtures/m_adam_cooling/data.json
📚 Learning: 2024-12-13T11:26:00.100Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 671
File: plugwise/legacy/data.py:97-106
Timestamp: 2024-12-13T11:26:00.100Z
Learning: In `plugwise/legacy/data.py`, the legacy thermostat does not support cooling, so handling of the cooling state is unnecessary in this code.
Applied to files:
plugwise/helper.py
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
Applied to files:
plugwise/helper.py
🧬 Code graph analysis (1)
plugwise/helper.py (2)
plugwise/util.py (1)
format_measure(196-214)plugwise/constants.py (1)
GwEntityData(519-581)
🔇 Additional comments (11)
tests/test_adam.py (3)
37-37: Entity count increased to reflect new zone profile fields.The increment by 4 aligns with adding 2 new fields (
select_zone_profile,zone_profiles) to 2 ThermoZone entries in the adam_plus_anna_new fixture.
351-351: Entity count increments for zone profile fields.These increments (+2 for adam_onoff_cooling_fake_firmware and +8 for adam_jip) reflect the addition of per-zone profile metadata. Ensure these align with the number of climate zones × 2 fields in each fixture.
Also applies to: 417-417
327-327: Incorrect review comment - baseline and increment values are wrong.The review comment contains material factual errors:
Wrong baseline: The previous value was 538, not 519. The actual increment is 538→539 = +1, not +20.
Fixture changes are formatting only: The changes in commit a8d282a (which updated the fixtures) are JSON structural reformatting—arrays and objects expanded to multiple lines for readability. No new fields were added to the ThermoZone entries; the data is identical.
Invalid hypothesis: The assumption that "10 ThermoZone entries × 2 new fields = +20 increment" doesn't align with the actual +1 increment observed, and the fixture contains no such field additions.
The entity_items value of 539 is correct and does not require verification of a +20 increment hypothesis.
Likely an incorrect or invalid review comment.
fixtures/m_adam_cooling/data.json (2)
199-220: LGTM! Zone profile fields added consistently.The addition of
select_zone_profileandzone_profilesto the Living room ThermoZone is correctly structured and aligns with the PR objectives. The placement afterselect_schedulemaintains logical field ordering.
238-255: LGTM! Consistent schema extension for Bathroom zone.The Bathroom ThermoZone receives the same schema extension with appropriate values (passive profile selected). The
zone_profilesarray is identical across zones, which is correct for available options.fixtures/adam_heatpump_cooling/data.json (1)
17-34: LGTM! Comprehensive zone profile schema extension.All ThermoZone entries have been consistently updated with
select_zone_profileandzone_profilesfields. The uniformzone_profilesarray["active", "off", "passive"]correctly represents available options, whileselect_zone_profileappropriately varies per zone configuration.Also applies to: 137-154, 253-269, 397-414, 431-447, 484-501, 538-555, 572-589, 704-721, 808-825
fixtures/m_adam_jip/data.json (1)
11-26: LGTM! Uniform zone profile configuration.All four ThermoZone entries (Slaapkamer, Woonkamer, Kinderkamer, Logeerkamer) have been consistently updated with zone profile fields. All zones use the "active" profile, which is appropriate for this fixture's scenario.
Also applies to: 38-53, 255-270, 306-321
fixtures/m_adam_heating/data.json (1)
198-219: LGTM! Zone profile fields match fixture purpose.The heating fixture correctly adds zone profile metadata. Living room uses "active" profile while Bathroom uses "passive", which is consistent with the cooling fixture pattern and appropriate for testing different zone configurations.
Also applies to: 237-254
tests/data/adam/adam_plus_anna_new_regulation_off.json (1)
236-253: LGTM! Test data updated for zone profiles.Test data correctly includes the new zone profile fields, ensuring test coverage for the regulation_control feature in "off" mode. The Living room (active) and Bathroom (passive) profile selections are consistent with other test fixtures.
Also applies to: 271-288
fixtures/adam_plus_anna_new_regulation_off/data.json (1)
236-253: LGTM! Fixture updated for regulation_off scenario.The regulation_off fixture correctly adds zone profile fields to both ThermoZone entries, maintaining consistency with the corresponding test data and other fixtures in this PR.
Also applies to: 271-288
plugwise/helper.py (1)
798-824: Signature change verified as complete.The method signature change from
_control_state(self, data, loc_id)to_control_state(self, data)has been successfully applied. The only call to_control_statein the active codebase is atplugwise/data.py:168and correctly uses the new signature with only thedataparameter. The method now readscontrol_statedirectly fromdata["thermostat"]as intended, with no legacy calls remaining.
bouwew
force-pushed
the
regulation_control
branch
2 times, most recently
from
dcfee2d to
13f74e2
Compare
bouwew
force-pushed
the
regulation_control
branch
from
13f74e2 to
1e4d62d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugwise/legacy/helper.py (1)
368-375: Handle non-numeric actuator values in legacy helper.Line 374 keeps calling
format_measureeven though the newACTIVE_KEYStuple includes string fields such ascontrol_stateandregulation_control. On legacy setups, encountering one of these will raise aValueErrorand abort data collection. Please mirror the non-legacy helper’s try/except fallback so we keep the original string when conversion fails:if (pw_function := xml.find(locator)) is not None: act_key = cast(ActuatorDataType, key) - temp_dict[act_key] = format_measure(pw_function.text, TEMP_CELSIUS) + try: + temp_dict[act_key] = format_measure( + pw_function.text, TEMP_CELSIUS + ) + except ValueError: + temp_dict[act_key] = str(pw_function.text) self._count += 1Without this adjustment, legacy thermostats will crash as soon as these keys appear.
🧹 Nitpick comments (1)
plugwise/helper.py (1)
849-852: Reuse the new zone profile constant.Line 849 assigns a hard-coded list that duplicates
ALLOWED_ZONE_PROFILES. Please reuse the constant (e.g.,list(ALLOWED_ZONE_PROFILES)) so future profile additions stay in sync automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG.md(1 hunks)fixtures/adam_heatpump_cooling/data.json(20 hunks)fixtures/adam_jip/data.json(8 hunks)fixtures/adam_onoff_cooling_fake_firmware/data.json(2 hunks)fixtures/adam_plus_anna_new/data.json(4 hunks)fixtures/adam_plus_anna_new_regulation_off/data.json(4 hunks)fixtures/m_adam_cooling/data.json(4 hunks)fixtures/m_adam_heating/data.json(4 hunks)fixtures/m_adam_jip/data.json(8 hunks)plugwise/constants.py(4 hunks)plugwise/data.py(1 hunks)plugwise/helper.py(5 hunks)plugwise/legacy/helper.py(2 hunks)plugwise/smile.py(3 hunks)tests/data/adam/adam_heatpump_cooling.json(20 hunks)tests/data/adam/adam_jip.json(8 hunks)tests/data/adam/adam_onoff_cooling_fake_firmware.json(2 hunks)tests/data/adam/adam_plus_anna_new.json(4 hunks)tests/data/adam/adam_plus_anna_new_regulation_off.json(4 hunks)tests/test_adam.py(8 hunks)tests/test_init.py(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/data/adam/adam_onoff_cooling_fake_firmware.json
- tests/data/adam/adam_plus_anna_new_regulation_off.json
- tests/data/adam/adam_jip.json
- tests/data/adam/adam_heatpump_cooling.json
- tests/data/adam/adam_plus_anna_new.json
- fixtures/m_adam_heating/data.json
- fixtures/adam_plus_anna_new/data.json
- fixtures/adam_heatpump_cooling/data.json
- fixtures/m_adam_cooling/data.json
- fixtures/adam_jip/data.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
Applied to files:
fixtures/adam_onoff_cooling_fake_firmware/data.jsonfixtures/adam_plus_anna_new_regulation_off/data.jsonfixtures/m_adam_jip/data.json
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
Applied to files:
plugwise/legacy/helper.py
📚 Learning: 2024-12-13T11:26:00.100Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 671
File: plugwise/legacy/data.py:97-106
Timestamp: 2024-12-13T11:26:00.100Z
Learning: In `plugwise/legacy/data.py`, the legacy thermostat does not support cooling, so handling of the cooling state is unnecessary in this code.
Applied to files:
plugwise/constants.pyplugwise/helper.py
🧬 Code graph analysis (5)
plugwise/data.py (1)
plugwise/helper.py (2)
_regulation_control(843-852)_control_state(798-824)
plugwise/smile.py (2)
plugwise/exceptions.py (1)
PlugwiseError(24-25)plugwise/legacy/smile.py (1)
call_request(322-329)
plugwise/helper.py (2)
plugwise/util.py (1)
format_measure(196-214)plugwise/constants.py (1)
GwEntityData(520-582)
tests/test_adam.py (1)
tests/test_init.py (1)
tinker_zone_profile(992-1018)
tests/test_init.py (4)
plugwise/smile.py (1)
set_select(230-245)plugwise/__init__.py (1)
set_select(343-356)plugwise/legacy/smile.py (1)
set_select(170-175)plugwise/exceptions.py (2)
PlugwiseError(24-25)ConnectionFailedError(8-9)
🔇 Additional comments (9)
fixtures/m_adam_jip/data.json (1)
1-386: Zone profile fixture data is correct and complete.All four ThermoZone entries are uniformly configured with the new fields. Each has
select_zone_profile: "active"with correspondingzone_profiles: ["active", "off", "passive"], and the selected profile value is present in the available profiles array. JSON syntax is valid, all required fields are present, and no breaking changes to existing data.plugwise/constants.py (1)
254-262: Restoresetpoint_high/setpoint_lowinACTIVE_KEYS.Line 254 removed both bounds from the iteration list used by
_get_actuator_functionalities. As a result, Adam/Anna zones that expose dual setpoints no longer populatesetpoint_highandsetpoint_low, breaking cooling/heating target handling. Please add them back:ACTIVE_KEYS: Final[tuple[str, ...]] = ( "control_state", "lower_bound", "offset", "regulation_control", "resolution", "setpoint", + "setpoint_high", + "setpoint_low", "upper_bound", )This mirrors the previous
LIMITStuple and keeps existing integrations functional.⛔ Skipped due to learnings
Learnt from: bouwew Repo: plugwise/python-plugwise PR: 671 File: plugwise/legacy/data.py:97-106 Timestamp: 2024-12-13T11:26:00.100Z Learning: In `plugwise/legacy/data.py`, the legacy thermostat does not support cooling, so handling of the cooling state is unnecessary in this code.fixtures/adam_plus_anna_new_regulation_off/data.json (2)
236-236: LGTM! Zone profile fields added correctly for Living room.The new
select_zone_profileandzone_profilesfields are properly structured. The active selection ("active") correctly matches one of the available profiles in the array.Also applies to: 252-253
271-271: LGTM! Zone profile fields added correctly for Bathroom.The structure mirrors the Living room zone with a different active selection ("passive"), properly demonstrating per-zone profile configuration.
Also applies to: 287-288
tests/test_adam.py (5)
37-37: LGTM! Entity count correctly updated.The increase from 216 to 220 (+4) correctly reflects the addition of 2 new zone profile fields (
select_zone_profile,zone_profiles) across 2 ThermoZone entries.
105-108: LGTM! API signature updated consistently.The
tinker_switchcalls now correctly passapias the first argument, aligning with the refactored helper method signature pattern used throughout the test suite.Also applies to: 117-119
142-145: LGTM! Zone profile happy path test added.The test correctly exercises zone profile switching for the Living room zone, cycling through valid profiles and expecting the invalid "bogus" profile to fail as intended.
190-194: LGTM! Zone profile unhappy path test added.The test properly validates error handling by expecting connection failures (unhappy=True), ensuring robust coverage of the zone profile functionality.
341-341: LGTM! Entity counts updated across fixtures.The entity count increases correctly reflect the addition of zone profile fields to various fixtures:
- adam_heatpump_cooling: +20 entities
- adam_onoff_cooling_fake_firmware: +2 entities
- adam_jip: +8 entities
The counts are consistent with 2 new fields per ThermoZone entry.
Also applies to: 365-365, 431-431
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugwise/constants.py(4 hunks)plugwise/helper.py(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
Applied to files:
plugwise/helper.py
📚 Learning: 2024-12-13T11:26:00.100Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 671
File: plugwise/legacy/data.py:97-106
Timestamp: 2024-12-13T11:26:00.100Z
Learning: In `plugwise/legacy/data.py`, the legacy thermostat does not support cooling, so handling of the cooling state is unnecessary in this code.
Applied to files:
plugwise/helper.pyplugwise/constants.py
🧬 Code graph analysis (1)
plugwise/helper.py (2)
plugwise/util.py (1)
format_measure(196-214)plugwise/constants.py (1)
GwEntityData(520-582)
|
Summary by CodeRabbit
New Features
Tests
Documentation