Add new Jip, separate Tom and Floor devices #842
Conversation
WalkthroughThis PR updates device model naming conventions across fixture and test data files, standardizing model field values from "Tom/Floor" to "Tom" for multiple device entries. It also updates the HW_MODELS mapping in constants.py to include new device types and modifies the changelog. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
|
Solution for home-assistant/core#159741 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #842 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3435 3435
=========================================
Hits 3435 3435 ☔ 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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
CHANGELOG.mdfixtures/adam_jip/data.jsonfixtures/adam_multiple_devices_per_zone/data.jsonfixtures/adam_plus_anna_new/data.jsonfixtures/adam_plus_anna_new_regulation_off/data.jsonfixtures/adam_zone_per_device/data.jsonfixtures/m_adam_cooling/data.jsonfixtures/m_adam_heating/data.jsonfixtures/m_adam_jip/data.jsonfixtures/m_adam_multiple_devices_per_zone/data.jsonplugwise/constants.pytests/data/adam/adam_jip.jsontests/data/adam/adam_multiple_devices_per_zone.jsontests/data/adam/adam_plus_anna_new.jsontests/data/adam/adam_plus_anna_new_regulation_off.jsontests/data/adam/adam_zone_per_device.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/m_adam_jip/data.jsonfixtures/m_adam_heating/data.jsontests/data/adam/adam_multiple_devices_per_zone.jsontests/data/adam/adam_zone_per_device.jsonfixtures/m_adam_cooling/data.jsonfixtures/adam_multiple_devices_per_zone/data.jsonfixtures/m_adam_multiple_devices_per_zone/data.jsonfixtures/adam_zone_per_device/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/constants.py
📚 Learning: 2025-12-25T12:02:58.781Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 835
File: plugwise/helper.py:837-841
Timestamp: 2025-12-25T12:02:58.781Z
Learning: In the Python Plugwise library, zones are not separate appliances in gw_entities. When a zone is added to self.gw_entities, the location_id is used as the entity_id. In code reviews, validate that zone checks rely on location_id being present in _existing_zones and that lookups use self.gw_entities[location_id]. Do not treat zones as independent appliances.
Applied to files:
plugwise/constants.py
🔇 Additional comments (9)
fixtures/adam_plus_anna_new/data.json (1)
74-74: LGTM! Model name standardization looks correct.The model field update from "Tom/Floor" to "Tom" for model_id "106-03" aligns with the HW_MODELS mapping change in constants.py. This is part of the systematic refactoring to separate combined device models into distinct types.
fixtures/adam_plus_anna_new_regulation_off/data.json (1)
48-48: LGTM! Consistent model name update.The model field correctly updated to "Tom" for this thermostatic radiator valve, matching the HW_MODELS mapping change.
fixtures/m_adam_heating/data.json (1)
68-68: LGTM! Model standardization applied correctly.Consistent update matching the HW_MODELS change for model_id "106-03".
fixtures/m_adam_cooling/data.json (1)
63-63: LGTM! Model field updated correctly.Consistent with the model naming standardization across all fixtures.
fixtures/m_adam_jip/data.json (1)
87-87: LGTM! All Tom device models updated consistently.All four thermostatic radiator valves with model_id "106-03" correctly updated from "Tom/Floor" to "Tom", maintaining consistency with the HW_MODELS mapping change.
Also applies to: 111-111, 178-178, 278-278
tests/data/adam/adam_plus_anna_new_regulation_off.json (1)
48-48: LGTM! Test data updated to match fixtures.The model field correctly updated in test data to align with the fixture changes and constants mapping.
tests/data/adam/adam_plus_anna_new.json (1)
74-74: LGTM! Test data synchronized with fixture updates.The model field correctly updated to maintain consistency between test data and fixture files.
plugwise/constants.py (1)
53-58: New model IDs "165-01" and "168-02" not found in test fixtures.The model mapping updates look good. The "106-03" → "Tom" change correctly aligns with the fixture data changes throughout the PR. However, neither "165-01" (Floor) nor "168-02" (Jip variant) appear anywhere in the existing test fixtures. The variant "168-01" for Jip already has test coverage in multiple fixtures, making these new additions represent untested hardware variants. Confirm these model IDs correspond to actual Plugwise device versions before merging.
⛔ Skipped due to learnings
Learnt from: CoMPaTech Repo: plugwise/python-plugwise PR: 698 File: fixtures/m_adam_multiple_devices_per_zone/data.json:21-21 Timestamp: 2025-01-29T19:14:31.257Z Learning: MAC addresses in test fixtures of the python-plugwise repository are mock addresses and do not represent real device information.CHANGELOG.md (1)
3-6: LGTM! Clear documentation of ongoing work.The addition of the "Ongoing" section appropriately documents the model-data standardization effort for Jip, Tom, and Floor devices.
| "hardware": "1", | ||
| "location": "08963fec7c53423ca5680aa4cb502c63", | ||
| "model": "Tom/Floor", | ||
| "model": "Tom", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the target file and verify the model field changes at specified lines
cat -n tests/data/adam/adam_multiple_devices_per_zone.json | sed -n '190,200p'Repository: plugwise/python-plugwise
Length of output: 476
🏁 Script executed:
# Check other specified line ranges
cat -n tests/data/adam/adam_multiple_devices_per_zone.json | sed -n '333,340p'Repository: plugwise/python-plugwise
Length of output: 398
🏁 Script executed:
# Search for any remaining "Tom/Floor" references in the codebase
rg "Tom/Floor" --type=json --type=pyRepository: plugwise/python-plugwise
Length of output: 128
🏁 Script executed:
# Check HW_MODELS mapping in constants.py for model_id "106-03"
fd constants.py | head -5Repository: plugwise/python-plugwise
Length of output: 89
🏁 Script executed:
# Check remaining line ranges
cat -n tests/data/adam/adam_multiple_devices_per_zone.json | sed -n '360,365p'Repository: plugwise/python-plugwise
Length of output: 272
🏁 Script executed:
# Check line 470
cat -n tests/data/adam/adam_multiple_devices_per_zone.json | sed -n '468,473p'Repository: plugwise/python-plugwise
Length of output: 289
🏁 Script executed:
# Check line 538
cat -n tests/data/adam/adam_multiple_devices_per_zone.json | sed -n '536,541p'Repository: plugwise/python-plugwise
Length of output: 276
🏁 Script executed:
# Search for Tom/Floor more comprehensively
rg "Tom/Floor" --type=json --type=py -B 2 -A 2Repository: plugwise/python-plugwise
Length of output: 387
🏁 Script executed:
# Check HW_MODELS mapping in constants.py
wc -l plugwise/constants.pyRepository: plugwise/python-plugwise
Length of output: 93
🏁 Script executed:
# Get the HW_MODELS section from constants.py
rg -n "HW_MODELS" plugwise/constants.py -A 30Repository: plugwise/python-plugwise
Length of output: 1063
Update the comment in constants.py to reflect the Tom/Floor separation.
All model field changes from "Tom/Floor" to "Tom" in the JSON test data are confirmed across lines 196, 337, 362, 470, and 538. The HW_MODELS mapping correctly reflects the separation with "106-03" mapped to "Tom" and "165-01" mapped to "Floor" as distinct entries.
However, the comment at line 26 of constants.py (# Specific for a Lisa or Tom/Floor) is outdated and should be updated to reflect the separated device types, or removed if no longer applicable. This comment may confuse future maintainers about the current model structure.
🤖 Prompt for AI Agents
In constants.py around line 26, the inline comment "# Specific for a Lisa or
Tom/Floor" is outdated because test JSON now uses separate model names "Tom" and
"Floor" (see tests/data/adam/adam_multiple_devices_per_zone.json lines 196, 337,
362, 470, 538); update the comment to reflect the separated device types (e.g.,
"# Specific for a Lisa, Tom, or Floor" or a clearer phrase describing distinct
models) or remove the comment entirely to avoid confusion, ensuring it matches
the HW_MODELS mapping that maps "106-03" -> "Tom" and "165-01" -> "Floor".
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.