Add persistence logic to CurrentConnections list attribute in PushAVTransport cluster#42837
Merged
mergify[bot] merged 16 commits intoproject-chip:masterfrom Feb 11, 2026
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds persistence for the CurrentConnections attribute in the PushAVTransport cluster using the AttributePersistenceProvider. The changes include adding persistence logic for creating and removing connections, and loading them on initialization. While the overall direction is correct, I've found a few issues: a critical bug in the loading logic that prevents data from being correctly restored, a high-severity issue where updates to connections are not persisted, and a medium-severity issue with redundant code. I've provided suggestions to fix these issues.
pidarped
force-pushed
the
PushAVPersistence
branch
from
dec42e6 to
f69d06d
Compare
|
PR #42837: Size comparison from ac0beeb to 6d48cee Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32, telink)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
6d48cee to
59f777c
Compare
|
PR #42837: Size comparison from 5eba912 to 59f777c Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32, telink)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
712e8c8 to
083ae1b
Compare
|
PR #42837: Size comparison from 2a48c1e to 083ae1b Full report (9 builds for cc13x4_26x4, cc32xx, realtek, stm32)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
083ae1b to
dd49afc
Compare
|
PR #42837: Size comparison from 2a48c1e to dd49afc Full report (11 builds for cc13x4_26x4, cc32xx, qpg, realtek, stm32)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
dd49afc to
ff7fb42
Compare
|
PR #42837: Size comparison from 2a48c1e to ff7fb42 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32, telink)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
ff7fb42 to
7e72a04
Compare
|
PR #42837: Size comparison from bea86d8 to 2a69df0 Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32, telink)
|
PushAVTransportCluster. * Added Store and Load functions for persistence of TLV encoded currentConnections list attribute. * Updated logic to store during allocation/deallocation of PushAV transport. * Load persisted attribute during cluster Init. * Added test case in TestPushAVStreamTransportCluster.cpp: ** AllocatePushTransport and store currentConnections ** Restart server. ** Validate loading of persisted currentConnection ** DeallocatePushAVTransport and store currentConnections. ** Restart server. ** Validate updated contents of persisted list.
pidarped
force-pushed
the
PushAVPersistence
branch
from
2a69df0 to
8eecc4a
Compare
|
PR #42837: Size comparison from 3a19d52 to 8eecc4a Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds persistence logic for the CurrentConnections list attribute in the PushAVTransport cluster. The implementation moves the responsibility for persisting connection state from the application delegate to the cluster itself, using the SafeAttributePersistenceProvider.
Changes:
- Adds direct persistence of CurrentConnections attribute using SafeAttributePersistenceProvider
- Implements StoreCurrentConnections() and LoadCurrentConnections() methods in the cluster logic
- Adds configuration constants for maximum push transports and zones
- Updates all tests to initialize the persistence provider properly
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/core/CHIPConfig.h | Adds CHIP_CONFIG_MAX_NUM_PUSH_TRANSPORTS (2) and CHIP_CONFIG_MAX_NUM_ZONES (4) configuration constants |
| src/app/clusters/push-av-stream-transport-server/push-av-stream-transport-storage.h | Adds size calculations for TLV serialization of connection data structures, including constants for buffer sizing |
| src/app/clusters/push-av-stream-transport-server/PushAVStreamTransportLogic.h | Declares StoreCurrentConnections() and LoadCurrentConnections() helper methods and serialization size constants |
| src/app/clusters/push-av-stream-transport-server/PushAVStreamTransportLogic.cpp | Implements persistence logic, replacing delegate-based loading with direct cluster persistence using TLV encoding |
| src/app/clusters/push-av-stream-transport-server/PushAVStreamTransportCluster.h | Fixes typo in comment (Bitflags → BitFlags) |
| src/app/clusters/push-av-stream-transport-server/BUILD.gn | Adds dependencies on persistence modules |
| src/app/clusters/push-av-stream-transport-server/tests/TestPushAVStreamTransportCluster.cpp | Updates existing tests and adds comprehensive persistence test covering allocation, shutdown/startup cycle, and deallocation |
| src/app/clusters/push-av-stream-transport-server/tests/TestURLParsing.cpp | Updates test to initialize persistence provider properly |
|
PR #42837: Size comparison from 3a19d52 to 4c3dceb Full report (5 builds for cc32xx, realtek, stm32)
|
pidarped
force-pushed
the
PushAVPersistence
branch
from
4c3dceb to
7bc797e
Compare
- Make the persistenceProvider a member in the test fixture. - Make RemoveStreamTransportConnection return error that is handled by the caller to return a Status::Failure.
pidarped
force-pushed
the
PushAVPersistence
branch
from
cbe13ab to
2bbdeee
Compare
|
PR #42837: Size comparison from 66447c5 to 2bbdeee Full report (25 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32)
|
-Ensure to deallocate stream to cleanup after each allocateTransport request during TestURLParsing test cases.
|
PR #42837: Size comparison from 66447c5 to bfb838c Full report (25 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32)
|
marktrayer
reviewed
Feb 10, 2026
pidarped
force-pushed
the
PushAVPersistence
branch
from
e5cbdd3 to
91fc656
Compare
pidarped
force-pushed
the
PushAVPersistence
branch
from
bce7651 to
2d9e5d0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/app/clusters/push-av-stream-transport-server/PushAVStreamTransportLogic.cpp:219
- LoadPersistentAttributes() logs and ignores LoadCurrentConnections() failures but still calls PersistentAttributesLoadedCallback(). If the persisted TLV is corrupted or too large, this can leave mCurrentConnections stale/partial while the delegate re-allocates transports based on that data. Consider making LoadPersistentAttributes() clear mCurrentConnections and/or skip the delegate callback when LoadCurrentConnections() returns an error (or propagate the error up from Init()).
void PushAvStreamTransportServerLogic::LoadPersistentAttributes()
{
// Load currentConnections
LogErrorOnFailure(LoadCurrentConnections());
// Signal delegate that all persistent configuration attributes have been loaded.
TEMPORARY_RETURN_IGNORED mDelegate->PersistentAttributesLoadedCallback();
}
|
PR #42837: Size comparison from 4a1dff7 to 2d9e5d0 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
is cleared if storage read fails during LoadCurrentConnections.
|
PR #42837: Size comparison from 4a1dff7 to e69a69d Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
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
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.
Summary
Persistence logic for CurrentConnections attribute in PushAVTransport cluster.
Related issues
Testing
Added unit tests
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines