[PATCH v2 0/4] android:Add initial support for GATT Client

[PATCH v2 0/4] android:Add initial support for GATT Client

[Lukasz Rymanowski]

This patch set add GATT clien scan, connect/disconnect
functionality.

Some changes in handling discovery has been done as
there are two clients of discovery: HAL and GATT

For now daemon makes sure that device is available before
connecting it. It is because in case device is not available
we would block connect to other devices.

Once white list will be available we can simplify that code.

v2: Fixes in discovery handling

Jakub Tyszkowski (2):
  android/bluetooth: Add GATT notifications on LE discovery
  android/gatt: Use Core profile for LE scan

Lukasz Rymanowski (2):
  android/gatt: Add GATT Connect
  android/gatt: Add disconnect GATT device

 android/Android.mk  |   4 +
 android/Makefile.am |   3 +
 android/bluetooth.c | 105 ++++++++--
 android/bluetooth.h |   7 +
 android/gatt.c      | 589 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 691 insertions(+), 17 deletions(-)

-- 
1.8.4

[PATCH v2 1/4] android/bluetooth: Add GATT notifications on LE discovery

[Lukasz Rymanowski]

From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>

This patch introduce API which GATT can use to start/stop discovery
and register for required events.

This is because GATT needs to get from GAP notifications about
founded devices and also notification when discovery has been stopped.

GATT will need it explicity when GATT client calls scan, and also in
case of connect device, as before le connect is sent we do scan first
to make sure that device is available.

For now on adapter have two variables tracing discovery.
1. cur_discovery_type which show type of ongoing discovery type.
2. exp_discovery_type which shows type of next discovery session.

We need this because of scenarion when GATT is interesting in scan and
in the same time HAL wants to do scanning.
---
 android/bluetooth.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++------
 android/bluetooth.h |   7 ++++
 2 files changed, 100 insertions(+), 12 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 3e2e547..50a7393 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -84,6 +84,8 @@
 #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
 #define SCAN_TYPE_DUAL (SCAN_TYPE_BREDR | SCAN_TYPE_LE)
 
+#define BDADDR_LE (BDADDR_LE_RANDOM | BDADDR_LE_PUBLIC)
+
 struct device {
 	bdaddr_t bdaddr;
 	uint8_t bdaddr_type;
@@ -122,7 +124,8 @@ static struct {
 
 	uint32_t current_settings;
 
-	bool discovering;
+	uint8_t cur_discovery_type;
+	uint8_t exp_discovery_type;
 	uint32_t discoverable_timeout;
 
 	GSList *uuids;
@@ -131,7 +134,8 @@ static struct {
 	.dev_class = 0,
 	.name = NULL,
 	.current_settings = 0,
-	.discovering = false,
+	.cur_discovery_type = 0,
+	.exp_discovery_type = 0,
 	.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT,
 	.uuids = NULL,
 };
@@ -149,6 +153,10 @@ static struct mgmt *mgmt_if = NULL;
 static GSList *bonded_devices = NULL;
 static GSList *cached_devices = NULL;
 
+
+static bt_le_device_found gatt_device_found_cb = NULL;
+static bt_le_discovery_stopped gatt_discovery_stopped_cb = NULL;
+
 /* This list contains addresses which are asked for records */
 static GSList *browse_reqs;
 
@@ -509,7 +517,6 @@ static void settings_changed(uint32_t settings)
 	if (changed_mask & MGMT_SETTING_POWERED)
 		powered_changed();
 
-
 	scan_mode_mask = MGMT_SETTING_CONNECTABLE |
 					MGMT_SETTING_DISCOVERABLE;
 
@@ -1023,7 +1030,7 @@ static bool start_discovery(uint8_t type)
 	DBG("type=0x%x", cp.type);
 
 	if (mgmt_send(mgmt_if, MGMT_OP_START_DISCOVERY, adapter.index,
-				sizeof(cp), &cp, NULL, NULL, NULL) > 0)
+					sizeof(cp), &cp, NULL, NULL, NULL) > 0)
 		return true;
 
 	error("Failed to start discovery");
@@ -1036,6 +1043,7 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 {
 	const struct mgmt_ev_discovering *ev = param;
 	struct hal_ev_discovery_state_changed cp;
+	bool is_discovering = adapter.cur_discovery_type;
 
 	if (length < sizeof(*ev)) {
 		error("Too small discovering event");
@@ -1045,14 +1053,14 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 	DBG("hci%u type %u discovering %u", index, ev->type,
 							ev->discovering);
 
-	if (adapter.discovering == !!ev->discovering)
+	if (is_discovering == !!ev->discovering)
 		return;
 
-	adapter.discovering = !!ev->discovering;
+	adapter.cur_discovery_type = ev->discovering ? ev->type : 0;
 
 	DBG("new discovering state %u", ev->discovering);
 
-	if (adapter.discovering) {
+	if (adapter.cur_discovery_type) {
 		cp.state = HAL_DISCOVERY_STATE_STARTED;
 	} else {
 		g_slist_foreach(bonded_devices, clear_device_found, NULL);
@@ -1062,6 +1070,24 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 
 	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
 			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
+
+	if (gatt_discovery_stopped_cb && !adapter.cur_discovery_type) {
+		/* One shot notification about discovery stopped send to gatt*/
+		gatt_discovery_stopped_cb();
+		gatt_discovery_stopped_cb = NULL;
+		return;
+	}
+
+	/* If discovery is ON or there is no expected next discovery session
+	 * then just return
+	 */
+	if (adapter.cur_discovery_type || !adapter.exp_discovery_type)
+		return;
+
+	start_discovery(adapter.exp_discovery_type);
+
+	/* Maintain expected discovery type if there is gatt client registered */
+	adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
 }
 
 static void confirm_device_name_cb(uint8_t status, uint16_t length,
@@ -1143,7 +1169,7 @@ static void update_new_device(struct device *dev, int8_t rssi,
 
 	memset(buf, 0, sizeof(buf));
 
-	if (adapter.discovering)
+	if (adapter.cur_discovery_type)
 		dev->found = true;
 
 	size = sizeof(*ev);
@@ -1251,6 +1277,11 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 		update_device(dev, rssi, &eir);
 	}
 
+	/* Notify Gatt if its registered for LE events */
+	if (gatt_device_found_cb && (dev->bdaddr_type & BDADDR_LE))
+		gatt_device_found_cb(&dev->bdaddr, dev->bdaddr_type,
+						dev->rssi, sizeof(eir), &eir);
+
 	eir_data_free(&eir);
 
 	if (dev->bond_state != HAL_BOND_STATE_BONDED)
@@ -2511,6 +2542,38 @@ static bool stop_discovery(uint8_t type)
 	return false;
 }
 
+bool bt_le_discovery_stop(bt_le_discovery_stopped cb)
+{
+	if (!adapter.cur_discovery_type) {
+		if (cb)
+			cb();
+		return true;
+	}
+
+	gatt_discovery_stopped_cb = cb;
+	/* Remove device found callback */
+	gatt_device_found_cb = NULL;
+	adapter.exp_discovery_type &= ~SCAN_TYPE_LE;
+
+	return stop_discovery(adapter.cur_discovery_type);
+}
+
+bool bt_le_discovery_start(bt_le_device_found cb)
+{
+	if (!(adapter.current_settings & MGMT_SETTING_POWERED))
+		return false;
+
+	gatt_device_found_cb = cb;
+
+	adapter.exp_discovery_type |= SCAN_TYPE_LE;
+
+	/* If core is discovering, don't bother */
+	if (adapter.cur_discovery_type)
+		return true;
+
+	return start_discovery(adapter.exp_discovery_type);
+}
+
 static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
 {
 	const uint8_t *mode = buf;
@@ -3180,7 +3243,8 @@ static void handle_start_discovery_cmd(const void *buf, uint16_t len)
 {
 	uint8_t status;
 
-	if (adapter.discovering) {
+	/* Check if there is discovery with BREDR type */
+	if (adapter.cur_discovery_type & SCAN_TYPE_BREDR) {
 		status = HAL_STATUS_SUCCESS;
 		goto reply;
 	}
@@ -3190,12 +3254,26 @@ static void handle_start_discovery_cmd(const void *buf, uint16_t len)
 		goto reply;
 	}
 
-	if (!start_discovery(SCAN_TYPE_DUAL)) {
+	adapter.exp_discovery_type |= SCAN_TYPE_DUAL;
+
+	/* If there is no discovery ongoing, try to start discovery */
+	if (!adapter.cur_discovery_type) {
+		if (!start_discovery(adapter.exp_discovery_type))
+			status = HAL_STATUS_FAILED;
+		else
+			status = HAL_STATUS_SUCCESS;
+		goto reply;
+	}
+
+	/* Stop discovery here. Once it is stop we will restart it
+	 * with exp_discovery_settings */
+	if (!stop_discovery(adapter.cur_discovery_type)) {
 		status = HAL_STATUS_FAILED;
 		goto reply;
 	}
 
 	status = HAL_STATUS_SUCCESS;
+
 reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_START_DISCOVERY,
 									status);
@@ -3205,7 +3283,7 @@ static void handle_cancel_discovery_cmd(const void *buf, uint16_t len)
 {
 	uint8_t status;
 
-	if (!adapter.discovering) {
+	if (!adapter.cur_discovery_type) {
 		status = HAL_STATUS_SUCCESS;
 		goto reply;
 	}
@@ -3215,7 +3293,10 @@ static void handle_cancel_discovery_cmd(const void *buf, uint16_t len)
 		goto reply;
 	}
 
-	if (!stop_discovery(SCAN_TYPE_DUAL)) {
+	/* Take into account that gatt might want to keep discover */
+	adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
+
+	if (!stop_discovery(adapter.cur_discovery_type)) {
 		status = HAL_STATUS_FAILED;
 		goto reply;
 	}
diff --git a/android/bluetooth.h b/android/bluetooth.h
index f436178..a03305d 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -34,3 +34,10 @@ void bt_bluetooth_unregister(void);
 
 int bt_adapter_add_record(sdp_record_t *rec, uint8_t svc_hint);
 void bt_adapter_remove_record(uint32_t handle);
+
+typedef void (*bt_le_device_found)(bdaddr_t *addr, uint8_t addr_type, int rssi,
+					uint16_t eir_len, const void *eir);
+bool bt_le_discovery_start(bt_le_device_found cb);
+
+typedef void (*bt_le_discovery_stopped)(void);
+bool bt_le_discovery_stop(bt_le_discovery_stopped cb);
-- 
1.8.4

Re: [PATCH v2 1/4] android/bluetooth: Add GATT notifications on LE discovery

[Szymon Janc]

Hi,

On Friday 14 of March 2014 14:49:36 Lukasz Rymanowski wrote:
> From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>
> 
> This patch introduce API which GATT can use to start/stop discovery
> and register for required events.
> 
> This is because GATT needs to get from GAP notifications about
> founded devices and also notification when discovery has been stopped.
> 
> GATT will need it explicity when GATT client calls scan, and also in
> case of connect device, as before le connect is sent we do scan first
> to make sure that device is available.
> 
> For now on adapter have two variables tracing discovery.
> 1. cur_discovery_type which show type of ongoing discovery type.
> 2. exp_discovery_type which shows type of next discovery session.
> 
> We need this because of scenarion when GATT is interesting in scan and
> in the same time HAL wants to do scanning.
> ---
>  android/bluetooth.c | 105
> ++++++++++++++++++++++++++++++++++++++++++++++------ android/bluetooth.h | 
>  7 ++++
>  2 files changed, 100 insertions(+), 12 deletions(-)
> 
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 3e2e547..50a7393 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -84,6 +84,8 @@
>  #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
>  #define SCAN_TYPE_DUAL (SCAN_TYPE_BREDR | SCAN_TYPE_LE)
> 
> +#define BDADDR_LE (BDADDR_LE_RANDOM | BDADDR_LE_PUBLIC)
> +
>  struct device {
>  	bdaddr_t bdaddr;
>  	uint8_t bdaddr_type;
> @@ -122,7 +124,8 @@ static struct {
> 
>  	uint32_t current_settings;
> 
> -	bool discovering;
> +	uint8_t cur_discovery_type;
> +	uint8_t exp_discovery_type;
>  	uint32_t discoverable_timeout;
> 
>  	GSList *uuids;
> @@ -131,7 +134,8 @@ static struct {
>  	.dev_class = 0,
>  	.name = NULL,
>  	.current_settings = 0,
> -	.discovering = false,
> +	.cur_discovery_type = 0,
> +	.exp_discovery_type = 0,
>  	.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT,
>  	.uuids = NULL,
>  };
> @@ -149,6 +153,10 @@ static struct mgmt *mgmt_if = NULL;
>  static GSList *bonded_devices = NULL;
>  static GSList *cached_devices = NULL;
> 
> +

Unneeded empty line.

> +static bt_le_device_found gatt_device_found_cb = NULL;
> +static bt_le_discovery_stopped gatt_discovery_stopped_cb = NULL;
> +
>  /* This list contains addresses which are asked for records */
>  static GSList *browse_reqs;
> 
> @@ -509,7 +517,6 @@ static void settings_changed(uint32_t settings)
>  	if (changed_mask & MGMT_SETTING_POWERED)
>  		powered_changed();
> 
> -
>  	scan_mode_mask = MGMT_SETTING_CONNECTABLE |
>  					MGMT_SETTING_DISCOVERABLE;
> 
> @@ -1023,7 +1030,7 @@ static bool start_discovery(uint8_t type)
>  	DBG("type=0x%x", cp.type);
> 
>  	if (mgmt_send(mgmt_if, MGMT_OP_START_DISCOVERY, adapter.index,
> -				sizeof(cp), &cp, NULL, NULL, NULL) > 0)
> +					sizeof(cp), &cp, NULL, NULL, NULL) > 0)

This change is not related.

>  		return true;
> 
>  	error("Failed to start discovery");
> @@ -1036,6 +1043,7 @@ static void mgmt_discovering_event(uint16_t index,
> uint16_t length, {
>  	const struct mgmt_ev_discovering *ev = param;
>  	struct hal_ev_discovery_state_changed cp;
> +	bool is_discovering = adapter.cur_discovery_type;

I think we should have SCAN_TYPE_NONE defined and explicitly used for 
checking. Otherwise code is w bit hard to follow, especially if bool and non-
bool is mixed.

> 
>  	if (length < sizeof(*ev)) {
>  		error("Too small discovering event");
> @@ -1045,14 +1053,14 @@ static void mgmt_discovering_event(uint16_t index,
> uint16_t length, DBG("hci%u type %u discovering %u", index, ev->type,
>  							ev->discovering);
> 
> -	if (adapter.discovering == !!ev->discovering)
> +	if (is_discovering == !!ev->discovering)
>  		return;
> 
> -	adapter.discovering = !!ev->discovering;
> +	adapter.cur_discovery_type = ev->discovering ? ev->type : 0;
> 
>  	DBG("new discovering state %u", ev->discovering);
> 
> -	if (adapter.discovering) {
> +	if (adapter.cur_discovery_type) {

As mentioned above, this will be more readable with something like:
if (adapter.cur_discovery_type != SCAN_TYPE_NONE) ...

>  		cp.state = HAL_DISCOVERY_STATE_STARTED;
>  	} else {
>  		g_slist_foreach(bonded_devices, clear_device_found, NULL);
> @@ -1062,6 +1070,24 @@ static void mgmt_discovering_event(uint16_t index,
> uint16_t length,
> 
>  	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
>  			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
> +
> +	if (gatt_discovery_stopped_cb && !adapter.cur_discovery_type) {
> +		/* One shot notification about discovery stopped send to gatt*/
> +		gatt_discovery_stopped_cb();
> +		gatt_discovery_stopped_cb = NULL;
> +		return;
> +	}

You don't check expected discovery type here, is this ok to just return here?

> +
> +	/* If discovery is ON or there is no expected next discovery session
> +	 * then just return
> +	 */
> +	if (adapter.cur_discovery_type || !adapter.exp_discovery_type)
> +		return;
> +
> +	start_discovery(adapter.exp_discovery_type);
> +
> +	/* Maintain expected discovery type if there is gatt client registered */
> +	adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
>  }
> 
>  static void confirm_device_name_cb(uint8_t status, uint16_t length,
> @@ -1143,7 +1169,7 @@ static void update_new_device(struct device *dev,
> int8_t rssi,
> 
>  	memset(buf, 0, sizeof(buf));
> 
> -	if (adapter.discovering)
> +	if (adapter.cur_discovery_type)
>  		dev->found = true;
> 
>  	size = sizeof(*ev);
> @@ -1251,6 +1277,11 @@ static void update_found_device(const bdaddr_t
> *bdaddr, uint8_t bdaddr_type, update_device(dev, rssi, &eir);
>  	}
> 
> +	/* Notify Gatt if its registered for LE events */
> +	if (gatt_device_found_cb && (dev->bdaddr_type & BDADDR_LE))
> +		gatt_device_found_cb(&dev->bdaddr, dev->bdaddr_type,
> +						dev->rssi, sizeof(eir), &eir);
> +
>  	eir_data_free(&eir);
> 
>  	if (dev->bond_state != HAL_BOND_STATE_BONDED)
> @@ -2511,6 +2542,38 @@ static bool stop_discovery(uint8_t type)
>  	return false;
>  }
> 
> +bool bt_le_discovery_stop(bt_le_discovery_stopped cb)
> +{
> +	if (!adapter.cur_discovery_type) {
> +		if (cb)
> +			cb();
> +		return true;
> +	}
> +
> +	gatt_discovery_stopped_cb = cb;
> +	/* Remove device found callback */
> +	gatt_device_found_cb = NULL;
> +	adapter.exp_discovery_type &= ~SCAN_TYPE_LE;
> +
> +	return stop_discovery(adapter.cur_discovery_type);
> +}
> +
> +bool bt_le_discovery_start(bt_le_device_found cb)
> +{
> +	if (!(adapter.current_settings & MGMT_SETTING_POWERED))
> +		return false;
> +
> +	gatt_device_found_cb = cb;
> +
> +	adapter.exp_discovery_type |= SCAN_TYPE_LE;
> +
> +	/* If core is discovering, don't bother */
> +	if (adapter.cur_discovery_type)
> +		return true;
> +
> +	return start_discovery(adapter.exp_discovery_type);
> +}
> +
>  static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
>  {
>  	const uint8_t *mode = buf;
> @@ -3180,7 +3243,8 @@ static void handle_start_discovery_cmd(const void
> *buf, uint16_t len) {
>  	uint8_t status;
> 
> -	if (adapter.discovering) {
> +	/* Check if there is discovery with BREDR type */
> +	if (adapter.cur_discovery_type & SCAN_TYPE_BREDR) {
>  		status = HAL_STATUS_SUCCESS;
>  		goto reply;
>  	}
> @@ -3190,12 +3254,26 @@ static void handle_start_discovery_cmd(const void
> *buf, uint16_t len) goto reply;
>  	}
> 
> -	if (!start_discovery(SCAN_TYPE_DUAL)) {
> +	adapter.exp_discovery_type |= SCAN_TYPE_DUAL;
> +
> +	/* If there is no discovery ongoing, try to start discovery */
> +	if (!adapter.cur_discovery_type) {
> +		if (!start_discovery(adapter.exp_discovery_type))
> +			status = HAL_STATUS_FAILED;
> +		else
> +			status = HAL_STATUS_SUCCESS;

Ad empty line here.

> +		goto reply;
> +	}
> +
> +	/* Stop discovery here. Once it is stop we will restart it
> +	 * with exp_discovery_settings */
> +	if (!stop_discovery(adapter.cur_discovery_type)) {
>  		status = HAL_STATUS_FAILED;
>  		goto reply;
>  	}
> 
>  	status = HAL_STATUS_SUCCESS;
> +
>  reply:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_START_DISCOVERY,
>  									status);
> @@ -3205,7 +3283,7 @@ static void handle_cancel_discovery_cmd(const void
> *buf, uint16_t len) {
>  	uint8_t status;
> 
> -	if (!adapter.discovering) {
> +	if (!adapter.cur_discovery_type) {
>  		status = HAL_STATUS_SUCCESS;
>  		goto reply;
>  	}
> @@ -3215,7 +3293,10 @@ static void handle_cancel_discovery_cmd(const void
> *buf, uint16_t len) goto reply;
>  	}
> 
> -	if (!stop_discovery(SCAN_TYPE_DUAL)) {
> +	/* Take into account that gatt might want to keep discover */
> +	adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
> +
> +	if (!stop_discovery(adapter.cur_discovery_type)) {
>  		status = HAL_STATUS_FAILED;
>  		goto reply;
>  	}
> diff --git a/android/bluetooth.h b/android/bluetooth.h
> index f436178..a03305d 100644
> --- a/android/bluetooth.h
> +++ b/android/bluetooth.h
> @@ -34,3 +34,10 @@ void bt_bluetooth_unregister(void);
> 
>  int bt_adapter_add_record(sdp_record_t *rec, uint8_t svc_hint);
>  void bt_adapter_remove_record(uint32_t handle);
> +
> +typedef void (*bt_le_device_found)(bdaddr_t *addr, uint8_t addr_type, int
> rssi, +					uint16_t eir_len, const void *eir);
> +bool bt_le_discovery_start(bt_le_device_found cb);
> +
> +typedef void (*bt_le_discovery_stopped)(void);
> +bool bt_le_discovery_stop(bt_le_discovery_stopped cb);

-- 
BR
Szymon Janc

Re: [PATCH v2 1/4] android/bluetooth: Add GATT notifications on LE discovery

[Lukasz Rymanowski]

HI Szymon,

On Sun, Mar 16, 2014 at 5:26 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi,
>
> On Friday 14 of March 2014 14:49:36 Lukasz Rymanowski wrote:
>> From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>
>>
>> This patch introduce API which GATT can use to start/stop discovery
>> and register for required events.
>>
>> This is because GATT needs to get from GAP notifications about
>> founded devices and also notification when discovery has been stopped.
>>
>> GATT will need it explicity when GATT client calls scan, and also in
>> case of connect device, as before le connect is sent we do scan first
>> to make sure that device is available.
>>
>> For now on adapter have two variables tracing discovery.
>> 1. cur_discovery_type which show type of ongoing discovery type.
>> 2. exp_discovery_type which shows type of next discovery session.
>>
>> We need this because of scenarion when GATT is interesting in scan and
>> in the same time HAL wants to do scanning.
>> ---
>>  android/bluetooth.c | 105
>> ++++++++++++++++++++++++++++++++++++++++++++++------ android/bluetooth.h |
>>  7 ++++
>>  2 files changed, 100 insertions(+), 12 deletions(-)
>>
>> diff --git a/android/bluetooth.c b/android/bluetooth.c
>> index 3e2e547..50a7393 100644
>> --- a/android/bluetooth.c
>> +++ b/android/bluetooth.c
>> @@ -84,6 +84,8 @@
>>  #define SCAN_TYPE_LE ((1 << BDADDR_LE_PUBLIC) | (1 << BDADDR_LE_RANDOM))
>>  #define SCAN_TYPE_DUAL (SCAN_TYPE_BREDR | SCAN_TYPE_LE)
>>
>> +#define BDADDR_LE (BDADDR_LE_RANDOM | BDADDR_LE_PUBLIC)
>> +
>>  struct device {
>>       bdaddr_t bdaddr;
>>       uint8_t bdaddr_type;
>> @@ -122,7 +124,8 @@ static struct {
>>
>>       uint32_t current_settings;
>>
>> -     bool discovering;
>> +     uint8_t cur_discovery_type;
>> +     uint8_t exp_discovery_type;
>>       uint32_t discoverable_timeout;
>>
>>       GSList *uuids;
>> @@ -131,7 +134,8 @@ static struct {
>>       .dev_class = 0,
>>       .name = NULL,
>>       .current_settings = 0,
>> -     .discovering = false,
>> +     .cur_discovery_type = 0,
>> +     .exp_discovery_type = 0,
>>       .discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT,
>>       .uuids = NULL,
>>  };
>> @@ -149,6 +153,10 @@ static struct mgmt *mgmt_if = NULL;
>>  static GSList *bonded_devices = NULL;
>>  static GSList *cached_devices = NULL;
>>
>> +
>
> Unneeded empty line.
>
>> +static bt_le_device_found gatt_device_found_cb = NULL;
>> +static bt_le_discovery_stopped gatt_discovery_stopped_cb = NULL;
>> +
>>  /* This list contains addresses which are asked for records */
>>  static GSList *browse_reqs;
>>
>> @@ -509,7 +517,6 @@ static void settings_changed(uint32_t settings)
>>       if (changed_mask & MGMT_SETTING_POWERED)
>>               powered_changed();
>>
>> -
>>       scan_mode_mask = MGMT_SETTING_CONNECTABLE |
>>                                       MGMT_SETTING_DISCOVERABLE;
>>
>> @@ -1023,7 +1030,7 @@ static bool start_discovery(uint8_t type)
>>       DBG("type=0x%x", cp.type);
>>
>>       if (mgmt_send(mgmt_if, MGMT_OP_START_DISCOVERY, adapter.index,
>> -                             sizeof(cp), &cp, NULL, NULL, NULL) > 0)
>> +                                     sizeof(cp), &cp, NULL, NULL, NULL) > 0)
>
> This change is not related.
>
>>               return true;
>>
>>       error("Failed to start discovery");
>> @@ -1036,6 +1043,7 @@ static void mgmt_discovering_event(uint16_t index,
>> uint16_t length, {
>>       const struct mgmt_ev_discovering *ev = param;
>>       struct hal_ev_discovery_state_changed cp;
>> +     bool is_discovering = adapter.cur_discovery_type;
>
> I think we should have SCAN_TYPE_NONE defined and explicitly used for
> checking. Otherwise code is w bit hard to follow, especially if bool and non-
> bool is mixed.
>
>>
>>       if (length < sizeof(*ev)) {
>>               error("Too small discovering event");
>> @@ -1045,14 +1053,14 @@ static void mgmt_discovering_event(uint16_t index,
>> uint16_t length, DBG("hci%u type %u discovering %u", index, ev->type,
>>                                                       ev->discovering);
>>
>> -     if (adapter.discovering == !!ev->discovering)
>> +     if (is_discovering == !!ev->discovering)
>>               return;
>>
>> -     adapter.discovering = !!ev->discovering;
>> +     adapter.cur_discovery_type = ev->discovering ? ev->type : 0;
>>
>>       DBG("new discovering state %u", ev->discovering);
>>
>> -     if (adapter.discovering) {
>> +     if (adapter.cur_discovery_type) {
>
> As mentioned above, this will be more readable with something like:
> if (adapter.cur_discovery_type != SCAN_TYPE_NONE) ...
>
>>               cp.state = HAL_DISCOVERY_STATE_STARTED;
>>       } else {
>>               g_slist_foreach(bonded_devices, clear_device_found, NULL);
>> @@ -1062,6 +1070,24 @@ static void mgmt_discovering_event(uint16_t index,
>> uint16_t length,
>>
>>       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
>>                       HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
>> +
>> +     if (gatt_discovery_stopped_cb && !adapter.cur_discovery_type) {
>> +             /* One shot notification about discovery stopped send to gatt*/
>> +             gatt_discovery_stopped_cb();
>> +             gatt_discovery_stopped_cb = NULL;
>> +             return;
>> +     }
>
> You don't check expected discovery type here, is this ok to just return here?
>
>> +
>> +     /* If discovery is ON or there is no expected next discovery session
>> +      * then just return
>> +      */
>> +     if (adapter.cur_discovery_type || !adapter.exp_discovery_type)
>> +             return;
>> +
>> +     start_discovery(adapter.exp_discovery_type);
>> +
>> +     /* Maintain expected discovery type if there is gatt client registered */
>> +     adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
>>  }
>>
>>  static void confirm_device_name_cb(uint8_t status, uint16_t length,
>> @@ -1143,7 +1169,7 @@ static void update_new_device(struct device *dev,
>> int8_t rssi,
>>
>>       memset(buf, 0, sizeof(buf));
>>
>> -     if (adapter.discovering)
>> +     if (adapter.cur_discovery_type)
>>               dev->found = true;
>>
>>       size = sizeof(*ev);
>> @@ -1251,6 +1277,11 @@ static void update_found_device(const bdaddr_t
>> *bdaddr, uint8_t bdaddr_type, update_device(dev, rssi, &eir);
>>       }
>>
>> +     /* Notify Gatt if its registered for LE events */
>> +     if (gatt_device_found_cb && (dev->bdaddr_type & BDADDR_LE))
>> +             gatt_device_found_cb(&dev->bdaddr, dev->bdaddr_type,
>> +                                             dev->rssi, sizeof(eir), &eir);
>> +
>>       eir_data_free(&eir);
>>
>>       if (dev->bond_state != HAL_BOND_STATE_BONDED)
>> @@ -2511,6 +2542,38 @@ static bool stop_discovery(uint8_t type)
>>       return false;
>>  }
>>
>> +bool bt_le_discovery_stop(bt_le_discovery_stopped cb)
>> +{
>> +     if (!adapter.cur_discovery_type) {
>> +             if (cb)
>> +                     cb();
>> +             return true;
>> +     }
>> +
>> +     gatt_discovery_stopped_cb = cb;
>> +     /* Remove device found callback */
>> +     gatt_device_found_cb = NULL;
>> +     adapter.exp_discovery_type &= ~SCAN_TYPE_LE;
>> +
>> +     return stop_discovery(adapter.cur_discovery_type);
>> +}
>> +
>> +bool bt_le_discovery_start(bt_le_device_found cb)
>> +{
>> +     if (!(adapter.current_settings & MGMT_SETTING_POWERED))
>> +             return false;
>> +
>> +     gatt_device_found_cb = cb;
>> +
>> +     adapter.exp_discovery_type |= SCAN_TYPE_LE;
>> +
>> +     /* If core is discovering, don't bother */
>> +     if (adapter.cur_discovery_type)
>> +             return true;
>> +
>> +     return start_discovery(adapter.exp_discovery_type);
>> +}
>> +
>>  static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
>>  {
>>       const uint8_t *mode = buf;
>> @@ -3180,7 +3243,8 @@ static void handle_start_discovery_cmd(const void
>> *buf, uint16_t len) {
>>       uint8_t status;
>>
>> -     if (adapter.discovering) {
>> +     /* Check if there is discovery with BREDR type */
>> +     if (adapter.cur_discovery_type & SCAN_TYPE_BREDR) {
>>               status = HAL_STATUS_SUCCESS;
>>               goto reply;
>>       }
>> @@ -3190,12 +3254,26 @@ static void handle_start_discovery_cmd(const void
>> *buf, uint16_t len) goto reply;
>>       }
>>
>> -     if (!start_discovery(SCAN_TYPE_DUAL)) {
>> +     adapter.exp_discovery_type |= SCAN_TYPE_DUAL;
>> +
>> +     /* If there is no discovery ongoing, try to start discovery */
>> +     if (!adapter.cur_discovery_type) {
>> +             if (!start_discovery(adapter.exp_discovery_type))
>> +                     status = HAL_STATUS_FAILED;
>> +             else
>> +                     status = HAL_STATUS_SUCCESS;
>
> Ad empty line here.
>
>> +             goto reply;
>> +     }
>> +
>> +     /* Stop discovery here. Once it is stop we will restart it
>> +      * with exp_discovery_settings */
>> +     if (!stop_discovery(adapter.cur_discovery_type)) {
>>               status = HAL_STATUS_FAILED;
>>               goto reply;
>>       }
>>
>>       status = HAL_STATUS_SUCCESS;
>> +
>>  reply:
>>       ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_START_DISCOVERY,
>>                                                                       status);
>> @@ -3205,7 +3283,7 @@ static void handle_cancel_discovery_cmd(const void
>> *buf, uint16_t len) {
>>       uint8_t status;
>>
>> -     if (!adapter.discovering) {
>> +     if (!adapter.cur_discovery_type) {
>>               status = HAL_STATUS_SUCCESS;
>>               goto reply;
>>       }
>> @@ -3215,7 +3293,10 @@ static void handle_cancel_discovery_cmd(const void
>> *buf, uint16_t len) goto reply;
>>       }
>>
>> -     if (!stop_discovery(SCAN_TYPE_DUAL)) {
>> +     /* Take into account that gatt might want to keep discover */
>> +     adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
>> +
>> +     if (!stop_discovery(adapter.cur_discovery_type)) {
>>               status = HAL_STATUS_FAILED;
>>               goto reply;
>>       }
>> diff --git a/android/bluetooth.h b/android/bluetooth.h
>> index f436178..a03305d 100644
>> --- a/android/bluetooth.h
>> +++ b/android/bluetooth.h
>> @@ -34,3 +34,10 @@ void bt_bluetooth_unregister(void);
>>
>>  int bt_adapter_add_record(sdp_record_t *rec, uint8_t svc_hint);
>>  void bt_adapter_remove_record(uint32_t handle);
>> +
>> +typedef void (*bt_le_device_found)(bdaddr_t *addr, uint8_t addr_type, int
>> rssi, +                                       uint16_t eir_len, const void *eir);
>> +bool bt_le_discovery_start(bt_le_device_found cb);
>> +
>> +typedef void (*bt_le_discovery_stopped)(void);
>> +bool bt_le_discovery_stop(bt_le_discovery_stopped cb);
>

all comments will be handled in next patch version.

> --
> BR
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] android/gatt: Use Core profile for LE scan

[Lukasz Rymanowski]

From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>

This makes gatt capable of triggering LE scan using functionality
exposed by Core API. GATT registers its own callbacks for discovering
events.
---
 android/gatt.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 99 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index b566470..38f7c1b 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -32,10 +32,12 @@
 
 #include "ipc.h"
 #include "ipc-common.h"
-#include "lib/bluetooth.h"
+#include "lib/sdp.h"
+#include "bluetooth.h"
 #include "gatt.h"
 #include "src/log.h"
 #include "hal-msg.h"
+#include "utils.h"
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
 
@@ -47,6 +49,7 @@ struct gatt_client {
 static struct ipc *hal_ipc = NULL;
 static bdaddr_t adapter_addr;
 static struct queue *gatt_clients = NULL;
+static struct queue *scan_clients = NULL;
 
 static bool match_client_by_uuid(const void *data, const void *user_data)
 {
@@ -64,6 +67,36 @@ static bool match_client_by_id(const void *data, const void *user_data)
 	return client->id == exp_id;
 }
 
+static bool match_by_value(const void *data, const void *user_data)
+{
+	return data == user_data;
+}
+
+static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int rssi,
+					uint16_t eir_len, const void *eir)
+{
+	uint8_t buf[IPC_MTU];
+	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
+	char bda[18];
+
+	if (queue_isempty(scan_clients))
+		return;
+
+	ba2str(addr, bda);
+	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
+							eir ? true : false);
+
+	bdaddr2android(addr, ev->bda);
+	ev->rssi = rssi;
+	ev->len = eir_len;
+
+	memcpy(ev->adv_data, eir, ev->len);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+						HAL_EV_GATT_CLIENT_SCAN_RESULT,
+						sizeof(ev) + eir_len, ev);
+}
+
 static void handle_client_register(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_register *cmd = buf;
@@ -124,6 +157,13 @@ static void handle_client_unregister(const void *buf, uint16_t len)
 		goto failed;
 	}
 
+	queue_remove_if(scan_clients, match_by_value,
+						INT_TO_PTR(cmd->client_if));
+
+	/* If there is no client interesting in scan, just stop it */
+	if (queue_isempty(scan_clients))
+		bt_le_discovery_stop(NULL);
+
 	free(cl);
 	status = HAL_STATUS_SUCCESS;
 
@@ -134,10 +174,59 @@ failed:
 
 static void handle_client_scan(const void *buf, uint16_t len)
 {
-	DBG("");
+	const struct hal_cmd_gatt_client_scan *cmd = buf;
+	uint8_t status;
+	void *registered;
+	void *l;
+
+	DBG("new state %d", cmd->start);
 
+	registered = queue_find(gatt_clients, match_client_by_id,
+						INT_TO_PTR(cmd->client_if));
+	/* Turn off scan */
+	if (!cmd->start) {
+		if (registered)
+			queue_remove_if(scan_clients, match_by_value,
+						INT_TO_PTR(cmd->client_if));
+
+		if (queue_isempty(scan_clients)) {
+			DBG("Stopping LE SCAN");
+			bt_le_discovery_stop(NULL);
+		}
+
+		status = HAL_STATUS_SUCCESS;
+		goto reply;
+	}
+
+	/* If device already do scan, reply with success and avoid to add it
+	 * again to the list
+	 */
+	l = queue_find(scan_clients, match_by_value,
+						INT_TO_PTR(cmd->client_if));
+	if (l) {
+		status = HAL_STATUS_SUCCESS;
+		goto reply;
+	}
+
+	if (!bt_le_discovery_start(le_device_found_handler)) {
+		error("gatt: LE scan switch failed");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	/* Add scan client to the list if its registered */
+	if (registered && !queue_push_tail(scan_clients,
+						INT_TO_PTR(cmd->client_if))) {
+		error("gatt: Cannot push scan client");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	status = HAL_STATUS_SUCCESS;
+
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_SCAN,
-							HAL_STATUS_FAILED);
+									status);
 }
 
 static void handle_client_connect(const void *buf, uint16_t len)
@@ -530,6 +619,13 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 		return false;
 	}
 
+	scan_clients = queue_new();
+	if (!scan_clients) {
+		error("gatt: Cannot allocate scan_clients");
+		queue_destroy(gatt_clients, NULL);
+		return false;
+	}
+
 	return true;
 }
 
-- 
1.8.4

Re: [PATCH v2 2/4] android/gatt: Use Core profile for LE scan

[Szymon Janc]

Hi,

On Friday 14 of March 2014 14:49:37 Lukasz Rymanowski wrote:
> From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>
> 
> This makes gatt capable of triggering LE scan using functionality
> exposed by Core API. GATT registers its own callbacks for discovering
> events.
> ---
>  android/gatt.c | 102
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed,
> 99 insertions(+), 3 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index b566470..38f7c1b 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -32,10 +32,12 @@
> 
>  #include "ipc.h"
>  #include "ipc-common.h"
> -#include "lib/bluetooth.h"
> +#include "lib/sdp.h"
> +#include "bluetooth.h"
>  #include "gatt.h"
>  #include "src/log.h"
>  #include "hal-msg.h"
> +#include "utils.h"
>  #include "src/shared/util.h"
>  #include "src/shared/queue.h"
> 
> @@ -47,6 +49,7 @@ struct gatt_client {
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
>  static struct queue *gatt_clients = NULL;
> +static struct queue *scan_clients = NULL;
> 
>  static bool match_client_by_uuid(const void *data, const void *user_data)
>  {
> @@ -64,6 +67,36 @@ static bool match_client_by_id(const void *data, const
> void *user_data) return client->id == exp_id;
>  }
> 
> +static bool match_by_value(const void *data, const void *user_data)
> +{
> +	return data == user_data;
> +}

This is not directly related with this patch but I wonder if we should have 
API in queue for that...

> +
> +static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int
> rssi, +					uint16_t eir_len, const void *eir)
> +{
> +	uint8_t buf[IPC_MTU];
> +	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
> +	char bda[18];
> +
> +	if (queue_isempty(scan_clients))
> +		return;
> +
> +	ba2str(addr, bda);
> +	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
> +							eir ? true : false);

No need to prefix debugs, function name will be printed and is self 
explanatory.

> +
> +	bdaddr2android(addr, ev->bda);
> +	ev->rssi = rssi;
> +	ev->len = eir_len;
> +
> +	memcpy(ev->adv_data, eir, ev->len);
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +						HAL_EV_GATT_CLIENT_SCAN_RESULT,
> +						sizeof(ev) + eir_len, ev);

Use ev->len instead of eir_len here.

> +}
> +
>  static void handle_client_register(const void *buf, uint16_t len)
>  {
>  	const struct hal_cmd_gatt_client_register *cmd = buf;
> @@ -124,6 +157,13 @@ static void handle_client_unregister(const void *buf,
> uint16_t len) goto failed;
>  	}
> 
> +	queue_remove_if(scan_clients, match_by_value,
> +						INT_TO_PTR(cmd->client_if));
> +
> +	/* If there is no client interesting in scan, just stop it */
> +	if (queue_isempty(scan_clients))
> +		bt_le_discovery_stop(NULL);
> +
>  	free(cl);
>  	status = HAL_STATUS_SUCCESS;
> 
> @@ -134,10 +174,59 @@ failed:
> 
>  static void handle_client_scan(const void *buf, uint16_t len)
>  {
> -	DBG("");
> +	const struct hal_cmd_gatt_client_scan *cmd = buf;
> +	uint8_t status;
> +	void *registered;
> +	void *l;
> +
> +	DBG("new state %d", cmd->start);
> 
> +	registered = queue_find(gatt_clients, match_client_by_id,
> +						INT_TO_PTR(cmd->client_if));
> +	/* Turn off scan */
> +	if (!cmd->start) {
> +		if (registered)
> +			queue_remove_if(scan_clients, match_by_value,
> +						INT_TO_PTR(cmd->client_if));
> +
> +		if (queue_isempty(scan_clients)) {
> +			DBG("Stopping LE SCAN");
> +			bt_le_discovery_stop(NULL);
> +		}
> +
> +		status = HAL_STATUS_SUCCESS;
> +		goto reply;
> +	}
> +
> +	/* If device already do scan, reply with success and avoid to add it
> +	 * again to the list
> +	 */
> +	l = queue_find(scan_clients, match_by_value,
> +						INT_TO_PTR(cmd->client_if));
> +	if (l) {
> +		status = HAL_STATUS_SUCCESS;
> +		goto reply;
> +	}
> +
> +	if (!bt_le_discovery_start(le_device_found_handler)) {
> +		error("gatt: LE scan switch failed");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	/* Add scan client to the list if its registered */
> +	if (registered && !queue_push_tail(scan_clients,
> +						INT_TO_PTR(cmd->client_if))) {
> +		error("gatt: Cannot push scan client");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;
> +
> +reply:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_SCAN,
> -							HAL_STATUS_FAILED);
> +									status);
>  }
> 
>  static void handle_client_connect(const void *buf, uint16_t len)
> @@ -530,6 +619,13 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t
> *addr) return false;
>  	}
> 
> +	scan_clients = queue_new();
> +	if (!scan_clients) {
> +		error("gatt: Cannot allocate scan_clients");
> +		queue_destroy(gatt_clients, NULL);
> +		return false;
> +	}
> +
>  	return true;
>  }

-- 
BR
Szymon Janc

Re: [PATCH v2 2/4] android/gatt: Use Core profile for LE scan

[Lukasz Rymanowski]

Hi Szymon,

On Sun, Mar 16, 2014 at 5:26 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi,
>
> On Friday 14 of March 2014 14:49:37 Lukasz Rymanowski wrote:
>> From: Jakub Tyszkowski <jakub.tyszkowski@tieto.com>
>>
>> This makes gatt capable of triggering LE scan using functionality
>> exposed by Core API. GATT registers its own callbacks for discovering
>> events.
>> ---
>>  android/gatt.c | 102
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed,
>> 99 insertions(+), 3 deletions(-)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index b566470..38f7c1b 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -32,10 +32,12 @@
>>
>>  #include "ipc.h"
>>  #include "ipc-common.h"
>> -#include "lib/bluetooth.h"
>> +#include "lib/sdp.h"
>> +#include "bluetooth.h"
>>  #include "gatt.h"
>>  #include "src/log.h"
>>  #include "hal-msg.h"
>> +#include "utils.h"
>>  #include "src/shared/util.h"
>>  #include "src/shared/queue.h"
>>
>> @@ -47,6 +49,7 @@ struct gatt_client {
>>  static struct ipc *hal_ipc = NULL;
>>  static bdaddr_t adapter_addr;
>>  static struct queue *gatt_clients = NULL;
>> +static struct queue *scan_clients = NULL;
>>
>>  static bool match_client_by_uuid(const void *data, const void *user_data)
>>  {
>> @@ -64,6 +67,36 @@ static bool match_client_by_id(const void *data, const
>> void *user_data) return client->id == exp_id;
>>  }
>>
>> +static bool match_by_value(const void *data, const void *user_data)
>> +{
>> +     return data == user_data;
>> +}
>
> This is not directly related with this patch but I wonder if we should have
> API in queue for that...
>
>> +
>> +static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int
>> rssi, +                                       uint16_t eir_len, const void *eir)
>> +{
>> +     uint8_t buf[IPC_MTU];
>> +     struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
>> +     char bda[18];
>> +
>> +     if (queue_isempty(scan_clients))
>> +             return;
>> +
>> +     ba2str(addr, bda);
>> +     DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
>> +                                                     eir ? true : false);
>
> No need to prefix debugs, function name will be printed and is self
> explanatory.
>
>> +
>> +     bdaddr2android(addr, ev->bda);
>> +     ev->rssi = rssi;
>> +     ev->len = eir_len;
>> +
>> +     memcpy(ev->adv_data, eir, ev->len);
>> +
>> +     ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                                             HAL_EV_GATT_CLIENT_SCAN_RESULT,
>> +                                             sizeof(ev) + eir_len, ev);
>
> Use ev->len instead of eir_len here.
>
>> +}
>> +
>>  static void handle_client_register(const void *buf, uint16_t len)
>>  {
>>       const struct hal_cmd_gatt_client_register *cmd = buf;
>> @@ -124,6 +157,13 @@ static void handle_client_unregister(const void *buf,
>> uint16_t len) goto failed;
>>       }
>>
>> +     queue_remove_if(scan_clients, match_by_value,
>> +                                             INT_TO_PTR(cmd->client_if));
>> +
>> +     /* If there is no client interesting in scan, just stop it */
>> +     if (queue_isempty(scan_clients))
>> +             bt_le_discovery_stop(NULL);
>> +
>>       free(cl);
>>       status = HAL_STATUS_SUCCESS;
>>
>> @@ -134,10 +174,59 @@ failed:
>>
>>  static void handle_client_scan(const void *buf, uint16_t len)
>>  {
>> -     DBG("");
>> +     const struct hal_cmd_gatt_client_scan *cmd = buf;
>> +     uint8_t status;
>> +     void *registered;
>> +     void *l;
>> +
>> +     DBG("new state %d", cmd->start);
>>
>> +     registered = queue_find(gatt_clients, match_client_by_id,
>> +                                             INT_TO_PTR(cmd->client_if));
>> +     /* Turn off scan */
>> +     if (!cmd->start) {
>> +             if (registered)
>> +                     queue_remove_if(scan_clients, match_by_value,
>> +                                             INT_TO_PTR(cmd->client_if));
>> +
>> +             if (queue_isempty(scan_clients)) {
>> +                     DBG("Stopping LE SCAN");
>> +                     bt_le_discovery_stop(NULL);
>> +             }
>> +
>> +             status = HAL_STATUS_SUCCESS;
>> +             goto reply;
>> +     }
>> +
>> +     /* If device already do scan, reply with success and avoid to add it
>> +      * again to the list
>> +      */
>> +     l = queue_find(scan_clients, match_by_value,
>> +                                             INT_TO_PTR(cmd->client_if));
>> +     if (l) {
>> +             status = HAL_STATUS_SUCCESS;
>> +             goto reply;
>> +     }
>> +
>> +     if (!bt_le_discovery_start(le_device_found_handler)) {
>> +             error("gatt: LE scan switch failed");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     /* Add scan client to the list if its registered */
>> +     if (registered && !queue_push_tail(scan_clients,
>> +                                             INT_TO_PTR(cmd->client_if))) {
>> +             error("gatt: Cannot push scan client");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     status = HAL_STATUS_SUCCESS;
>> +
>> +reply:
>>       ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_SCAN,
>> -                                                     HAL_STATUS_FAILED);
>> +                                                                     status);
>>  }
>>
>>  static void handle_client_connect(const void *buf, uint16_t len)
>> @@ -530,6 +619,13 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t
>> *addr) return false;
>>       }
>>
>> +     scan_clients = queue_new();
>> +     if (!scan_clients) {
>> +             error("gatt: Cannot allocate scan_clients");
>> +             queue_destroy(gatt_clients, NULL);
>> +             return false;
>> +     }
>> +
>>       return true;
>>  }
>
Next version will fix all those issues.

> --
> BR
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/4] android/gatt: Add GATT Connect

[Lukasz Rymanowski]

This patch introduce connect LE device functionality.

There is gatt_device representing remote le device. Each gatt device
has a list own list of clients as it is possible that more apps
would like to use same remote device.

Possible connect scenarios:

1.  There is no ACL connection to device:
Then new dev is put on conn_wait_queue and le scan is enabled.
Once device is found we do connect it.

Once device is connected then device is moved form conn_wait_queue to
conn_list and success event is sent to client(s) with conn_id

2. Device is already connected:
Then we update client list, reply with success and do send connect event.

3. For unregisterd clients or uknown conn_id, failed response is sent.
---
 android/Android.mk  |   4 +
 android/Makefile.am |   3 +
 android/gatt.c      | 475 +++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 463 insertions(+), 19 deletions(-)

diff --git a/android/Android.mk b/android/Android.mk
index 34e21ea..0e0932d 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -60,9 +60,13 @@ LOCAL_SRC_FILES := \
 	bluez/lib/sdp.c \
 	bluez/lib/bluetooth.c \
 	bluez/lib/hci.c \
+	bluez/lib/uuid.c \
 	bluez/btio/btio.c \
 	bluez/src/sdp-client.c \
 	bluez/profiles/network/bnep.c \
+	bluez/attrib/gattrib.c \
+	bluez/attrib/gatt.c \
+	bluez/attrib/att.c
 
 LOCAL_C_INCLUDES := \
 	$(call include-path-for, glib) \
diff --git a/android/Makefile.am b/android/Makefile.am
index e7de560..1d0747e 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -42,6 +42,9 @@ android_bluetoothd_SOURCES = android/main.c \
 				android/handsfree.h android/handsfree.c \
 				android/gatt.h android/gatt.c \
 				android/health.h android/health.c \
+				attrib/att.c attrib/att.h \
+				attrib/gatt.c attrib/gatt.h \
+				attrib/gattrib.c attrib/gattrib.h \
 				btio/btio.h btio/btio.c \
 				src/sdp-client.h src/sdp-client.c \
 				profiles/network/bnep.h profiles/network/bnep.c
diff --git a/android/gatt.c b/android/gatt.c
index 38f7c1b..9afdf4b 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -29,10 +29,13 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <glib.h>
+#include <errno.h>
+#include <sys/socket.h>
 
 #include "ipc.h"
 #include "ipc-common.h"
 #include "lib/sdp.h"
+#include "lib/uuid.h"
 #include "bluetooth.h"
 #include "gatt.h"
 #include "src/log.h"
@@ -40,16 +43,38 @@
 #include "utils.h"
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
+#include "attrib/gattrib.h"
+#include "attrib/att.h"
+#include "attrib/gatt.h"
+#include "btio/btio.h"
 
 struct gatt_client {
 	int32_t id;
 	uint8_t uuid[16];
 };
 
+struct gatt_device {
+	bdaddr_t bdaddr;
+	uint8_t bdaddr_type;
+
+	struct queue *clients;
+
+	bool connect_ready;
+	int32_t conn_id;
+
+	GAttrib *attrib;
+	GIOChannel *att_io;
+
+	guint watch_id;
+};
+
 static struct ipc *hal_ipc = NULL;
 static bdaddr_t adapter_addr;
+
 static struct queue *gatt_clients = NULL;
 static struct queue *scan_clients = NULL;
+static struct queue *conn_list	= NULL;		/* Connected devices */
+static struct queue *conn_wait_queue = NULL;	/* Devices waiting for connect */
 
 static bool match_client_by_uuid(const void *data, const void *user_data)
 {
@@ -72,29 +97,25 @@ static bool match_by_value(const void *data, const void *user_data)
 	return data == user_data;
 }
 
-static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int rssi,
-					uint16_t eir_len, const void *eir)
+static bool match_dev_by_bdaddr(const void *data, const void *user_data)
 {
-	uint8_t buf[IPC_MTU];
-	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
-	char bda[18];
+	const struct gatt_device *dev = data;
+	const bdaddr_t *addr = user_data;
 
-	if (queue_isempty(scan_clients))
-		return;
-
-	ba2str(addr, bda);
-	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
-							eir ? true : false);
+	return !bacmp(&dev->bdaddr, addr);
+}
 
-	bdaddr2android(addr, ev->bda);
-	ev->rssi = rssi;
-	ev->len = eir_len;
+static bool match_dev_connect_ready(const void *data, const void *user_data)
+{
+	const struct gatt_device *dev = data;
 
-	memcpy(ev->adv_data, eir, ev->len);
+	return dev->connect_ready;
+}
 
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
-						HAL_EV_GATT_CLIENT_SCAN_RESULT,
-						sizeof(ev) + eir_len, ev);
+static void destroy_device(struct gatt_device *dev)
+{
+	queue_destroy(dev->clients, NULL);
+	free(dev);
 }
 
 static void handle_client_register(const void *buf, uint16_t len)
@@ -172,6 +193,253 @@ failed:
 					HAL_OP_GATT_CLIENT_UNREGISTER, status);
 }
 
+static void connection_cleanup(struct gatt_device *device)
+{
+	if (device->watch_id) {
+		g_source_remove(device->watch_id);
+		device->watch_id = 0;
+	}
+
+	if (device->att_io) {
+		g_io_channel_shutdown(device->att_io, FALSE, NULL);
+		g_io_channel_unref(device->att_io);
+		device->att_io = NULL;
+	}
+
+	if (device->attrib) {
+		GAttrib *attrib = device->attrib;
+		device->attrib = NULL;
+		g_attrib_cancel_all(attrib);
+		g_attrib_unref(attrib);
+	}
+}
+
+static void send_disconnect_notify(void *data, void *user_data)
+{
+	struct hal_ev_gatt_client_disconnect ev;
+	struct gatt_device *dev = user_data;
+
+	ev.client_if = PTR_TO_INT(data);
+	ev.conn_id = dev->conn_id;
+	ev.status = HAL_STATUS_SUCCESS;
+	bdaddr2android(&dev->bdaddr, &ev.bda);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+			HAL_EV_GATT_CLIENT_DISCONNECT, sizeof(ev), &ev);
+}
+
+static bool is_device_wating_for_connect(const bdaddr_t *addr, uint8_t addr_type)
+{
+	struct gatt_device *dev;
+
+	DBG("");
+
+	dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, (void *)addr);
+	if (!dev)
+		return false;
+
+	dev->bdaddr_type = addr_type;
+
+	/* Mark that this device is ready for connect.
+	 * Need it because will continue with connect after scan is stopped
+	 */
+	dev->connect_ready = true;
+
+	return true;
+}
+
+static void bt_le_discovery_stop_cb(void);
+
+static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type,
+						int rssi, uint16_t eir_len,
+							const void *eir)
+{
+	uint8_t buf[IPC_MTU];
+	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
+	char bda[18];
+
+	if (queue_isempty(scan_clients))
+		goto connect;
+
+	ba2str(addr, bda);
+	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
+									!!eir);
+
+	bdaddr2android(addr, ev->bda);
+	ev->rssi = rssi;
+	ev->len = eir_len;
+
+	memcpy(ev->adv_data, eir, ev->len);
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+						HAL_EV_GATT_CLIENT_SCAN_RESULT,
+						sizeof(ev) + eir_len, ev);
+
+connect:
+	if (!is_device_wating_for_connect(addr, addr_type))
+		return;
+
+	/* We are ok to perform connect now. Stop discovery
+	* and once it is stopped continue with creating ACL
+	*/
+	bt_le_discovery_stop(bt_le_discovery_stop_cb);
+}
+
+static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	bdaddr_t *addr = user_data;
+	struct gatt_device *dev;
+	int sock, err = 0;
+	socklen_t len;
+
+	sock = g_io_channel_unix_get_fd(io);
+	len = sizeof(err);
+	getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len);
+
+	DBG("%s (%d)", strerror(err), err);
+
+	dev = queue_remove_if(conn_list, match_dev_by_bdaddr, addr);
+	connection_cleanup(dev);
+
+	/* Keep scanning/re-connection active if disconnection reason
+	 * is connection timeout, remote user terminated connection or local
+	 * initiated disconnection.
+	 */
+	if (err == ETIMEDOUT || err == ECONNRESET || err == ECONNABORTED) {
+		if (!queue_push_tail(conn_wait_queue, dev)) {
+			error("gatt: Cannot push data");
+		} else {
+			bt_le_discovery_start(le_device_found_handler);
+			return FALSE;
+		}
+	}
+
+	queue_foreach(dev->clients, send_disconnect_notify, dev);
+	destroy_device(dev);
+
+	return FALSE;
+}
+
+static void send_client_connect_notify(void *data, void *user_data)
+{
+	struct hal_ev_gatt_client_connect *ev = user_data;
+	int32_t id = PTR_TO_INT(data);
+
+	ev->client_if = id;
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+				HAL_EV_GATT_CLIENT_CONNECT, sizeof(*ev), ev);
+
+}
+static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
+{
+	bdaddr_t *addr = user_data;
+	struct gatt_device *dev;
+	struct hal_ev_gatt_client_connect ev;
+	GAttrib *attrib;
+	static uint32_t conn_id = 0;
+	uint8_t status;
+
+	/* Take device form conn waiting queue */
+	dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
+	if (!dev) {
+		error("gatt: Device not on the connect wait queue!?");
+		g_io_channel_shutdown(io, TRUE, NULL);
+		return;
+	}
+
+	g_io_channel_unref(dev->att_io);
+	dev->att_io = NULL;
+
+	/* Set address and client id in the event */
+	bdaddr2android(&dev->bdaddr, &ev.bda);
+
+	if (gerr) {
+		error("gatt: connection failed %s", gerr->message);
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	attrib = g_attrib_new(io);
+	if (!attrib) {
+		error("gatt: unable to create new GAttrib instance");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	dev->attrib = attrib;
+	dev->watch_id = g_io_add_watch(io, G_IO_HUP, disconnected_cb, dev);
+	dev->conn_id = ++conn_id;
+
+	/* Move gatt device from connect queue to conn_list */
+	if (!queue_push_tail(conn_list, dev)) {
+		error("gatt: Cannot push dev on conn_list");
+		connection_cleanup(dev);
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	status = HAL_STATUS_SUCCESS;
+	goto reply;
+
+reply:
+	ev.conn_id = dev ? conn_id : 0;
+	ev.status = status;
+
+	queue_foreach(dev->clients, send_client_connect_notify, &ev);
+
+	/* If connection did not succeed, destroy device */
+	if (status)
+		destroy_device(dev);
+}
+
+static int connect_le(struct gatt_device *dev)
+{
+	BtIOSecLevel sec_level;
+	GIOChannel *io;
+	GError *gerr = NULL;
+	char addr[18];
+
+	ba2str(&dev->bdaddr, addr);
+
+	/* There is one connection attempt going on */
+	if (dev->att_io) {
+		info("gatt: connection to dev %s is ongoing", addr);
+		return -EALREADY;
+	}
+
+	DBG("Connection attempt to: %s", addr);
+
+	/*TODO: If we are bonded then we should use higier sec level */
+	sec_level = BT_IO_SEC_LOW;
+
+	/*
+	 * This connection will help us catch any PDUs that comes before
+	 * pairing finishes
+	 */
+	io = bt_io_connect(connect_cb, dev, NULL, &gerr,
+			BT_IO_OPT_SOURCE_BDADDR,
+			&adapter_addr,
+			BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
+			BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
+			BT_IO_OPT_DEST_TYPE, dev->bdaddr_type,
+			BT_IO_OPT_CID, ATT_CID,
+			BT_IO_OPT_SEC_LEVEL, sec_level,
+			BT_IO_OPT_INVALID);
+	if (!io) {
+		error("gatt: Failed bt_io_connect(%s): %s", addr,
+							gerr->message);
+		g_error_free(gerr);
+		return -EIO;
+	}
+
+	/* Keep this, so we can cancel the connection */
+	dev->att_io = io;
+
+	return 0;
+}
+
 static void handle_client_scan(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_scan *cmd = buf;
@@ -229,12 +497,166 @@ reply:
 									status);
 }
 
+static int connect_next_dev(void)
+{
+	struct gatt_device *dev;
+
+	DBG("");
+
+	if (queue_isempty(conn_wait_queue))
+		return 0;
+
+	/* Discovery has been stopped because there is connection waiting */
+	dev = queue_find(conn_wait_queue, match_dev_connect_ready, NULL);
+	if (!dev)
+		/* Lets try again. */
+		return -1;
+
+	dev->connect_ready = false;
+
+	return connect_le(dev);
+}
+
+static void bt_le_discovery_stop_cb(void)
+{
+	DBG("");
+
+	/* Check now if there is any device ready to connect*/
+	if (connect_next_dev() < 0)
+		bt_le_discovery_start(le_device_found_handler);
+}
+
+static struct gatt_device *find_device(bdaddr_t *addr)
+{
+	struct gatt_device *dev;
+
+	dev = queue_find(conn_list, match_dev_by_bdaddr, addr);
+	if (dev)
+		return dev;
+
+	dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, addr);
+	if (dev)
+		return dev;
+
+	return NULL;
+}
+
 static void handle_client_connect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_gatt_client_connect *cmd = buf;
+	struct gatt_device *dev = NULL;
+	void *l;
+	bdaddr_t addr;
+	uint8_t status;
+	bool send_notify = false;
+
 	DBG("");
 
+	/* Check if client is registered */
+	l = queue_find(gatt_clients, match_client_by_id,
+						INT_TO_PTR(cmd->client_if));
+	if (!l) {
+		error("gatt: Client id %d not found", cmd->client_if);
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	android2bdaddr(&cmd->bdaddr, &addr);
+
+	/* We do support many clients for one device connection so lets check
+	  * If device is connected or in connecting state just update list of
+	  * clients
+	  */
+	dev = find_device(&addr);
+	if (dev) {
+
+		status = HAL_STATUS_SUCCESS;
+
+		/* Remeber to send dummy notification event  if we area
+		 * connected
+		 */
+		if (dev->conn_id)
+			send_notify = true;
+
+		if (queue_find(dev->clients, match_by_value,
+						INT_TO_PTR(cmd->client_if)))
+				goto reply;
+
+		/* Store another client */
+		if (!queue_push_tail(dev->clients,
+						INT_TO_PTR(cmd->client_if))) {
+			error("gatt: Cannot push client on gatt device list");
+			status = HAL_STATUS_FAILED;
+			goto reply;
+		}
+
+		goto reply;
+	}
+
+	/* Lets create new gatt device and put it on conn_wait_queue.
+	  * Once it is connected we move it to conn_list
+	  */
+	dev = new0(struct gatt_device, 1);
+	if (!dev) {
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	memcpy(&dev->bdaddr, &addr, sizeof(bdaddr_t));
+
+	/* Create queue to keep list of clients for given device*/
+	dev->clients = queue_new();
+	if (!dev->clients) {
+		error("gatt: Cannot create client queue");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	/* Update client list of device */
+	if (!queue_push_tail(dev->clients, INT_TO_PTR(cmd->client_if))) {
+		error("gatt: Cannot push client on the client queue!?");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	/* Start le scan if not started */
+	if (queue_isempty(scan_clients)) {
+		if (!bt_le_discovery_start(le_device_found_handler)) {
+			error("gatt: Could not start scan");
+			status = HAL_STATUS_FAILED;
+			goto reply;
+		}
+	}
+
+	if (!queue_push_tail(conn_wait_queue, dev)) {
+		error("gatt: Cannot push device on conn_wait_queue");
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	status = HAL_STATUS_SUCCESS;
+	goto reply;
+
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_CONNECT,
-							HAL_STATUS_FAILED);
+								status);
+
+	/* Send dummy notification since ACL is already up*/
+	if (send_notify) {
+		struct hal_ev_gatt_client_connect ev;
+
+		ev.conn_id = dev->conn_id;
+		ev.status = HAL_STATUS_SUCCESS;
+		ev.client_if = cmd->client_if;
+		bdaddr2android(&addr, &ev.bda);
+
+		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
+						HAL_EV_GATT_CLIENT_CONNECT,
+						sizeof(ev), &ev);
+	}
+
+	if (status && dev)
+		destroy_device(dev);
 }
 
 static void handle_client_disconnect(const void *buf, uint16_t len)
@@ -610,6 +1032,18 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
 
 	hal_ipc = ipc;
 
+	conn_list = queue_new();
+	if (!conn_list) {
+		error("gatt: Can not create conn queue");
+		return false;
+	}
+
+	conn_wait_queue = queue_new();
+	if (!conn_wait_queue) {
+		error("gatt: Can not create conn queue");
+		return false;
+	}
+
 	ipc_register(hal_ipc, HAL_SERVICE_ID_GATT, cmd_handlers,
 						G_N_ELEMENTS(cmd_handlers));
 
@@ -637,4 +1071,7 @@ void bt_gatt_unregister(void)
 
 	ipc_unregister(hal_ipc, HAL_SERVICE_ID_GATT);
 	hal_ipc = NULL;
+
+	queue_destroy(conn_list, NULL);
+	queue_destroy(conn_wait_queue, NULL);
 }
-- 
1.8.4

Re: [PATCH v2 3/4] android/gatt: Add GATT Connect

[Szymon Janc]

Hi,

On Friday 14 of March 2014 14:49:38 Lukasz Rymanowski wrote:
> This patch introduce connect LE device functionality.
> 
> There is gatt_device representing remote le device. Each gatt device
> has a list own list of clients as it is possible that more apps
> would like to use same remote device.
> 
> Possible connect scenarios:
> 
> 1.  There is no ACL connection to device:
> Then new dev is put on conn_wait_queue and le scan is enabled.
> Once device is found we do connect it.
> 
> Once device is connected then device is moved form conn_wait_queue to
> conn_list and success event is sent to client(s) with conn_id
> 
> 2. Device is already connected:
> Then we update client list, reply with success and do send connect event.
> 
> 3. For unregisterd clients or uknown conn_id, failed response is sent.
> ---
>  android/Android.mk  |   4 +
>  android/Makefile.am |   3 +
>  android/gatt.c      | 475
> +++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 463
> insertions(+), 19 deletions(-)
> 
> diff --git a/android/Android.mk b/android/Android.mk
> index 34e21ea..0e0932d 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -60,9 +60,13 @@ LOCAL_SRC_FILES := \
>  	bluez/lib/sdp.c \
>  	bluez/lib/bluetooth.c \
>  	bluez/lib/hci.c \
> +	bluez/lib/uuid.c \
>  	bluez/btio/btio.c \
>  	bluez/src/sdp-client.c \
>  	bluez/profiles/network/bnep.c \
> +	bluez/attrib/gattrib.c \
> +	bluez/attrib/gatt.c \
> +	bluez/attrib/att.c
> 
>  LOCAL_C_INCLUDES := \
>  	$(call include-path-for, glib) \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index e7de560..1d0747e 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -42,6 +42,9 @@ android_bluetoothd_SOURCES = android/main.c \
>  				android/handsfree.h android/handsfree.c \
>  				android/gatt.h android/gatt.c \
>  				android/health.h android/health.c \
> +				attrib/att.c attrib/att.h \
> +				attrib/gatt.c attrib/gatt.h \
> +				attrib/gattrib.c attrib/gattrib.h \
>  				btio/btio.h btio/btio.c \
>  				src/sdp-client.h src/sdp-client.c \
>  				profiles/network/bnep.h profiles/network/bnep.c
> diff --git a/android/gatt.c b/android/gatt.c
> index 38f7c1b..9afdf4b 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -29,10 +29,13 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <glib.h>
> +#include <errno.h>
> +#include <sys/socket.h>
> 
>  #include "ipc.h"
>  #include "ipc-common.h"
>  #include "lib/sdp.h"
> +#include "lib/uuid.h"
>  #include "bluetooth.h"
>  #include "gatt.h"
>  #include "src/log.h"
> @@ -40,16 +43,38 @@
>  #include "utils.h"
>  #include "src/shared/util.h"
>  #include "src/shared/queue.h"
> +#include "attrib/gattrib.h"
> +#include "attrib/att.h"
> +#include "attrib/gatt.h"
> +#include "btio/btio.h"
> 
>  struct gatt_client {
>  	int32_t id;
>  	uint8_t uuid[16];
>  };
> 
> +struct gatt_device {
> +	bdaddr_t bdaddr;
> +	uint8_t bdaddr_type;
> +
> +	struct queue *clients;
> +
> +	bool connect_ready;
> +	int32_t conn_id;
> +
> +	GAttrib *attrib;
> +	GIOChannel *att_io;
> +
> +	guint watch_id;
> +};
> +
>  static struct ipc *hal_ipc = NULL;
>  static bdaddr_t adapter_addr;
> +
>  static struct queue *gatt_clients = NULL;
>  static struct queue *scan_clients = NULL;
> +static struct queue *conn_list	= NULL;		/* Connected devices */
> +static struct queue *conn_wait_queue = NULL;	/* Devices waiting for connect
> */
> 
>  static bool match_client_by_uuid(const void *data, const void *user_data)
>  {
> @@ -72,29 +97,25 @@ static bool match_by_value(const void *data, const void
> *user_data) return data == user_data;
>  }
> 
> -static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int
> rssi, -					uint16_t eir_len, const void *eir)
> +static bool match_dev_by_bdaddr(const void *data, const void *user_data)
>  {
> -	uint8_t buf[IPC_MTU];
> -	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
> -	char bda[18];
> +	const struct gatt_device *dev = data;
> +	const bdaddr_t *addr = user_data;
> 
> -	if (queue_isempty(scan_clients))
> -		return;
> -
> -	ba2str(addr, bda);
> -	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
> -							eir ? true : false);
> +	return !bacmp(&dev->bdaddr, addr);
> +}
> 
> -	bdaddr2android(addr, ev->bda);
> -	ev->rssi = rssi;
> -	ev->len = eir_len;
> +static bool match_dev_connect_ready(const void *data, const void
> *user_data) +{
> +	const struct gatt_device *dev = data;
> 
> -	memcpy(ev->adv_data, eir, ev->len);
> +	return dev->connect_ready;
> +}
> 
> -	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> -						HAL_EV_GATT_CLIENT_SCAN_RESULT,
> -						sizeof(ev) + eir_len, ev);
> +static void destroy_device(struct gatt_device *dev)
> +{
> +	queue_destroy(dev->clients, NULL);
> +	free(dev);
>  }
> 
>  static void handle_client_register(const void *buf, uint16_t len)
> @@ -172,6 +193,253 @@ failed:
>  					HAL_OP_GATT_CLIENT_UNREGISTER, status);
>  }
> 
> +static void connection_cleanup(struct gatt_device *device)
> +{
> +	if (device->watch_id) {
> +		g_source_remove(device->watch_id);
> +		device->watch_id = 0;
> +	}
> +
> +	if (device->att_io) {
> +		g_io_channel_shutdown(device->att_io, FALSE, NULL);
> +		g_io_channel_unref(device->att_io);
> +		device->att_io = NULL;
> +	}
> +
> +	if (device->attrib) {
> +		GAttrib *attrib = device->attrib;
> +		device->attrib = NULL;
> +		g_attrib_cancel_all(attrib);
> +		g_attrib_unref(attrib);
> +	}
> +}
> +
> +static void send_disconnect_notify(void *data, void *user_data)
> +{
> +	struct hal_ev_gatt_client_disconnect ev;
> +	struct gatt_device *dev = user_data;
> +
> +	ev.client_if = PTR_TO_INT(data);
> +	ev.conn_id = dev->conn_id;
> +	ev.status = HAL_STATUS_SUCCESS;
> +	bdaddr2android(&dev->bdaddr, &ev.bda);
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +			HAL_EV_GATT_CLIENT_DISCONNECT, sizeof(ev), &ev);
> +}

