-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Follow-up for Camera TLS Certificate Management Cluster to bring it up to latest spec #40552
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
base: master
Are you sure you want to change the base?
Conversation
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.
Code Review
This pull request updates the TLS Certificate Management Cluster to align with the latest specification. The changes are propagated from the cluster definition files (.matter
and .xml
) to the generated code across various languages (C++, Java, Kotlin, Objective-C, and Python). Key modifications include removing a StatusCodeEnum
, renaming and resizing fields in CSR-related commands, and altering the structure of the ProvisionClientCertificateRequest
. The changes are consistent across all affected files and appear to correctly implement the spec update. Based on the provided diffs, the changes look good.
PR #40552: Size comparison from d5106c0 to 90b9cdd Full report (26 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, qpg, stm32, telink)
|
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.
Can we also bring the TLS client management cluster upto the same spec version so we match the corresponding cert management updates here? Or are you planning to do that as a follow-on to this follow-up PR? :)
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.
Might also need to re-run codegen after the merge of #40549 so that these changes are reflected in the camera app as well - I'm not sure if CI or other conflicts would enforce that update otherwise.
separate PR (not follow-up, parallel) - the PRs get big enough on their own, & splitting it on the cluster boundary is a good way to parallelize without making big PRs 😅🤣 |
90b9cdd
to
66320d1
Compare
PR #40552: Size comparison from 8283cdb to 66320d1 Full report (5 builds for cc32xx, realtek, stm32)
|
66320d1
to
15daa98
Compare
PR #40552: Size comparison from 233d3f7 to 15daa98 Full report (4 builds for cc32xx, realtek)
|
….. TLSCertificateManagement.adoc
15daa98
to
db3a54c
Compare
db3a54c
to
4ad005d
Compare
PR #40552: Size comparison from e086a20 to 4ad005d Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40552 +/- ##
=======================================
Coverage 50.72% 50.73%
=======================================
Files 1356 1356
Lines 99337 99334 -3
Branches 12876 12871 -5
=======================================
+ Hits 50390 50397 +7
+ Misses 48947 48937 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
This is a follow-up to #39754:
Related issues
Fixes #40507
Testing
CI