[*ChangeListener] replace AttributesChangedListener with ProviderChan… #41518
Conversation
This file contains hidden or 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
…geListener AttributesChangedListener is a duplicate of ProviderChangeListener and there is no need to keep both. Since AttributesChangedListener is for ember and ProviderChangeListener is from DataModel, it makes more sense to keep ProviderChangeListener.
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a good refactoring that replaces the duplicate AttributesChangedListener with DataModel::ProviderChangeListener. The changes are consistent across the codebase and improve maintainability by removing redundant code. I have one minor suggestion to update a stale comment to reflect this refactoring.
andy31415
reviewed
Oct 17, 2025
andy31415
previously approved these changes
Oct 17, 2025
andy31415
dismissed
their stale review
Dismiss for now - we seem to need a few compile fixes for this
|
PR #41518: Size comparison from c87ece5 to 5635fa9 Full report (16 builds for cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
zenghuchen-gg
force-pushed
the
changed_listener_combination
branch
from
5635fa9 to
d0d96b3
Compare
zenghuchen-gg
force-pushed
the
changed_listener_combination
branch
from
5c1ee05 to
0cbab35
Compare
andy31415
reviewed
Oct 17, 2025
src/data-model-providers/codegen/CodegenDataModelProvider_Write.cpp
Outdated
Show resolved
Hide resolved
|
PR #41518: Size comparison from c87ece5 to e59c6b0 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
…rChangeListener now
|
PR #41518: Size comparison from c87ece5 to 1e7ccca Full report (8 builds for cc32xx, nrfconnect, qpg, realtek, stm32)
|
|
PR #41518: Size comparison from c87ece5 to 140f775 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
|
PR #41518: Size comparison from c87ece5 to 7aab8cd Full report (16 builds for cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
zenghuchen-gg
force-pushed
the
changed_listener_combination
branch
2 times, most recently
from
f774309 to
21a9875
Compare
|
PR #41518: Size comparison from ac328e4 to 21a9875 Full report (5 builds for cc32xx, realtek, stm32)
|
zenghuchen-gg
force-pushed
the
changed_listener_combination
branch
from
21a9875 to
7371d70
Compare
|
PR #41518: Size comparison from ac328e4 to 7371d70 Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #41518 +/- ##
==========================================
- Coverage 51.04% 51.03% -0.01%
==========================================
Files 1386 1385 -1
Lines 100958 100943 -15
Branches 13061 13061
==========================================
- Hits 51534 51518 -16
- Misses 49424 49425 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
andy31415
approved these changes
Oct 20, 2025
andy31415
added
the
sdk-maintainer-approved
|
PR #41518: Size comparison from 685d31a to 0d3e000 Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
bzbarsky-apple
approved these changes
Oct 20, 2025
chip::app:: prefix is redundant because the function is defined inside chip::app namespace Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #41518: Size comparison from 685d31a to f46717b Full report (22 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
|
PR #41518: Size comparison from c1f377d to c63eb54 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41518: Size comparison from c38154b to 8d1395c Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
zenghuchen-gg
added a commit
to zenghuchen-gg/connectedhomeip
that referenced
this pull request
Oct 21, 2025
project-chip#41518) * [*ChangeListener] replace AttributesChangedListener with ProviderChangeListener AttributesChangedListener is a duplicate of ProviderChangeListener and there is no need to keep both. Since AttributesChangedListener is for ember and ProviderChangeListener is from DataModel, it makes more sense to keep ProviderChangeListener. * Restyled by clang-format * Restyled by gn * add direct dependency on data-model-provider * Update comment to reflect the correct usage * Remove ContextAttributesChangeListener, which is a wrapper of ProviderChangeListener now * Remove ContextAttributesChangeListener in server-cluster-shim * Restyled by clang-format * Replace AttributesChangedListener with ProviderChangedListener in ember functions * Restyled by clang-format * include header needed for Providerchangedlistener * Apply suggestions from code review chip::app:: prefix is redundant because the function is defined inside chip::app namespace Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * [no-ccache] * [no-ccache] --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Andrei Litvin <andy314@gmail.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Sergio Soares <sergiosoares@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
…geListener
Summary
AttributesChangedListener is a duplicate of ProviderChangeListener and there is no need to keep both. Since AttributesChangedListener is for ember and ProviderChangeListener is from DataModel, it makes more sense to keep ProviderChangeListener.
Testing
Tested manually by building nordic light examples and the image size is reduced by 100 bytes.
Existing CI is expected to cover that things still work (no extra CI tests are needed/added)