[PATCH 1/4] android: Add gatt CCC value storage

[PATCH 1/4] android: Add gatt CCC value storage

[Jakub Tyszkowski]

Client Characteristic Configuration for Service Changed Characteristic
should be stored for every bonded device, so we know if we should be
sending value (range affected by changes) indication.
---
 android/bluetooth.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 android/bluetooth.h |  4 ++++
 2 files changed, 56 insertions(+)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 8ee2025..0a7a81a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -158,6 +158,7 @@ struct device {
 	bool valid_local_csrk;
 	uint8_t local_csrk[16];
 	uint32_t local_sign_cnt;
+	uint16_t gatt_ccc;
 };
 
 struct browse_req {
@@ -645,6 +646,51 @@ static void mgmt_dev_class_changed_event(uint16_t index, uint16_t length,
 	/* TODO: Gatt attrib set*/
 }
 
+void bt_store_gatt_ccc(const bdaddr_t *dst, uint16_t value)
+{
+	struct device *dev;
+	GKeyFile *key_file;
+	gsize length = 0;
+	char addr[18];
+	char *data;
+
+	key_file = g_key_file_new();
+
+	dev = find_device(dst);
+	if (!dev)
+		return;
+
+	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL)) {
+		g_key_file_free(key_file);
+		return;
+	}
+
+	ba2str(dst, addr);
+
+	DBG("%s Gatt CCC %d", addr, value);
+
+	g_key_file_set_integer(key_file, addr, "GattCCC", value);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(DEVICES_FILE, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+
+	dev->gatt_ccc = value;
+}
+
+uint16_t bt_get_gatt_ccc(const bdaddr_t *addr)
+{
+	struct device *dev;
+
+	dev = find_device(addr);
+	if (!dev)
+		return 0;
+
+	return dev->gatt_ccc;
+}
+
 static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
 					uint8_t type, uint8_t pin_length)
 {
@@ -2408,6 +2454,12 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 						"RemoteCSRKSignCounter", NULL);
 	}
 
+	str = g_key_file_get_string(key_file, peer, "GattCCC", NULL);
+	if (str) {
+		dev->gatt_ccc = atoi(str);
+		g_free(str);
+	}
+
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
 	if (str) {
 		g_free(dev->name);
diff --git a/android/bluetooth.h b/android/bluetooth.h
index 1c14377..b4a5f32 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -66,3 +66,7 @@ bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type,
 					uint8_t key[16], uint32_t *sign_cnt);
 
 void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type);
+
+void bt_store_gatt_ccc(const bdaddr_t *addr, uint16_t value);
+
+uint16_t bt_get_gatt_ccc(const bdaddr_t *addr);
-- 
2.0.0

[PATCH 2/4] android/gatt: Store CCC and indicate service change

[Jakub Tyszkowski]

We indicate service change on service stop and service start API calls.
Indication is also send when connecting to bonded device to force it
rebuilding its cache. Write request is properly handled on the response
queue know.
---
 android/gatt.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 90 insertions(+), 16 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 429181f..058cf87 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -160,8 +160,6 @@ struct gatt_device {
 	struct queue *services;
 	bool partial_srvc_search;
 
-	bool notify_services_changed;
-
 	guint watch_id;
 	guint server_id;
 
@@ -189,6 +187,8 @@ static struct queue *app_connections = NULL;
 static struct queue *listen_apps = NULL;
 static struct gatt_db *gatt_db = NULL;
 
+static uint16_t service_changed_handle = 0;
+
 static GIOChannel *listening_io = NULL;
 
 static struct bt_crypto *crypto = NULL;
@@ -1076,10 +1076,45 @@ static void send_exchange_mtu_request(struct gatt_device *device)
 		device_unref(device);
 }
 
