Skip to content

Add support for Sinope TH1134ZB-HC thermostat #3821

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

Closed
wants to merge 12 commits into from

Conversation

claudegel
Copy link
Contributor

Proposed change

Add support for the new TH1134ZB-HC Sinope thermostat. Added many attributes and support for weather icon control on thermostat display.

Additional information

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

Copy link

codecov bot commented Feb 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.47%. Comparing base (e3d3849) to head (ba19eae).
Report is 162 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3821      +/-   ##
==========================================
+ Coverage   89.85%   91.47%   +1.61%     
==========================================
  Files         322      337      +15     
  Lines       10380    11841    +1461     
==========================================
+ Hits         9327    10831    +1504     
+ Misses       1053     1010      -43     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claudegel
Copy link
Contributor Author

Can you explain how to add reporting configuration for new devices in ZHA or in the quirks ? I have all reporting configuration data for Sinopé devices and would like to add this to ZHA or the quirks.

@TheJulianJES TheJulianJES added needs review This PR should be reviewed soon, as it generally looks good. needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). v2 quirk candidate This PR is a good fit to be converted to a v2 quirk. labels Feb 6, 2025
@claudegel
Copy link
Contributor Author

I'm ready to help for the v2 quirk if you want

@TheJulianJES
Copy link
Collaborator

As an initial example, something like this would replace the v1 quirk from this PR:

(
    QuirkBuilder(SINOPE, "TH1134ZB-HC")
    .replaces(SinopeTechnologiesElectricalMeasurementCluster)
    .replaces(SinopeTechnologiesThermostatCluster)
    .replaces(SinopeTechnologiesManufacturerCluster)
    .add_to_registry()
)

But no custom entities are added with that and no reporting config is applied. For that, see the IKEA example linked below.

For other examples, search QuirkBuilder in the repo and take a look at the quirks, i.e. the IKEA Vindstyrka quirk that sets up a custom sensor entity in HA. For the attribute it's used, a reporting_config is also set up (which requires the device to be reconfigured through the UI once or re-paired to apply the updated reporting config).

Do note that reporting_config can currently only be set up if you also create a custom entity for something. Using it in just one entity will also cause ZHA to bind the cluster. If more advanced functionality is required, it needs to go directly into ZHA at the moment.

@claudegel
Copy link
Contributor Author

Thank you for the info, sorry I was away since three days. I'm testing the Sinope devices quirks via files loaded in config/zhaquirks. (custom_quirks_path: /config/zhaquirks/). Do you know if I switch all quirks for thermostats, light, switch and sensor to V2 I'll be able to use the same method to test the changes before I'll push a PR.
Do you think you can merge this PR so I will have time to switch all Sinope quirks to V2 and add the reporting config also.

@claudegel
Copy link
Contributor Author

About the V2 transfer. If I understand correctly I only need to add sensor, number, switch, select and/or binary sensor for attributes in manufacturer cluster or attributes added to regular cluster. For regular cluster those sensor, number, switch, select and binary sensor will be already added for new device added to ZHA.
If I have two device with the same name like RM3250ZB but one is V2 and have different clusters/attrributes how do I name them in QuirkBuilder()
QuirkBuilder("SINOPE", "RM3250ZB")
QuirkBuilder("SINOPE", "RM3250ZB-V2") ??
Thanks for your help

@claudegel
Copy link
Contributor Author

Forget about RM3250ZB-V2, I've found it

@claudegel
Copy link
Contributor Author

@TheJulianJES I've ported all Sinopé quirks to quirks V2. Almost ready to make a PR. For now all file are in https://github.com/claudegel/sinope-zha/tree/quirks-V2. If you can have a look before I push the PR it will be appreciated.

@TheJulianJES
Copy link
Collaborator

From a very quick look, it seems like good work. Feel free to already create the PR(s), as it's easier to review/comment for me and other reviewers then.

