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 <chavez-bermudez@fh-aachen.de>
Willmish · Jun 19, 2023 · bee43bd · bee43bd
1 parent f32fae0
commit bee43bd
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -7,16 +7,24 @@
 #include <zephyr/logging/log_output.h>
 #include <zephyr/logging/log_backend_ble.h>
 #include <zephyr/bluetooth/gatt.h>
+#include <zephyr/bluetooth/conn.h>
 #include <zephyr/logging/log_ctrl.h>
 
-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, &notify_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;
 }