This is one of the parts of a series of patches related to connection
parameters update.
This initial implementation adds supports to GAP Peripheral Preferred
Connection Parameters (PPCP) characteristic and the Slave Connection Interval
Range Advertising Data (AD).
There is a problem described in #4 that due to a limitation of the BlueZ API
it is not possible to update the current connection with specific parameters.
I am working on a socket option API to allow something similar. Hopefully I
will be able to write something that covers this use-case as well. This
problem also applies to patch #6.
Changes from v1:
* Added Slave Connection Interval Range AD support;
* Unit tests for connection interval.
Felipe F. Tonello (7):
profiles/gap: Some code cleanup
src/adapter: Added connection parameter load/store functions
src/device: Added function to set connection parameters
profiles/gap: Added support for PPCP characteristic
src/eir: Added support for Slave Connection Interval Range AD
src/adapter: Store Slave Connection Interval Range
unit/test-eir: Added tests for connection interval
profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
src/adapter.c | 46 +++++++++++++++++++++-
src/adapter.h | 9 +++++
src/device.c | 16 ++++++++
src/device.h | 4 ++
src/eir.c | 7 ++++
src/eir.h | 3 ++
unit/test-eir.c | 26 +++++++++++--
8 files changed, 196 insertions(+), 26 deletions(-)
--
2.11.1
Hi Felipe,
On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
> It is interesting to let other parts of bluetoothd to access these
> functions since there are few different use-cases where this updating
> and loading connection parameters can happen.
> ---
> src/adapter.c | 24 ++++++++++++++++++++++--
> src/adapter.h | 9 +++++++++
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 3dac7d6490d6..b63883e3cae5 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -3609,6 +3609,26 @@ static void load_conn_params(struct btd_adapter *adapter, GSList *params)
> btd_error(adapter->dev_id, "Load connection parameters failed");
> }
>
> +void adapter_load_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
> + uint8_t bdaddr_type, uint16_t min_interval,
> + uint16_t max_interval, uint16_t latency,
> + uint16_t timeout)
> +{
> + GSList *params = NULL;
> + struct conn_param param;
> +
> + bacpy(¶m.bdaddr, peer);
> + param.bdaddr_type = bdaddr_type;
> + param.max_interval = max_interval;
> + param.min_interval = min_interval;
> + param.latency = latency;
> + param.timeout = timeout;
> +
> + params = g_slist_append(params, ¶m);
> + load_conn_params(adapter, params);
Actually calling load_conn_params with just 1 setting does not work,
the kernel does call hci_conn_params_clear_disabled before parsing the
list of the connections which means it may endup cleaning up all the
settings previously loaded. So we have to decide if the kernel should
continue doing this thus the userspace has to reload everything or we
change the handling in the kernel to update the existing settings
which perhaps it is just a matter of removing
hci_conn_params_clear_disabled call from load_conn_param since the
following code code seems to be handling the updates:
hci_param = hci_conn_params_add(hdev, ¶m->addr.bdaddr,
addr_type);
if (!hci_param) {
BT_ERR("Failed to add connection parameters");
continue;
}
hci_param->conn_min_interval = min;
hci_param->conn_max_interval = max;
hci_param->conn_latency = latency;
hci_param->supervision_timeout = timeout;
> + g_slist_free(params);
> +}
> +
> static uint8_t get_le_addr_type(GKeyFile *keyfile)
> {
> uint8_t addr_type;
> @@ -7251,7 +7271,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
> btd_device_set_temporary(device, false);
> }
>
> -static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
> +void adapter_store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
> uint8_t bdaddr_type, uint16_t min_interval,
> uint16_t max_interval, uint16_t latency,
> uint16_t timeout)
> @@ -7325,7 +7345,7 @@ static void new_conn_param(uint16_t index, uint16_t length,
> if (!ev->store_hint)
> return;
>
> - store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
> + adapter_store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
> ev->min_interval, ev->max_interval,
> ev->latency, ev->timeout);
> }
> diff --git a/src/adapter.h b/src/adapter.h
> index f9178d59e74a..ca2f6678cfca 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -227,3 +227,12 @@ void btd_adapter_for_each_device(struct btd_adapter *adapter,
>
> bool btd_le_connect_before_pairing(void);
>
> +void adapter_load_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
> + uint8_t bdaddr_type, uint16_t min_interval,
> + uint16_t max_interval, uint16_t latency,
> + uint16_t timeout);
> +
> +void adapter_store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
> + uint8_t bdaddr_type, uint16_t min_interval,
> + uint16_t max_interval, uint16_t latency,
> + uint16_t timeout);
> --
> 2.11.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi Luiz,
On 20/02/17 15:21, Luiz Augusto von Dentz wrote:
> Hi Felipe,
>
> On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
>> This is one of the parts of a series of patches related to connection
>> parameters update.
>>
>> This initial implementation adds supports to GAP Peripheral Preferred
>> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
>> Range Advertising Data (AD).
>
>
> Are there any devices implementing these? Id like to do a some testing
> but haven't find anything implementing this, or well I can generate
> the PPCP but the SCRAD is probably a waste of AD since that is pretty
> limited in size.
PPCP not that I know of. But the AD there are devices, including in the
unit-eir there is one.
The PPCP and AD is important specially for MIDI devices who require
low-latency.
I did test it here, it will be nice if you could as well.
>
>> There is a problem described in #4 that due to a limitation of the BlueZ API
>> it is not possible to update the current connection with specific parameters.
>> I am working on a socket option API to allow something similar. Hopefully I
>> will be able to write something that covers this use-case as well. This
>> problem also applies to patch #6.
>>
>> Changes from v1:
>> * Added Slave Connection Interval Range AD support;
>> * Unit tests for connection interval.
>>
>> Felipe F. Tonello (7):
>> profiles/gap: Some code cleanup
>> src/adapter: Added connection parameter load/store functions
>> src/device: Added function to set connection parameters
>> profiles/gap: Added support for PPCP characteristic
>> src/eir: Added support for Slave Connection Interval Range AD
>> src/adapter: Store Slave Connection Interval Range
>> unit/test-eir: Added tests for connection interval
>>
>> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
>> src/adapter.c | 46 +++++++++++++++++++++-
>> src/adapter.h | 9 +++++
>> src/device.c | 16 ++++++++
>> src/device.h | 4 ++
>> src/eir.c | 7 ++++
>> src/eir.h | 3 ++
>> unit/test-eir.c | 26 +++++++++++--
>> 8 files changed, 196 insertions(+), 26 deletions(-)
>>
>> --
>> 2.11.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
--
Felipe
Hi Luiz,
On 20/02/17 16:55, Luiz Augusto von Dentz wrote:
> Hi Felipe,
>
> On Fri, Feb 17, 2017 at 2:17 PM, Felipe Ferreri Tonello
> <[email protected]> wrote:
>> Hi,
>>
>> On 16/02/17 18:03, Felipe F. Tonello wrote:
>>> This function allows plugins to set the connection parameters of the
>>> respective btd_device object.
>>>
>>> It is useful for GAP Peripheral Preferred Connection Parameters
>>> characteristic for example.
>>> ---
>>> src/device.c | 16 ++++++++++++++++
>>> src/device.h | 4 ++++
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/src/device.c b/src/device.c
>>> index 8693eb826b54..d4aa7f90561e 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -6122,3 +6122,19 @@ void btd_device_cleanup(void)
>>> {
>>> btd_service_remove_state_cb(service_state_cb_id);
>>> }
>>> +
>>> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
>>> + uint16_t max_interval, uint16_t latency,
>>> + uint16_t timeout)
>>> +{
>>> + btd_assert(device != NULL);
>>
>> This assert shouldn't be here. I'll fix next revision.
>
> Is this new revision coming anytime soon?
I was waiting on any comments before.
>
>>> +
>>> + adapter_store_conn_param(device->adapter, &device->bdaddr,
>>> + device->bdaddr_type, min_interval,
>>> + max_interval, latency,
>>> + timeout);
>>> + adapter_load_conn_param(device->adapter, &device->bdaddr,
>>> + device->bdaddr_type, min_interval,
>>> + max_interval, latency,
>>> + timeout);
>>> +}
>>> diff --git a/src/device.h b/src/device.h
>>> index 3cab366eeaea..88076d2d196d 100644
>>> --- a/src/device.h
>>> +++ b/src/device.h
>>> @@ -163,3 +163,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
>>>
>>> void btd_device_init(void);
>>> void btd_device_cleanup(void);
>>> +
>>> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
>>> + uint16_t max_interval, uint16_t latency,
>>> + uint16_t timeout);
>>>
>>
>> --
>> Felipe
>
>
>
--
Felipe
Hi Felipe,
On Fri, Feb 17, 2017 at 2:17 PM, Felipe Ferreri Tonello
<[email protected]> wrote:
> Hi,
>
> On 16/02/17 18:03, Felipe F. Tonello wrote:
>> This function allows plugins to set the connection parameters of the
>> respective btd_device object.
>>
>> It is useful for GAP Peripheral Preferred Connection Parameters
>> characteristic for example.
>> ---
>> src/device.c | 16 ++++++++++++++++
>> src/device.h | 4 ++++
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 8693eb826b54..d4aa7f90561e 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -6122,3 +6122,19 @@ void btd_device_cleanup(void)
>> {
>> btd_service_remove_state_cb(service_state_cb_id);
>> }
>> +
>> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
>> + uint16_t max_interval, uint16_t latency,
>> + uint16_t timeout)
>> +{
>> + btd_assert(device != NULL);
>
> This assert shouldn't be here. I'll fix next revision.
Is this new revision coming anytime soon?
>> +
>> + adapter_store_conn_param(device->adapter, &device->bdaddr,
>> + device->bdaddr_type, min_interval,
>> + max_interval, latency,
>> + timeout);
>> + adapter_load_conn_param(device->adapter, &device->bdaddr,
>> + device->bdaddr_type, min_interval,
>> + max_interval, latency,
>> + timeout);
>> +}
>> diff --git a/src/device.h b/src/device.h
>> index 3cab366eeaea..88076d2d196d 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -163,3 +163,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
>>
>> void btd_device_init(void);
>> void btd_device_cleanup(void);
>> +
>> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
>> + uint16_t max_interval, uint16_t latency,
>> + uint16_t timeout);
>>
>
> --
> Felipe
--
Luiz Augusto von Dentz
Hi Felipe,
On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
> This is one of the parts of a series of patches related to connection
> parameters update.
>
> This initial implementation adds supports to GAP Peripheral Preferred
> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
> Range Advertising Data (AD).
Are there any devices implementing these? Id like to do a some testing
but haven't find anything implementing this, or well I can generate
the PPCP but the SCRAD is probably a waste of AD since that is pretty
limited in size.
> There is a problem described in #4 that due to a limitation of the BlueZ API
> it is not possible to update the current connection with specific parameters.
> I am working on a socket option API to allow something similar. Hopefully I
> will be able to write something that covers this use-case as well. This
> problem also applies to patch #6.
>
> Changes from v1:
> * Added Slave Connection Interval Range AD support;
> * Unit tests for connection interval.
>
> Felipe F. Tonello (7):
> profiles/gap: Some code cleanup
> src/adapter: Added connection parameter load/store functions
> src/device: Added function to set connection parameters
> profiles/gap: Added support for PPCP characteristic
> src/eir: Added support for Slave Connection Interval Range AD
> src/adapter: Store Slave Connection Interval Range
> unit/test-eir: Added tests for connection interval
>
> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
> src/adapter.c | 46 +++++++++++++++++++++-
> src/adapter.h | 9 +++++
> src/device.c | 16 ++++++++
> src/device.h | 4 ++
> src/eir.c | 7 ++++
> src/eir.h | 3 ++
> unit/test-eir.c | 26 +++++++++++--
> 8 files changed, 196 insertions(+), 26 deletions(-)
>
> --
> 2.11.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi,
On 16/02/17 18:03, Felipe F. Tonello wrote:
> This function allows plugins to set the connection parameters of the
> respective btd_device object.
>
> It is useful for GAP Peripheral Preferred Connection Parameters
> characteristic for example.
> ---
> src/device.c | 16 ++++++++++++++++
> src/device.h | 4 ++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 8693eb826b54..d4aa7f90561e 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -6122,3 +6122,19 @@ void btd_device_cleanup(void)
> {
> btd_service_remove_state_cb(service_state_cb_id);
> }
> +
> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
> + uint16_t max_interval, uint16_t latency,
> + uint16_t timeout)
> +{
> + btd_assert(device != NULL);
This assert shouldn't be here. I'll fix next revision.
> +
> + adapter_store_conn_param(device->adapter, &device->bdaddr,
> + device->bdaddr_type, min_interval,
> + max_interval, latency,
> + timeout);
> + adapter_load_conn_param(device->adapter, &device->bdaddr,
> + device->bdaddr_type, min_interval,
> + max_interval, latency,
> + timeout);
> +}
> diff --git a/src/device.h b/src/device.h
> index 3cab366eeaea..88076d2d196d 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -163,3 +163,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
>
> void btd_device_init(void);
> void btd_device_cleanup(void);
> +
> +void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
> + uint16_t max_interval, uint16_t latency,
> + uint16_t timeout);
>
--
Felipe
It is interesting to let other parts of bluetoothd to access these
functions since there are few different use-cases where this updating
and loading connection parameters can happen.
---
src/adapter.c | 24 ++++++++++++++++++++++--
src/adapter.h | 9 +++++++++
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 3dac7d6490d6..b63883e3cae5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3609,6 +3609,26 @@ static void load_conn_params(struct btd_adapter *adapter, GSList *params)
btd_error(adapter->dev_id, "Load connection parameters failed");
}
+void adapter_load_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
+ uint8_t bdaddr_type, uint16_t min_interval,
+ uint16_t max_interval, uint16_t latency,
+ uint16_t timeout)
+{
+ GSList *params = NULL;
+ struct conn_param param;
+
+ bacpy(¶m.bdaddr, peer);
+ param.bdaddr_type = bdaddr_type;
+ param.max_interval = max_interval;
+ param.min_interval = min_interval;
+ param.latency = latency;
+ param.timeout = timeout;
+
+ params = g_slist_append(params, ¶m);
+ load_conn_params(adapter, params);
+ g_slist_free(params);
+}
+
static uint8_t get_le_addr_type(GKeyFile *keyfile)
{
uint8_t addr_type;
@@ -7251,7 +7271,7 @@ static void new_irk_callback(uint16_t index, uint16_t length,
btd_device_set_temporary(device, false);
}
-static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
+void adapter_store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
uint8_t bdaddr_type, uint16_t min_interval,
uint16_t max_interval, uint16_t latency,
uint16_t timeout)
@@ -7325,7 +7345,7 @@ static void new_conn_param(uint16_t index, uint16_t length,
if (!ev->store_hint)
return;
- store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
+ adapter_store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
ev->min_interval, ev->max_interval,
ev->latency, ev->timeout);
}
diff --git a/src/adapter.h b/src/adapter.h
index f9178d59e74a..ca2f6678cfca 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -227,3 +227,12 @@ void btd_adapter_for_each_device(struct btd_adapter *adapter,
bool btd_le_connect_before_pairing(void);
+void adapter_load_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
+ uint8_t bdaddr_type, uint16_t min_interval,
+ uint16_t max_interval, uint16_t latency,
+ uint16_t timeout);
+
+void adapter_store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
+ uint8_t bdaddr_type, uint16_t min_interval,
+ uint16_t max_interval, uint16_t latency,
+ uint16_t timeout);
--
2.11.1
The Peripheral Preferred Connection Parameters (PPCP) characteristic
contains the preferred connection parameters of a peripheral device.
These parameters are stored in the info file and loaded to Kernel using
MGMT's respective command.
IMPORTANT: The connection will only be updated upon next reconnection to
the peripheral device. This is a limitation of the current BlueZ API
that doesn't allow change specific parameters of current connection.
---
profiles/gap/gas.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 0d34503ad446..a9f09f0b3299 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -162,6 +162,75 @@ static void handle_appearance(struct gas *gas, uint16_t value_handle)
DBG("Failed to send request to read appearance");
}
+static void read_ppcp_cb(bool success, uint8_t att_ecode,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
+{
+ struct gas *gas = user_data;
+ uint16_t min_interval, max_interval, latency, timeout, max_latency;
+
+ if (!success) {
+ DBG("Reading PPCP failed with ATT error: %u", att_ecode);
+ return;
+ }
+
+ if (length != 8) {
+ DBG("Malformed PPCP value");
+ return;
+ }
+
+ min_interval = get_le16(&value[0]);
+ max_interval = get_le16(&value[2]);
+ latency = get_le16(&value[4]);
+ timeout = get_le16(&value[6]);
+
+ DBG("GAP Peripheral Preferred Connection Parameters:");
+ DBG("\tMinimum connection interval: %u", min_interval);
+ DBG("\tMaximum connection interval: %u", max_interval);
+ DBG("\tSlave latency: %u", latency);
+ DBG("\tConnection Supervision timeout multiplier: %u", timeout);
+
+ /* 0xffff indicates no specific min/max */
+ if (min_interval == 0xffff)
+ min_interval = 6;
+
+ if (max_interval == 0xffff)
+ max_interval = 3200;
+
+ /* avoid persisting connection parameters that are not valid */
+ if (min_interval > max_interval ||
+ min_interval < 6 || max_interval > 3200) {
+ warn("GAS PPCP: Invalid Connection Parameters values");
+ return;
+ }
+
+ if (timeout < 10 || timeout > 3200) {
+ warn("GAS PPCP: Invalid Connection Parameters values");
+ return;
+ }
+
+ if (max_interval >= timeout * 8) {
+ warn("GAS PPCP: Invalid Connection Parameters values");
+ return;
+ }
+
+ max_latency = (timeout * 4 / max_interval) - 1;
+ if (latency > 499 || latency > max_latency) {
+ warn("GAS PPCP: Invalid Connection Parameters values");
+ return;
+ }
+
+ btd_device_set_conn_param(gas->device, min_interval, max_interval,
+ latency, timeout);
+}
+
+static void handle_ppcp(struct gas *gas, uint16_t value_handle)
+{
+ if (!bt_gatt_client_read_value(gas->client, value_handle,
+ read_ppcp_cb, gas, NULL))
+ DBG("Failed to send request to read PPCP");
+}
+
static inline bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
{
bt_uuid_t lhs;
@@ -188,6 +257,8 @@ static void handle_characteristic(struct gatt_db_attribute *attr,
handle_device_name(gas, value_handle);
else if (uuid_cmp(GATT_CHARAC_APPEARANCE, &uuid))
handle_appearance(gas, value_handle);
+ else if (uuid_cmp(GATT_CHARAC_PERIPHERAL_PREF_CONN, &uuid))
+ handle_ppcp(gas, value_handle);
else {
char uuid_str[MAX_LEN_UUID_STR];
--
2.11.1
This patch incorporates some unit-tests for the Slave Connection
Interval Range AD and also does some minor cleanups.
---
unit/test-eir.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/unit/test-eir.c b/unit/test-eir.c
index 421d0db13a41..e9d0d60fdd39 100644
--- a/unit/test-eir.c
+++ b/unit/test-eir.c
@@ -44,6 +44,7 @@ struct test_data {
bool name_complete;
int8_t tx_power;
const char **uuid;
+ uint16_t min_interval, max_interval;
};
static const unsigned char macbookair_data[] = {
@@ -508,6 +509,8 @@ static const struct test_data citizen_adv_test = {
.name = "Eco-Drive Proximity",
.name_complete = true,
.tx_power = 127,
+ .min_interval = 383,
+ .max_interval = 399,
};
static const unsigned char citizen_scan_data[] = {
@@ -565,6 +568,8 @@ static void test_parsing(gconstpointer data)
tester_debug("Flags: %d", eir.flags);
tester_debug("Name: %s", eir.name);
tester_debug("TX power: %d", eir.tx_power);
+ tester_debug("Min/Max connection interval: %u/%u",
+ eir.le_min_conn_interval, eir.le_max_conn_interval);
for (list = eir.services; list; list = list->next) {
char *uuid_str = list->data;
@@ -581,7 +586,9 @@ static void test_parsing(gconstpointer data)
g_assert(eir.name == NULL);
}
- g_assert(eir.tx_power == test->tx_power);
+ g_assert_cmpint(eir.tx_power, == ,test->tx_power);
+ g_assert_cmpuint(eir.le_min_conn_interval, ==, test->min_interval);
+ g_assert_cmpuint(eir.le_max_conn_interval, ==, test->max_interval);
if (test->uuid) {
GSList *list;
@@ -654,6 +661,18 @@ static const struct test_data uri_beacon_test = {
.uuid = uri_beacon_uuid,
};
+static const unsigned char conn_interval_data[] = {
+ 0x05, 0x12, 0x03, 0x00, 0x0f, 0x00
+};
+
+static const struct test_data conn_interval_test = {
+ .eir_data = conn_interval_data,
+ .eir_size = sizeof(conn_interval_data),
+ .tx_power = 127,
+ .min_interval = 3,
+ .max_interval = 15,
+};
+
int main(int argc, char *argv[])
{
tester_init(&argc, &argv);
@@ -677,8 +696,9 @@ int main(int argc, char *argv[])
tester_add("/ad/citizen1", &citizen_adv_test, NULL, test_parsing, NULL);
tester_add("/ad/citizen2", &citizen_scan_test, NULL, test_parsing,
NULL);
- tester_add("ad/g-tag", &gigaset_gtag_test, NULL, test_parsing, NULL);
- tester_add("ad/uri-beacon", &uri_beacon_test, NULL, test_parsing, NULL);
+ tester_add("/ad/g-tag", &gigaset_gtag_test, NULL, test_parsing, NULL);
+ tester_add("/ad/uri-beacon", &uri_beacon_test, NULL, test_parsing, NULL);
+ tester_add("/ad/conn-interval", &conn_interval_test, NULL, test_parsing, NULL);
return tester_run();
}
--
2.11.1
The Slave Connection Interval Range data type contains the Peripheral's
preferred connection interval range, for all logical connections.
---
src/eir.c | 7 +++++++
src/eir.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/src/eir.c b/src/eir.c
index c984fa5a787c..98fe44beb4b3 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -346,6 +346,13 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
eir_parse_msd(eir, data, data_len);
break;
+ case EIR_SLAVE_CONN_INT:
+ if (data_len < 4)
+ break;
+ eir->le_min_conn_interval = get_le16(&data[0]);
+ eir->le_max_conn_interval = get_le16(&data[2]);
+ break;
+
}
eir_data += field_len + 1;
diff --git a/src/eir.h b/src/eir.h
index 219ee794b8ee..51aa875b1600 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -40,6 +40,7 @@
#define EIR_SSP_HASH 0x0E /* SSP Hash */
#define EIR_SSP_RANDOMIZER 0x0F /* SSP Randomizer */
#define EIR_DEVICE_ID 0x10 /* device ID */
+#define EIR_SLAVE_CONN_INT 0x12 /* Slave Connection Interval Range */
#define EIR_SOLICIT16 0x14 /* LE: Solicit UUIDs, 16-bit */
#define EIR_SOLICIT128 0x15 /* LE: Solicit UUIDs, 128-bit */
#define EIR_SVC_DATA16 0x16 /* LE: Service data, 16-bit UUID */
@@ -90,6 +91,8 @@ struct eir_data {
uint16_t did_product;
uint16_t did_version;
uint16_t did_source;
+ uint16_t le_min_conn_interval;
+ uint16_t le_max_conn_interval;
GSList *msd_list;
GSList *sd_list;
};
--
2.11.1
It is important to store and load the connection parameters before the
connection actually take place, so the appropriate parameters are used.
---
src/adapter.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index b63883e3cae5..df4288b0ef29 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5709,6 +5709,28 @@ static void update_found_devices(struct btd_adapter *adapter,
if (bdaddr_type != BDADDR_BREDR)
device_set_flags(dev, eir_data.flags);
+ /* 0xffff indicates no specific min/max */
+ if (eir_data.le_min_conn_interval == 0xffff)
+ eir_data.le_min_conn_interval = 6;
+
+ if (eir_data.le_max_conn_interval == 0xffff)
+ eir_data.le_max_conn_interval = 3200;
+
+ if (eir_data.le_min_conn_interval <= eir_data.le_max_conn_interval &&
+ eir_data.le_min_conn_interval >= 6 &&
+ eir_data.le_max_conn_interval <= 3200) {
+ adapter_store_conn_param(adapter, bdaddr, bdaddr_type,
+ eir_data.le_min_conn_interval,
+ eir_data.le_max_conn_interval,
+ /* these are default values from the kernel*/
+ 0x0000, 0x002a);
+ adapter_load_conn_param(adapter, bdaddr, bdaddr_type,
+ eir_data.le_min_conn_interval,
+ eir_data.le_max_conn_interval,
+ /* these are default values from the kernel*/
+ 0x0000, 0x002a);
+ }
+
eir_data_free(&eir_data);
/*
--
2.11.1
Just removing unecessary function and code duplication.
---
profiles/gap/gas.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 43b7c3d0ef07..0d34503ad446 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -57,10 +57,18 @@ struct gas {
struct gatt_db_attribute *attr;
};
-static void gas_free(struct gas *gas)
+static void gas_reset(struct gas *gas)
{
+ gas->attr = NULL;
gatt_db_unref(gas->db);
+ gas->db = NULL;
bt_gatt_client_unref(gas->client);
+ gas->client = NULL;
+}
+
+static void gas_free(struct gas *gas)
+{
+ gas_reset(gas);
btd_device_unref(gas->device);
g_free(gas);
}
@@ -154,7 +162,7 @@ static void handle_appearance(struct gas *gas, uint16_t value_handle)
DBG("Failed to send request to read appearance");
}
-static bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
+static inline bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
{
bt_uuid_t lhs;
@@ -190,11 +198,6 @@ static void handle_characteristic(struct gatt_db_attribute *attr,
}
}
-static void handle_gap_service(struct gas *gas)
-{
- gatt_db_service_foreach_char(gas->attr, handle_characteristic, gas);
-}
-
static int gap_probe(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
@@ -248,16 +251,7 @@ static void foreach_gap_service(struct gatt_db_attribute *attr, void *user_data)
}
gas->attr = attr;
- handle_gap_service(gas);
-}
-
-static void gas_reset(struct gas *gas)
-{
- gas->attr = NULL;
- gatt_db_unref(gas->db);
- gas->db = NULL;
- bt_gatt_client_unref(gas->client);
- gas->client = NULL;
+ gatt_db_service_foreach_char(gas->attr, handle_characteristic, gas);
}
static int gap_accept(struct btd_service *service)
@@ -268,13 +262,15 @@ static int gap_accept(struct btd_service *service)
struct gas *gas = btd_service_get_user_data(service);
char addr[18];
bt_uuid_t gap_uuid;
+ int err = 0;
ba2str(device_get_address(device), addr);
DBG("GAP profile accept (%s)", addr);
if (!gas) {
error("GAP service not handled by profile");
- return -1;
+ err = -1;
+ goto _finish;
}
gas->db = gatt_db_ref(db);
@@ -287,12 +283,14 @@ static int gap_accept(struct btd_service *service)
if (!gas->attr) {
error("GAP attribute not found");
gas_reset(gas);
- return -1;
+ err = -1;
}
- btd_service_connecting_complete(service, 0);
+_finish:
- return 0;
+ btd_service_connecting_complete(service, err);
+
+ return err;
}
static int gap_disconnect(struct btd_service *service)
--
2.11.1
This function allows plugins to set the connection parameters of the
respective btd_device object.
It is useful for GAP Peripheral Preferred Connection Parameters
characteristic for example.
---
src/device.c | 16 ++++++++++++++++
src/device.h | 4 ++++
2 files changed, 20 insertions(+)
diff --git a/src/device.c b/src/device.c
index 8693eb826b54..d4aa7f90561e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -6122,3 +6122,19 @@ void btd_device_cleanup(void)
{
btd_service_remove_state_cb(service_state_cb_id);
}
+
+void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
+ uint16_t max_interval, uint16_t latency,
+ uint16_t timeout)
+{
+ btd_assert(device != NULL);
+
+ adapter_store_conn_param(device->adapter, &device->bdaddr,
+ device->bdaddr_type, min_interval,
+ max_interval, latency,
+ timeout);
+ adapter_load_conn_param(device->adapter, &device->bdaddr,
+ device->bdaddr_type, min_interval,
+ max_interval, latency,
+ timeout);
+}
diff --git a/src/device.h b/src/device.h
index 3cab366eeaea..88076d2d196d 100644
--- a/src/device.h
+++ b/src/device.h
@@ -163,3 +163,7 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services);
void btd_device_init(void);
void btd_device_cleanup(void);
+
+void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
+ uint16_t max_interval, uint16_t latency,
+ uint16_t timeout);
--
2.11.1
Hi Felipe,
On Wed, Mar 8, 2017 at 12:48 PM, Felipe Ferreri Tonello
<[email protected]> wrote:
> Hi Luiz,
>
> Sorry for the late reply, I was unwell this last week.
>
> On 07/03/17 09:15, Luiz Augusto von Dentz wrote:
>> Hi Felipe,
>>
>> On Tue, Feb 21, 2017 at 11:40 AM, Felipe Ferreri Tonello
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 20/02/17 15:21, Luiz Augusto von Dentz wrote:
>>>> Hi Felipe,
>>>>
>>>> On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
>>>>> This is one of the parts of a series of patches related to connection
>>>>> parameters update.
>>>>>
>>>>> This initial implementation adds supports to GAP Peripheral Preferred
>>>>> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
>>>>> Range Advertising Data (AD).
>>>>
>>>>
>>>> Are there any devices implementing these? Id like to do a some testing
>>>> but haven't find anything implementing this, or well I can generate
>>>> the PPCP but the SCRAD is probably a waste of AD since that is pretty
>>>> limited in size.
>>>
>>> PPCP not that I know of. But the AD there are devices, including in the
>>> unit-eir there is one.
>>>
>>> The PPCP and AD is important specially for MIDI devices who require
>>> low-latency.
>>>
>>> I did test it here, it will be nice if you could as well.
>>
>> Lets try to get some points resolved here, Ive briefly chatted with
>> Johan regarding this last week and he said that we might have to
>> introduce a new command to add new parameters to the kernel, as oppose
>> to reuse load.
>>
>> We might have to choose between these 2 options:
>>
>> 1. Reload all the settings, and then move on to implement the new command.
>
> Reload all settings as in bluetoothd reloading all settings?
>
> What new command?
>
>> 2. Don't bother with old kernels and just make it work with newer one
>> with proper support.
>
> New command to update the settings for one device only?
Yep, the reason for that is that changing the existing logic of load
may break the API for older kernels as it would no longer remove
existing settings but just update.
>>
>>>>
>>>>> There is a problem described in #4 that due to a limitation of the BlueZ API
>>>>> it is not possible to update the current connection with specific parameters.
>>>>> I am working on a socket option API to allow something similar. Hopefully I
>>>>> will be able to write something that covers this use-case as well. This
>>>>> problem also applies to patch #6.
>>>>>
>>>>> Changes from v1:
>>>>> * Added Slave Connection Interval Range AD support;
>>>>> * Unit tests for connection interval.
>>>>>
>>>>> Felipe F. Tonello (7):
>>>>> profiles/gap: Some code cleanup
>>>>> src/adapter: Added connection parameter load/store functions
>>>>> src/device: Added function to set connection parameters
>>>>> profiles/gap: Added support for PPCP characteristic
>>>>> src/eir: Added support for Slave Connection Interval Range AD
>>>>> src/adapter: Store Slave Connection Interval Range
>>>>> unit/test-eir: Added tests for connection interval
>>>>>
>>>>> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
>>>>> src/adapter.c | 46 +++++++++++++++++++++-
>>>>> src/adapter.h | 9 +++++
>>>>> src/device.c | 16 ++++++++
>>>>> src/device.h | 4 ++
>>>>> src/eir.c | 7 ++++
>>>>> src/eir.h | 3 ++
>>>>> unit/test-eir.c | 26 +++++++++++--
>>>>> 8 files changed, 196 insertions(+), 26 deletions(-)
>>>>>
>>>>> --
>>>>> 2.11.1
>>>>>
>
> --
> Felipe
--
Luiz Augusto von Dentz
Hi Luiz,
On 08/03/17 11:19, Luiz Augusto von Dentz wrote:
> Hi Felipe,
>
> On Wed, Mar 8, 2017 at 12:48 PM, Felipe Ferreri Tonello
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> Sorry for the late reply, I was unwell this last week.
>>
>> On 07/03/17 09:15, Luiz Augusto von Dentz wrote:
>>> Hi Felipe,
>>>
>>> On Tue, Feb 21, 2017 at 11:40 AM, Felipe Ferreri Tonello
>>> <[email protected]> wrote:
>>>> Hi Luiz,
>>>>
>>>> On 20/02/17 15:21, Luiz Augusto von Dentz wrote:
>>>>> Hi Felipe,
>>>>>
>>>>> On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
>>>>>> This is one of the parts of a series of patches related to connection
>>>>>> parameters update.
>>>>>>
>>>>>> This initial implementation adds supports to GAP Peripheral Preferred
>>>>>> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
>>>>>> Range Advertising Data (AD).
>>>>>
>>>>>
>>>>> Are there any devices implementing these? Id like to do a some testing
>>>>> but haven't find anything implementing this, or well I can generate
>>>>> the PPCP but the SCRAD is probably a waste of AD since that is pretty
>>>>> limited in size.
>>>>
>>>> PPCP not that I know of. But the AD there are devices, including in the
>>>> unit-eir there is one.
>>>>
>>>> The PPCP and AD is important specially for MIDI devices who require
>>>> low-latency.
>>>>
>>>> I did test it here, it will be nice if you could as well.
>>>
>>> Lets try to get some points resolved here, Ive briefly chatted with
>>> Johan regarding this last week and he said that we might have to
>>> introduce a new command to add new parameters to the kernel, as oppose
>>> to reuse load.
>>>
>>> We might have to choose between these 2 options:
>>>
>>> 1. Reload all the settings, and then move on to implement the new command.
>>
>> Reload all settings as in bluetoothd reloading all settings?
>>
>> What new command?
>>
>>> 2. Don't bother with old kernels and just make it work with newer one
>>> with proper support.
>>
>> New command to update the settings for one device only?
>
> Yep, the reason for that is that changing the existing logic of load
> may break the API for older kernels as it would no longer remove
> existing settings but just update.
Right. I think it is better too.
Could you please, then, take a look at my kernel RFC patch[1]? I'll use
that patch-set to create this new MGMT command.
[1] The email subject is "[RFC][PATCH v2 BlueZ 0/2] Connection Update
improvements​".
>
>>>
>>>>>
>>>>>> There is a problem described in #4 that due to a limitation of the BlueZ API
>>>>>> it is not possible to update the current connection with specific parameters.
>>>>>> I am working on a socket option API to allow something similar. Hopefully I
>>>>>> will be able to write something that covers this use-case as well. This
>>>>>> problem also applies to patch #6.
>>>>>>
>>>>>> Changes from v1:
>>>>>> * Added Slave Connection Interval Range AD support;
>>>>>> * Unit tests for connection interval.
>>>>>>
>>>>>> Felipe F. Tonello (7):
>>>>>> profiles/gap: Some code cleanup
>>>>>> src/adapter: Added connection parameter load/store functions
>>>>>> src/device: Added function to set connection parameters
>>>>>> profiles/gap: Added support for PPCP characteristic
>>>>>> src/eir: Added support for Slave Connection Interval Range AD
>>>>>> src/adapter: Store Slave Connection Interval Range
>>>>>> unit/test-eir: Added tests for connection interval
>>>>>>
>>>>>> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
>>>>>> src/adapter.c | 46 +++++++++++++++++++++-
>>>>>> src/adapter.h | 9 +++++
>>>>>> src/device.c | 16 ++++++++
>>>>>> src/device.h | 4 ++
>>>>>> src/eir.c | 7 ++++
>>>>>> src/eir.h | 3 ++
>>>>>> unit/test-eir.c | 26 +++++++++++--
>>>>>> 8 files changed, 196 insertions(+), 26 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 2.11.1
>>>>>>
--
Felipe
Hi Luiz,
Sorry for the late reply, I was unwell this last week.
On 07/03/17 09:15, Luiz Augusto von Dentz wrote:
> Hi Felipe,
>
> On Tue, Feb 21, 2017 at 11:40 AM, Felipe Ferreri Tonello
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 20/02/17 15:21, Luiz Augusto von Dentz wrote:
>>> Hi Felipe,
>>>
>>> On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
>>>> This is one of the parts of a series of patches related to connection
>>>> parameters update.
>>>>
>>>> This initial implementation adds supports to GAP Peripheral Preferred
>>>> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
>>>> Range Advertising Data (AD).
>>>
>>>
>>> Are there any devices implementing these? Id like to do a some testing
>>> but haven't find anything implementing this, or well I can generate
>>> the PPCP but the SCRAD is probably a waste of AD since that is pretty
>>> limited in size.
>>
>> PPCP not that I know of. But the AD there are devices, including in the
>> unit-eir there is one.
>>
>> The PPCP and AD is important specially for MIDI devices who require
>> low-latency.
>>
>> I did test it here, it will be nice if you could as well.
>
> Lets try to get some points resolved here, Ive briefly chatted with
> Johan regarding this last week and he said that we might have to
> introduce a new command to add new parameters to the kernel, as oppose
> to reuse load.
>
> We might have to choose between these 2 options:
>
> 1. Reload all the settings, and then move on to implement the new command.
Reload all settings as in bluetoothd reloading all settings?
What new command?
> 2. Don't bother with old kernels and just make it work with newer one
> with proper support.
New command to update the settings for one device only?
>
>>>
>>>> There is a problem described in #4 that due to a limitation of the BlueZ API
>>>> it is not possible to update the current connection with specific parameters.
>>>> I am working on a socket option API to allow something similar. Hopefully I
>>>> will be able to write something that covers this use-case as well. This
>>>> problem also applies to patch #6.
>>>>
>>>> Changes from v1:
>>>> * Added Slave Connection Interval Range AD support;
>>>> * Unit tests for connection interval.
>>>>
>>>> Felipe F. Tonello (7):
>>>> profiles/gap: Some code cleanup
>>>> src/adapter: Added connection parameter load/store functions
>>>> src/device: Added function to set connection parameters
>>>> profiles/gap: Added support for PPCP characteristic
>>>> src/eir: Added support for Slave Connection Interval Range AD
>>>> src/adapter: Store Slave Connection Interval Range
>>>> unit/test-eir: Added tests for connection interval
>>>>
>>>> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
>>>> src/adapter.c | 46 +++++++++++++++++++++-
>>>> src/adapter.h | 9 +++++
>>>> src/device.c | 16 ++++++++
>>>> src/device.h | 4 ++
>>>> src/eir.c | 7 ++++
>>>> src/eir.h | 3 ++
>>>> unit/test-eir.c | 26 +++++++++++--
>>>> 8 files changed, 196 insertions(+), 26 deletions(-)
>>>>
>>>> --
>>>> 2.11.1
>>>>
--
Felipe
Hi Felipe,
On Tue, Feb 21, 2017 at 11:40 AM, Felipe Ferreri Tonello
<[email protected]> wrote:
> Hi Luiz,
>
> On 20/02/17 15:21, Luiz Augusto von Dentz wrote:
>> Hi Felipe,
>>
>> On Thu, Feb 16, 2017 at 8:03 PM, Felipe F. Tonello <[email protected]> wrote:
>>> This is one of the parts of a series of patches related to connection
>>> parameters update.
>>>
>>> This initial implementation adds supports to GAP Peripheral Preferred
>>> Connection Parameters (PPCP) characteristic and the Slave Connection Interval
>>> Range Advertising Data (AD).
>>
>>
>> Are there any devices implementing these? Id like to do a some testing
>> but haven't find anything implementing this, or well I can generate
>> the PPCP but the SCRAD is probably a waste of AD since that is pretty
>> limited in size.
>
> PPCP not that I know of. But the AD there are devices, including in the
> unit-eir there is one.
>
> The PPCP and AD is important specially for MIDI devices who require
> low-latency.
>
> I did test it here, it will be nice if you could as well.
Lets try to get some points resolved here, Ive briefly chatted with
Johan regarding this last week and he said that we might have to
introduce a new command to add new parameters to the kernel, as oppose
to reuse load.
We might have to choose between these 2 options:
1. Reload all the settings, and then move on to implement the new command.
2. Don't bother with old kernels and just make it work with newer one
with proper support.
>>
>>> There is a problem described in #4 that due to a limitation of the BlueZ API
>>> it is not possible to update the current connection with specific parameters.
>>> I am working on a socket option API to allow something similar. Hopefully I
>>> will be able to write something that covers this use-case as well. This
>>> problem also applies to patch #6.
>>>
>>> Changes from v1:
>>> * Added Slave Connection Interval Range AD support;
>>> * Unit tests for connection interval.
>>>
>>> Felipe F. Tonello (7):
>>> profiles/gap: Some code cleanup
>>> src/adapter: Added connection parameter load/store functions
>>> src/device: Added function to set connection parameters
>>> profiles/gap: Added support for PPCP characteristic
>>> src/eir: Added support for Slave Connection Interval Range AD
>>> src/adapter: Store Slave Connection Interval Range
>>> unit/test-eir: Added tests for connection interval
>>>
>>> profiles/gap/gas.c | 111 +++++++++++++++++++++++++++++++++++++++++++----------
>>> src/adapter.c | 46 +++++++++++++++++++++-
>>> src/adapter.h | 9 +++++
>>> src/device.c | 16 ++++++++
>>> src/device.h | 4 ++
>>> src/eir.c | 7 ++++
>>> src/eir.h | 3 ++
>>> unit/test-eir.c | 26 +++++++++++--
>>> 8 files changed, 196 insertions(+), 26 deletions(-)
>>>
>>> --
>>> 2.11.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>
> --
> Felipe
--
Luiz Augusto von Dentz