[PATCH v2 0/2] core: Add plugin-support for Manufacturer Specific Data EIR

[PATCH v2 0/2] core: Add plugin-support for Manufacturer Specific Data EIR

[Alfonso Acosta]

Athough the Manufacturer Specific Data (MSD) field is not used
internally, it's useful for external plugins, e.g. iBeacon. 

For instance we are dealing with a device which, when losing its LTK
on a reset, notifies the master about the need of rebonding by broadcasting
specific content in the MSD field of ADV_IND reports.

The first patch simply adds the field and parsing support.

The second one adds a subscription mechanism for plugins so that they can
be notified about new Manufacturer Specific Data being received.

Alfonso Acosta (2):
  core: Add Manufacturer Specific Data EIR field
  core: Add subscription API for Manufacturer Specific Data

 plugins/neard.c |  1 -
 src/adapter.c   | 38 +++++++++++++++++++++++++++++++++++++-
 src/adapter.h   | 10 ++++++++++
 src/eir.c       | 11 +++++++++++
 src/eir.h       |  8 ++++++++
 5 files changed, 66 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] core: Add Manufacturer Specific Data EIR field

[Alfonso Acosta]

Add data structure and parsing support.
---
 src/eir.c | 11 +++++++++++
 src/eir.h |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/src/eir.c b/src/eir.c
index d22ad91..4a92efd 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -53,6 +53,8 @@ void eir_data_free(struct eir_data *eir)
 	eir->hash = NULL;
 	g_free(eir->randomizer);
 	eir->randomizer = NULL;
+	g_free(eir->msd);
+	eir->msd = NULL;
 }
 
 static void eir_parse_uuid16(struct eir_data *eir, const void *data,
@@ -240,6 +242,15 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
 			eir->did_product = data[4] | (data[5] << 8);
 			eir->did_version = data[6] | (data[7] << 8);
 			break;
+
+		case EIR_MANUFACTURER_DATA:
+			if (data_len < 2)
+				break;
+			eir->msd = g_malloc(sizeof(*eir->msd));
+			eir->msd->company = get_le16(data);
+			eir->msd->data_len = data_len - 2;
+			memcpy(&eir->msd->data, data + 2, eir->msd->data_len);
+			break;
 		}
 
 		eir_data += field_len + 1;
diff --git a/src/eir.h b/src/eir.h
index e486fa2..4cc9dbf 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -37,6 +37,7 @@
 #define EIR_SSP_RANDOMIZER          0x0F  /* SSP Randomizer */
 #define EIR_DEVICE_ID               0x10  /* device ID */
 #define EIR_GAP_APPEARANCE          0x19  /* GAP appearance */
+#define EIR_MANUFACTURER_DATA       0xFF  /* Manufacturer Specific Data */
 
 /* Flags Descriptions */
 #define EIR_LIM_DISC                0x01 /* LE Limited Discoverable Mode */
@@ -47,6 +48,12 @@
 #define EIR_SIM_HOST                0x10 /* Simultaneous LE and BR/EDR to Same
 					    Device Capable (Host) */
 
+struct eir_msd {
+	uint16_t company;
+	uint8_t data[HCI_MAX_EIR_LENGTH];
+	uint8_t data_len;
+};
+
 struct eir_data {
 	GSList *services;
 	unsigned int flags;
@@ -62,6 +69,7 @@ struct eir_data {
 	uint16_t did_product;
 	uint16_t did_version;
 	uint16_t did_source;
+	struct eir_msd *msd;
 };
 
 void eir_data_free(struct eir_data *eir);
-- 
1.9.1

[PATCH v2 2/2] core: Add subscription API for Manufacturer Specific Data

[Alfonso Acosta]

This patch allows plugins to be notified whenever an adapter receives
Manufacturer Specific Data inside from a device.

This can happen when new device is discovered or when we autoconnect
to it.
---
 plugins/neard.c |  1 -
 src/adapter.c   | 38 +++++++++++++++++++++++++++++++++++++-
 src/adapter.h   | 10 ++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 137d601..a975417 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -38,7 +38,6 @@
 #include "src/dbus-common.h"
 #include "src/adapter.h"
 #include "src/device.h"
-#include "src/eir.h"
 #include "src/agent.h"
 #include "src/hcid.h"
 
diff --git a/src/adapter.c b/src/adapter.c
index 92ee1a0..8302cb4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -68,7 +68,6 @@
 #include "attrib/att.h"
 #include "attrib/gatt.h"
 #include "attrib-server.h"
-#include "eir.h"
 
 #define ADAPTER_INTERFACE	"org.bluez.Adapter1"
 
@@ -206,6 +205,7 @@ struct btd_adapter {
 	gboolean initialized;
 
 	GSList *pin_callbacks;
+	GSList *msd_callbacks;
 
 	GSList *drivers;
 	GSList *profiles;
@@ -4549,6 +4549,10 @@ static void adapter_remove(struct btd_adapter *adapter)
 
 	g_slist_free(adapter->pin_callbacks);
 	adapter->pin_callbacks = NULL;
+
+	g_slist_free(adapter->msd_callbacks);
+	adapter->msd_callbacks = NULL;
+
 }
 
 const char *adapter_get_path(struct btd_adapter *adapter)
@@ -4647,6 +4651,20 @@ static void confirm_name(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 						confirm_name_timeout, adapter);
 }
 
+void adapter_msd_notify(struct btd_adapter *adapter, struct btd_device *dev,
+				const struct eir_msd *msd)
+{
+	GSList *l, *next;
+
+	for (l = adapter->msd_callbacks; l != NULL; l = next) {
+		btd_msd_cb_t cb = l->data;
+
+		next = g_slist_next(l);
+
+		cb(adapter, dev, msd);
+	}
+}
+
 static void update_found_devices(struct btd_adapter *adapter,
 					const bdaddr_t *bdaddr,
 					uint8_t bdaddr_type, int8_t rssi,
@@ -4738,6 +4756,9 @@ static void update_found_devices(struct btd_adapter *adapter,
 
 	device_add_eir_uuids(dev, eir_data.services);
 
+	if (eir_data.msd)
+		adapter_msd_notify(adapter, dev, eir_data.msd);
+
 	eir_data_free(&eir_data);
 
 	/*
@@ -5173,6 +5194,18 @@ void btd_adapter_unregister_pin_cb(struct btd_adapter *adapter,
 	adapter->pin_callbacks = g_slist_remove(adapter->pin_callbacks, cb);
 }
 
+void btd_adapter_unregister_msd_cb(struct btd_adapter *adapter,
+							btd_msd_cb_t cb)
+{
+	adapter->msd_callbacks = g_slist_remove(adapter->msd_callbacks, cb);
+}
+
+void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
+							btd_msd_cb_t cb)
+{
+	adapter->msd_callbacks = g_slist_prepend(adapter->msd_callbacks, cb);
+}
+
 int btd_adapter_set_fast_connectable(struct btd_adapter *adapter,
 							gboolean enable)
 {
@@ -6663,6 +6696,9 @@ static void connected_callback(uint16_t index, uint16_t length,
 		btd_device_device_set_name(device, eir_data.name);
 	}
 
+	if (eir_data.msd)
+		adapter_msd_notify(adapter, device, eir_data.msd);
+
 	eir_data_free(&eir_data);
 }
 
diff --git a/src/adapter.h b/src/adapter.h
index 6801fee..e72af89 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -30,6 +30,8 @@
 #include <glib.h>
 #include <stdbool.h>
 
+#include "eir.h"
+
 #define MAX_NAME_LENGTH		248
 
 /* Invalid SSP passkey value used to indicate negative replies */
@@ -138,6 +140,14 @@ struct btd_adapter_pin_cb_iter *btd_adapter_pin_cb_iter_new(
 void btd_adapter_pin_cb_iter_free(struct btd_adapter_pin_cb_iter *iter);
 bool btd_adapter_pin_cb_iter_end(struct btd_adapter_pin_cb_iter *iter);
 
+typedef void (*btd_msd_cb_t) (struct btd_adapter *adapter,
+				struct btd_device *dev,
+				const struct eir_msd *msd);
+void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
+					btd_msd_cb_t cb);
+void btd_adapter_unregister_msd_cb(struct btd_adapter *adapter,
+						btd_msd_cb_t cb);
+
 /* If TRUE, enables fast connectabe, i.e. reduces page scan interval and changes
  * type. If FALSE, disables fast connectable, i.e. sets page scan interval and
  * type to default values. Valid for both connectable and discoverable modes. */
-- 
1.9.1

Re: [PATCH v2 2/2] core: Add subscription API for Manufacturer Specific Data

[Johan Hedberg]

Hi Alfonso,

On Fri, Oct 10, 2014, Alfonso Acosta wrote:
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -30,6 +30,8 @@
>  #include <glib.h>
>  #include <stdbool.h>
>  
> +#include "eir.h"
> +
>  #define MAX_NAME_LENGTH		248
>  
>  /* Invalid SSP passkey value used to indicate negative replies */
> @@ -138,6 +140,14 @@ struct btd_adapter_pin_cb_iter *btd_adapter_pin_cb_iter_new(
>  void btd_adapter_pin_cb_iter_free(struct btd_adapter_pin_cb_iter *iter);
>  bool btd_adapter_pin_cb_iter_end(struct btd_adapter_pin_cb_iter *iter);
>  
> +typedef void (*btd_msd_cb_t) (struct btd_adapter *adapter,
> +				struct btd_device *dev,
> +				const struct eir_msd *msd);
> +void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
> +					btd_msd_cb_t cb);

In our user space code we try to follow the following principles for
internal header files:

1) The c-file that includes them should also include the prerequisites
2) We don't use multi-include guards

