-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Generate Conformance Data with Alchemy #36186
Conversation
Review changes with SemanticDiff. |
PR #36186: Size comparison from 9ee0499 to 0d5943c Full report (3 builds for cc32xx, stm32)
|
0d5943c
to
16f0467
Compare
16f0467
to
f88303a
Compare
f88303a
to
a436efc
Compare
a436efc
to
2d5612b
Compare
PR #36186: Size comparison from 0b93b0d to 2d5612b Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
<attribute code="0x0005" side="server" define="COMMISSIONING_ARL" type="array" entryType="CommissioningAccessRestrictionEntryStruct" optional="true">CommissioningARL</attribute> | ||
<attribute code="0x0006" side="server" define="ARL" type="array" entryType="AccessRestrictionEntryStruct" optional="true">ARL</attribute> | ||
|
||
<attribute side="server" code="0x0002" define="SUBJECTS_PER_ACCESS_CONTROL_ENTRY" type="int16u" min="4" default="4"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised this on Slack, but this merged before I had a chance to comment on the PR @jmartinez-silabs @andy31415 @ethanzhouyc :
This attribute syntax is because it is really hard to tell where the attribute name should go. But now we forced it on all attributes, but adding lots of <mandatoryConform/>
everywhere instead of just assuming M conformance if none is listed. Ideally we would just not have the noise, but if we really have to, we need to make the syntax make some sort of sense: the attribute name is not a "description".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rolled back in #36220
This reverts commit 334c669.
…36186)" (project-chip#36220)" This reverts commit a4a911c.
…36186)" (project-chip#36220)" This reverts commit a4a911c.
…36186)" (project-chip#36220)" This reverts commit a4a911c.
* generate conformance with Alchemy * add conform to color-control cluster extension
…ribute Description (project-chip#36233) * Revert "Revert "Generate Conformance Data with Alchemy (project-chip#36186)" (project-chip#36220)" This reverts commit a4a911c. * move attribute name - move attribute name from description to name inside attribute tag
Description:
This PR adds conformance data to attributes, commands, and events in ZAP XMLs using automated generation from Alchemy.
Generation Method:
Since Alchemy does not provide a command to directly generate conformance, the changes in this PR were produced by generating the diff between running Alchemy commands with and without the conformance flag. As a result, some unrelated changes are also included: Alchemy removes attributes with default values inside
<attribute>
tags. They are redundant and will not affect XML parsing. For example:optional="false"
andmin="0"
(for unsigned int types) are omitted.Validation and Discoveries:
To validate the generation result, Paul and I spent an entire day on reviewing and comparing all conformance-related updates against the spec element by element to ensure accuracy. Here are key findings:
color-control-cluster.xml
was not generated by Alchemy and has been manually added based on the spec.mandatoryConform
in XML.desc
in the spec, no conformance will be added to the XML. When combined with other conformance types, for example,P, desc
, it translates to aprovisionalConform
insideotherwiseConform
, anddesc
translates to nothing. It is wrong and should just beprovisionalConform