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 Tuya pool sensor quirk #2927

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Add Tuya pool sensor quirk #2927

wants to merge 9 commits into from

Conversation

tschiex
Copy link
Contributor

@tschiex tschiex commented Jan 20, 2024

Proposed change

New quirk for a Pool sensor device. This is currently unusable because it relies on new entities/classes in ZHA (see https://github.com/tschiex/core) but it does not break anything.

Additional information

A quirk_id for the sensor has been added.

Relevant issue: #2565

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

@TheJulianJES TheJulianJES changed the title Adding one quirk with associated quirk_id. Add Tuya pool sensor quirk Jan 21, 2024
@TheJulianJES TheJulianJES added new quirk Adds support for a new device Tuya Request/PR regarding a Tuya device labels Jan 21, 2024
@TheJulianJES
Copy link
Collaborator

Thanks for the PR!

From #2565 (comment):
The spell tests will not tolerate that a spell sends 2 messages. So my PR fails the tests, but I cannot do anything to pass them.

The Tuya data command is also used for other Tuya devices in Z2M, right?
I could probably make a PR that also pulls out this data command/"advanced spell" part to the EnchantedDevice / spell stuff, but only send both spells when an attribute like TUYA_SPELL = 2 is set.
I'd also update the tests accordingly and then rebase this PR to be compatible.

This is currently unusable because it relies on new entities/classes in ZHA

As a note for others wanting to test this without changes to ZHA, you should already be able to access the attributes through the clusters menu (device page, "Manage Zigbee device", clusters, tuya cluster).

@TheJulianJES TheJulianJES self-assigned this Jan 21, 2024
@tschiex
Copy link
Contributor Author

tschiex commented Jan 22, 2024

Nice solution :-)

The Tuya data command is also used for other Tuya devices in Z2M, right?

Yes, Koenkk immediately identified the issue from the Tuya Zigbee trace I provided, saying he had already seen this in a few devices (he gave examples, they are somewhere in Koenkk/zigbee2mqtt#18704).

I'm wondering how I can contribute the code to HA/core/components/zha now? It breaks HA because of the missing quirk_id. Should I wait for the new zha-device-handler library to be integrated in HA? It's here: https://github.com/tschiex/core/tree/dev

@tschiex
Copy link
Contributor Author

tschiex commented Jan 22, 2024

BTW, for people trying to use this. I noticed that after pairing, HA has to be restarted twice for this to properly work. The first time, only the temperature sensor shows. After one restart, all the sensors show up as well as the Button. After a 2nd restart, all 8 'Number' entities show up.

@TheJulianJES Any possible explanation for this strange behavior?
May be it's just a matter of waiting long enough?

@TheJulianJES
Copy link
Collaborator

Hmm, generally it's enough to add the values to ZCL_INIT_ATTRS. ZHA then polls them when pairing the device.
If there's no value in the attribute cache afterwards, ZHA will not set up the sensor until the integration is reloaded (so HA restarted in your case).

However, with the Tuya devices, the attributes are all fake anyway, so when ZHA tries to poll the attributes, it never actually gets any values. I guess we might need to do something like this:

  • def __init__(self, *args, **kwargs):
    """Init."""
    super().__init__(*args, **kwargs)
    # put a default value so the sensors are created
    if self.POWER_ID not in self._attr_cache:
    self._update_attribute(self.POWER_ID, 0)
  • self._attr_cache: dict[int, Any] = {
    ZCL_DISABLE_LED_INDICATOR: False,
    ZCL_CHILD_LOCK: False,
    ZCL_FEEDING_MODE: self.FeedingMode.Manual,
    ZCL_SERVING_SIZE: 1,
    ZCL_PORTION_WEIGHT: 8,
    ZCL_ERROR_DETECTED: False,
    ZCL_PORTIONS_DISPENSED: 0,
    ZCL_WEIGHT_DISPENSED: 0,
    }

The second approach seems nicer(?) if that works correctly for you.

It might be better to send the special "update" command during from ZHA, but I don't think that's easily possible(?)

@TheJulianJES
Copy link
Collaborator

I've rebased your branch to include the latest "Tuya data query spell" PRs.
Also added a new commit that changes from your custom spell implementation to the new shared/quirks implementation.

Let me know if this still works. For testing, you'll likely have to replace the complete zhaquirks library (in the venv), as it can't easily be used with just a custom quirk (until the next quirks bump is made to HA and released).

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (0336e0d) 87.62% compared to head (3159b4e) 87.63%.

Files Patch % Lines
zhaquirks/tuya/ts0601_pool_sensor.py 90.90% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2927      +/-   ##
==========================================
+ Coverage   87.62%   87.63%   +0.01%     
==========================================
  Files         295      296       +1     
  Lines        9039     9084      +45     
==========================================
+ Hits         7920     7961      +41     
- Misses       1119     1123       +4     

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

@tschiex
Copy link
Contributor Author

tschiex commented Jan 28, 2024

@TheJulianJES I tested the PR under my HA instance and the device is properly recognized and answers requests. I further "pushed" it on my running HA instance (with the edited version of the HA/zha files) and everything shows up nicely.

Looks fine!

@TheJulianJES
Copy link
Collaborator

TheJulianJES commented Jan 28, 2024

Awesome!
The only thing I'm still thinking about is the fake attribute id for the query data button.
Since the query data command seems to be a "real" Zigbee command (and not some special Tuya construct), it would be more correct if ZHA sends the command from a button (instead of quirks needing to translate the attribute update to a command call).

However, that could also mean that we need to pull in some quirks part to properly test the code in ZHA.
I think this is fine for now. I'll try to add some tests to cover those lines.

EDIT: Hmm, although not awaiting the response of the command isn't great. (Because it's catched in _update_attribute)