This should have proper types as parameters and if needed dedicated wrapper 
callback for list iteration should be added.

> +
> +static bool is_device_wating_for_connect(const bdaddr_t *addr, uint8_t
> addr_type) +{
> +	struct gatt_device *dev;
> +
> +	DBG("");
> +
> +	dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, (void *)addr);

No need cast to (void *) here.

> +	if (!dev)
> +		return false;
> +
> +	dev->bdaddr_type = addr_type;
> +
> +	/* Mark that this device is ready for connect.
> +	 * Need it because will continue with connect after scan is stopped
> +	 */
> +	dev->connect_ready = true;
> +
> +	return true;
> +}
> +
> +static void bt_le_discovery_stop_cb(void);

If this is really needed I'd prefer to have this before any function 
definitions at file's top.

> +
> +static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type,
> +						int rssi, uint16_t eir_len,
> +							const void *eir)
> +{
> +	uint8_t buf[IPC_MTU];
> +	struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
> +	char bda[18];
> +
> +	if (queue_isempty(scan_clients))
> +		goto connect;
> +
> +	ba2str(addr, bda);
> +	DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
> +									!!eir);
> +
> +	bdaddr2android(addr, ev->bda);
> +	ev->rssi = rssi;
> +	ev->len = eir_len;
> +
> +	memcpy(ev->adv_data, eir, ev->len);
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +						HAL_EV_GATT_CLIENT_SCAN_RESULT,
> +						sizeof(ev) + eir_len, ev);

