Remove Product model and simplify model name conversion #449
Conversation
WalkthroughThe pull request introduces significant changes to the HomeWizard Energy library's model handling. The primary modifications involve removing the Changes
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 #449 +/- ##
==========================================
- Coverage 98.21% 98.13% -0.08%
==========================================
Files 8 8
Lines 503 483 -20
Branches 36 33 -3
==========================================
- Hits 494 474 -20
Misses 8 8
Partials 1 1 ☔ 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 (2)
homewizard_energy/models.py (1)
34-37: Consider adding debug logging for unsupported models
While raising aValueErroris appropriate, adding a debug or warning log statement before the exception can help diagnose configuration issues if logs are available.tests/v1/__snapshots__/test_v1_models.ambr (1)
45-45: Retain naming consistency for the socket device.Renaming from product objects to
model_name='Wi-Fi Energy Socket'is correct. Confirm that any references to the old product object have been updated in your acceptance tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
homewizard_energy/const.py(1 hunks)homewizard_energy/models.py(3 hunks)tests/test_models.py(1 hunks)tests/v1/__snapshots__/test_v1_homewizard_energy.ambr(1 hunks)tests/v1/__snapshots__/test_v1_models.ambr(1 hunks)tests/v2/__snapshots__/test_v2_homewizard_energy.ambr(1 hunks)tests/v2/__snapshots__/test_v2_models.ambr(1 hunks)
🔇 Additional comments (22)
homewizard_energy/models.py (3)
14-14: Imports look consistent with PR objectives
The direct imports for MODEL_TO_ID and MODEL_TO_NAME facilitate a simpler approach to model handling.
44-44: Field initialization is consistent and clear
Defining model_name and id at the class level ensures clarity for the device representation. This approach separates model representation from other fields (e.g., firmware_version).
58-58: Post-deserialization logic effectively uses new constants
Using MODEL_TO_NAME and get_verification_hostname simplifies the device setup and ensures consistency with the new mapping dictionaries.
homewizard_energy/const.py (2)
6-17: MODEL_TO_ID dictionary is well-defined
This mapping effectively replaces the older PRODUCTS structure, directly linking model identifiers to unique IDs.
19-28: MODEL_TO_NAME dictionary streamlines device naming
Providing a direct name mapping for each model removes the need for a separate Product class and minimizes complexity.
tests/test_models.py (1)
5-5: Import for get_verification_hostname is appropriate
This aligns the test suite with the removal of the Product class and the new function-based model handling.
tests/v2/__snapshots__/test_v2_homewizard_energy.ambr (1)
3-3: Device snapshot updated with model_name
Replacing the nested product structure with model_name is consistent with the simplified design.
tests/v2/__snapshots__/test_v2_models.ambr (2)
3-3: Snapshot for Battery device aligns with new approach
Using model_name='Plug-In Battery' confirms that the dictionary-based mapping is functioning correctly.
6-6: Snapshot for P1 device reflects simplified model handling
The updated model_name='Wi-Fi P1 Meter' confirms consistency with the new mapping logic.
tests/v1/__snapshots__/test_v1_models.ambr (6)
36-36: Consistent with the updated model structure.
The snapshot now captures the simplified approach by using model_name directly. This consistency is crucial for verifying that the device representation is correct moving forward.
39-39: Confirm updated device type references.
The introduction of model_name='Wi-Fi kWh Meter 3-phase' aligns with the product_type='HWE-KWH3'. Ensure that references to HWE-KWH3 throughout the codebase remain accurate for successful integration tests.
42-42: Ensure test coverage for P1 meters.
Now that the device references model_name='Wi-Fi P1 Meter', verify that existing tests for P1 meter functionalities still pass as expected.
48-48: Confirm water meter references.
Swapping to model_name='Wi-Fi Watermeter' is consistent with the new approach. Double-check that all older references to Product(name="Wi-Fi Watermeter") have been eliminated to avoid confusion.
51-51: Check brand labeling for SDM230 devices.
Scopes that rely on the brand naming (e.g., 'SDM230-wifi') might need corresponding updates elsewhere. Confirm that third-party integrations, if any, are unaffected by this new model_name.
54-54: Validate references for SDM630 device.
The renamed usage to model_name='Wi-Fi kWh Meter 3-phase' for SDM630-wifi is consistent with the other 3-phase references. Confirm tests that rely on the older naming convention are updated accordingly.
tests/v1/__snapshots__/test_v1_homewizard_energy.ambr (7)
63-63: Align test objects with device model changes.
Replacing the nested Product object with model_name='Wi-Fi kWh Meter 1-phase' ensures consistency with the direct mapping approach. Nice refactor!
66-66: Maintain uniform naming for multi-phase meters.
Using model_name='Wi-Fi kWh Meter 3-phase' is consistent, but verify up-stream references (e.g., UIs or logs) for any mismatch in naming or formatting.
69-69: Accurate naming for P1 Meters.
This update consolidates the P1 Meter representation. Confirm that all relevant integration tests reflect the new name format.
72-72: Check external references for the Energy Socket.
The new model_name='Wi-Fi Energy Socket' might require updates in documentation or external usage references. Verify that everything remains consistent.
75-75: Consistent transition for the water meter.
Moving to model_name='Wi-Fi Watermeter' eliminates dependencies on the old Product object. Double-check acceptance/integration tests rely solely on the new approach.
78-78: Ensure correctness for SDM230 naming.
Snapshot updates to model_name='Wi-Fi kWh Meter 1-phase' should align with the recognized device class. Confirm third-party scripts or references to SDM230-wifi remain unaffected.
81-81: Verify 3-phase references for SDM630.
Renaming to model_name='Wi-Fi kWh Meter 3-phase' keeps snapshots uniform. Check that user-facing documentation aligns with this updated reference.
Summary by CodeRabbit
New Features
MODEL_TO_IDandMODEL_TO_NAMEfor mapping HomeWizard Energy device models to their IDs and namesProductclassRefactor
Deviceclass to usemodel_nameinstead of nestedproductattributeget_verification_hostnamefunction to use new model ID mappingTests
Productclass