[ble] Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED #21582
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
If CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED is set, which turns out to be true on Zephyr, NXP and, conditionally, ESP32 platforms, the BLE manager will not start BLE advertising if IsFullyProvisioned() method of the ConnectivityManager class returns true. The problem is that the method seems obsolete and the proper logic is already in place in Server.cpp, so we should align the platforms and remove the legacy code. Signed-off-by: Damian Krolik <[email protected]>
Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED and the associated code from platforms that never set it. Signed-off-by: Damian Krolik <[email protected]>
…ED from K32 Signed-off-by: Damian Krolik <[email protected]>
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
dhrishi,
electrocucaracha,
emargolis,
erjiaqing,
franck-apple,
gjc13,
harsha-rajendran,
hawk248,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kghost,
kpschoedel,
lazarkov,
LuDuda,
mlepage-google and
msandstedt
pullapprove
bot
requested review from
tecimovic,
tehampson,
vijs,
vivien-apple,
wbschiller,
woody-apple,
xylophone21 and
yufengwangca
|
PR #21582: Size comparison from 03778b2 to 472f778 Increases (5 builds for bl602, cc13x2_26x2, cyw30739, telink)
Decreases (3 builds for cc13x2_26x2, k32w)
Full report (38 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, p6, telink)
|
|
PR #21582: Size comparison from 03778b2 to e6342ac Increases (4 builds for bl602, cyw30739, telink)
Decreases (13 builds for esp32, k32w, nrfconnect)
Full report (55 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
dhrishi
approved these changes
Aug 3, 2022
markaj-nordic
approved these changes
Aug 3, 2022
LuDuda
approved these changes
Aug 3, 2022
tcarmelveilleux
approved these changes
Aug 3, 2022
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
…VISIONED (project-chip#21582) * [zephyr] Remove usage of IsFullyProvisioned() in BLE manager If CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED is set, which turns out to be true on Zephyr, NXP and, conditionally, ESP32 platforms, the BLE manager will not start BLE advertising if IsFullyProvisioned() method of the ConnectivityManager class returns true. The problem is that the method seems obsolete and the proper logic is already in place in Server.cpp, so we should align the platforms and remove the legacy code. Signed-off-by: Damian Krolik <[email protected]> * Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED and the associated code from platforms that never set it. Signed-off-by: Damian Krolik <[email protected]> * Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED from K32 Signed-off-by: Damian Krolik <[email protected]> * Restyled by clang-format * Remove CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONED from ESP32 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.
Problem
BLE Managers contain obsolete code for turning off the BLE advertising when the device is fully provisioned, guarded by
CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONEDconfig.Proper code for this purpose is already implemented in
ServerandCommissioningWindowManager, so the one in BLE managers does not seem useful at all.Also, the config is disabled on most platforms except NXP, Zephyr and, conditionally, ESP32.
Change overview
CHIP_DEVICE_CONFIG_CHIPOBLE_DISABLE_ADVERTISING_WHEN_PROVISIONEDon all platforms.Testing
Tested that commissioning still works correctly on nRF Connect platform and the advertising is stopped after that.