samples: Bluetooth: Broadcast multiple legacy and extended advertising #94304
Conversation
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
9 times, most recently
from
771caba to
7afe802
Compare
zephyrbot
added
platform: nRF BSIM
zephyrbot
requested review from
alwa-nordic,
asbjornsabo,
fredrikdanebjer,
HaavardRei,
hermabe,
jhedberg,
jthm-ot,
kartben,
larsgk,
nashif and
niym-ot
There was a problem hiding this comment.
I'll leave the same comment here as I usually do when you modify the samples for testing purposes: IMO samples should not be used for this purpose, and should be kept as simple as possible. I don't think the changes to the sample here will help users, and it seems to only be done to help test some features, which should done in a separate test application, instead of (ab)using samples for test purposes
samples/bluetooth/broadcaster_multiple/src/broadcaster_multiple.c
Outdated
Show resolved
Hide resolved
samples/bluetooth/broadcaster_multiple/src/broadcaster_multiple.c
Outdated
Show resolved
Hide resolved
samples/bluetooth/hci_ipc/nrf5340_cpunet_iso-bt_ll_sw_split.conf
Outdated
Show resolved
Hide resolved
There are no samples today demonstrating advertising both legacy and extended advertising (LL CI testing has coverage already); Certain phones do not fully/correctly support extended advertising and it has been a need to have advertising both legacy and extended in certain usecases to have a general inter-operability with phones not supporting versus ones that support. |
| # Enable Coded PHY support in Zephyr Controller, if testing 4 advertising sets | ||
| # CONFIG_BT_CTLR_PHY_CODED=y |
There was a problem hiding this comment.
@jhedberg Currently, the Controller specific is commented for this sample, do you agree that this be moved to a overlay and sample document/readme updated according to indicate the same, right?
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
3 times, most recently
from
e99363d to
eda4498
Compare
Enable similar Controller Kconfig features between nRF52 and nRF5340 BabbleSIM LE Audio testing. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Tune the aux offset calculation overhead assertion to use EVENT_OVERHEAD_START_US; this is the correct maximum overhead causing ISR latency for start of a radio event that occurs thereafter. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Update broadcaster_multiple sample to start multiple advertising sets of type legacy and extended advertising. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
cvinayak
force-pushed
the
github_broadcast_multiple_with_legacy
branch
from
eda4498 to
3c717d8
Compare
|
| /* Use 1M legacy PDU */ | ||
| {.options = BT_LE_ADV_OPT_NONE, | ||
| .ad = ad_short, | ||
| .ad_size = ARRAY_SIZE(ad_short), | ||
| }, | ||
| /* Use 2M auxiliary PDU */ | ||
| {.options = BT_LE_ADV_OPT_EXT_ADV, |
There was a problem hiding this comment.
This doesn't look like it's following a correct indentation style, where the contents of a new { ... } block should always be indented by one more tab. If you did it like the following it should be correct, and not even add any new lines:
{ /* Use 1M legacy PDU */
.options = BT_LE_ADV_OPT_NONE,
.ad = ad_short,
.ad_size = ARRAY_SIZE(ad_short),
},
{ /* Use 2M auxiliary PDU */
.options = BT_LE_ADV_OPT_EXT_ADV,
...There was a problem hiding this comment.
Thanks, will fix this indentation.
| /* Use 1M legacy PDU */ | ||
| {.options = BT_LE_ADV_OPT_NONE, | ||
| .ad = ad_short, | ||
| .ad_size = ARRAY_SIZE(ad_short), | ||
| }, | ||
| /* Use 2M auxiliary PDU */ | ||
| {.options = BT_LE_ADV_OPT_EXT_ADV, |
There was a problem hiding this comment.
Thanks, will fix this indentation.
cvinayak
removed request for
kartben,
nashif,
PavelVPV,
larsgk,
sjanc,
fredrikdanebjer,
rugeGerritsen,
hermabe,
asbjornsabo,
alwa-nordic,
HaavardRei,
niym-ot and
jthm-ot
Update broadcaster_multiple sample to start multiple advertising sets of type legacy and extended advertising.
How they appear on-air (BabbleSim import), for 4 advertising sets:
