Make TlsClientManagementCommandDelegate persist data (Non-volatile) #41383
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
gmarcosb
force-pushed
the
camera-tls-client-final
branch
from
de17600 to
63cb052
Compare
gmarcosb
force-pushed
the
camera-tls-client-final
branch
2 times, most recently
from
5708c61 to
2eb94b3
Compare
…s required by spec
gmarcosb
force-pushed
the
camera-tls-client-final
branch
from
2eb94b3 to
03f6288
Compare
|
PR #41383: Size comparison from 876fcf5 to 03f6288 Increases above 0.2%:
Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41383: Size comparison from 876fcf5 to 6d6191a Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41383: Size comparison from a83d0cb to 4cb410c Increases above 0.2%:
Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #41383 +/- ##
==========================================
+ Coverage 50.97% 51.32% +0.34%
==========================================
Files 1384 1386 +2
Lines 100849 101058 +209
Branches 13064 12982 -82
==========================================
+ Hits 51412 51865 +453
+ Misses 49437 49193 -244 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmarcosb
commented
Oct 10, 2025
src/app/clusters/push-av-stream-transport-server/push-av-stream-transport-logic.cpp
Show resolved
Hide resolved
|
/gemini review |
![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 significant and well-structured refactoring to add persistence to TlsClientManagementCommandDelegate, which was previously using in-memory storage. The introduction of the IncrementingIdHelper is a good abstraction that simplifies ID management and is correctly reused in CertificateTableImpl. The overall changes align with the goal of making the feature non-volatile. I have identified one critical issue regarding a large stack allocation that could lead to stack overflows on embedded devices, and a couple of medium-severity issues to improve robustness and error handling. Addressing these points will make the implementation safer and more maintainable.
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Outdated
Show resolved
Hide resolved
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Show resolved
Hide resolved
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Outdated
Show resolved
Hide resolved
|
PR #41383: Size comparison from a83d0cb to c78836b Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
gmarcosb
force-pushed
the
camera-tls-client-final
branch
from
c78836b to
09eec0c
Compare
|
PR #41383: Size comparison from 5ca0aad to 09eec0c Full report (12 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, realtek, stm32)
|
gmarcosb
force-pushed
the
camera-tls-client-final
branch
from
09eec0c to
165efaf
Compare
pidarped
reviewed
Oct 14, 2025
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Show resolved
Hide resolved
gmarcosb
force-pushed
the
camera-tls-client-final
branch
from
165efaf to
fd45376
Compare
pidarped
reviewed
Oct 14, 2025
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Show resolved
Hide resolved
examples/all-clusters-app/all-clusters-common/src/tls-client-management-instance.cpp
Show resolved
Hide resolved
|
PR #41383: Size comparison from 41556af to fd45376 Full report (30 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
pidarped
approved these changes
Oct 14, 2025
|
PR #41383: Size comparison from 41556af to ff6d62e Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41383: Size comparison from 41556af to 540ae0c Increases above 0.2%:
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41383: Size comparison from 39b5015 to fbc6efa Full report (16 builds for cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
|
PR #41383: Size comparison from 39b5015 to d18edba Increases above 0.2%:
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41383: Size comparison from 39b5015 to 0a11fd9 Increases above 0.2%:
Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
yufengwangca
approved these changes
Oct 24, 2025
samadDotDev
approved these changes
Oct 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
As required by spec
Testing
Existing TC tests @ TC_TLSCLIENT run in CI
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