Bluetooth: host: Add extended advertising API

[joerchan]

Add extended advertising API. The use-cases that are included in this
are Extended Advertising Data, connecting using Long Range feature, and
multiple advertisers.

---

The new advertising API and the options for scanning and creating connections should be backwards compatible. The advertising set API is modeled to be similar to the connection API.

I intend to keep the old advertising API in addition to the new advertising set API when Extended Advertising Feature is enabled. The use-case for this is to continue with the old Advertising API while using the new Extended Advertising HCI commands. This has the advantage of enabling advertising with a different Identity while scanner/initiator uses the default Identity. In this case the stack will keep an advertising set internally and manage this without giving away the advertising set reference.

Periodic advertising has not been considered yet. But I think this feature can be made as an addition on top of the advertising set API. I'm not sure if we should include periodic advertising in this PR, but if we don't then we should atleast make sure that it can be added without requiring changes to the advertising set API.

Following changes to the existing API:

• Remove BT_LE_SCAN_FILTER_EXTENDED (not functional, and shouldn't have been added in the first place).
• Renamed scan filter_dup to options in struct bt_le_scan_param
• Renamed scan filter options enum values prefix from BT_LE_SCAN to BT_LE_SCAN_OPT.
• Removed BT_LE_ADV PDU types defined in hci.h and introduced new defines in gap.h
• Rename bt_conn_create_le to bt_conn_le_create, this changes to return error code instead and having the conn object as out parameter, and additional parameters to control scan parameters and scan options of the initiator.
• Rename bt_conn_create_slave_le to bt_conn_le_create_slave, this changes to return error code and having th econn object as out parameter.

Note about privacy and OOB data.
There is only a single RPA timer, this has the implication that if one RPA address needs to be cycled by the host, then all RPA addresses are cycled and RPA timeout reset. Addresses are cycled by the host in the following cases:

• RPA timeout (normal case)
• Generating OOB data
• Calling bt_conn_le_create close to the RPA timeout (3 seconds is default).

To handle the case where duration and num_events are used as advertising parameters > 0, this type of advertiser is defined as a "limited" advertiser, meaning it will be stopped by controller once a limit has been reached.
A limited advertiser is not bound by the RPA timeout, instead it generates a new RPA address each time it is started, and cannot be active for longer than the RPA timeout value. In a sense it has its own RPA timer.
To get the OOB data for central role, or an advertiser started using bt_le_adv_start then bt_le_oob_get_local should be used.
To get the OOB data for advertisers started usingbt_le_ext_adv_create then bt_le_ext_adv_oob_get_local should be used.

Currently I have decided to leave out the following features:
The use of period.
Channel MAP (also not present in old API).

Features that will be improved later:
Limited functionality to only one advertising set.
Extendend advertising data fragmentation and re-assembly.

Fixes: #18047
Fixes: #18046
Fixes: #18050

Fixes: #23247

[zephyrbot]

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[joerchan]

Added this as draft PR to get input on the general directions. There are many small details that we can leave for later.

[joerchan]

CC: @aescolar Any input from your camp? Even though periodic advertising is not included yet, we could get some perspective on how it could possibly be extended?

[aescolar]

CC: @aescolar Any input from your camp? Even though periodic advertising is not included yet, we could get some perspective on how it could possibly be extended?

CC: @wopu-ot @thoh-ot @wdhpawa

[Vudentz]

So the idea here is that we would do memory allocation? I rather not do this since we may have different parts trying to advertise so application would only notice the lack of memory at runtime, so I rather leave the memory allocation to the application directly.

[joerchan]

@Vudentz It would allocate one of a fixed number of advertising contexts in the host. This one is quite small and mostly just has the advertising_handle and some state variables. This API would create a new advertising context in the controller. And it is not possible to give allocated memory to the controller over the HCI interface.

This is also similar to how conn objects are allocated. This would be symmetrical to bt_conn_create_le (which wraps the connection handle plus internal state).

[Vudentz]

So we are assuming we know the number of sets available in the controller? How about split builds? Either way we cannot mix legacy advertisement with extended ones over HCI as that would fail so having an API which masks this sort of detail is probably desirable, we could make the sets work even without the controller supporting it by doing the advertising set context on the host, anyway the spec don't seem to describe how the scheduling should happen so just rotating the sets should work.

[joerchan]

So we are assuming we know the number of sets available in the controller?
How about split builds?

That's a good question actually. The number of advertising sets in the controller can be read using
LE Read Number of Supported Advertising Sets command. However it has this note:

Note: The number of advertising sets that can be supported is not fixed and the
Controller can change it at any time because the memory used to store
advertising sets can also be used for other purposes.

I think we can assume in combined build that we would have a specific number of advertising sets set by Kconfig.
In the split build case, I think we should just make sure that we atleast have enough advertising set contexts in the host.
The context itself is not very large so i don't see this is a problem.
In any case the larger memory usage is in the HCI both for advertising context and advertising data.
In split build we cannot assign memory to the controller, maybe in the combined case, but that would still have to be something that is outside of HCI.

Either way we cannot mix legacy advertisement with extended ones over HCI as that would fail so having an API which masks this sort of detail is probably desirable,

We cannot mix legacy and extended, but we can support the old advertising API with the new API, limited to one set.
And the old advertising API can use the new advertising HCI commands, with the added advantage of solving the multiple identities problem, solving a structural issue with the current API.

we could make the sets work even without the controller supporting it by doing the advertising set context on the host, anyway the spec don't seem to describe how the scheduling should happen so just rotating the sets should work.

In this case we would have to rotate advertising parameters and advertising data, so then we would either have to copy them in the host, or require the application
to provide us pointers to memory that stays valid while for the lifetime of the advertising set.
In this case I think it would maybe be easier to keep with the old recommendation of cycling the advertising data of a single advertiser instead.

[carlescufi]

, I think we should just make sure that we atleast have enough advertising set contexts in the host.

This is similar to what happens today with buffer counts and sizes.

[Vudentz]

Is this suppose to copy ad and sd data over to bt_adv or just reference them? I guess we want the later which means the application will have to manage part of the adv data anyway so it would more sense to let it deal with all.

[joerchan]

It would copy the data directly to the controller. The host does not keep a copy. This function is bt_le_adv_data_update with the adv set context as an additional argument.

[Vudentz]

Right, I assumed we were tracking the actual data set to not have duplicated calls but it seems we don't do that.

[joerchan]

This would clear the old data, and set new data. We don't append the data to the existing. (Or did I misunderstand what you meant by duplicate calls?)

[sjanc]

on HCI level what is here sent and connected callback is single event, why split this? eg num_sent is also valid when set was terminated by connection

[joerchan]

I thought it was a reasonable simplification. Since they are two different things.
If the application is interested in both, then both callbacks will be called.

[carlescufi]

Aren't there cases where sent() is called by not connected()? like non-conn adv?

[joerchan]

If the application set num_sent, then sent callback will be called when adv set is terminated.
If the adv set is terminated because it was connected, then connected will be called.

So both could happen, or just one.

[sjanc]

Hmm I don't think flat buffers are suitable for EA, data can be up to 1650 bytes...

[joerchan]

Which data-structure would be better? net_buf?

[jhedberg]

It's not quite a flat buffer in this case though since ad is an array (one element for each data type to be included to the advertising data (the ad_len parameter is the array length). That said, net_buf might indeed be a more lucrative choice here.

[joerchan]

Right. I keep forgetting how that works. :)

