linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/4] util: Introduce util_debug_va
@ 2021-01-08 21:15 Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 2/4] shared/att: Add debug level to bt_att_set_debug Luiz Augusto von Dentz
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-08 21:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This introduces util_debug_va which can take a va_list that enables
callers to create wrapper functions if they need to.
---
 src/shared/util.c | 19 ++++++++++++++-----
 src/shared/util.h |  4 ++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index 525302164..9c2054211 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -15,7 +15,6 @@
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <ctype.h>
-#include <stdarg.h>
 #include <stdbool.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -42,20 +41,30 @@ void *btd_malloc(size_t size)
 	return NULL;
 }
 
+void util_debug_va(util_debug_func_t function, void *user_data,
+				const char *format, va_list va)
+{
+	char str[78];
+
+	if (!function || !format)
+		return;
+
+	vsnprintf(str, sizeof(str), format, va);
+
+	function(str, user_data);
+}
+
 void util_debug(util_debug_func_t function, void *user_data,
 						const char *format, ...)
 {
-	char str[78];
 	va_list ap;
 
 	if (!function || !format)
 		return;
 
 	va_start(ap, format);
-	vsnprintf(str, sizeof(str), format, ap);
+	util_debug_va(function, user_data, format, ap);
 	va_end(ap);
-
-	function(str, user_data);
 }
 
 void util_hexdump(const char dir, const unsigned char *buf, size_t len,
diff --git a/src/shared/util.h b/src/shared/util.h
index 6fb702797..d6de55885 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -10,6 +10,7 @@
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <stdarg.h>
 #include <alloca.h>
 #include <byteswap.h>
 #include <string.h>
@@ -89,6 +90,9 @@ void *btd_malloc(size_t size);
 
 typedef void (*util_debug_func_t)(const char *str, void *user_data);
 
+void util_debug_va(util_debug_func_t function, void *user_data,
+				const char *format, va_list va);
+
 void util_debug(util_debug_func_t function, void *user_data,
 						const char *format, ...)
 					__attribute__((format(printf, 3, 4)));
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 2/4] shared/att: Add debug level to bt_att_set_debug
  2021-01-08 21:15 [PATCH BlueZ 1/4] util: Introduce util_debug_va Luiz Augusto von Dentz
