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

Add HA entities for Schneider Electric Thermostats #3575

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

uvNikita
Copy link
Contributor

@uvNikita uvNikita commented Dec 3, 2024

Proposed change

Add most of the manufacturer specific attributes as Home Assistant entities (numbers, enums).

Additional information

#1705
#3473
Koenkk/zigbee2mqtt#23739

Screenshots

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

@uvNikita uvNikita force-pushed the se/thermostat-ha-controls branch 2 times, most recently from df07cb1 to b1412ed Compare December 3, 2024 11:27
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.85%. Comparing base (97413e3) to head (ef32e58).
Report is 5 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3575      +/-   ##
==========================================
+ Coverage   89.79%   89.85%   +0.06%     
==========================================
  Files         323      321       -2     
  Lines       10414    10368      -46     
==========================================
- Hits         9351     9316      -35     
+ Misses       1063     1052      -11     

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

@uvNikita
Copy link
Contributor Author

uvNikita commented Dec 3, 2024

Tested the changes on my devices and everything is working good.

A couple of things I couldn't figure out:

  • Missing percent unit, e.g. in display_brightness and display_inactive_brightness
  • Also control_status enum should be readonly (see comment), but not sure how to achieve it with quirks v2

Edit: figured out the second issue (had to set entity_platform=EntityPlatform.SENSOR)

@uvNikita uvNikita force-pushed the se/thermostat-ha-controls branch from b1412ed to 1d9f9ac Compare December 3, 2024 20:36
@TheJulianJES TheJulianJES added needs review This PR should be reviewed soon, as it generally looks good. v2 quirk Quirks using v2 API. Might add custom entities that need translation keys in HA. labels Dec 3, 2024
attribute_name=SEUserInterface.AttributeDefs.se_brightness.name,
translation_key="display_brightness",
fallback_name="Display Brightness",
# unit="%",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have PERCENTAGE as a unit for this. It's just a string: PERCENTAGE = "%", so this should already work when uncommented. Can you confirm?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested, and if I uncomment this line, I get the following error:

    Failed to create platform entity: <class 'zha.application.platforms.number.NumberConfigurationEntity'> [args=('b0:c7:de:ff:fe:be:38:1f-1', [<zha.zigbee.cluster_handlers.hvac.UserInterfaceClusterHandler object at 0x7fe3642a55e0>], <zha.zigbee.endpoint.Endpoint object at 0x7fe3642a4c80>, <CustomDeviceV2 model='EKO07259' manuf='Schneider Electric' nwk=0xDC1A ieee=b0:c7:de:ff:fe:be:38:1f is_initialized=True> - quirk_applied: True - quirk_or_device_class: zigpy.quirks.v2.CustomDeviceV2 - quirk_id: None), kwargs={'entity_metadata': NumberMetadata(entity_platform=<EntityPlatform.NUMBER: 'number'>, entity_type=<EntityType.CONFIG: 'config'>, cluster_id=516, endpoint_id=1, cluster_type=<ClusterType.Server: 0>, initially_disabled=False, attribute_initialized_from_cache=True, translation_key='display_inactive_brightness', fallback_name='Display inactive brightness', attribute_name='se_inactive_brightness', reporting_config=None, min=0, max=100, step=1, unit='%', mode=None, multiplier=None, device_class=None)}]

Traceback (most recent call last):
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/application/gateway.py", line 335, in create_platform_entities
    platform_entity = platform_entity_class.create_platform_entity(
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/application/platforms/number/__init__.py", line 229, in create_platform_entity
    return cls(unique_id, cluster_handlers, endpoint, device, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/application/platforms/number/__init__.py", line 242, in __init__
    super().__init__(unique_id, cluster_handlers, endpoint, device, **kwargs)
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/application/platforms/__init__.py", line 297, in __init__
    self._init_from_quirks_metadata(entity_metadata)
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/application/platforms/number/__init__.py", line 269, in _init_from_quirks_metadata
    self._attr_native_unit_of_measurement = validate_unit(
                                            ^^^^^^^^^^^^^^
  File "/nix/store/2dcvclynic8hvxi9lcfrcsy1k1iwbz1p-python3.12-zha-0.0.39/lib/python3.12/site-packages/zha/units.py", line 167, in validate_unit
    return UNITS_OF_MEASURE[type(external_unit).__name__](external_unit.value)
           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'str'

Copy link
Contributor Author

@uvNikita uvNikita Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, as of now, unit can't be string since then the code resolves into:

UNITS_OF_MEASURE[type("%").__name__](external_unit.value) -> UNITS_OF_MEASURE['str'](external_unit.value) -> KeyError: 'str'

@uvNikita uvNikita force-pushed the se/thermostat-ha-controls branch from 4c2b386 to f811d82 Compare December 10, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review This PR should be reviewed soon, as it generally looks good. v2 quirk Quirks using v2 API. Might add custom entities that need translation keys in HA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants