Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print native string value instead Enum when using ExternalDevice.DeviceType value #478

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

DCSBL
Copy link
Collaborator

@DCSBL DCSBL commented Jan 7, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Updated device type enumeration to use string-based representation
    • Enhanced compatibility with HomeWizard Energy API data structures
  • Bug Fixes

    • Corrected handling of device attributes and measurement data
    • Improved backward compatibility for system and measurement attributes
  • Refactor

    • Simplified external device naming conventions
    • Updated test snapshots to reflect new data model changes
  • Documentation

    • Added deprecation note for certain device attributes

Copy link
Contributor

coderabbitai bot commented Jan 7, 2025

Walkthrough

The pull request introduces significant changes to the HomeWizard Energy models, primarily focusing on enum handling and data structure modifications. The DeviceType enum is converted to a StrEnum, allowing string-based representation. The Measurement class has been updated with new mappings and a deprecation note for wifi_ssid. The CombinedModels class now handles backward compatibility by adjusting attribute locations, such as moving wifi_ssid from measurement to system and brightness from state to system.

Changes

File Change Summary
homewizard_energy/models.py - Replaced Enum with StrEnum for DeviceType
- Modified __post_serialize__ and __pre_deserialize__ methods in Measurement
- Updated CombinedModels to handle backward compatibility
tests/v1/__snapshots__/test_v1_homewizard_energy.ambr - Modified external device keys
- Removed and updated snapshots for HWE-P1 device
tests/v1/__snapshots__/test_v1_models.ambr - Updated Measurement entities with new identifiers
- Modified State to allow None for switch_lock and brightness
tests/v2/__snapshots__/test_v2_homewizard_energy.ambr - Updated external device keys and unique IDs
tests/v2/__snapshots__/test_v2_models.ambr - Modified measurement values
- Updated external device key formatting

Sequence Diagram

sequenceDiagram
    participant Models as Models Module
    participant DeviceType as DeviceType Enum
    participant Measurement as Measurement Class
    participant CombinedModels as CombinedModels Class

    Models->>DeviceType: Convert from Enum to StrEnum
    Models->>Measurement: Update serialization methods
    Measurement-->>CombinedModels: Adjust attribute locations
    CombinedModels->>Models: Implement backward compatibility
Loading

Possibly related PRs

Suggested labels

breaking-change


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d478b7e and 4d1ce6d.

📒 Files selected for processing (5)
  • homewizard_energy/models.py (2 hunks)
  • tests/v1/__snapshots__/test_v1_homewizard_energy.ambr (4 hunks)
  • tests/v1/__snapshots__/test_v1_models.ambr (2 hunks)
  • tests/v2/__snapshots__/test_v2_homewizard_energy.ambr (1 hunks)
  • tests/v2/__snapshots__/test_v2_models.ambr (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: coverage
  • GitHub Check: Scanning
🔇 Additional comments (7)
homewizard_energy/models.py (2)

7-7: LGTM! Good choice using StrEnum for better string representation.

The change from Enum to StrEnum is a good improvement that allows direct string representation of enum values without the class prefix, making the output more readable and user-friendly.

Also applies to: 451-457


Line range hint 458-479: LGTM! Well-structured implementation with proper type safety.

The ExternalDevice class is well-implemented with:

  • Proper type hints and field metadata
  • Custom deserialization for handling edge cases
  • Clean integration with the new StrEnum-based DeviceType
tests/v2/__snapshots__/test_v2_models.ambr (1)

15-15: LGTM! Test snapshots correctly reflect the new string representation.

The snapshot updates properly demonstrate the new format for external device dictionary keys, changing from 'DeviceType.TYPE_uniqueid' to 'type_uniqueid', which aligns with the StrEnum implementation.

Also applies to: 18-18, 21-21

tests/v2/__snapshots__/test_v2_homewizard_energy.ambr (1)

21-21: LGTM! Consistent test coverage for the API integration.

The snapshot updates maintain proper test coverage while correctly reflecting the new string representation format for external device types.

tests/v1/__snapshots__/test_v1_models.ambr (1)

18-18: LGTM! Proper handling of both v1 and v2 API formats.

The snapshot updates demonstrate that the changes maintain backward compatibility while implementing the new string representation format consistently across API versions.

Also applies to: 39-39

tests/v1/__snapshots__/test_v1_homewizard_energy.ambr (2)

Line range hint 1-120: Great test coverage across device types and scenarios!

The test snapshots thoroughly verify the string representation changes across:

  • Multiple device types (HWE-P1, HWE-KWH1, etc.)
  • Various external device scenarios
  • Edge cases and different data combinations

9-9: LGTM! The changes consistently represent device types as native strings.

The test snapshots now correctly show external device types as string values (e.g., 'gas_meter') instead of Enum references, which aligns with the PR objective.

Let's verify that these changes match the implementation:

Also applies to: 30-30, 51-51, 81-81

✅ Verification successful

Implementation correctly uses StrEnum with matching string values

The DeviceType class is properly implemented using StrEnum with string values that exactly match the test snapshots (e.g., "gas_meter"). The only remaining enum reference is correctly used in the deserialization logic.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the DeviceType enum is properly configured to output string values
# and that the string representations match the test snapshots

# Search for DeviceType class definition and its string representations
ast-grep --pattern 'class DeviceType {
  $$$
}'

# Search for any remaining enum-style references that should have been converted
rg "DeviceType\." --type py

Length of output: 187


Script:

#!/bin/bash
# Try different patterns to locate DeviceType class and its implementation
ast-grep --pattern 'class DeviceType'

# Search for DeviceType definition with Enum inheritance
ast-grep --pattern 'class DeviceType($$$)'

# Backup search using ripgrep
rg "class.*DeviceType" --type py -A 10

Length of output: 807


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@DCSBL DCSBL changed the title Print native string value instead Enum when using ExternalDevice.Devi… Print native string value instead Enum when using ExternalDevice.DeviceType value Jan 7, 2025
@DCSBL DCSBL enabled auto-merge (squash) January 7, 2025 22:38
@DCSBL DCSBL merged commit d1b3937 into main Jan 7, 2025
30 checks passed
@DCSBL DCSBL deleted the DCSBL-2025-29 branch January 7, 2025 22:40
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.62%. Comparing base (d478b7e) to head (4d1ce6d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #478   +/-   ##
=======================================
  Coverage   99.62%   99.62%           
=======================================
  Files           8        8           
  Lines         539      539           
  Branches       40       40           
=======================================
  Hits          537      537           
  Misses          1        1           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DCSBL DCSBL added the Bug/Bugfix Something isn't working, or is fixed label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug/Bugfix Something isn't working, or is fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant