Modified bt_le_ext_adv_stop() to fix an issue in which BT Controller #90757
Conversation
could return HCI errors because Zephyr code skipped sending BT_HCI_OP_LE_SET_EXT_ADV_ENABLE command to stop advertising. This happened because BT_ADV_ENABLED flag was cleared when BT_HCI_EVT_LE_ADV_SET_TERMINATED HCI event was received after a device connected Bluetooth. BT Controller still requires advertising to be stopped before it will allow advertising parameters to be changed. Signed-off-by: Doug Young <dougyoung@meta.com>
github-actions
bot
added
area: Bluetooth Host
github-actions
bot
requested review from
alwa-nordic,
cvinayak,
hermabe,
jhedberg,
rugeGerritsen,
sjanc,
Thalley and
theob-pro
|
| @@ -1444,7 +1444,7 @@ int bt_le_adv_stop(void) | |||
| int err; | |||
|
|
|||
| if (!adv) { | |||
| LOG_ERR("No valid legacy adv"); | |||
| LOG_ERR("%s: No valid legacy adv", __func__); | |||
There was a problem hiding this comment.
Please revert the log changes
| /* Don't return early if BT_ADV_ENABLED is not set, because this bit | ||
| * could have been cleared by the BT_HCI_EVT_LE_ADV_SET_TERMINATED | ||
| * HCI event. Some BT Controllers will return errors to subsequent HCI | ||
| * command modifying advertising parameters if BT_HCI_OP_LE_SET_EXT_ADV_ENABLE | ||
| * is not sent to disable advertising after BT_HCI_EVT_LE_ADV_SET_TERMINATED | ||
| * is received. Per the BT spec, "Disabling an advertising set that is already | ||
| * disabled has no effect", so it should be okay to send the HCI command twice | ||
| * in the event that bt_le_ext_adv_stop() is called twice before advertising is | ||
| * enabled again. | ||
| */ |
There was a problem hiding this comment.
I guess you are referring to
If the Host issues this command when advertising is enabled for the specified advertising set, the Controller shall return the error code Command Disallowed (0x0C).
from 7.8.53 LE Set Extended Advertising Parameters command of a case where the controller may send the BT_HCI_EVT_LE_ADV_SET_TERMINATED event, but still considers the advertising set "enabled", and would thus reject new parameters?
It's an interesting issue, because it is only if BT_HCI_EVT_LE_ADV_SET_TERMINATED does not bring the advertising set into the "disabled" state that the above can happen. I can't find anything in the core spec that explicitly states that the terminated event brings the advertising set to the disabled state.
Additionally, it's also possible to send HCI_LE_Set_Extended_Advertising_Enable while the advertising set is enabled:
If the HCI_LE_Set_Extended_Advertising_Enable command is sent again for an advertising set while that set is enabled, the timer used for the duration and the number of events counter are reset and any change to the random address shall take effect.
So we actually have another invalid check in:
int bt_le_ext_adv_start(struct bt_le_ext_adv *adv,
const struct bt_le_ext_adv_start_param *param)
{
struct bt_conn *conn = NULL;
int err;
CHECKIF(adv == NULL) {
LOG_DBG("adv is NULL");
return -EINVAL;
}
if (atomic_test_bit(adv->flags, BT_ADV_ENABLED)) {
return -EALREADY;
}@alwa-nordic similar to the discussion in #90185 (comment), using the "start" and "stop" terms doesn't really match the behavior of the core spec if we can actually enable (but not start) a set multiple times, or disable (but not stop) it multiple times.
There was a problem hiding this comment.
I can't find anything in the core spec that explicitly states that the terminated event brings the advertising set to the disabled state.
The spec stipulates the advertiser enters disabled state before or at the same time as the advertiser set terminated event. We can gather this from these two pieces:
4.E.7.8.69 says an advertiser set terminated event implies a connection has been created or the advertising set timed out or reached the maximum number of events.
[The LE Advertising Set Terminated event] event shall be generated every time connectable advertising in an advertising set results in a connection being created or because the advertising duration or the maximum number of extended advertising events has been reached.
4.E.7.8.56 says the advertiser set becomes disabled when a connection is created as well as all other times the advertising set terminated event occurs.
An advertising set shall be disabled when [...] a connection is created using that advertising set, the duration specified in the Duration[i] parameter expires, or the number of extended advertising events transmitted for the set exceeds the Max_Extended_Advertising_Events[i] parameter.
There was a problem hiding this comment.
Can you fix it in the controller, @dwyoung43?
Otherwise I think this has to be treated as a controller quirk, in which case I think the best workaround is to send the disable command when we receive the terminate event.
There was a problem hiding this comment.
@alwa-nordic Thanks - The two quotes you added does heavily imply that disabled == terminated (at least for connectable advertisements), and thus this seems like a interop issue with this specific controller
could return HCI errors because Zephyr code skipped sending BT_HCI_OP_LE_SET_EXT_ADV_ENABLE command to stop advertising. This happened because BT_ADV_ENABLED flag was cleared when BT_HCI_EVT_LE_ADV_SET_TERMINATED HCI event was received after a device connected Bluetooth. BT Controller still requires advertising to be stopped before it will allow advertising parameters to be changed.