Cleanup Linux platform source includes #20362
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
Linux platform is unconditionally including bluez source files. But this only makes sense if ble is enabled. Configure includes to be conditional on this.
…ble_wifi Linux platform is unconditionally including the WiFI commissioning driver source files. But this only makes sense if wifi is enabled. Configure includes to be conditional on this.
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kghost,
kpschoedel and
lazarkov
pullapprove
bot
requested review from
tehampson,
turon,
vijs,
vivien-apple,
wbschiller,
woody-apple,
xylophone21,
yufengwangca and
yunhanw-google
|
PR #20362: Size comparison from 126f6b2 to f89243e Increases (3 builds for cc13x2_26x2)
Decreases (14 builds for cc13x2_26x2, efr32, k32w, nrfconnect, p6, telink)
Full report (26 builds for cc13x2_26x2, cyw30739, efr32, k32w, mbed, nrfconnect, p6, telink)
|
Network commissioning source is unconditionally included in the Linux platform. But this only makes sense for commissionable apps that actually use this code. This commit moves these source includes to the apps and also fixes a build error that is arising from blindly assigning size_t to a uint8_t.
But, use static_cast<uint8_t> to fix build warning
msandstedt
force-pushed
the
cleanup-source-includes
branch
from
f89243e to
cf6500e
Compare
|
PR #20362: Size comparison from 126f6b2 to cf6500e Increases (1 build for efr32)
Decreases (2 builds for esp32, telink)
Full report (28 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, mbed, nrfconnect, p6, telink)
|
This allows general and ethernet network commissioning source to be conditionally included in platform configuration.
msandstedt
force-pushed
the
cleanup-source-includes
branch
from
2d7c866 to
bc3bd1e
Compare
|
PR #20362: Size comparison from 36ff674 to 8d62f0b Increases (3 builds for cc13x2_26x2, efr32, linux)
Decreases (3 builds for esp32, linux, telink)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
Damian-Nordic
approved these changes
Jul 6, 2022
andy31415
approved these changes
Jul 6, 2022
github-actions bot
pushed a commit
that referenced
this pull request
Jul 7, 2022
* Make bluez source include conditional on chip_enable_ble Linux platform is unconditionally including bluez source files. But this only makes sense if ble is enabled. Configure includes to be conditional on this. * Make WiFi commissioning driver source include conditional on chip_enable_wifi Linux platform is unconditionally including the WiFI commissioning driver source files. But this only makes sense if wifi is enabled. Configure includes to be conditional on this. * Move Network Commissioning driver source includes into app projects Network commissioning source is unconditionally included in the Linux platform. But this only makes sense for commissionable apps that actually use this code. This commit moves these source includes to the apps and also fixes a build error that is arising from blindly assigning size_t to a uint8_t. * per Damian-Nordic, revert networkIDLen to uint8_t But, use static_cast<uint8_t> to fix build warning * per Damian-Nordic, introduce chip_enable_network_commissioning This allows general and ethernet network commissioning source to be conditionally included in platform configuration. * Revert NetworkCommissioningDriver changes
woody-apple
added a commit
that referenced
this pull request
Jul 7, 2022
* Make bluez source include conditional on chip_enable_ble Linux platform is unconditionally including bluez source files. But this only makes sense if ble is enabled. Configure includes to be conditional on this. * Make WiFi commissioning driver source include conditional on chip_enable_wifi Linux platform is unconditionally including the WiFI commissioning driver source files. But this only makes sense if wifi is enabled. Configure includes to be conditional on this. * Move Network Commissioning driver source includes into app projects Network commissioning source is unconditionally included in the Linux platform. But this only makes sense for commissionable apps that actually use this code. This commit moves these source includes to the apps and also fixes a build error that is arising from blindly assigning size_t to a uint8_t. * per Damian-Nordic, revert networkIDLen to uint8_t But, use static_cast<uint8_t> to fix build warning * per Damian-Nordic, introduce chip_enable_network_commissioning This allows general and ethernet network commissioning source to be conditionally included in platform configuration. * Revert NetworkCommissioningDriver changes Co-authored-by: Michael Sandstedt <[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
Linux platform is including network commissioning code that is only relevant for commissionable apps.
Change overview
This moves BLE and WiFi includes inside the chip_enable_ble and chip_enable_wifi conditionals and moves Ethernet includes into the commissionable app code where it's being called.
Testing
Tested chip-tool and relevant app Linux builds. All succeed.