Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BlueZ in CHIP #1608

Merged
merged 3 commits into from
Jul 21, 2020
Merged

Add BlueZ in CHIP #1608

merged 3 commits into from
Jul 21, 2020

Conversation

yunhanw-google
Copy link
Contributor

-- Add BlueZ in Third-party for CHIP Linux device layer BLE manager integration

fixes #741

@CLAassistant
Copy link

CLAassistant commented Jul 15, 2020

CLA assistant check
All committers have signed the CLA.

@turon turon self-requested a review July 15, 2020 00:55
@turon
Copy link
Contributor

turon commented Jul 15, 2020

Please add initial GN support as well, or at least let's discuss the plan for it with @mspang.

@mspang
Copy link
Contributor

mspang commented Jul 15, 2020

Please add initial GN support as well, or at least let's discuss the plan for it with @mspang.

Can you provide some context on why bluez needs to be managed? Is it possible to have two copies of bluez running at the same time on the developer's system?

Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok, however needs a build fix.

Copy link
Contributor

@rwalker-apple rwalker-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woble->chipoble?

configure.ac Outdated Show resolved Hide resolved
@yunhanw-google
Copy link
Contributor Author

Please add initial GN support as well, or at least let's discuss the plan for it with @mspang.

Can you provide some context on why bluez needs to be managed? Is it possible to have two copies of bluez running at the same time on the developer's system?

In the past several year, since bluez5.44, the bluez BLE dbus APIs starts to become ready for public experimental use, and experience continuous changes on API behavior and parameters, bugs are introduced in recent years. When engineers, including me, add fix in bluez upstream, there is only sanity and manual test to verify if this fix work, and then those fix easily break other features in lack of comprehensive unit and integration test. Different ubuntu, debian, embedded linux, chrome os I have worked before are using different version of BlueZ, for example, chrome os is still using bluez5.44, debian(tesing/experimental) are using 5.50, other different ubuntu OS are using various bluez version. Currently our qa teams proves 5.47 and 5.48 are pretty stable on BLE APIs, for example, 5.45,5.46 have several ble stability bugs we found. Therefore we lock down the commit in 5.48 in Weave. We would also plan BlueZ 5.50 or latest after enough tests have been done.

BlueZ is using bluetoothd daemon, currently in this pr, it is compiling in linux device layer, but not running. When testing it, developers usually kill the system's bluetoothd process, and run the compiled bluetoothd during testing. In weave, we have automated this test procedure for weave ble integration test via virtual btvirt, the bluez bt emulator generated from BlueZ, note os system itself is not including this btvirt tool, and these weave over BlueZ tests are running during presubmit and postsubmit CI. Usually in weave, at default, these ble tests are at default disabled. developer locally don't need run these weave over BlueZ test, when developer touched these codes, they need to run the automate test.

That's why we need to compile blueZ. It is not possible to have two BlueZ bluetoothd running at the same time in same host now.

Btw, in project cirque, we also are exploring how to bring up multiple bluetoothd across multiple docker in same host for BlueZ BLE virtualization in docker, which would need some additional changes in blueZ upstream.

@yunhanw-google
Copy link
Contributor Author

Seems ok, however needs a build fix.

Hi, Andrei

Yes, I am fixing them!

Best wishes
Yunhan

@mspang
Copy link
Contributor

mspang commented Jul 15, 2020

Please add initial GN support as well, or at least let's discuss the plan for it with @mspang.

Can you provide some context on why bluez needs to be managed? Is it possible to have two copies of bluez running at the same time on the developer's system?

