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 Aqara Wall Outlet H2 EU #3653

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

Conversation

andreibsk
Copy link

Proposed change

This PR should add new configuration options and remove unused entities for Aqara Wall Outlet H2 EU (Aqara lumi.plug.aeu001) using the quirks v2 api.

Additional information

This is my first time writing a quirk and first time coding in Python, so please advise where improvements are possible.

Changes:

  • removed temperature as it wasn't working as expected
  • removed unknown/unused switch
  • added Button lock
  • added Charging Protection
  • added option to configure LED indicator
  • added Overload Protection
  • added Power on behaviour

All the new configuration options have been tested on a device I own.

Related: #3265 #3187

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 Dec 29, 2024

Codecov Report

Attention: Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.83%. Comparing base (1718afe) to head (94f6a15).
Report is 34 commits behind head on dev.

Files with missing lines Patch % Lines
zhaquirks/xiaomi/aqara/plug_eu.py 72.72% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3653      +/-   ##
==========================================
- Coverage   89.84%   89.83%   -0.01%     
==========================================
  Files         322      320       -2     
  Lines       10371    10403      +32     
==========================================
+ Hits         9318     9346      +28     
- Misses       1053     1057       +4     

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

@andreibsk
Copy link
Author

Here's a screenshot of how it looks in my setup:

image

@TheJulianJES TheJulianJES added Xiaomi Request/PR regarding a Xiaomi device 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 29, 2024
@andreibsk
Copy link
Author

I noticed that for 1 out of my 5 devices the 'Summation delivered' reading was fluctuating up and down causing warnings and wrong data in the Energy dashboard.
This was because ElectricalMeasurementCluster received values by
Received command 0x0A (TSN 31): Report_Attributes(attribute_reports=[Attribute(attrid=0x0000, value=TypeValue(type=uint48_t, value=23469))])
and additionally from XiaomiCluster -> XIAOMI_AQARA_ATTRIBUTE_E1 -> CONSUMPTION

if CONSUMPTION in attributes:

The former one seems to lag behind in updating the summ and still sends the outdated value. The latter one sends values that are up to date. This leads the aggregated value switching between the outdated and the actual value:
image

To avoid this, I've added the PlugAEU001MeteringCluster in the latest commit. Please let me know if this an appropriate solution or if there are better approaches.
a8e6e3c

cluster_id=PlugAEU001Cluster.cluster_id,
force_inverted=True,
translation_key="button_lock",
fallback_name="Button Lock",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Per HA spec, only the first letter should be capitalized, all other words should be lowercase (except for abbreviations). So, this should be "Buton lock", although we may want to use child_lock for the translation_key (and fallback_name if that is the same, as that's what we use for other devices).

Please also adjust the names of the other entities. (E.g. "Power on behavior", "LED indicator", ...) Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Changed to child_lock and updated the other names as well. 👍

Comment on lines +381 to +395
class PlugAEU001MeteringCluster(MeteringCluster):
"""Custom cluster for Aqara lumi plug AEU001."""

def _update_attribute(self, attrid, value):
if attrid == self.CURRENT_SUMM_DELIVERED_ID:
current_value = self._attr_cache.get(attrid, 0)
if value < current_value:
_LOGGER.debug(
"Ignoring attribute update for %s: new value %s is less than current value %s",
attrid,
value,
current_value,
)
return
super()._update_attribute(attrid, value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I'll have to check back on this. If the custom Xiaomi reports are more up-to-date/accurate and are also always sent, we could just ignore the ZCL reports coming in by overriding handle_cluster_general_request like this:

class LocalOccupancyCluster(LocalDataCluster, OccupancyCluster):
"""Local occupancy cluster that ignores messages from device."""
def handle_cluster_general_request(
self,
hdr: zigpy.zcl.foundation.ZCLHeader,
args: list,
*,
dst_addressing: AddressingMode | None = None,
) -> None:
"""Ignore occupancy attribute reports on this cluster, as they're invalid and sent by the sensor every hour."""

That would ignore all attribute reports from the EM cluster though, since we essentially remove all of this: https://github.com/zigpy/zigpy/blob/07e0451ce5317185e04b38e43aeb9fb168c62f28/zigpy/zcl/__init__.py#L464-L530

We could probably also block the calls to set up attribute reporting for that attribute, but we should still block incoming attribute reports, as some devices may have been connected before.

I'll need to come back to this. Hopefully soon.

Copy link
Author

Choose a reason for hiding this comment

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

Let me know if/how I could help in this regard.

@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 Jan 16, 2025
@ChristophCaina ChristophCaina mentioned this pull request Jan 17, 2025
3 tasks
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. needs reviewer answer An answer from a reviewer is needed (e.g. why a PR isn't acceptable in the current state). v2 quirk Quirks using v2 API. Might add custom entities that need translation keys in HA. Xiaomi Request/PR regarding a Xiaomi device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants