[PATCH] Don't set LE flags in mgmt_start_discovery()

[PATCH] Don't set LE flags in mgmt_start_discovery()

[Andre Guedes]

Since LE based discovery procedures are not yet supported, we should
not set the LE flags in mgmt_start_discovery(). Otherwise, we might
get an error from management interface.
---
 plugins/mgmtops.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
index b9e9ad6..d28f89c 100644
--- a/plugins/mgmtops.c
+++ b/plugins/mgmtops.c
@@ -1689,8 +1689,6 @@ static int mgmt_start_discovery(int index)
 	hdr->index = htobs(index);
 
 	hci_set_bit(MGMT_ADDR_BREDR, &cp->type);
-	hci_set_bit(MGMT_ADDR_LE_PUBLIC, &cp->type);
-	hci_set_bit(MGMT_ADDR_LE_RANDOM, &cp->type);
 
 	if (write(mgmt_sock, buf, sizeof(buf)) < 0)
 		return -errno;
-- 
1.7.7.1

Re: [PATCH] Don't set LE flags in mgmt_start_discovery()

[Johan Hedberg]

Hi Andre,

On Tue, Nov 22, 2011, Andre Guedes wrote:
> Since LE based discovery procedures are not yet supported, we should
> not set the LE flags in mgmt_start_discovery(). Otherwise, we might
> get an error from management interface.
> ---
>  plugins/mgmtops.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
> index b9e9ad6..d28f89c 100644
> --- a/plugins/mgmtops.c
> +++ b/plugins/mgmtops.c
> @@ -1689,8 +1689,6 @@ static int mgmt_start_discovery(int index)
>  	hdr->index = htobs(index);
>  
>  	hci_set_bit(MGMT_ADDR_BREDR, &cp->type);
> -	hci_set_bit(MGMT_ADDR_LE_PUBLIC, &cp->type);
> -	hci_set_bit(MGMT_ADDR_LE_RANDOM, &cp->type);
>  
>  	if (write(mgmt_sock, buf, sizeof(buf)) < 0)
>  		return -errno;

Since setting these bits is what user space needs to do anyway in the
long run I don't thing it's worth to remove them temporarily. Instead
I'd just make the kernel modifications initially so that an error is
only triggered of MGMT_ADDR_BREDR isn't set and in other cases just do
normal BR/EDR discovery (and ignore the LE bits).

Johan

Re: [PATCH] Don't set LE flags in mgmt_start_discovery()

[Andre Guedes]

Hi Johan,

On Nov 22, 2011, at 6:58 PM, Johan Hedberg wrote:

> Hi Andre,
> 
> On Tue, Nov 22, 2011, Andre Guedes wrote:
>> Since LE based discovery procedures are not yet supported, we should
>> not set the LE flags in mgmt_start_discovery(). Otherwise, we might
>> get an error from management interface.
>> ---
>> plugins/mgmtops.c |    2 --
>> 1 files changed, 0 insertions(+), 2 deletions(-)
>> 
>> diff --git a/plugins/mgmtops.c b/plugins/mgmtops.c
>> index b9e9ad6..d28f89c 100644
>> --- a/plugins/mgmtops.c
>> +++ b/plugins/mgmtops.c
>> @@ -1689,8 +1689,6 @@ static int mgmt_start_discovery(int index)
>> 	hdr->index = htobs(index);
>> 
>> 	hci_set_bit(MGMT_ADDR_BREDR, &cp->type);
>> -	hci_set_bit(MGMT_ADDR_LE_PUBLIC, &cp->type);
>> -	hci_set_bit(MGMT_ADDR_LE_RANDOM, &cp->type);
>> 
>> 	if (write(mgmt_sock, buf, sizeof(buf)) < 0)
>> 		return -errno;
> 
> Since setting these bits is what user space needs to do anyway in the
> long run I don't thing it's worth to remove them temporarily. Instead
> I'd just make the kernel modifications initially so that an error is
> only triggered of MGMT_ADDR_BREDR isn't set and in other cases just do
> normal BR/EDR discovery (and ignore the LE bits).

In that case, let's just ignore the kernel patch "Consider the type
param in start_discovery" and keep ignoring the type parameter.

I think I do not quite understand this change in start discovery
command. How should the command work now?

For instance, let's say we have the userspace always setting those bits
(BR/EDR and LE). If the hardware doesn't support LE should the kernel
return error? Or do we expect it does a kind of "best effort" and runs
the inquiry only?

BR,

Andre

Re: [PATCH] Don't set LE flags in mgmt_start_discovery()

[Johan Hedberg]

Hi Andre,

On Thu, Nov 24, 2011, Andre Guedes wrote:
> I think I do not quite understand this change in start discovery
> command. How should the command work now?
> 
> For instance, let's say we have the userspace always setting those bits
> (BR/EDR and LE). If the hardware doesn't support LE should the kernel
> return error?

No.

> Or do we expect it does a kind of "best effort" and runs the inquiry
> only?

Yes, I think that would make most sense from user-space perspective
"find all devices you can". As for BR/EDR specific discovery (with no LE
bits) or LE specific discovery (BR/EDR bit not set) if the hardware
doesn't support the request then in this case I think it would in fact
make sense to return an error.

Johan

Re: [PATCH] Don't set LE flags in mgmt_start_discovery()

[Anderson Lizardo]

Hi,

On Thu, Nov 24, 2011 at 2:22 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Or do we expect it does a kind of "best effort" and runs the inquiry
>> only?
>
> Yes, I think that would make most sense from user-space perspective
> "find all devices you can". As for BR/EDR specific discovery (with no LE
> bits) or LE specific discovery (BR/EDR bit not set) if the hardware
> doesn't support the request then in this case I think it would in fact
> make sense to return an error.

So in summary, it should fail only if "requested_mask & supported_mask" is 0.

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

Re: [PATCH] Don't set LE flags in mgmt_start_discovery()

[Andre Guedes]

Hi Johan/Lizardo,

On Nov 24, 2011, at 3:29 PM, Anderson Lizardo wrote:

> Hi,
> 
> On Thu, Nov 24, 2011 at 2:22 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>>> Or do we expect it does a kind of "best effort" and runs the inquiry
>>> only?
>> 
>> Yes, I think that would make most sense from user-space perspective
>> "find all devices you can". As for BR/EDR specific discovery (with no LE
>> bits) or LE specific discovery (BR/EDR bit not set) if the hardware
>> doesn't support the request then in this case I think it would in fact
>> make sense to return an error.
> 
> So in summary, it should fail only if "requested_mask & supported_mask" is 0.

To keep things simpler, IMO we should have some sort of this:

DISCOV_AUTO:     The kernel choose the discovery procedure based on the
                 hardware capabilities.
DISCOV_BREDR:    The kernel runs the BR/EDR discovery procedure
DISCOV_LE:       The kernel runs the LE-Only discovery procedure

BR,

Andre