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

Monitor and report on validity of configuration #1183

Open
tecimovic opened this issue Oct 26, 2023 · 4 comments
Open

Monitor and report on validity of configuration #1183

tecimovic opened this issue Oct 26, 2023 · 4 comments
Assignees

Comments

@tecimovic
Copy link
Collaborator

tecimovic commented Oct 26, 2023

Problem:
users are not properly informed if their configuration is "according to spec" and will "pass certification".

Background:
based on experience with previous "spec and certification" systems, zap is developed with the following UI principle in mind:
- allow users to modify configuration however they want and whenever they want.
- but when data becomes "outside of spec" and "not certificable", then show them warnings/messages/errors/something.

So we need mechanisms that covers following use cases:
1.) You load in a .zap file. .zap file is "not according to spec". As soon as the file loads, the message shows describing this, and the elements that are causing the problem (clusters that should be there, missing clusters that are not there, etc, etc) are highlighted in UI.
2.) If user deselects a cluster/attribute/command/event that is considered MANDATORY in order to pass certification, the deselection SHOULD SUCCEED, however zap should immediatelly highlight this row as a "warning" and show some message describing what just happend. Something like: "This attribute is considered mandatory for device type DoorLock".
3.) If user loads in custom XML, which imposes additional "spec" constraints, zap should immediatelly after the load recalculate the state of "up to spec-ness", and pop up appropriate warnings/errors/message if they are required.
4.) User should have a place in UI where they can view ALL the messages related to "not being up to spec", and can go over them one by one and fix them.
5.) When generating from command line, there should be a clear warning shown if something is not up to spec.
6.) We should have a CLI switch "--allowSpecIssues". If this is present, then generation will happen, even though things are not up to spec. If it's not, then zap will simply blow up with a clear error and refuse to generate.

And, very obviously:
this functionality is "spec specific", so thoughts should be given to how this should work on Matter and how it should work on Zigbee. If we need to modify schema to add more data, we should. And things that SHOULD BE driven by the SDK, need to come from SDK. We don't want the code that says: "If matter then X else Y". All that should be driven from the zcl.json and it's child files.

@jepenven-silabs
Copy link
Contributor

Great issue, some work was already done in the https://github.com/project-chip/connectedhomeip/blob/master/scripts/rules.matterlint but if zap could validate everything that would be great.

@paulr34
Copy link
Collaborator

paulr34 commented Oct 26, 2023

@tecimovic so if the user changes anything that came from the XML we should pop a notification as suggested?

@tecimovic
Copy link
Collaborator Author

It's not that simple, @paulr34 : what is "according to spec" is more complicated. Sure the XML actually holds all the data, but the rules can be complex. There are definitely "global spec rules" such as which clusters are mandatory per device and which are not.

But then there are also SDK specific rules and implementation rules, such as some of these "storage" issues and such. These might result in devices not getting certified for implementation issues.

So we have to determine all of these specific cases that we know of now, have a good framework to be able to add cases that will arise in the future, and then build all this.

@paulr34
Copy link
Collaborator

paulr34 commented Oct 26, 2023

Thank you @tecimovic that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants