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

serial: Add UART over Bluetooth LE #69881

Merged
merged 8 commits into from Mar 22, 2024

Conversation

ubieda
Copy link
Collaborator

@ubieda ubieda commented Mar 6, 2024

Description

This PR introduces a UART-compatible implementation of Bluetooth LE GATT, using NUS (Nordic UART Service), which allows users to utilize Bluetooth LE as serial endpoints with UART APIs and take advantage of existing support of subsystems that already support UART.

Background

This PR fixes #12714. Also, this PR is a follow-up of #68076, with the agreed changes captured on the Bluetooth WG.

Changes

  • Add NUS as a Bluetooth LE GATT Service.
  • Add Sample demonstrating API usage of NUS.
  • Add UART-compatible driver for Bluetooth LE, using NUS service.
  • Add snippet for basic usage of UART over Bluetooth LE.
  • Add sample support of UART over BLuetooth LE to samples/subsys/logging/logger and samples/subsys/shell/shell_module.

Testing

UART over Bluetooth LE has been tested using this python-app on the PC-side, and the MCU-side running each of the following samples:

  • samples/drivers/uart/echo_bot.
  • samples/subsys/console/echo.
  • samples/subsys/console/getchar.
  • samples/subsys/console/getline.
  • samples/subsys/logging/logger.
  • samples/subsys/shell/shell_module.

Note

The feature is marked as experimental as the surface area of what uses UART goes beyond the aforementioned samples, and it would be beneficial to further test it before removing it.

subsys/logging/backends/log_backend_ble.c Outdated Show resolved Hide resolved
snippets/uart_bt/README.rst Outdated Show resolved Hide resolved
@ubieda ubieda force-pushed the feature/serial/bt-nus branch 2 times, most recently from b57238c to cc96618 Compare March 12, 2024 03:33
@ubieda
Copy link
Collaborator Author

ubieda commented Mar 19, 2024

Haven't tested it yet, will do tomorrow and then approve if it works. I don't have other comments apart from those (not NAKs).

@jori-nordic Thanks for reviewing. I'll address your comments/suggestions today.

NUS is implemented as a Bluetooth LE service, exchanging data treating
the NUS characteristics as Serial endpoints: RX characteristic to
receive, TX characteristic to send binary packets.

This implementation also enables the ability to define multiple
instances of the NUS Service, analogous to mutliple serial endpoints,
to use each one for different purposes. Unless disabled through Kconfig,
NUS instantiates a default instance, similar to what other services do,
which allows users not interested in using multiple instances, to not
worry about the inherent complexities.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Demonstrating basic usage of NUS to exchange data.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Enables usage Bluetooth LE GATT as a serial endpoint to exchange data
using UART APIs. This implementation is compatible with UART Interrupt
Driven APIs and uses the nus-uart device-tree node properties to
configure FIFO buffers for transmitting and receiving. Defining
multiple instances of the driver is possible and it allows implementing
multiple GATT NUS service instances to exchange data as separate serial
endpoints.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Enables compatibility of NUS for codebases that are not
Bluetooth-centric (e.g: Non-bluetooth samples that use UART over
Bluetooth LE for Console, Logging, Shell, others).

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Allowing to be applied on various samples to enable UART over Bluetooth
LE NUS without added complexity.
Tested with nrf52840dk/nrf52840 on the following samples:
- samples/subsys/console/echo.
- samples/subsys/console/getchar.
- samples/subsys/console/getline.
- samples/subsys/logging/logger.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Tested and working with nrf52840dk/nrf52840.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Tested and working with nrf52840dk/nrf52840.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
Add documentation on relevant sections to list addition of Bluetooth LE
service NUS (Nordic UART Service), as well as the UART driver for UART
over Bluetooth LE and snippets to easily enable it.

Signed-off-by: Luis Ubieda <luisf@croxel.com>
@ubieda ubieda reopened this Mar 19, 2024
@ubieda
Copy link
Collaborator Author

ubieda commented Mar 19, 2024

I've made the following changes:

  • Update outdated comments on bt_nus_send() and bt_nus_inst_send() APIs.
  • Add printks on errors on peripheral_nus sample.
  • Moved Bluetooth auto-init code to subsys/bluetooth/services/nus.
  • Changed Bluetooth auto-init code to execute on SYS_INIT().
  • Updated release notes to match updated Kconfig symbol for CONFIG_BT_NUS_AUTO_START_BLUETOOTH.

@jori-nordic
Copy link
Contributor

jori-nordic commented Mar 20, 2024

Logging + the echo (ie console) works w/ your script and the nrf connect apps.
The shell however doesn't. I tried with both nrf apps, and the script. I compiled shell_module both ways and see no output, nothing is even echoed back.

# using the snippet
west build -b nrf52840dk/nrf52840 -S nus-console
# using the overlays
west build -b nrf52840dk/nrf52840 -- -DDTC_OVERLAY_FILE=bt.overlay -DOVERLAY_CONFIG=overlay-bt.conf

edit: well, now the shell works when using the python script. Wonder what I did wrong on first try.

@nordicjm nordicjm dismissed their stale review March 20, 2024 12:26

Discussed at WG

LOG_ERR("RX Ring buffer full. received: %d, added to queue: %d", len, put_len);
}

k_work_submit(&dev_data->uart.cb_work);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be guarded by if (dev_data->uart.rx_irq_ena)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This can also optionally be followed up in a separate PR. It is unclear if spurious interrupts are allowed in the UART IRQ API. If you do take a look, you may also want to guard against TX interrupts from uart_bt_irq_tx_enable when the buffer is full. Maybe it's over-engineering.. but correctness! :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll take a closer look at this when following up on suggested improvements (separate PR). This is good feedback. Thanks!

@alwa-nordic
Copy link
Collaborator

Just wanted to say I think this high quality code. 👍 The code comments are on point - it makes it easy to read. And the documentation is good as well.

@ubieda ubieda requested a review from jhedberg March 21, 2024 11:49
@carlescufi carlescufi merged commit 0186a96 into zephyrproject-rtos:main Mar 22, 2024
39 of 41 checks passed
@ubieda ubieda deleted the feature/serial/bt-nus branch March 22, 2024 13:01
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.

UART driver over Bluetooth LE
9 participants