[RFC 0/5] Remote device cache support

[RFC 0/5] Remote device cache support

[Szymon Janc]

Hi All,

This adds support for permanent cache of remote devices info. Cache size is
limited to DEVICES_CACHE_MAX (currently 100) and oldest devices (based on
timestamp) is removed if cache gets full.

notes:
 - there is still place for improvements in number of reads/writes done.
   Will work on that when current cache approach is accepted.
 - there is no need to cache SDP (we cache UUIDs)

Comments are welcome.

-- 
BR
Szymon Janc

Szymon Janc (5):
  android/bluetooth: Split devices list to devices and bonded_devices
  android/bluetooth: Use defines for settings and devices files paths
  android/bluetooth: Always store device info
  android/bluetooth: Add support for caching remote device info
  android/bluetooth: Add support for loading caches devices from storage

 android/bluetooth.c | 199 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 148 insertions(+), 51 deletions(-)

-- 
1.8.3.2

[RFC 1/5] android/bluetooth: Split devices list to devices and bonded_devices

[Szymon Janc]

From: Szymon Janc <szymon.janc@gmail.com>

Bonded devices are permament until unbondedn. Non-bonded devices will
be held in (size limited) cache based on timestamp property so split
list to ease separation.
---
 android/bluetooth.c | 47 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 735b03e..78e98c1 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -133,6 +133,8 @@ static const uint16_t uuid_list[] = {
 };
 
 static struct mgmt *mgmt_if = NULL;
+
+static GSList *bonded_devices = NULL;
 static GSList *devices = NULL;
 
 /* This list contains addresses which are asked for records */
@@ -284,6 +286,10 @@ static struct device *find_device(const bdaddr_t *bdaddr)
 {
 	GSList *l;
 
+	l = g_slist_find_custom(bonded_devices, bdaddr, device_match);
+	if (l)
+		return l->data;
+
 	l = g_slist_find_custom(devices, bdaddr, device_match);
 	if (l)
 		return l->data;
@@ -560,12 +566,30 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
 	if (!dev)
 		return;
 
-	if (dev->bond_state != state) {
-		dev->bond_state = state;
-		send_bond_state_change(&dev->bdaddr, status, state);
+	if (dev->bond_state == state)
+		return;
 
-		store_device_info(dev);
+	switch (state) {
+	case HAL_BOND_STATE_NONE:
+		if (dev->bond_state == HAL_BOND_STATE_BONDED) {
+			bonded_devices = g_slist_remove(bonded_devices, dev);
+			devices = g_slist_prepend(devices, dev);
+		}
+		break;
+	case HAL_BOND_STATE_BONDED:
+		devices = g_slist_remove(devices, dev);
+		bonded_devices = g_slist_prepend(bonded_devices, dev);
+		break;
+	case HAL_BOND_STATE_BONDING:
+	default:
+		break;
 	}
+
+	dev->bond_state = state;
+
+	store_device_info(dev);
+
+	send_bond_state_change(&dev->bdaddr, status, state);
 }
 
 static  void send_device_property(const bdaddr_t *bdaddr, uint8_t type,
@@ -2134,18 +2158,15 @@ static uint8_t get_adapter_scan_mode(void)
 
 static uint8_t get_adapter_bonded_devices(void)
 {
-	uint8_t buf[sizeof(bdaddr_t) * g_slist_length(devices)];
+	uint8_t buf[sizeof(bdaddr_t) * g_slist_length(bonded_devices)];
 	int i = 0;
 	GSList *l;
 
 	DBG("");
 
-	for (l = devices; l; l = g_slist_next(l)) {
+	for (l = bonded_devices; l; l = g_slist_next(l)) {
 		struct device *dev = l->data;
 
-		if (dev->bond_state != HAL_BOND_STATE_BONDED)
-			continue;
-
 		bdaddr2android(&dev->bdaddr, buf + (i * sizeof(bdaddr_t)));
 		i++;
 	}
@@ -2697,11 +2718,10 @@ static void send_bonded_devices_props(void)
 {
 	GSList *l;
 
-	for (l = devices; l; l = g_slist_next(l)) {
+	for (l = bonded_devices; l; l = g_slist_next(l)) {
 		struct device *dev = l->data;
 
-		if (dev->bond_state == HAL_BOND_STATE_BONDED)
-			get_remote_device_props(dev);
+		get_remote_device_props(dev);
 	}
 }
 
@@ -3099,6 +3119,9 @@ void bt_bluetooth_unregister(void)
 {
 	DBG("");
 
+	g_slist_free_full(bonded_devices, (GDestroyNotify) free_device);
+	bonded_devices = NULL;
+
 	g_slist_free_full(devices, (GDestroyNotify) free_device);
 	devices = NULL;
 
-- 
1.8.3.2

[RFC 2/5] android/bluetooth: Use defines for settings and devices files paths

[Szymon Janc]

From: Szymon Janc <szymon.janc@gmail.com>

---
 android/bluetooth.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 78e98c1..23a528a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -54,6 +54,9 @@
 
 #define DUT_MODE_FILE "/sys/kernel/debug/bluetooth/hci%u/dut_mode"
 
+#define SETTINGS_FILE ANDROID_STORAGEDIR"/settings"
+#define DEVICES_FILE ANDROID_STORAGEDIR"/devices"
+
 #define DEVICE_ID_SOURCE	0x0002	/* USB */
 #define DEVICE_ID_VENDOR	0x1d6b	/* Linux Foundation */
 #define DEVICE_ID_PRODUCT	0x0247	/* BlueZ for Android */
@@ -149,8 +152,7 @@ static void store_adapter_config(void)
 
 	key_file = g_key_file_new();
 
-	g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings", 0,
-									NULL);
+	g_key_file_load_from_file(key_file, SETTINGS_FILE, 0, NULL);
 
 	ba2str(&adapter.bdaddr, addr);
 
@@ -161,7 +163,7 @@ static void store_adapter_config(void)
 
 	data = g_key_file_to_data(key_file, &length, NULL);
 
-	g_file_set_contents(ANDROID_STORAGEDIR"/settings", data, length, NULL);
+	g_file_set_contents(SETTINGS_FILE, data, length, NULL);
 
 	g_free(data);
 	g_key_file_free(key_file);
@@ -174,8 +176,7 @@ static void load_adapter_config(void)
 	char *str;
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/settings", 0,
-									NULL);
+	g_key_file_load_from_file(key_file, SETTINGS_FILE, 0, NULL);
 
 	str = g_key_file_get_string(key_file, "General", "Address", NULL);
 	if (!str) {
@@ -217,8 +218,7 @@ static void store_device_info(struct device *dev)
 	ba2str(&dev->bdaddr, addr);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
-									NULL);
+	g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
 
 	if (dev->bond_state == HAL_BOND_STATE_NONE) {
 		g_key_file_remove_group(key_file, addr, NULL);
@@ -267,7 +267,7 @@ static void store_device_info(struct device *dev)
 
 done:
 	str = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(ANDROID_STORAGEDIR"/devices", str, length, NULL);
+	g_file_set_contents(DEVICES_FILE, str, length, NULL);
 	g_free(str);
 
 	g_key_file_free(key_file);
@@ -522,8 +522,7 @@ static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
 
 	key_file = g_key_file_new();
 
-	if (!g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices",
-								0, NULL))
+	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL))
 		return;
 
 	ba2str(dst, addr);
@@ -538,7 +537,7 @@ static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
 	g_key_file_set_integer(key_file, addr, "LinkKeyPinLength", pin_length);
 
 	data = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(ANDROID_STORAGEDIR"/devices", data, length, NULL);
+	g_file_set_contents(DEVICES_FILE, data, length, NULL);
 	g_free(data);
 
 	g_key_file_free(key_file);
@@ -1732,8 +1731,7 @@ static void load_devices_info(bt_bluetooth_ready cb)
 
 	key_file = g_key_file_new();
 
-	g_key_file_load_from_file(key_file, ANDROID_STORAGEDIR"/devices", 0,
-									NULL);
+	g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
 
 	devs = g_key_file_get_groups(key_file, &len);
 
-- 
1.8.3.2

[RFC 3/5] android/bluetooth: Always store device info

[Szymon Janc]

This allows to cache remote device informations.
---
 android/bluetooth.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 23a528a..8c91d87 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -208,23 +208,11 @@ static void store_device_info(struct device *dev)
 	char **uuids = NULL;
 	char *str;
 
-	/* We only store bonded devices and need to modify the storage
-	 * if the state is either NONE or BONDED.
-	 */
-	if (dev->bond_state != HAL_BOND_STATE_BONDED &&
-					dev->bond_state != HAL_BOND_STATE_NONE)
-		return;
-
 	ba2str(&dev->bdaddr, addr);
 
 	key_file = g_key_file_new();
 	g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
 
-	if (dev->bond_state == HAL_BOND_STATE_NONE) {
-		g_key_file_remove_group(key_file, addr, NULL);
-		goto done;
-	}
-
 	g_key_file_set_integer(key_file, addr, "Type", dev->bdaddr_type);
 
 	g_key_file_set_string(key_file, addr, "Name", dev->name);
@@ -265,7 +253,6 @@ static void store_device_info(struct device *dev)
 		g_key_file_remove_key(key_file, addr, "Services", NULL);
 	}
 
-done:
 	str = g_key_file_to_data(key_file, &length, NULL);
 	g_file_set_contents(DEVICES_FILE, str, length, NULL);
 	g_free(str);
@@ -543,6 +530,33 @@ static void store_link_key(const bdaddr_t *dst, const uint8_t *key,
 	g_key_file_free(key_file);
 }
 
+static void remove_stored_link_key(const bdaddr_t *dst)
+{
+	GKeyFile *key_file;
+	gsize length = 0;
+	char addr[18];
+	char *data;
+
+	key_file = g_key_file_new();
+
+	if (!g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL))
+		return;
+
+	ba2str(dst, addr);
+
+	DBG("%s", addr);
+
+	g_key_file_remove_key(key_file, addr, "LinkKey", NULL);
+	g_key_file_remove_key(key_file, addr, "LinkKeyType", NULL);
+	g_key_file_remove_key(key_file, addr, "LinkKeyPinLength", NULL);
+
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(DEVICES_FILE, data, length, NULL);
+	g_free(data);
+
+	g_key_file_free(key_file);
+}
+
 static void send_bond_state_change(const bdaddr_t *addr, uint8_t status,
 								uint8_t state)
 {
@@ -2449,6 +2463,8 @@ static void unpair_device_complete(uint8_t status, uint16_t length,
 	if (status != MGMT_STATUS_SUCCESS)
 		return;
 
+	remove_stored_link_key(&rp->addr.bdaddr);
+
 	set_device_bond_state(&rp->addr.bdaddr, HAL_STATUS_SUCCESS,
 							HAL_BOND_STATE_NONE);
 }
-- 
1.8.3.2

[RFC 4/5] android/bluetooth: Add support for caching remote device info

[Szymon Janc]

Cache is limited to DEVICES_CACHE_MAX. Devices are sorted with
timestamp so if cache is full olderst device is removed.
---
 android/bluetooth.c | 69 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 8c91d87..df442a3 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -70,6 +70,8 @@
 /* Default discoverable timeout 120sec as in Android */
 #define DEFAULT_DISCOVERABLE_TIMEOUT 120
 
+#define DEVICES_CACHE_MAX 100
+
 #define BASELEN_PROP_CHANGED (sizeof(struct hal_ev_adapter_props_changed) \
 					+ (sizeof(struct hal_property)))
 
@@ -261,6 +263,27 @@ static void store_device_info(struct device *dev)
 	g_strfreev(uuids);
 }
 
+static void remove_device_info(struct device *dev)
+{
+	GKeyFile *key_file;
+	gsize length = 0;
+	char addr[18];
+	char *str;
+
+	ba2str(&dev->bdaddr, addr);
+
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, DEVICES_FILE, 0, NULL);
+
+	g_key_file_remove_group(key_file, addr, NULL);
+
+	str = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(DEVICES_FILE, str, length, NULL);
+	g_free(str);
+
+	g_key_file_free(key_file);
+}
+
 static int device_match(gconstpointer a, gconstpointer b)
 {
 	const struct device *dev = a;
@@ -284,6 +307,34 @@ static struct device *find_device(const bdaddr_t *bdaddr)
 	return NULL;
 }
 
+static void free_device(struct device *dev)
+{
+	g_free(dev->name);
+	g_free(dev->friendly_name);
+	g_slist_free_full(dev->uuids, g_free);
+	g_free(dev);
+}
+
+static void cache_device(struct device *new_dev)
+{
+	struct device *dev;
+	GSList *l;
+
+	if (g_slist_length(devices) < DEVICES_CACHE_MAX)
+		goto done;
+
+	l = g_slist_last(devices);
+	dev = l->data;
+
+	devices = g_slist_remove(bonded_devices, dev);
+	remove_device_info(dev);
+	free_device(dev);
+
+done:
+	devices = g_slist_prepend(devices, new_dev);
+	store_device_info(new_dev);
+}
+
 static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
 {
 	struct device *dev;
@@ -302,19 +353,10 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t type)
 	/* use address for name, will be change if one is present
 	 * eg. in EIR or set by set_property. */
 	dev->name = g_strdup(addr);