In the past several year, since bluez5.44, the bluez BLE dbus APIs starts to become ready for public experimental use, and experience continuous changes on API behavior and parameters, bugs are introduced in recent years. When engineers, including me, add fix in bluez upstream, there is only sanity and manual test to verify if this fix work, and then those fix easily break other features in lack of comprehensive unit and integration test. Different ubuntu, debian, embedded linux, chrome os I have worked before are using different version of BlueZ, for example, chrome os is still using bluez5.44, debian(tesing/experimental) are using 5.50, other different ubuntu OS are using various bluez version. Currently our qa teams proves 5.47 and 5.48 are pretty stable on BLE APIs, for example, 5.45,5.46 have several ble stability bugs we found. Therefore we lock down the commit in 5.48 in Weave. We would also plan BlueZ 5.50 or latest after enough tests have been done.

BlueZ is using bluetoothd daemon, currently in this pr, it is compiling in linux device layer, but not running. When testing it, developers usually kill the system's bluetoothd process, and run the compiled bluetoothd during testing.

Ok, that's what I feared, you can't really solve this problem by simply building a new bluez inside CHIP; running it interferes with the system install.

We might consider just providing instructions on installing a backport and just asserting that the version is OK. It sounds like Debian testing/gLinux are new enough, and CrOS we can fix ourselves..

In weave, we have automated this test procedure for weave ble integration test via virtual btvirt, the bluez bt emulator generated from BlueZ, note os system itself is not including this btvirt tool, and these weave over BlueZ tests are running during presubmit and postsubmit CI. Usually in weave, at default, these ble tests are at default disabled. developer locally don't need run these weave over BlueZ test, when developer touched these codes, they need to run the automate test.

That's why we need to compile blueZ. It is not possible to have two BlueZ bluetoothd running at the same time in same host now.

Btw, in project cirque, we also are exploring how to bring up multiple bluetoothd across multiple docker in same host for BlueZ BLE virtualization in docker, which would need some additional changes in blueZ upstream.

@yunhanw-google yunhanw-google force-pushed the master branch 2 times, most recently from f8a592c to 84e7021 Compare July 15, 2020 18:16
@yunhanw-google
Copy link
Contributor Author

yunhanw-google commented Jul 15, 2020

"Ok, that's what I feared, you can't really solve this problem by simply building a new bluez inside CHIP; running it interferes with the system install.

We might consider just providing instructions on installing a backport and just asserting that the version is OK. It sounds like Debian testing/gLinux are new enough, and CrOS we can fix ourselves..

It will not interfere with the system install, it just compiled it inside CHIP device layer automatically, and not install in system path. As I mentioned, if you don't run ble related test, you don't need touch it, which would not impact you. If you run it, Chip BlueZ support would automatically help you. Besides, if you compile chip in docker, and run bluetoothd in docker, it would not impact/interfere your system blueZ. The docker installation/validation/test would be recommended way in presubmit and local validation to provide the unified/controllable CHIP environment for developer/tester for the IOT world.

In github CHIP docker, there is no bluez inside, there is no interference at all from that point currently.

Debian testing/glinux's 5.50 is not working as we expect, we need to lock down specific commit.

We also added bluez in weave, from internal or external partners, we don't hear the related complains on the bluetoothd interference issue. From the experience, developer/tester bring up woble test, and run it smoothly.

@mspang
Copy link
Contributor

mspang commented Jul 15, 2020

"Ok, that's what I feared, you can't really solve this problem by simply building a new bluez inside CHIP; running it interferes with the system install.

We might consider just providing instructions on installing a backport and just asserting that the version is OK. It sounds like Debian testing/gLinux are new enough, and CrOS we can fix ourselves..

It will not interfere with the system install, it just compiled it inside CHIP device layer automatically, and not install in system path. As I mentioned, if you don't run ble related test, you don't need touch it, which would not impact you. If you run it, Chip BlueZ support would automatically help you. Besides, if you compile chip in docker, and run bluetoothd in docker, it would not impact/interfere your system blueZ. The docker installation/validation/test would be recommended way in presubmit and local validation to provide the unified/controllable CHIP environment for developer/tester for the IOT world.

In github CHIP docker, there is no bluez inside, there is no interference at all from that point currently.

