Add HomeWizardEnergy base class which is implemented in v1 and v2 #452
Conversation
WalkthroughThe pull request introduces a new base class Changes
Sequence DiagramsequenceDiagram
participant Client
participant BaseClass as HomeWizardEnergy
participant V1Class as HomeWizardEnergyV1
participant V2Class as HomeWizardEnergyV2
participant API as HomeWizard API
Client->>BaseClass: Create instance
BaseClass-->>Client: Base class instance
Client->>V1Class: Create V1 instance
V1Class-->>Client: V1 instance
Client->>V2Class: Create V2 instance
V2Class-->>Client: V2 instance
Client->>V1Class: measurement()
V1Class->>API: Fetch measurement
API-->>V1Class: Return measurement
V1Class-->>Client: Return measurement
Client->>V2Class: device()
V2Class->>API: Fetch device info
API-->>V2Class: Return device info
V2Class-->>Client: Return device info
Possibly related PRs
🪧 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #452 +/- ##
==========================================
+ Coverage 96.90% 96.95% +0.05%
==========================================
Files 8 9 +1
Lines 484 492 +8
Branches 34 33 -1
==========================================
+ Hits 469 477 +8
Misses 11 11
Partials 4 4 ☔ View full report in Codecov by Sentry. |
![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 (8)
homewizard_energy/homewizard_energy.py (3)
13-14: Consider using an abstract base class for mandatory methods.
You're raisingNotImplementedErrorin the derived methods, which is a valid approach. However, it might be clearer to markHomeWizardEnergyas abstract via Python'sabcmodule to explicitly communicate that classes must override these methods.
16-19: Add type annotations or docstrings for private attributes.
Including brief docstrings or type annotations for the_session,_close_session,_request_timeout, and_hostattributes helps ensure readability and consistency with the rest of the codebase, especially if multiple contributors are involved.
83-88: Validate session usage before closing.
This code only checks that_sessionis non-Nonebefore closing, but you may want to confirm the session is still open or ensure no subsequent calls use it. Providing a small safeguard or logging at the call sites would help diagnose potential usage after close.homewizard_energy/v1/__init__.py (1)
44-45: Confirm no missing overridden methods.
You disable the abstract-method warning here, but the base class defines several methods that raiseNotImplementedError. Since you're actually implementing them (device, measurement, etc.), verify that all relevant methods are covered if future expansions occur.homewizard_energy/v2/__init__.py (3)
58-65: Recommend clarifying docstring forclientsession.
Since you’re now acceptingclientsession, add documentation that clarifies if you expect an already-open session and how the class handles session closure.
220-221: Ensure proper session creation.
When_sessionisNone, you await_get_session(). This is fine, but watch for race conditions if_requestcan be called from multiple tasks at once before_sessionis set.
235-235: Log request data carefully.
You’re usingLOGGER.debugto print request status and response text. Double-check that no sensitive data (like tokens) are ever logged.tests/v1/test_v1_homewizard_energy.py (1)
233-235: Consider consolidating repeated test logicLines 233–235 and later lines (278–280) perform nearly identical assertions on
measurement. You could reduce duplication by factoring out the common logic into a shared helper or parameterizing the test. This improvement can make the test suite more concise and easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
homewizard_energy/homewizard_energy.py(1 hunks)homewizard_energy/v1/__init__.py(3 hunks)homewizard_energy/v2/__init__.py(6 hunks)tests/test_homewizard_energy.py(1 hunks)tests/v1/test_v1_homewizard_energy.py(2 hunks)tests/v2/test_v2_homewizard_energy.py(1 hunks)
🔇 Additional comments (11)
homewizard_energy/homewizard_energy.py (2)
21-38: Ensure that user-provided timeouts are handled consistently.
Your constructor sets a default 10-second timeout. Double-check that all future overridden methods or HTTP request logic correctly utilize _request_timeout, so that user-specified timeouts apply uniformly.
89-103: Consider handling exceptions in __aexit__.
Currently, __aexit__ always calls await self.close() regardless of whether an exception occurred. You might want to handle or log exceptions here to diagnose failures, especially if a finalized closure after an error might complicate debugging.
homewizard_energy/v1/__init__.py (2)
23-23: Validate consistent imports.
You’re now importing HomeWizardEnergy from ..homewizard_energy. Ensure that other references (e.g., session creation or method overrides) match the base class usage in this file.
60-60: Method name aligns well with the base class.
Renaming data to measurement keeps the API consistent with HomeWizardEnergy. Good job on maintaining consistency.
homewizard_energy/v2/__init__.py (3)
32-32: Inheritance from HomeWizardEnergy.
Using the new base class promotes code reuse and consistency. Verify that v2-specific quirks (like token handling) are covered properly.
54-55: Same recommendation as v1: confirm abstract methods coverage.
Ensure that all relevant methods from HomeWizardEnergy are fully implemented or intentionally left for overriding in the future.
185-185: Check concurrency if _get_session can be called multiple times.
If _get_session may be invoked concurrently, ensure you prevent creating multiple sessions or overwriting the _session while another request is using it.
tests/test_homewizard_energy.py (1)
1-25: Test coverage for base class methods is excellent.
You verify that each of the abstract-like methods in HomeWizardEnergy raises NotImplementedError. As a possible future step, consider adding coverage for the close/context-manager usage to ensure resource cleanup works as expected.
tests/v2/test_v2_homewizard_energy.py (2)
522-523: Use _session consistently.
Great update from _clientsession to _session. This ensures consistency with the base class. Also verify test coverage for other methods that might rely on the session.
528-528: Check exponential backoff behavior.
The test asserts the request retries five times. Since you’re combining backoff with your request approach, confirm that repeated retries are truly desired in all production scenarios to avoid possible resource overuse if the device is offline.
tests/v1/test_v1_homewizard_energy.py (1)
278-280: Validation steps for measurement correctness look good
The updated assertion structure blends well with snapshot matching, ensuring the measurement() method returns valid data. The logic aligns with typical test best practices.
Summary by CodeRabbit
New Features
HomeWizardEnergyfor interacting with the HomeWizard Energy APIRefactor
HomeWizardEnergyV1andHomeWizardEnergyV2classes to inherit from new base classTests