-	devices = g_slist_prepend(devices, dev);
 
 	return dev;
 }
 
-static void free_device(struct device *dev)
-{
-	g_free(dev->name);
-	g_free(dev->friendly_name);
-	g_slist_free_full(dev->uuids, g_free);
-	g_free(dev);
-}
-
 static struct device *get_device(const bdaddr_t *bdaddr, uint8_t type)
 {
 	struct device *dev;
@@ -586,7 +628,8 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
 	case HAL_BOND_STATE_NONE:
 		if (dev->bond_state == HAL_BOND_STATE_BONDED) {
 			bonded_devices = g_slist_remove(bonded_devices, dev);
-			devices = g_slist_prepend(devices, dev);
+			remove_stored_link_key(&dev->bdaddr);
+			cache_device(dev);
 		}
 		break;
 	case HAL_BOND_STATE_BONDED:
@@ -600,8 +643,6 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
 
 	dev->bond_state = state;
 
-	store_device_info(dev);
-
 	send_bond_state_change(&dev->bdaddr, status, state);
 }
 
@@ -1107,6 +1148,8 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 		(*num_prop)++;
 	}
 
+	cache_device(dev);
+
 	ipc_send_notif(HAL_SERVICE_ID_BLUETOOTH, opcode, size, buf);
 
 	if (confirm) {
@@ -2463,8 +2506,6 @@ static void unpair_device_complete(uint8_t status, uint16_t length,
 	if (status != MGMT_STATUS_SUCCESS)
 		return;
 
-	remove_stored_link_key(&rp->addr.bdaddr);
-
 	set_device_bond_state(&rp->addr.bdaddr, HAL_STATUS_SUCCESS,
 							HAL_BOND_STATE_NONE);
 }
-- 
1.8.3.2

[RFC 5/5] android/bluetooth: Add support for loading caches devices from storage

[Szymon Janc]

Info is now stored for all devices and bond state depends on linkkey
being stored. Based on that devices loaded from storage are put either
to cache or to bonded_devices list.
---
 android/bluetooth.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index df442a3..dfbbe1a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1695,7 +1695,8 @@ static void clear_uuids(void)
 					sizeof(cp), &cp, NULL, NULL, NULL);
 }
 
-static void create_device_from_info(GKeyFile *key_file, const char *peer)
+static struct device *create_device_from_info(GKeyFile *key_file,
+							const char *peer)
 {
 	struct device *dev;
 	uint8_t type;
@@ -1710,7 +1711,7 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
 	str2ba(peer, &bdaddr);
 	dev = create_device(&bdaddr, type);
 
-	dev->bond_state = HAL_BOND_STATE_BONDED;
+	dev->bond_state = HAL_BOND_STATE_NONE;
 
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
 	if (str) {
@@ -1746,6 +1747,8 @@ static void create_device_from_info(GKeyFile *key_file, const char *peer)
 
 		g_strfreev(uuids);
 	}
+
+	return dev;
 }
 
 static struct mgmt_link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
@@ -1778,6 +1781,14 @@ failed:
 	return info;
 }
 
