[PATCH 1/2] Cleanup functions that update found UUIDs(EIR)

[PATCH 1/2] Cleanup functions that update found UUIDs(EIR)

[Claudio Takahasi]

---
 src/adapter.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 8dbd62c..f7bbc3e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3059,11 +3059,11 @@ static struct remote_dev_info *get_found_dev(struct btd_adapter *adapter,
 static void remove_same_uuid(gpointer data, gpointer user_data)
 {
 	struct remote_dev_info *dev = user_data;
+	const char *new_uuid = data;
 	GSList *l;
 
 	for (l = dev->services; l; l = l->next) {
 		char *current_uuid = l->data;
-		char *new_uuid = data;
 
 		if (strcmp(current_uuid, new_uuid) == 0) {
 			g_free(current_uuid);
@@ -3076,7 +3076,7 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
 static void dev_prepend_uuid(gpointer data, gpointer user_data)
 {
 	struct remote_dev_info *dev = user_data;
-	char *new_uuid = data;
+	const char *new_uuid = data;
 
 	dev->services = g_slist_prepend(dev->services, g_strdup(new_uuid));
 }
-- 
1.7.5.rc3

[PATCH 2/2] Remove wrong checking for legacy devices

[Claudio Takahasi]

Infer that the found device is a legacy device based on the presence
of its name in the storage is wrong.
---
 src/event.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/event.c b/src/event.c
index 5373e33..b873000 100644
--- a/src/event.c
+++ b/src/event.c
@@ -481,8 +481,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 
 	if (data)
 		legacy = FALSE;
-	else if (name == NULL)
-		legacy = TRUE;
 	else if (read_remote_features(local, peer, NULL, features) == 0) {
 		if (features[0] & 0x01)
 			legacy = FALSE;
-- 
1.7.5.rc3

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Luiz Augusto von Dentz]

Hi Claudio,

On Fri, Apr 29, 2011 at 1:34 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> Infer that the found device is a legacy device based on the presence
> of its name in the storage is wrong.
> ---
>  src/event.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/src/event.c b/src/event.c
> index 5373e33..b873000 100644
> --- a/src/event.c
> +++ b/src/event.c
> @@ -481,8 +481,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
>
>        if (data)
>                legacy = FALSE;
> -       else if (name == NULL)
> -               legacy = TRUE;
>        else if (read_remote_features(local, peer, NULL, features) == 0) {
>                if (features[0] & 0x01)
>                        legacy = FALSE;
> --

That was some use of it, but I can't remember now, maybe some broken
stack which has feature bits broken or we do actually infer if we ever
connect to the device before trying to read its features from the
storage.


-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Claudio Takahasi]

Hi Luiz,

On Fri, Apr 29, 2011 at 8:27 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Claudio,
>
> On Fri, Apr 29, 2011 at 1:34 AM, Claudio Takahasi
> <claudio.takahasi@openbossa.org> wrote:
>> Infer that the found device is a legacy device based on the presence
>> of its name in the storage is wrong.
>> ---
>>  src/event.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/event.c b/src/event.c
>> index 5373e33..b873000 100644
>> --- a/src/event.c
>> +++ b/src/event.c
>> @@ -481,8 +481,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
>>
>>        if (data)
>>                legacy = FALSE;
>> -       else if (name == NULL)
>> -               legacy = TRUE;
>>        else if (read_remote_features(local, peer, NULL, features) == 0) {
>>                if (features[0] & 0x01)
>>                        legacy = FALSE;
>> --
>
> That was some use of it, but I can't remember now, maybe some broken
> stack which has feature bits broken or we do actually infer if we ever
> connect to the device before trying to read its features from the
> storage.
>
>
> --
> Luiz Augusto von Dentz
> Computer Engineer
>

I found the commit that introduced this code, but it is still not clear to me:
989c60c0b9c96edf1fbdf80356abf05bac336673

BTW: why this information needs to be exposed through the DeviceFound() signal?
It is not used internally in the adapter.c source.

BR,
Claudio

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Johan Hedberg]

Hi Claudio,

On Fri, Apr 29, 2011, Claudio Takahasi wrote:
> > On Fri, Apr 29, 2011 at 1:34 AM, Claudio Takahasi
> > <claudio.takahasi@openbossa.org> wrote:
> >> Infer that the found device is a legacy device based on the presence
> >> of its name in the storage is wrong.
> >> ---
> >>  src/event.c |    2 --
> >>  1 files changed, 0 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/event.c b/src/event.c
> >> index 5373e33..b873000 100644
> >> --- a/src/event.c
> >> +++ b/src/event.c
> >> @@ -481,8 +481,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
> >>
> >>        if (data)
> >>                legacy = FALSE;
> >> -       else if (name == NULL)
> >> -               legacy = TRUE;
> >>        else if (read_remote_features(local, peer, NULL, features) == 0) {
> >>                if (features[0] & 0x01)
> >>                        legacy = FALSE;
> >> --
> >
> > That was some use of it, but I can't remember now, maybe some broken
> > stack which has feature bits broken or we do actually infer if we ever
> > connect to the device before trying to read its features from the
> > storage.
> >
> >
> > --
> > Luiz Augusto von Dentz
> > Computer Engineer
> >
> 
> I found the commit that introduced this code, but it is still not clear to me:
> 989c60c0b9c96edf1fbdf80356abf05bac336673

I think the logic has gone like this: we get the remote features as a
side effect of a successful remote name request, so by checking for the
name first we don't do an unnecessary file-system lookup.

> BTW: why this information needs to be exposed through the DeviceFound() signal?
> It is not used internally in the adapter.c source.

It's useful for pairing UIs. If a device is expected to produce legacy
pairing when calling CreatePairedDevice the UI can ask the user for the
PIN *before* calling CreatePairedDevice and thereby eliminate the risk of
user response timeout locally.

Johan

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Anderson Lizardo]

Hi Johan,

On Fri, Apr 29, 2011 at 1:56 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> BTW: why this information needs to be exposed through the DeviceFound() signal?
>> It is not used internally in the adapter.c source.
>
> It's useful for pairing UIs. If a device is expected to produce legacy
> pairing when calling CreatePairedDevice the UI can ask the user for the
> PIN *before* calling CreatePairedDevice and thereby eliminate the risk of
> user response timeout locally.

Maybe this is useful do be documented on doc/device-api.txt? The
LegacyPairing only mentions "This property is useful in the
Adapter.DeviceFound signal to anticipate whether legacy or simple
pairing will occur.", but it is not mentioned *why* the API user might
want to detect legacy pairing devices.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Anderson Lizardo]

On Fri, Apr 29, 2011 at 2:41 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Johan,
>
> On Fri, Apr 29, 2011 at 1:56 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>> BTW: why this information needs to be exposed through the DeviceFound() signal?
>>> It is not used internally in the adapter.c source.
>>
>> It's useful for pairing UIs. If a device is expected to produce legacy
>> pairing when calling CreatePairedDevice the UI can ask the user for the
>> PIN *before* calling CreatePairedDevice and thereby eliminate the risk of
>> user response timeout locally.
>
> Maybe this is useful do be documented on doc/device-api.txt? The
> LegacyPairing only mentions "This property is useful in the
> Adapter.DeviceFound signal to anticipate whether legacy or simple
> pairing will occur.", but it is not mentioned *why* the API user might
> want to detect legacy pairing devices.

On another story, I'm not sure about BR/EDR, but for LE there is a
timeout associated with SMP pairing. Would this "anticipate user PIN
request" thing be necessary on this case as well?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 2/2] Remove wrong checking for legacy devices

[Claudio Takahasi]

Hi Johan,

So please ignore this patch.

On Fri, Apr 29, 2011 at 2:56 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Claudio,
>
> On Fri, Apr 29, 2011, Claudio Takahasi wrote:
>> > On Fri, Apr 29, 2011 at 1:34 AM, Claudio Takahasi
>> > <claudio.takahasi@openbossa.org> wrote:
>> >> Infer that the found device is a legacy device based on the presence
>> >> of its name in the storage is wrong.
>> >> ---
>> >>  src/event.c |    2 --
>> >>  1 files changed, 0 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/src/event.c b/src/event.c
>> >> index 5373e33..b873000 100644
>> >> --- a/src/event.c
>> >> +++ b/src/event.c
>> >> @@ -481,8 +481,6 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
>> >>
>> >>        if (data)
>> >>                legacy = FALSE;
>> >> -       else if (name == NULL)
>> >> -               legacy = TRUE;
>> >>        else if (read_remote_features(local, peer, NULL, features) == 0) {
>> >>                if (features[0] & 0x01)
>> >>                        legacy = FALSE;
>> >> --
>> >
>> > That was some use of it, but I can't remember now, maybe some broken
>> > stack which has feature bits broken or we do actually infer if we ever
>> > connect to the device before trying to read its features from the
>> > storage.
>> >
>> >
>> > --
>> > Luiz Augusto von Dentz
>> > Computer Engineer
>> >
>>
>> I found the commit that introduced this code, but it is still not clear to me:
>> 989c60c0b9c96edf1fbdf80356abf05bac336673
>
> I think the logic has gone like this: we get the remote features as a
> side effect of a successful remote name request, so by checking for the
> name first we don't do an unnecessary file-system lookup.

I got the idea, I will keep the same approach in the discovery cleanup patches.

So, if I understood correctly, in the case that it is the first the
device is seen(name is unknown) and EIR data is not sent the
legacy=TRUE will be emitted.
In the first connection, the remote features will be retrieved
"automatically" by the kernel and user space will set "legacy"
variable based on the features bits. The updated legacy value will be
emitted only in the next discovery session.

BR,
Claudio

>
>> BTW: why this information needs to be exposed through the DeviceFound() signal?
>> It is not used internally in the adapter.c source.
>
> It's useful for pairing UIs. If a device is expected to produce legacy
> pairing when calling CreatePairedDevice the UI can ask the user for the
> PIN *before* calling CreatePairedDevice and thereby eliminate the risk of
> user response timeout locally.
>
> Johan
>