@ 2021-01-08 21:15 ` Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 3/4] device: Enable ATT layer debugging Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-08 21:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This creates different levels of debugging which can be passed to
bt_att_set_debug as depending on the application it may not need to
print everything which can pollute the logs quite a bit.
---
 src/shared/att.c      | 108 +++++++++++++++++++++++-------------------
 src/shared/att.h      |   9 +++-
 tools/btgatt-client.c |   3 +-
 tools/btgatt-server.c |   3 +-
 unit/test-gatt.c      |   2 +-
 5 files changed, 71 insertions(+), 54 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 429ba6696..ccc753c4e 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -77,6 +77,7 @@ struct bt_att {
 	bt_att_destroy_func_t timeout_destroy;
 	void *timeout_data;
 
+	uint8_t debug_level;
 	bt_att_debug_func_t debug_callback;
 	bt_att_destroy_func_t debug_destroy;
 	void *debug_data;
@@ -274,6 +275,34 @@ static bool match_disconn_id(const void *a, const void *b)
 	return disconn->id == id;
 }
 
+static void att_log(struct bt_att *att, uint8_t level, const char *format,
+								...)
+{
+	va_list va;
+
+	if (att->debug_level < level)
+		return;
+
+	va_start(va, format);
+	util_debug_va(att->debug_callback, att->debug_data, format, va);
+	va_end(va);
+}
+
+#define att_debug(_att, _format, _arg...) \
+	att_log(_att, BT_ATT_DEBUG, _format, ## _arg)
+
+#define att_verbose(_att, _format, _arg...) \
+	att_log(_att, BT_ATT_DEBUG_VERBOSE, _format, ## _arg)
+
+static void att_hexdump(struct bt_att *att, char dir, const void *data,
+							size_t len)
+{
+	if (att->debug_level < 2)
+		return;
+
+	util_hexdump(dir, data, len, att->debug_callback, att->debug_data);
+}
+
 static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
 					const void *pdu, uint16_t length)
 {
@@ -309,8 +338,7 @@ static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
 				sign_cnt, &((uint8_t *) op->pdu)[1 + length])))
 		return true;
 
-	util_debug(att->debug_callback, att->debug_data,
-					"ATT unable to generate signature");
+	att_debug(att, "ATT unable to generate signature");
 
 fail:
 	free(op->pdu);
@@ -432,9 +460,8 @@ static bool timeout_cb(void *user_data)
 	if (!op)
 		return false;
 
-	util_debug(att->debug_callback, att->debug_data,
-				"(chan %p) Operation timed out: 0x%02x",
-				chan, op->opcode);
+	att_debug(att, "(chan %p) Operation timed out: 0x%02x", chan,
+						op->opcode);
 
 	if (att->timeout_callback)
 		att->timeout_callback(op->id, op->opcode, att->timeout_data);
@@ -469,20 +496,18 @@ static ssize_t bt_att_chan_write(struct bt_att_chan *chan, uint8_t opcode,
 	iov.iov_base = (void *) pdu;
 	iov.iov_len = len;
 
-	util_debug(att->debug_callback, att->debug_data,
-					"(chan %p) ATT op 0x%02x",
-					chan, opcode);
+	att_verbose(att, "(chan %p) ATT op 0x%02x", chan, opcode);
 
 	ret = io_send(chan->io, &iov, 1);
 	if (ret < 0) {
-		util_debug(att->debug_callback, att->debug_data,
-					"(chan %p) write failed: %s",
-					chan, strerror(-ret));
-
+		att_debug(att, "(chan %p) write failed: %s", chan,
+						strerror(-ret));
 		return ret;
 	}
 
-	util_hexdump('<', pdu, ret, att->debug_callback, att->debug_data);
+	if (att->debug_level)
+		util_hexdump('<', pdu, ret, att->debug_callback,
+						att->debug_data);
 
 	return ret;
 }
@@ -608,15 +633,12 @@ static bool disconnect_cb(struct io *io, void *user_data)
 	len = sizeof(err);
 
 	if (getsockopt(chan->fd, SOL_SOCKET, SO_ERROR, &err, &len) < 0) {
-		util_debug(chan->att->debug_callback, chan->att->debug_data,
-					"(chan %p) Failed to obtain disconnect"
-					" error: %s", chan, strerror(errno));
+		att_debug(att, "(chan %p) Failed to obtain disconnect "
+				"error: %s", chan, strerror(errno));
 		err = 0;
 	}
 
-	util_debug(chan->att->debug_callback, chan->att->debug_data,
-					"Channel %p disconnected: %s",
-					chan, strerror(err));
+	att_debug(att, "Channel %p disconnected: %s", chan, strerror(err));
 
 	/* Dettach channel */
 	queue_remove(att->chans, chan);
@@ -745,9 +767,7 @@ static bool handle_error_rsp(struct bt_att_chan *chan, uint8_t *pdu,
 		op->timeout_id = 0;
 	}
 
-	util_debug(att->debug_callback, att->debug_data,
-						"(chan %p) Retrying operation "
-						"%p", chan, op);
+	att_debug(att, "(chan %p) Retrying operation %p", chan, op);
 
 	chan->pending_req = NULL;
 
@@ -770,9 +790,8 @@ static void handle_rsp(struct bt_att_chan *chan, uint8_t opcode, uint8_t *pdu,
 	 * the bearer.
 	 */
 	if (!op) {
-		util_debug(att->debug_callback, att->debug_data,
-					"(chan %p) Received unexpected ATT "
-					"response", chan);
+		att_debug(att, "(chan %p) Received unexpected ATT response",
+								chan);
 		io_shutdown(chan->io);
 		return;
 	}
@@ -803,8 +822,7 @@ static void handle_rsp(struct bt_att_chan *chan, uint8_t opcode, uint8_t *pdu,
 	goto done;
 
 fail:
-	util_debug(att->debug_callback, att->debug_data,
-			"(chan %p) Failed to handle response PDU; opcode: "
+	att_debug(att, "(chan %p) Failed to handle response PDU; opcode: "
 			"0x%02x", chan, opcode);
 
 	rsp_opcode = BT_ATT_OP_ERROR_RSP;
@@ -829,8 +847,7 @@ static void handle_conf(struct bt_att_chan *chan, uint8_t *pdu, ssize_t pdu_len)
 	 * invalid.
 	 */
 	if (!op || pdu_len) {
-		util_debug(att->debug_callback, att->debug_data,
-				"(chan %p) Received unexpected/invalid ATT "
+		att_debug(att, "(chan %p) Received unexpected/invalid ATT "
 				"confirmation", chan);
 		io_shutdown(chan->io);
 		return;
@@ -904,8 +921,7 @@ static bool handle_signed(struct bt_att *att, uint8_t *pdu, ssize_t pdu_len)
 	return true;
 
 fail:
-	util_debug(att->debug_callback, att->debug_data,
-			"ATT failed to verify signature: 0x%02x", opcode);
+	att_debug(att, "ATT failed to verify signature: 0x%02x", opcode);
 
 	return false;
 }
@@ -987,12 +1003,9 @@ static bool can_read_data(struct io *io, void *user_data)
 	if (bytes_read < 0)
 		return false;
 
-	util_debug(att->debug_callback, att->debug_data,
-				"(chan %p) ATT received: %zd",
-				chan, bytes_read);
+	att_verbose(att, "(chan %p) ATT received: %zd", chan, bytes_read);
 
-	util_hexdump('>', chan->buf, bytes_read,
-				att->debug_callback, att->debug_data);
+	att_hexdump(att, '>', chan->buf, bytes_read);
 
 	if (bytes_read < ATT_MIN_PDU_LEN)
 		return true;
@@ -1005,14 +1018,12 @@ static bool can_read_data(struct io *io, void *user_data)
 	/* Act on the received PDU based on the opcode type */
 	switch (get_op_type(opcode)) {
 	case ATT_OP_TYPE_RSP:
-		util_debug(att->debug_callback, att->debug_data,
-				"(chan %p) ATT response received: 0x%02x",
+		att_verbose(att, "(chan %p) ATT response received: 0x%02x",
 				chan, opcode);
 		handle_rsp(chan, opcode, pdu + 1, bytes_read - 1);
 		break;
 	case ATT_OP_TYPE_CONF:
-		util_debug(att->debug_callback, att->debug_data,
-				"(chan %p) ATT confirmation received: 0x%02x",
+		att_verbose(att, "(chan %p) ATT confirmation received: 0x%02x",
 				chan, opcode);
 		handle_conf(chan, pdu + 1, bytes_read - 1);
 		break;
@@ -1023,8 +1034,7 @@ static bool can_read_data(struct io *io, void *user_data)
 		 * promptly notify the upper layer via disconnect handlers.
 		 */
 		if (chan->in_req) {
-			util_debug(att->debug_callback, att->debug_data,
-					"(chan %p) Received request while "
+			att_debug(att, "(chan %p) Received request while "
 					"another is pending: 0x%02x",
 					chan, opcode);
 			io_shutdown(chan->io);
@@ -1044,9 +1054,8 @@ static bool can_read_data(struct io *io, void *user_data)
 		/* For all other opcodes notify the upper layer of the PDU and
 		 * let them act on it.
 		 */
-		util_debug(att->debug_callback, att->debug_data,
-					"(chan %p) ATT PDU received: 0x%02x",
-					chan, opcode);
+		att_debug(att, "(chan %p) ATT PDU received: 0x%02x", chan,
+							opcode);
 		handle_notify(chan, pdu, bytes_read);
 		break;
 	}
@@ -1198,8 +1207,7 @@ static void bt_att_attach_chan(struct bt_att *att, struct bt_att_chan *chan)
 
 	io_set_close_on_destroy(chan->io, att->close_on_unref);
 
-	util_debug(att->debug_callback, att->debug_data, "Channel %p attached",
-									chan);
+	att_debug(att, "Channel %p attached", chan);
 
 	wakeup_chan_writer(chan, NULL);
 }
@@ -1315,8 +1323,9 @@ int bt_att_get_channels(struct bt_att *att)
 	return queue_length(att->chans);
 }
 
-bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
-				void *user_data, bt_att_destroy_func_t destroy)
+bool bt_att_set_debug(struct bt_att *att, uint8_t level,
+			bt_att_debug_func_t callback, void *user_data,
+			bt_att_destroy_func_t destroy)
 {
 	if (!att)
 		return false;
@@ -1324,6 +1333,7 @@ bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
 	if (att->debug_destroy)
 		att->debug_destroy(att->debug_data);
 
+	att->debug_level = level;
 	att->debug_callback = callback;
 	att->debug_destroy = destroy;
 	att->debug_data = user_data;
diff --git a/src/shared/att.h b/src/shared/att.h
index 1938fc724..03a450988 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -13,6 +13,10 @@
 
 #include "src/shared/att-types.h"
 
+#define BT_ATT_DEBUG		0x00
+#define BT_ATT_DEBUG_VERBOSE	0x01
+#define BT_ATT_DEBUG_HEXDUMP	0x02
+
 struct bt_att;
 struct bt_att_chan;
 
@@ -41,8 +45,9 @@ typedef void (*bt_att_timeout_func_t)(unsigned int id, uint8_t opcode,
 typedef void (*bt_att_disconnect_func_t)(int err, void *user_data);
 typedef bool (*bt_att_counter_func_t)(uint32_t *sign_cnt, void *user_data);
 
-bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
-				void *user_data, bt_att_destroy_func_t destroy);
+bool bt_att_set_debug(struct bt_att *att, uint8_t level,
+			bt_att_debug_func_t callback, void *user_data,
+			bt_att_destroy_func_t destroy);
 
 uint16_t bt_att_get_mtu(struct bt_att *att);
 bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 523d6ec30..8c9365aa2 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -218,7 +218,8 @@ static struct client *client_create(int fd, uint16_t mtu)
 								NULL, NULL);
 
 	if (verbose) {
-		bt_att_set_debug(cli->att, att_debug_cb, "att: ", NULL);
+		bt_att_set_debug(cli->att, BT_ATT_DEBUG_VERBOSE, att_debug_cb,
+								"att: ", NULL);
 		bt_gatt_client_set_debug(cli->gatt, gatt_debug_cb, "gatt: ",
 									NULL);
 	}
diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index aa3448765..000145a3d 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -584,7 +584,8 @@ static struct server *server_create(int fd, uint16_t mtu, bool hr_visible)
 	server->hr_visible = hr_visible;
 
 	if (verbose) {
-		bt_att_set_debug(server->att, att_debug_cb, "att: ", NULL);
+		bt_att_set_debug(server->att, BT_ATT_DEBUG_VERBOSE,
+						att_debug_cb, "att: ", NULL);
 		bt_gatt_server_set_debug(server->gatt, gatt_debug_cb,
 							"server: ", NULL);
 	}
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 6a49210c5..4aa87d09c 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -658,7 +658,7 @@ static struct context *create_context(uint16_t mtu, gconstpointer data)
 
 	switch (test_data->context_type) {
 	case ATT:
-		bt_att_set_debug(context->att, print_debug, "bt_att:", NULL);
+		bt_att_set_debug(context->att, 1, print_debug, "bt_att:", NULL);
 
 		bt_gatt_exchange_mtu(context->att, mtu, NULL, NULL, NULL);
 		break;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 3/4] device: Enable ATT layer debugging
  2021-01-08 21:15 [PATCH BlueZ 1/4] util: Introduce util_debug_va Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 2/4] shared/att: Add debug level to bt_att_set_debug Luiz Augusto von Dentz
@ 2021-01-08 21:15 ` Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed Luiz Augusto von Dentz
  2021-01-08 21:40 ` [BlueZ,1/4] util: Introduce util_debug_va bluez.test.bot
  3 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-08 21:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This uses bt_att_set_debug to enable ATT debugging which is useful for
detecting error such as an ATT transaction timing out.
---
 src/device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/device.c b/src/device.c
index 2e97876ec..fe885aa64 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5336,6 +5336,8 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 
 	bt_att_ref(dev->att);
 
+	bt_att_set_debug(dev->att, BT_ATT_DEBUG, gatt_debug, NULL, NULL);
+
 	dev->att_disconn_id = bt_att_register_disconnect(dev->att,
 						att_disconnected_cb, dev, NULL);
 	bt_att_set_close_on_unref(dev->att, true);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed
  2021-01-08 21:15 [PATCH BlueZ 1/4] util: Introduce util_debug_va Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 2/4] shared/att: Add debug level to bt_att_set_debug Luiz Augusto von Dentz
  2021-01-08 21:15 ` [PATCH BlueZ 3/4] device: Enable ATT layer debugging Luiz Augusto von Dentz
