[Media] XML sync with spec #33524
Draft
[Media] XML sync with spec #33524
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[Media] ZAP template XML sync with spec
chrisdecenzo
approved these changes
May 21, 2024
lazarkov
reviewed
May 22, 2024
| @@ -23,6 +23,7 @@ limitations under the License. | |||
| <define>APPLICATION_BASIC_CLUSTER</define> | |||
| <client init="false" tick="false">true</client> | |||
| <server init="false" tick="false">true</server> | |||
| <globalAttribute code="0xFFFD" side="either" value="1"/> | |||
There was a problem hiding this comment.
What's the purpose of this global attribute?
There was a problem hiding this comment.
It's the cluster revision.
lazarkov
reviewed
May 22, 2024
| <arg id="1" name="EndTime" type="epoch_s"/> | ||
| <arg id="2" name="ChannelList" type="ChannelInfoStruct" array="true" optional="true" length="255"/> | ||
| <arg id="3" name="PageToken" type="PageTokenStruct" optional="true" isNullable="true"/> | ||
| <arg id="5" name="RecordingFlag" type="RecordingFlagBitmap" optional="true" isNullable="true" min="0x00" max="0x07"/> |
There was a problem hiding this comment.
is min, max, isNullable supported?
There was a problem hiding this comment.
IsNullable is definitely supported. I don't think the min/max does anything, but isn't hurting.
lazarkov
reviewed
May 22, 2024
| @@ -45,14 +45,15 @@ limitations under the License. | |||
| <arg name="Node" type="node_id" optional="true"/> | |||
| </command> | |||
|
|
|||
| <command source="server" code="0x01" name="GetSetupPINResponse" optional="false" disableDefaultResponse="true"> | |||
| <command source="server" code="0x01" name="GetSetupPINResponse" optional="false" disableDefaultResponse="true" isFabricScoped="true"> | |||
There was a problem hiding this comment.
isFabricScoped supproted now?
There was a problem hiding this comment.
Many other clusters are using it.
lazarkov
reviewed
May 22, 2024
| <item name="DisplayName" type="CHAR_STRING" isNullable="true" optional="true"/> | ||
| <item fieldId="0" name="LanguageCode" type="char_string" length="32"/> | ||
| <item fieldId="1" name="Characteristics" type="CharacteristicEnum" isNullable="true" optional="true" array="true"/> | ||
| <item fieldId="2" name="DisplayName" type="long_char_string" isNullable="true" optional="true" length="256"/> | ||
| <!-- <item name="Characteristics" type="CharacteristicEnum" optional="true" array="true"/> --> |
There was a problem hiding this comment.
Can this be removed now? I think it wasn't able to compile before
There was a problem hiding this comment.
If it's not compiling and you don't want it, we should remove it from the spec.
sharadb-amazon
approved these changes
May 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes many areas in which the media cluster ZAP template XML files are out of sync with the spec; it fixes many of the problems in #32669 but ignores min/max errors, as they are not enforced anyway.
These are the changes in commit debbc2e (especially concerning errors in bold):
Account Login:
Channel:
Content App Observer:
Content Control:
Content Launch:
Application Basic, Application Launcher, Content App Observer, Keypad Input, Low Power, and Media Input were all missing the revision number attribute, and Content Launch had the wrong revision number.
For some of these errors, it might be worth considering changing the spec instead; e.g. there are many string attributes and fields with a max length of 256, which requires them to be long_char_strings in the spec; were their max lengths 255, they could remain char_strings.