The general idea is that we don't want hidden and implicit dependencies
but prefer having them explicitly spelled out. This practice also helps
detect circular dependencies. For public header files or those of
library-like modules we don't follow this practice (e.g. gdbus/gdbus.h).

As for your patch, I'd suggest to spell out each of the three variables
in your bt_msd_cb_t instead of using the "struct eir_msd" in adapter.h.
That way you don't have a dependency to eir.h from adapter.h.

Johan

Re: [PATCH v2 2/2] core: Add subscription API for Manufacturer Specific Data

[Alfonso Acosta]

Hi Johan,


> On Fri, Oct 10, 2014, Alfonso Acosta wrote:
>> --- a/src/adapter.h
>> +++ b/src/adapter.h
>> @@ -30,6 +30,8 @@
>>  #include <glib.h>
>>  #include <stdbool.h>
>>
>> +#include "eir.h"
>> +
>>  #define MAX_NAME_LENGTH              248
>>
>>  /* Invalid SSP passkey value used to indicate negative replies */
>> @@ -138,6 +140,14 @@ struct btd_adapter_pin_cb_iter *btd_adapter_pin_cb_iter_new(
>>  void btd_adapter_pin_cb_iter_free(struct btd_adapter_pin_cb_iter *iter);
>>  bool btd_adapter_pin_cb_iter_end(struct btd_adapter_pin_cb_iter *iter);
>>
>> +typedef void (*btd_msd_cb_t) (struct btd_adapter *adapter,
>> +                             struct btd_device *dev,
>> +                             const struct eir_msd *msd);
>> +void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
>> +                                     btd_msd_cb_t cb);
>
> In our user space code we try to follow the following principles for
> internal header files:
>
> 1) The c-file that includes them should also include the prerequisites
> 2) We don't use multi-include guards
>
> The general idea is that we don't want hidden and implicit dependencies
> but prefer having them explicitly spelled out. This practice also helps
> detect circular dependencies. For public header files or those of
> library-like modules we don't follow this practice (e.g. gdbus/gdbus.h).
>
> As for your patch, I'd suggest to spell out each of the three variables
> in your bt_msd_cb_t instead of using the "struct eir_msd" in adapter.h.
> That way you don't have a dependency to eir.h from adapter.h.

Thanks for the explanation. v3 corrects this.

-- 
Alfonso Acosta

Embedded Systems Engineer at Spotify
Birger Jarlsgatan 61, Stockholm, Sweden
http://www.spotify.com