2014-05-06 23:58:10

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH] mgmt-api: Add support to get connection characteristics

This patch introduces Get Connection Characteristics command.
---
doc/mgmt-api.txt | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index cd5cd24..5d31a90 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -1679,6 +1679,42 @@ Load Identity Resolving Keys Command
Invalid Index


+Get Connection Characteristics Command
+======================================
+
+ Command Code: 0x0031
+ Controller Index: <controller id>
+ Command Parameters: Address (6 Octets)
+ Address_Type (1 Octet)
+ Requested_Characteristics (4 Octet)
+
+ Return Parameters: Address (6 Octets)
+ Address_Type (1 Octet)
+ RSSI (1 Octet)
+ EIR_Data_Length (2 Octets)
+ EIR_Data (0-65535 Octets)
+
+ This command is used to get connection characteristics information.
+
+ Requested_Characteristics parameter is a bitmask with currently the
+ following available bits:
+
+ 0 RSSI
+ 1 TX Power
+
+ All the requested characteristics other than RSSI are returned in
+ EIR_Data.
+
+ This command generates a Command Complete event on success and
+ on failure.
+
+ Possible errors: Busy
+ Not Connected
+ Not Powered
+ Invalid Parameters
+ Invalid Index
+
+
Command Complete Event
======================

--
1.8.4


2014-05-07 07:48:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] mgmt-api: Add support to get connection characteristics

Hi Andrzej,

>>> This patch introduces Get Connection Characteristics command.
>>> ---
>>> doc/mgmt-api.txt | 36 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 36 insertions(+)
>>>
>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>> index cd5cd24..5d31a90 100644
>>> --- a/doc/mgmt-api.txt
>>> +++ b/doc/mgmt-api.txt
>>> @@ -1679,6 +1679,42 @@ Load Identity Resolving Keys Command
>>> Invalid Index
>>>
>>>
>>> +Get Connection Characteristics Command
>>> +======================================
>>
>> we might need to discuss if we better call it ?Get Connection Information?. I wait for other people to comment here.
>
> I like it, should be also nicer to put in code - 'characteristics' is
> horrible to combine into variable names and abbreviate ;-)
>
>>> +
>>> + Command Code: 0x0031
>>> + Controller Index: <controller id>
>>> + Command Parameters: Address (6 Octets)
>>> + Address_Type (1 Octet)
>>> + Requested_Characteristics (4 Octet)
>>
>> Maybe just calling this Data_Types is better. Not that it matters much, but the field name Requested_Characteristics sound a bit too much.
>>
>>> +
>>
>> Remove this extra empty line.
>>
>>> + Return Parameters: Address (6 Octets)
>>> + Address_Type (1 Octet)
>>> + RSSI (1 Octet)
>>
>> Add the Flags (4 Octets) value here. We want to be flexible and being able to indicate certain flags might come in handy at some point.
>>
>>> + EIR_Data_Length (2 Octets)
>>> + EIR_Data (0-65535 Octets)
>>> +
>>> + This command is used to get connection characteristics information.
>>> +
>>> + Requested_Characteristics parameter is a bitmask with currently the
>>> + following available bits:
>>> +
>>> + 0 RSSI
>>> + 1 TX Power
>>
>> Start with 0 = TX Power here. There is pretty much no point for RSSI since that will always be returned.
>
> There will always be RSSI parameter in response but it does not need
> to be valid (perhaps we can use Flags for this) unless we query for
> RSSI on every request from user-space or poll for RSSI in background,
> but I don't think this is what we really want here.

if it is invalid we can return -127 or whatever value is defined for invalid.

> As I understand we want to minimize requests to controller so we do
> something like: have timestamps for most recent RSSI and TX power
> received and once information is requested, check which one can be
> answered from cache and query controller only for missing one. This
> implies that both values don't need to be from exactly the same point
> of time but they both are no-older-than some interval. Does this make
> sense?

We can have a single timestamp here. No need to be super smart. At least not in the initial implementation.

The one thing I really want is that we do not bring the controller out of sniff mode for this. The host will be woken up anyway. So it is not a big deal if we request both values.

>> Also we need to note that all other bits are reserved and shall be set to zero. Unknown bits will be ignored by the kernel on reception.
>>
>>> + All the requested characteristics other than RSSI are returned in
>>> + EIR_Data.
>>> +
>>> + This command generates a Command Complete event on success and
>>> + on failure.
>>> +
>>> + Possible errors: Busy
>>> + Not Connected
>>> + Not Powered
>>> + Invalid Parameters
>>> + Invalid Index
>>
>> How can this return Busy? As I said before, we need to just have an internal configured via debugfs in which this command will return the cached values. So even if you call this twice in a row or at the same time, we should just return quickly.
>
> In case requested values cannot be answered from cache we need to
> query for them and when this is in ongoing we'll request Busy for
> other requests. Or we can just queue requests and answer them when
> response arrive so we don't need to return Busy.

Actually I am more thinking that if we already have an ongoing request from one client, just let the second one wait and return responses to both requests at the same time once the initial one finishes. It is an async API afterall.

The one thing I do not want, hey, busy, lets poll again, busy again, lets poll again. Implementing a back off algorithm in the client seems like a pretty stupid idea.

Regards

Marcel


2014-05-07 07:08:18

by Andrzej Kaczmarek

