Draft enabling wifipaf on sample apps #34716
Conversation
|
Review changes with SemanticDiff. |
pullapprove
bot
requested review from
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille,
chapongatien,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
doru91,
fessehaeve,
harimau-qirex,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo and
jtung-apple
pullapprove
bot
requested review from
selissia,
sharadb-amazon,
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang and
yufengwangca
src/platform/device.gni
Outdated
| @@ -25,7 +25,7 @@ declare_args() { | |||
| chip_fake_platform = false | |||
|
|
|||
| # Include wifi-paf to commission the device or not | |||
| chip_device_config_enable_wifipaf = false | |||
| chip_device_config_enable_wifipaf = true | |||
There was a problem hiding this comment.
Do we have a reason to have wifipaf as a default?
Otherwise we should just change what the TH builds, like I see https://github.com/project-chip/connectedhomeip/blob/master/integrations/docker/images/chip-cert-bins/Dockerfile#L177
You could add a wifi-paf variant to build things like linux-x64-all-clusters-ipv6only-wifi-paf.
To get that variant working you add the variant in https://github.com/project-chip/connectedhomeip/blob/master/scripts/build/build/targets.py#L187 and make https://github.com/project-chip/connectedhomeip/blob/master/scripts/build/builders/host.py#L331 support some form of flag that makes the gn argument take some value.
There was a problem hiding this comment.
@andy31415 I don't have answer for that if it should be default or not. I just realized support was not there. Thank you for your quick answer Andrei. I will raise up this question and get back here.
There was a problem hiding this comment.
@andy31415 we are going to use the chip-cert-bins in the TH image for testing , also the way the test plan is written , it uses the same sample app for validating the commissioning flow for BLE , Wifi and on network , so I guess we would have to enable it by default ?
Let me start a conversation with other folks in slack as well
There was a problem hiding this comment.
Given that you also need a patched wpa_supplicant version to actually have working PAF support I don't think it's helpful to enable this by default. Is the TH not able to pass build arguments to the binaries it builds?
There was a problem hiding this comment.
(I also think in general it's unhelpful to treat current_os == "linux" as "you're building for development and want to enable every option under the sun". I've been thinking about adding some option like matter_enable_recommended that can be defaulted to true and then be used as the default choice for other options, so that in scenarios where I want to explicitly selected what gets turned on I can just set that one option to false, without having to see what other RPC or tracing or feature xyz options have been added recently and are now defaulting to True because I happen to build on Linux.)
There was a problem hiding this comment.
Marking as request changes until we get feedback from people familiar with PAF (maybe @ksperling-apple ?).
The question is if this is sufficient to just get PAF support on most linuxes. Do most linuxes support PAF or do we need special HW.
|
PR #34716: Size comparison from 5f6f3f1 to d0c4fd3 Increases above 0.2%:
Full report (50 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, mbed, nrfconnect, nxp, psoc6, qpg, stm32, tizen)
|
|
I modified the src/platform/device.gni file. I changed chip_device_config_enable_wifipaf = false to chip_device_config_enable_wifipaf = true, and then compiled the chip tool. During the compilation, I encountered an error. The error log is as follows: ERROR at //third_party/connectedhomeip/src/include/platform/ConnectivityManager.h:39:11: Include not allowed. Is there an issue with the way I made the modification? |
The fix does not seem to work: there is a dependency loop here: tests/raw depends on platform and vice-versa. This is more complex to fix. |
|
This pull request has been automatically marked as stale because it has not had recent activity. |
Support for wifipaf is not being enabled on the example apps on the SDK Container.
SDK container is generated by using Dockerfile:
https://github.com/project-chip/connectedhomeip/blob/master/integrations/docker/images/chip-cert-bins/Dockerfile
This PR is related / should be understood in the context of this Commit:
Add --wifipaf commission in chip-tool and apps of Linux platform (#33977). 30 Jul 2024 at 00:21
SHA: 085f57f
The initial changes done here in this PR are not supposed to the complete or even the right solution for this issue but the entry point to get help from who knew the right places that must be changed in order to get wifipaf available on the sample apps (SDK container) used by Test Harness execution.