Move command handler registered list out of InteractionModelEngine and into a separate Registry
#34414
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
github-actions
bot
added
documentation
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille,
chapongatien,
chrisdecenzo and
chshu
pullapprove
bot
requested review from
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang and
yufengwangca
|
PR #34414: Size comparison from fca7948 to e59ee29 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34414: Size comparison from fca7948 to 07fdae9 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
approved these changes
Jul 19, 2024
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #34414: Size comparison from a10b47d to 84013df Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
tehampson
reviewed
Jul 23, 2024
tehampson
approved these changes
Jul 23, 2024
j-ororke
pushed a commit
to j-ororke/connectedhomeip
that referenced
this pull request
Jul 31, 2024
…d into a separate `Registry` (project-chip#34414) * Starting to define a commandhandlerregistry * Add missing files and replace some deprecated files usage * Replace all usages of IME to the command handler registry direct calls * Restyle * Fix wrong comment paste * Fix include * Remove unused include * Fix linter error * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/controller/tests/TestServerCommandDispatch.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/controller/tests/TestServerCommandDispatch.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
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 is done for symmetry with the AttributeAccessInterface. The intent is to decouple usage (registering such interfaces now does not rely on the InteractionModelEngine) and allow eventual separation in DataModel processing (the processing of these are specific to data handling, same as attributes, and should not be separated out in the IME).
This has the downside that now separate IME instances cannot have separate command handler lists. I expect this to still be OK for now as for AttributeAccessInterface we did not have a separation to start with.
Changes
CommandHandlerInterfaceRegistrywith register/unregister functionsupgrade.mdwith description of what functions to use to replace the old register/unregister methodsThe only odd TODO is that IME used to clear all registered command handler interfaces on shutdown, which seems somewhat reasonable for a list owned by the IME, however it does seem odd for a globally managed list. I kept that clear for now, however I expect we probably want to remove that clear in the future and eventually move it to
CodegenDataModel::Shutdownonce data models are in use.