Don't remove PASE connection until after commissioning complete #17023
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
In the unlikely situation that we complete case, but fail to send the commissioning complete, this is more in line with the spec.
It's not used at all in CommissioneeDeviceProxy. See issue project-chip#16766 for CASEServer.
This is only a problem if we fail after setting the noc because we don't send a new arm failsafe, but we do send a report for the cleanup stage.
pullapprove
bot
requested review from
andy31415,
anush-apple,
austinh0,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha and
franck-apple
pullapprove
bot
requested review from
wbschiller,
woody-apple,
xylophone21 and
yufengwangca
cecille
changed the title
msandstedt
reviewed
Apr 4, 2022
woody-apple
approved these changes
Apr 4, 2022
|
PR #17023: Size comparison from c3b1810 to 571593a Increases (29 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (19 builds for cc13x2_26x2, efr32, esp32, linux, nrfconnect)
Full report (31 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
msandstedt
approved these changes
Apr 4, 2022
|
Draft while I'm sorting out the test issues with the breadcrumb - it's not getting initialized correctly, and I think the expecatations with the darwin test are incorrect. |
src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
Show resolved
Hide resolved
src/app/clusters/general-commissioning-server/general-commissioning-server.cpp
Show resolved
Hide resolved
cecille
force-pushed
the
PASE_removal_after_complete
branch
from
9033ea5 to
f3113ec
Compare
bzbarsky-apple
approved these changes
Apr 19, 2022
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
|
PR #17023: Size comparison from ff32d56 to 206e3a5 Increases (32 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (16 builds for cc13x2_26x2, esp32, linux, nrfconnect)
Full report (32 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, 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.
Problem
Commissioner closes the pase session too early - if we're commissioning, we need to hold onto it until the commissioning complete command response is received because the spec indicates that we need to go back to network configuration over pase if that command is unsuccessful.
Also stop the case server from prematurely closing the pase session. Since the case server is no longer handling the PASE removal, we can also remove the BLE component.
Also remove BLE component from CommissioneeDeviceProxy since it's completely unused right now.
Fixes #16766
Change overview
Testing