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

Cluster inspection is a problem with Matter which is being abused by Apple. #36408

Closed
jonsmirl opened this issue Nov 6, 2024 · 8 comments
Closed
Labels
feature request feature work needs triage waiting for feedback Waiting for feedback from reporter/others about bug reproducing and status.

Comments

@jonsmirl
Copy link
Contributor

jonsmirl commented Nov 6, 2024

Feature description

@bzbarsky-apple

This has already been filed with Apple. FB15690689 #36403
Matter needs to close the door to this.

Problem description:
My product is a light switch which has a built in LED to indicate the status of the switch. I created a standard LightSwitch endpoint which was marked with the LightSwitch device type. I then extended the standard endpoint by adding a LevelControl cluster representing my LED. I marked this extension by adding a VendorDeviceType. I was careful to do my extension in such a way that it in no way impacted the functions required by the standard LightSwitch device type. My extension is a pure super set.

When I added this device to Apple Home I observed a completely unexpected behavior. Apple Home added the Light Switch but then it performed cluster inspection and also added a Light Bulb device.
image

Impacts:
Using cluster inspection to extend endpoint capability is a backdoor way to extend the spec and it should be prohibited by the spec. Due to Apple Home performing cluster inspection I have now lost any ability to create vendor specific super sets of a device type. I lose this ability because I have no mechanism to disable Apple Home's cluster inspection. Also consider what is going to happen if the spec adds indicator LEDs to the Light Switch specification. Apple's creation of this UI disrupts how an indicator LED would work (the LED should not have a UI, instead it is bound to the switch group so that it tracks the state of the group). But now, because of Apple Home's cluster inspection, we can't add indicator LEDs to the Light Switch type.

Proposed solution:

First, all cluster inspection should be banned. Endpoint capability is determined by the device type(s) and only the device type(s). This is the path to madness, if these extensions are not marked with a vendor device type there is no mechanism to sort out conflicting extensions.

Any extensions to an endpoint MUST be marked by a VendorDeviceType. And these extensions MUST be a pure super set. It MUST still be possible to use the endpoint's base device type and to ignore the vendor extensions. Endpoints like this would list two device types, the base device type and the vendor device type.

Apple Home can still generate this extended UI, however, to trigger it you will need to add an AppleDeviceType on your endpoint in addition to the base device type to indicate that it has the extended capabilities. If that AppleDeviceType isn't there, Apple Home should not go probing for clusters. Note that even though this is an AppleDeviceType any vendor can use it, it is an AppleDeviceType because Apple defined it.

Since Apple Home has already shipped this a legacy solution is needed. I would propose that Apple Home define and document this AppleDeviceType. Newly shipped products would need to include it in the future in order to trigger the UI. For existing products Apple Home should be modified to check for the existence of a VendorDeviceType on the endpoint and if there is a VendorDeviceType it should disable this UI. Using lack of a VendorDeviceType to enable the Apple Home extended behavior is a legacy solution. All future devices should add the AppleDeviceType to trigger this extended UI.

The conformance tests should be modified to enforce this. If an endpoint contains clusters or attributes which are not part of the base device type then it MUST be marked with a vendor device type.

Requiring extensions to be clearly marked and prohibiting cluster inspection will close this backdoor to spec extensions....

Platform

all

Platform Version(s)

No response

Anything else?

No response

@andy31415
Copy link
Contributor

Reading the description I am not convinced "all cluster inspection should be banned" is a obvious following conclusion from the issue you encounter.

Overall I would expect each ecosystem integration would try to do their best to implement what the user expects or can use. It seems as a user if I see "on/off + level control" it is likely something that I can turn on/off and I can control some level and myself as a user would like a UI with a on/off button and a slider for each of those functionalities.

This expectation would be personal and maybe as a tinkerer/advanced user, however I 100% want that if I can understand a cluster I can show a usable UI for it. I imagine if you want something completely hidden you could implement a custom cluster and then no UI would be visisble. However then you would be somewhat working against matter wanting things to interoperate to the greatest extent possible.

@andy31415
Copy link
Contributor

Apart from my previous reply @jonsmirl , what resolution would you expect from the SDK specifically for Matter needs to close the door to this. ?

I still read this as a request for "Apple UI shows me something that I did not expect/want, please fix it".
If you are asking for some sort of ban, that sounds like some spec requirement instead.

@andy31415 andy31415 added the waiting for feedback Waiting for feedback from reporter/others about bug reproducing and status. label Nov 6, 2024
@jonsmirl
Copy link
Contributor Author

jonsmirl commented Nov 6, 2024

The root problem is that if everyone starts adding extension clusters then it is almost certain different companies will use those extensions for different, conflicting purposes and that's what happen here.

But this is not a problem if everyone who adds extensions simply claims ownership for those extensions by marking them with a VendorDeviceType. Then if you don't recognize that added VendorDeviceType tag, leave them alone. Image if this was a medical device and a LevelControl UI was blindly added allowing control of drug flow which was never meant for end user access.

So there needs to be a mechanism to tell other companies to leave these clusters alone. You don't know what they do and I don't want you exposing a UI for them.