@TheJulianJES
Copy link
Collaborator

Thought about this again, and it might be better to override async_update in the custom entity or the custom handler to send the "query data" command.

If the device does not get the query data command, does it sends updates on a regular interval or not at all?
(Maybe we should remove the "attribute update part" from quirks for now and see if it can be implemented properly in HA later.)

@mbuiot5
Copy link

mbuiot5 commented Mar 7, 2024

Hello. I have that same device and I cannot get any information on ZHA. I understand you made a quirk for it but I don't understand where to get it and how to apply it in hassio. I looked at zha-device-handlers but there to there no explanation on how to add it too hassio. Does anyone has a noob easy explanation on how to add this device to ZHA?

@tschiex
Copy link
Contributor Author

tschiex commented Mar 7, 2024 via email

@mbuiot5
Copy link

mbuiot5 commented Mar 7, 2024

thank you. Where is the latest quirk code?

@mbuiot5
Copy link

mbuiot5 commented Mar 8, 2024

May I request a little more help? I added the quirk and rebooted but the quirk didn't applied. I got the logs and I get this message. Any idea of what is wrong ?

2024-03-08 01:31:31.196 DEBUG (MainThread) [zhaquirks] Loading custom quirk module 'Radar' 2024-03-08 01:31:31.210 DEBUG (MainThread) [zhaquirks] Loading custom quirk module 'ts0601_garage' 2024-03-08 01:31:31.215 DEBUG (MainThread) [zhaquirks] Loading custom quirk module 'ts0601_pool_sensor' 2024-03-08 01:31:31.217 ERROR (MainThread) [zhaquirks] Unexpected exception importing custom quirk 'ts0601_pool_sensor' Traceback (most recent call last): File "/usr/local/lib/python3.12/site-packages/zhaquirks/__init__.py", line 458, in setup spec.loader.exec_module(module) File "<frozen importlib._bootstrap_external>", line 995, in exec_module File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed File "/config/custom_zha_quirks/ts0601_pool_sensor.py", line 24, in <module> from zhaquirks.quirk_ids import TUYA_POOL_SENSOR ImportError: cannot import name 'TUYA_POOL_SENSOR' from 'zhaquirks.quirk_ids' (/usr/local/lib/python3.12/site-packages/zhaquirks/quirk_ids.py) 2024-03-08 01:31:31.238 WARNING (MainThread) [zhaquirks] Loaded custom quirks. Please contribute them to https://github.com/zigpy/zha-device-handlers

@TheJulianJES
Copy link
Collaborator

a proper way requires changes in ZHA and HA core

Quirks v2 will allow exposing enough metadata from quirks to have ZHA create "custom entities" from that. So, with quirks v2, it's very likely that no HA changes are needed for this.

@tschiex
Copy link
Contributor Author

tschiex commented Mar 8, 2024

@mbuiot5 Sorry, I didn't remind the fact that the quirk already assumes changes in HA/ZHA. You will not be able to test it in its current state. My bad. If I find time this WE I will try to produce a "barely usable" version (using the ZHA toolkit)..

@TheJulianJES What would you advise here? Should I stop and wait for Quirks v2 to be integrated in ZHA/HA or insist on using the existing Quirk system? I have the feeling that my changes in HA/ZHA go beyond registering/matching (units and measures that are not in the ZCL. Would Quirk v2 deal with this?)

@chriscamicas
Copy link

@tschiex Hi, thanks for your hard work!
Is there any way I can help move this integration forward?
I'd really like to integrate this pool sensors to HA :)

