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

[Feature Request] Matter "Strict" Mode #1150

Open
jvmahon opened this issue Sep 27, 2023 · 17 comments
Open

[Feature Request] Matter "Strict" Mode #1150

jvmahon opened this issue Sep 27, 2023 · 17 comments
Labels
enhancement New feature or request matter Important to Matter SDK zigbee pro Important to ZigbeePro stack

Comments

@jvmahon
Copy link

jvmahon commented Sep 27, 2023

I would love to see a Matter 1.1 and 1.2 "Strict" mode added. I believe it would have a tremendous benefit of reducing developer confusion and of creating devices that more reliably conform to the Matter standards.

I'm hoping there is some kind of a rule database / engine in ZAP that may make something like this possible.

Here's what I'm thinking (and I realize not all of this may be possible, but maybe some is) . . .

The current Zap implementation allows many matter clusters / attributes to be toggled on or off by a developer. It also allows the developer to select Client / Server mode for many of the cluster, and allows leaving attribute fields blanks even if they are mandatory (with no warning).

What I would like to see is a "Matter Strict" mode where any Mandatory provision of the Matter spec is strictly enforced without user choice. So, for example, if I select a Dimmer (0x104) endpoint, Zap should include the Identify cluster as both a Server and a Client, On/Off as a Client Only, Level as a Client Only, and should give no ability to change these mandatory items. ZAP should also show Groups and Scenes with an On/Off toggle but only allow Client. Similarly, the Zap output file should conform - currently, the ZAP output file often includes both client and server clusters with one having a "enable" setting of 0. If something isn't allowed, it would make more sense (and simplify the file) if it just wasn't there as a distraction.

Getting to the next stage, at the "Level" cluster, the FeatureMap should allow the 0, 1, and 2 bits to be set, and once they are set, the relevant "Mandatory" attributes appear. but can't be disabled without changing the FeatureMap. If a particular FeatureMap supports optional attributes, then only those options can be toggled.

Similarly, where possible, the attribute should be filled in. For example, for the root Endpoint, the PartsList should be able to automatically include each endpoint as required by the Standard (or at least a clear warning that the user has to fill it in).

Given Matter 1.2 is near release, maybe this starts with a "Matter 1.2 Strict" mode (with a future Matter 1.3 Strict, Matter 2.0 Strict, etc.). There could also be a "Matter Experimental Mode" where it is more of an "anything goes" with full freedom for the developer.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

@jvmahon the device types have features that are defined in the spec. Allowing the user to disable or enable features is contradictory to your request, no?

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

we have added some restrictions and are working on adding a "spec check" feature which tells the user what is required for the device type they have chosen

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

Here is how it works currently:

  • User opens ZAP
  • User creates an endpoint
  • User chooses a device type which is a super set of features

If the user disables anything required by the device type then it gets a warning

If we allow the user to enable features that are not defined for that device type then that would not be "strict", right?

If we want to have a fully strict interface it should probably be redesigned since in that case why would the user need to enable or disable anything at all? They would simply be choosing device types. No?

Let me know your thoughts. Thanks.

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

If we want to have a fully strict interface it should probably be redesigned since in that case why would the user need to enable or disable anything at all? They would simply be choosing device types. No?

I personally think a "Fully Strict" is needed and would cut down on confusion. I think doing thisis a bit more than just choosing a device type, as you also have to consider the attributes. Basically, the tool should "beginner proof" some of the work which I think would help get products to market must faster with better quality firmware

Here's some thoughts, and realizing maybe not all can be done at once, but continuous progress in this direction would help.

Clusters
As I read the Matter spec for a Devices, there are 3 issues when it comes to clusters:

  1. Is a cluster a "Server" or a "Client". If the specification says a cluster for a particular device type is to be a Server, it should never be possible to set it to Client, and vice-versa. If the device type says both Client and Server are needed, it should never be possible to have less than both. This is something that could be done just based on the device type.

  2. Mandatory and Optional. If a cluster is Mandatory, it should never be possible to delete it. On the other hand, Optional clusters should be added and removable.. Again, this is based on device type.

  3. If a cluster is not Mandatory or Optional, it should not be possible to include it for a device. Indeed, if you could include "other" clusters, it would take away all meaning of the word "Optional" (i.e., if anything can be added, then everything is Optional - rendering the "O" designation meaningless - that would not make sense). So the "optional" clusters must only be those that the specification says are Optional for a particular device type and nothing more.

Attributes:
Now, for Attributes, it gets a little more complex. For Attributes, you also need to consider the FeatureMap bitmap.

For example, if you were working with the Color Control Cluster and you set FeatureMap to 0x10001, then the attributes for Hue and Color Temperature should be enabled (mandatory), but the attributes for CurrentX, CurrentY, Enhanced Current Hue, ColorLoop, etc. should all not be available.

And Eliminate Non-Functional Data Entry Fields ...
At present, there are many fields in the Zap interface where it looks like the user should enter something, but the user really can't (e.g., fields where there is a list or struct type entry, like PartsList). For new users, this is very confusing and lots of time is wasted trying to figure this out. If a field does not actually allow input because the tool just hasn't advanced enough yet, there should, instead, be a link or note that takes the place of the data entry field. For example, for a PartsList, it may say "See note #123.4" and the notes could point to an example or explanation. "Note 123.4: This data must be entered programmatically, not through the Zap interface. See code example provided in example device file XXX at YYY"

Basically, to the maximum extent possible, a "Strict" mode should take away a user's ability to make mistakes, either by forgetting something, or adding something that doesn't below.

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

we have added some restrictions and are working on adding a "spec check" feature which tells the user what is required for the device type they have chosen