+static int device_timestamp_cmp(gconstpointer  a, gconstpointer  b)
+{
+	const struct device *deva = a;
+	const struct device *devb = b;
+
+	return deva->timestamp < devb->timestamp;
+}
+
 static void load_devices_info(bt_bluetooth_ready cb)
 {
 	GKeyFile *key_file;
@@ -1794,16 +1805,24 @@ static void load_devices_info(bt_bluetooth_ready cb)
 
 	for (i = 0; i < len; i++) {
 		struct mgmt_link_key_info *key_info;
+		struct device *dev;
 
-		create_device_from_info(key_file, devs[i]);
+		dev = create_device_from_info(key_file, devs[i]);
 
 		key_info = get_key_info(key_file, devs[i]);
-		if (key_info)
+		if (key_info) {
 			keys = g_slist_prepend(keys, key_info);
+			bonded_devices = g_slist_prepend(bonded_devices, dev);
+			dev->bond_state = HAL_BOND_STATE_BONDED;
+		} else {
+			devices = g_slist_prepend(devices, dev);
+		}
 
 		/* TODO ltk */
 	}
 
+	devices = g_slist_sort(devices, device_timestamp_cmp);
+
 	load_link_keys(keys, cb);
 	g_strfreev(devs);
 	g_slist_free_full(keys, g_free);
-- 
1.8.3.2

Re: [RFC 1/5] android/bluetooth: Split devices list to devices and bonded_devices

[Luiz Augusto von Dentz]

Hi Szymon,

On Tue, Jan 14, 2014 at 1:11 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> From: Szymon Janc <szymon.janc@gmail.com>
>
> Bonded devices are permament until unbondedn. Non-bonded devices will
> be held in (size limited) cache based on timestamp property so split
> list to ease separation.
> ---
>  android/bluetooth.c | 47 +++++++++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 735b03e..78e98c1 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -133,6 +133,8 @@ static const uint16_t uuid_list[] = {
>  };
>
>  static struct mgmt *mgmt_if = NULL;
> +
> +static GSList *bonded_devices = NULL;
>  static GSList *devices = NULL;
>
>  /* This list contains addresses which are asked for records */
> @@ -284,6 +286,10 @@ static struct device *find_device(const bdaddr_t *bdaddr)
>  {
>         GSList *l;
>
> +       l = g_slist_find_custom(bonded_devices, bdaddr, device_match);
> +       if (l)
> +               return l->data;
> +
>         l = g_slist_find_custom(devices, bdaddr, device_match);
>         if (l)
>                 return l->data;
> @@ -560,12 +566,30 @@ static void set_device_bond_state(const bdaddr_t *addr, uint8_t status,
>         if (!dev)
>                 return;
>
> -       if (dev->bond_state != state) {
> -               dev->bond_state = state;
> -               send_bond_state_change(&dev->bdaddr, status, state);
> +       if (dev->bond_state == state)
> +               return;
>
> -               store_device_info(dev);
> +       switch (state) {
> +       case HAL_BOND_STATE_NONE:
> +               if (dev->bond_state == HAL_BOND_STATE_BONDED) {
> +                       bonded_devices = g_slist_remove(bonded_devices, dev);
> +                       devices = g_slist_prepend(devices, dev);
> +               }
> +               break;
> +       case HAL_BOND_STATE_BONDED:
> +               devices = g_slist_remove(devices, dev);
> +               bonded_devices = g_slist_prepend(bonded_devices, dev);
> +               break;
> +       case HAL_BOND_STATE_BONDING:
> +       default:
> +               break;
>         }
> +
> +       dev->bond_state = state;
> +
> +       store_device_info(dev);
> +
> +       send_bond_state_change(&dev->bdaddr, status, state);
>  }
>
>  static  void send_device_property(const bdaddr_t *bdaddr, uint8_t type,
> @@ -2134,18 +2158,15 @@ static uint8_t get_adapter_scan_mode(void)
>
>  static uint8_t get_adapter_bonded_devices(void)
>  {
> -       uint8_t buf[sizeof(bdaddr_t) * g_slist_length(devices)];
> +       uint8_t buf[sizeof(bdaddr_t) * g_slist_length(bonded_devices)];
>         int i = 0;
>         GSList *l;
>
>         DBG("");
>
> -       for (l = devices; l; l = g_slist_next(l)) {
> +       for (l = bonded_devices; l; l = g_slist_next(l)) {
>                 struct device *dev = l->data;
>
> -               if (dev->bond_state != HAL_BOND_STATE_BONDED)
> -                       continue;
> -
>                 bdaddr2android(&dev->bdaddr, buf + (i * sizeof(bdaddr_t)));
>                 i++;
>         }
> @@ -2697,11 +2718,10 @@ static void send_bonded_devices_props(void)
>  {
>         GSList *l;
>
> -       for (l = devices; l; l = g_slist_next(l)) {
> +       for (l = bonded_devices; l; l = g_slist_next(l)) {
>                 struct device *dev = l->data;
>
> -               if (dev->bond_state == HAL_BOND_STATE_BONDED)
> -                       get_remote_device_props(dev);
> +               get_remote_device_props(dev);
>         }
>  }
>
> @@ -3099,6 +3119,9 @@ void bt_bluetooth_unregister(void)
>  {
>         DBG("");
>
> +       g_slist_free_full(bonded_devices, (GDestroyNotify) free_device);
> +       bonded_devices = NULL;
> +
>         g_slist_free_full(devices, (GDestroyNotify) free_device);

You can make free_device to take a void pointer so you don't have to
cast in such cases.



-- 
Luiz Augusto von Dentz

Re: [RFC 4/5] android/bluetooth: Add support for caching remote device info

[Luiz Augusto von Dentz]

Hi Szymon,

On Tue, Jan 14, 2014 at 1:11 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Cache is limited to DEVICES_CACHE_MAX. Devices are sorted with
> timestamp so if cache is full olderst device is removed.
> ---
>  android/bluetooth.c | 69 ++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 8c91d87..df442a3 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -70,6 +70,8 @@
>  /* Default discoverable timeout 120sec as in Android */
>  #define DEFAULT_DISCOVERABLE_TIMEOUT 120
>
> +#define DEVICES_CACHE_MAX 100

Is this being limited because Android UI cannot handle more than that?
Or perhaps because our IPC/MTU cannot carry more than that? If the
latter I believe we can actually calculate the maximun e.g. MTU / pdu
size.

Re: [RFC 4/5] android/bluetooth: Add support for caching remote device info

[Szymon Janc]

Hi Luiz,

On Tuesday 14 January 2014 17:49:54 Luiz Augusto von Dentz wrote:
> Hi Szymon,
> 
> On Tue, Jan 14, 2014 at 1:11 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > Cache is limited to DEVICES_CACHE_MAX. Devices are sorted with
> > timestamp so if cache is full olderst device is removed.
> > ---
> > 
> >  android/bluetooth.c | 69
> >  ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 55
> >  insertions(+), 14 deletions(-)
> > 
> > diff --git a/android/bluetooth.c b/android/bluetooth.c
> > index 8c91d87..df442a3 100644
> > --- a/android/bluetooth.c
> > +++ b/android/bluetooth.c
> > @@ -70,6 +70,8 @@
> > 
> >  /* Default discoverable timeout 120sec as in Android */
> >  #define DEFAULT_DISCOVERABLE_TIMEOUT 120
> > 
> > +#define DEVICES_CACHE_MAX 100
> 
> Is this being limited because Android UI cannot handle more than that?
> Or perhaps because our IPC/MTU cannot carry more than that? If the
> latter I believe we can actually calculate the maximun e.g. MTU / pdu
> size.

This is not related to Android UI nor MTU. This is just to limit max resources 
usage. Value is totally arbitrary. If device is not in cache some HAL 
functions might fail (notably set/get device property).

-- 
Szymon K. Janc
szymon.janc@gmail.com

Re: [RFC 1/5] android/bluetooth: Split devices list to devices and bonded_devices

[Szymon Janc]

Hi Luiz,

On Tuesday 14 January 2014 17:42:02 Luiz Augusto von Dentz wrote:
> Hi Szymon,
> 
> On Tue, Jan 14, 2014 at 1:11 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > From: Szymon Janc <szymon.janc@gmail.com>
> > 
> > Bonded devices are permament until unbondedn. Non-bonded devices will
> > be held in (size limited) cache based on timestamp property so split
> > list to ease separation.
> > ---
> > 
> >  android/bluetooth.c | 47 +++++++++++++++++++++++++++++++++++------------
> >  1 file changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/android/bluetooth.c b/android/bluetooth.c
> > index 735b03e..78e98c1 100644
> > --- a/android/bluetooth.c
> > +++ b/android/bluetooth.c
> > @@ -133,6 +133,8 @@ static const uint16_t uuid_list[] = {
> > 
> >  };
> >  
> >  static struct mgmt *mgmt_if = NULL;
> > 
> > +
> > +static GSList *bonded_devices = NULL;
> > 
> >  static GSList *devices = NULL;
> >  
> >  /* This list contains addresses which are asked for records */
> > 
> > @@ -284,6 +286,10 @@ static struct device *find_device(const bdaddr_t
> > *bdaddr)> 
> >  {
> >  
> >         GSList *l;
> > 
> > +       l = g_slist_find_custom(bonded_devices, bdaddr, device_match);
> > +       if (l)
> > +               return l->data;
> > +
> > 
> >         l = g_slist_find_custom(devices, bdaddr, device_match);
> >         if (l)
> >         
> >                 return l->data;
> > 
> > @@ -560,12 +566,30 @@ static void set_device_bond_state(const bdaddr_t
> > *addr, uint8_t status,> 
> >         if (!dev)
> >         
> >                 return;
> > 
> > -       if (dev->bond_state != state) {
> > -               dev->bond_state = state;
> > -               send_bond_state_change(&dev->bdaddr, status, state);
> > +       if (dev->bond_state == state)
> > +               return;
> > 
> > -               store_device_info(dev);
> > +       switch (state) {
> > +       case HAL_BOND_STATE_NONE:
> > +               if (dev->bond_state == HAL_BOND_STATE_BONDED) {
> > +                       bonded_devices = g_slist_remove(bonded_devices,
> > dev); +                       devices = g_slist_prepend(devices, dev);
> > +               }
> > +               break;
> > +       case HAL_BOND_STATE_BONDED:
> > +               devices = g_slist_remove(devices, dev);
> > +               bonded_devices = g_slist_prepend(bonded_devices, dev);
> > +               break;
> > +       case HAL_BOND_STATE_BONDING:
> > +       default:
> > +               break;
> > 
> >         }
> > 
> > +
> > +       dev->bond_state = state;
> > +
> > +       store_device_info(dev);
> > +
> > +       send_bond_state_change(&dev->bdaddr, status, state);
> > 
> >  }
> >  
> >  static  void send_device_property(const bdaddr_t *bdaddr, uint8_t type,
> > 
> > @@ -2134,18 +2158,15 @@ static uint8_t get_adapter_scan_mode(void)
> > 
> >  static uint8_t get_adapter_bonded_devices(void)
> >  {
> > 
> > -       uint8_t buf[sizeof(bdaddr_t) * g_slist_length(devices)];
> > +       uint8_t buf[sizeof(bdaddr_t) * g_slist_length(bonded_devices)];
> > 
> >         int i = 0;
> >         GSList *l;
> >         
> >         DBG("");
> > 
> > -       for (l = devices; l; l = g_slist_next(l)) {
> > +       for (l = bonded_devices; l; l = g_slist_next(l)) {
> > 
> >                 struct device *dev = l->data;
> > 
> > -               if (dev->bond_state != HAL_BOND_STATE_BONDED)
> > -                       continue;
> > -
> > 
> >                 bdaddr2android(&dev->bdaddr, buf + (i *
> >                 sizeof(bdaddr_t)));
> >                 i++;
> >         
> >         }
> > 
> > @@ -2697,11 +2718,10 @@ static void send_bonded_devices_props(void)
> > 
> >  {
> >  
> >         GSList *l;
> > 
> > -       for (l = devices; l; l = g_slist_next(l)) {
> > +       for (l = bonded_devices; l; l = g_slist_next(l)) {
> > 
> >                 struct device *dev = l->data;
> > 
> > -               if (dev->bond_state == HAL_BOND_STATE_BONDED)
> > -                       get_remote_device_props(dev);
> > +               get_remote_device_props(dev);
> > 
> >         }
> >  
> >  }
> > 
> > @@ -3099,6 +3119,9 @@ void bt_bluetooth_unregister(void)
> > 
> >  {
> >  
> >         DBG("");
> > 
> > +       g_slist_free_full(bonded_devices, (GDestroyNotify) free_device);
> > +       bonded_devices = NULL;
> > +
> > 
> >         g_slist_free_full(devices, (GDestroyNotify) free_device);
> 
> You can make free_device to take a void pointer so you don't have to
> cast in such cases.

Yes, that could be done, since this function is already present I'll change 
that in separate patch.

-- 
Szymon K. Janc
szymon.janc@gmail.com