Skip to content

Commit

Permalink
Add global types (structs/enums/bitmaps) to matter file parsing (#34515)
Browse files Browse the repository at this point in the history
* Make the IDL parser parse global types

* Restyle

* Parsing ok, started adding compatibility unit tests (which fail)

* backwards compatibility tests pass now

* Start implementing global type merging. Still needs unit tests

* Prepare matter tests

* A first test for global parsing

* Fix bugs and have a working unit test for the non-recursive bit

* Recursive unit test passes

* Fix lint

* Fix lint

* Add check for global type uniqueness

* Restyle

---------

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Dec 10, 2024
1 parent a789a43 commit 2023745
Show file tree
Hide file tree
Showing 8 changed files with 502 additions and 41 deletions.
6 changes: 6 additions & 0 deletions scripts/py_matter_idl/matter_idl/backwards_compatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,12 @@ def check(self):

self._check_cluster_list_compatible(
self._original_idl.clusters, self._updated_idl.clusters)
self._check_enum_list_compatible(
"", self._original_idl.global_enums, self._updated_idl.global_enums)
self._check_bitmap_list_compatible(
"", self._original_idl.global_bitmaps, self._updated_idl.global_bitmaps)
self._check_struct_list_compatible(
"", self._original_idl.global_structs, self._updated_idl.global_structs)

return self.compatible

Expand Down
60 changes: 56 additions & 4 deletions scripts/py_matter_idl/matter_idl/generators/idl/MatterIdl.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,65 @@

// This IDL was auto-generated from a parsed data structure

{% for cluster in idl.clusters %}
{% for enum in idl.global_enums %}
enum {{enum.name}} : {{ enum.base_type}} {
{% for entry in enum.entries %}
{{entry.name}} = {{entry.code}};
{% endfor %}
}

{% endfor %}

{%- for bitmap in idl.global_bitmaps %}
bitmap {{bitmap.name}} : {{ bitmap.base_type}} {
{% for entry in bitmap.entries %}
{{entry.name}} = 0x{{"%X" | format(entry.code)}};
{% endfor %}
}

{% endfor %}

{%- for s in idl.global_structs %}
{{render_struct(s)}}

{% endfor %}

{%- for cluster in idl.clusters %}
{% if cluster.description %}/** {{cluster.description}} */
{% endif %}
{{cluster.api_maturity | idltxt}}cluster {{cluster.name}} = {{cluster.code}} {
revision {{cluster.revision}};

{% for enum in cluster.enums %}
{% for enum in cluster.enums | selectattr("is_global")%}
/* GLOBAL:
enum {{enum.name}} : {{ enum.base_type}} {
{% for entry in enum.entries %}
{{entry.name}} = {{entry.code}};
{% endfor %}
}
*/

{% endfor %}

{%- for bitmap in cluster.bitmaps | selectattr("is_global")%}
/* GLOBAL:
bitmap {{bitmap.name}} : {{ bitmap.base_type}} {
{% for entry in bitmap.entries %}
{{entry.name}} = 0x{{"%X" | format(entry.code)}};
{% endfor %}
}
*/

{% endfor %}

{%- for s in cluster.structs | selectattr("is_global") %}
/* GLOBAL:
{{render_struct(s)}}
*/

{% endfor %}

{%- for enum in cluster.enums | rejectattr("is_global")%}
enum {{enum.name}} : {{ enum.base_type}} {
{% for entry in enum.entries %}
{{entry.name}} = {{entry.code}};
Expand All @@ -47,7 +99,7 @@

{% endfor %}

{%- for bitmap in cluster.bitmaps %}
{%- for bitmap in cluster.bitmaps | rejectattr("is_global")%}
bitmap {{bitmap.name}} : {{ bitmap.base_type}} {
{% for entry in bitmap.entries %}
{{entry.name}} = 0x{{"%X" | format(entry.code)}};
Expand All @@ -56,7 +108,7 @@

{% endfor %}

{%- for s in cluster.structs | rejectattr("tag") %}
{%- for s in cluster.structs | rejectattr("tag") | rejectattr("is_global") %}
{{render_struct(s)}}

{% endfor %}
Expand Down
2 changes: 1 addition & 1 deletion scripts/py_matter_idl/matter_idl/matter_grammar.lark
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ POSITIVE_INTEGER: /\d+/
HEX_INTEGER: /0x[A-Fa-f0-9]+/
ID: /[a-zA-Z_][a-zA-Z0-9_]*/

idl: (cluster|endpoint)*
idl: (struct|enum|bitmap|cluster|endpoint)*

%import common.ESCAPED_STRING
%import common.WS
Expand Down
118 changes: 113 additions & 5 deletions scripts/py_matter_idl/matter_idl/matter_idl_parser.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env python

import dataclasses
import functools
import logging
from typing import Dict, Optional
from typing import Dict, Optional, Set

from lark import Lark
from lark.lexer import Token
Expand Down Expand Up @@ -504,15 +505,25 @@ def idl(self, items):
clusters = []
endpoints = []

global_bitmaps = []
global_enums = []
global_structs = []

for item in items:
if isinstance(item, Cluster):
clusters.append(item)
elif isinstance(item, Endpoint):
endpoints.append(item)
elif isinstance(item, Enum):
global_enums.append(dataclasses.replace(item, is_global=True))
elif isinstance(item, Bitmap):
global_bitmaps.append(dataclasses.replace(item, is_global=True))
elif isinstance(item, Struct):
global_structs.append(dataclasses.replace(item, is_global=True))
else:
raise Exception("UNKNOWN idl content item: %r" % item)

return Idl(clusters=clusters, endpoints=endpoints)
return Idl(clusters=clusters, endpoints=endpoints, global_bitmaps=global_bitmaps, global_enums=global_enums, global_structs=global_structs)

def prefix_doc_comment(self):
print("TODO: prefix")
Expand All @@ -524,9 +535,92 @@ def c_comment(self, token: Token):
self.doc_comments.append(PrefixCppDocComment(token))


def _referenced_type_names(cluster: Cluster) -> Set[str]:
"""
Return the names of all data types referenced by the given cluster.
"""
types = set()
for s in cluster.structs:
for f in s.fields:
types.add(f.data_type.name)

for e in cluster.events:
for f in e.fields:
types.add(f.data_type.name)

for a in cluster.attributes:
types.add(a.definition.data_type.name)

return types


class GlobalMapping:
"""
Maintains global type mapping from an IDL
"""

def __init__(self, idl: Idl):
self.bitmap_map = {b.name: b for b in idl.global_bitmaps}
self.enum_map = {e.name: e for e in idl.global_enums}
self.struct_map = {s.name: s for s in idl.global_structs}

self.global_types = set(self.bitmap_map.keys()).union(set(self.enum_map.keys())).union(set(self.struct_map.keys()))

# Spec does not enforce unique naming in bitmap/enum/struct, however in practice
# if we have both enum Foo and bitmap Foo for example, it would be impossible
# to disambiguate `attribute Foo foo = 1` for the actual type we want.
#
# As a result, we do not try to namespace this and just error out
if len(self.global_types) != len(self.bitmap_map) + len(self.enum_map) + len(self.struct_map):
raise ValueError("Global type names are not unique.")

def merge_global_types_into_cluster(self, cluster: Cluster) -> Cluster:
"""
Merges all referenced global types (bitmaps/enums/structs) into the cluster types.
This happens recursively.
"""
global_types_added = set()

changed = True
while changed:
changed = False
for type_name in _referenced_type_names(cluster):
if type_name not in self.global_types:
continue # not a global type name

if type_name in global_types_added:
continue # already added

# check if this is a global type
if type_name in self.bitmap_map:
global_types_added.add(type_name)
changed = True
cluster.bitmaps.append(self.bitmap_map[type_name])
elif type_name in self.enum_map:
global_types_added.add(type_name)
changed = True
cluster.enums.append(self.enum_map[type_name])
elif type_name in self.struct_map:
global_types_added.add(type_name)
changed = True
cluster.structs.append(self.struct_map[type_name])

return cluster


def _merge_global_types_into_clusters(idl: Idl) -> Idl:
"""
Adds bitmaps/enums/structs from idl.global_* into clusters as long as
clusters reference those type names
"""
mapping = GlobalMapping(idl)
return dataclasses.replace(idl, clusters=[mapping.merge_global_types_into_cluster(cluster) for cluster in idl.clusters])


class ParserWithLines:
def __init__(self, skip_meta: bool):
def __init__(self, skip_meta: bool, merge_globals: bool):
self.transformer = MatterIdlTransformer(skip_meta)
self.merge_globals = merge_globals

# NOTE: LALR parser is fast. While Earley could parse more ambigous grammars,
# earley is much slower:
Expand Down Expand Up @@ -572,14 +666,28 @@ def parse(self, file: str, file_name: Optional[str] = None):
for comment in self.transformer.doc_comments:
comment.appply_to_idl(idl, file)

if self.merge_globals:
idl = _merge_global_types_into_clusters(idl)

return idl


def CreateParser(skip_meta: bool = False):
def CreateParser(skip_meta: bool = False, merge_globals=True):
"""
Generates a parser that will process a ".matter" file into a IDL
Arguments:
skip_meta - do not add metadata (line position) for items. Metadata is
useful for error reporting, however it does not work well
for unit test comparisons
merge_globals - places global items (enums/bitmaps/structs) into any
clusters that reference them, so that cluster types
are self-sufficient. Useful as a backwards-compatible
code generation if global definitions are not supported.
"""
return ParserWithLines(skip_meta)
return ParserWithLines(skip_meta, merge_globals)


if __name__ == '__main__':
Expand Down
8 changes: 8 additions & 0 deletions scripts/py_matter_idl/matter_idl/matter_idl_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class Struct:
code: Optional[int] = None # for responses only
qualities: StructQuality = StructQuality.NONE
api_maturity: ApiMaturity = ApiMaturity.STABLE
is_global: bool = False


@dataclass
Expand Down Expand Up @@ -193,6 +194,7 @@ class Enum:
base_type: str
entries: List[ConstantEntry]
api_maturity: ApiMaturity = ApiMaturity.STABLE
is_global: bool = False


@dataclass
Expand All @@ -201,6 +203,7 @@ class Bitmap:
base_type: str
entries: List[ConstantEntry]
api_maturity: ApiMaturity = ApiMaturity.STABLE
is_global: bool = False


@dataclass
Expand Down Expand Up @@ -290,5 +293,10 @@ class Idl:
clusters: List[Cluster] = field(default_factory=list)
endpoints: List[Endpoint] = field(default_factory=list)

# Global types
global_bitmaps: List[Bitmap] = field(default_factory=list)
global_enums: List[Enum] = field(default_factory=list)
global_structs: List[Struct] = field(default_factory=list)

# IDL file name is available only if parsing provides a file name
parse_file_name: Optional[str] = field(default=None)
77 changes: 77 additions & 0 deletions scripts/py_matter_idl/matter_idl/test_backwards_compatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,83 @@ def ValidateUpdate(self, name: str, old: str, new: str, flags: Compatibility):
with self.subTest(direction="OLD-to-OLD"):
self._AssumeCompatiblity(old, old, old_idl, old_idl, True)

def test_global_enums_delete(self):
self.ValidateUpdate(
"delete a top level enum",
"enum A: ENUM8{} enum B: ENUM8{}",
"enum A: ENUM8{}",
Compatibility.FORWARD_FAIL)

def test_global_enums_change(self):
self.ValidateUpdate(
"change an enum type",
"enum A: ENUM8{}",
"enum A: ENUM16{}",
Compatibility.FORWARD_FAIL | Compatibility.BACKWARD_FAIL)

def test_global_enums_add_remove(self):
self.ValidateUpdate(
"Adding enum values is ok, removing values is not",
"enum A: ENUM8 {A = 1; B = 2;}",
"enum A: ENUM8 {A = 1; }",
Compatibility.FORWARD_FAIL)

def test_global_enums_code(self):
self.ValidateUpdate(
"Switching enum codes is never ok",
"enum A: ENUM8 {A = 1; B = 2; }",
"enum A: ENUM8 {A = 1; B = 3; }",
Compatibility.FORWARD_FAIL | Compatibility.BACKWARD_FAIL)

def test_global_bitmaps_delete(self):
self.ValidateUpdate(
"Deleting a global bitmap",
"bitmap A: BITMAP8{} bitmap B: BITMAP8{}",
"bitmap A: BITMAP8{} ",
Compatibility.FORWARD_FAIL)

def test_global_bitmaps_type(self):
self.ValidateUpdate(
"Changing a bitmap type is never ok",
" bitmap A: BITMAP8{}",
" bitmap A: BITMAP16{}",
Compatibility.FORWARD_FAIL | Compatibility.BACKWARD_FAIL)

def test_global_bitmap_values(self):
self.ValidateUpdate(
"Adding bitmap values is ok, removing values is not",
" bitmap A: BITMAP8 { kA = 0x01; kB = 0x02; } ",
" bitmap A: BITMAP8 { kA = 0x01; } ",
Compatibility.FORWARD_FAIL)

def test_global_struct_removal(self):
self.ValidateUpdate(
"Structure removal is not ok, but adding is ok",
"struct Foo {} struct Bar {} ",
"struct Foo {} ",
Compatibility.FORWARD_FAIL)

def test_global_struct_content_type_change(self):
self.ValidateUpdate(
"Changing structure data types is never ok",
"struct Foo { int32u x = 1; }",
"struct Foo { int64u x = 1; }",
Compatibility.FORWARD_FAIL | Compatibility.BACKWARD_FAIL)

def test_global_struct_content_rename_reorder(self):
self.ValidateUpdate(
"Structure content renames and reorder is ok.",
"struct Foo { int32u x = 1; int8u y = 2; }",
"struct Foo { int8u a = 2; int32u y = 1; }",
Compatibility.ALL_OK)

def test_global_struct_content_add_remove(self):
self.ValidateUpdate(
"Structure content change is not ok.",
"struct Foo { int32u x = 1; }",
"struct Foo { int32u x = 1; int8u y = 2; }",
Compatibility.FORWARD_FAIL | Compatibility.BACKWARD_FAIL)

def test_basic_clusters_enum(self):
self.ValidateUpdate(
"Adding an enum is ok. Also validates code formatting",
Expand Down
1 change: 1 addition & 0 deletions scripts/py_matter_idl/matter_idl/test_data_model_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ def assertIdlEqual(self, a: Idl, b: Idl):
tofile='expected.matter',
)
self.assertEqual(a, b, '\n' + ''.join(delta))
self.fail("IDLs are not equal (above delta should have failed)")

def testBasicInput(self):

Expand Down
Loading

0 comments on commit 2023745

Please sign in to comment.