Non concurrent mode now sends connect network response as per spec #31739
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
|
PR #31739: Size comparison from f63b505 to 202888d Increases (2 builds for mbed, stm32)
Full report (4 builds for cc32xx, mbed, stm32)
|
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,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs and
jmeg-sfy
|
PR #31739: Size comparison from 5677529 to 491f160 Increases (70 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
Decreases (4 builds for linux)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
src/app/clusters/network-commissioning/network-commissioning.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/network-commissioning/network-commissioning.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/network-commissioning/network-commissioning.cpp
Outdated
Show resolved
Hide resolved
tcarmelveilleux
requested changes
Feb 9, 2024
There was a problem hiding this comment.
- Return all existing public API and layer new features in addition to old names, even if they go to the same place
- Refactor direct access to
DeviceLayer::ConnectivityMgr().GetBleLayer()->mState - This seems risky/brittle given the breadth of changes. Please add an integration test that does not rely on multiple manual tests people have to keep running to prove the functioning of this feature.
- Make BLEManagerImpl.cpp error handling non-build-conditional wherever they apply uniformly
- Justify the move to
mServer->GetBleLayerObject()->Shutdown();and how it is possible to guarantee that after a mid-commissioning failure, the window could still be open for the remainder of the time, to allow more BLE connections to support finishing the commissioning without reboot of the device.
bzbarsky-apple
approved these changes
Feb 9, 2024
There was a problem hiding this comment.
Approving, assuming @tcarmelveilleux's issues are addressed.
Close GATT so that BlueZ will disconnect. Add kOperationalNetworkStarted event type
|
tcarmelveilleux
approved these changes
Feb 14, 2024
swan-amazon
approved these changes
Feb 14, 2024
|
PR #31739: Size comparison from 7113c98 to f58ab1f Increases (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
Decreases (5 builds for bl702l, linux)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
raul-marquez-csa
pushed a commit
to raul-marquez-csa/connectedhomeip
that referenced
this pull request
Feb 16, 2024
…roject-chip#31739) * Update Non-concurrent mode to latest spec (project-chip#31660) * Handle unused variable (project-chip#31660) * Restyled by clang-format * Trigger from BTP, use regular member (project-chip#31660) * Fix override (project-chip#31660) * Restyled by whitespace * Restyled by clang-format * Use generic OperationalNetworkStarted (project-chip#31660) * Replace variable with mState (project-chip#31660) Use mState instead of mTerminateOnPacketTxComplete * Sorted namespace out (project-chip#31660) * Restyled by clang-format * Address review comments (project-chip#31660)ffffffffffffffffffff Close GATT so that BlueZ will disconnect. Add kOperationalNetworkStarted event type * Restyled by clang-format * Access BleLayer mState through function (project-chip#31660) * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]>
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 #31660, Implement Non-Concurrent Spec Change #8728 in the SDK.
This change now sends the ConnectNetworkResponse(Success) on the commissioning channel before closing it and enabling the operation channel as required by the https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/8728 spec change.
This changes the commissioner and Linux Raspberry Pi lighting app commissionee. The Non-concurrent mode in lighting app requires examples/lighting-app/linux/include/CHIPProjectAppConfig.h to be modified as follows to disable concurrent connection mode.
#define CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTION 0This has been tested with the
CHIP_DEVICE_CONFIG_SUPPORTS_CONCURRENT_CONNECTIONset to zero and one, the passing test logs are attached.chip_output_concurrent_mode.txt
light-app-concurrent.txt
light-app-non-concurrent.txt
chip_output_non-concurrent_mode.txt