+static void notify_att_range_change(struct gatt_device *dev,
+							struct att_range *range)
+{
+	uint16_t length = 0;
+	uint16_t ccc;
+	uint8_t *pdu;
+	size_t mtu;
+
+	ccc = bt_get_gatt_ccc(&dev->bdaddr);
+	if (!ccc)
+		return;
+
+	pdu = g_attrib_get_buffer(dev->attrib, &mtu);
+
+	switch (ccc) {
+	case 0x0001:
+		length = enc_notification(service_changed_handle,
+						(uint8_t *) range,
+						sizeof(*range), pdu, mtu);
+		break;
+	case 0x0002:
+		length = enc_indication(service_changed_handle,
+					(uint8_t *) range, sizeof(*range), pdu,
+					mtu);
+		break;
+	default:
+		/* 0xfff4 reserved for future use */
+		break;
+	}
+
+	if (length)
+		g_attrib_send(dev->attrib, 0, pdu, length, NULL, NULL, NULL);
+}
+
 static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 {
 	struct gatt_device *dev = user_data;
 	struct connect_data data;
+	struct att_range range;
 	uint32_t status;
 	GAttrib *attrib;
 
@@ -1125,6 +1160,22 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
 	/* TODO: Dont exchange mtu if no client apps */
 	send_exchange_mtu_request(dev);
 
+	/*
+	 * Service Changed Characteristic and CCC Descriptor handles
+	 * should not change if there are bonded devices. We have them
+	 * constant all the time, thus they should be excluded from
+	 * range indicating changes.
+	 */
+	range.start = service_changed_handle + 2;
+	range.end = 0xffff;
+
+	/*
+	 * If there is ccc stored for that device we were acting as server for
+	 * it, and as we dont have last connect and last services (de)activation
+	 * timestamps we should always assume something has changed.
+	 */
+	notify_att_range_change(dev, &range);
+
 	status = GATT_SUCCESS;
 
 reply:
@@ -4407,6 +4458,20 @@ failed:
 				HAL_OP_GATT_SERVER_ADD_DESCRIPTOR, status);
 }
 
+static void notify_service_change(void *data, void *user_data)
+{
+	struct att_range range;
+
+	range.start = PTR_TO_UINT(user_data);
+	range.end = gatt_db_get_end_handle(gatt_db, range.start);
+
+	/* In case of db error */
+	if (!range.end)
+		return;
+
+	notify_att_range_change(data, &range);
+}
+
 static void handle_server_start_service(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_server_start_service *cmd = buf;
@@ -4432,6 +4497,9 @@ static void handle_server_start_service(const void *buf, uint16_t len)
 		goto failed;
 	}
 
+	queue_foreach(gatt_devices, notify_service_change,
+					UINT_TO_PTR(cmd->service_handle));
+
 	status = HAL_STATUS_SUCCESS;
 
 failed:
@@ -4468,6 +4536,9 @@ static void handle_server_stop_service(const void *buf, uint16_t len)
 	else
 		status = HAL_STATUS_SUCCESS;
 
+	queue_foreach(gatt_devices, notify_service_change,
+					UINT_TO_PTR(cmd->service_handle));
+
 failed:
 	ev.status = status == HAL_STATUS_SUCCESS ? GATT_SUCCESS : GATT_FAILURE;
 	ev.server_if = cmd->server_if;
@@ -5588,10 +5659,8 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
 						bdaddr_t *bdaddr,
 						void *user_data)
 {
+	struct pending_request *entry;
 	struct gatt_device *dev;
-	uint16_t length;
-	size_t mtu;
-	uint8_t *pdu;
 
 	dev = find_device_by_addr(bdaddr);
 	if (!dev) {
@@ -5599,16 +5668,20 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
 		return;
 	}
 
-	pdu = g_attrib_get_buffer(dev->attrib, &mtu);
-
-	/* TODO handle CCC */
+	entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
+							UINT_TO_PTR(handle));
+	if (!entry)
+		return;
 
-	/* Set services changed notification flag */
-	dev->notify_services_changed = !!(*val);
+	entry->state = REQUEST_DONE;
 
-	length = enc_write_resp(pdu);
+	if (!bt_device_is_bonded(bdaddr)) {
+		entry->error = ATT_ECODE_AUTHORIZATION;
+		return;
+	}
 
-	g_attrib_send(dev->attrib, 0, pdu, length, NULL, NULL, NULL);
+	/* Set services changed indication value */
+	bt_store_gatt_ccc(bdaddr, *val);
 }
 
 static void register_gatt_service(void)
@@ -5622,14 +5695,15 @@ static void register_gatt_service(void)
 	srvc_handle = gatt_db_add_service(gatt_db, &uuid, true, 4);
 
 	bt_uuid16_create(&uuid, GATT_CHARAC_SERVICE_CHANGED);
