Generate DoorLock cluster with Alchemy#43545
Generate DoorLock cluster with Alchemy#43545tersal wants to merge 1 commit intoproject-chip:masterfrom
Conversation
github-actions
bot
added
examples
darwin
controller
app
examples chef
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request automatically regenerates the DoorLock cluster definition using Alchemy. The changes align the data model with the latest specification, removing redundant attributes and updating descriptions. I've found a few potential issues in the generated files: a risky default value for an enum in a C++ header, and a couple of typos in user-facing summary strings in the XML definition.
| // to kUnknownEnumValue. This is a helper enum value that should only | ||
| // be used by code to process how it handles receiving and unknown | ||
| // enum value. This specific should never be transmitted. | ||
| kUnknownEnumValue = 0, |
There was a problem hiding this comment.
The kUnknownEnumValue for StatusCodeEnum is set to 0. This is risky because 0 is often used for Success in other status enums (e.g., DlStatus). If an unknown status code is received, it would be mapped to 0, which could be misinterpreted as success. Other enums in this file use a value outside the valid range for kUnknownEnumValue. It would be safer to use a value that is not 0, for example 4.
There was a problem hiding this comment.
The enum StatusCodeEnum has two values with values 2 and 3, I'm assuming the generator is taking the first unused number to define as kUnknownEnumValue, which will be odd in this case...
| <optionalConform/> | ||
| </otherwiseConform> | ||
| </feature> | ||
| <feature bit="3" code="LOG" name="Logging" summary="TLLock supports local/on-lock logging when Events are not supportedGt"> |
There was a problem hiding this comment.
There appear to be typos in the summary attribute. It says "TLLock" and "supportedGt". It should probably be "Lock" and "supported". Since this file is auto-generated, the fix is likely needed in the source for the generator (src/app_clusters/DoorLock.adoc).
There was a problem hiding this comment.
This description was generated through the errata, it seems that the typo is there (probably a copy/paste error), however these features are no longer supported in the spec.
| </orTerm> | ||
| </mandatoryConform> | ||
| </feature> | ||
| <feature bit="9" code="NOT" name="Notification" summary="TOperation and Programming Notificationst"> |
There was a problem hiding this comment.
There appear to be typos in the summary attribute. It says "TOperation" and "Notificationst". It should probably be "Operation" and "Notifications". Since this file is auto-generated, the fix is likely needed in the source for the generator (src/app_clusters/DoorLock.adoc).
There was a problem hiding this comment.
This description was generated through the errata, it seems that the typo is there (probably a copy/paste error), however these features are no longer supported in the spec.
There was a problem hiding this comment.
Pull request overview
This PR converts the DoorLock cluster XML file to be Alchemy-generated (as part of issue #37391), aligning it with the rest of the zcl data-model XML files that have already been migrated to Alchemy. The key substantive changes are: removing provisional from ALIRO-related attributes (per spec), adding the new StatusCodeEnum enum, and updating constraints/types on various command fields (e.g., char_string<10> for userName, octet_string instead of long_octet_string for credentialData). The .matter files and generated code files in zzz_generated/ are updated accordingly.
Changes:
door-lock-cluster.xml: Replaced manual XML with Alchemy-generated output; removedprovisionalfrom ALIRO attributes, removed redundant<access op="read" .../>elements, updated field constraints, addedStatusCodeEnum, and converted command codes to hex.- Multiple
.matterfiles (controller, examples, lock-apps): Updated to reflect the XML changes, including removingprovisionalfrom Aliro attributes, addingStatusCodeEnum, changinguserNametochar_string<10>,credentialDatafromlong_octet_stringtooctet_string, and doc-comment updates. zzz_generated/files: AddedStatusCodeEnumC++/Python/Objective-C enum definitions.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/app/zap-templates/zcl/data-model/chip/door-lock-cluster.xml |
Replaced with Alchemy-generated XML; added DO NOT EDIT header, removed provisional from ALIRO attributes, updated constraints, added StatusCodeEnum |
zzz_generated/app-common/clusters/DoorLock/Enums.h |
Added new StatusCodeEnum C++ enum |
zzz_generated/app-common/clusters/DoorLock/EnumsCheck.h |
Added EnsureKnownEnumValue for the new StatusCodeEnum |
src/controller/python/matter/clusters/Objects.py |
Added StatusCodeEnum Python enum |
src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h |
Added MTRDoorLockStatusCode ObjC enum, updated command doc-comments |
src/controller/data_model/controller-clusters.matter |
Updated DoorLock cluster: removed provisional, added StatusCodeEnum, updated field types/comments |
Multiple examples/**/*.matter and examples/lock-app/**/*.matter |
Same DoorLock cluster updates propagated to all example apps |
|
PR #43545: Size comparison from 2509a55 to 12612e7 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43545 +/- ##
=======================================
Coverage 54.05% 54.05%
=======================================
Files 1549 1549
Lines 106697 106697
Branches 13318 13318
=======================================
Hits 57675 57675
Misses 49022 49022 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Alchemy generation of DoorLock cluster, most meaningful changes are:
provisionalfrom ALIRO feature related attributes according to how it is described in the spec.Related issues
Part of #37391
Testing
No meaningful code change, CI should run tests and validate backwards compatibility.