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

Bluetooth: mesh: Add split build configuration for sample #76161

Conversation

jori-nordic
Copy link
Collaborator

@jori-nordic jori-nordic commented Jul 22, 2024

Add a build configuration that enables mesh when building one of the hci_something applications.

Also direct users to it from the README.

Fixes #75614

Add a build configuration that enables mesh when building one of the
hci_something applications.

Also direct users to it from the README.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
@matt-wood-ct
Copy link

+1

Copy link
Collaborator

@omkar3141 omkar3141 left a comment

Choose a reason for hiding this comment

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

Additionally,

  1. It is worth to add nrf52840dk_nrf52840.overlay to "hci_spi"

  2. and nrf52840dk_nrf52840.overlay as "nrf52840dk_nrf52840_hci_spi.overlay" to mesh sample.

This along with doc changes would allow users to evaluate this properly.

To run the application on an :ref:`nrf5340dk_nrf5340`, a Bluetooth controller application
must also run on the network core. The :ref:`bluetooth-hci-ipc-sample` sample
application may be used. Build this sample with configuration
Additional kconfig options need to be set on the Bluetooth controller
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is worth to set this off as a subsection in sample docs (and move it as a last para):

Suggested change
Additional kconfig options need to be set on the Bluetooth controller
Bluetooth controller on different device:
=========================================
Additional kconfig options need to be set on the Bluetooth controller

To run the application on an :ref:`nrf5340dk_nrf5340`, a Bluetooth controller application
must also run on the network core. The :ref:`bluetooth-hci-ipc-sample` sample
application may be used. Build this sample with configuration
Additional kconfig options need to be set on the Bluetooth controller
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes should be added in all mesh samples in README.rst file (mesh, mesh_demo, provisioner) since this is the common configuration issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can you please do it then?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy-paste this in other 2 files? :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought about this earlier.

Perhaps we should have a common RST fragment that tells how to run sample using the split config, and include that in both readmes.

I feel, after following the instructions in this sample and then by using hci_spi sample, users should ideally be able to fully evaluate this, at least on 52 platform (other platform support could be enabled in hci_spi by respective contributors) without having to add any extra magic settings.

@@ -0,0 +1,18 @@
# This configuration fragment enables mesh support for a controller
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think I agree this file should be located in mesh samples. This is part of controller configuration. This should be as an extra configuration file for controller sample.
I kind of disagree to make it as platform overlay since this is not related to any chip. Probably as snippet to be able to use in any controller sample but up to implementer?

However, it is not clear to me how to synchronize a number of advertiser instances. I'm not sure these changes are fully correct since mesh might use a different number of instances (depending on other options): https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/mesh/Kconfig#L95-L151

It works in your demonstration, until mesh didn't try to use an advertiser that doesn't exist in controller.
Since there is no clear parameter synchronization mechanism in Zephyr between cores or chips, it is worth at least to mention it in documentation in appropriate subclause of README.rst file.

@alxelax
Copy link
Collaborator

alxelax commented Jul 22, 2024

Hi @jori-nordic, please apply my patch: alxelax@ea3784f

@jori-nordic
Copy link
Collaborator Author

I think documenting this properly is better handled by the mesh experts 📜🧙

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLE Mesh Sample using HCI SPI controller
7 participants