Use ev->len here instead of eir_len.

> +
> +connect:
> +	if (!is_device_wating_for_connect(addr, addr_type))
> +		return;
> +
> +	/* We are ok to perform connect now. Stop discovery
> +	* and once it is stopped continue with creating ACL
> +	*/
> +	bt_le_discovery_stop(bt_le_discovery_stop_cb);
> +}
> +
> +static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
> +							gpointer user_data)
> +{
> +	bdaddr_t *addr = user_data;
> +	struct gatt_device *dev;
> +	int sock, err = 0;
> +	socklen_t len;
> +
> +	sock = g_io_channel_unix_get_fd(io);
> +	len = sizeof(err);
> +	getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len);

Should check if getsockopt() succeed. Also this watch should probably check 
for G_IO_ERR and G_IO_NVAL as well.

> +
> +	DBG("%s (%d)", strerror(err), err);
> +
> +	dev = queue_remove_if(conn_list, match_dev_by_bdaddr, addr);
> +	connection_cleanup(dev);
> +
> +	/* Keep scanning/re-connection active if disconnection reason
> +	 * is connection timeout, remote user terminated connection or local
> +	 * initiated disconnection.
> +	 */
> +	if (err == ETIMEDOUT || err == ECONNRESET || err == ECONNABORTED) {
> +		if (!queue_push_tail(conn_wait_queue, dev)) {
> +			error("gatt: Cannot push data");
> +		} else {
> +			bt_le_discovery_start(le_device_found_handler);
> +			return FALSE;
> +		}
> +	}
> +
> +	queue_foreach(dev->clients, send_disconnect_notify, dev);
> +	destroy_device(dev);
> +
> +	return FALSE;
> +}
> +
> +static void send_client_connect_notify(void *data, void *user_data)
> +{
> +	struct hal_ev_gatt_client_connect *ev = user_data;
> +	int32_t id = PTR_TO_INT(data);
> +
> +	ev->client_if = id;
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +				HAL_EV_GATT_CLIENT_CONNECT, sizeof(*ev), ev);
> +
> +}
> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
> +{
> +	bdaddr_t *addr = user_data;
> +	struct gatt_device *dev;
> +	struct hal_ev_gatt_client_connect ev;
> +	GAttrib *attrib;
> +	static uint32_t conn_id = 0;
> +	uint8_t status;
> +
> +	/* Take device form conn waiting queue */

typo: form->from

> +	dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
> +	if (!dev) {
> +		error("gatt: Device not on the connect wait queue!?");
> +		g_io_channel_shutdown(io, TRUE, NULL);
> +		return;
> +	}
> +
> +	g_io_channel_unref(dev->att_io);
> +	dev->att_io = NULL;
> +
> +	/* Set address and client id in the event */
> +	bdaddr2android(&dev->bdaddr, &ev.bda);
> +
> +	if (gerr) {
> +		error("gatt: connection failed %s", gerr->message);
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	attrib = g_attrib_new(io);
> +	if (!attrib) {
> +		error("gatt: unable to create new GAttrib instance");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	dev->attrib = attrib;
> +	dev->watch_id = g_io_add_watch(io, G_IO_HUP, disconnected_cb, dev);
> +	dev->conn_id = ++conn_id;
> +
> +	/* Move gatt device from connect queue to conn_list */
> +	if (!queue_push_tail(conn_list, dev)) {
> +		error("gatt: Cannot push dev on conn_list");
> +		connection_cleanup(dev);
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;
> +	goto reply;
> +
> +reply:
> +	ev.conn_id = dev ? conn_id : 0;

I would do  dev ? dev->conn_id : 0 for clarity.

> +	ev.status = status;
> +
> +	queue_foreach(dev->clients, send_client_connect_notify, &ev);
> +
> +	/* If connection did not succeed, destroy device */
> +	if (status)
> +		destroy_device(dev);
> +}
> +
> +static int connect_le(struct gatt_device *dev)
> +{
> +	BtIOSecLevel sec_level;
> +	GIOChannel *io;
> +	GError *gerr = NULL;
> +	char addr[18];
> +
> +	ba2str(&dev->bdaddr, addr);
> +
> +	/* There is one connection attempt going on */
> +	if (dev->att_io) {
> +		info("gatt: connection to dev %s is ongoing", addr);
> +		return -EALREADY;
> +	}
> +
> +	DBG("Connection attempt to: %s", addr);
> +
> +	/*TODO: If we are bonded then we should use higier sec level */
> +	sec_level = BT_IO_SEC_LOW;
> +
> +	/*
> +	 * This connection will help us catch any PDUs that comes before
> +	 * pairing finishes
> +	 */
> +	io = bt_io_connect(connect_cb, dev, NULL, &gerr,
> +			BT_IO_OPT_SOURCE_BDADDR,
> +			&adapter_addr,
> +			BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
> +			BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
> +			BT_IO_OPT_DEST_TYPE, dev->bdaddr_type,
> +			BT_IO_OPT_CID, ATT_CID,
> +			BT_IO_OPT_SEC_LEVEL, sec_level,
> +			BT_IO_OPT_INVALID);
> +	if (!io) {
> +		error("gatt: Failed bt_io_connect(%s): %s", addr,
> +							gerr->message);
> +		g_error_free(gerr);
> +		return -EIO;
> +	}
> +
> +	/* Keep this, so we can cancel the connection */
> +	dev->att_io = io;
> +
> +	return 0;
> +}
> +
>  static void handle_client_scan(const void *buf, uint16_t len)
>  {
>  	const struct hal_cmd_gatt_client_scan *cmd = buf;
> @@ -229,12 +497,166 @@ reply:
>  									status);
>  }
> 
> +static int connect_next_dev(void)
> +{
> +	struct gatt_device *dev;
> +
> +	DBG("");
> +
> +	if (queue_isempty(conn_wait_queue))
> +		return 0;
> +
> +	/* Discovery has been stopped because there is connection waiting */
> +	dev = queue_find(conn_wait_queue, match_dev_connect_ready, NULL);
> +	if (!dev)
> +		/* Lets try again. */
> +		return -1;
> +
> +	dev->connect_ready = false;
> +
> +	return connect_le(dev);
> +}
> +
> +static void bt_le_discovery_stop_cb(void)
> +{
> +	DBG("");
> +
> +	/* Check now if there is any device ready to connect*/
> +	if (connect_next_dev() < 0)
> +		bt_le_discovery_start(le_device_found_handler);
> +}
> +
> +static struct gatt_device *find_device(bdaddr_t *addr)
> +{
> +	struct gatt_device *dev;
> +
> +	dev = queue_find(conn_list, match_dev_by_bdaddr, addr);
> +	if (dev)
> +		return dev;
> +
> +	dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, addr);
> +	if (dev)
> +		return dev;
> +
> +	return NULL;
> +}
> +
>  static void handle_client_connect(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_gatt_client_connect *cmd = buf;
> +	struct gatt_device *dev = NULL;
> +	void *l;
> +	bdaddr_t addr;
> +	uint8_t status;
> +	bool send_notify = false;
> +
>  	DBG("");
> 
> +	/* Check if client is registered */
> +	l = queue_find(gatt_clients, match_client_by_id,
> +						INT_TO_PTR(cmd->client_if));
> +	if (!l) {
> +		error("gatt: Client id %d not found", cmd->client_if);
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	android2bdaddr(&cmd->bdaddr, &addr);
> +
> +	/* We do support many clients for one device connection so lets check
> +	  * If device is connected or in connecting state just update list of
> +	  * clients
> +	  */
> +	dev = find_device(&addr);
> +	if (dev) {
> +
> +		status = HAL_STATUS_SUCCESS;

Set status before goto only.

> +
> +		/* Remeber to send dummy notification event  if we area
> +		 * connected
> +		 */
> +		if (dev->conn_id)
> +			send_notify = true;
> +
> +		if (queue_find(dev->clients, match_by_value,
> +						INT_TO_PTR(cmd->client_if)))
> +				goto reply;
> +
> +		/* Store another client */
> +		if (!queue_push_tail(dev->clients,
> +						INT_TO_PTR(cmd->client_if))) {
> +			error("gatt: Cannot push client on gatt device list");
> +			status = HAL_STATUS_FAILED;
> +			goto reply;
> +		}
> +
> +		goto reply;
> +	}
> +
> +	/* Lets create new gatt device and put it on conn_wait_queue.
> +	  * Once it is connected we move it to conn_list
> +	  */
> +	dev = new0(struct gatt_device, 1);
> +	if (!dev) {
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	memcpy(&dev->bdaddr, &addr, sizeof(bdaddr_t));
> +
> +	/* Create queue to keep list of clients for given device*/
> +	dev->clients = queue_new();
> +	if (!dev->clients) {
> +		error("gatt: Cannot create client queue");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	/* Update client list of device */
> +	if (!queue_push_tail(dev->clients, INT_TO_PTR(cmd->client_if))) {
> +		error("gatt: Cannot push client on the client queue!?");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	/* Start le scan if not started */
> +	if (queue_isempty(scan_clients)) {
> +		if (!bt_le_discovery_start(le_device_found_handler)) {
> +			error("gatt: Could not start scan");
> +			status = HAL_STATUS_FAILED;
> +			goto reply;
> +		}
> +	}
> +
> +	if (!queue_push_tail(conn_wait_queue, dev)) {
> +		error("gatt: Cannot push device on conn_wait_queue");
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	status = HAL_STATUS_SUCCESS;
> +	goto reply;
> +

This goto is not needed :)

> +reply:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_CONNECT,
> -							HAL_STATUS_FAILED);
> +								status);
> +
> +	/* Send dummy notification since ACL is already up*/
> +	if (send_notify) {
> +		struct hal_ev_gatt_client_connect ev;
> +
> +		ev.conn_id = dev->conn_id;
> +		ev.status = HAL_STATUS_SUCCESS;
> +		ev.client_if = cmd->client_if;
> +		bdaddr2android(&addr, &ev.bda);
> +
> +		ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> +						HAL_EV_GATT_CLIENT_CONNECT,
> +						sizeof(ev), &ev);
> +	}
> +

You are already dereferencing dev in if above. 

> +	if (status && dev)
> +		destroy_device(dev);
>  }
> 
>  static void handle_client_disconnect(const void *buf, uint16_t len)
> @@ -610,6 +1032,18 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t
> *addr)
> 
>  	hal_ipc = ipc;
> 
> +	conn_list = queue_new();
> +	if (!conn_list) {
> +		error("gatt: Can not create conn queue");
> +		return false;
> +	}
> +
> +	conn_wait_queue = queue_new();
> +	if (!conn_wait_queue) {
> +		error("gatt: Can not create conn queue");
> +		return false;
> +	}
> +
>  	ipc_register(hal_ipc, HAL_SERVICE_ID_GATT, cmd_handlers,
>  						G_N_ELEMENTS(cmd_handlers));
> 
> @@ -637,4 +1071,7 @@ void bt_gatt_unregister(void)
> 
>  	ipc_unregister(hal_ipc, HAL_SERVICE_ID_GATT);
>  	hal_ipc = NULL;
> +
> +	queue_destroy(conn_list, NULL);
> +	queue_destroy(conn_wait_queue, NULL);

Full those lists here for completeness.

>  }