-	gatt_db_add_characteristic(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
+	service_changed_handle =  gatt_db_add_characteristic(gatt_db,
+					srvc_handle, &uuid, 0,
 					GATT_CHR_PROP_INDICATE, NULL, NULL,
 					NULL);
 
 	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-	gatt_db_add_char_descriptor(gatt_db, srvc_handle, &uuid, GATT_PERM_READ,
-					NULL, gatt_srvc_change_register_cb,
-					NULL);
+	gatt_db_add_char_descriptor(gatt_db, srvc_handle, &uuid,
+					GATT_PERM_WRITE, NULL,
+					gatt_srvc_change_register_cb, NULL);
 
 	gatt_db_service_set_active(gatt_db, srvc_handle, true);
 }
-- 
2.0.0

[PATCH 3/4] android/gatt: Make CCC descriptor readable

[Jakub Tyszkowski]

Descriptor unlike characteristic should be readable.
---
 android/gatt.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 058cf87..b916c79 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5653,7 +5653,7 @@ static void register_device_info_service(void)
 	gatt_db_service_set_active(gatt_db, srvc_handle, true);
 }
 
-static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
+static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
 						const uint8_t *val, size_t len,
 						uint8_t att_opcode,
 						bdaddr_t *bdaddr,
@@ -5684,6 +5684,39 @@ static void gatt_srvc_change_register_cb(uint16_t handle, uint16_t offset,
 	bt_store_gatt_ccc(bdaddr, *val);
 }
 
+static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
+					uint8_t att_opcode, bdaddr_t *bdaddr,
+					void *user_data)
+{
+	struct pending_request *entry;
+	struct gatt_device *dev;
+	uint16_t ccc = 0;
+
+	dev = find_device_by_addr(bdaddr);
+	if (!dev) {
+		error("gatt: Could not find device ?!");
+		return;
+	}
+
+	entry = queue_find(dev->pending_requests, match_dev_request_by_handle,
+							UINT_TO_PTR(handle));
+	if (!entry)
+		return;
+
+	ccc = bt_get_gatt_ccc(&dev->bdaddr);
+	entry->state = REQUEST_DONE;
+
+	entry->value = (uint8_t *) new0(uint16_t, 1);
+	if (!entry->value) {
+		entry->error = ATT_ECODE_INSUFF_RESOURCES;
+
+		return;
+	}
+
+	entry->length = sizeof(uint16_t);
+	memcpy(entry->value, &ccc, sizeof(ccc));
+}
+
 static void register_gatt_service(void)
 {
 	bt_uuid_t uuid;
@@ -5702,8 +5735,9 @@ static void register_gatt_service(void)
 
 	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
 	gatt_db_add_char_descriptor(gatt_db, srvc_handle, &uuid,
-					GATT_PERM_WRITE, NULL,
-					gatt_srvc_change_register_cb, NULL);
+					GATT_PERM_READ | GATT_PERM_WRITE,
+					gatt_srvc_change_read_cb,
+					gatt_srvc_change_write_cb, NULL);
 
 	gatt_db_service_set_active(gatt_db, srvc_handle, true);
 }
-- 
2.0.0

[PATCH 4/4] android/pts: Update GATT results

[Jakub Tyszkowski]

Update results for server's Service Changed Indication (CCC) tests.
---
 android/pts-gatt.txt | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/android/pts-gatt.txt b/android/pts-gatt.txt
index 08ade26..e4d38c0 100644
--- a/android/pts-gatt.txt
+++ b/android/pts-gatt.txt
@@ -918,11 +918,25 @@ TC_GAI_CL_BV_01_C	PASS	haltest:
 				gattc get_descriptor
 				gattc write_descriptor 2 <hex_value> 0002
 				gattc disconnect
-TC_GAI_SR_BV_01_C	INC
+TC_GAI_SR_BV_01_C	PASS	haltest:
+				gatts add_service
+				gatts add_chaaracteristic:
+						<properties> 42 <permissions> 17
+				gatts add_descriptor: <permissions> 17
+				gatts start_service
+				gatts add_service
+				gatts start_service
 TC_GAS_CL_BV_01_C	PASS	haltest:
 				gattc connect
 				gattc disconnect
-TC_GAS_SR_BV_01_C	INC
+TC_GAS_SR_BV_01_C	PASS	haltest:
+				gatts add_service
+				gatts add_chaaracteristic:
+						<properties> 42 <permissions> 17
+				gatts add_descriptor: <permissions> 17
+				gatts start_service
+				gatts add_service
+				gatts start_service
 TC_GAT_CL_BV_01_C	PASS	haltest:
 				gattc connect
 				gattc search_service