Then I don't necessarily see the downside in continuing to use the same parameters as before.

What might be more important is making is the scan callback recv I think right now it has a net_buf_simple. And possibly bt_data_parse

[carlescufi]

@sjanc the advantage of mbuf-style chaining of multiple buffers are mostly evident when you have a global allocator that all clients consume memory from. This is not the case in this Host, and in general in Zephyr the Host assumes flat buffers. That said, I do support the idea of using net_buf so that we do not have to change the signature later if we start supporting fragment chains.

[carlescufi]

Nice job! I've added some comments to the API, I have not reviewed the implementation.

[carlescufi]

Aren't there cases where sent() is called by not connected()? like non-conn adv?

[carlescufi]

, I think we should just make sure that we atleast have enough advertising set contexts in the host.

This is similar to what happens today with buffer counts and sizes.

[carlescufi]

Why is this needed? Won't all controllers that support AE support the legacy commands too?

[joerchan]

This was intended as an option for when using the new advertising set API was not needed, but using the new extended
HCI commands was needed. It doesn't seem like there is much to gain from this option in terms of code reduction though. So I think I will just remove this option.

I think what we might need instead however is to assume the controller supports the feature, and skip checking the feature flag. For combined builds this would make sense, and we could compile out all the legacy HCI commands.

[carlescufi]

I think what we might need instead however is to assume the controller supports the feature, and skip checking the feature flag. For combined builds this would make sense, and we could compile out all the legacy HCI commands.

Agreed, for combined builds in the future (when we support AE in the controller) then BT_ADV_EXT_LEGACY_SUPPORT should default to n (once it's implemented).

[carlescufi]

LGTM now, thanks!

[a1ien]

This merge broken compilation c++ project. If include

#include <bluetooth/conn.h>

We get error

In file included from /home/embedded/Documents/workspace/bt_modem/core/main.cpp:73:
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h: In function 'bt_conn* bt_conn_create_le(const bt_addr_le_t*, const bt_le_conn_param*)':
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:349:38: error: taking address of temporary array
  349 |  ((struct bt_conn_le_create_param[]) { { \
      |  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
  350 |   .options = (_options), \
      |   ~~~~~~~~~~~~~~~~~~~~~~~~            
  351 |   .interval = (_interval), \
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~          
  352 |   .window = (_window), \
      |   ~~~~~~~~~~~~~~~~~~~~~~              
  353 |   .interval_coded = 0, \
      |   ~~~~~~~~~~~~~~~~~~~~~~              
  354 |   .window_coded = 0, \
      |   ~~~~~~~~~~~~~~~~~~~~                
  355 |   .timeout = 0, \
      |   ~~~~~~~~~~~~~~~                     
  356 |   } })
      |   ~~~~                                
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:362:2: note: in expansion of macro 'BT_CONN_LE_CREATE_PARAM'
  362 |  BT_CONN_LE_CREATE_PARAM(BT_LE_CONN_OPT_NONE, \
      |  ^~~~~~~~~~~~~~~~~~~~~~~
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:402:30: note: in expansion of macro 'BT_CONN_LE_CREATE_CONN'
  402 |  if (bt_conn_le_create(peer, BT_CONN_LE_CREATE_CONN, conn_param,
      |                              ^~~~~~~~~~~~~~~~~~~~~~
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h: In function 'int bt_conn_create_auto_le(const bt_le_conn_param*)':
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:349:38: error: taking address of temporary array
  349 |  ((struct bt_conn_le_create_param[]) { { \
      |  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
  350 |   .options = (_options), \
      |   ~~~~~~~~~~~~~~~~~~~~~~~~            
  351 |   .interval = (_interval), \
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~          
  352 |   .window = (_window), \
      |   ~~~~~~~~~~~~~~~~~~~~~~              
  353 |   .interval_coded = 0, \
      |   ~~~~~~~~~~~~~~~~~~~~~~              
  354 |   .window_coded = 0, \
      |   ~~~~~~~~~~~~~~~~~~~~                
  355 |   .timeout = 0, \
      |   ~~~~~~~~~~~~~~~                     
  356 |   } })
      |   ~~~~                                
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:371:2: note: in expansion of macro 'BT_CONN_LE_CREATE_PARAM'
  371 |  BT_CONN_LE_CREATE_PARAM(BT_LE_CONN_OPT_NONE, \
      |  ^~~~~~~~~~~~~~~~~~~~~~~
/home/embedded/Documents/workspace/zephyr_os/zephyr/include/bluetooth/conn.h:431:32: note: in expansion of macro 'BT_CONN_LE_CREATE_CONN_AUTO'
  431 |  return bt_conn_le_create_auto(BT_CONN_LE_CREATE_CONN_AUTO, conn_param);
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/app.dir/build.make:63: CMakeFiles/app.dir/core/main.cpp.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:201: CMakeFiles/app.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

Related #5695

[joerchan]

@a1ien Thanks for reporting. Can you give #24829 a try and report if it fixes you issue?