Trying solution for Core issue #132479 #661
Conversation
WalkthroughThe changes in this pull request introduce a new binary sensor, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json (1)
60-62: Consider documenting cooling threshold parameters.The cooling parameters would benefit from documentation explaining:
- The significance of the 26.0°C activation temperature
- How the 3.0°C deactivation threshold is applied
plugwise/helper.py (1)
801-801: Consider splitting the state assignment for better readability.While the current implementation is correct, consider splitting the state assignment for better clarity:
- self._cooling_enabled = data["binary_sensors"]["cooling_enabled"] = data["elga_status_code"] in (8, 9) + cooling_enabled = data["elga_status_code"] in (8, 9) + self._cooling_enabled = cooling_enabled + data["binary_sensors"]["cooling_enabled"] = cooling_enabled
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
fixtures/adam_heatpump_cooling/all_data.json(1 hunks)fixtures/adam_onoff_cooling_fake_firmware/all_data.json(1 hunks)plugwise/__init__.py(2 hunks)plugwise/data.py(2 hunks)plugwise/helper.py(2 hunks)plugwise/smile.py(3 hunks)tests/data/adam/adam_heatpump_cooling.json(1 hunks)tests/data/adam/adam_onoff_cooling_fake_firmware.json(1 hunks)tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json(1 hunks)tests/test_adam.py(1 hunks)tests/test_anna.py(1 hunks)tests/test_init.py(1 hunks)
🔇 Additional comments (14)
tests/data/adam/adam_onoff_cooling_fake_firmware.json (1)
5-5: LGTM! States are logically consistent.
The addition of cooling_enabled: true aligns correctly with:
cooling_state: trueheating_state: false- Gateway's
select_regulation_mode: "cooling"
tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json (1)
6-12: LGTM! Binary sensor states are logically consistent.
The states correctly reflect a heating scenario:
cooling_enabled: falsealigns withcooling_state: falseheating_state: trueis appropriate when cooling is disabled
fixtures/adam_onoff_cooling_fake_firmware/all_data.json (1)
5-5: LGTM! Cooling configuration is consistent across all levels.
The addition of cooling_enabled: true is properly supported by:
- Device level:
cooling_state: trueandheating_state: false - Gateway level:
cooling_present: trueand appropriate regulation modes
plugwise/data.py (1)
228-229: LGTM: Clean implementation of cooling_enabled binary sensor.
The implementation correctly adds the cooling_enabled binary sensor for heater_central devices when cooling is present, with proper condition checks and documentation.
Also applies to: 240-244
tests/test_adam.py (1)
323-324: LGTM: Good test coverage for cooling functionality.
The added assertions properly validate both the presence and enabled state of the cooling functionality.
plugwise/__init__.py (2)
68-68: LGTM: Proper initialization of cooling_enabled flag.
The _cooling_enabled attribute is correctly initialized as False by default and follows the established pattern of boolean flags in the class.
134-135: LGTM: Clean addition of cooling parameters to SmileAPI.
The cooling-related parameters are properly passed to SmileAPI constructor while maintaining the existing structure and not affecting the legacy API path.
plugwise/smile.py (2)
Line range hint 52-75: LGTM: Constructor parameter addition is well-implemented
The addition of _cooling_enabled parameter to the constructor is clean and follows existing patterns.
135-136: LGTM: Improved error handling clarity
The simplified error handling with a clear comment improves code maintainability.
tests/test_anna.py (1)
410-422: LGTM: Well-structured test for seasonal transition
The test case effectively simulates and validates the transition from cooling to heating mode with appropriate assertions.
tests/data/adam/adam_heatpump_cooling.json (1)
46-46: LGTM: Clean addition of cooling_enabled sensor
The addition of the cooling_enabled binary sensor is well-structured and maintains JSON format consistency.
fixtures/adam_heatpump_cooling/all_data.json (1)
46-46: LGTM! The addition of cooling_enabled binary sensor is consistent with the device's cooling capabilities.
The new binary sensor aligns with:
- The device's "Generic heater/cooler" model type
- The gateway's cooling support (
cooling_present: true) - The existing cooling-related sensors in the device
tests/test_init.py (1)
649-649: LGTM! Enhanced test observability for cooling functionality.
The added logging statement improves debugging capabilities by exposing the cooling enabled state during test execution.
plugwise/helper.py (1)
250-250: LGTM! Improved type safety with bool annotation.
The explicit type annotation enhances code maintainability and helps catch type-related errors early.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
pyproject.toml (1)
Line range hint
25-25: Consider relaxing the Python version requirementThe project requires Python >=3.12.0, which is quite recent. This might limit adoption as many users may still be on Python 3.11 or earlier. Consider if Python 3.11 compatibility can be maintained unless there are specific 3.12 features being used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
bouwew
force-pushed
the
test-elga_2
branch
2 times, most recently
from
ecca135 to
e654a08
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #661 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 3415 3436 +21
=========================================
+ Hits 3415 3436 +21 ☔ View full report in Codecov by Sentry. |
|
Will set to v1.6.3 when approved. |
| """ | ||
| if self.smile(ADAM): | ||
| if entity["dev_class"] == "heater_central": |
There was a problem hiding this comment.
Maybe we should 'match/case' this, but at least just return before the match against gateway (as that wouldn't be processed e.g. there is nothing more to do once heater central is done)?
|
All changes:
cooling_enabledbinary_sensordirectly toself._cooling_enabledcooling_enabledbinary_sensorfor Adam where missingself._cooling_enabledis, in__init__.py, linked to thebinary_sensorcooling_enabled. When this one is not representingself._cooling_enabledbut the xml-keycooling_enabled, errors might occur.Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
cooling_enabled, to indicate the cooling functionality for specific devices.Tests
Chores
These updates improve the overall functionality and reliability of the cooling features within the system.