@tschiex
Copy link
Contributor Author

tschiex commented May 13, 2024

Well. The PR I had (in ZHA but also in the core of HA https://github.com/tschiex/core) was working, you may have a look at it. The only likely remaining issue was the possibility that the device still needed "Data Query" requests from ZHA to "speak". This can easily be solved by an automation and the ZHA toolkit. My understanding is that the quirks which were required to make it work (in HA, not ZHA) may become a thing of the past with the arrival of quirk v2. So I'm mostly waiting for quirk v2...

@guillaumedelre
Copy link

thanks all for your work, i'd really like to integrate this pool sensors to HA too :)

@brenard
Copy link

brenard commented May 25, 2024

Hello, I have the same device and I try to make it working with ZHA and your quirk. I rebased your changes on HA core to the latest 2024.5.5 tag and build a HA docker image with it. I put my Dockerfile bellow:

FROM ghcr.io/home-assistant/home-assistant:stable

WORKDIR /usr/src
COPY homeassistant/components/sensor/const.py homeassistant/components/sensor/
COPY homeassistant/components/sensor/device_condition.py homeassistant/components/sensor/
COPY homeassistant/components/sensor/device_trigger.py homeassistant/components/sensor/
COPY homeassistant/components/sensor/strings.json homeassistant/components/sensor/
COPY homeassistant/components/zha/button.py homeassistant/components/zha/
COPY homeassistant/components/zha/core/cluster_handlers/measurement.py homeassistant/components/zha/core/cluster_handlers/
COPY homeassistant/components/zha/number.py homeassistant/components/zha/
COPY homeassistant/components/zha/sensor.py homeassistant/components/zha/
COPY homeassistant/const.py homeassistant/
RUN python -m pip install git+https://github.com/tschiex/zha-device-handlers.git@dev
RUN sed -i '/zha-quirks/d' homeassistant/homeassistant/components/zha/manifest.json

Note: the last sed command is needed because otherwise the official zha-quirks is reinstalled during HA startup.

I successfully linked my device with ZHA but I only see the temperature sensor without value retrieved.

Capture d’écran du 2024-05-25 18-35-14
Capture d’écran du 2024-05-25 18-35-37
Capture d’écran du 2024-05-25 18-35-58

May be I missed something to have all the sensors. Do you have some advices?

Many tanks!

@tschiex
Copy link
Contributor Author

tschiex commented May 27, 2024

I haven't fully understood why but this device will need two HA reboots to show up all its sensors. See #2927 (comment)

If this is not enough, I would advise to install the ZHA toolkit and send a data query packet (Send cluster command, command id 3) to the device (target endpoint 1, use the device IEEE address). When I started trying to have the device work, I installed an automation that does this regularly.

@brenard
Copy link

brenard commented May 28, 2024

I haven't fully understood why but this device will need two HA reboots to show up all its sensors. See #2927 (comment)

If this is not enough, I would advise to install the ZHA toolkit and send a data query packet (Send cluster command, command id 3) to the device (target endpoint 1, use the device IEEE address). When I started trying to have the device work, I installed an automation that does this regularly.

Thanks for your return. I try to link my device and reboot HA two times and I still have only the temperature sensor with an unknown value. I try to send the command as explain, but I'm not sure to know what I have to set as cluster value. I try different values without success.

An example of sent request:

service: zha_toolkit.zcl_cmd
data:
  ieee: 7c:c6:b6:ff:fe:xx:xx:xx
  cluster: 1
  cmd: 3
  endpoint: 1

The answer:

zha_toolkit_version: v1.1.10
zigpy_version: 0.64.0
zigpy_rf_version: 0.38.4
ieee_org:
  - 117
  - 190
  - 138
  - 254
  - 255
  - 182
  - 198
  - 124
ieee: 7c:c6:b6:ff:fe:xx:xx:xx
command: zcl_cmd
command_data: null
start_time: "2024-05-28T12:59:21.996665+00:00"
errors: []
params:
  cmd_id: 3
  endpoint_id: 1
  cluster_id: 1
  dir: 0
  tries: 1
  expect_reply: true
  args: []
  read_before_write: true
  read_after_write: true
cmd_reply:
  - 3
  - 195
success: true

It seem to have no effect. I join my device debug info.
zha-a1d60fa889415f8bf4de3daa749b9c29-_TZE200_v1jqz5cy TS0601-765652a5ba56ab759d450a4adca9bf2a(1).json

Do you have an example of request (in YAML format) I have to sent ? Or could you please share your automation that sent the request ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new quirk Adds support for a new device Tuya Request/PR regarding a Tuya device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants