zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.71k stars 6.54k forks source link

logging: BLE backend unnecessarily sends full buffer regardless of log message size #76565

Closed dathpo closed 2 months ago

dathpo commented 2 months ago

Describe the bug

When the CONFIG_LOG_BACKEND_BLE option is enabled, the BLE Peripheral sends log messages to the Central. The Peripheral appears to send the full buffer on every logging call as opposed to only sending the actual content of a log message.

To Reproduce

  1. Configure a BLE peripheral
  2. Set CONFIG_LOG_BACKEND_BLE=y in the prj.conf
  3. Connect to the Peripheral from a Central
  4. Subscribe to the NUS
  5. Set the MTU to 495
  6. Call LOG_INF("Hello, world")
  7. Check the size of message sent for the log above. It will report a size of 492 octets

Expected behavior

The BLE frame size should be 14 octets.

Impact

It causes a waste of resources by sending a BLE frame larger than necessary.

Logs and console output

The following log message received below should not have been 492 octets. The end of the log message is "bonded: 1", the rest of the buffer is either zero-filled or full of old log data.

image

Environment (please complete the following information):

Additional context

Suggestions:

This code here is the cause of the problem:

    if (attr_data_len < LOG_BACKEND_BLE_BUF_SIZE) {
        notify_len = attr_data_len;
    } else {
        notify_len = LOG_BACKEND_BLE_BUF_SIZE;
    }

The following change addresses the problem:

    notify_len = MIN(length, MIN(attr_data_len, LOG_BACKEND_BLE_BUF_SIZE));
github-actions[bot] commented 2 months ago

Hi @dathpo! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

hermabe commented 2 months ago

CC @vChavezB

vChavezB commented 2 months ago

@dathpo Thanks for raising the issue I will take a look.