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

Silabs #375

Merged
merged 18 commits into from
Jan 21, 2022
Merged

Silabs #375

merged 18 commits into from
Jan 21, 2022

Conversation

tecimovic
Copy link
Collaborator

@tecimovic tecimovic commented Jan 19, 2022

@tecimovic
Copy link
Collaborator Author

For people mostly interested in hew "reportingPolicy" (@bzbarsky-apple , @mrjerryjohns ):

Attributes in the XML can now have following options:
reportingPolicy="prohibited"
reportingPolicy="optional"
reportingPolicy="suggested"
reportingPolicy="mandatory"

Prohibited and mandatory remove user option. (The UI is not yet following along, I'll add an issue to be fixed by the UI consultants).

For backwards compatibility:
reportable="true" is equivalent to reportingPolicy="suggested"
reportable="false" is equivalent to reportingPolicy="optional"

Where you had defaultReportable earlier, you can now specify defaultReportingPolicy and use one of the 4 options, so you don't have to type all of these into XML.

This is sorting out the data model and the schema. I did not touch the Matter XML, but I've added a simple example into Zap's unit test test1.xml file.

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

I only reviewed the reporting policy changes.

They are OK as far as they go, but:

  1. They do not affect the value returned from endpointTypeAttribute queries in any way.
  2. There is, as far as I can tell, no way to get to the attribute-defined policy from the endpointTypeAttribute query.

so none of this really solves things for Matter on its own. We can specify the reporting policy all we want, and it will just be ignored in the data we actually have to work with.

Could we expose the policy in the queries that use endpointTypeAttribute for mapping?

@tecimovic
Copy link
Collaborator Author

@bzbarsky-apple : i see... You have data that already circumvented this policy and you need to deal with this.
This is essentially a data upgrading problem, similar to what happens if spec version 2 declared some attributes as "mandatory reporting", where earlier they were "optional" and users turned them off in the ZAP file.

I can do this:

  • when reading user configuration, if the configuration conflicts with the static attribute setting, UI will show a warning or something. We could also provide a "data upgrade" option with "spec force" which would cause the data in the ZAP file to be properly "fixed" by forcing it to comply with the spec.

Would that work?

It's also an entry towards this much larger process that we will have to do with at some point:
how to upgrate zap files made with older spec, to the latest spec....

@bzbarsky-apple
Copy link
Contributor

Fundamentally, we don't want to be getting this information from the zap files at all, for Matter. We want to be getting it directly from the XML. But the queries we have to work with don't seem to expose it?

@tecimovic
Copy link
Collaborator Author

@bzbarsky-apple :
I have added the logic, which will peg the boolean in the zap file to the value from XML upon loading the file, if the policy is "mandatory" or "prohibited".

You are correct, that in case of these policies, we shouldn't even have a boolean in the ZAP file. We'll address this as part of the UI fix to properly make UI follow this.

@tecimovic tecimovic merged commit 1f8065a into project-chip:master Jan 21, 2022
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

Successfully merging this pull request may close these issues.

For Matter, reportability is entirely spec-defined, not user-controlled
2 participants