From bee43bd972d2174ca624f08207bc8ed92c8f9edb Mon Sep 17 00:00:00 2001 From: Victor Chavez Date: Sat, 15 Apr 2023 09:31:33 +0200 Subject: [PATCH] logging: Fixed BLE backend buffer size independent of MTU The BLE backend has a bug that when the MTU size is less than the buffer the notification always fails and does not allow to flush the buffer. This fix checks for the MTU size of the current connection and adjusts it. In addition, the buffer size is no longer settable by the user and depends on the transmission size of MTU set with CONFIG_BT_L2CAP_TX_MTU. Signed-off-by: Victor Chavez --- samples/subsys/logging/ble_backend/README.rst | 3 + samples/subsys/logging/ble_backend/prj.conf | 1 - subsys/logging/backends/Kconfig.ble | 11 --- subsys/logging/backends/log_backend_ble.c | 67 +++++++++++++++++-- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/samples/subsys/logging/ble_backend/README.rst b/samples/subsys/logging/ble_backend/README.rst index 6c12071cae07..2c3726dc0bf1 100644 --- a/samples/subsys/logging/ble_backend/README.rst +++ b/samples/subsys/logging/ble_backend/README.rst @@ -10,6 +10,9 @@ Sample that demonstrates how to setup and use the BLE Logging backend. The BLE Logger uses the NRF Connect SDK NUS service as UUID to make it compatible with already existing apps to debug BLE connections over UART. +The notification size of the ble backend buffer is dependent on the +transmission size of the mtu set with `CONFIG_BT_L2CAP_TX_MTU`. Be sure +to change this configuration to increase the logger throughput over BLE. Requirements ************ diff --git a/samples/subsys/logging/ble_backend/prj.conf b/samples/subsys/logging/ble_backend/prj.conf index 4a1ab5099b05..f98016410b98 100644 --- a/samples/subsys/logging/ble_backend/prj.conf +++ b/samples/subsys/logging/ble_backend/prj.conf @@ -8,4 +8,3 @@ CONFIG_LOG_PROCESS_THREAD_STACK_SIZE=2048 # Uncomment to use the maximum buffer size # CONFIG_BT_L2CAP_TX_MTU=600 # CONFIG_BT_BUF_ACL_RX_SIZE=600 -# CONFIG_LOG_BACKEND_BLE_BUF_SIZE=512 diff --git a/subsys/logging/backends/Kconfig.ble b/subsys/logging/backends/Kconfig.ble index b2ae6901fe20..016573c0f7ba 100644 --- a/subsys/logging/backends/Kconfig.ble +++ b/subsys/logging/backends/Kconfig.ble @@ -16,17 +16,6 @@ config LOG_BACKEND_BLE if LOG_BACKEND_BLE -config LOG_BACKEND_BLE_BUF_SIZE - int "BLE Logger Backend Buffer size" - range 20 512 - default 20 - help - Maximum buffer size that can be transmitted over the BLE Logger - notification characteristic. The minimum size is 20 for the smallest - MTU packet. Be sure to increase the MTU size in your application to use - bigger values. - Both BT_L2CAP_TX_MTU and BT_BUF_ACL_RX_SIZE will need to be increased. - backend = BLE backend-str = ble source "subsys/logging/Kconfig.template.log_format_config" diff --git a/subsys/logging/backends/log_backend_ble.c b/subsys/logging/backends/log_backend_ble.c index 626b7fdff64a..9487dc72b717 100644 --- a/subsys/logging/backends/log_backend_ble.c +++ b/subsys/logging/backends/log_backend_ble.c @@ -7,16 +7,24 @@ #include #include #include +#include #include -static uint8_t output_buf[CONFIG_LOG_BACKEND_BLE_BUF_SIZE]; +#define ATT_NOTIFY_SIZE 3 +#define LOG_BACKEND_BLE_BUF_SIZE (CONFIG_BT_L2CAP_TX_MTU - ATT_NOTIFY_SIZE) + +static uint8_t output_buf[LOG_BACKEND_BLE_BUF_SIZE]; static bool panic_mode; static uint32_t log_format_current = CONFIG_LOG_BACKEND_BLE_OUTPUT_DEFAULT; static logger_backend_ble_hook user_hook; static bool first_enable; static void *user_ctx; +static struct bt_conn *ble_backend_conn; + /* Forward declarations*/ -const struct log_backend *log_backend_ble_get(void); +static const struct log_backend *log_backend_ble_get(void); +static void log_backend_ble_connect(struct bt_conn *conn, uint8_t err); +static void log_backend_ble_disconnect(struct bt_conn *conn, uint8_t reason); /** * @brief Callback for the subscription to the ble logger notification characteristic @@ -39,6 +47,13 @@ static void log_notify_changed(const struct bt_gatt_attr *attr, uint16_t value); #define LOGGER_RX_SERVICE_UUID \ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x6E400002, 0xB5A3, 0xF393, 0xE0A9, 0xE50E24DCCA9E)) +BT_CONN_CB_DEFINE(log_backend_ble) = { + .connected = log_backend_ble_connect, + .disconnected = log_backend_ble_disconnect, + .le_param_req = NULL, + .le_param_updated = NULL +}; + /** * @brief BLE Service that represents this backend * @note Only transmission characteristic is used. The RX characteristic @@ -63,6 +78,20 @@ void logger_backend_ble_set_hook(logger_backend_ble_hook hook, void *ctx) user_ctx = ctx; } +static void log_backend_ble_connect(struct bt_conn *conn, uint8_t err) +{ + if (err == 0) { + ble_backend_conn = conn; + } +} + +static void log_backend_ble_disconnect(struct bt_conn *conn, uint8_t reason) +{ + ARG_UNUSED(conn); + ARG_UNUSED(reason); + ble_backend_conn = NULL; +} + void log_notify_changed(const struct bt_gatt_attr *attr, uint16_t value) { ARG_UNUSED(attr); @@ -86,19 +115,41 @@ void log_notify_changed(const struct bt_gatt_attr *attr, uint16_t value) static int line_out(uint8_t *data, size_t length, void *output_ctx) { - const int notify_res = bt_gatt_notify(NULL, log_characteristic, data, length); + ARG_UNUSED(output_ctx); + const uint16_t mtu_size = bt_gatt_get_mtu(ble_backend_conn); + const uint16_t attr_data_len = mtu_size - ATT_NOTIFY_SIZE; + uint16_t notify_len; - if (notify_res == 0) { - return length; + if (attr_data_len < LOG_BACKEND_BLE_BUF_SIZE) { + notify_len = attr_data_len; } else { - return 0; + notify_len = LOG_BACKEND_BLE_BUF_SIZE; } + + struct bt_gatt_notify_params notify_param = { + .uuid = NULL, + .attr = log_characteristic, + .data = data, + .len = notify_len, + .func = NULL, + .user_data = NULL, + #if defined(CONFIG_BT_EATT) + .chan_opt = BT_ATT_CHAN_OPT_NONE + #endif + }; + + const int notify_res = bt_gatt_notify_cb(ble_backend_conn, ¬ify_param); + /* ignore notification result and continue sending msg*/ + ARG_UNUSED(notify_res); + + return length; } LOG_OUTPUT_DEFINE(log_output_ble, line_out, output_buf, sizeof(output_buf)); static void process(const struct log_backend *const backend, union log_msg_generic *msg) { + ARG_UNUSED(backend); uint32_t flags = LOG_OUTPUT_FLAG_FORMAT_SYSLOG | LOG_OUTPUT_FLAG_TIMESTAMP; if (panic_mode) { @@ -112,17 +163,20 @@ static void process(const struct log_backend *const backend, union log_msg_gener static int format_set(const struct log_backend *const backend, uint32_t log_type) { + ARG_UNUSED(backend); log_format_current = log_type; return 0; } static void init_ble(struct log_backend const *const backend) { + ARG_UNUSED(backend); log_backend_deactivate(log_backend_ble_get()); } static void panic(struct log_backend const *const backend) { + ARG_UNUSED(backend); panic_mode = true; } @@ -136,6 +190,7 @@ static void panic(struct log_backend const *const backend) */ static int backend_ready(const struct log_backend *const backend) { + ARG_UNUSED(backend); return -EACCES; }