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

Support to Bluetooth RPC Host for nRF54H20 #15634

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

ppryga-nordic
Copy link
Contributor

@ppryga-nordic ppryga-nordic commented Jun 3, 2024

There is a possibility to use complete Bluetooth stack on e.g. nRF5340 network core, meaning Host and Controller run in same core. An application communicates with the stack over Bluetooth RPC subsystem that wraps Bluetooth API into RPC.

The PR adds possibility to use the same approach for samples running on nRF54H20 SOC.
To make it easy, the nordic-rcp-host snippet was added.
Also a peripheral_uart Kconfig.sysbuild was modified to make possible to select radio domain target sample application.

One additional change was done to Bluetooth RPC subsystem, handling of settings initialization on remote. That change was done because former implementation required to increase main thread stack size.

@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jun 3, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jun 3, 2024

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
test-fw-nrfconnect-ble X
test-fw-nrfconnect-ble_samples X
test-sdk-dfu X
test-sdk-find-my X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@ppryga-nordic ppryga-nordic changed the title [DNM] Support to Bluetooth RPC Host for nRF54H20 Support to Bluetooth RPC Host for nRF54H20 Jun 3, 2024
@ppryga-nordic ppryga-nordic requested review from hubertmis and removed request for tejlmand, carlescufi, alwa-nordic and jori-nordic June 3, 2024 12:20
snippets/nordic-rpc-host/README.rst Outdated Show resolved Hide resolved
snippets/nordic-rpc-host/snippet.yml Outdated Show resolved Hide resolved
@ppryga-nordic ppryga-nordic added this to the 2.7.0 milestone Jun 3, 2024
@ppryga-nordic ppryga-nordic force-pushed the github-rpc-host-for-nrf54h20 branch 2 times, most recently from d05cc93 to 539f5f3 Compare June 3, 2024 15:22
@ppryga-nordic
Copy link
Contributor Author

Fixed licenses and added CODEOWNERS entry for the new snippet.

Copy link
Contributor

@hubertmis hubertmis left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@dchat-nordic dchat-nordic left a comment

Choose a reason for hiding this comment

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

The code itself looks good, but according to NCSDK-27179 the default firmware for network core should be ipc_radio.
The ipc_radio supports the rpc configuration, have you tried to use it instead of rpc_host sample?

#include "nrf54h20dk_nrf54h20-mem-map-move.dtsi"

&cpurad_rw_partitions {
status = "okay";
Copy link
Contributor

Choose a reason for hiding this comment

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

too many indents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines 9 to 15
config NRF_IPC_RADIO_RPC_HOST
bool "Enable building RPC Host for Radio domain"
help
The option enables building sample rpc_host application for Radio
domain. This sysbuild configuration option should be selected only if
an Application core application is build with CONFIG_BT_RPC_STACK
enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need yet another kconfig for the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my bad, I should have added description to NRF_DEFAULT_RPC_HOST.
Anyway the config was removed. Since there is change in approach and ipc_radio application will be used instead of rpc_host part of the content in this file was extracted to sysbuild.conf and sysbuild_bt_rpc.conf.

@ppryga-nordic
Copy link
Contributor Author

Added nR53 support to new snippet. Fixed build issue caused by wrong snippet name in sample.yaml.

@ppryga-nordic
Copy link
Contributor Author

ppryga-nordic commented Jun 4, 2024

changed sample.yaml to workaround twister issue: missing CONFIG_ at the beginning of entries for extra_configs.

@ppryga-nordic
Copy link
Contributor Author

ppryga-nordic commented Jun 5, 2024

PR is blocked by failing tests that require update in SDC library.

Waiting for: #15740

The CI failure was solved by PR: #15713.

There is experimental RPC wrapper for Bluetooth API.
To make possible use of BLE samples with nRF54h20 and the RPC API
there is required a settings partition for Radio core.
That means current memory map has to be changed to make room for new
settings partition.
To make an application to use the Bluetooth rpc subsystem there is
added CONFIG_BT_RPC_STACK. It enables BT RPC host client wrapper to be
used instead of regular Bluetooth API.

With these changes end user needs to add -S nrf-bt-rpc to west call
to build a compatible BLE sample for BT RPC host.

To make clean complete build for both Radio and App cores addtitional
changes are required in sysbuild configuration. These will be added
in follow up commits.

The snippet can be also used for builds for nRF5340, though there is
no need for memory map reorganization.

Signed-off-by: Piotr Pryga <[email protected]>
There were no simple way to use sysbuild for building BLE samples
with rpc_host running on Radio core. Add required configuration to
sysbuild. Now an application can be targeted to be build with
rpc_host by adding NRF_DEFAULT_RPC_HOST=y to Kconfig.sysbuild.

Signed-off-by: Piotr Pryga <[email protected]>
Add possibility to build the sample application targeted to nrf54h20
with use of Bluetooth API wrapped with RPC Host client. The Radio
core application in such case must be ipc_radio with enabled Bluetooth
RPC host. That reqires change to sysbuild configuration of the
sample application.

One note, the Bluetooth RPC subsystem requires to have same Bluetooth
device name on obth ends of RPC communication, so there is used
FILE_SUFFIX=bt_rpc and provided sysbuild configuration files that
are include when the suffix is used in build call.

To build the sample with enabled Bluetooth RPC subsys use:
'west build -b nrf54h20dk/nrf54h20/cpuapp --sysbuild -p
 -S nordic-bt-rpc -- -DFILE_SUFFIX=bt_rpc'

Signed-off-by: Piotr Pryga <[email protected]>
The former implementation of settings initialization was based on
a command send by client by rpc_settings_set() function. That function
was called by stetings subsustem during settings_load() execution.
That together created deep execution call stack that required extended
main thread stack size.

To avoid the change to stack size, the implementation is changed,
to send the settings load command to host if CONFIG_SETTINGS is
eanbled. The command is send directly from bt_enable() function
in bt_rpc_gap_client.c.

Signed-off-by: Piotr Pryga <[email protected]>
@guwa
Copy link
Contributor

guwa commented Jun 5, 2024

BLE CI has a setup failure due to commit message check. It should not block the PR as long as the test job has passed

@rlubos rlubos merged commit ca4f64c into nrfconnect:main Jun 5, 2024
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants