-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add preliminary support for HWE-BAT #438
Conversation
WalkthroughThe pull request introduces new optional attributes, Changes
Sequence DiagramsequenceDiagram
participant Client
participant Measurement
participant Device
Client->>Measurement: Create with optional cycles and state_of_charge_pct
Measurement-->>Client: Measurement object
Client->>Device: Retrieve device measurements
Device-->>Client: Return Measurement with new attributes
The sequence diagram illustrates how the new attributes can be incorporated into the measurement creation and retrieval process, showing the optional nature of the 🪧 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 #438 +/- ##
=======================================
Coverage 98.94% 98.94%
=======================================
Files 9 9
Lines 567 569 +2
Branches 41 41
=======================================
+ Hits 561 563 +2
Misses 6 6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/v2/test_v2_models.py (1)
43-48
: Expanded measurement test coverage to HWE-BAT
Including"HWE-BAT"
in the measurement parameterization is a solid improvement, ensuring that the new measurement fields (likecycles
andstate_of_charge_pct
) are tested. Consider extending the test coverage to include edge or boundary values for these new fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
homewizard_energy/v2/models.py
(2 hunks)tests/v2/__snapshots__/test_v2_homewizard_energy.ambr
(1 hunks)tests/v2/__snapshots__/test_v2_models.ambr
(1 hunks)tests/v2/fixtures/HWE-BAT/device.json
(1 hunks)tests/v2/fixtures/HWE-BAT/measurement.json
(1 hunks)tests/v2/fixtures/HWE-BAT/system.json
(1 hunks)tests/v2/test_v2_models.py
(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/v2/fixtures/HWE-BAT/system.json
- tests/v2/fixtures/HWE-BAT/device.json
- tests/v2/fixtures/HWE-BAT/measurement.json
🔇 Additional comments (13)
tests/v2/test_v2_models.py (2)
19-19
: Introduced new test parameter for HWE-BAT device
The addition of "HWE-BAT"
to the device test coverage looks good. This ensures that the new device type is properly tested alongside existing ones.
66-66
: Added HWE-BAT to system test parameter
Including the "HWE-BAT"
model in the system test ensures that all relevant device types have system-level coverage. No immediate issues are observed.
tests/v2/__snapshots__/test_v2_homewizard_energy.ambr (2)
6-6
: Snapshot includes new fields for HWE-BAT
The snapshot's Measurement
instances now capture cycles=None
and state_of_charge_pct=None
. This is aligned with the newly introduced attributes. Be sure to confirm that None
states are tested for correctness.
9-9
: Second snapshot with new optional fields
Similarly, this snapshot also shows the added attributes. Looks consistent with the updated model.
tests/v2/__snapshots__/test_v2_models.ambr (7)
2-3
: Added snapshot for HWE-BAT device
Lines 2-3 introduce a new snapshot test for the HWE-BAT device. Validates product_type='HWE-BAT'
and relevant fields. Implementation is consistent, no issues found.
8-9
: Added snapshot for HWE-BAT measurement
Lines 8-9 show a new measurement snapshot for "HWE-BAT"
, including state_of_charge_pct=50.123
. This aligns with the new attribute in the model.
12-12
: Updated snapshot with new measurement fields
The updated snapshot includes cycles=None
and state_of_charge_pct=None
. Matches the model changes.
15-15
: Updated snapshot with new measurement fields
Again, verifying that cycles=None
and state_of_charge_pct=None
are included in the measurement snapshot. No concerns.
18-18
: Support for edge-case measurement snapshot
Shows cycles=None
and state_of_charge_pct=None
for an edge-case measurement. This ensures correct handling for uninitialized data.
21-21
: Another updated snapshot with new measurement fields
Same pattern here with cycles=None
and state_of_charge_pct=None
. Good to see consistency across multiple fixtures.
23-24
: Added system snapshot for HWE-BAT
These lines show the new system test snapshot for HWE-BAT. The fields match the data from the fixture (api_v1_enabled=None
is tested as well).
homewizard_energy/v2/models.py (2)
81-82
: New fields in Measurement data class
The attributes cycles
(int | None) and state_of_charge_pct
(float | None) effectively extend functionality for battery devices. This is well-structured, and using optional types is appropriate for cases where values may not be available.
189-190
: Extended from_dict to map new attributes
Mapping cycles
and state_of_charge_pct
in from_dict
ensures the new fields are deserialized properly. Good job including them consistently with other Measurement attributes.
326d452
to
ffd4652
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/v2/__snapshots__/test_v2_models.ambr (1)
8-9
: HWE-BAT measurement test snapshot is aligned with the newly introduced attributes.The addition of
cycles=123
andstate_of_charge_pct=50.123
aligns well with the PR objectives. Ensure any new logic referencing these fields is also tested against unexpected or edge values (e.g., negative cycles or 100+ state_of_charge_pct).tests/v2/fixtures/HWE-BAT/measurement.json (1)
1-10
: New measurement fixture for HWE-BAT includes newly introduced fields.The JSON structure (including
cycles
andstate_of_charge_pct
) is aligned with the updatedMeasurement
data class. Double-check for potential data type validations (e.g., negative cycle counts, out-of-range SoC).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
homewizard_energy/v2/models.py
(2 hunks)tests/v2/__snapshots__/test_v2_homewizard_energy.ambr
(1 hunks)tests/v2/__snapshots__/test_v2_models.ambr
(1 hunks)tests/v2/fixtures/HWE-BAT/device.json
(1 hunks)tests/v2/fixtures/HWE-BAT/measurement.json
(1 hunks)tests/v2/fixtures/HWE-BAT/system.json
(1 hunks)tests/v2/test_v2_models.py
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/v2/fixtures/HWE-BAT/device.json
- tests/v2/fixtures/HWE-BAT/system.json
- homewizard_energy/v2/models.py
- tests/v2/test_v2_models.py
- tests/v2/snapshots/test_v2_homewizard_energy.ambr
🔇 Additional comments (8)
tests/v2/__snapshots__/test_v2_models.ambr (8)
2-3
: Preliminary HWE-BAT test device addition looks good.
The new snapshot for test_device[HWE-BAT-fixtures1]
is consistent with the HWE-BAT device definition and includes the expected battery-specific parameters.
10-10
: Snapshot separator comment.
No functional changes detected here.
12-12
: Updated label for HWE-P1 measurement test snapshot.
No issues found with changing the snapshot name and referencing for the same or extended attributes.
15-15
: Re-labeled HWE-P1 measurement snapshot.
No concerns here; simply an additional scenario.
18-18
: Another HWE-P1 measurement snapshot.
This still references older or default values for cycles and state_of_charge_pct (both are None). No issues found.
21-21
: HWE-P1 measurement scenario #3.
Again, no functional changes aside from possible snapshot labeling for test coverage.
22-22
: Snapshot separator comment.
No changes to functionality.
23-24
: System snapshot for HWE-BAT.
The addition of this snapshot includes attributes relevant to HWE-BAT, such as disabling cloud and providing an uptime. Looks like a straightforward new test scenario.
Add support for two new sensor values (
cycles
andstate_of_charge_pct
).Important: These decision of these sensors are not yet final.
Summary by CodeRabbit
New Features
cycles
andstate_of_charge_pct
to the measurement data model.Bug Fixes
Documentation