Update adam_plus_anna_new testing #799
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 7 minutes and 18 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)
WalkthroughRemoved an obsolete fixture and test; added and updated multiple fixtures and tests to introduce Emma (zone_thermostat) and Jip (zone_thermometer), bump Adam gateway firmware to 3.9.0, change many zigbee/mac addresses and sensor values, reorder schedules/presets, and update tests (one test removed, entity counts increased). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 #799 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3414 3403 -11
=========================================
- Hits 3414 3403 -11 ☔ 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: 0
🧹 Nitpick comments (1)
tests/test_adam.py (1)
38-40: Clarify inline comment placement.The inline comment
# Badkameron line 40 clarifies which location the assertion refers to. Consider whether this comment adds value or if the assertion could be made more self-documenting by assigning the result to a named variable.Apply this diff if you prefer clearer variable naming:
- assert ( - api._last_active["f871b8c4d63549319221e294e4f88074"] == "Weekschema" - ) # Badkamer + badkamer_location_id = "f871b8c4d63549319221e294e4f88074" + assert api._last_active[badkamer_location_id] == "Weekschema" # Badkamer
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
fixtures/adam_anna_new_2/data.json(17 hunks)fixtures/adam_plus_anna_new/data.json(13 hunks)fixtures/m_adam_cooling/data.json(9 hunks)fixtures/m_adam_heating/data.json(8 hunks)scripts/tests_and_coverage.sh(1 hunks)tests/data/adam/adam_anna_new_2.json(17 hunks)tests/data/adam/adam_plus_anna_new.json(12 hunks)tests/test_adam.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T14:05:29.012Z
Learnt from: bouwew
PR: plugwise/python-plugwise#797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.012Z
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.json
🧬 Code graph analysis (1)
tests/test_adam.py (4)
tests/test_init.py (1)
device_test(480-601)plugwise/helper.py (1)
gateway_id(90-92)plugwise/legacy/helper.py (1)
gateway_id(77-79)plugwise/__init__.py (1)
gateway_id(96-98)
🪛 Gitleaks (8.28.0)
tests/test_adam.py
[high] 36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish Python 🐍 distributions 📦 to TestPyPI
🔇 Additional comments (27)
scripts/tests_and_coverage.sh (1)
61-61: LGTM: Verbose output helps with expanded test coverage.Adding the
-vflag provides more detailed test output, which is valuable given the expanded device scenarios (Emma, Jip) introduced in this PR.tests/test_adam.py (5)
36-36: False positive: Gateway ID is not a secret.The static analysis tool flagged this as a potential API key, but
da224107914542988a88561b4452b0f6is a device UUID/identifier used in test fixtures, not a secret credential.
35-35: Verify future-dated test timestamp.The test time is set to
2025-10-12 00:00:01, which is in the future relative to the current date (October 2025). Ensure this is intentional and aligns with the frozen time strategy used in other tests.
21-21: LGTM: Docstring accurately reflects expanded test scope.The updated docstring correctly describes the broader Adam setup with Anna, Emma, Jip, and switch-group, aligning with the fixture changes.
32-41: LGTM: Test expectations updated for firmware 3.9.0 and new devices.The firmware version bump to 3.9.0 and increased entity count (216) appropriately reflect the addition of Emma and Jip devices in the test fixtures.
48-54: LGTM: Entity list updated with new device IDs.The entity list correctly includes the new device IDs for Emma (
14df5c4dc8cb4ba69f9d1ac0eaf7c5c6) and Jip (da575e9e09b947e281fb6e3ebce3b174), along with updated plug IDs.fixtures/m_adam_cooling/data.json (5)
28-53: LGTM: Emma device properly defined.The new Emma device (zone_thermostat) is well-structured with all required fields including sensors, binary_sensors, temperature_offset, and zigbee_mac_address.
124-144: LGTM: Jip device properly defined.The new Jip device (zone_thermometer) includes appropriate sensor data and metadata, consistent with the device class.
67-71: Verify valve closed state is intentional.Tom Badkamer's valve_position changed from 100 to 0.0, indicating a fully closed valve. Ensure this reflects the intended test scenario for the cooling fixture.
100-123: LGTM: Firmware and MAC address updates.The firmware upgrade to 3.9.0 and systematic MAC address updates (to ...CBA0 pattern) are consistent across all fixtures in this PR.
212-216: LGTM: Thermostat group expanded to include Emma and Jip.The primary thermostats list now correctly includes the new devices (Emma: 14df5c4d..., Jip: da575e9e...) alongside Anna, reflecting the expanded test scenario.
fixtures/m_adam_heating/data.json (3)
33-58: LGTM: Emma device consistent across heating scenario.Emma's definition matches the cooling fixture with appropriate heating-specific sensor values.
123-143: LGTM: Jip device consistent across heating scenario.Jip's sensor readings are appropriate for the heating test scenario.
211-215: LGTM: Thermostat grouping matches cooling fixture structure.The primary thermostats array consistently includes Emma, Jip, and Anna across both heating and cooling fixtures.
fixtures/adam_anna_new_2/data.json (3)
6-8: LGTM: Heating state activated appropriately.The OpenTherm binary sensors now show active heating (flame_state: true, heating_state: true), which aligns with the increased boiler temperatures in this fixture.
29-31: LGTM: Emma binary_sensors added.The low_battery binary sensor for Emma completes the device definition, matching the structure in other fixtures.
239-263: LGTM: Living room climate state updated coherently.The changes to active_preset (vacation→home), climate_mode (heat→auto), control_state (idle→heating), and select_schedule (off→Weekschema) form a coherent scenario update.
tests/data/adam/adam_anna_new_2.json (2)
28-53: LGTM: Test data matches fixture structure.Emma's device definition in the test data correctly mirrors the fixture file with the "devices" wrapper structure.
239-275: LGTM: Living room test data matches fixture updates.The climate state updates are consistent with the corresponding fixture file.
tests/data/adam/adam_plus_anna_new.json (4)
28-53: LGTM: Emma device complete in test data.Emma's full definition with all sensors and metadata is correctly structured for test validation.
179-199: LGTM: Jip device complete in test data.Jip's zone_thermometer definition includes all required sensors and metadata.
Based on learnings: The location ID referenced by Jip doesn't require a separate top-level entry since it's not a climate zone.
105-122: Device renamed from Plug Werkplek to Plug Thermex Boiler.The device at ID
29542b2b6a6a4169acecc15c72a599b8has been renamed and reclassified from a generic plug towater_heater_vessel_plug. Verify this reflects an actual device change or test scenario update.
267-271: LGTM: Primary thermostats array expanded.The Living room ThermoZone now correctly references Anna, Emma, and Jip as primary thermostats.
fixtures/adam_plus_anna_new/data.json (4)
39-64: LGTM: Emma device fixture matches test data.The Emma device definition is consistent with the corresponding test data file.
207-227: LGTM: Jip device fixture matches test data.Jip's definition in the fixture file correctly corresponds to the test data.
189-206: LGTM: Gateway firmware and configuration updated.The Adam gateway firmware upgrade to 3.9.0, MAC address updates, and regulation_modes reordering are consistent across all fixtures.
295-299: LGTM: Complete thermostat hierarchy established.The Living room ThermoZone's primary thermostats array now includes all three devices (Anna, Emma, Jip), completing the expanded test scenario structure.
bouwew
force-pushed
the
upd-adam-plus-anna-new
branch
from
ff1e88f to
33e461d
Compare
bouwew
force-pushed
the
upd-adam-plus-anna-new
branch
from
33e461d to
fbaeeff
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)
tests/test_adam.py (1)
41-58: Discrepancy found: entity_items assertion does not match fixture structure.The test asserts
entity_items == 216, but calculation from the fixture data yields 198. The 15 device IDs in entity_list are correct and match the fixture, but the count is off by 18 items. This suggests either the fixture was incompletely regenerated or the test count was not updated properly. Verify the fixture data is complete and recalculate the expected count.
🧹 Nitpick comments (1)
tests/test_adam.py (1)
38-40: Minor: Formatting change for assertion.The Badkamer last_active assertion was moved to a multi-line format with an inline comment. While this improves readability, ensure this formatting is consistent with the project's style guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
fixtures/adam_anna_new_2/data.json(0 hunks)fixtures/adam_plus_anna_new/data.json(13 hunks)fixtures/m_adam_cooling/data.json(9 hunks)fixtures/m_adam_heating/data.json(8 hunks)tests/data/adam/adam_anna_new_2.json(0 hunks)tests/data/adam/adam_plus_anna_new.json(12 hunks)tests/test_adam.py(2 hunks)
💤 Files with no reviewable changes (2)
- tests/data/adam/adam_anna_new_2.json
- fixtures/adam_anna_new_2/data.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
PR: plugwise/python-plugwise#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.json
🧬 Code graph analysis (1)
tests/test_adam.py (4)
tests/test_init.py (1)
device_test(480-601)plugwise/legacy/helper.py (1)
gateway_id(77-79)plugwise/helper.py (1)
gateway_id(90-92)plugwise/__init__.py (1)
gateway_id(96-98)
🪛 Gitleaks (8.28.0)
tests/test_adam.py
[high] 36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (14)
fixtures/m_adam_heating/data.json (4)
33-58: LGTM: Emma device addition looks correct.The new Emma zone_thermostat device is properly structured with all required fields including binary_sensors, sensors, temperature_offset configuration, and Zigbee MAC address.
123-143: LGTM: Jip device addition looks correct.The new Jip zone_thermometer device is properly structured with consistent sensor data (battery, humidity, setpoint, temperature) matching the Emma device pattern.
105-105: LGTM: Gateway firmware and configuration updates.The firmware upgrade to 3.9.0 and corresponding MAC address updates are consistent with the PR objectives.
Also applies to: 109-109, 114-114, 121-121
211-215: Code properly handles multiple primary thermostats in zones.The fixture change adding three primary thermostats (Anna, Emma, Jip) to the Living room zone is supported by the codebase design. The code explicitly documents support for "several primary and secondary thermostats", and the implementation uses safe list operations (append, concatenation) that correctly handle multiple devices. No single-device assumptions were found in the thermostat ranking or processing logic. Existing test coverage validates this scenario with the
adam_multiple_devices_per_zonefixture.fixtures/m_adam_cooling/data.json (2)
28-53: LGTM: Consistent device data across fixtures.Emma and Jip device definitions are identical to the heating fixture, which ensures consistency across different test scenarios.
Also applies to: 124-143
212-216: LGTM: Consistent thermostat grouping.The primary thermostats list matches the heating fixture, properly including Anna, Emma, and Jip.
fixtures/adam_plus_anna_new/data.json (3)
39-64: LGTM: Consistent device additions.Emma and Jip devices match the other fixtures perfectly, ensuring test consistency.
Also applies to: 207-227
116-121: LGTM: Device reclassification and rename.The device formerly known as "Plug Werkplek" has been reclassified to "water_heater_vessel_plug" and renamed to "Plug Thermex Boiler", which better reflects its actual purpose.
118-118: Verify location reference for reclassified device.The location ID "8201a2ac4d1b4303bf994e18d67311eb" is referenced but may not have a corresponding top-level location object. Based on learnings, this is acceptable for organizational locations that aren't climate zones, but worth verifying this location is properly defined somewhere.
Based on learnings
tests/test_adam.py (2)
21-21: LGTM: Test description accurately updated.The test description correctly reflects the updated firmware version (3.9) and new devices (Emma, Jip).
32-32: LGTM: Test assertions updated for firmware 3.9.The firmware version and test timestamp are correctly updated to match the new fixture data.
Also applies to: 35-35
tests/data/adam/adam_plus_anna_new.json (3)
28-53: LGTM: Test data matches fixture data.Emma and Jip device definitions in the test data file match the fixture files exactly, ensuring test consistency.
Also applies to: 179-199
267-271: LGTM: Thermostat grouping consistent across files.The Living room's primary thermostats list matches the fixture files, correctly including Anna, Emma, and Jip.
105-110: LGTM: Device reclassification matches fixtures.The "Plug Thermex Boiler" device with dev_class "water_heater_vessel_plug" is consistent with the fixture file changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
3-5: Minor formatting inconsistency in PR reference.The changelog entry uses
via PR [#799]while existing entries (e.g., line 9) usevia [#802]without the "PR" prefix. Consider aligning the format for consistency.Apply this diff to match the existing format:
- Refresh adam_plus_anna_new userdata (Emma Pro included), update related test-asserts and files via PR [#799](https://github.com/plugwise/python-plugwise/pull/799) + Refresh adam_plus_anna_new userdata (Emma Pro included), update related test-asserts and files via [#799](https://github.com/plugwise/python-plugwise/pull/799)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Process test coverage
bouwew
force-pushed
the
upd-adam-plus-anna-new
branch
from
bb00ee5 to
c0d10d5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/data/adam/adam_plus_anna_new.json(12 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
PR: plugwise/python-plugwise#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.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run pytest using Python 3.13
- GitHub Check: Run pytest using Python 3.14
🔇 Additional comments (3)
tests/data/adam/adam_plus_anna_new.json (3)
28-53: Emma device addition is well-structured.The new Emma zone_thermostat entry follows proper conventions with all required fields (dev_class, hardware, model_id, sensors including battery/humidity/setpoint/temperature, temperature_offset block with bounds). Location reference to Living room zone is correct.
179-198: Jip device addition is well-structured.The new Jip zone_thermometer entry has all required fields and correct structure. Location reference to Living room zone aligns with Emma and Anna.
267-271: Verify thermostat array consistency in Living room zone.The primary thermostats array now includes three devices (Anna, Emma, Jip). Confirm this reflects the intended test scenario:
- Anna (thermostat): setpoint 20.5
- Emma (zone_thermostat): setpoint 20.5
- Jip (zone_thermometer): setpoint 20.5
All three have matching setpoints aligned with the zone's thermostat setpoint (20.5), which is correct. However, verify that test code properly handles multiple primary thermostats in this zone.
with the adam_anna_new_2 data
bouwew
force-pushed
the
upd-adam-plus-anna-new
branch
from
9e3697a to
b10b1bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CHANGELOG.md(1 hunks)fixtures/adam_anna_new_2/data.json(0 hunks)fixtures/adam_plus_anna_new/data.json(13 hunks)fixtures/m_adam_cooling/data.json(9 hunks)fixtures/m_adam_heating/data.json(8 hunks)plugwise/constants.py(1 hunks)plugwise/helper.py(1 hunks)tests/data/adam/adam_anna_new_2.json(0 hunks)tests/data/adam/adam_plus_anna_new.json(13 hunks)tests/test_adam.py(2 hunks)
💤 Files with no reviewable changes (2)
- fixtures/adam_anna_new_2/data.json
- tests/data/adam/adam_anna_new_2.json
✅ Files skipped from review due to trivial changes (2)
- plugwise/helper.py
- plugwise/constants.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-29T19:14:31.257Z
Learnt from: CoMPaTech
PR: plugwise/python-plugwise#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.
Applied to files:
tests/data/adam/adam_plus_anna_new.json
🧬 Code graph analysis (1)
tests/test_adam.py (2)
tests/test_init.py (1)
device_test(480-601)plugwise/__init__.py (1)
gateway_id(96-98)
🪛 GitHub Actions: Latest commit
CHANGELOG.md
[error] 4-4: markdownlint: MD032/blanks-around-lists - Lists should be surrounded by blank lines [Context: "- Refresh adam_plus_anna_new u..."]
[error] 5-5: markdownlint: MD022/blanks-around-headings - Headings should be surrounded by blank lines [Context: "## v1.8.1"]
🪛 Gitleaks (8.28.0)
tests/test_adam.py
[high] 36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (12)
fixtures/m_adam_heating/data.json (3)
33-58: LGTM! Emma device properly added.The new Emma Pro zone_thermostat is correctly structured with all required fields including sensors, temperature_offset bounds, and Zigbee MAC address.
123-143: LGTM! Jip device properly added.The new Jip zone_thermometer is correctly structured with firmware, sensors, and Zigbee MAC address.
105-114: LGTM! Gateway firmware and configuration updated.The Adam gateway firmware upgrade to 3.9.0 and regulation_modes reordering align with the PR's objective to refresh test data.
fixtures/adam_plus_anna_new/data.json (2)
39-64: LGTM! Emma device added consistently.The Emma Pro configuration matches the structure in other fixtures, maintaining consistency across test data.
116-134: Device reclassification looks intentional.The rename from "Plug Werkplek" to "Plug Thermex Boiler" and dev_class change to "water_heater_vessel_plug" appears to better reflect the device's actual function in the test scenario.
tests/test_adam.py (2)
21-41: LGTM! Test updated for firmware 3.9 and new devices.The test description, version assertion, and entity count correctly reflect the addition of Emma and Jip devices to the test fixture.
36-36: Static analysis false positive - not an API key.The Gitleaks tool flagged line 36 as containing a potential API key, but this is a false positive. The string
"2025-10-12 00:00:01"is clearly a test timestamp used withfreeze_time, not a secret.fixtures/m_adam_cooling/data.json (2)
28-53: LGTM! Cooling fixture updated with Emma.The Emma device is properly configured for the cooling scenario with identical structure to the heating fixture.
117-117: Appropriate regulation mode for cooling fixture.Setting
select_regulation_modeto "cooling" is correct for this cooling-specific test fixture.tests/data/adam/adam_plus_anna_new.json (3)
40-65: LGTM! Emma properly integrated into test data.The Emma Pro zone_thermostat is correctly added with complete sensor data, temperature_offset configuration, and unique Zigbee MAC address.
208-228: LGTM! Jip zone_thermometer added.The Jip device is properly configured as a zone_thermometer with battery, humidity, setpoint, and temperature sensors.
113-171: Verify Zigbee MAC addresses are unique across all plugs.Based on learnings, MAC addresses should be unique. Let me verify the three plug devices have distinct addresses:
- Line 113:
000D6F000D13CBA1(Plug MediaTV)- Line 134:
000D6F000D13CBA2(Plug Thermex Boiler)- Line 171:
000D6F000D13CBA0(Plug Vloerverwarming)All three addresses are unique (ending in CBA1, CBA2, and CBA0 respectively). The past duplicate MAC issue has been properly resolved.
|
2 participants
Summary by CodeRabbit
New Features
Chores
Tests
Documentation