* [PATCH 0/3] Modifications composed during UPF38
@ 2011-02-16 21:17 Brian Gix
2011-02-16 21:17 ` [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers Brian Gix
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Brian Gix @ 2011-02-16 21:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: johan.hedberg, padovan
The major change in these patches is to register Primary Services
into the SDP databases as required when registering them
into the attrib-server's database. The Core v4.0 specification
indicates that the only valid/foolproof way for a BR/EDR
client to query a server for Primary Services is to do so via
an SDP query. This is from Core v4.0, Vol 3, Part G, Sec 4.4:
"GATT service discovery over BR/EDR transport will list services that only run
over an LE transport and therefore only SDP service discovery shall be used
on BR/EDR."
This also means that there can be "LE Only" primary services that do NOT
need to be in the SDP database, so if a valid range is not specified
when registering a Primary Service, it will be omitted from SDP, and
only appear when performing a GATT based Primary Service Discover.
Also -- The ATT_UUID we were using was incorrect, and I extended the
functionality of the gatttool to allow for listening without sending
anything, and ability to perform both a char-write and char-cmd.
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers 2011-02-16 21:17 [PATCH 0/3] Modifications composed during UPF38 Brian Gix @ 2011-02-16 21:17 ` Brian Gix 2011-02-15 17:08 ` Johan Hedberg 2011-02-16 21:18 ` [PATCH 2/3] Add SDP registration of Primary GATT services Brian Gix 2011-02-16 21:18 ` [PATCH 3/3] Add gatttool enhancements for UPF Brian Gix 2 siblings, 1 reply; 11+ messages in thread From: Brian Gix @ 2011-02-16 21:17 UTC (permalink / raw) To: linux-bluetooth; +Cc: johan.hedberg, padovan, Brian Gix --- lib/sdp.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/sdp.h b/lib/sdp.h index 16b59fc..e901b2a 100644 --- a/lib/sdp.h +++ b/lib/sdp.h @@ -55,6 +55,7 @@ extern "C" { #define TCS_BIN_UUID 0x0005 #define TCS_AT_UUID 0x0006 #define OBEX_UUID 0x0008 +#define ATT_UUID 0x0007 #define IP_UUID 0x0009 #define FTP_UUID 0x000a #define HTTP_UUID 0x000c @@ -72,7 +73,6 @@ extern "C" { #define MCAP_CTRL_UUID 0x001e #define MCAP_DATA_UUID 0x001f #define L2CAP_UUID 0x0100 -#define ATT_UUID 0x1801 /* * Service class identifiers of standard services and service groups -- 1.7.1 -- Brian Gix bgix@codeaurora.org Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers 2011-02-16 21:17 ` [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers Brian Gix @ 2011-02-15 17:08 ` Johan Hedberg 0 siblings, 0 replies; 11+ messages in thread From: Johan Hedberg @ 2011-02-15 17:08 UTC (permalink / raw) To: Brian Gix; +Cc: linux-bluetooth, padovan Hi Brian, On Wed, Feb 16, 2011, Brian Gix wrote: > --- > lib/sdp.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) I've pushed this patch upstream but will wait for the other ones since there were some comments regarding them. Johan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Add SDP registration of Primary GATT services 2011-02-16 21:17 [PATCH 0/3] Modifications composed during UPF38 Brian Gix 2011-02-16 21:17 ` [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers Brian Gix @ 2011-02-16 21:18 ` Brian Gix 2011-02-14 21:53 ` Anderson Lizardo 2011-02-16 21:18 ` [PATCH 3/3] Add gatttool enhancements for UPF Brian Gix 2 siblings, 1 reply; 11+ messages in thread From: Brian Gix @ 2011-02-16 21:18 UTC (permalink / raw) To: linux-bluetooth; +Cc: johan.hedberg, padovan, Brian Gix SDP registration can be supressed by passing Zero as the end handle argument to attrib_db_add(). --- attrib/example.c | 119 +++++++++++++++++++++++++++++++------------ src/attrib-server.c | 139 +++++++++++++++++++++++++++++++++++---------------- src/attrib-server.h | 6 ++- 3 files changed, 184 insertions(+), 80 deletions(-) diff --git a/attrib/example.c b/attrib/example.c index 1911912..eab3c0f 100644 --- a/attrib/example.c +++ b/attrib/example.c @@ -31,6 +31,7 @@ #include <bluetooth/sdp.h> #include <bluetooth/sdp_lib.h> +#include <src/sdpd.h> #include <glib.h> @@ -59,6 +60,9 @@ #define FMT_KILOGRAM_UUID 0xA010 #define FMT_HANGING_UUID 0xA011 +#define SDP_RECORD_COUNT 10 +sdp_record_t *sdp_records[SDP_RECORD_COUNT]; + static int register_attributes(void) { const char *desc_out_temp = "Outside Temperature"; @@ -77,59 +81,73 @@ static int register_attributes(void) uint8_t atval[256]; uuid_t uuid; int len; + int i = 0; /* Battery state service: primary service definition */ sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); att_put_u16(BATTERY_STATE_SVC_UUID, &atval[0]); - attrib_db_add(0x0100, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + sdp_records[i++] = attrib_db_add(0x0100, 0x0111, &uuid, + "Battery State Service", + ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Battery: battery state characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0110, &atval[1]); att_put_u16(BATTERY_STATE_UUID, &atval[3]); - attrib_db_add(0x0106, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0106, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Battery: battery state attribute */ sdp_uuid16_create(&uuid, BATTERY_STATE_UUID); atval[0] = 0x04; - attrib_db_add(0x0110, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 1); + attrib_db_add(0x0110, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 1); /* Battery: Client Characteristic Configuration */ sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID); atval[0] = 0x00; atval[1] = 0x00; - attrib_db_add(0x0111, &uuid, ATT_NONE, ATT_AUTHENTICATION, atval, 2); + attrib_db_add(0x0111, 0, &uuid, NULL, ATT_NONE, ATT_AUTHENTICATION, + atval, 2); /* Thermometer: primary service definition */ sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); att_put_u16(THERM_HUMIDITY_SVC_UUID, &atval[0]); - attrib_db_add(0x0200, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + sdp_records[i++] = attrib_db_add(0x0200, 0x0214, &uuid, "Thermometer", + ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Thermometer: Include */ sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID); att_put_u16(0x0500, &atval[0]); att_put_u16(0x0504, &atval[2]); att_put_u16(MANUFACTURER_SVC_UUID, &atval[4]); - attrib_db_add(0x0201, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 6); + attrib_db_add(0x0201, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 6); /* Thermometer: Include */ att_put_u16(0x0550, &atval[0]); att_put_u16(0x0568, &atval[2]); - attrib_db_add(0x0202, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 4); + att_put_u16(VENDOR_SPECIFIC_SVC_UUID, &atval[4]); + attrib_db_add(0x0202, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 6); /* Thermometer: temperature characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0204, &atval[1]); att_put_u16(TEMPERATURE_UUID, &atval[3]); - attrib_db_add(0x0203, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0203, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Thermometer: temperature characteristic value */ sdp_uuid16_create(&uuid, TEMPERATURE_UUID); atval[0] = 0x8A; atval[1] = 0x02; - attrib_db_add(0x0204, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + attrib_db_add(0x0204, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Thermometer: temperature characteristic format */ sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID); @@ -138,25 +156,29 @@ static int register_attributes(void) att_put_u16(FMT_CELSIUS_UUID, &atval[2]); atval[4] = 0x01; att_put_u16(FMT_OUTSIDE_UUID, &atval[5]); - attrib_db_add(0x0205, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 7); + attrib_db_add(0x0205, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 7); /* Thermometer: characteristic user description */ sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID); len = strlen(desc_out_temp); strncpy((char *) atval, desc_out_temp, len); - attrib_db_add(0x0206, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0206, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Thermometer: relative humidity characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0212, &atval[1]); att_put_u16(RELATIVE_HUMIDITY_UUID, &atval[3]); - attrib_db_add(0x0210, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0210, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Thermometer: relative humidity value */ sdp_uuid16_create(&uuid, RELATIVE_HUMIDITY_UUID); atval[0] = 0x27; - attrib_db_add(0x0212, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 1); + attrib_db_add(0x0212, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 1); /* Thermometer: relative humidity characteristic format */ sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID); @@ -165,68 +187,79 @@ static int register_attributes(void) att_put_u16(FMT_PERCENT_UUID, &atval[2]); att_put_u16(BLUETOOTH_SIG_UUID, &atval[4]); att_put_u16(FMT_OUTSIDE_UUID, &atval[6]); - attrib_db_add(0x0213, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 8); + attrib_db_add(0x0213, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 8); /* Thermometer: characteristic user description */ sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID); len = strlen(desc_out_hum); strncpy((char *) atval, desc_out_hum, len); - attrib_db_add(0x0214, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0214, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Secondary Service: Manufacturer Service */ sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID); att_put_u16(MANUFACTURER_SVC_UUID, &atval[0]); - attrib_db_add(0x0500, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + attrib_db_add(0x0500, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Manufacturer name characteristic definition */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0502, &atval[1]); att_put_u16(MANUFACTURER_NAME_UUID, &atval[3]); - attrib_db_add(0x0501, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0501, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Manufacturer name characteristic value */ sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID); len = strlen(manufacturer_name1); strncpy((char *) atval, manufacturer_name1, len); - attrib_db_add(0x0502, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0502, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Manufacturer serial number characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0504, &atval[1]); att_put_u16(MANUFACTURER_SERIAL_UUID, &atval[3]); - attrib_db_add(0x0503, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0503, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Manufacturer serial number characteristic value */ sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID); len = strlen(serial1); strncpy((char *) atval, serial1, len); - attrib_db_add(0x0504, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0504, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Secondary Service: Manufacturer Service */ sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID); att_put_u16(MANUFACTURER_SVC_UUID, &atval[0]); - attrib_db_add(0x0505, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + attrib_db_add(0x0505, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Manufacturer name characteristic definition */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0507, &atval[1]); att_put_u16(MANUFACTURER_NAME_UUID, &atval[3]); - attrib_db_add(0x0506, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0506, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Secondary Service: Vendor Specific Service */ sdp_uuid16_create(&uuid, GATT_SND_SVC_UUID); att_put_u16(VENDOR_SPECIFIC_SVC_UUID, &atval[0]); - attrib_db_add(0x0550, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + attrib_db_add(0x0550, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 2); /* Vendor Specific Type characteristic definition */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0568, &atval[1]); att_put_u16(VENDOR_SPECIFIC_TYPE_UUID, &atval[3]); - attrib_db_add(0x0560, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0560, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Vendor Specific Type characteristic value */ sdp_uuid16_create(&uuid, VENDOR_SPECIFIC_TYPE_UUID); @@ -236,45 +269,53 @@ static int register_attributes(void) atval[3] = 0x64; atval[4] = 0x6F; atval[5] = 0x72; - attrib_db_add(0x0568, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 6); + attrib_db_add(0x0568, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 6); /* Manufacturer name attribute */ sdp_uuid16_create(&uuid, MANUFACTURER_NAME_UUID); len = strlen(manufacturer_name2); strncpy((char *) atval, manufacturer_name2, len); - attrib_db_add(0x0507, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0507, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Characteristic: serial number */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0509, &atval[1]); att_put_u16(MANUFACTURER_SERIAL_UUID, &atval[3]); - attrib_db_add(0x0508, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0508, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* Serial number characteristic value */ sdp_uuid16_create(&uuid, MANUFACTURER_SERIAL_UUID); len = strlen(serial2); strncpy((char *) atval, serial2, len); - attrib_db_add(0x0509, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0509, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); /* Weight service: primary service definition */ sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); memcpy(atval, prim_weight_uuid, 16); - attrib_db_add(0x0680, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 16); + sdp_records[i++] = attrib_db_add(0x0680, 0x685, &uuid, "Weight Service", + ATT_NONE, ATT_NOT_PERMITTED, + atval, 16); /* Weight: include */ sdp_uuid16_create(&uuid, GATT_INCLUDE_UUID); att_put_u16(0x0505, &atval[0]); att_put_u16(0x0509, &atval[2]); att_put_u16(MANUFACTURER_SVC_UUID, &atval[4]); - attrib_db_add(0x0681, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 6); + attrib_db_add(0x0681, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 6); /* Weight: characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0683, &atval[1]); memcpy(&atval[3], char_weight_uuid, 16); - attrib_db_add(0x0682, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 19); + attrib_db_add(0x0682, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 19); /* Weight: characteristic value */ sdp_uuid128_create(&uuid, char_weight_uuid); @@ -282,7 +323,8 @@ static int register_attributes(void) atval[1] = 0x55; atval[2] = 0x00; atval[3] = 0x00; - attrib_db_add(0x0683, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 4); + attrib_db_add(0x0683, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 4); /* Weight: characteristic format */ sdp_uuid16_create(&uuid, GATT_CHARAC_FMT_UUID); @@ -291,13 +333,15 @@ static int register_attributes(void) att_put_u16(FMT_KILOGRAM_UUID, &atval[2]); att_put_u16(BLUETOOTH_SIG_UUID, &atval[4]); att_put_u16(FMT_HANGING_UUID, &atval[6]); - attrib_db_add(0x0684, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 8); + attrib_db_add(0x0684, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 8); /* Weight: characteristic user description */ sdp_uuid16_create(&uuid, GATT_CHARAC_USER_DESC_UUID); len = strlen(desc_weight); strncpy((char *) atval, desc_weight, len); - attrib_db_add(0x0685, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, len); + attrib_db_add(0x0685, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, len); return 0; } @@ -309,4 +353,11 @@ int server_example_init(void) void server_example_exit(void) { + int i; + + for (i = 0; i < SDP_RECORD_COUNT; i++) + if (sdp_records[i]) { + remove_record_from_server(sdp_records[i]->handle); + sdp_records[i] = NULL; + } } diff --git a/src/attrib-server.c b/src/attrib-server.c index 72f5b17..db42ab2 100644 --- a/src/attrib-server.c +++ b/src/attrib-server.c @@ -70,7 +70,8 @@ struct group_elem { static GIOChannel *l2cap_io = NULL; static GIOChannel *le_io = NULL; static GSList *clients = NULL; -static uint32_t sdp_handle = 0; +static uint32_t gatt_sdp_handle = 0; +static uint32_t gap_sdp_handle = 0; static uuid_t prim_uuid = { .type = SDP_UUID16, @@ -81,14 +82,19 @@ static uuid_t snd_uuid = { .value.uuid16 = GATT_SND_SVC_UUID }; -static sdp_record_t *server_record_new(void) +static sdp_record_t *server_record_new(uuid_t *uuid, uint16_t start, uint16_t end) { - sdp_list_t *svclass_id, *apseq, *proto[2], *profiles, *root, *aproto; + sdp_list_t *svclass_id, *apseq, *proto[2], *root, *aproto; uuid_t root_uuid, proto_uuid, gatt_uuid, l2cap; - sdp_profile_desc_t profile; sdp_record_t *record; sdp_data_t *psm, *sh, *eh; - uint16_t lp = GATT_PSM, start = 0x0001, end = 0xffff; + uint16_t lp = GATT_PSM; + + if (uuid == NULL) + return NULL; + + if (start > end) + return NULL; record = sdp_record_alloc(); if (record == NULL) @@ -99,17 +105,10 @@ static sdp_record_t *server_record_new(void) sdp_set_browse_groups(record, root); sdp_list_free(root, NULL); - sdp_uuid16_create(&gatt_uuid, GENERIC_ATTRIB_SVCLASS_ID); - svclass_id = sdp_list_append(NULL, &gatt_uuid); + svclass_id = sdp_list_append(NULL, uuid); sdp_set_service_classes(record, svclass_id); sdp_list_free(svclass_id, NULL); - sdp_uuid16_create(&profile.uuid, GENERIC_ATTRIB_PROFILE_ID); - profile.version = 0x0100; - profiles = sdp_list_append(NULL, &profile); - sdp_set_profile_descs(record, profiles); - sdp_list_free(profiles, NULL); - sdp_uuid16_create(&l2cap, L2CAP_UUID); proto[0] = sdp_list_append(NULL, &l2cap); psm = sdp_data_alloc(SDP_UINT16, &lp); @@ -127,11 +126,6 @@ static sdp_record_t *server_record_new(void) aproto = sdp_list_append(NULL, apseq); sdp_set_access_protos(record, aproto); - sdp_set_info_attr(record, "Generic Attribute Profile", "BlueZ", NULL); - - sdp_set_url_attr(record, "http://www.bluez.org/", - "http://www.bluez.org/", "http://www.bluez.org/"); - sdp_set_service_id(record, gatt_uuid); sdp_data_free(psm); @@ -609,7 +603,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle, static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu, uint8_t *pdu, int len) { - channel->mtu = MIN(mtu, ATT_MAX_MTU); + channel->mtu = MIN(mtu, channel->mtu); return enc_mtu_resp(channel->mtu, pdu, len); } @@ -796,43 +790,68 @@ static void confirm_event(GIOChannel *io, void *user_data) return; } -static void register_core_services(void) +static gboolean register_core_services(void) { uint8_t atval[256]; uuid_t uuid; int len; + sdp_record_t *record; /* GAP service: primary service definition */ sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); att_put_u16(GENERIC_ACCESS_PROFILE_ID, &atval[0]); - attrib_db_add(0x0001, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + record = attrib_db_add(0x0001, 0x0006, &uuid, "Generic Access Profile", + ATT_NONE, ATT_NOT_PERMITTED, atval, 2); /* GAP service: device name characteristic */ sdp_uuid16_create(&uuid, GATT_CHARAC_UUID); atval[0] = ATT_CHAR_PROPER_READ; att_put_u16(0x0006, &atval[1]); att_put_u16(GATT_CHARAC_DEVICE_NAME, &atval[3]); - attrib_db_add(0x0004, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 5); + attrib_db_add(0x0004, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, + atval, 5); /* GAP service: device name attribute */ sdp_uuid16_create(&uuid, GATT_CHARAC_DEVICE_NAME); len = strlen(main_opts.name); - attrib_db_add(0x0006, &uuid, ATT_NONE, ATT_NOT_PERMITTED, + attrib_db_add(0x0006, 0, &uuid, NULL, ATT_NONE, ATT_NOT_PERMITTED, (uint8_t *) main_opts.name, len); /* TODO: Implement Appearance characteristic. It is mandatory for * Peripheral/Central GAP roles. */ + if (record == NULL) { + error("Failed to register GAP service record"); + goto failed; + } + + gap_sdp_handle = record->handle; + /* GATT service: primary service definition */ sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); att_put_u16(GENERIC_ATTRIB_PROFILE_ID, &atval[0]); - attrib_db_add(0x0010, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + record = attrib_db_add(0x0010, 0x0010, &uuid, "Generic Attribute Profile", + ATT_NONE, ATT_NOT_PERMITTED, atval, 2); + + if (record == NULL) { + error("Failed to register GATT service record"); + goto failed; + } + + gatt_sdp_handle = record->handle; + + return TRUE; + +failed: + if (gap_sdp_handle) + remove_record_from_server(gap_sdp_handle); + + return FALSE; } int attrib_server_init(void) { GError *gerr = NULL; - sdp_record_t *record; /* BR/EDR socket */ l2cap_io = bt_io_listen(BT_IO_L2CAP, NULL, confirm_event, @@ -848,21 +867,8 @@ int attrib_server_init(void) return -1; } - record = server_record_new(); - if (record == NULL) { - error("Unable to create GATT service record"); + if (!register_core_services()) goto failed; - } - - if (add_record_to_server(BDADDR_ANY, record) < 0) { - error("Failed to register GATT service record"); - sdp_record_free(record); - goto failed; - } - - sdp_handle = record->handle; - - register_core_services(); if (!main_opts.le) return 0; @@ -921,15 +927,60 @@ void attrib_server_exit(void) g_slist_free(clients); - if (sdp_handle) - remove_record_from_server(sdp_handle); + if (gatt_sdp_handle) + remove_record_from_server(gatt_sdp_handle); + + if (gap_sdp_handle) + remove_record_from_server(gap_sdp_handle); } -int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs, - const uint8_t *value, int len) +sdp_record_t *attrib_db_add(uint16_t handle, uint16_t end, uuid_t *uuid, + const char *name, + int read_reqs, int write_reqs, + const uint8_t *value, int len) { struct attribute *a; + uuid_t svc; + sdp_record_t *record = NULL; + + /* If a valid range is being added AND this is a Primary Service, */ + /* add it to the SDP database as well. */ + + if (end == 0 || handle > end) + goto no_sdp; + + if (uuid->type != SDP_UUID16 || uuid->value.uuid16 != GATT_PRIM_SVC_UUID) + goto no_sdp; + + if (len == 2) { + svc.type = SDP_UUID16; + svc.value.uuid16 = att_get_u16(value); + } else if (len == 16) { + svc.type = SDP_UUID128; + memcpy(&svc.value.uuid128, value, sizeof(uint128_t)); + } else + return NULL; + record = server_record_new(&svc, handle, end); + if (record == NULL) + return NULL; + + if (name) + sdp_set_info_attr(record, name, "BlueZ", NULL); + + if (svc.type == SDP_UUID16 && + svc.value.uuid16 == GENERIC_ACCESS_PROFILE_ID) { + sdp_set_url_attr(record, "http://www.bluez.org/", + "http://www.bluez.org/", + "http://www.bluez.org/"); + } + + if (add_record_to_server(BDADDR_ANY, record) < 0) { + sdp_record_free(record); + return NULL; + } + +no_sdp: /* FIXME: handle conflicts */ a = g_malloc0(sizeof(struct attribute) + len); @@ -942,7 +993,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs, database = g_slist_insert_sorted(database, a, attribute_cmp); - return 0; + return record; } int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value, diff --git a/src/attrib-server.h b/src/attrib-server.h index ba90ff4..2601d0e 100644 --- a/src/attrib-server.h +++ b/src/attrib-server.h @@ -25,8 +25,10 @@ int attrib_server_init(void); void attrib_server_exit(void); -int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs, - const uint8_t *value, int len); +sdp_record_t *attrib_db_add(uint16_t handle, uint16_t end, uuid_t *uuid, + const char *name, + int read_reqs, int write_reqs, + const uint8_t *value, int len); int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value, int len); int attrib_db_del(uint16_t handle); -- 1.7.1 -- Brian Gix bgix@codeaurora.org Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Add SDP registration of Primary GATT services 2011-02-16 21:18 ` [PATCH 2/3] Add SDP registration of Primary GATT services Brian Gix @ 2011-02-14 21:53 ` Anderson Lizardo 2011-02-16 16:26 ` Brian Gix 0 siblings, 1 reply; 11+ messages in thread From: Anderson Lizardo @ 2011-02-14 21:53 UTC (permalink / raw) To: Brian Gix; +Cc: linux-bluetooth, johan.hedberg, padovan On Wed, Feb 16, 2011 at 6:18 PM, Brian Gix <bgix@codeaurora.org> wrote: > SDP registration can be supressed by passing Zero as the end > handle argument to attrib_db_add(). > --- > attrib/example.c | 119 +++++++++++++++++++++++++++++++------------ > src/attrib-server.c | 139 +++++++++++++++++++++++++++++++++++---------------- > src/attrib-server.h | 6 ++- > 3 files changed, 184 insertions(+), 80 deletions(-) > > diff --git a/attrib/example.c b/attrib/example.c > index 1911912..eab3c0f 100644 > --- a/attrib/example.c > +++ b/attrib/example.c > @@ -31,6 +31,7 @@ > > #include <bluetooth/sdp.h> > #include <bluetooth/sdp_lib.h> > +#include <src/sdpd.h> > > #include <glib.h> > > @@ -59,6 +60,9 @@ > #define FMT_KILOGRAM_UUID 0xA010 > #define FMT_HANGING_UUID 0xA011 > > +#define SDP_RECORD_COUNT 10 > +sdp_record_t *sdp_records[SDP_RECORD_COUNT]; > + > static int register_attributes(void) > { > const char *desc_out_temp = "Outside Temperature"; > @@ -77,59 +81,73 @@ static int register_attributes(void) > uint8_t atval[256]; > uuid_t uuid; > int len; > + int i = 0; > > /* Battery state service: primary service definition */ > sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); > att_put_u16(BATTERY_STATE_SVC_UUID, &atval[0]); > - attrib_db_add(0x0100, &uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); > + sdp_records[i++] = attrib_db_add(0x0100, 0x0111, &uuid, > + "Battery State Service", > + ATT_NONE, ATT_NOT_PERMITTED, > + atval, 2); What if instead of changing attrib_db_add() signature to return a SDP record, you create a "sdp_record_from_attrib()" for this purpose? This function could get the struct attribute * pointers for start/end. There are plans for attrib_db_add() to return the created struct attribute *a (as shown in a patch I sent sometime ago which I still need to resend), I can send an updated version of that patch if you agree to this approach. > @@ -609,7 +603,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle, > static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu, > uint8_t *pdu, int len) > { > - channel->mtu = MIN(mtu, ATT_MAX_MTU); > + channel->mtu = MIN(mtu, channel->mtu); > > return enc_mtu_resp(channel->mtu, pdu, len); > } This change looks unrelated to the patch. Regards, -- Anderson Lizardo OpenBossa Labs - INdT Manaus - Brazil ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Add SDP registration of Primary GATT services 2011-02-14 21:53 ` Anderson Lizardo @ 2011-02-16 16:26 ` Brian Gix 2011-02-16 17:00 ` Anderson Lizardo 0 siblings, 1 reply; 11+ messages in thread From: Brian Gix @ 2011-02-16 16:26 UTC (permalink / raw) To: Anderson Lizardo; +Cc: linux-bluetooth, johan.hedberg, padovan Hi Anderson, On 2/14/2011 1:53 PM, Anderson Lizardo wrote: > On Wed, Feb 16, 2011 at 6:18 PM, Brian Gix<bgix@codeaurora.org> wrote: >> SDP registration can be supressed by passing Zero as the end >> handle argument to attrib_db_add(). >> --- >> attrib/example.c | 119 +++++++++++++++++++++++++++++++------------ >> src/attrib-server.c | 139 +++++++++++++++++++++++++++++++++++---------------- >> src/attrib-server.h | 6 ++- >> 3 files changed, 184 insertions(+), 80 deletions(-) >> >> diff --git a/attrib/example.c b/attrib/example.c >> index 1911912..eab3c0f 100644 >> --- a/attrib/example.c >> +++ b/attrib/example.c >> @@ -31,6 +31,7 @@ >> >> #include<bluetooth/sdp.h> >> #include<bluetooth/sdp_lib.h> >> +#include<src/sdpd.h> >> >> #include<glib.h> >> >> @@ -59,6 +60,9 @@ >> #define FMT_KILOGRAM_UUID 0xA010 >> #define FMT_HANGING_UUID 0xA011 >> >> +#define SDP_RECORD_COUNT 10 >> +sdp_record_t *sdp_records[SDP_RECORD_COUNT]; >> + >> static int register_attributes(void) >> { >> const char *desc_out_temp = "Outside Temperature"; >> @@ -77,59 +81,73 @@ static int register_attributes(void) >> uint8_t atval[256]; >> uuid_t uuid; >> int len; >> + int i = 0; >> >> /* Battery state service: primary service definition */ >> sdp_uuid16_create(&uuid, GATT_PRIM_SVC_UUID); >> att_put_u16(BATTERY_STATE_SVC_UUID,&atval[0]); >> - attrib_db_add(0x0100,&uuid, ATT_NONE, ATT_NOT_PERMITTED, atval, 2); >> + sdp_records[i++] = attrib_db_add(0x0100, 0x0111,&uuid, >> + "Battery State Service", >> + ATT_NONE, ATT_NOT_PERMITTED, >> + atval, 2); > > What if instead of changing attrib_db_add() signature to return a SDP > record, you create a "sdp_record_from_attrib()" for this purpose? This > function could get the struct attribute * pointers for start/end. > > There are plans for attrib_db_add() to return the created struct > attribute *a (as shown in a patch I sent sometime ago which I still > need to resend), I can send an updated version of that patch if you > agree to this approach. I was considering an approach like this during UPF but abandoned it for lack of time. There are few hours available to make hot fixes during these events, and I just learned of the requirement for SDP records in "real time". However, here are the issues as I see them: 1. We currently register everything into the attribute database a single attribute at a time. The current attrib_db_add therefore has no inherent knowledge of entire "Service" objects, and lacks the start-end range that is required not only by the SDP record, but also the ATT driven GATT Service Discovery. Ideally I would like to see a "Register Service" API which internally would register all INCLUDES, CHARACTERISTICS, and SDP Records, but this would be a significant departure from the current code. 2. A less significant change would be to create an sdp_record_from_attrib() API, but it would need to be called after all attributes for the service have been registered, because it needs to calculate the "end" handle of the service, much like the current GATT procedures to Discover and Find Services. This would probably constitute the "least change necessary" to the current paradigm. 3. I'm not sure that the "struct attrib *a" would add anything here, except as a way to prevent having to explicitly specify the attribute handle value in the first place. We should eventually get away from specifying explicit attribute handle values, because that makes it hard to have mix-and-match services. A service app writer should not be expected to know the remaining 0-65535 "number space" that is available on every device that they want to target there service app to. In that respect, having attrib_db_add assign an available handle, and return a "struct attrib *" pointer could then be used instead of a handle when registering the SDP of a completely registered primary service. I propose that I change this patch set to follow plan "2" above. If you want to subsequently change the return of attrib_db_add() as you have described, it should then be trivial to change the arguments to the new sdp_record_from_attrib() API to accept the attrib struct pointer rather than the explicit start handle. > >> @@ -609,7 +603,7 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle, >> static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu, >> uint8_t *pdu, int len) >> { >> - channel->mtu = MIN(mtu, ATT_MAX_MTU); >> + channel->mtu = MIN(mtu, channel->mtu); >> >> return enc_mtu_resp(channel->mtu, pdu, len); >> } > > This change looks unrelated to the patch. > > Regards, -- Brian Gix bgix@codeaurora.org Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Add SDP registration of Primary GATT services 2011-02-16 16:26 ` Brian Gix @ 2011-02-16 17:00 ` Anderson Lizardo 0 siblings, 0 replies; 11+ messages in thread From: Anderson Lizardo @ 2011-02-16 17:00 UTC (permalink / raw) To: Brian Gix; +Cc: linux-bluetooth, johan.hedberg, padovan Hi Brian, On Wed, Feb 16, 2011 at 1:26 PM, Brian Gix <bgix@codeaurora.org> wrote: > 1. We currently register everything into the attribute database a single > attribute at a time. The current attrib_db_add therefore has no inherent > knowledge of entire "Service" objects, and lacks the start-end range that is > required not only by the SDP record, but also the ATT driven GATT Service > Discovery. Ideally I would like to see a "Register Service" API which > internally would register all INCLUDES, CHARACTERISTICS, and SDP Records, > but this would be a significant departure from the current code. You are right. In fact, we have a proposal (currently not implemented) to create a server side API for registering GATT services. Please see my e-mail from 2010-12-30 entitled "[RFC] BlueZ server side API for registering GATT services". I was actually expecting comments from you :) Suggestions/comments on the proposal are welcome. Please try to keep discussion on this topic here on the list because there are other people interested on contributing to the implementation as well. ALso note the proposal is to create an API on top of the current one, with minimal changes to allow both to be used in parallel (it may be necessary in cases we want more control over the attributes). > 2. A less significant change would be to create an sdp_record_from_attrib() > API, but it would need to be called after all attributes for the service > have been registered, because it needs to calculate the "end" handle of the > service, much like the current GATT procedures to Discover and Find > Services. This would probably constitute the "least change necessary" to the > current paradigm. This is my suggestion on this thread. After the attributes of a specific service have been inserted into the database, you can know the start-end attributes. > 3. I'm not sure that the "struct attrib *a" would add anything here, except > as a way to prevent having to explicitly specify the attribute handle value > in the first place. We should eventually get away from specifying explicit > attribute handle values, because that makes it hard to have mix-and-match > services. A service app writer should not be expected to know the remaining > 0-65535 "number space" that is available on every device that they want to > target there service app to. In that respect, having attrib_db_add assign > an available handle, and return a "struct attrib *" pointer could then be > used instead of a handle when registering the SDP of a completely registered > primary service. What I had in mind (still not discussed on the list), was to make the "automatic handle allocation" feature together with the high-level GATT Service oriented API. This would work by keeping track of used/unused start-end ranges, and allocating ranges for services as they are registered/unregistered. Example: * "free range list" starts with: [0x1000, 0xFFFF] * Register service A with 10 attributes: [0x1000, 0x1009] (free range: [0x100A, 0xFFFF]) * Register service B with 5 attributes: [0x100A, 0x100E] (free range: [0x100F, 0xFFFF]) * Register service C with 11 attributes: [0x100F, 0x1019] (free range: [0x101A, 0xFFFF]) * Unregister service B (free ranges: [0x100A, 0x100E], [0x101A, 0xFFFF]) * Register service D with 4 attributes: [0x100A, 0x100D] (free ranges: [0x100E, 0x100E], [0x101A, 0xFFFF]) > I propose that I change this patch set to follow plan "2" above. If you > want to subsequently change the return of attrib_db_add() as you have > described, it should then be trivial to change the arguments to the new > sdp_record_from_attrib() API to accept the attrib struct pointer rather than > the explicit start handle. Agreed. sdp_record_from_attrib() may receive just start/end handles if other fields of struct attrubute are not necessary. >>> @@ -609,7 +603,7 @@ static uint16_t write_value(struct gatt_channel >>> *channel, uint16_t handle, >>> static uint16_t mtu_exchange(struct gatt_channel *channel, uint16_t mtu, >>> uint8_t *pdu, int len) >>> { >>> - channel->mtu = MIN(mtu, ATT_MAX_MTU); >>> + channel->mtu = MIN(mtu, channel->mtu); >>> >>> return enc_mtu_resp(channel->mtu, pdu, len); >>> } >> >> This change looks unrelated to the patch. Any comments on this? Regards, -- Anderson Lizardo Instituto Nokia de Tecnologia - INdT Manaus - Brazil ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] Add gatttool enhancements for UPF 2011-02-16 21:17 [PATCH 0/3] Modifications composed during UPF38 Brian Gix 2011-02-16 21:17 ` [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers Brian Gix 2011-02-16 21:18 ` [PATCH 2/3] Add SDP registration of Primary GATT services Brian Gix @ 2011-02-16 21:18 ` Brian Gix 2011-02-14 21:41 ` Anderson Lizardo 2 siblings, 1 reply; 11+ messages in thread From: Brian Gix @ 2011-02-16 21:18 UTC (permalink / raw) To: linux-bluetooth; +Cc: johan.hedberg, padovan, Brian Gix Including seperating write-cmd from write-char, and enabling --listen to stand on it's own if needed. (PTS tester) --- attrib/gatttool.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 52 insertions(+), 5 deletions(-) diff --git a/attrib/gatttool.c b/attrib/gatttool.c index 8e8ed8e..054adb1 100644 --- a/attrib/gatttool.c +++ b/attrib/gatttool.c @@ -63,6 +63,7 @@ static gboolean opt_listen = FALSE; static gboolean opt_char_desc = FALSE; static gboolean opt_le = FALSE; static gboolean opt_char_write = FALSE; +static gboolean opt_char_cmd = FALSE; static GMainLoop *event_loop; static gboolean got_error = FALSE; @@ -90,9 +91,9 @@ static GIOChannel *do_connect(gboolean le) /* This check is required because currently setsockopt() returns no * errors for MTU values smaller than the allowed minimum. */ - if (opt_mtu != 0 && opt_mtu < ATT_MIN_MTU_L2CAP) { + if (opt_mtu != 0 && opt_mtu < 23) { g_printerr("MTU cannot be smaller than %d\n", - ATT_MIN_MTU_L2CAP); + 23); return NULL; } @@ -277,6 +278,14 @@ static gboolean characteristics(gpointer user_data) return FALSE; } +static void char_write_cb(guint8 status, const guint8 *pdu, guint16 plen, + gpointer user_data) +{ + if (plen == 1) + g_print("Attrib Write Succeeded\n"); + else + g_printerr("Attrib Write failed: %s\n", att_ecode2str(status)); +} static void char_read_cb(guint8 status, const guint8 *pdu, guint16 plen, gpointer user_data) { @@ -427,7 +436,40 @@ static gboolean characteristics_write(gpointer user_data) goto error; } + gatt_write_char(attrib, opt_handle, value, len, char_write_cb, value); + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); + + return FALSE; + +error: + g_main_loop_quit(event_loop); + return FALSE; +} + +static gboolean characteristics_cmd(gpointer user_data) +{ + GAttrib *attrib = user_data; + uint8_t *value; + size_t len; + + if (opt_handle <= 0) { + g_printerr("A valid handle is required\n"); + goto error; + } + + if (opt_value == NULL || opt_value[0] == '\0') { + g_printerr("A value is required\n"); + goto error; + } + + len = attr_data_from_string(opt_value, &value); + if (len == 0) { + g_printerr("Invalid value\n"); + goto error; + } + gatt_write_cmd(attrib, opt_handle, value, len, mainloop_quit, value); + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); return FALSE; @@ -531,6 +573,8 @@ static GOptionEntry gatt_options[] = { "Characteristics Value/Descriptor Read", NULL }, { "char-write", 0, 0, G_OPTION_ARG_NONE, &opt_char_write, "Characteristics Value Write", NULL }, + { "char-cmd", 0, 0, G_OPTION_ARG_NONE, &opt_char_cmd, + "Characteristics Value Cmd", NULL }, { "char-desc", 0, 0, G_OPTION_ARG_NONE, &opt_char_desc, "Characteristics Descriptor Discovery", NULL }, { "listen", 0, 0, G_OPTION_ARG_NONE, &opt_listen, @@ -561,7 +605,7 @@ int main(int argc, char *argv[]) GError *gerr = NULL; GAttrib *attrib; GIOChannel *chan; - GSourceFunc callback; + GSourceFunc callback = NULL; context = g_option_context_new(NULL); g_option_context_add_main_entries(context, options, NULL); @@ -602,9 +646,11 @@ int main(int argc, char *argv[]) callback = characteristics_read; else if (opt_char_write) callback = characteristics_write; + else if (opt_char_cmd) + callback = characteristics_cmd; else if (opt_char_desc) callback = characteristics_desc; - else { + else if (!opt_listen) { gchar *help = g_option_context_get_help(context, TRUE, NULL); g_print("%s\n", help); g_free(help); @@ -625,7 +671,8 @@ int main(int argc, char *argv[]) if (opt_listen) g_idle_add(listen_start, attrib); - g_idle_add(callback, attrib); + if (callback) + g_idle_add(callback, attrib); g_main_loop_run(event_loop); -- 1.7.1 -- Brian Gix bgix@codeaurora.org Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Add gatttool enhancements for UPF 2011-02-16 21:18 ` [PATCH 3/3] Add gatttool enhancements for UPF Brian Gix @ 2011-02-14 21:41 ` Anderson Lizardo 2011-02-16 15:53 ` Brian Gix 0 siblings, 1 reply; 11+ messages in thread From: Anderson Lizardo @ 2011-02-14 21:41 UTC (permalink / raw) To: Brian Gix; +Cc: linux-bluetooth, johan.hedberg, padovan Hi Brian, On Wed, Feb 16, 2011 at 6:18 PM, Brian Gix <bgix@codeaurora.org> wrote: > @@ -90,9 +91,9 @@ static GIOChannel *do_connect(gboolean le) > > /* This check is required because currently setsockopt() returns no > * errors for MTU values smaller than the allowed minimum. */ > - if (opt_mtu != 0 && opt_mtu < ATT_MIN_MTU_L2CAP) { > + if (opt_mtu != 0 && opt_mtu < 23) { > g_printerr("MTU cannot be smaller than %d\n", > - ATT_MIN_MTU_L2CAP); > + 23); > return NULL; > } The changes above seem unrelated to this patch. > > @@ -277,6 +278,14 @@ static gboolean characteristics(gpointer user_data) > return FALSE; > } > > +static void char_write_cb(guint8 status, const guint8 *pdu, guint16 plen, > + gpointer user_data) > +{ > + if (plen == 1) > + g_print("Attrib Write Succeeded\n"); > + else > + g_printerr("Attrib Write failed: %s\n", att_ecode2str(status)); Why not check by the status instead of plen ? Also, I'd suggest "Characteristic write" instead of "Attrib Write". > +} > static void char_read_cb(guint8 status, const guint8 *pdu, guint16 plen, > gpointer user_data) > { > @@ -427,7 +436,40 @@ static gboolean characteristics_write(gpointer user_data) > goto error; > } > > + gatt_write_char(attrib, opt_handle, value, len, char_write_cb, value); > + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); Why both read and write ? > + > + return FALSE; > + > +error: > + g_main_loop_quit(event_loop); > + return FALSE; > +} > + > +static gboolean characteristics_cmd(gpointer user_data) > +{ > + GAttrib *attrib = user_data; > + uint8_t *value; > + size_t len; > + > + if (opt_handle <= 0) { > + g_printerr("A valid handle is required\n"); > + goto error; > + } > + > + if (opt_value == NULL || opt_value[0] == '\0') { > + g_printerr("A value is required\n"); > + goto error; > + } > + > + len = attr_data_from_string(opt_value, &value); > + if (len == 0) { > + g_printerr("Invalid value\n"); > + goto error; > + } > + > gatt_write_cmd(attrib, opt_handle, value, len, mainloop_quit, value); > + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); Same question here. > > return FALSE; > > @@ -531,6 +573,8 @@ static GOptionEntry gatt_options[] = { > "Characteristics Value/Descriptor Read", NULL }, > { "char-write", 0, 0, G_OPTION_ARG_NONE, &opt_char_write, > "Characteristics Value Write", NULL }, > + { "char-cmd", 0, 0, G_OPTION_ARG_NONE, &opt_char_cmd, > + "Characteristics Value Cmd", NULL }, Suggestion: "Characteristic Value write using Write Command" (or something similar). > { "char-desc", 0, 0, G_OPTION_ARG_NONE, &opt_char_desc, > "Characteristics Descriptor Discovery", NULL }, > { "listen", 0, 0, G_OPTION_ARG_NONE, &opt_listen, > @@ -561,7 +605,7 @@ int main(int argc, char *argv[]) > GError *gerr = NULL; > GAttrib *attrib; > GIOChannel *chan; > - GSourceFunc callback; > + GSourceFunc callback = NULL; > > context = g_option_context_new(NULL); > g_option_context_add_main_entries(context, options, NULL); > @@ -602,9 +646,11 @@ int main(int argc, char *argv[]) > callback = characteristics_read; > else if (opt_char_write) > callback = characteristics_write; > + else if (opt_char_cmd) > + callback = characteristics_cmd; > else if (opt_char_desc) > callback = characteristics_desc; > - else { > + else if (!opt_listen) { > gchar *help = g_option_context_get_help(context, TRUE, NULL); > g_print("%s\n", help); > g_free(help); > @@ -625,7 +671,8 @@ int main(int argc, char *argv[]) > if (opt_listen) > g_idle_add(listen_start, attrib); > > - g_idle_add(callback, attrib); > + if (callback) > + g_idle_add(callback, attrib); > > g_main_loop_run(event_loop); Regards, -- Anderson Lizardo OpenBossa Labs - INdT Manaus - Brazil ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Add gatttool enhancements for UPF 2011-02-14 21:41 ` Anderson Lizardo @ 2011-02-16 15:53 ` Brian Gix 2011-02-16 16:38 ` Anderson Lizardo 0 siblings, 1 reply; 11+ messages in thread From: Brian Gix @ 2011-02-16 15:53 UTC (permalink / raw) To: Anderson Lizardo; +Cc: linux-bluetooth, johan.hedberg, padovan Hi Anderson, The modifications to gatttool were largely organic in nature and on the fly responses to the needs of my UPF testing. Also, it is of course just a test tool. More inline. On 2/14/2011 1:41 PM, Anderson Lizardo wrote: > Hi Brian, > > On Wed, Feb 16, 2011 at 6:18 PM, Brian Gix<bgix@codeaurora.org> wrote: >> @@ -90,9 +91,9 @@ static GIOChannel *do_connect(gboolean le) >> >> /* This check is required because currently setsockopt() returns no >> * errors for MTU values smaller than the allowed minimum. */ >> - if (opt_mtu != 0&& opt_mtu< ATT_MIN_MTU_L2CAP) { >> + if (opt_mtu != 0&& opt_mtu< 23) { >> g_printerr("MTU cannot be smaller than %d\n", >> - ATT_MIN_MTU_L2CAP); >> + 23); >> return NULL; >> } > > The changes above seem unrelated to this patch. The code was incorrect in that it limited it to 48 instead of 23. However, I should have used the ATT_MIN_MTU_LE define. This will be fixed. > >> >> @@ -277,6 +278,14 @@ static gboolean characteristics(gpointer user_data) >> return FALSE; >> } >> >> +static void char_write_cb(guint8 status, const guint8 *pdu, guint16 plen, >> + gpointer user_data) >> +{ >> + if (plen == 1) >> + g_print("Attrib Write Succeeded\n"); >> + else >> + g_printerr("Attrib Write failed: %s\n", att_ecode2str(status)); > > Why not check by the status instead of plen ? During testing, it was apparent that status did not contain Zero. This may be due to there being no status octet in the WRITE_RSP packet. I didn't have time to trace this back into the gatt code, but will do so now. > > Also, I'd suggest "Characteristic write" instead of "Attrib Write". > >> +} >> static void char_read_cb(guint8 status, const guint8 *pdu, guint16 plen, >> gpointer user_data) >> { >> @@ -427,7 +436,40 @@ static gboolean characteristics_write(gpointer user_data) >> goto error; >> } >> >> + gatt_write_char(attrib, opt_handle, value, len, char_write_cb, value); >> + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); > > Why both read and write ? I needed to do this to confirm that the write actually succeeded within the bounds of the testing, which included not dropping the connection between the two procedures. This will not be needed for the forthcoming interactive gatttool, but that was not available for UPF. As it is, I would argue that this is more useful in a test tool than superfluous. > >> + >> + return FALSE; >> + >> +error: >> + g_main_loop_quit(event_loop); >> + return FALSE; >> +} >> + >> +static gboolean characteristics_cmd(gpointer user_data) >> +{ >> + GAttrib *attrib = user_data; >> + uint8_t *value; >> + size_t len; >> + >> + if (opt_handle<= 0) { >> + g_printerr("A valid handle is required\n"); >> + goto error; >> + } >> + >> + if (opt_value == NULL || opt_value[0] == '\0') { >> + g_printerr("A value is required\n"); >> + goto error; >> + } >> + >> + len = attr_data_from_string(opt_value,&value); >> + if (len == 0) { >> + g_printerr("Invalid value\n"); >> + goto error; >> + } >> + >> gatt_write_cmd(attrib, opt_handle, value, len, mainloop_quit, value); >> + gatt_read_char(attrib, opt_handle, char_read_cb, attrib); > > Same question here. Same response here. > >> >> return FALSE; >> >> @@ -531,6 +573,8 @@ static GOptionEntry gatt_options[] = { >> "Characteristics Value/Descriptor Read", NULL }, >> { "char-write", 0, 0, G_OPTION_ARG_NONE,&opt_char_write, >> "Characteristics Value Write", NULL }, >> + { "char-cmd", 0, 0, G_OPTION_ARG_NONE,&opt_char_cmd, >> + "Characteristics Value Cmd", NULL }, > > Suggestion: "Characteristic Value write using Write Command" (or > something similar). > >> { "char-desc", 0, 0, G_OPTION_ARG_NONE,&opt_char_desc, >> "Characteristics Descriptor Discovery", NULL }, >> { "listen", 0, 0, G_OPTION_ARG_NONE,&opt_listen, >> @@ -561,7 +605,7 @@ int main(int argc, char *argv[]) >> GError *gerr = NULL; >> GAttrib *attrib; >> GIOChannel *chan; >> - GSourceFunc callback; >> + GSourceFunc callback = NULL; >> >> context = g_option_context_new(NULL); >> g_option_context_add_main_entries(context, options, NULL); >> @@ -602,9 +646,11 @@ int main(int argc, char *argv[]) >> callback = characteristics_read; >> else if (opt_char_write) >> callback = characteristics_write; >> + else if (opt_char_cmd) >> + callback = characteristics_cmd; >> else if (opt_char_desc) >> callback = characteristics_desc; >> - else { >> + else if (!opt_listen) { >> gchar *help = g_option_context_get_help(context, TRUE, NULL); >> g_print("%s\n", help); >> g_free(help); >> @@ -625,7 +671,8 @@ int main(int argc, char *argv[]) >> if (opt_listen) >> g_idle_add(listen_start, attrib); >> >> - g_idle_add(callback, attrib); >> + if (callback) >> + g_idle_add(callback, attrib); >> >> g_main_loop_run(event_loop); > > Regards, -- Brian Gix bgix@codeaurora.org Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] Add gatttool enhancements for UPF 2011-02-16 15:53 ` Brian Gix @ 2011-02-16 16:38 ` Anderson Lizardo 0 siblings, 0 replies; 11+ messages in thread From: Anderson Lizardo @ 2011-02-16 16:38 UTC (permalink / raw) To: Brian Gix; +Cc: linux-bluetooth, johan.hedberg, padovan Hi Brian, On Wed, Feb 16, 2011 at 12:53 PM, Brian Gix <bgix@codeaurora.org> wrote: > Hi Anderson, > > The modifications to gatttool were largely organic in nature and on the fly > responses to the needs of my UPF testing. Also, it is of course just a test > tool. Understood, but if you could split these changes into more "logic" pieces (separate commits), with commit messages which explain the reasoning of the change (e.g. the answers on this thread), it will be nice. Regards, -- Anderson Lizardo Instituto Nokia de Tecnologia - INdT Manaus - Brazil ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-02-16 21:18 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-16 21:17 [PATCH 0/3] Modifications composed during UPF38 Brian Gix 2011-02-16 21:17 ` [PATCH 1/3] Fixed def of ATT_UUID per BT Assigned Numbers Brian Gix 2011-02-15 17:08 ` Johan Hedberg 2011-02-16 21:18 ` [PATCH 2/3] Add SDP registration of Primary GATT services Brian Gix 2011-02-14 21:53 ` Anderson Lizardo 2011-02-16 16:26 ` Brian Gix 2011-02-16 17:00 ` Anderson Lizardo 2011-02-16 21:18 ` [PATCH 3/3] Add gatttool enhancements for UPF Brian Gix 2011-02-14 21:41 ` Anderson Lizardo 2011-02-16 15:53 ` Brian Gix 2011-02-16 16:38 ` Anderson Lizardo
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.