Skip to content
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

update_cluster_revisions.py: Add possibility to bump cluster revisions to the latest in spec #36456

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
192 changes: 192 additions & 0 deletions scripts/tools/zap/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
---
orphan: true
---

# ZAP

This directory contains various tools related to ZAP.

# ZAP Cluster Revision Update Tool (update_cluster_revisions.py)

This tool parses all ZAP files in the codebase and updates cluster revisions.

**WARNING**: This tool only updates the revision number. Please ensure any new
attributes, events or commands are implemented accordingly.

## Prerequisites

This tool may require the python environment to parse the latest version of the
cluster revisions (i.e. if you don't provide a '--new-revision' argument). The python
environment can be built and activated using:

```
# Build
./scripts/build_python.sh -i out/python_env

# Activate
source out/python_env/bin/activate
```

## How to run

Usage:

```
usage: update_cluster_revisions.py [-h] [--cluster-id CLUSTER_ID] [--new-revision NEW_REVISION] [--old-revision OLD_REVISION] [--dry-run] [--parallel] [--no-parallel]

Update the ClusterRevision for a chosen cluster in all .zap files

options:
-h, --help show this help message and exit
--cluster-id CLUSTER_ID
The id of the cluster, as hex, for which the cluster revision should be updated. If omitted, all outdated clusters are updated.
--new-revision NEW_REVISION
The new cluster revision as a decimal integer. If omitted, the cluster revision will be updated to the latest according to the specification
--old-revision OLD_REVISION
If set, only clusters with this old revision will be updated. This is a decimal integer.
--dry-run Don't do any generation, just log what .zap files would be updated (default: False)
--parallel
--no-parallel
```

[Note]

- Use `--dry-run` to print only, don't update ZAP files
- Omit `--cluster-id` to search for all clusters
- Omit `--new-revision` to update to the latest revision according to the
specification
- Optionally provide `--old-revision`, `--cluster-id` and `--new-revision` to
update only clusters that match `old-revision`.

Example #1: Check all outdated cluster revisions and print only (do not modify
the ZAP file `--dry-run`):

```
./scripts/tools/zap/update_cluster_revisions.py --dry-run --no-parallel
```

Example output if outdated clusters are found:

```
...
Checking for outdated cluster revisions on: examples/light-switch-app/light-switch-common/icd-lit-light-switch-app.zap
6 found!
Endpoint: 0 cluster_code: 40 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Endpoint: 0 cluster_code: 48 cluster_revision: 1 cluster_spec_revision: 2 name: General Commissioning
Endpoint: 0 cluster_code: 49 cluster_revision: 1 cluster_spec_revision: 2 name: Network Commissioning
Endpoint: 0 cluster_code: 53 cluster_revision: 2 cluster_spec_revision: 3 name: Thread Network Diagnostics
Endpoint: 1 cluster_code: 3 cluster_revision: 4 cluster_spec_revision: 5 name: Identify
Endpoint: 2 cluster_code: 3 cluster_revision: 2 cluster_spec_revision: 5 name: Identify
Checking for outdated cluster revisions on: examples/light-switch-app/light-switch-common/light-switch-app.zap
6 found!
Endpoint: 0 cluster_code: 40 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Endpoint: 0 cluster_code: 48 cluster_revision: 1 cluster_spec_revision: 2 name: General Commissioning
Endpoint: 0 cluster_code: 49 cluster_revision: 1 cluster_spec_revision: 2 name: Network Commissioning
Endpoint: 0 cluster_code: 53 cluster_revision: 2 cluster_spec_revision: 3 name: Thread Network Diagnostics
Endpoint: 1 cluster_code: 3 cluster_revision: 4 cluster_spec_revision: 5 name: Identify
Endpoint: 2 cluster_code: 3 cluster_revision: 2 cluster_spec_revision: 5 name: Identify
Checking for outdated cluster revisions on: examples/light-switch-app/qpg/zap/switch.zap
7 found!
Endpoint: 0 cluster_code: 40 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Endpoint: 0 cluster_code: 47 cluster_revision: 1 cluster_spec_revision: 3 name: Power Source
Endpoint: 0 cluster_code: 48 cluster_revision: 1 cluster_spec_revision: 2 name: General Commissioning
Endpoint: 0 cluster_code: 49 cluster_revision: 1 cluster_spec_revision: 2 name: Network Commissioning
Endpoint: 0 cluster_code: 53 cluster_revision: 2 cluster_spec_revision: 3 name: Thread Network Diagnostics
Endpoint: 1 cluster_code: 3 cluster_revision: 4 cluster_spec_revision: 5 name: Identify
Endpoint: 2 cluster_code: 3 cluster_revision: 4 cluster_spec_revision: 5 name: Identify
Checking for outdated cluster revisions on: src/controller/data_model/controller-clusters.zap
0 found!

Checking for outdated cluster revisions on: scripts/tools/zap/tests/inputs/lighting-app.zap
8 found!
Endpoint: 0 cluster_code: 40 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Endpoint: 0 cluster_code: 48 cluster_revision: 1 cluster_spec_revision: 2 name: General Commissioning
Endpoint: 0 cluster_code: 49 cluster_revision: 1 cluster_spec_revision: 2 name: Network Commissioning
Endpoint: 0 cluster_code: 53 cluster_revision: 2 cluster_spec_revision: 3 name: Thread Network Diagnostics
Endpoint: 0 cluster_code: 59 cluster_revision: 1 cluster_spec_revision: 2 name: Switch
Endpoint: 1 cluster_code: 3 cluster_revision: 4 cluster_spec_revision: 5 name: Identify
Endpoint: 1 cluster_code: 6 cluster_revision: 5 cluster_spec_revision: 6 name: On/Off
Endpoint: 1 cluster_code: 1030 cluster_revision: 4 cluster_spec_revision: 5 name: Occupancy Sensing
...
```

