All of lore.kernel.org
 help / color / mirror / Atom feed
* [Bluez PATCH v2] gatt: Support DeviceInfo Service when vid/pid is specified
@ 2020-07-17 14:40 Alain Michaud
  2020-07-17 20:37 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 2+ messages in thread
From: Alain Michaud @ 2020-07-17 14:40 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Alain Michaud, Miao-chen Chou

This patch adds support for the PNPID characteristic when configured in
main.conf.

This was validated as read correclty both by manually reading the valud
and confirming in the Ellisys Analyzer.

ATT Read (PnP ID: Source=Bluetooth ID, Vendor=224, Product=50181,
ATT Read Response Packet (Source=Bluetooth ID, Vendor=224,
Product=50181, Version=86)     | OK     | 7 bytes (01 E0 00 05 C4 56 00)

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Alain Michaud <alainm@chromium.org>
---
I admit I don't know how to quantify the compatibility risk with adding
the Device Info Service if the DeviceID is specified.  I can see that
some system may be configured with an app to publish the DIS and this
may break it.

If the community feels it is necessary, I can include a DeviceIdOverLE
configuration which defaults to false in main.conf to address this
compatibility risk.

Changes in v2:
 - Removing file I didn't intend to commit (peripheral/gatt.c)

 src/gatt-database.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 95ba39897..07d567078 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -57,6 +57,7 @@
 
 #define UUID_GAP	0x1800
 #define UUID_GATT	0x1801
+#define UUID_DIS	0x180a
 
 #ifndef MIN
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
@@ -1233,11 +1234,51 @@ static void populate_gatt_service(struct btd_gatt_database *database)
 	database_add_record(database, service);
 }
 
+static void device_info_read_pnp_id_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	uint8_t pdu[7];
+
+	pdu[0] = main_opts.did_source;
+	put_le16(main_opts.did_vendor, &pdu[1]);
+	put_le16(main_opts.did_product, &pdu[3]);
+	put_le16(main_opts.did_version, &pdu[5]);
+
+	gatt_db_attribute_read_result(attrib, id, 0, pdu, sizeof(pdu));
+}
+
+static void populate_devinfo_service(struct btd_gatt_database *database)
+{
+	struct gatt_db_attribute *service;
+	bt_uuid_t uuid;
+
+	bt_uuid16_create(&uuid, UUID_DIS);
+	service = gatt_db_add_service(database->db, &uuid, true, 3);
+
+	if (main_opts.did_source > 0) {
+		bt_uuid16_create(&uuid, GATT_CHARAC_PNP_ID);
+		gatt_db_service_add_characteristic(service, &uuid,
+						BT_ATT_PERM_READ,
+						BT_GATT_CHRC_PROP_READ,
+						device_info_read_pnp_id_cb,
+						NULL, database);
+	}
+
+	gatt_db_service_set_active(service, true);
+
+	database_add_record(database, service);
+}
 
 static void register_core_services(struct btd_gatt_database *database)
 {
 	populate_gap_service(database);
 	populate_gatt_service(database);
+
+	if (main_opts.did_source > 0)
+		populate_devinfo_service(database);
+
 }
 
 static void conf_cb(void *user_data)
-- 
2.28.0.rc0.105.gf9edc3c819-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [Bluez PATCH v2] gatt: Support DeviceInfo Service when vid/pid is specified
  2020-07-17 14:40 [Bluez PATCH v2] gatt: Support DeviceInfo Service when vid/pid is specified Alain Michaud
@ 2020-07-17 20:37 ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 2+ messages in thread
From: Luiz Augusto von Dentz @ 2020-07-17 20:37 UTC (permalink / raw)
  To: Alain Michaud; +Cc: linux-bluetooth, Miao-chen Chou

Hi Alain,

On Fri, Jul 17, 2020 at 7:43 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This patch adds support for the PNPID characteristic when configured in
> main.conf.
>
> This was validated as read correclty both by manually reading the valud
> and confirming in the Ellisys Analyzer.
>
> ATT Read (PnP ID: Source=Bluetooth ID, Vendor=224, Product=50181,
> ATT Read Response Packet (Source=Bluetooth ID, Vendor=224,
> Product=50181, Version=86)     | OK     | 7 bytes (01 E0 00 05 C4 56 00)
>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
> I admit I don't know how to quantify the compatibility risk with adding
> the Device Info Service if the DeviceID is specified.  I can see that
> some system may be configured with an app to publish the DIS and this
> may break it.
>
> If the community feels it is necessary, I can include a DeviceIdOverLE
> configuration which defaults to false in main.conf to address this
> compatibility risk.
>
> Changes in v2:
>  - Removing file I didn't intend to commit (peripheral/gatt.c)
>
>  src/gatt-database.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 95ba39897..07d567078 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -57,6 +57,7 @@
>
>  #define UUID_GAP       0x1800
>  #define UUID_GATT      0x1801
> +#define UUID_DIS       0x180a
>
>  #ifndef MIN
>  #define MIN(a, b) ((a) < (b) ? (a) : (b))
> @@ -1233,11 +1234,51 @@ static void populate_gatt_service(struct btd_gatt_database *database)
>         database_add_record(database, service);
>  }
>
> +static void device_info_read_pnp_id_cb(struct gatt_db_attribute *attrib,
> +                                       unsigned int id, uint16_t offset,
> +                                       uint8_t opcode, struct bt_att *att,
> +                                       void *user_data)
> +{
> +       uint8_t pdu[7];
> +
> +       pdu[0] = main_opts.did_source;
> +       put_le16(main_opts.did_vendor, &pdu[1]);
> +       put_le16(main_opts.did_product, &pdu[3]);
> +       put_le16(main_opts.did_version, &pdu[5]);
> +
> +       gatt_db_attribute_read_result(attrib, id, 0, pdu, sizeof(pdu));
> +}
> +
> +static void populate_devinfo_service(struct btd_gatt_database *database)
> +{
> +       struct gatt_db_attribute *service;
> +       bt_uuid_t uuid;
> +
> +       bt_uuid16_create(&uuid, UUID_DIS);
> +       service = gatt_db_add_service(database->db, &uuid, true, 3);
> +
> +       if (main_opts.did_source > 0) {
> +               bt_uuid16_create(&uuid, GATT_CHARAC_PNP_ID);
> +               gatt_db_service_add_characteristic(service, &uuid,
> +                                               BT_ATT_PERM_READ,
> +                                               BT_GATT_CHRC_PROP_READ,
> +                                               device_info_read_pnp_id_cb,
> +                                               NULL, database);
> +       }
> +
> +       gatt_db_service_set_active(service, true);
> +
> +       database_add_record(database, service);
> +}
>
>  static void register_core_services(struct btd_gatt_database *database)
>  {
>         populate_gap_service(database);
>         populate_gatt_service(database);
> +
> +       if (main_opts.did_source > 0)
> +               populate_devinfo_service(database);
> +
>  }
>
>  static void conf_cb(void *user_data)
> --
> 2.28.0.rc0.105.gf9edc3c819-goog

Applied, thanks.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-17 20:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 14:40 [Bluez PATCH v2] gatt: Support DeviceInfo Service when vid/pid is specified Alain Michaud
2020-07-17 20:37 ` Luiz Augusto von Dentz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.