-- 
BR
Szymon Janc

Re: [PATCH v2 3/4] android/gatt: Add GATT Connect

[Lukasz Rymanowski]

Hi Szymon,

On Sun, Mar 16, 2014 at 6:32 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi,
>
> On Friday 14 of March 2014 14:49:38 Lukasz Rymanowski wrote:
>> This patch introduce connect LE device functionality.
>>
>> There is gatt_device representing remote le device. Each gatt device
>> has a list own list of clients as it is possible that more apps
>> would like to use same remote device.
>>
>> Possible connect scenarios:
>>
>> 1.  There is no ACL connection to device:
>> Then new dev is put on conn_wait_queue and le scan is enabled.
>> Once device is found we do connect it.
>>
>> Once device is connected then device is moved form conn_wait_queue to
>> conn_list and success event is sent to client(s) with conn_id
>>
>> 2. Device is already connected:
>> Then we update client list, reply with success and do send connect event.
>>
>> 3. For unregisterd clients or uknown conn_id, failed response is sent.
>> ---
>>  android/Android.mk  |   4 +
>>  android/Makefile.am |   3 +
>>  android/gatt.c      | 475
>> +++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 463
>> insertions(+), 19 deletions(-)
>>
>> diff --git a/android/Android.mk b/android/Android.mk
>> index 34e21ea..0e0932d 100644
>> --- a/android/Android.mk
>> +++ b/android/Android.mk
>> @@ -60,9 +60,13 @@ LOCAL_SRC_FILES := \
>>       bluez/lib/sdp.c \
>>       bluez/lib/bluetooth.c \
>>       bluez/lib/hci.c \
>> +     bluez/lib/uuid.c \
>>       bluez/btio/btio.c \
>>       bluez/src/sdp-client.c \
>>       bluez/profiles/network/bnep.c \
>> +     bluez/attrib/gattrib.c \
>> +     bluez/attrib/gatt.c \
>> +     bluez/attrib/att.c
>>
>>  LOCAL_C_INCLUDES := \
>>       $(call include-path-for, glib) \
>> diff --git a/android/Makefile.am b/android/Makefile.am
>> index e7de560..1d0747e 100644
>> --- a/android/Makefile.am
>> +++ b/android/Makefile.am
>> @@ -42,6 +42,9 @@ android_bluetoothd_SOURCES = android/main.c \
>>                               android/handsfree.h android/handsfree.c \
>>                               android/gatt.h android/gatt.c \
>>                               android/health.h android/health.c \
>> +                             attrib/att.c attrib/att.h \
>> +                             attrib/gatt.c attrib/gatt.h \
>> +                             attrib/gattrib.c attrib/gattrib.h \
>>                               btio/btio.h btio/btio.c \
>>                               src/sdp-client.h src/sdp-client.c \
>>                               profiles/network/bnep.h profiles/network/bnep.c
>> diff --git a/android/gatt.c b/android/gatt.c
>> index 38f7c1b..9afdf4b 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -29,10 +29,13 @@
>>  #include <stdlib.h>
>>  #include <stdint.h>
>>  #include <glib.h>
>> +#include <errno.h>
>> +#include <sys/socket.h>
>>
>>  #include "ipc.h"
>>  #include "ipc-common.h"
>>  #include "lib/sdp.h"
>> +#include "lib/uuid.h"
>>  #include "bluetooth.h"
>>  #include "gatt.h"
>>  #include "src/log.h"
>> @@ -40,16 +43,38 @@
>>  #include "utils.h"
>>  #include "src/shared/util.h"
>>  #include "src/shared/queue.h"
>> +#include "attrib/gattrib.h"
>> +#include "attrib/att.h"
>> +#include "attrib/gatt.h"
>> +#include "btio/btio.h"
>>
>>  struct gatt_client {
>>       int32_t id;
>>       uint8_t uuid[16];
>>  };
>>
>> +struct gatt_device {
>> +     bdaddr_t bdaddr;
>> +     uint8_t bdaddr_type;
>> +
>> +     struct queue *clients;
>> +
>> +     bool connect_ready;
>> +     int32_t conn_id;
>> +
>> +     GAttrib *attrib;
>> +     GIOChannel *att_io;
>> +
>> +     guint watch_id;
>> +};
>> +
>>  static struct ipc *hal_ipc = NULL;
>>  static bdaddr_t adapter_addr;
>> +
>>  static struct queue *gatt_clients = NULL;
>>  static struct queue *scan_clients = NULL;
>> +static struct queue *conn_list       = NULL;         /* Connected devices */
>> +static struct queue *conn_wait_queue = NULL; /* Devices waiting for connect
>> */
>>
>>  static bool match_client_by_uuid(const void *data, const void *user_data)
>>  {
>> @@ -72,29 +97,25 @@ static bool match_by_value(const void *data, const void
>> *user_data) return data == user_data;
>>  }
>>
>> -static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type, int
>> rssi, -                                       uint16_t eir_len, const void *eir)
>> +static bool match_dev_by_bdaddr(const void *data, const void *user_data)
>>  {
>> -     uint8_t buf[IPC_MTU];
>> -     struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
>> -     char bda[18];
>> +     const struct gatt_device *dev = data;
>> +     const bdaddr_t *addr = user_data;
>>
>> -     if (queue_isempty(scan_clients))
>> -             return;
>> -
>> -     ba2str(addr, bda);
>> -     DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
>> -                                                     eir ? true : false);
>> +     return !bacmp(&dev->bdaddr, addr);
>> +}
>>
>> -     bdaddr2android(addr, ev->bda);
>> -     ev->rssi = rssi;
>> -     ev->len = eir_len;
>> +static bool match_dev_connect_ready(const void *data, const void
>> *user_data) +{
>> +     const struct gatt_device *dev = data;
>>
>> -     memcpy(ev->adv_data, eir, ev->len);
>> +     return dev->connect_ready;
>> +}
>>
>> -     ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> -                                             HAL_EV_GATT_CLIENT_SCAN_RESULT,
>> -                                             sizeof(ev) + eir_len, ev);
>> +static void destroy_device(struct gatt_device *dev)
>> +{
>> +     queue_destroy(dev->clients, NULL);
>> +     free(dev);
>>  }
>>
>>  static void handle_client_register(const void *buf, uint16_t len)
>> @@ -172,6 +193,253 @@ failed:
>>                                       HAL_OP_GATT_CLIENT_UNREGISTER, status);
>>  }
>>
>> +static void connection_cleanup(struct gatt_device *device)
>> +{
>> +     if (device->watch_id) {
>> +             g_source_remove(device->watch_id);
>> +             device->watch_id = 0;
>> +     }
>> +
>> +     if (device->att_io) {
>> +             g_io_channel_shutdown(device->att_io, FALSE, NULL);
>> +             g_io_channel_unref(device->att_io);
>> +             device->att_io = NULL;
>> +     }
>> +
>> +     if (device->attrib) {
>> +             GAttrib *attrib = device->attrib;
>> +             device->attrib = NULL;
>> +             g_attrib_cancel_all(attrib);
>> +             g_attrib_unref(attrib);
>> +     }
>> +}
>> +
>> +static void send_disconnect_notify(void *data, void *user_data)
>> +{
>> +     struct hal_ev_gatt_client_disconnect ev;
>> +     struct gatt_device *dev = user_data;
>> +
>> +     ev.client_if = PTR_TO_INT(data);
>> +     ev.conn_id = dev->conn_id;
>> +     ev.status = HAL_STATUS_SUCCESS;
>> +     bdaddr2android(&dev->bdaddr, &ev.bda);
>> +
>> +     ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                     HAL_EV_GATT_CLIENT_DISCONNECT, sizeof(ev), &ev);
>> +}
>
> This should have proper types as parameters and if needed dedicated wrapper
> callback for list iteration should be added.
>
>> +
>> +static bool is_device_wating_for_connect(const bdaddr_t *addr, uint8_t
>> addr_type) +{
>> +     struct gatt_device *dev;
>> +
>> +     DBG("");
>> +
>> +     dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, (void *)addr);
>
> No need cast to (void *) here.