Example #2: Check for possible outdated revisions of the cluster 0x28 revisions
and print only (do not modify the ZAP file):

```
./scripts/tools/zap/update_cluster_revisions.py --dry-run --no-parallel --cluster-id 0x28
```

Example output:

```
...
Checking for outdated cluster revisions on: examples/bridge-app/bridge-common/bridge-app.zap
1 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Checking for outdated cluster revisions on: examples/window-app/common/window-app.zap
1 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Checking for outdated cluster revisions on: examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.zap
1 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Checking for outdated cluster revisions on: examples/smoke-co-alarm-app/smoke-co-alarm-common/smoke-co-alarm-app.zap
1 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Checking for outdated cluster revisions on: examples/placeholder/linux/apps/app1/config.zap
2 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 2 cluster_spec_revision: 4 name: Basic Information
Endpoint: 1 cluster_code: 0x28 cluster_revision: 2 cluster_spec_revision: 4 name: Basic Information
...
```

Example #3: Update outdated revisions of the cluster 0x28 to the latest in the
specification:

```
./scripts/tools/zap/update_cluster_revisions.py --no-parallel --cluster-id 0x28
```

Example output:

```
Checking for outdated cluster revisions on: examples/window-app/common/window-app.zap
1 found!
Endpoint: 0 cluster_code: 0x28 cluster_revision: 3 cluster_spec_revision: 4 name: Basic Information
Cluster revisions updated successfully!

Searching for zcl file from /usr/local/google/home/sergiosoares/connectedhomeip/examples/window-app/common/window-app.zap
🔧 Using state directory: /usr/local/google/home/sergiosoares/.zap
🤖 Conversion started
🔍 input files: /usr/local/google/home/sergiosoares/connectedhomeip/examples/window-app/common/window-app.zap
🔍 output pattern: /usr/local/google/home/sergiosoares/connectedhomeip/examples/window-app/common/window-app.zap
🐝 database and schema initialized
🐝 zcl package loaded: /usr/local/google/home/sergiosoares/connectedhomeip/src/app/zap-templates/zcl/zcl.json
🐝 templates loaded: /usr/local/google/home/sergiosoares/connectedhomeip/src/app/zap-templates/app-templates.json


🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

Application is failing the Device Type Specification as follows:

- ⚠ Check Device Type Compliance on endpoint: 0, device type: MA-rootdevice, cluster: Localization Configuration, attribute: ActiveLocale needs to be enabled
- ⚠ Check Device Type Compliance on endpoint: 0, device type: MA-rootdevice, cluster: Localization Configuration, attribute: SupportedLocales needs to be enabled
- ⚠ Check Device Type Compliance on endpoint: 2, device type: MA-windowcovering, cluster: Window Covering server needs bit 3 enabled in the Feature Map attribute

Application is failing the Cluster Specification as follows:


🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨


👈 read in: /usr/local/google/home/sergiosoares/connectedhomeip/examples/window-app/common/window-app.zap
👉 write out: /usr/local/google/home/sergiosoares/connectedhomeip/examples/window-app/common/window-app.zap
👉 write out: undefined
😎 Conversion done!
```

Example #4: Update outdated revisions of the cluster 0x28 to version 3:

```
./scripts/tools/zap/update_cluster_revisions.py --no-parallel --cluster-id 0x28 --new-revision 3
```
135 changes: 111 additions & 24 deletions scripts/tools/zap/update_cluster_revisions.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import os
import subprocess
import sys
from dataclasses import dataclass
from pathlib import Path

BASIC_INFORMATION_CLUSTER_ID = int("0x0039", 16)
Expand All @@ -32,6 +33,24 @@
}


@dataclass
class ClusterInfo():
endpoint_id: int
cluster_code: int
cluster_spec_revision: int
cluster_name: str
json_attribute: object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, for hinting isn't this a dict[str,object]?


def cluster_revision(self):
return int(self.json_attribute["defaultValue"])

def __str__(self):
return ('Endpoint: %d cluster_code: %s cluster_revision: %d cluster_spec_revision: %d name: %s' % (self.endpoint_id, hex(self.cluster_code), self.cluster_revision(), self.cluster_spec_revision, self.cluster_name))

def update_cluster_revision(self):
self.json_attribute["defaultValue"] = self.cluster_spec_revision


def getTargets(cluster_id: int):
ROOTS_TO_SEARCH = [
'./examples',
Expand All @@ -51,6 +70,34 @@ def getTargets(cluster_id: int):
return targets


def get_outdated_clusters(data: object, xml_clusters: dict, args) -> list[ClusterInfo]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contents of args could have been state of a VersionUpdateProcessor class.

Overall this script relies too much on passing data around that could be state of a class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, there is a mix of functions in this PR mixed with with snake_case and lowerCamelCase. Overall I think autopep8 does say we should to functions are snake_case. My suggestions is keep it the same for the file and if we want to migrate the entire file we do it all at once in the file as a follow up refactor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or follow Tennessee's recommendation of starting the switch to pep8 and doing that for all your newly introduced functions

result = []
for endpoint in data.get("endpointTypes", []):
endpoint_id = endpoint.get("id") - 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is NOT the endpoint_id. Endpoint IDs are separate from the id field. This may work oddly on ZAP files that have non-contiguous endpoint IDs.

for cluster in endpoint.get("clusters", []):
# Skip cluster if user passed a cluster id and it doesn't match
if args.cluster_id is not None and cluster['code'] != args.cluster_id:
continue
for attribute in cluster.get("attributes", []):
if attribute.get("name") != "ClusterRevision" or attribute.get("storageOption") != "RAM":
soares-sergio marked this conversation as resolved.
Show resolved Hide resolved
continue
try:
cluster_revision = int(attribute.get("defaultValue"))
spec_revision = xml_clusters[cluster.get("code")].revision
except (KeyError, ValueError):
continue
# Filter in outdated clusters only
if (cluster_revision == spec_revision):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid C-style () around whole condition

break
# If old_revision is present, filter in matching only
if (args.old_revision is not None and cluster_revision != args.old_revision):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parenthesize sub-expressions:

Suggested change
if (args.old_revision is not None and cluster_revision != args.old_revision):
if (args.old_revision is not None) and (cluster_revision != args.old_revision):

break
cluster_info = ClusterInfo(endpoint_id=endpoint_id, cluster_code=cluster.get("code"),
cluster_spec_revision=spec_revision, cluster_name=cluster.get("name"), json_attribute=attribute)
result.append(cluster_info)
return result


def checkPythonVersion():
if sys.version_info[0] < 3:
print('Must use Python 3. Current version is ' +
Expand All @@ -61,12 +108,12 @@ def checkPythonVersion():
def runArgumentsParser():
parser = argparse.ArgumentParser(
description='Update the ClusterRevision for a chosen cluster in all .zap files')
parser.add_argument('--cluster-id', default=None, action='store',
help='The id of the cluster, as hex, for which the cluster revision should be updated.')
parser.add_argument('--new-revision', default=None, action='store',
help='The new cluster revision as a decimal integer')
parser.add_argument('--cluster-id', required='--new-revision' in sys.argv, default=None, action='store',
help='The id of the cluster, as hex, for which the cluster revision should be updated. If omitted, all outdated clusters are updated.')
parser.add_argument('--new-revision', required='--old-revision' in sys.argv, default=None, action='store',
help='The new cluster revision as a decimal integer. If omitted, the cluster revision will be updated to the latest according to the specification')
parser.add_argument('--old-revision', default=None, action='store',
help='If set, only clusters with this old revision will be updated. This is a decimal integer.')
help='If set, only clusters with this old revision will be updated. This is a decimal integer.')
parser.add_argument('--dry-run', default=False, action='store_true',
help="Don't do any generation, just log what .zap files would be updated (default: False)")
parser.add_argument('--parallel', action='store_true')
Expand All @@ -75,15 +122,8 @@ def runArgumentsParser():

args = parser.parse_args()

if args.cluster_id is None:
logging.error("Must have a cluster id")
sys.exit(1)

if args.new_revision is None:
logging.error("Must have a new cluster revision")
sys.exit(1)

args.cluster_id = int(args.cluster_id, 16)
if args.cluster_id:
args.cluster_id = int(args.cluster_id, 16)

return args

Expand All @@ -106,7 +146,11 @@ def updateOne(item):
"""
Helper method that may be run in parallel to update a single target.
"""
(args, target) = item
(args, target, spec_xml_clusters) = item

print(f"Will try to update: {target}")
if args.dry_run:
return

with open(target, "r") as file:
data = json.load(file)
Expand All @@ -126,6 +170,41 @@ def updateOne(item):
subprocess.check_call(['./scripts/tools/zap/convert.py', target])


def updateOneToLatest(item):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow pep8 satule (e.g. update_one_to_latest)

"""
Helper method that may be run in parallel to update all clusters in a single .zap file to the latest revision according to the spec.
"""
(args, target, spec_xml_clusters) = item

with open(target, "r") as file:
data = json.load(file)

print("Checking for outdated cluster revisions on: %s" % target)

outdated_clusters = get_outdated_clusters(data, spec_xml_clusters, args)
print("%d found!" % len(outdated_clusters))
print(*outdated_clusters, sep='\n')

if args.dry_run:
return

# Update outdated cluster revisions according to the spec
for cluster in outdated_clusters:
cluster.update_cluster_revision()

# Check there's no longer any outdated cluster
assert (not get_outdated_clusters(data, spec_xml_clusters, args))

# If found outdated clusters, then save and reformat zap
if (outdated_clusters):
print('Cluster revisions updated successfully!\n')
with open(target, "w") as file:
json.dump(data, file)

# Now run convert.py on the file to have ZAP reformat it however it likes.
subprocess.check_call(['./scripts/tools/zap/convert.py', target])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include python3 in the command



def main():
checkPythonVersion()

Expand All @@ -136,26 +215,34 @@ def main():

args = runArgumentsParser()

os.chdir(CHIP_ROOT_DIR)
print("**WARNING**: This tool only updates the revision number. Please ensure any new attributes, events or commands are implemented accordingly.")

targets = getTargets(args.cluster_id)
os.chdir(CHIP_ROOT_DIR)

if args.dry_run:
for target in targets:
print(f"Will try to update: {target}")
sys.exit(0)
targets = getTargets(args.cluster_id if args.cluster_id else 0)

items = [(args, target) for target in targets]
if args.new_revision:
update_func = updateOne
items = [(args, target, None) for target in targets]
else:
update_func = updateOneToLatest
try:
from chip.testing.spec_parsing import build_xml_clusters
except ImportError:
print("Couldn't import 'chip.testing.spec_parsing'. Try building/activating your python environment: ./scripts/build_python.sh -i out/python_env && source out/python_env/bin/activate)")
return 1
spec_xml_clusters, problems = build_xml_clusters()
items = [(args, target, spec_xml_clusters) for target in targets]

if args.parallel:
# Ensure each zap run is independent
os.environ['ZAP_TEMPSTATE'] = '1'
with multiprocessing.Pool() as pool:
for _ in pool.imap_unordered(updateOne, items):
for _ in pool.imap_unordered(update_func, items):
pass
else:
for item in items:
updateOne(item)
update_func(item)


if __name__ == '__main__':
Expand Down
Loading