Skip to content

Modified bt_le_ext_adv_stop() to fix an issue in which BT Controller #90757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+12 −5
Open

Modified bt_le_ext_adv_stop() to fix an issue in which BT Controller #90757

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions subsys/bluetooth/host/adv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the log changes

return 0;
}

Expand Down Expand Up @@ -1529,7 +1529,7 @@ void bt_le_adv_resume(void)
int err;

if (!adv) {
LOG_DBG("No valid legacy adv");
LOG_DBG("%s: No valid legacy adv", __func__);
return;
}

Expand Down Expand Up @@ -1754,9 +1754,16 @@ int bt_le_ext_adv_stop(struct bt_le_ext_adv *adv)

atomic_clear_bit(adv->flags, BT_ADV_PERSIST);

if (!atomic_test_bit(adv->flags, BT_ADV_ENABLED)) {
return 0;
}
/* 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.
*/
Comment on lines +1757 to +1766
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@alwa-nordic alwa-nordic Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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


if (atomic_test_and_clear_bit(adv->flags, BT_ADV_LIMITED)) {
bt_id_adv_limited_stopped(adv);
Expand Down
Loading