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

Add Camera clusters XML #35773

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/tests.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do split these out. Finding several small chunks of time to review separate PRs is much easier than finding a big chunk of time to review a single large PR. Splitting things out also allows the review work to be parallelized across more people, in general...

Note that there are merge issues here with ZLL_COMMISSIONING_CLUSTER being incorrectly re-introduced and whatnot. Those would be easier to catch in a smaller PR.

In terms of shared concepts... those are not really relevant to reviewing whether the spec was correctly translated into the XML.

By the way, whatever hand-edits you had to make should probably be turned into alchemy issues, if they were edits to alchemy output?

Copy link
Contributor Author

@gmarcosb gmarcosb Sep 25, 2024

Choose a reason for hiding this comment

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

Sounds good, will split it out!

Had some general questions here though before doing that:

  1. I put these in /draft - is that correct, or should I just put them in /chip?
  2. Hand edits: the camera spec is dependent on ~typedefs, e.g. VideoStreamID: This data type is derived from uint16 - but there doesn't seem to be a way to specify that in the zap xml schema, so I think it'd actually need zap generation changes
  3. structs used across clusters: should I put these in global-structs.xml as I have here, have them defined in one of the clusters, or something else altogether?
  4. If I don't add a <cluster code="0x0551"/> to those shared structures, zap generation ends up generating it in every single .matter file (even non-camera ones); if I add that restriction, though, it seems it causes duplicate definition (see build errors: multiple definition of ‘enum class chip::app::Clusters::detail::StreamTypeEnum’) - what's the right way of dealing with these shared structures?

Thanks Boris!

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Good question. I think the "draft" bits got removed in Remove all un-specified draft clusters from SDK #35583 so putting them in draft likely does not do the right thing.
  2. Ah, yes, I discussed this with @andyg-apple just yesterday. Please talk to @hasty about Alchemy handling this by desugaring the typedefs for now?
  3. global-structs should be for actually global structs, which are not tied to any cluster or list of clusters at all. There are some open questions about which, if any, of the camera structs will end up that way. For now, I'd put them in one of the clusters, or a new camera-structs.xml or something.
  4. For global things (not associated with any clusters), listing them in all .matter files is correct. Things that are associated with clusters should work; please drop me a link on Slack to a branch or SHA that has the duplicate definition error for you, and I will take a look.

Copy link
Contributor Author

@gmarcosb gmarcosb Sep 25, 2024

Choose a reason for hiding this comment

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

  1. As far as I can tell, as long as the /draft path is specified in xmlRoot of zcl.json, it seems to work & generate the files - but if we're moving away from the /draft path (which maybe seems to be the case?) then I'll just put these in /chip (let me know otherwise)
  2. Sounds good, I'll make the changes to alchemy, shouldn't be a problem (the files it will generate should look like my hand-edited ones, but I'll make the changes & verify in parallel)
  3. Will do 👍
  4. Will do 👍 (UPDATED: welp I can't get it to repro so I must be missing something, ignore this one)

Copy link
Contributor Author

@gmarcosb gmarcosb Sep 25, 2024

Choose a reason for hiding this comment

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

First smaller PR with prereqs: #35775
WebRTC provider cluster: #35779
Chime cluster: #35806

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Right, but it's not specified there anymore, right? I figure putting these in chip makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alchemy support for "typedefs" so that we automatically get the ID types to be output as their raw simple types @ project-chip/alchemy#5

Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,13 @@ jobs:
--no-print \
--log-level info \
src/app/zap-templates/zcl/data-model/chip/global-attributes.xml \
src/app/zap-templates/zcl/data-model/chip/camera-av-settings-user-level-management-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/camera-av-stream-management-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/chime-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/global-bitmaps.xml \
src/app/zap-templates/zcl/data-model/chip/global-enums.xml \
src/app/zap-templates/zcl/data-model/chip/global-structs.xml \
src/app/zap-templates/zcl/data-model/chip/push-av-stream-transport-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/semantic-tag-namespace-enums.xml \
src/app/zap-templates/zcl/data-model/chip/access-control-definitions.xml \
src/app/zap-templates/zcl/data-model/chip/access-control-cluster.xml \
Expand Down Expand Up @@ -197,6 +201,8 @@ jobs:
src/app/zap-templates/zcl/data-model/chip/wake-on-lan-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/washer-controls-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/water-heater-management-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/web-rtc-provider-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/web-rtc-requestor-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/wifi-network-diagnostics-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/wifi-network-management-cluster.xml \
src/app/zap-templates/zcl/data-model/chip/window-covering.xml \
Expand Down
6 changes: 6 additions & 0 deletions docs/zap_clusters.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ Generally regenerate using one of:
| 1294 | 0x50E | AccountLogin |
| 1295 | 0x50F | ContentControl |
| 1296 | 0x510 | ContentAppObserver |
| 1361 | 0x551 | CameraAvStreamManagement |
| 1362 | 0x552 | CameraAvSettingsUserLevelManagement |
| 1363 | 0x553 | WebRTCTransportProvider |
| 1364 | 0x554 | WebRTCTransportRequestor |
| 1365 | 0x555 | PushAvStreamTransport |
| 1366 | 0x556 | Chime |
| 1872 | 0x750 | EcosystemInformation |
| 1873 | 0x751 | CommissionerControl |
| 4294048773 | 0xFFF1FC05 | UnitTesting |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/bridge-app/bridge-common/bridge-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_airpurifier_73a6fe2651.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_dishwasher_cc105034fe.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_doorlock_aNKYAreMXE.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_fan_7N2TobIlOX.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_flowsensor_1zVxHedlaV.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
7 changes: 7 additions & 0 deletions examples/chef/devices/rootnode_lightsensor_lZQycTFcJK.matter
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ enum TestGlobalEnum : enum8 {
kFinalValue = 2;
}

enum ThreeLevelAutoEnum : enum8 {
kLow = 0;
kMedium = 1;
kHigh = 2;
kAutomatic = 3;
}

bitmap TestGlobalBitmap : bitmap32 {
kFirstBit = 0x1;
kSecondBit = 0x2;
Expand Down
Loading
Loading