@@ -937,7 +951,14 @@ TC_GAT_CL_BV_02_C	PASS	haltest:
 								handle from logs
 				gattc write_characcteristic 2 <short_value>
 				gattc disconnect
-TC_GAT_SR_BV_01_C	INC
+TC_GAT_SR_BV_01_C	PASS	haltest:
+				gatts add_service
+				gatts add_chaaracteristic:
+						<properties> 42 <permissions> 17
+				gatts add_descriptor: <permissions> 17
+				gatts start_service
+				gatts add_service
+				gatts start_service
 TC_GPA_CL_BV_01_C	PASS	haltest:
 				gattc connect
 				test_command: <cmd> 224 [u1] 8
-- 
2.0.0

Re: [PATCH 1/4] android: Add gatt CCC value storage

[Szymon Janc]

Hi Jakub,

On Wednesday 04 of June 2014 16:36:30 Jakub Tyszkowski wrote:
> Client Characteristic Configuration for Service Changed Characteristic
> should be stored for every bonded device, so we know if we should be
> sending value (range affected by changes) indication.
> ---
>  android/bluetooth.c | 52
> ++++++++++++++++++++++++++++++++++++++++++++++++++++ android/bluetooth.h | 
> 4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 8ee2025..0a7a81a 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -158,6 +158,7 @@ struct device {
>  	bool valid_local_csrk;
>  	uint8_t local_csrk[16];
>  	uint32_t local_sign_cnt;
> +	uint16_t gatt_ccc;
>  };
> 
>  struct browse_req {
> @@ -645,6 +646,51 @@ static void mgmt_dev_class_changed_event(uint16_t
> index, uint16_t length, /* TODO: Gatt attrib set*/
>  }
> 
> +void bt_store_gatt_ccc(const bdaddr_t *dst, uint16_t value)
> +{
> +	struct device *dev;
> +	GKeyFile *key_file;
> +	gsize length = 0;
> +	char addr[18];
> +	char *data;
> +
> +	key_file = g_key_file_new();
> +
> +	dev = find_device(dst);
> +	if (!dev)
> +		return;

This would leak key_file. But I've fixed this so no need to resend.

> +
> +	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL)) {
> +		g_key_file_free(key_file);
> +		return;
> +	}
> +
> +	ba2str(dst, addr);
> +
> +	DBG("%s Gatt CCC %d", addr, value);
> +
> +	g_key_file_set_integer(key_file, addr, "GattCCC", value);
> +
> +	data = g_key_file_to_data(key_file, &length, NULL);
> +	g_file_set_contents(DEVICES_FILE, data, length, NULL);
> +	g_free(data);
> +
> +	g_key_file_free(key_file);
> +
> +	dev->gatt_ccc = value;
> +}
> +
> +uint16_t bt_get_gatt_ccc(const bdaddr_t *addr)
> +{
> +	struct device *dev;
> +
> +	dev = find_device(addr);
> +	if (!dev)
> +		return 0;
> +
> +	return dev->gatt_ccc;
> +}
> +
>  static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
>  					uint8_t type, uint8_t pin_length)
>  {
> @@ -2408,6 +2454,12 @@ static struct device
> *create_device_from_info(GKeyFile *key_file, "RemoteCSRKSignCounter",
> NULL);
>  	}
> 
> +	str = g_key_file_get_string(key_file, peer, "GattCCC", NULL);
> +	if (str) {
> +		dev->gatt_ccc = atoi(str);
> +		g_free(str);
> +	}
> +
>  	str = g_key_file_get_string(key_file, peer, "Name", NULL);
>  	if (str) {
>  		g_free(dev->name);
> diff --git a/android/bluetooth.h b/android/bluetooth.h
> index 1c14377..b4a5f32 100644
> --- a/android/bluetooth.h
> +++ b/android/bluetooth.h
> @@ -66,3 +66,7 @@ bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type
> type, uint8_t key[16], uint32_t *sign_cnt);
> 
>  void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type);
> +
> +void bt_store_gatt_ccc(const bdaddr_t *addr, uint16_t value);
> +
> +uint16_t bt_get_gatt_ccc(const bdaddr_t *addr);

All patches applied. Thanks.

-- 
BR
Szymon Janc