[permalink] [raw]
Subject: Re: [PATCH] mgmt-api: Add support to get connection characteristics

Hi Marcel,

On 7 May 2014 03:59, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
>> This patch introduces Get Connection Characteristics command.
>> ---
>> doc/mgmt-api.txt | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> index cd5cd24..5d31a90 100644
>> --- a/doc/mgmt-api.txt
>> +++ b/doc/mgmt-api.txt
>> @@ -1679,6 +1679,42 @@ Load Identity Resolving Keys Command
>> Invalid Index
>>
>>
>> +Get Connection Characteristics Command
>> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>
> we might need to discuss if we better call it =E2=80=9CGet Connection Inf=
ormation=E2=80=9D. I wait for other people to comment here.

I like it, should be also nicer to put in code - 'characteristics' is
horrible to combine into variable names and abbreviate ;-)

>> +
>> + Command Code: 0x0031
>> + Controller Index: <controller id>
>> + Command Parameters: Address (6 Octets)
>> + Address_Type (1 Octet)
>> + Requested_Characteristics (4 Octet)
>
> Maybe just calling this Data_Types is better. Not that it matters much, b=
ut the field name Requested_Characteristics sound a bit too much.
>
>> +
>
> Remove this extra empty line.
>
>> + Return Parameters: Address (6 Octets)
>> + Address_Type (1 Octet)
>> + RSSI (1 Octet)
>
> Add the Flags (4 Octets) value here. We want to be flexible and being abl=
e to indicate certain flags might come in handy at some point.
>
>> + EIR_Data_Length (2 Octets)
>> + EIR_Data (0-65535 Octets)
>> +
>> + This command is used to get connection characteristics information=
.
>> +
>> + Requested_Characteristics parameter is a bitmask with currently th=
e
>> + following available bits:
>> +
>> + 0 RSSI
>> + 1 TX Power
>
> Start with 0 =3D TX Power here. There is pretty much no point for RSSI si=
nce that will always be returned.

There will always be RSSI parameter in response but it does not need
to be valid (perhaps we can use Flags for this) unless we query for
RSSI on every request from user-space or poll for RSSI in background,
but I don't think this is what we really want here.

As I understand we want to minimize requests to controller so we do
something like: have timestamps for most recent RSSI and TX power
received and once information is requested, check which one can be
answered from cache and query controller only for missing one. This
implies that both values don't need to be from exactly the same point
of time but they both are no-older-than some interval. Does this make
sense?

> Also we need to note that all other bits are reserved and shall be set to=
zero. Unknown bits will be ignored by the kernel on reception.
>
>> + All the requested characteristics other than RSSI are returned in
>> + EIR_Data.
>> +
>> + This command generates a Command Complete event on success and
>> + on failure.
>> +
>> + Possible errors: Busy
>> + Not Connected
>> + Not Powered
>> + Invalid Parameters
>> + Invalid Index
>
> How can this return Busy? As I said before, we need to just have an inter=
nal configured via debugfs in which this command will return the cached val=
ues. So even if you call this twice in a row or at the same time, we should=
just return quickly.

In case requested values cannot be answered from cache we need to
query for them and when this is in ongoing we'll request Busy for
other requests. Or we can just queue requests and answer them when
response arrive so we don't need to return Busy.


BR,
Andrzej

2014-05-07 01:59:07

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] mgmt-api: Add support to get connection characteristics

Hi Lukasz,

> This patch introduces Get Connection Characteristics command.
> ---
> doc/mgmt-api.txt | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index cd5cd24..5d31a90 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -1679,6 +1679,42 @@ Load Identity Resolving Keys Command
> Invalid Index
>
>
> +Get Connection Characteristics Command
> +======================================

we might need to discuss if we better call it ?Get Connection Information?. I wait for other people to comment here.

> +
> + Command Code: 0x0031
> + Controller Index: <controller id>
> + Command Parameters: Address (6 Octets)
> + Address_Type (1 Octet)
> + Requested_Characteristics (4 Octet)

Maybe just calling this Data_Types is better. Not that it matters much, but the field name Requested_Characteristics sound a bit too much.

> +

Remove this extra empty line.

> + Return Parameters: Address (6 Octets)
> + Address_Type (1 Octet)
> + RSSI (1 Octet)

Add the Flags (4 Octets) value here. We want to be flexible and being able to indicate certain flags might come in handy at some point.

> + EIR_Data_Length (2 Octets)
> + EIR_Data (0-65535 Octets)
> +
> + This command is used to get connection characteristics information.
> +
> + Requested_Characteristics parameter is a bitmask with currently the
> + following available bits:
> +
> + 0 RSSI
> + 1 TX Power

Start with 0 = TX Power here. There is pretty much no point for RSSI since that will always be returned.

Also we need to note that all other bits are reserved and shall be set to zero. Unknown bits will be ignored by the kernel on reception.

> + All the requested characteristics other than RSSI are returned in
> + EIR_Data.
> +
> + This command generates a Command Complete event on success and
> + on failure.
> +
> + Possible errors: Busy
> + Not Connected
> + Not Powered
> + Invalid Parameters
> + Invalid Index

How can this return Busy? As I said before, we need to just have an internal configured via debugfs in which this command will return the cached values. So even if you call this twice in a row or at the same time, we should just return quickly.

Regards

Marcel