I have not checked this out yet (I've put aside my Matter programming project for a bit while handling other work, but plan to get back to it in the new year) but thanks - this seems like a good start.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

@jvmahon Device Types abstract Attributes, Clusters, Commands etc.

I am confused how we can allow the user to enable or disable anything in strict mode considering the device type (which is in the spec too) will not be up to spec.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

For example, if a user disables an attribute that is required by the device type then the device type does not have the features that it requires and is not up to spec.

The devices in a network need to know what features the device has and the device type implies what features are enabled.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

If you disable a "required attribute" for a device type there is a small warning and we are working on making it much more apparent.

Thank you for raising this concern. And please continue to provide your thoughts.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

Basically, a new user could do the following:

-add a device type and not change anything (my argument is strict mode would just be enforcing this)

-play around and make mistakes (good way to learn and become an experienced user)

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

For example, if a user disables an attribute that is required by the device type then the device type does not have the features that it requires and is not up to spec.

I may not have been clear. With what I am proposing, a user could never disable a required cluster for a device type, and could never disable a required attribute. And could never add clusters that were not on the Mandatory or Optional list and could *never" add attributes not either Mandatory or Optional.

For example, consider the On/Off Plug-in Unit (ID 0x010A), it has cluster requirements:
image

Here, Zap should enforce having the Identify, Groups, Scenes, and On/Off clusters and should only let them be "Server". Zap would then allow the user to choose of LevelControl was included (since that is Optional), but it would still require that it be Server.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

That is a good idea and we should make a list of things to change.

My concern about "Optional" and "Mandatory" is this:

Is it optional for the device type?

My understanding is the device type abstracts all these ideas and whatever is defined in the device type is actually mandatory for that device type.

For example,

A cluster with 1 mandatory feature and 2 optional features means that if that cluster is enabled then the mandatory feature must also be enabled

But for device types if that device type contains the above mentioned cluster and the device type is defined to have the mandatory feature and 1 of the optional features, then that optional feature for the cluster is actually mandatory for the device type.

Is that an incorrect interpretation of the idea of device types?

@jvmahon again, thank you for the discussion

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

Now look at cluster / attribute level.
Say you were doing an Extended Color Light (0x010D), that device has a "Color Control" cluster (0x0300).

Looking now just at the Color Control Cluster, it has a bunch of features set by the FeatureMap. :
image

image

The acceptable attributes for the Color Cluster are many and confusing ...

image

IDeally, what Zap should do is first require a developer to input the "FeatureMap" for the Color Control cluster. If, for example, FeatureMap was sent to 0x00001 - meaning only the "HS" feature was enabled, then the Zap should require as mandatory the two attributes with a "HS" Conformance" and the 4 attributes with the "M" allow the developer to optionally select 0x0002, 0x0005, 0x0006. None of the other attributes are valid for that FeatureMap so they should all be unavailable / hidden. This would greatly simplify what a developer sees.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

That is a good idea and we should make a list of things to change.

My concern about "Optional" and "Mandatory" is this:

Is it optional for the device type?

My understanding is the device type abstracts all these ideas and whatever is defined in the device type is actually mandatory for that device type.

For example,

A cluster with 1 mandatory feature and 2 optional features means that if that cluster is enabled then the mandatory feature must also be enabled

But for device types if that device type contains the above mentioned cluster and the device type is defined to have the mandatory feature and 1 of the optional features, then that optional feature for the cluster is actually mandatory for the device type.

Is that an incorrect interpretation of the idea of device types?

@jvmahon again, thank you for the discussion

I have to sign off. Will be back tomorrow. But I am very interested in what you think of this message.

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

-play around and make mistakes (good way to learn and become an experienced user)

My point is to remove the ability to make mistakes in "strict" mode. A developer should learn from the Matter specification instead, or switch out of "strict" mode. As an end-user / purchaser, I'd like the creation tools like Zap to enforce "perfect" implementation (as much as that can be done by a tool!) so end users who buy products aren't subject to developers who didn't quite get all the nuances right.

That's my view anyway.

@jvmahon
Copy link
Author

jvmahon commented Dec 6, 2023

My understanding is the device type abstracts all these ideas and whatever is defined in the device type is actually mandatory for that device type.

Except some device types have "optional" so you have to consider that for some device types, developers can make choices about specific clusters. And there are many "options" for the attributed based on the FeatureMap bit settings. Frankly, I think Matter, like Zigbee, gives too many options making implementations harder, but that's an already done decision.

And then Clusters can be "server" or "client" - for most device types, and for most clusters, the correct setting is "server" , but for some reason, Zap allows the developer to change to "client" even if it is wrong for a particular type. No reason for that to happen.

PS - I haven't looked at Zap in the last month or two, so if any of this has been fixed, comments may be out of date!

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

I completely agree with you.

I am just trying to understand device types in relation to clusters.

It seems to me that whatever is defined for a device type is required for that device type. It doesn't matter if it's not required for the cluster. Meaning, even if it's optional for the cluster it's not optional for the device type.

-play around and make mistakes (good way to learn and become an experienced user)

My point is to remove the ability to make mistakes in "strict" mode. A developer should learn from the Matter specification instead, or switch out of "strict" mode. As an end-user / purchaser, I'd like the creation tools like Zap to enforce "perfect" implementation (as much as that can be done by a tool!) so end users who buy products aren't subject to developers who didn't quite get all the nuances right.

That's my view anyway.

@paulr34
Copy link
Collaborator

paulr34 commented Dec 6, 2023

@jvmahon no worries at all. I appreciate the discussion.

I do agree that if something is mandatory it should be immutable.

ZAP is fundamentally based on adding device types which is why im trying to understand.

Thank you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request matter Important to Matter SDK zigbee pro Important to ZigbeePro stack
Development

No branches or pull requests

3 participants