Ble-WiFi commissioning test in CI #32889
Open
+689
−47
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
|
Right now, I'm using a docker image from my fork. After merging the changes required to create this image in the main repository, I will change the address and make this PR ready for merge. I created this PR to show a possible solution and get feedback. |
jlatusek
changed the title
jlatusek
changed the title
|
PR #32889: Size comparison from f46cdf3 to 4c71adb Decreases (1 build for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
jlatusek
force-pushed
the
ubuntu-qemu
branch
2 times, most recently
from
3848f88 to
7e54edb
Compare
|
PR #32889: Size comparison from f29ccbe to 7e54edb Decreases (1 build for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
|
PR #32889: Size comparison from f29ccbe to 03a8c1e Decreases (1 build for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
Comment on lines
381
to
383
| "pairing", "ble-wifi", TEST_NODE_ID, "Virtual_Wifi", "ExamplePassword", "20202021", "3840", ] | ||
| pairing_server_args = [ | ||
| "--ble-adapter", str(tool_hci_number)] |
There was a problem hiding this comment.
pairing code-wifi should work, no? Why is this using ble-wifi, which is not really maintained?
There was a problem hiding this comment.
@bzbarsky-apple
At that time I wasn't aware of cmd pairing code-wifi. I've change implementation to use new cmd and update everything to works with ubuntu 24.04
|
PR #32889: Size comparison from 0180ee6 to a46b697 Full report (81 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
|
PR #32889: Size comparison from 3219a5f to 88a573c Decreases (2 builds for efr32)
Full report (81 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
pullapprove
bot
requested review from
rcasallas-silabs,
ReneJosefsen,
saurabhst,
selissia,
sharadb-amazon,
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang and
yunhanw-google
|
PR #32889: Size comparison from 07789d4 to 361b07f Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #32889: Size comparison from 07789d4 to 18d38f5 Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
| @@ -106,7 +106,6 @@ SpacesInSquareBrackets: false | |||
| Standard: Cpp11 | |||
| TabWidth: 8 | |||
| UseTab: Never | |||
| InsertNewlineAtEOF: true | |||
| build \ | ||
| --copy-artifacts-to objdir-clone \ | ||
| " | ||
| # There is no enough space for running the test withouth cleaning the environment |
There was a problem hiding this comment.
Suggested change
| # There is no enough space for running the test withouth cleaning the environment | |
| # There is not enough space for running the test without cleaning the environment |
| echo "CMD:" | ||
| cat "$PROJECT_PATH/runner.sh" | ||
|
|
||
| "$PROJECT_PATH/integrations/docker/images/stage-2/chip-build-linux-qemu/run-img.sh" |
There was a problem hiding this comment.
This is pretty specific for a "run_in_vm" script. The script needs a better name.
| "ip link set dev eth-ci up", | ||
| "ip link set dev eth-ci-switch up", | ||
| ] | ||
|
|
||
| if not ble_wifi: | ||
| COMMANDS += [ | ||
| "ip link add eth-app-direct type veth peer name eth-tool-direct", |
There was a problem hiding this comment.
Please have @andy31415 review these bits; I am not very familiar with the netns stuff.
| subprocess.check_call(["ip", "netns", "exec", netns, "ip", "addr", "add", ip, "dev", dev]) | ||
|
|
||
| def start(self): | ||
| hostapd_cmd = ["ip", "netns", "exec", "tool", self._hostapd_path, self._hostapd_conf] |
There was a problem hiding this comment.
Again, this needs review from someone actually familiar with this code.
| TEST_NODE_ID, | ||
| "Virtual_Wifi", | ||
| "ExamplePassword", | ||
| "MT:-24J042C00KA0648G00", |
There was a problem hiding this comment.
Doesn't that need to be setupCode or some derivative of it instead of a hardcoded thing? Or at least document why this is being hardcoded (and e.g. is therefore hardcoding a specific discriminator).
| chiptest.linux.PrepareNamespacesForTestExecution( | ||
| context.obj.in_unshare) | ||
| chiptest.linux.PrepareNamespacesForTestExecution(context.obj.in_unshare, ble_wifi) | ||
| if ble_wifi: |
There was a problem hiding this comment.
Doesn't the non-linux case need to just fail out immediately if ble_wifi is true instead of silently not working right?
There was a problem hiding this comment.
Also needs review by someone who is not me.
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
There is no testing of commissioning with ble-wifi in the CI
Solution
Currently, there is no way to use virtual Bluetooth and wifi interfaces in CI. Github runner doesn't have
required modules to create virtual interfaces: hci_vhci and mac80211_hwsim.
The proposed solution involves creating a custom qemu image with the required modules and using it in CI.
The created qemu image is based on ubuntu:24.04 with a custom kernel and Bluez installed. The patch used to make the custom kernel is required to ensure that two devices can discover each other every time.
Tests use their dbus-daemon instance to separate them from the system dbus. This will allow developers
to run tests on their main system without needing a virtual machine. Unfortunately, there is no way to run two instances of Bluez simultaneously and select hci interfaces for each. That's why there is a proxy between the system and test buses, which forwards Bluez messages between them.
Changes