Separate out AttributePathExpandIterator::Position
#36980
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
…y use it (so I can validate tests)
|
Review changes with |
|
PR #36980: Size comparison from e7f6d0e to 652e389 Full report (11 builds for cc13x4_26x4, cc32xx, qpg, stm32, tizen)
|
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
andy31415
force-pushed
the
attribute-path-expand-state
branch
from
e3b7aba to
e6b8003
Compare
|
PR #36980: Size comparison from e7082e2 to e6b8003 Full report (3 builds for cc32xx, stm32)
|
…t. Hoping for cleaner code
|
PR #36980: Size comparison from b528132 to 9de15ec Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
bzbarsky-apple
approved these changes
Jan 10, 2025
|
PR #36980: Size comparison from 87f7c4c to 13eef77 Full report (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Alami-Amine
pushed a commit
to Alami-Amine/connectedhomeip
that referenced
this pull request
Jan 12, 2025
) * Copied over the new AttributePathExpandIterator and will incrementally use it (so I can validate tests) * Rename AttributePathExpandIterator to legacy * Prepare for using new style iterators ... checking NOT YET enabled though * Enabled checks ... and unit tests fail, but this now can be debugged * Fix some of the underlying bugs: read handling logic assumes we are ok to undo * Unit tests pass now * Restyle * Use new iterator in IME * Update logic to use the new iterator on testRead * more updates * Restyle * Remove the legacy attribute path expand iterator * Update naming * Restyle * Remove extra argument for ReadHandler constructor * Restyle * Slight flash improvement * Fix up includes * Removed empty line * added comment on why state is a friend class * Comment updates * Restyle, add some comments and add extra checks on validity check only for expansion. This saves a tiny amount of flash (32 bytes) * Remove an include * Comment updates, renamed mLastOutputPath to mOutputPath * Fix one typo * Re-arrange members of ReadHandler to optimize for memory layout. This saves 8 bytes for struct. We still have a 20-byte padding which I am unsure how to get rid of * Restyle * Rename State to Position * One more rename * Remove redundant assigment ...we are at a net 0 txt increase now on qpg * Add more unit tests for non-obvious requirement that wildcard expansion checks path validity, however non-wildcard does not check it * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/ReadHandler.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/ReadHandler.cpp Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> * Use different values for the cluster ids for testing * One more state to position change * mExpanded is now set during output path returning. Removed 2 more sets to save another tinier amount of .text * Remove some tests that seem redundant, keep only one * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/AttributePathExpandIterator.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/ReadHandler.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/AttributePathExpandIterator.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/ReadHandler.h Co-authored-by: Boris Zbarsky <[email protected]> * Use mCompletePosition * Another rename * Undo submodule update * Restyle * Update comment text to not sound like graph parsing * Rename method to be more descriptive * Update peek attribute iterator to rollback and update code logic a bit. Hoping for cleaner code --------- Co-authored-by: Andrei Litvin <[email protected]> Co-authored-by: Tennessee Carmel-Veilleux <[email protected]> Co-authored-by: Boris Zbarsky <[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.
We currently iterate over attribute paths for wildcard expansion only (and an extra check for path validity). We do not require a full iterator (e.g. data model provider) unless actively iterating.
This PR splits AttributePathExpandIterator into actual iterator and
position(which is used to resume an iteration). This saves some RAM (don't need to have DMProvider for example) and allows easier management/changes of this iterator in the future as we consider options for DM::Provider ineterfaces.Testing