Do note that ZHA will still create its (default) entities, even with quirks v2.
E.g. battery_percentage_remaining and IasZone entities, and so on. These should also stay the same for now, so the entities do not change for existing users (i.e. the old one doesn't work anymore and a new one is created). So, you likely shouldn't include them in quirks v2. Or is there another reason you did?

There are ways to "become" an old ZHA entity by including the cluster ID in unique_id_suffix, but it's probably not anything we should do for now.
If you need to prevent default ZHA entity discovery without removing the entire cluster, that can be done like this:

.prevent_default_entity_creation(endpoint_id=1, cluster_id=OnOff.cluster_id)

@claudegel
Copy link
Contributor Author

@TheJulianJES I agree that ZHA should take care of default entities but some manufacturer specific cluster (0xff01) attributes are already managed by ZHA. Like the on_led_color, off_led_color, on_led_intensity and off_led_intensity. Normally they should be part of the new quirks ?
So I'll finish few things and will push a PR that will replace this one as the TH1134ZB-HC will be included in the V2.

Maybe you can help me with another of my custom component. Can you point me to where is the code to add those new sensor, button, binary_sensor etc to the device page. In my other custom_component I can create all the sensors etc from device attribute but didn't succeed to get them added to the main device page.

Thank you for your help.

@TheJulianJES
Copy link
Collaborator

but some manufacturer specific cluster (0xff01) attributes are already managed by ZHA. Like the on_led_color, off_led_color, on_led_intensity and off_led_intensity. Normally they should be part of the new quirks ?

Yes, those need to stay in ZHA for now. ZHA finds them by attribute name, so they have to stay the same.
In the future, we can hopefully move them over to zha-quirks, but we'll need to do that in the way that keeps the unique ID in Home Assistant the same, so the actual entities will be the same in the end.

Maybe you can help me with another of my custom component. Can you point me to where is the code to add those new sensor, button, binary_sensor etc to the device page. In my other custom_component I can create all the sensors etc from device attribute but didn't succeed to get them added to the main device page.

So, all v2 quirk entities will show up on the device page of a ZHA device. By default, most are in the configuration section, not the main section. The configuration and diagnostics section/categories won't be show on auto-generated Home Assistant interfaces. That's expected.
Go to the device page and you'll find them there.

If you want to categorize some of the entities differently, you should be able to add entity_type=EntityType.STANDARD to the affected entities.

@TheJulianJES TheJulianJES added waiting for changes Waiting for changes to the PR and removed needs review This PR should be reviewed soon, as it generally looks good. needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). labels Apr 30, 2025
@claudegel
Copy link
Contributor Author

Ijust finish to debug my V2 quirks for all Sinopé devices. I'll push a PR this week. Do you want me to replace this PR or make another one and at the end we will remove this one as it is useless for V2. I already port the TH1134ZB-HC to V2.

@TheJulianJES
Copy link
Collaborator

It doesn't really matter, but a new PR might make sense, since it'll be quite different from this one and we can have a fresh conversation then.

@claudegel
Copy link
Contributor Author

Look like there is a bug with the number entities creation. when I add this in my quirks V2:

    .number( # Timer
        attribute_name=LightManufacturerCluster.AttributeDefs.timer.name,
        cluster_id=LightManufacturerCluster.cluster_id,
        step=1,
        min_value=0,
        max_value=86400,
        unit=UnitOfTime.SECONDS,
        translation_key="timer",
        fallback_name="Timer",
        entity_type=EntityType.CONFIG,
    )

I get this error:

2025-04-30 10:45:37.896 ERROR (MainThread) [zha.application.discovery] Failed to create entity during discovery
Traceback (most recent call last):
  File "/usr/local/lib/python3.13/site-packages/zha/application/discovery.py", line 237, in inner
    yield next(iterator)
          ~~~~^^^^^^^^^^
  File "/usr/local/lib/python3.13/site-packages/zha/application/discovery.py", line 267, in discover_device_entities
    yield from self.discover_quirks_v2_entities(device)
  File "/usr/local/lib/python3.13/site-packages/zha/application/discovery.py", line 399, in discover_quirks_v2_entities
    yield entity_class(
          ~~~~~~~~~~~~^
        unique_id=endpoint.unique_id,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<3 lines>...
        entity_metadata=entity_metadata,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/usr/local/lib/python3.13/site-packages/zha/application/platforms/number/__init__.py", line 90, in __init__
    self._analog_output_cluster_handler: ClusterHandler = self.cluster_handlers[
                                                          ~~~~~~~~~~~~~~~~~~~~~^
        CLUSTER_HANDLER_ANALOG_OUTPUT
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ]
    ^
KeyError: 'analog_output'

Is it on my side or in the number platform ?

@TheJulianJES TheJulianJES added the needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). label Apr 30, 2025
@TheJulianJES
Copy link
Collaborator

Ugh, that's an issue in ZHA. It's using the wrong number class for the quirks v2 entities in the discovery schema, although the EntityType.CONFIG should have the correct class...

Try to remove the entity_type. CONFIG is default for most quirks v2 entities, but this is still a ZHA bug. I'll take a look later.

@claudegel
Copy link
Contributor Author

Removing the line entity_type=EntityType.CONFIG, didn't fix it sorry. Will check on my side also
Thank's

@claudegel
Copy link
Contributor Author

I just push a new PR #4042 to port all Sinopé devices to V2 and it include support for the devices included in this PR.

@claudegel
Copy link
Contributor Author

So I'll close this one as it is replaced by PR 4042. Thank you for your help.

@claudegel claudegel closed this May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). v2 quirk candidate This PR is a good fit to be converted to a v2 quirk. waiting for changes Waiting for changes to the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants