Modify Matter.framework to allow per-controller storage. #28533
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
gjc13,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kkasperczyk-no,
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda,
mhazley and
mkardous-silabs
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
|
PR #28533: Size comparison from 9524db4 to 73d668f Increases above 0.2%:
Increases (34 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, psoc6)
Decreases (23 builds for cc32xx, efr32, linux, mbed, nrfconnect, psoc6, qpg)
Full report (44 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
reviewed
Aug 29, 2023
nivi-apple
approved these changes
Aug 29, 2023
|
PR #28533: Size comparison from 9524db4 to e6278e2 Increases above 0.2%:
Increases (38 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, psoc6, telink)
Decreases (39 builds for bl702, bl702l, efr32, linux, mbed, nrfconnect, qpg, telink)
Full report (62 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
woody-apple
approved these changes
Aug 30, 2023
bzbarsky-apple
force-pushed
the
darwin-storage-revamp
branch
from
5fe76db to
f2b9084
Compare
…ctly. For built-in types it seems to not do that.
…e we no longer encode those.
bzbarsky-apple
force-pushed
the
darwin-storage-revamp
branch
from
7ebbf7d to
8479524
Compare
|
PR #28533: Size comparison from b3c0ac1 to 8479524 Increases (1 build for telink)
Decreases (1 build for efr32)
Full report (67 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
Fast-tracking platform-specific change with platform owner reviews. |
abpoth
pushed a commit
to abpoth/connectedhomeip
that referenced
this pull request
Sep 12, 2023
…p#28533) * Modify Matter.framework to allow per-controller storage. This also allows starting multiple controllers with different node IDs on a single fabric. Public API changes: * It's now possible to initialize MTRDeviceControllerFactoryParams without storage. When a factory is then started with those params, it will expect storage to be provided for every controller that is created. * Controllers to be created in the new setup use MTRDeviceControllerStartupParameters, not MTRDeviceControllerStartupParams. * When starting a controller, API clients provide a UUID for that controller (which is then exposed on the MTRDeviceController) and a storage delegate. * For now, the only supported controller startup mode is for the client to provide the full certificate chain, operational key and vendor ID, via MTRDeviceControllerExternalCertificateStartupParameters. For controllers that will commission devices, that means also providing an MTROperationalCertificateIssuer. * The new "create a controller" API is called createController. * The new MTRDeviceControllerStorageDelegate API provides some context for the key/value pairs in terms of whether they need to be stored in encrypted storage or not, and whether they can be shared across multiple devices and under what conditions. Implementation notes: * MTRDemuxingStorage handles directing storage requests to the right per-controller storage object. * MTRDeviceControllerDataStore wraps the raw storage delegate and provides a semantic API on top of its key/value storage for the storage operations we actually want to perform. * MTRSessionResumptionStorageBridge implements session resumption storage, acting as an adapter between the Matter session resumption storage API and MTRDeviceControllerDataStore. In particular, it happens locating the right controller(s) to talk to and so on. This avoids dealing with the default Matter implementation's use of non-fabric-index-scoped keys for storing session resumption information. Fixes project-chip#27394 * Require consumers to provide a dispatch queue for the storage delegate. * Address review comments. * Apply spelling/grammar suggestions from code review Co-authored-by: Nivi Sarkar <[email protected]> * Address review comments. * Address more review comments. * Address more review comments. * Address more review comments. * Add some validation when deserializing node IDs and CATs. * Stop trusting the secure coding stuff to actually enforce types correctly. For built-in types it seems to not do that. * Remove NSMutableArray and NSSet from controller storage classes, since we no longer encode those. --------- Co-authored-by: Nivi Sarkar <[email protected]>
HunsupJung
pushed a commit
to HunsupJung/connectedhomeip
that referenced
this pull request
Oct 23, 2023
…p#28533) * Modify Matter.framework to allow per-controller storage. This also allows starting multiple controllers with different node IDs on a single fabric. Public API changes: * It's now possible to initialize MTRDeviceControllerFactoryParams without storage. When a factory is then started with those params, it will expect storage to be provided for every controller that is created. * Controllers to be created in the new setup use MTRDeviceControllerStartupParameters, not MTRDeviceControllerStartupParams. * When starting a controller, API clients provide a UUID for that controller (which is then exposed on the MTRDeviceController) and a storage delegate. * For now, the only supported controller startup mode is for the client to provide the full certificate chain, operational key and vendor ID, via MTRDeviceControllerExternalCertificateStartupParameters. For controllers that will commission devices, that means also providing an MTROperationalCertificateIssuer. * The new "create a controller" API is called createController. * The new MTRDeviceControllerStorageDelegate API provides some context for the key/value pairs in terms of whether they need to be stored in encrypted storage or not, and whether they can be shared across multiple devices and under what conditions. Implementation notes: * MTRDemuxingStorage handles directing storage requests to the right per-controller storage object. * MTRDeviceControllerDataStore wraps the raw storage delegate and provides a semantic API on top of its key/value storage for the storage operations we actually want to perform. * MTRSessionResumptionStorageBridge implements session resumption storage, acting as an adapter between the Matter session resumption storage API and MTRDeviceControllerDataStore. In particular, it happens locating the right controller(s) to talk to and so on. This avoids dealing with the default Matter implementation's use of non-fabric-index-scoped keys for storing session resumption information. Fixes project-chip#27394 * Require consumers to provide a dispatch queue for the storage delegate. * Address review comments. * Apply spelling/grammar suggestions from code review Co-authored-by: Nivi Sarkar <[email protected]> * Address review comments. * Address more review comments. * Address more review comments. * Address more review comments. * Add some validation when deserializing node IDs and CATs. * Stop trusting the secure coding stuff to actually enforce types correctly. For built-in types it seems to not do that. * Remove NSMutableArray and NSSet from controller storage classes, since we no longer encode those. --------- Co-authored-by: Nivi Sarkar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also allows starting multiple controllers with different node IDs on a single fabric.
Open questions:
more work for the API consumer and more work in our implementation, because
we then have to double-check that we got the type we expected back from
storage.
Public API changes:
Implementation notes:
Fixes #27394