Actually we do need this. addr is const and queue_find wants non const
user_data. We would think about changing it in queue in the future.
Other comments will be fixed  in next patch version.

>
>> +     if (!dev)
>> +             return false;
>> +
>> +     dev->bdaddr_type = addr_type;
>> +
>> +     /* Mark that this device is ready for connect.
>> +      * Need it because will continue with connect after scan is stopped
>> +      */
>> +     dev->connect_ready = true;
>> +
>> +     return true;
>> +}
>> +
>> +static void bt_le_discovery_stop_cb(void);
>
> If this is really needed I'd prefer to have this before any function
> definitions at file's top.
>
>> +
>> +static void le_device_found_handler(bdaddr_t *addr, uint8_t addr_type,
>> +                                             int rssi, uint16_t eir_len,
>> +                                                     const void *eir)
>> +{
>> +     uint8_t buf[IPC_MTU];
>> +     struct hal_ev_gatt_client_scan_result *ev = (void *) buf;
>> +     char bda[18];
>> +
>> +     if (queue_isempty(scan_clients))
>> +             goto connect;
>> +
>> +     ba2str(addr, bda);
>> +     DBG("gatt: LE Device found: %s, rssi: %d, adv_data: %d", bda, rssi,
>> +                                                                     !!eir);
>> +
>> +     bdaddr2android(addr, ev->bda);
>> +     ev->rssi = rssi;
>> +     ev->len = eir_len;
>> +
>> +     memcpy(ev->adv_data, eir, ev->len);
>> +
>> +     ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                                             HAL_EV_GATT_CLIENT_SCAN_RESULT,
>> +                                             sizeof(ev) + eir_len, ev);
>
> Use ev->len here instead of eir_len.
>
>> +
>> +connect:
>> +     if (!is_device_wating_for_connect(addr, addr_type))
>> +             return;
>> +
>> +     /* We are ok to perform connect now. Stop discovery
>> +     * and once it is stopped continue with creating ACL
>> +     */
>> +     bt_le_discovery_stop(bt_le_discovery_stop_cb);
>> +}
>> +
>> +static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
>> +                                                     gpointer user_data)
>> +{
>> +     bdaddr_t *addr = user_data;
>> +     struct gatt_device *dev;
>> +     int sock, err = 0;
>> +     socklen_t len;
>> +
>> +     sock = g_io_channel_unix_get_fd(io);
>> +     len = sizeof(err);
>> +     getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len);
>
> Should check if getsockopt() succeed. Also this watch should probably check
> for G_IO_ERR and G_IO_NVAL as well.
>
>> +
>> +     DBG("%s (%d)", strerror(err), err);
>> +
>> +     dev = queue_remove_if(conn_list, match_dev_by_bdaddr, addr);
>> +     connection_cleanup(dev);
>> +
>> +     /* Keep scanning/re-connection active if disconnection reason
>> +      * is connection timeout, remote user terminated connection or local
>> +      * initiated disconnection.
>> +      */
>> +     if (err == ETIMEDOUT || err == ECONNRESET || err == ECONNABORTED) {
>> +             if (!queue_push_tail(conn_wait_queue, dev)) {
>> +                     error("gatt: Cannot push data");
>> +             } else {
>> +                     bt_le_discovery_start(le_device_found_handler);
>> +                     return FALSE;
>> +             }
>> +     }
>> +
>> +     queue_foreach(dev->clients, send_disconnect_notify, dev);
>> +     destroy_device(dev);
>> +
>> +     return FALSE;
>> +}
>> +
>> +static void send_client_connect_notify(void *data, void *user_data)
>> +{
>> +     struct hal_ev_gatt_client_connect *ev = user_data;
>> +     int32_t id = PTR_TO_INT(data);
>> +
>> +     ev->client_if = id;
>> +
>> +     ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                             HAL_EV_GATT_CLIENT_CONNECT, sizeof(*ev), ev);
>> +
>> +}
>> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>> +{
>> +     bdaddr_t *addr = user_data;
>> +     struct gatt_device *dev;
>> +     struct hal_ev_gatt_client_connect ev;
>> +     GAttrib *attrib;
>> +     static uint32_t conn_id = 0;
>> +     uint8_t status;
>> +
>> +     /* Take device form conn waiting queue */
>
> typo: form->from
>
>> +     dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
>> +     if (!dev) {
>> +             error("gatt: Device not on the connect wait queue!?");
>> +             g_io_channel_shutdown(io, TRUE, NULL);
>> +             return;
>> +     }
>> +
>> +     g_io_channel_unref(dev->att_io);
>> +     dev->att_io = NULL;
>> +
>> +     /* Set address and client id in the event */
>> +     bdaddr2android(&dev->bdaddr, &ev.bda);
>> +
>> +     if (gerr) {
>> +             error("gatt: connection failed %s", gerr->message);
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     attrib = g_attrib_new(io);
>> +     if (!attrib) {
>> +             error("gatt: unable to create new GAttrib instance");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     dev->attrib = attrib;
>> +     dev->watch_id = g_io_add_watch(io, G_IO_HUP, disconnected_cb, dev);
>> +     dev->conn_id = ++conn_id;
>> +
>> +     /* Move gatt device from connect queue to conn_list */
>> +     if (!queue_push_tail(conn_list, dev)) {
>> +             error("gatt: Cannot push dev on conn_list");
>> +             connection_cleanup(dev);
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     status = HAL_STATUS_SUCCESS;
>> +     goto reply;
>> +
>> +reply:
>> +     ev.conn_id = dev ? conn_id : 0;
>
> I would do  dev ? dev->conn_id : 0 for clarity.
>
>> +     ev.status = status;
>> +
>> +     queue_foreach(dev->clients, send_client_connect_notify, &ev);
>> +
>> +     /* If connection did not succeed, destroy device */
>> +     if (status)
>> +             destroy_device(dev);
>> +}
>> +
>> +static int connect_le(struct gatt_device *dev)
>> +{
>> +     BtIOSecLevel sec_level;
>> +     GIOChannel *io;
>> +     GError *gerr = NULL;
>> +     char addr[18];
>> +
>> +     ba2str(&dev->bdaddr, addr);
>> +
>> +     /* There is one connection attempt going on */
>> +     if (dev->att_io) {
>> +             info("gatt: connection to dev %s is ongoing", addr);
>> +             return -EALREADY;
>> +     }
>> +
>> +     DBG("Connection attempt to: %s", addr);
>> +
>> +     /*TODO: If we are bonded then we should use higier sec level */
>> +     sec_level = BT_IO_SEC_LOW;
>> +
>> +     /*
>> +      * This connection will help us catch any PDUs that comes before
>> +      * pairing finishes
>> +      */
>> +     io = bt_io_connect(connect_cb, dev, NULL, &gerr,
>> +                     BT_IO_OPT_SOURCE_BDADDR,
>> +                     &adapter_addr,
>> +                     BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
>> +                     BT_IO_OPT_DEST_BDADDR, &dev->bdaddr,
>> +                     BT_IO_OPT_DEST_TYPE, dev->bdaddr_type,
>> +                     BT_IO_OPT_CID, ATT_CID,
>> +                     BT_IO_OPT_SEC_LEVEL, sec_level,
>> +                     BT_IO_OPT_INVALID);
>> +     if (!io) {
>> +             error("gatt: Failed bt_io_connect(%s): %s", addr,
>> +                                                     gerr->message);
>> +             g_error_free(gerr);
>> +             return -EIO;
>> +     }
>> +
>> +     /* Keep this, so we can cancel the connection */
>> +     dev->att_io = io;
>> +
>> +     return 0;
>> +}
>> +
>>  static void handle_client_scan(const void *buf, uint16_t len)
>>  {
>>       const struct hal_cmd_gatt_client_scan *cmd = buf;
>> @@ -229,12 +497,166 @@ reply:
>>                                                                       status);
>>  }
>>
>> +static int connect_next_dev(void)
>> +{
>> +     struct gatt_device *dev;
>> +
>> +     DBG("");
>> +
>> +     if (queue_isempty(conn_wait_queue))
>> +             return 0;
>> +
>> +     /* Discovery has been stopped because there is connection waiting */
>> +     dev = queue_find(conn_wait_queue, match_dev_connect_ready, NULL);
>> +     if (!dev)
>> +             /* Lets try again. */
>> +             return -1;
>> +
>> +     dev->connect_ready = false;
>> +
>> +     return connect_le(dev);
>> +}
>> +
>> +static void bt_le_discovery_stop_cb(void)
>> +{
>> +     DBG("");
>> +
>> +     /* Check now if there is any device ready to connect*/
>> +     if (connect_next_dev() < 0)
>> +             bt_le_discovery_start(le_device_found_handler);
>> +}
>> +
>> +static struct gatt_device *find_device(bdaddr_t *addr)
>> +{
>> +     struct gatt_device *dev;
>> +
>> +     dev = queue_find(conn_list, match_dev_by_bdaddr, addr);
>> +     if (dev)
>> +             return dev;
>> +
>> +     dev = queue_find(conn_wait_queue, match_dev_by_bdaddr, addr);
>> +     if (dev)
>> +             return dev;
>> +
>> +     return NULL;
>> +}
>> +
>>  static void handle_client_connect(const void *buf, uint16_t len)
>>  {
>> +     const struct hal_cmd_gatt_client_connect *cmd = buf;
>> +     struct gatt_device *dev = NULL;
>> +     void *l;
>> +     bdaddr_t addr;
>> +     uint8_t status;
>> +     bool send_notify = false;
>> +
>>       DBG("");
>>
>> +     /* Check if client is registered */
>> +     l = queue_find(gatt_clients, match_client_by_id,
>> +                                             INT_TO_PTR(cmd->client_if));
>> +     if (!l) {
>> +             error("gatt: Client id %d not found", cmd->client_if);
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     android2bdaddr(&cmd->bdaddr, &addr);
>> +
>> +     /* We do support many clients for one device connection so lets check
>> +       * If device is connected or in connecting state just update list of
>> +       * clients
>> +       */
>> +     dev = find_device(&addr);
>> +     if (dev) {
>> +
>> +             status = HAL_STATUS_SUCCESS;
>
> Set status before goto only.
>
>> +
>> +             /* Remeber to send dummy notification event  if we area
>> +              * connected
>> +              */
>> +             if (dev->conn_id)
>> +                     send_notify = true;
>> +
>> +             if (queue_find(dev->clients, match_by_value,
>> +                                             INT_TO_PTR(cmd->client_if)))
>> +                             goto reply;
>> +
>> +             /* Store another client */
>> +             if (!queue_push_tail(dev->clients,
>> +                                             INT_TO_PTR(cmd->client_if))) {
>> +                     error("gatt: Cannot push client on gatt device list");
>> +                     status = HAL_STATUS_FAILED;
>> +                     goto reply;
>> +             }
>> +
>> +             goto reply;
>> +     }
>> +
>> +     /* Lets create new gatt device and put it on conn_wait_queue.
>> +       * Once it is connected we move it to conn_list
>> +       */
>> +     dev = new0(struct gatt_device, 1);
>> +     if (!dev) {
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     memcpy(&dev->bdaddr, &addr, sizeof(bdaddr_t));
>> +
>> +     /* Create queue to keep list of clients for given device*/
>> +     dev->clients = queue_new();
>> +     if (!dev->clients) {
>> +             error("gatt: Cannot create client queue");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     /* Update client list of device */
>> +     if (!queue_push_tail(dev->clients, INT_TO_PTR(cmd->client_if))) {
>> +             error("gatt: Cannot push client on the client queue!?");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     /* Start le scan if not started */
>> +     if (queue_isempty(scan_clients)) {
>> +             if (!bt_le_discovery_start(le_device_found_handler)) {
>> +                     error("gatt: Could not start scan");
>> +                     status = HAL_STATUS_FAILED;
>> +                     goto reply;
>> +             }
>> +     }
>> +
>> +     if (!queue_push_tail(conn_wait_queue, dev)) {
>> +             error("gatt: Cannot push device on conn_wait_queue");
>> +             status = HAL_STATUS_FAILED;
>> +             goto reply;
>> +     }
>> +
>> +     status = HAL_STATUS_SUCCESS;
>> +     goto reply;
>> +
>
> This goto is not needed :)
>
>> +reply:
>>       ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_CONNECT,
>> -                                                     HAL_STATUS_FAILED);
>> +                                                             status);
>> +
>> +     /* Send dummy notification since ACL is already up*/
>> +     if (send_notify) {
>> +             struct hal_ev_gatt_client_connect ev;
>> +
>> +             ev.conn_id = dev->conn_id;
>> +             ev.status = HAL_STATUS_SUCCESS;
>> +             ev.client_if = cmd->client_if;
>> +             bdaddr2android(&addr, &ev.bda);
>> +
>> +             ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> +                                             HAL_EV_GATT_CLIENT_CONNECT,
>> +                                             sizeof(ev), &ev);
>> +     }
>> +
>
> You are already dereferencing dev in if above.
>
>> +     if (status && dev)
>> +             destroy_device(dev);
>>  }
>>
>>  static void handle_client_disconnect(const void *buf, uint16_t len)
>> @@ -610,6 +1032,18 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t
>> *addr)
>>
>>       hal_ipc = ipc;
>>
>> +     conn_list = queue_new();
>> +     if (!conn_list) {
>> +             error("gatt: Can not create conn queue");
>> +             return false;
>> +     }
>> +
>> +     conn_wait_queue = queue_new();
>> +     if (!conn_wait_queue) {
>> +             error("gatt: Can not create conn queue");
>> +             return false;
>> +     }
>> +
>>       ipc_register(hal_ipc, HAL_SERVICE_ID_GATT, cmd_handlers,
>>                                               G_N_ELEMENTS(cmd_handlers));
>>
>> @@ -637,4 +1071,7 @@ void bt_gatt_unregister(void)
>>
>>       ipc_unregister(hal_ipc, HAL_SERVICE_ID_GATT);
>>       hal_ipc = NULL;
>> +
>> +     queue_destroy(conn_list, NULL);
>> +     queue_destroy(conn_wait_queue, NULL);
>
> Full those lists here for completeness.
>
>>  }
>
> --
> BR
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] android/gatt: Add disconnect GATT device

