Skip to content

Commit f8a1093

Browse files
cecillegemini-code-assist[bot]AryaHassanli
authored
Spec parsing: Add revision history, update revision checker (project-chip#42755)
* Spec parsing: Add revision history, update revision checker Add revison parsing to the spec parsing data, updates the revision checker script to do a diff of the provisional status and add revision history. The revision checker had a lot of repeated info for provisional markings, and things were getting lost in the data. It's also hard to understand what happened with some clusters if there are revision changes without any corresponding visible data model changes so added those to the printouts. * Update src/python_testing/matter_testing_infrastructure/matter/testing/spec_parsing.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/python_testing/matter_testing_infrastructure/matter/testing/spec_parsing.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update scripts/spec_xml/spec_revision_diff_summary.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update scripts/spec_xml/spec_revision_diff_summary.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update scripts/spec_xml/spec_revision_diff_summary.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update scripts/spec_xml/spec_revision_diff_summary.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * More verbose string on device type clusters * linter + fix stray line break * linter * mypy * Add revision desc to unit test * Apply suggestions from code review Co-authored-by: Arya Hassanli <31996976+AryaHassanli@users.noreply.github.com> * review comments * Remove unused import --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Arya Hassanli <31996976+AryaHassanli@users.noreply.github.com>
1 parent f9a032e commit f8a1093

3 files changed

Lines changed: 126 additions & 37 deletions

File tree

‎scripts/spec_xml/spec_revision_diff_summary.py‎

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ def diff_clusters(prior_revision: PrebuiltDataModelDirectory, new_revision: Preb
7979
changes = []
8080
if old.revision != new.revision:
8181
changes.append(f'\tRevision change - old: {old.revision} new: {new.revision}')
82+
for r in range(old.revision+1, new.revision+1):
83+
try:
84+
changes.append(f'\t\t{r}: {new.revision_desc[r]}')
85+
except KeyError:
86+
changes.append(f'\t\t{r}: NOT PRESENT IN SPEC')
8287
changes.extend(str_element_changes('Features', old.features, new.features))
8388
changes.extend(str_element_changes('Attributes', old.attributes, new.attributes))
8489
changes.extend(str_element_changes('Accepted Commands', old.accepted_commands, new.accepted_commands))
@@ -110,6 +115,11 @@ def diff_device_types(prior_revision: PrebuiltDataModelDirectory, new_revision:
110115
changes = []
111116
if old.revision != new.revision:
112117
changes.append(f'\tRevision change - old: {old.revision} new: {new.revision}')
118+
for r in range(old.revision+1, new.revision+1):
119+
try:
120+
changes.append(f'\t\t{r}: {new.revision_desc[r]}')
121+
except KeyError:
122+
changes.append(f'\t\t{r}: NOT PRESENT IN SPEC')
113123
changes.extend(str_element_changes('Server Clusters', old.server_clusters, new.server_clusters))
114124
changes.extend(str_element_changes('Client Clusters', old.client_clusters, new.client_clusters))
115125

@@ -119,37 +129,64 @@ def diff_device_types(prior_revision: PrebuiltDataModelDirectory, new_revision:
119129

120130

121131
def _get_provisional(items):
122-
return [e.name for e in items if e.conformance(EMPTY_CLUSTER_GLOBAL_ATTRIBUTES).decision == ConformanceDecision.PROVISIONAL]
132+
return {e.name for e in items if e.conformance(EMPTY_CLUSTER_GLOBAL_ATTRIBUTES).decision == ConformanceDecision.PROVISIONAL}
123133

124134

125-
def get_all_provisional_clusters(new_revision: PrebuiltDataModelDirectory):
126-
clusters, _ = build_xml_clusters(new_revision)
135+
def get_provisional_diff(rev1: PrebuiltDataModelDirectory, rev2: PrebuiltDataModelDirectory):
136+
clusters_rev1, _ = build_xml_clusters(rev1)
137+
clusters_rev2, _ = build_xml_clusters(rev2)
127138

128-
provisional_clusters = [c.name for c in clusters.values() if c.is_provisional]
129-
print('\n\nProvisional Clusters')
130-
print(f'\t{sorted(provisional_clusters)}')
139+
provisional_clusters_rev1 = [c.name for c in clusters_rev1.values() if c.is_provisional]
140+
provisional_clusters_rev2 = [c.name for c in clusters_rev2.values() if c.is_provisional]
131141

132-
for c in clusters.values():
133-
features = _get_provisional(c.features.values())
134-
attributes = _get_provisional(c.attributes.values())
135-
accepted_commands = _get_provisional(c.accepted_commands.values())
136-
generated_commands = _get_provisional(c.generated_commands.values())
137-
events = _get_provisional(c.events.values())
142+
rev2_additional_provisional_clusters = set(provisional_clusters_rev2) - set(provisional_clusters_rev1)
143+
print(f'\n\nProvisional clusters in {rev2.dirname} not in {rev1.dirname}')
144+
print(f'\t{sorted(rev2_additional_provisional_clusters)}')
145+
146+
for id, c2 in clusters_rev2.items():
147+
if id not in clusters_rev1:
148+
continue
149+
c1 = clusters_rev1[id]
150+
rev2_provisional_features = _get_provisional(c2.features.values())
151+
rev1_provisional_features = _get_provisional(c1.features.values())
152+
features = rev2_provisional_features - rev1_provisional_features
153+
154+
rev2_provisional_attributes = _get_provisional(c2.attributes.values())
155+
rev1_provisional_attributes = _get_provisional(c1.attributes.values())
156+
attributes = rev2_provisional_attributes - rev1_provisional_attributes
157+
158+
rev2_provisional_accepted_commands = _get_provisional(c2.accepted_commands.values())
159+
rev1_provisional_accepted_commands = _get_provisional(c1.accepted_commands.values())
160+
accepted_commands = rev2_provisional_accepted_commands - rev1_provisional_accepted_commands
161+
162+
rev2_provisional_generated_commands = _get_provisional(c2.generated_commands.values())
163+
rev1_provisional_generated_commands = _get_provisional(c1.generated_commands.values())
164+
generated_commands = rev2_provisional_generated_commands - rev1_provisional_generated_commands
165+
166+
rev2_provisional_events = _get_provisional(c2.events.values())
167+
rev1_provisional_events = _get_provisional(c1.events.values())
168+
events = rev2_provisional_events - rev1_provisional_events
138169

139170
if not features and not attributes and not accepted_commands and not generated_commands and not events:
140171
continue
141172

142-
print(f'\n{c.name}')
173+
print(f'\n{c2.name}')
174+
print(f'Provisional elements in {rev2.dirname} that are not provisional in {rev1.dirname}')
143175
if features:
144-
print(f'\tProvisional features: {features}')
176+
print(f'\tFeatures: {features}')
145177
if attributes:
146-
print(f'\tProvisional attributes: {attributes}')
178+
print(f'\tAttributes: {attributes}')
147179
if accepted_commands:
148-
print(f'\tProvisional accepted commands: {accepted_commands}')
180+
print(f'\tAccepted commands: {accepted_commands}')
149181
if generated_commands:
150-
print(f'\tProvisional generated commands: {generated_commands}')
182+
print(f'\tGenerated commands: {generated_commands}')
151183
if events:
152-
print(f'\tProvisional events: {events}')
184+
print(f'\tEvents: {events}')
185+
186+
187+
def get_all_provisional_clusters(prior_revision: PrebuiltDataModelDirectory, new_revision: PrebuiltDataModelDirectory):
188+
get_provisional_diff(prior_revision, new_revision)
189+
get_provisional_diff(new_revision, prior_revision)
153190

154191

155192
def get_all_provisional_device_types(new_revision: PrebuiltDataModelDirectory):
@@ -172,7 +209,8 @@ def get_all_provisional_device_types(new_revision: PrebuiltDataModelDirectory):
172209
'1.4': PrebuiltDataModelDirectory.k1_4,
173210
'1.4.1': PrebuiltDataModelDirectory.k1_4_1,
174211
'1.4.2': PrebuiltDataModelDirectory.k1_4_2,
175-
'1.5': PrebuiltDataModelDirectory.k1_5}
212+
'1.5': PrebuiltDataModelDirectory.k1_5,
213+
'1.5.1': PrebuiltDataModelDirectory.k1_5_1}
176214

177215

178216
@click.command()
@@ -181,7 +219,8 @@ def get_all_provisional_device_types(new_revision: PrebuiltDataModelDirectory):
181219
def main(prior_revision: str, new_revision: str):
182220
diff_clusters(REVISIONS[prior_revision], REVISIONS[new_revision])
183221
diff_device_types(REVISIONS[prior_revision], REVISIONS[new_revision])
184-
get_all_provisional_clusters(REVISIONS[new_revision])
222+
print('\n\n---------------Provisional checks----------------')
223+
get_all_provisional_clusters(REVISIONS[prior_revision], REVISIONS[new_revision])
185224
get_all_provisional_device_types(REVISIONS[new_revision])
186225

187226

‎src/python_testing/matter_testing_infrastructure/matter/testing/spec_parsing.py‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import importlib
1919
import importlib.resources as pkg_resources
2020
import logging
21+
import math
2122
import os
2223
import re
2324
import typing
@@ -27,7 +28,7 @@
2728
from dataclasses import dataclass, field
2829
from enum import Enum, StrEnum, auto
2930
from importlib.abc import Traversable
30-
from typing import Callable, Optional, Union
31+
from typing import Optional, Union
3132

3233
import matter.clusters as Clusters
3334
import matter.testing.conformance as conformance_support
@@ -177,6 +178,7 @@ class XmlCluster:
177178
bitmaps: dict[str, XmlDataType]
178179
pics: str
179180
is_provisional: bool
181+
revision_desc: dict[int, str]
180182

181183

182184
class ClusterSide(Enum):
@@ -190,12 +192,33 @@ class XmlDeviceTypeClusterRequirements:
190192
side: ClusterSide
191193
conformance: ConformanceCallable
192194
# Feature mask (1 << feature_id) to conformance
193-
feature_overrides: dict[uint, Callable] = field(default_factory=dict)
194-
attribute_overrides: dict[uint, Callable] = field(default_factory=dict)
195-
command_overrides: dict[uint, Callable] = field(default_factory=dict)
195+
feature_overrides: dict[uint, ConformanceCallable] = field(default_factory=dict)
196+
attribute_overrides: dict[uint, ConformanceCallable] = field(default_factory=dict)
197+
command_overrides: dict[uint, ConformanceCallable] = field(default_factory=dict)
198+
199+
def str_overrides(self):
200+
ret = ""
201+
if self.feature_overrides:
202+
overrides = ""
203+
for mask, conformance in self.feature_overrides.items():
204+
bit = math.log2(mask)
205+
overrides += f'bit {int(bit)}: {str(conformance)} '
206+
ret += f'-- Feature overrides: {overrides}'
207+
if self.attribute_overrides:
208+
overrides = ""
209+
for id, conformance in self.attribute_overrides.items():
210+
overrides += f'{id:04X}: {str(conformance)} '
211+
ret += f'-- Attribute overrides: {overrides}'
212+
if self.command_overrides:
213+
overrides = ""
214+
for id, conformance in self.command_overrides.items():
215+
overrides += f'{id:04X}: {str(conformance)} '
216+
ret += f'-- Command overrides: {overrides}'
217+
218+
return ret
196219

197220
def __str__(self):
198-
return f'{self.name}: {str(self.conformance)}'
221+
return f'{self.name} {self.side}: {str(self.conformance)} {self.str_overrides()}'
199222

200223

201224
@dataclass
@@ -232,6 +255,7 @@ class XmlDeviceType:
232255
# Keeping these as strings for now because the exact definitions are being discussed in DMTT
233256
classification_class: str
234257
classification_scope: str
258+
revision_desc: dict[int, str]
235259
superset_of_device_type_name: Optional[str] = None
236260
superset_of_device_type_id: int = 0
237261

@@ -294,6 +318,10 @@ def get_location_from_element(element: ElementTree.Element, cluster_id: Optional
294318
return AttributePathLocation(endpoint_id=0, cluster_id=cluster_id, attribute_id=int(element.attrib['id'], 0))
295319
if element.tag == 'event':
296320
return EventPathLocation(endpoint_id=0, cluster_id=cluster_id, event_id=int(element.attrib['id'], 0))
321+
if element.tag == 'cluster':
322+
return ClusterPathLocation(endpoint_id=0, cluster_id=int(element.attrib['id'], 0))
323+
if element.tag == 'deviceType':
324+
return DeviceTypePathLocation(endpoint_id=0, device_type_id=int(element.attrib['id'], 0))
297325
return cluster_location
298326
except (KeyError, ValueError):
299327
# If we can't find the id or can't parse it
@@ -314,6 +342,22 @@ def get_conformance(element: ElementTree.Element, cluster_id: Optional[uint]) ->
314342
XmlElementDescriptor = tuple[ElementTree.Element, ElementTree.Element, Optional[ElementTree.Element]]
315343

316344

345+
def parse_revision_history(top_level: ElementTree.Element) -> tuple[dict[int, str], list[ProblemNotice]]:
346+
revision_desc = {}
347+
problems = []
348+
history = top_level.find('revisionHistory')
349+
if history:
350+
for e in history.iter('revision'):
351+
try:
352+
rev = int(e.get('revision', 'error'), 0)
353+
revision_desc[rev] = e.get('summary', '')
354+
except ValueError:
355+
problems.append(ProblemNotice(test_name='Spec XML parsing', location=get_location_from_element(top_level, None),
356+
severity=ProblemSeverity.WARNING,
357+
problem='Revision in revision history is missing or is not an int'))
358+
return revision_desc, problems
359+
360+
317361
class ClusterParser:
318362
# Cluster ID is optional to support base clusters that have no ID of their own.
319363
def __init__(self, cluster: ElementTree.Element, cluster_id: Optional[uint], name: str):
@@ -354,6 +398,9 @@ def __init__(self, cluster: ElementTree.Element, cluster_id: Optional[uint], nam
354398
self.params = ConformanceParseParameters(feature_map=self.create_feature_map(), attribute_map=self.create_attribute_map(),
355399
command_map=self.create_command_map())
356400

401+
self._revision_desc, problems = parse_revision_history(cluster)
402+
self._problems.extend(problems)
403+
357404
def get_conformance(self, element: ElementTree.Element) -> ElementTree.Element:
358405
element, problem = get_conformance(element, self._cluster_id)
359406
if problem:
@@ -833,7 +880,8 @@ def create_cluster(self) -> XmlCluster:
833880
structs=self._parse_data_type(DataTypeEnum.kStruct),
834881
enums=self._parse_data_type(DataTypeEnum.kEnum),
835882
bitmaps=self._parse_data_type(DataTypeEnum.kBitmap),
836-
pics=self._pics if self._pics is not None else "", is_provisional=self._is_provisional)
883+
pics=self._pics if self._pics is not None else "", is_provisional=self._is_provisional,
884+
revision_desc=self._revision_desc)
837885

838886
def get_problems(self) -> list[ProblemNotice]:
839887
return self._problems
@@ -1109,8 +1157,7 @@ def combine_attributes(base: dict[uint, XmlAttribute], derived: dict[uint, XmlAt
11091157
ret[id].write_access = override.write_access
11101158

11111159
for attr_id, attribute in ret.items():
1112-
if attribute.read_access == ACCESS_CONTROL_PRIVILEGE_ENUM.kUnknownEnumValue and \
1113-
attribute.write_access == ACCESS_CONTROL_PRIVILEGE_ENUM.kUnknownEnumValue:
1160+
if attribute.read_access == ACCESS_CONTROL_PRIVILEGE_ENUM.kUnknownEnumValue and attribute.write_access == ACCESS_CONTROL_PRIVILEGE_ENUM.kUnknownEnumValue:
11141161
location = AttributePathLocation(endpoint_id=0, cluster_id=cluster_id, attribute_id=attr_id)
11151162
problems.append(ProblemNotice(test_name='Spec XML parsing', location=location,
11161163
severity=ProblemSeverity.WARNING, problem=f'Attribute {attribute.name} (ID: {attr_id}) in cluster {cluster_id} has unknown read and write access after combining base and derived values.'))
@@ -1156,12 +1203,13 @@ def combine_attributes(base: dict[uint, XmlAttribute], derived: dict[uint, XmlAt
11561203
else:
11571204
unknown_commands.append(cmd)
11581205
provisional = c.is_provisional or base.is_provisional
1206+
revision_desc = c.revision_desc
11591207

11601208
new = XmlCluster(revision=c.revision, derived=c.derived, name=c.name,
11611209
feature_map=feature_map, attribute_map=attribute_map, command_map=command_map,
11621210
features=features, attributes=attributes, accepted_commands=accepted_commands,
11631211
generated_commands=generated_commands, unknown_commands=unknown_commands, events=events, structs=structs,
1164-
enums=enums, bitmaps=bitmaps, pics=c.pics, is_provisional=provisional)
1212+
enums=enums, bitmaps=bitmaps, pics=c.pics, is_provisional=provisional, revision_desc=revision_desc)
11651213
xml_clusters[id] = new
11661214

11671215

@@ -1330,7 +1378,8 @@ def parse_single_device_type(root: ElementTree.Element, cluster_definition_xml:
13301378
if id in DEVICE_TYPE_NAME_FIXES:
13311379
device_name = DEVICE_TYPE_NAME_FIXES[id]
13321380

1333-
location = DeviceTypePathLocation(device_type_id=id)
1381+
revision_desc, rev_problems = parse_revision_history(d)
1382+
problems.extend(rev_problems)
13341383

13351384
try:
13361385
classification = next(d.iter('classification'))
@@ -1349,7 +1398,8 @@ def parse_single_device_type(root: ElementTree.Element, cluster_definition_xml:
13491398
severity=ProblemSeverity.WARNING, problem="Unable to find classification data for device type"))
13501399
return device_types, problems
13511400
device_types[id] = XmlDeviceType(name=device_name, revision=revision, server_clusters={}, client_clusters={},
1352-
classification_class=device_class, classification_scope=scope, superset_of_device_type_name=superset_of_device_type_name)
1401+
classification_class=device_class, revision_desc=revision_desc,
1402+
classification_scope=scope, superset_of_device_type_name=superset_of_device_type_name)
13531403
try:
13541404
main_endpoint_clusters = next(d.iter('clusters'))
13551405
clusters = main_endpoint_clusters.findall('cluster')

‎src/python_testing/test_testing/TestSpecParsingDeviceType.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -577,17 +577,17 @@ def _build_superset_tree(self) -> dict[int, XmlDeviceType]:
577577
# 4 -> 3
578578
# 5 - all alone
579579
# 6 - utility endpoint
580-
one = XmlDeviceType('one', 1, [], [], 'simple', 'endpoint',
580+
one = XmlDeviceType('one', 1, [], [], 'simple', 'endpoint', {1: ""},
581581
superset_of_device_type_name='two', superset_of_device_type_id=2)
582-
two = XmlDeviceType('two', 1, [], [], 'simple', 'endpoint',
582+
two = XmlDeviceType('two', 1, [], [], 'simple', 'endpoint', {1: ""},
583583
superset_of_device_type_name='three', superset_of_device_type_id=3)
584-
three = XmlDeviceType('three', 1, [], [], 'simple', 'endpoint',
584+
three = XmlDeviceType('three', 1, [], [], 'simple', 'endpoint', {1: ""},
585585
superset_of_device_type_name=None, superset_of_device_type_id=0)
586-
four = XmlDeviceType('four', 1, [], [], 'simple', 'endpoint',
586+
four = XmlDeviceType('four', 1, [], [], 'simple', 'endpoint', {1: ""},
587587
superset_of_device_type_name='three', superset_of_device_type_id=3)
588-
five = XmlDeviceType('five', 1, [], [], 'simple', 'endpoint',
588+
five = XmlDeviceType('five', 1, [], [], 'simple', 'endpoint', {1: ""},
589589
superset_of_device_type_name=None, superset_of_device_type_id=0)
590-
six = XmlDeviceType('six', 1, [], [], 'utility', 'endpoint',
590+
six = XmlDeviceType('six', 1, [], [], 'utility', 'endpoint', {1: ""},
591591
superset_of_device_type_name=None, superset_of_device_type_id=0)
592592
return {1: one, 2: two, 3: three, 4: four, 5: five, 6: six}
593593

0 commit comments

Comments
 (0)