MYNEWT-715 Extended Advertising - Advertiser support for controller#360
MYNEWT-715 Extended Advertising - Advertiser support for controller#360rymanluk merged 27 commits intoapache:bluetooth5from
Conversation
wes3
left a comment
wes3
left a comment
There was a problem hiding this comment.
Just a nit-pick. This looks a bit odd (the 01).
#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY) ==01)
|
Changes looks good to me. |
There is no need for special handling of single iteration loop as compiler should catch this.
Name it BLE_ANDROID_MULTI_ADV_SUPPORT as it will be used only do enable Android specific vendor commands.
There is no need for handling VS commands if none is required.
As defined in Core Specification 5.0 Vol. 2 Part E 7.8.52.
This adds the necessary boilerplater for new commands.
sjanc
force-pushed
the
bt5_ext_adv
branch
2 times, most recently
from
9f02c4a to
36a57a2
Compare
|
I've updated PR branch. Fixed nitpick pointed by WIll and fixed sending scan req notification to host. |
rymanluk
left a comment
rymanluk
left a comment
There was a problem hiding this comment.
in general it looks good.
Some minor comments.
| #define BLE_HCI_LE_SET_EXT_ADV_PROP_INC_TX_PWR (0x0040) | ||
| #define BLE_HCI_LE_SET_EXT_ADV_PROP_MASK (0x7F) | ||
|
|
||
| #define BLE_HCI_LE_SET_EXT_ADV_PROP_LEGACY_IND (0x0013) |
There was a problem hiding this comment.
I think this is already defined in this file
There was a problem hiding this comment.
only first 5 props match what is set also in extended advertising event, but those are for separate command and (there is also anon adv and tx power which are not present in event), also mask size is different,
I'd leave it as is
| ext_hdr_len += BLE_LL_EXT_ADV_TX_POWER_SIZE; | ||
| } | ||
|
|
||
| if (acad) { |
|
|
||
| /* Set transmit start time. */ | ||
| #if MYNEWT_VAL(OS_CPUTIME_FREQ) == 32768 | ||
| txstart = sch->start_time + g_ble_ll_sched_offset_ticks; |
There was a problem hiding this comment.
I think we support only Low Power clock, so no need for that define
| txstart = sch->start_time + | ||
| os_cputime_usecs_to_ticks(XCVR_TX_SCHED_DELAY_USECS); | ||
| rc = ble_phy_tx_set_start_time(txstart); | ||
| #endif |
| max_usecs += XCVR_PROC_DELAY_USECS; | ||
|
|
||
| #if MYNEWT_VAL(OS_CPUTIME_FREQ) == 32768 | ||
| sch->start_time = advsm->adv_secondary_start_time - g_ble_ll_sched_offset_ticks; |
| /* The event start time is when we start transmission of the adv PDU */ | ||
| advsm->adv_event_start_time = sch_start + g_ble_ll_sched_offset_ticks; | ||
| #else | ||
| /* The event start time is when we start transmission of the adv PDU */ |
| } | ||
|
|
||
| if (!(advsm->props & BLE_HCI_LE_SET_EXT_ADV_PROP_SCANNABLE)) { | ||
| return BLE_ERR_INV_HCI_CMD_PARMS; |
There was a problem hiding this comment.
Looks like redundant double check
|
|
||
| if (!advsm->adv_enabled || !advsm->adv_len || datalen) { | ||
| return BLE_ERR_INV_HCI_CMD_PARMS; | ||
| } |
There was a problem hiding this comment.
just asking. below on !adv_enabled we send back command_disallowed and here invalid parameters. is that ok?
There was a problem hiding this comment.
yeah, some operations are not allowed if advertising is enabled, but I just noticed that operation 0x04 is valid only for setting advertising data, not scan_rsp. I'lld fix that.
This is in preparation for handling extended advertising PDUs. Legacy advertising PDUs are mapped as describbed in Core Spec Vol. 2 Part E. 7.8.53 table 7.2.
This allows to advertise with ADV_EXT_IND and AUX_ADV_IND PDUs using supported PHYs. AUX_ADV_IND are scheduled separately and ADV_EXT_IND use its start time for offset calculation. This is initial support and not all options are supported yet.
If AUX_SCAN_REQ is received then reply with AUX_SCAN_RSP.
Notify host about SCAN_REQ received if event is enabled.
This allows to create connection while advertising using Extended Advertising PDUs. CSA2 is mandatory for this type of connection so ChSel bit is RFU and is not used.
Currently this is done only for connection complete.
Those parameters are already handled.
RxAdd, TxAdd, Scan Request Notifications and Connection Response Sent are single bit values and can be stored as flags in common byte.
Anchor point depends on PHY used and should be adjusted accordingly.
This gets rid of artificial delays in scheduling of AUX_ADV_IND.
This allows host to set prefered TX power for advertising instance. Controller is allowed to ignore or adjust this value.
Move some fields around to minimize padding.
All functionality provided by those commands is available via standard HCI commands introduced in Bluetooth 5 specification. Also events generated but Android commands were not implemented correctly as vendor events. This resulted in spurious events being sent to host.
SID and DID are always transmitted together in ADI field. There is no need to keep them separated as ADI can be constructed when setting it from HCI. This saves both memory and computing
There is no need for adding additional bytes to advertising state machine structure.
|
I've updated branch with comments addresses (except flags define which I left as is) |
|
Thanks, looks good. |
3 participants
This adds support for controller for Extended Advertiser. Included:
Limitations (will be address in followup PRs):