The proposal is:

  1. any extension to an endpoint MUST be marked by adding an additional VendorDeviceType. Note that this VendorDeviceType can be shared, Apple could make an AppleDeviceType which can be shared by anyone wanting access to the Apple extensions.
  2. If you don't recognize the added VendorDeviceType leave the clusters alone since you don't know what they do.

The SDK would enforce adding a VendorDeviceType to the endpoint before allowing the creation of extension clusters or attributes.

@jonsmirl
Copy link
Contributor Author

jonsmirl commented Nov 7, 2024

Apart from my previous reply @jonsmirl , what resolution would you expect from the SDK specifically for Matter needs to close the door to this. ?

I still read this as a request for "Apple UI shows me something that I did not expect/want, please fix it". If you are asking for some sort of ban, that sounds like some spec requirement instead.

Apple has indicated they like it the way that it is and they won't fix it.

How can anyone create vendor extensions in a world like this? My extensions are not for human use, they are machine to machine and are used to synchronize the indicator display. If you use that UI to interact with them they will get out of sync. Plus it is cluttering up Apple Home with dozens of useless devices. I need a mechanism to tell Apple to go away and leave my clusters alone. Using the SDK to enforce the addition of a VendorDeviceType when extensions are present achieves that. That creates a way to indicate which extensions are safe to play with and which aren't.

@bzbarsky-apple
Copy link
Contributor

This is not an SDK issue.

In terms of the spec, there was discussion about this earlier today, and the spec will likely be tightened up to say that standard clusters that are not part of the device type(s) for an endpoint must not be present on that endpoint. And presumably some device types will grow some optional clusters to reflect what people might be doing right now....

@bzbarsky-apple
Copy link
Contributor

And @jonsmirl you are of course right that people just adding clusters on endpoints and assuming semantics for them, on both server and client, is bad for interop, hence the above-mentioned plan to tighten up the spec here.

@jonsmirl
Copy link
Contributor Author

jonsmirl commented Nov 7, 2024

The reason people are asking for these extensions is because the existing Matter Light Switch device type is insanely hard to use. As I've discussed with Boris my light switch keypad device has become a Frankenstein monster after being forced to implement 42 different endpoints from a single device if it is going to do behave as current spec requires.

There are two problems with lighting devices, one simple and one complex.

The simple one. In multi-gang devices sometimes you don't use all of the gangs. If the gang is not connected to a load you don't want to display a UI for it. The On/Off Cluster needs an attribute for '"NoLoad". Then there is a toggle in the phone app UI to suppress the display of devices with no load attached. This needs to be a toggle in the phone app since you need to see the NoLoad devices during setup, but once they are set up you want to suppress their display.

The complex one. The number of endpoints needed to construct a smart light switch is very high, and for my devices it is currently five endpoints per lighting load. When ganged devices are involved that number multiplies. My eight button keypads now have 42 endpoints -- Root, Aggregator + 8 * (Light Bulb, Light Switch, Generic Switch, Indicator Led, Electrical Power). Since there is no defined smart switch type those endpoints have to be grouped using the TAGLIST feature on the Descriptor cluster (Matter 1.3) or the Fixed Label cluster (pre-1.3). None of the Google/Apple/Amazon apps support this crazy collection of endpoints.

An exploding number of endpoints is a particularly acute problem with lighting. My very large Insteon installation has 63 light switches and I am unable to replace it using Matter devices. If each of those 63 light switches has five endpoints, that's 315 endpoints to deal with and that becomes a major problem to manage. Light switches really should be a single endpoint just to keep things manageable in large installations.

I would propose a mechanism to help with this explosion of endpoints, initially explained here: #36329 The proposal is that if the clusters needed to implement a device type are mutually exclusive and those endpoints are related, then you can combine those device types onto a single endpoint and list all of the device types. The purpose in doing this is not as much to reduce the number of endpoints (which does help) but instead to make the grouping obvious in ganged devices.

For example: Light Bulb, Light Switch, Generic Switch and Electrical Power are all mutually exclusive clusters and are related together to form a smart switch. It would be possible to combine those four endpoints into a single endpoint and list all four device types. This is backwards compatible because the device types are mutually exclusive, if older code uses the Light Bulb device type it will just ignore the other clusters.

If this were an option, my keypads would have 10 endpoints instead of 42 - Root, Aggregator + 8 * (combo Bulb/Switch/GS/EP). The need for a special indicator LED endpoint disappears if I have the "NoLoad" flag on the OnOff cluster.

Two years ago I used this illustration in an issue about trying to build a pair of light switches which could control the top and bottom of a staircase. Despite two years of effort I have still not been able to achieve this. We are very close now, but still not quite there. This illustrates the use of "NoLoad". One switch in the pair has the load attached and the other one doesn't but you still want the indicator LED to work. However, you don't want two Light controls in the UI because there aren't two light bulbs.
grouped-dimmers

@markus-becker-tridonic-com
Copy link
Contributor

@jonsmirl For group communication with multiple senders per group, MCSP would likely be required, but it is not implemented in the SDK (#14853), and has been marked provisional in the spec (https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/data_model/Group-Key-Management-Cluster.adoc#4-features).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request feature work needs triage waiting for feedback Waiting for feedback from reporter/others about bug reproducing and status.
Projects
None yet
Development

No branches or pull requests

4 participants