[Lukasz Rymanowski]

Disconnect scenarios:

1. If there is more then one client for a given gatt_device then
client id is removed from dev->clients, success response is sent
together with success disconnect event.

2. If there is only one client for a given remote device then we
do what is decribed above plus clean of gattrib stuff

3. In case client_if or conn_id is incorrect, response failed is sent
---
 android/gatt.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 9afdf4b..ce72986 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -112,6 +112,14 @@ static bool match_dev_connect_ready(const void *data, const void *user_data)
 	return dev->connect_ready;
 }
 
+static bool match_dev_by_conn_id(const void *data, const void *user_data)
+{
+	const struct gatt_device *dev = data;
+	const int32_t conn_id = PTR_TO_INT(user_data);
+
+	return dev->conn_id == conn_id;
+}
+
 static void destroy_device(struct gatt_device *dev)
 {
 	queue_destroy(dev->clients, NULL);
@@ -661,10 +669,48 @@ reply:
 
 static void handle_client_disconnect(const void *buf, uint16_t len)
 {
+	const struct hal_cmd_gatt_client_disconnect *cmd = buf;
+	struct gatt_device *dev;
+	uint8_t status;
+
 	DBG("");
 
+	dev = queue_find(conn_list, match_dev_by_conn_id,
+						INT_TO_PTR(cmd->conn_id));
+	if (!dev) {
+		status = HAL_STATUS_FAILED;
+		goto reply;
+	}
+
+	/*Check if client owns this connection */
+	if (!queue_remove_if(dev->clients, match_by_value,
+						INT_TO_PTR(cmd->client_if)))
+		status = HAL_STATUS_FAILED;
+	else
+		status = HAL_STATUS_SUCCESS;
+
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
-			HAL_OP_GATT_CLIENT_DISCONNECT, HAL_STATUS_FAILED);
+			HAL_OP_GATT_CLIENT_DISCONNECT, status);
+
+	if (status == HAL_STATUS_FAILED)
+		return;
+
+	/* Just send disconnect event. If there is more clients on this
+	 * device then this is what we shall to do.
+	 * If this is last client, this is still OK to do because on connect
+	 * request we do le scan and wait until remote device start
+	 * advertisement */
+	send_disconnect_notify((void *)&cmd->client_if, (void *)dev);
+
+	/* If there is more clients just return */
+	if (!queue_isempty(dev->clients))
+		return;
+
+	/* If this is last client do more cleaning */
+	connection_cleanup(dev);
+	dev = queue_remove_if(conn_list, match_dev_by_bdaddr, &dev->bdaddr);
+	destroy_device(dev);
 }
 
 static void handle_client_listen(const void *buf, uint16_t len)
