Allow passing in a Path change listener to ember write functions
#35455
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
|
Review changes with SemanticDiff. |
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille,
chapongatien,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
doru91,
fessehaeve,
harimau-qirex,
harsha-rajendran and
hawk248
Co-authored-by: Boris Zbarsky <[email protected]>
…g to the linter to never call const methods without considering their return value
|
PR #35455: Size comparison from 96e687b to 39e9015 Full report (63 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35455: Size comparison from 7d57f07 to 3ebc3be Full report (12 builds for nrfconnect, nxp, qpg, stm32, tizen)
|
tcarmelveilleux
approved these changes
Sep 9, 2024
Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>
|
PR #35455: Size comparison from 7d57f07 to c2e80c2 Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
…andy31415/connectedhomeip into ember_cluster_version_increase_decouple
|
PR #35455: Size comparison from 7d57f07 to 6969378 Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
approved these changes
Sep 10, 2024
yyzhong-g
pushed a commit
to yyzhong-g/connectedhomeip
that referenced
this pull request
Dec 12, 2024
…roject-chip#35455) * Cleaner usage: no need of a separate function that is used in one place only * Attempt an API update * Fix typos in the Accessors src * Fix typo and regen * More fixes on accessors * Update signature for emAfWriteAttributeExternal * Add a comment about all the checks being vague * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/mock/CodegenEmberMocks.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * zap regen and restyle * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/util/attribute-table.h Co-authored-by: Boris Zbarsky <[email protected]> * Rename ChangedPathListener to AttributesChangedListener * Remove chip:: and chip::app * Update constructors of AttributePathParams and add nodiscard according to the linter to never call const methods without considering their return value * Restyled by clang-format * Remove auto-inserted include * Update again and zap regen: removed extra namespace prefixes in accessors.h/cpp * Add comment about uint8_t non-const usage... * Another rename given that the listener is now an attributes and not a path listener * Update src/app/util/attribute-table.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Comment update to talk more about AttributesChangedListener * Restyle --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Tennessee Carmel-Veilleux <[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.
Call paths description
MatterAttributeChangeCallbackis actually an ember functionality even though it resides insrc/app/reporting.cpp(rather than util). In practice we have this callback used by:attribute-storage.cpp, specifically for emberAfEndpointEnableDisable: once for the entire endpoint and then once per Descriptor::PartsList for the root endpoint and parent endpointsattribute-table.cppfor emAfWriteAttribute (with various dirty detection options)The change callback does two things:
Where things are not ideal
ember codeeven though in practice it is (it uses emberAfDataVersionStorage)Changes in the PR
emberAfAttributeChangedandemberAfEndpointChangedcalls to make them clearly ember and use only these insidesrc/app/util. Clusters can still use MatterAttributeChangeCallback, we may need to figure out how/if we can make that one decoupledAttributesChangedListenerthat can be used by the ember callbacks to mark things dirty (instead of accessing the singleton IME object and engine)