@ 2021-01-08 21:15 ` Luiz Augusto von Dentz
  2021-01-20  9:29   ` Sathish Narasimman
  2021-01-08 21:40 ` [BlueZ,1/4] util: Introduce util_debug_va bluez.test.bot
  3 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-08 21:15 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Unfortunately assuming service changed has been subscribed may cause
indication to time out in some peripherals (Logitech M720 Triathlon, Mx
Anywhere 2, Lenovo Mice N700, RAPOO BleMouse and Microsoft Designer
Mouse) even though the expect actually mandates that the client responds
with confirmation these peripherals just ignores it completely which
leads them to be disconnected whenever bluetoothd is restarted or the
system reboots.
---
 src/device.c        | 11 ++---------
 src/gatt-database.c |  2 +-
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/device.c b/src/device.c
index fe885aa64..af13badfc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5831,18 +5831,11 @@ void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
 	key_file = g_key_file_new();
 	g_key_file_load_from_file(key_file, filename, 0, NULL);
 
-	/*
-	 * If there is no "ServiceChanged" section we may be loading data from
-	 * old version which did not persist Service Changed CCC values. Let's
-	 * check if we are bonded and assume indications were enabled by peer
-	 * in such case - it should have done this anyway.
-	 */
 	if (!g_key_file_has_group(key_file, "ServiceChanged")) {
 		if (ccc_le)
-			*ccc_le = device->le_state.bonded ? 0x0002 : 0x0000;
+			*ccc_le = 0x0000;
 		if (ccc_bredr)
-			*ccc_bredr = device->bredr_state.bonded ?
-							0x0002 : 0x0000;
+			*ccc_bredr = 0x0000;
 		g_key_file_free(key_file);
 		return;
 	}
diff --git a/src/gatt-database.c b/src/gatt-database.c
index b7d2bea1d..d99604826 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -333,7 +333,7 @@ static void att_disconnected(int err, void *user_data)
 		handle = gatt_db_attribute_get_handle(state->db->svc_chngd_ccc);
 
 		ccc = find_ccc_state(state, handle);
-		if (ccc)
+		if (ccc && ccc->value)
 			device_store_svc_chng_ccc(device, state->bdaddr_type,
 								ccc->value);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [BlueZ,1/4] util: Introduce util_debug_va
  2021-01-08 21:15 [PATCH BlueZ 1/4] util: Introduce util_debug_va Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2021-01-08 21:15 ` [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed Luiz Augusto von Dentz
@ 2021-01-08 21:40 ` bluez.test.bot
  2021-01-11 16:46   ` Luiz Augusto von Dentz
  3 siblings, 1 reply; 8+ messages in thread
From: bluez.test.bot @ 2021-01-08 21:40 UTC (permalink / raw)
  To: linux-bluetooth, luiz.dentz

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=411499

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BlueZ,1/4] util: Introduce util_debug_va
  2021-01-08 21:40 ` [BlueZ,1/4] util: Introduce util_debug_va bluez.test.bot
@ 2021-01-11 16:46   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-11 16:46 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

On Fri, Jan 8, 2021 at 1:40 PM <bluez.test.bot@gmail.com> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=411499
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - PASS
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS

Pushed.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed
  2021-01-08 21:15 ` [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed Luiz Augusto von Dentz
@ 2021-01-20  9:29   ` Sathish Narasimman
  2021-01-20 15:35     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Sathish Narasimman @ 2021-01-20  9:29 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Bluez mailing list

Hi Luiz

On Sat, Jan 9, 2021 at 2:48 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> Unfortunately assuming service changed has been subscribed may cause
> indication to time out in some peripherals (Logitech M720 Triathlon, Mx
> Anywhere 2, Lenovo Mice N700, RAPOO BleMouse and Microsoft Designer
> Mouse) even though the expect actually mandates that the client responds
> with confirmation these peripherals just ignores it completely which
> leads them to be disconnected whenever bluetoothd is restarted or the
> system reboots.
> ---
>  src/device.c        | 11 ++---------
>  src/gatt-database.c |  2 +-
>  2 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index fe885aa64..af13badfc 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -5831,18 +5831,11 @@ void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
>         key_file = g_key_file_new();
>         g_key_file_load_from_file(key_file, filename, 0, NULL);
>
> -       /*
> -        * If there is no "ServiceChanged" section we may be loading data from
> -        * old version which did not persist Service Changed CCC values. Let's
> -        * check if we are bonded and assume indications were enabled by peer
> -        * in such case - it should have done this anyway.
> -        */
>         if (!g_key_file_has_group(key_file, "ServiceChanged")) {
>                 if (ccc_le)
> -                       *ccc_le = device->le_state.bonded ? 0x0002 : 0x0000;
> +                       *ccc_le = 0x0000;
>                 if (ccc_bredr)
> -                       *ccc_bredr = device->bredr_state.bonded ?
> -                                                       0x0002 : 0x0000;
> +                       *ccc_bredr = 0x0000;
>                 g_key_file_free(key_file);
>                 return;
>         }
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index b7d2bea1d..d99604826 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -333,7 +333,7 @@ static void att_disconnected(int err, void *user_data)
>                 handle = gatt_db_attribute_get_handle(state->db->svc_chngd_ccc);
>
>                 ccc = find_ccc_state(state, handle);
> -               if (ccc)
> +               if (ccc && ccc->value)
>                         device_store_svc_chng_ccc(device, state->bdaddr_type,
>                                                                 ccc->value);
>
> --
> 2.26.2
>

Was this patch is merged?

Regards
Sathish N

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed
  2021-01-20  9:29   ` Sathish Narasimman
@ 2021-01-20 15:35     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-20 15:35 UTC (permalink / raw)
  To: Sathish Narasimman; +Cc: Bluez mailing list

Hi Sathish,

On Wed, Jan 20, 2021 at 1:30 AM Sathish Narasimman <nsathish41@gmail.com> wrote:
>
> Hi Luiz
>
> On Sat, Jan 9, 2021 at 2:48 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > Unfortunately assuming service changed has been subscribed may cause
> > indication to time out in some peripherals (Logitech M720 Triathlon, Mx
> > Anywhere 2, Lenovo Mice N700, RAPOO BleMouse and Microsoft Designer
> > Mouse) even though the expect actually mandates that the client responds
> > with confirmation these peripherals just ignores it completely which
> > leads them to be disconnected whenever bluetoothd is restarted or the
> > system reboots.
> > ---
> >  src/device.c        | 11 ++---------
> >  src/gatt-database.c |  2 +-
> >  2 files changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/device.c b/src/device.c
> > index fe885aa64..af13badfc 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -5831,18 +5831,11 @@ void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
> >         key_file = g_key_file_new();
> >         g_key_file_load_from_file(key_file, filename, 0, NULL);
> >
> > -       /*
> > -        * If there is no "ServiceChanged" section we may be loading data from
> > -        * old version which did not persist Service Changed CCC values. Let's
> > -        * check if we are bonded and assume indications were enabled by peer
> > -        * in such case - it should have done this anyway.
> > -        */
> >         if (!g_key_file_has_group(key_file, "ServiceChanged")) {
> >                 if (ccc_le)
> > -                       *ccc_le = device->le_state.bonded ? 0x0002 : 0x0000;
> > +                       *ccc_le = 0x0000;
> >                 if (ccc_bredr)
> > -                       *ccc_bredr = device->bredr_state.bonded ?
> > -                                                       0x0002 : 0x0000;
> > +                       *ccc_bredr = 0x0000;
> >                 g_key_file_free(key_file);
> >                 return;
> >         }
> > diff --git a/src/gatt-database.c b/src/gatt-database.c
> > index b7d2bea1d..d99604826 100644
> > --- a/src/gatt-database.c
> > +++ b/src/gatt-database.c
> > @@ -333,7 +333,7 @@ static void att_disconnected(int err, void *user_data)
> >                 handle = gatt_db_attribute_get_handle(state->db->svc_chngd_ccc);
> >
> >                 ccc = find_ccc_state(state, handle);
> > -               if (ccc)
> > +               if (ccc && ccc->value)
> >                         device_store_svc_chng_ccc(device, state->bdaddr_type,
> >                                                                 ccc->value);
> >
> > --
> > 2.26.2
> >
>
> Was this patch is merged?

Yes, https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=39054d59c0ecdb102f8aa352cb7aa6fcbd7f2b6b

> Regards
> Sathish N



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-20 15:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08 21:15 [PATCH BlueZ 1/4] util: Introduce util_debug_va Luiz Augusto von Dentz
2021-01-08 21:15 ` [PATCH BlueZ 2/4] shared/att: Add debug level to bt_att_set_debug Luiz Augusto von Dentz
2021-01-08 21:15 ` [PATCH BlueZ 3/4] device: Enable ATT layer debugging Luiz Augusto von Dentz
2021-01-08 21:15 ` [PATCH BlueZ 4/4] gatt: Fix assuming service changed has been subscribed Luiz Augusto von Dentz
2021-01-20  9:29   ` Sathish Narasimman
2021-01-20 15:35     ` Luiz Augusto von Dentz
2021-01-08 21:40 ` [BlueZ,1/4] util: Introduce util_debug_va bluez.test.bot
2021-01-11 16:46   ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).