Follow-up Persistent Subscription tests #31310
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
github-actions
bot
added
examples
scripts
integrations
platform
controller
gn
test driver
docker
labels
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
harsha-rajendran and
hawk248
pullapprove
bot
requested review from
younghak-hwang,
yufengwangca and
yunhanw-google
|
PR #31310: Size comparison from e241e32 to 2e1ba11 Increases above 0.2%:
Increases (1 build for linux)
Decreases (1 build for linux)
Full report (38 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
|
PR #31310: Size comparison from e241e32 to f260377 Full report (5 builds for cc32xx, mbed, qpg)
|
|
PR #31310: Size comparison from e241e32 to 54445a1 Increases above 0.2%:
Increases (11 builds for linux)
Decreases (2 builds for efr32, linux)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
src/controller/python/test/test_scripts/subscription_resumption_capacity_test_ctrl1.py
Outdated
Show resolved
Hide resolved
yunhanw-google
approved these changes
Jan 10, 2024
|
PR #31310: Size comparison from e241e32 to fd6df24 Increases above 0.2%:
Increases (39 builds for bl602, bl702, bl702l, cc32xx, esp32, linux, psoc6, telink)
Decreases (43 builds for bl702l, cc13x4_26x4, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, qpg, telink)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
wqx6
force-pushed
the
subscription-resumption-test
branch
from
fd6df24 to
ab51c9d
Compare
|
PR #31310: Size comparison from 418b598 to ab51c9d Increases above 0.2%:
Increases (11 builds for linux)
Decreases (2 builds for efr32, linux)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
| @@ -1670,6 +1670,7 @@ endpoint 0 { | |||
| ram attribute clusterRevision default = 1; | |||
|
|
|||
| handle command OpenCommissioningWindow; | |||
| handle command OpenBasicCommissioningWindow; | |||
There was a problem hiding this comment.
Why is this being enabled? Where is it used? And why?
There was a problem hiding this comment.
Just to make the device can be commissioned to the second fabric easily. If using the two controller (devCtrl and devCtrl2) in one same container/host to execute this test, the accessory can still resolve the address for first controller even if the first controller is shutdown by 'self.devCtrl.Shutdown()'. So I used two containers for two fabrics. If using OpenBasicCommissioningWindow from the first controller, the second could use the original setup pincode to commission the server device.
There was a problem hiding this comment.
thanks for clarification
There was a problem hiding this comment.
Would follow-up it if it is still concerning. thanks
There was a problem hiding this comment.
So is the issue here that you can't communicate the right setup code from OpenCommissioningWindow to the other container?
The problem here is that this example was explicitly not enabling OpenCommissioningWindow because it's meant to be an example of how you actually do a real app. We are not messing that up because of our testing limitations. We should really not be doing that @wqx6 @yunhanw-google
joonhaengHeo
approved these changes
Jan 11, 2024
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #31310: Size comparison from 418b598 to 68d2a84 Increases above 0.2%:
Increases (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Decreases (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
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.
Fixes #30971
In this PR: