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

Handle Long entity names #115

Merged
merged 9 commits into from
May 12, 2024
Merged

Handle Long entity names #115

merged 9 commits into from
May 12, 2024

Conversation

GuyKh
Copy link
Owner

@GuyKh GuyKh commented May 12, 2024

User description

Handle Long entity names due to long DeviceInfo names

solves #113


PR Type

Bug fix, Enhancement


Description

  • Introduced IecEntityType enum to differentiate between GENERIC, CONTRACT, and METER entity types.
  • Refactored get_device_info function to utilize the new IecEntityType, improving how device information is constructed.
  • Enhanced binary sensor and sensor entities to initialize with specific entity types, improving data handling and accuracy.
  • Updated translation keys in en.json to make the UI text more concise and user-friendly.

Changes walkthrough 📝

Relevant files
Enhancement
binary_sensor.py
Enhance binary sensor initialization and device info handling

custom_components/iec/binary_sensor.py

  • Added IecEntityType to the binary sensor initialization.
  • Modified device_info method to use IecEntityType.
  • +4/-3     
    commons.py
    Introduce IecEntityType enum and refactor device info retrieval

    custom_components/iec/commons.py

  • Introduced IecEntityType enum for differentiating entity types.
  • Refactored get_device_info to utilize IecEntityType and enhanced
    device info details.
  • +27/-10 
    iec_entity.py
    Update IEC entity base class to support entity type differentiation

    custom_components/iec/iec_entity.py

  • Added IecEntityType to entity initialization.
  • Updated device info retrieval to include entity type.
  • +5/-3     
    sensor.py
    Refactor sensor descriptions and initialization for entity type
    specificity

    custom_components/iec/sensor.py

  • Introduced new data classes for meter and contract specific
    descriptions.
  • Updated sensor initialization to use new entity type specific classes.

  • +36/-15 
    en.json
    Streamline translation entries and improve user interface text

    custom_components/iec/translations/en.json

  • Updated sensor and binary sensor names to remove redundant 'IEC'
    prefix.
  • Simplified configuration step titles and descriptions.
  • +15/-15 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @GuyKh GuyKh linked an issue May 12, 2024 that may be closed by this pull request
    4 tasks
    @GuyKh GuyKh added the Bug fix label May 12, 2024
    @GuyKh GuyKh self-assigned this May 12, 2024
    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added the enhancement New feature or request label May 12, 2024
    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Description updated to latest commit (f100f72)

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves multiple files with significant changes including refactoring and introduction of new enums and methods. The logic changes are moderate, and understanding the context of entity types and their handling in device information requires careful review.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The use of iec_entity_type in get_device_info assumes that meter_id is provided when the entity type is METER. If meter_id is None, this could lead to incorrect or incomplete device information.

    Data Handling: The refactoring introduces new entity descriptions and modifies how device information is constructed. There is a potential for data mismatches or errors if not all scenarios are handled correctly, especially with the new enum types.

    🔒 Security concerns

    No

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestions                                                                                                                                                       
    Maintainability
    Improve variable naming for clarity and to avoid potential conflicts with built-in names.

    Consider using a more descriptive variable name for name in the get_device_info function
    to avoid confusion with the built-in Python function name() and to enhance code
    readability.

    custom_components/iec/commons.py [54]

    -name = "IEC"
    +device_name = "IEC"
     
    Refactor get_device_info into smaller functions for better maintainability and readability.

    To improve the maintainability and readability of the get_device_info function, consider
    refactoring the large function into smaller, more focused functions. This can help in
    isolating functionality and making the code easier to manage and test.

    custom_components/iec/commons.py [40-77]

     def get_device_info(contract_id: str, meter_id: str | None, iec_entity_type: IecEntityType = IecEntityType.GENERIC) -> DeviceInfo:
         """Get device information based on contract ID and optional meter ID.
         ...
    +    return create_device_info(identifier, name, model, serial_number)
    +
    +def create_device_info(identifier, name, model, serial_number):
         return DeviceInfo(
             identifiers={(DOMAIN, identifier)},
             name=name,
             manufacturer="Israel Electric Company",
             model=model,
             serial_number=serial_number,
         )
     
    Best practice
    Ensure consistent data type handling for contract_id by converting it at the function start.

    To ensure that the contract_id is always a string and to avoid repeated conversions,
    convert contract_id to a string at the beginning of the get_device_info function.

    custom_components/iec/commons.py [59]

    +contract_id = str(contract_id)
     contract_id = str(int(contract_id))
     
    Performance
    Simplify and enhance the performance of entity type handling using a dictionary mapping.

    Instead of using a match-case statement, consider using a dictionary mapping for
    iec_entity_type to functions or lambda expressions that return the required values. This
    approach can simplify the code and improve performance.

    custom_components/iec/commons.py [58-66]

    -match iec_entity_type:
    -    case IecEntityType.CONTRACT:
    -        contract_id = str(int(contract_id))
    -        name = f"IEC Contract [{contract_id}]"
    -        model = "Contract: " + contract_id
    -    case IecEntityType.METER:
    -        name = f"IEC Meter [{meter_id}]"
    -        model = "Contract: " + contract_id
    -        serial_number = ("Meter ID: " + meter_id) if meter_id else ""
    +entity_type_mapping = {
    +    IecEntityType.CONTRACT: lambda: (f"IEC Contract [{contract_id}]", "Contract: " + contract_id, None),
    +    IecEntityType.METER: lambda: (f"IEC Meter [{meter_id}]", "Contract: " + contract_id, ("Meter ID: " + meter_id) if meter_id else "")
    +}
    +name, model, serial_number = entity_type_mapping.get(iec_entity_type, lambda: ("IEC", None, None))()
     
    Possible issue
    Add validation for meter_id to prevent runtime errors when it is required.

    To avoid potential runtime errors, add a validation check for meter_id when the
    iec_entity_type is METER to ensure it is not None before using it in string operations.

    custom_components/iec/commons.py [65]

    +if iec_entity_type == IecEntityType.METER and meter_id is None:
    +    raise ValueError("meter_id is required when iec_entity_type is METER")
     serial_number = ("Meter ID: " + meter_id) if meter_id else ""
     

    @GuyKh GuyKh merged commit 8f46cd7 into main May 12, 2024
    8 checks passed
    @GuyKh GuyKh deleted the short-labels branch May 12, 2024 10:00
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Very long Entity names with Device Name prefixes
    1 participant