Add Niko Connected single/double switch

[RubenVerborgh]

Proposed change

Add support for the Niko Connected single switch (552-721X1) and Connected double switch (552-721X2).

Additional information

Request and previous work

• This PR implements [Device Support Request] Niko Double connectable switch,10A (552-721X2) #2286, informed by Add Niko Connectable switch support #2723
• Not all switches were working with previous approaches (Niko 552-721X2 endpoint actions not working Koenkk/zigbee2mqtt#13737), so I re-performed an analysis of the Niko protocol, which I incorporated into this PR.

Functionality

• These Niko switches actually consist of three kinds of entities, which are coupled in the default configuration but can be decoupled:
  • 1 or 2 switches
  • 2 or 4 buttons (each on-device button can have its external extension button)
  • 1 of 2 status LEDs
• This PR exposes each of those entities separately, such that they can be controlled individually:

Events

• The buttons additionally generate events (short and long presses):

Configuration

• Several configuration options are exposed as entities:

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.09%. Comparing base (ba3c7c7) to head (e6c8ad3).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3701      +/-   ##
==========================================
+ Coverage   90.94%   91.09%   +0.15%     
==========================================
  Files         326      328       +2     
  Lines       10585    10775     +190     
==========================================
+ Hits         9626     9816     +190     
  Misses        959      959              

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

[RubenVerborgh]

Feel free to already test and provide feedback; perhaps some points before this PR is merged.

Improvements

• The apply_custom_configuration (https://github.com/zigpy/zha-device-handlers/pull/3701/files#diff-8c13a1ac582c2c486609043c4ca29ce34c03f0a9e07000e97800f953921817d7R38) does not seem to be fired / work as expected. The goal is that a couple of initialization parameters are written, using a pattern I've copied from some other quirks in the repo (grep for attr_config).
• Similarly, bind (https://github.com/zigpy/zha-device-handlers/pull/3701/files#diff-8c13a1ac582c2c486609043c4ca29ce34c03f0a9e07000e97800f953921817d7R156) does not seem to be called, and the quirk is relying on it for pre-loading values. This may or may not be related to Cluster.bind is not async #3704, but again I've copied the pattern from other quirks.

Functionality

• Currently, I'm exposing the option to have different colored lights (white / red / blue / purple) as an alert functionality, which seems to be the original intention. However, I could also expose this as a choice for indicator light color, such that, for instance, an outside toilet/bathroom switch could light up in red rather than white to indicate occupation. This is trivial for the one-LED version, and feasible for the two-LED version. But I'm not sure to what extent it is desirable to encode such more advanced behavior in quirks, as opposed to directly translating the manufacturer's intentions.

Any feedback on the above would be helpful!

[TheJulianJES]

I'll need to go through this properly when I find some free time, but one thing:
Bus shouldn't really be needed anymore. The docs/README really have to be updated, but I just haven't found the time yet.

You can access update_attribute and other methods on a different cluster directly (from your current cluster) using something like this:

zha-device-handlers/zhaquirks/xiaomi/aqara/motion_agl1.py

Lines 137 to 140 in b15a6dc

	self.endpoint.ias_zone.update_attribute(
	IasZone.AttributeDefs.zone_status.id,
	IasZone.ZoneStatus.Alarm_1 if value == AqaraMotion.Moving else 0,
	)

You can also call methods on a different endpoint with self.endpoint.device.endpoints[2].ias_zone.do_something() for example (IIRC).

ias_zone is the ep_attribute of a cluster. You can also access it with self.endpoint.in_clusters[IasZone.cluster_id] e.g.

Doing this might avoid needing some of the tasks? Again, I have to look at this properly some time soon, but it seems a bit weird looking at it now.

Same goes for the inheritance with NikoCluster/the other Niko clusters.
And the NikoQuirkBuilder also is a bit out of the usual pattern that we wanted to have with "quirks v2", though I see why you're doing it.

Again, this isn't a review. Just some thoughts whilst having a very quick look at this.
Not sure why apply_custom_configuration wouldn't be working.. Your HA version is somewhat up-do-date?
You could also try overriding it in the CustomDeviceV2 class: https://github.com/zigpy/zigpy/blob/08f8f99df3d235aa0c0154331cb7c5b3dffde3e1/zigpy/quirks/__init__.py#L119-L137

[ccantill]

The same quirk also appears to work fine with the NIKO zigbee dimmer:

class NikoDimmer(NikoSwitch):
    """Niko Connected Dimmer 552-722X1."""

    model = "Connectable dimmer,3-200W,2-wire"
    model_friendly = "Connected dimmer"
    button_count = 2

[RubenVerborgh]

Thanks @TheJulianJES for looking at this!

I'll need to go through this properly when I find some free time, but one thing: Bus shouldn't really be needed anymore.

I see! I rewrote the code without Bus.
I still kept a listener-like mechanism to avoid inter-cluster dependencies.

Same goes for the inheritance with NikoCluster/the other Niko clusters.

The main thing this class now does is implementing the listener mechanism.

And the NikoQuirkBuilder also is a bit out of the usual pattern that we wanted to have with "quirks v2", though I see why you're doing it.

Yeah, there's quite some reuse between the switches. While I could have a static setup_buttons function somewhere, this would cause us to lose the chaining calling pattern. That's why the QuirkBuilder invites subclassing (as also used in TuyaQuirkBuilder).

I did get rid of Niko-specific device classes though, by putting all their functionality in the quirk builder.

Not sure why apply_custom_configuration wouldn't be working.. Your HA version is somewhat up-do-date?

Latest versions, yes. Had trouble getting apply_custom_configuration and bind to run.

---

The code's been updated based on your suggestions. I'll still do some testing myself, and would be happy if others provided feedback as well.

@ccantill That's neat! I don't have a dimmer here to test though. Would it need anything beyond the code you've added? There might be more dimmer-specific properties.

[ccantill]

@ccantill That's neat! I don't have a dimmer here to test though. Would it need anything beyond the code you've added? There might be more dimmer-specific properties.

Nothing additional needed. There might be more functionalities but I have no idea how to go about discovering those.