Peripheral role support for android

Peripheral role support for android

[Florian Grandel]

Hi guys,

the whole point of adding the multi adv feature to the kernel was trying 
to support the peripheral role on my nexus 4.

Now that multi adv made it into the kernel, I'd like to add the missing 
pieces to the android userspace sub-project in bluez.

Here are the todos I identified so far:

1) kernel backports: provide patches necessary to support the current 
linux-next master (I did this already... - patches all supplied to the 
backports project and msm kernel + modules compiled on my machine)

2) android/bluetooth: implement bt_le_read_adv_features() to retrieve 
num supported instances from the kernel

3) android/bluetooth: change prepare_le_features() to retrieve the max 
number of adv instances from the kernel and save it to the adapter 
struct - will use bt_le_read_adv_features()

4) android/gatt: change [un]register_app() to correctly manage app ids 
(aka instance ids) in the permitted range including appropriate error 
handling when the max number of instances is reached

5) android/bluetooth: implement bt_le_{add|remove}_advertising() to 
support the new multi-adv features in the kernel

6) android/gatt: implement the handle*multi_adv*() methods using 
bt_le_{add|remove}_advertising()

7) android/tester-{bluetooth|gatt}: write supporting tests

Do you agree that this should suffice to support basic advertising 
features in Lollipop? Is there anything important missing?

AFAICS there is (at least) one catch: the kernel does not support 
setting custom values for min/max adv interval, channel map, tx power, 
etc. while the HAL API seems to expect per-instance setup of these 
values. How should we deal with this? Should we ignore these values? Or 
should we error out with a NOT_SUPPORTED error if they are different 
from the kernel defaults?

Please let me know whether you agree to that roadmap and what you think 
about the unsupported features. Is it ok if I try to tackle these todos? 
Or is someone working on this already?

Florian

Re: Peripheral role support for android

[Marcel Holtmann]

Hi Florian,

> the whole point of adding the multi adv feature to the kernel was trying to support the peripheral role on my nexus 4.
> 
> Now that multi adv made it into the kernel, I'd like to add the missing pieces to the android userspace sub-project in bluez.
> 
> Here are the todos I identified so far:
> 
> 1) kernel backports: provide patches necessary to support the current linux-next master (I did this already... - patches all supplied to the backports project and msm kernel + modules compiled on my machine)
> 
> 2) android/bluetooth: implement bt_le_read_adv_features() to retrieve num supported instances from the kernel
> 
> 3) android/bluetooth: change prepare_le_features() to retrieve the max number of adv instances from the kernel and save it to the adapter struct - will use bt_le_read_adv_features()
> 
> 4) android/gatt: change [un]register_app() to correctly manage app ids (aka instance ids) in the permitted range including appropriate error handling when the max number of instances is reached

I think that this might need a simple mapping table. The instance ids from the kernel might not be the application ids handed out over the HAL.

> 5) android/bluetooth: implement bt_le_{add|remove}_advertising() to support the new multi-adv features in the kernel
> 
> 6) android/gatt: implement the handle*multi_adv*() methods using bt_le_{add|remove}_advertising()
> 
> 7) android/tester-{bluetooth|gatt}: write supporting tests
> 
> Do you agree that this should suffice to support basic advertising features in Lollipop? Is there anything important missing?

I do not think you are missing anything. Just get cracking and submit patches.

> AFAICS there is (at least) one catch: the kernel does not support setting custom values for min/max adv interval, channel map, tx power, etc. while the HAL API seems to expect per-instance setup of these values. How should we deal with this? Should we ignore these values? Or should we error out with a NOT_SUPPORTED error if they are different from the kernel defaults?

We never exposed the channel map since that is pretty much stupid. I would just ignore that parameter. Just accept whatever it asks of and see okay, I will do it, even if you don't in the end.

The TX power you could set whatever you want, but I would advise against it. Just accept the TX power you get from the HAL and then set Include TX Power flag. I mean the best you can do is check if TX Power is included or not. And allow the advertising data to reflect this.

And min/max advertising interval is also something you should ignore. Actually that is something an application should never be in control of anyway. Choosing bad parameters might cause the controller to make a choice between BR/EDR and LE in its schedule. And for obvious reasons, the application has not knowledge of the BR/EDR state. So just accept them and go on with it.

> Please let me know whether you agree to that roadmap and what you think about the unsupported features. Is it ok if I try to tackle these todos? Or is someone working on this already?

Go ahead.

Regards

Marcel

Re: Peripheral role support for android

[Florian Grandel]

>> AFAICS there is (at least) one catch: the kernel does not support setting custom values for min/max adv interval, channel map, tx power, etc. while the HAL API seems to expect per-instance setup of these values. How should we deal with this? Should we ignore these values? Or should we error out with a NOT_SUPPORTED error if they are different from the kernel defaults?
>
> We never exposed the channel map since that is pretty much stupid. I would just ignore that parameter. Just accept whatever it asks of and see okay, I will do it, even if you don't in the end.
>
> The TX power you could set whatever you want, but I would advise against it. Just accept the TX power you get from the HAL and then set Include TX Power flag. I mean the best you can do is check if TX Power is included or not. And allow the advertising data to reflect this.
>
> And min/max advertising interval is also something you should ignore. Actually that is something an application should never be in control of anyway. Choosing bad parameters might cause the controller to make a choice between BR/EDR and LE in its schedule. And for obvious reasons, the application has not knowledge of the BR/EDR state. So just accept them and go on with it.
>
>> Please let me know whether you agree to that roadmap and what you think about the unsupported features. Is it ok if I try to tackle these todos? Or is someone working on this already?
>
> Go ahead.
>
> Regards
>
> Marcel

Thanks Marcel! All questions answered. :-)

Florian

Re: Peripheral role support for android

[Florian Grandel]

Hi guys,

just a quick update from my side so that you know, I'm still active.

Done:
1) kernel backports: provide patches necessary to support the current 
linux-next master
2) android/bluetooth: implement bt_le_read_adv_features() to retrieve 
num supported instances from the kernel
3) android/bluetooth: change prepare_le_features() to retrieve the max 
number of adv instances from the kernel and save it to the adapter 
struct - will use bt_le_read_adv_features()
4) android/gatt: change [un]register_app() to correctly manage app ids 
(aka instance ids) in the permitted range including appropriate error 
handling when the max number of instances is reached
5) android/bluetooth: implement bt_le_{add|remove}_advertising() to 
support the new multi-adv features in the kernel
6) android/gatt: implement the handle*multi_adv*() methods using 
bt_le_{add|remove}_advertising()

Currently working on:
7) android/tester-{bluetooth|gatt}: write supporting tests

Florian