-
Notifications
You must be signed in to change notification settings - Fork 70
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
Matter 1.4 #1657
base: main
Are you sure you want to change the base?
Matter 1.4 #1657
Conversation
81345ae
to
1f5e1a7
Compare
This includes application-level upgrades to Matter 1.4. Upgrades the model, cluster definitions, endpoint types and behaviors. Some protocol-level features are still TODO. One notable change is that Basic Information "unique ID" is mandatory in 1.4. We now generate one unconditionally unless provided as input. This required tweaking current and deprecated codepaths and numerous tests but should be largely transparent to users. Extends conformance and constraint parsers with new flags and syntax, plus a couple of bits that were used previously but we didn't support. Some of these were complicated and consisted of ad-hoc syntax only used in one or two places... But validation of the final model now sits at 100%. Includes many upgrades to spec ingestion to handle special cases, mostly due to malformatting. Replaces some hard-coded fixes with automatic detection/repair of errors that have become more prevalent as the spec grows. Tweaks codegen for derived enums to generate union types that accept values from extended enums. This appears to have been unnecessary because the specification was changed to include the redundant values, possibly because the equivalent fix was deemed infeasible in CHIP and/or alchemy. However it does improve consistency so leaving in. Includes a few minor CHIP test tweaks. The new changes appear to have been transparent to CHIP but some test edits were no longer necessary and I fixed a few tests that had been broken previously. Finally, includes a large chunk of code to handle ANSI text processing. A bit of a rathole... Came about because I wanted to make test error reporting more legible, which requires complex nested merging of styled text from multiple sources. And I'd gotten tired of the mix of hand-coded styling and use of a 3rd-party library that has typing issues. Over time we can migrate to this new code and drop the 3rd-party dependency, but will require some package reorganization because it currently lives in "tooling" which is not available everywhere. So just approaching on an "as needed" basis for now.
children: [ | ||
{ tag: "field", name: "Decrement", id: 0 }, | ||
{ tag: "field", name: "Increment", id: 1 }, | ||
], | ||
}, | ||
|
||
// In 1.4 they created ColorLoopDirectionEnum but left the type of ColorLoopDirection attribute as uint8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -135,14 +140,30 @@ LocalMatter.children.push({ | |||
], | |||
}, | |||
|
|||
// In 1.4 the spec removed default values for these. Unsure why but bring them back as they are mandatory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact was removed because of the fact that no one changed it and in fact these defaults make o sense because no light will ever be able to support these values this. So the default was removed to ensure manufactuers set meaningful values here. So I would also tend to remove to make sure implementors set meaningful values here
// Set the correct type of MoveMode because just in the description | ||
{ | ||
tag: "command", | ||
id: 0x4b, | ||
name: "MoveColorTemperature", | ||
|
||
children: [ | ||
{ tag: "field", name: "MoveMode", id: 0x0, type: "MoveMode", conformance: "M", constraint: "desc" }, | ||
{ tag: "field", name: "MoveMode", id: 0x0, type: "MoveModeEnum", conformance: "M", constraint: "desc" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this not be in specs in 1.4 correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only edited these lines because the types changed and they otherwise break the model. I generally don't review the overrides to see which are unnecessary anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, see my comment ... we can also do that after merging here or incorporate it ... both ok for me
@@ -153,7 +174,7 @@ LocalMatter.children.push({ | |||
name: "StepColorTemperature", | |||
|
|||
children: [ | |||
{ tag: "field", name: "StepMode", id: 0x0, type: "StepMode", conformance: "M", constraint: "desc" }, | |||
{ tag: "field", name: "StepMode", id: 0x0, type: "StepModeEnum", conformance: "M", constraint: "desc" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this should be correct in 1.4 specs
@@ -21,7 +21,7 @@ LocalMatter.children.push({ | |||
tag: "field", | |||
name: "Ranges", | |||
|
|||
// 1.3 spec mislabels the "access" column as "default" | |||
// 1.3 spec mislabels the "access" column as "default" (still present in 1.4 as well) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed as Spec issue https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/10821
requirements: OccupancySensorRequirements, | ||
behaviors: SupportedBehaviors( | ||
OccupancySensorRequirements.server.mandatory.Identify, | ||
OccupancySensorRequirements.server.mandatory.OccupancySensing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is removed? it should be still mandatory
@@ -37,7 +40,7 @@ export const PowerSourceEndpointDefinition = MutableEndpoint({ | |||
deviceRevision: 1, | |||
deviceClass: DeviceClassification.Utility, | |||
requirements: PowerSourceRequirements, | |||
behaviors: SupportedBehaviors(PowerSourceRequirements.server.mandatory.PowerSource) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or this? Or has definition changed?
@lauckhart For overrides I also assume:
|
This includes application-level upgrades to Matter 1.4. Upgrades the model, cluster definitions, endpoint types and behaviors. Some protocol-level features are still TODO.
One notable change is that Basic Information "unique ID" is mandatory in 1.4. We now generate one unconditionally unless provided as input. This required tweaking current and deprecated codepaths and numerous tests but should be largely transparent to users.
Extends conformance and constraint parsers with new flags and syntax, plus a couple of bits that were used previously but we didn't support. Some of these were complicated and consisted of ad-hoc syntax only used in one or two places... But validation of the final model now sits at 100%.
Includes many upgrades to spec ingestion to handle special cases, mostly due to malformatting. Replaces some hard-coded fixes with automatic detection/repair of errors that have become more prevalent as the spec grows.
Tweaks codegen for derived enums to generate union types that accept values from extended enums. This appears to have been unnecessary because the specification was changed to include the redundant values, possibly because the equivalent fix was deemed infeasible in CHIP and/or alchemy. However it does improve consistency so leaving in.
Includes a few minor CHIP test tweaks. The new changes appear to have been transparent to CHIP but some test edits were no longer necessary and I fixed a few tests that had been broken previously.
Finally, includes a large chunk of code to handle ANSI text processing. A bit of a rathole... Came about because I wanted to make test error reporting more legible, which requires complex nested merging of styled text from multiple sources. And I'd gotten tired of the mix of hand-coded styling and use of a 3rd-party library that has typing issues. Over time we can migrate to this new code and drop the 3rd-party dependency, but will require some package reorganization because it currently lives in "tooling" which is not available everywhere. So just approaching on an "as needed" basis for now.