-- 
1.8.4

Re: [PATCH v2 4/4] android/gatt: Add disconnect GATT device

[Szymon Janc]

Hi,

On Friday 14 of March 2014 14:49:39 Lukasz Rymanowski wrote:
> Disconnect scenarios:
> 
> 1. If there is more then one client for a given gatt_device then
> client id is removed from dev->clients, success response is sent
> together with success disconnect event.
> 
> 2. If there is only one client for a given remote device then we
> do what is decribed above plus clean of gattrib stuff
> 
> 3. In case client_if or conn_id is incorrect, response failed is sent
> ---
>  android/gatt.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index 9afdf4b..ce72986 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -112,6 +112,14 @@ static bool match_dev_connect_ready(const void *data,
> const void *user_data) return dev->connect_ready;
>  }
> 
> +static bool match_dev_by_conn_id(const void *data, const void *user_data)
> +{
> +	const struct gatt_device *dev = data;
> +	const int32_t conn_id = PTR_TO_INT(user_data);
> +
> +	return dev->conn_id == conn_id;
> +}
> +
>  static void destroy_device(struct gatt_device *dev)
>  {
>  	queue_destroy(dev->clients, NULL);
> @@ -661,10 +669,48 @@ reply:
> 
>  static void handle_client_disconnect(const void *buf, uint16_t len)
>  {
> +	const struct hal_cmd_gatt_client_disconnect *cmd = buf;
> +	struct gatt_device *dev;
> +	uint8_t status;
> +
>  	DBG("");
> 
> +	dev = queue_find(conn_list, match_dev_by_conn_id,
> +						INT_TO_PTR(cmd->conn_id));
> +	if (!dev) {
> +		status = HAL_STATUS_FAILED;
> +		goto reply;
> +	}
> +
> +	/*Check if client owns this connection */
> +	if (!queue_remove_if(dev->clients, match_by_value,
> +						INT_TO_PTR(cmd->client_if)))
> +		status = HAL_STATUS_FAILED;
> +	else
> +		status = HAL_STATUS_SUCCESS;
> +
> +reply:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
> -			HAL_OP_GATT_CLIENT_DISCONNECT, HAL_STATUS_FAILED);
> +			HAL_OP_GATT_CLIENT_DISCONNECT, status);
> +
> +	if (status == HAL_STATUS_FAILED)
> +		return;
> +
> +	/* Just send disconnect event. If there is more clients on this
> +	 * device then this is what we shall to do.
> +	 * If this is last client, this is still OK to do because on connect
> +	 * request we do le scan and wait until remote device start
> +	 * advertisement */
> +	send_disconnect_notify((void *)&cmd->client_if, (void *)dev);

This should be function with proper parameters type and if needed extra 
wrapper for list iteration.

> +
> +	/* If there is more clients just return */
> +	if (!queue_isempty(dev->clients))
> +		return;
> +
> +	/* If this is last client do more cleaning */
> +	connection_cleanup(dev);
> +	dev = queue_remove_if(conn_list, match_dev_by_bdaddr, &dev->bdaddr);
> +	destroy_device(dev);
>  }
> 
>  static void handle_client_listen(const void *buf, uint16_t len)

-- 
BR
Szymon Janc