Debian testing/glinux's 5.50 is not working as we expect, we need to lock down specific commit. CROS has stopped bluez support, and reject all upgrade or fix from bluez 5.44.

Does it work with bluez's HEAD? If not, we have a future problem in addition to the problems you've listed already.

We also added bluez in weave, from internal or external partners, we don't hear the related complains on the bluetoothd interference issue. From the experience, developer/tester bring up woble test, and run it smoothly.

I thought you said you had to stop the OS daemon to run the local one? That's the interference I was referring to.

@yunhanw-google
Copy link
Contributor Author

Per offline chat with @mspang, we agree to build bluez in chip, I would try to work with @mspang for gn integration, i will open another ticket there.

-- Add BlueZ in Third-party for CHIP Linux device layer BLE manager integration

Issue: project-chip#741
Test: local compilation
@woody-apple
Copy link
Contributor

@github-actions
Copy link

Size increase report for "gn_nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "gn_linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@woody-apple
Copy link
Contributor

@saurabhst @jelderton ?

@github-actions
Copy link

Size increase report for "nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.out and ./pull_artifact/chip-nrf52840-lock-example.out:

sections,vmsize,filesize
.debug_str,0,28
.debug_macro,0,6
[Unmapped],0,-2


@github-actions
Copy link

Size increase report for "linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-standalone-demo.out and ./pull_artifact/chip-standalone-demo.out:

sections,vmsize,filesize
.debug_str,0,28
.debug_macro,0,6
[Unmapped],0,-2


@github-actions
Copy link

Size increase report for "esp32-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize


@woody-apple woody-apple merged commit e1d448c into project-chip:master Jul 21, 2020
jmartinez-silabs pushed a commit to SiliconLabs/matter that referenced this pull request Mar 18, 2024
…y-pick:Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

Merge in WMN_TOOLS/matter from cherry-pick/default_ble_discovery to RC_2.3.0-1.3-alpha.3

Squashed commit of the following:

commit 20312119b856eacabcde4beec9703a8dbb880957
Author: Junior Martinez <Junior.Martinez@silabs.com>
Date:   Tue Mar 5 20:39:44 2024 +0000

    Pull request project-chip#1608: Cherry-pick:Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

    Merge in WMN_TOOLS/matter from cherry-pick/ble_extend_adjustment_discovery_timeout to RC_2.3.0-1.3

    Squashed commit of the following:

    commit 5bcef0704cf5b2c29657f0cd433260fab599c502
    Author: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>
    Date:   Mon Feb 26 08:53:11 2024 -0500

        Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

        * add build arg to enable BLE extended advertisement feature

        * Set a increased based value for the Discovery timeout when BLE extended advertising is set
rcasallas-silabs pushed a commit to rcasallas-silabs/connectedhomeip that referenced this pull request Jun 20, 2024
…y-pick:Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

Merge in WMN_TOOLS/matter from cherry-pick/default_ble_discovery to RC_2.3.0-1.3-alpha.3

Squashed commit of the following:

commit 20312119b856eacabcde4beec9703a8dbb880957
Author: Junior Martinez <Junior.Martinez@silabs.com>
Date:   Tue Mar 5 20:39:44 2024 +0000

    Pull request project-chip#1608: Cherry-pick:Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

    Merge in WMN_TOOLS/matter from cherry-pick/ble_extend_adjustment_discovery_timeout to RC_2.3.0-1.3

    Squashed commit of the following:

    commit 5bcef0704cf5b2c29657f0cd433260fab599c502
    Author: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>
    Date:   Mon Feb 26 08:53:11 2024 -0500

        Adjust the default discovery timeout when BLE extend announcement is enabled. (project-chip#32300)

        * add build arg to enable BLE extended advertisement feature

        * Set a increased based value for the Discovery timeout when BLE extended advertising is set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[linux] Linux DeviceLayer needs BleManager
9 participants