Generate XML using Alchemy tool for closure cluster and device type #37371
Conversation
|
Review changes with Changed Files
|
jmartinez-silabs
changed the title
| <cluster code="0x0104"/> | ||
| <cluster code="0x0106"/> |
There was a problem hiding this comment.
In the spec.... you can't just randomly reference a data type from cluster X in cluster Y. I mean, you can write that, and Alchemy seems to "do something" with it as here, but it's broken, because data type names are not unique; so they are always scoped to the cluster they are defined in, except for global data types, which do have unique names.
| <cluster code="0x0104"/> | ||
| <cluster code="0x0106"/> |
There was a problem hiding this comment.
Why does this not apply to all the cluster IDs for Closure Dimension?
| <attribute code="0x0000" side="server" define="COUNTDOWN_TIME" type="elapsed_s" isNullable="true" max="259200" optional="true"> | ||
| <description>CountdownTime</description> | ||
| <optionalConform/> |
There was a problem hiding this comment.
Maybe this is an alchemy question, but why do we have both optional="true" and optionalConform here? @hasty
| <item name="AtPedestrian" value="0x04"/> | ||
| </enum> | ||
|
|
||
| <struct name="OverallStateStruct" apiMaturity="provisional"> |
There was a problem hiding this comment.
Shouldn't the enums also be tagged with apiMaturity="provisional"?
| <item fieldId="2" name="Speed" type="ThreeLevelAutoEnum" optional="true" min="0x00" max="0x03"/> | ||
| </struct> | ||
|
|
||
| <cluster> |
There was a problem hiding this comment.
Shouldn't this have apiMaturity="provisional"?
| <cluster> | ||
| <domain>Closures</domain> | ||
| <name>Closure 1st Dimension</name> | ||
| <code>0x0105</code> |
There was a problem hiding this comment.
I do wonder how the order of these clusters was determined; it's not ID order and it's not name order... it's a little odd. @hasty is this something Alchemy chose somehow?
| <include cluster="Proxy Discovery" client="false" server="true" clientLocked="true" serverLocked="true"/> | ||
| <include cluster="Proxy Valid" client="false" server="true" clientLocked="true" serverLocked="true"/> | ||
| <include cluster="Pulse Width Modulation" client="false" server="true" clientLocked="true" serverLocked="true"/> | ||
| <include cluster="Proxy Configuration" client="false" server="true" clientLocked="true" serverLocked="true"></include> |
There was a problem hiding this comment.
Why all these irrelevant changes to the empty tag syntax? They make it very hard to review what changes are real here and what changes are just noise.
If this needs to happen, it should happen in a separate PR with no actual changes to the XML semantics in it.
| <clusters> | ||
| <include cluster="Groups" client="false" server="false" clientLocked="true" serverLocked="false"></include> | ||
| <include cluster="Identify" client="false" server="false" clientLocked="false" serverLocked="false"></include> | ||
| <include cluster="Scenes Management" client="false" server="false" clientLocked="true" serverLocked="false"></include> |
There was a problem hiding this comment.
This is missing all the actual Closure clusters, no? Probably because the spec is broken and does not list the right cluster IDs here....
| <deviceType> | ||
| <name>MA-heatcool</name> | ||
| <domain>CHIP</domain> | ||
| <typeName>Heating/Cooling Unit</typeName> |
There was a problem hiding this comment.
Why is this being removed? Seems unrelated to this PR. If this is "general cleanup", it should happen in a separate PR before this one.
| @@ -150,9 +152,15 @@ | |||
| "manufacturersXml": "../../../../src/app/zap-templates/zcl/data-model/manufacturers.xml", | |||
| "options": { | |||
| "text": { | |||
| "defaultResponsePolicy": ["Always", "Conditional", "Never"] | |||
| "defaultResponsePolicy": [ | |||
There was a problem hiding this comment.
This change is making your PR fail CI, quite apart from making it harder to review.
Changelog
Testing
Manually verified the generated parameters with the specification