CHROMIUM: bluetooth: set advertising intervals after registering advertisements
There are some problems if advertising intervals are set after
registrations of advertisement instances:
- only 1 instance would keep advertising, and
- instead of the original public address, a random address will be used.
The problems are fixed by
- modifying the duration of every existing advertisement instance,
- only modifying the duration if the duration of an instance was
NOT specified during the registration
* individual_duration_flag: This adds a flag to mark the
advertising instances whose advertising durations were provided
during registration. The advertising durations of the instances
with this flag set to true would not be modified by the dbus
SetAdvertisingIntervals method. On the other hand,
the advertising duration of the instances with this flag
set to false would be updated with the new values specified
by the dbus SetAdvertisingIntervals method.
- removing the re-enabling advertising code which is unnecessary.
Since a new instance will be registered with advertising intervals
through the regular procedure which makes the configuration of
advertising simper and robust.
BUG=chromium:655251
TEST=Verify that the two chrome advertising test apps run correctly.
Original App:
Run the original chrome advertising test app which registers
3 advertisement instances *after* setting advertising intervals.
Use nRF Connect, and watch that the advertisement instances are
interleaved in a round-robin manner.
The new App:
Run the chrome advertising test "app2" which registers
3 advertisement instances *before* setting advertising intervals.
Use nRF Connect, and watch that the advertisement instances are
interleaved in a round-robin manner.
Change-Id: I7978e30ad582651124a8decd95808302bdce137c
Reviewed-on: https://chromium-review.googlesource.com/406775
Commit-Ready: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
(cherry picked from commit 2839031c3d631aae1a44c4f56cf8664ed32ad267)
Reviewed-on: https://chromium-review.googlesource.com/412561
Trybot-Ready: Miao-chen Chou <mcchou@chromium.org>
Commit-Queue: Shyh-In Hwang <josephsih@chromium.org>
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f358208f..9430ed1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -165,6 +165,7 @@
__u16 timeout;
__u16 remaining_time;
__u16 duration;
+ __u8 individual_duration_flag;
__u16 adv_data_len;
__u8 adv_data[HCI_MAX_AD_LENGTH];
__u16 scan_rsp_len;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b2b1d49..39bc112 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2699,10 +2699,13 @@
adv_instance->timeout = timeout;
adv_instance->remaining_time = timeout;
- if (duration == 0)
+ if (duration == 0) {
adv_instance->duration = hdev->le_adv_duration;
- else
+ adv_instance->individual_duration_flag = 0;
+ } else {
adv_instance->duration = duration;
+ adv_instance->individual_duration_flag = 1;
+ }
BT_DBG("%s for %dMR", hdev->name, instance);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 744e8326..6fa8fab 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4093,103 +4093,6 @@
return err;
}
-static void set_advertising_intervals_complete(struct hci_dev *hdev,
- u8 status, u16 opcode)
-{
- struct cmd_lookup match = { NULL, hdev };
- struct hci_request req;
- u8 instance;
- struct adv_info *adv_instance;
- int err;
-
- hci_dev_lock(hdev);
-
- if (status) {
- u8 mgmt_err = mgmt_status(status);
-
- mgmt_pending_foreach(MGMT_OP_SET_ADVERTISING_INTERVALS, hdev,
- cmd_status_rsp, &mgmt_err);
- goto unlock;
- }
-
- if (hci_dev_test_flag(hdev, HCI_LE_ADV))
- hci_dev_set_flag(hdev, HCI_ADVERTISING);
- else
- hci_dev_clear_flag(hdev, HCI_ADVERTISING);
-
- mgmt_pending_foreach(MGMT_OP_SET_ADVERTISING_INTERVALS, hdev,
- settings_rsp, &match);
-
- new_settings(hdev, match.sk);
-
- if (match.sk)
- sock_put(match.sk);
-
- /* If "Set Advertising" was just disabled and instance advertising was
- * set up earlier, then re-enable multi-instance advertising.
- */
- if (hci_dev_test_flag(hdev, HCI_ADVERTISING) ||
- list_empty(&hdev->adv_instances))
- goto unlock;
-
- instance = hdev->cur_adv_instance;
- if (!instance) {
- adv_instance = list_first_entry_or_null(&hdev->adv_instances,
- struct adv_info, list);
- if (!adv_instance)
- goto unlock;
-
- instance = adv_instance->instance;
- }
-
- hci_req_init(&req, hdev);
-
- err = __hci_req_schedule_adv_instance(&req, instance, true);
- if (!err)
- err = hci_req_run(&req, enable_advertising_instance);
- else
- BT_ERR("Failed to re-configure advertising intervals");
-
-unlock:
- hci_dev_unlock(hdev);
-}
-
-static int _reenable_advertising(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
-{
- struct mgmt_pending_cmd *cmd;
- struct hci_request req;
- int err;
-
- if (pending_find(MGMT_OP_SET_ADVERTISING_INTERVALS, hdev)) {
- return mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_SET_ADVERTISING_INTERVALS,
- MGMT_STATUS_BUSY);
- }
-
- cmd = mgmt_pending_add(sk, MGMT_OP_SET_ADVERTISING_INTERVALS, hdev,
- data, len);
- if (!cmd)
- return -ENOMEM;
-
- hci_req_init(&req, hdev);
- cancel_adv_timeout(hdev);
-
- /* Switch to instance "0" for the Set Advertising setting.
- * We cannot use update_[adv|scan_rsp]_data() here as the
- * HCI_ADVERTISING flag is not yet set.
- */
- hdev->cur_adv_instance = 0x00;
- /* This function disables advertising before enabling it. */
- __hci_req_enable_advertising(&req);
-
- err = hci_req_run(&req, set_advertising_intervals_complete);
- if (err < 0)
- mgmt_pending_remove(cmd);
-
- return err;
-}
-
static int set_advertising_intervals(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len)
{
@@ -4198,6 +4101,8 @@
u16 max_interval_ms, grace_period;
/* If both min_interval and max_interval are 0, use default values. */
bool use_default = cp->min_interval == 0 && cp->max_interval == 0;
+ struct adv_info *adv_instance;
+ int instance;
BT_DBG("%s", hdev->name);
@@ -4235,16 +4140,26 @@
hdev->le_adv_duration = max_interval_ms + grace_period;
}
- /* Re-enable advertising only when it is already on. */
- if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
- err = _reenable_advertising(sk, hdev, data, len);
- goto unlock;
+ /* hdev->le_adv_duration would be copied to adv instances created
+ * hereafter. However, for any existing adv instance of which the
+ * individual_duration_flag is false, we should modify its duration.
+ */
+ for (instance = 1; instance <= HCI_MAX_ADV_INSTANCES; instance++) {
+ adv_instance = hci_find_adv_instance(hdev, instance);
+ if (adv_instance && !adv_instance->individual_duration_flag)
+ adv_instance->duration = hdev->le_adv_duration;
}
+ /* If advertising is not enabled, the new parameters will take effect
+ * when advertising is enabled.
+ * If advertising has been enabled, the new parameters will take effect
+ * when next adv instance is scheduled by
+ * __hci_req_schedule_adv_instance().
+ * Hence, it is ok to send settings response now.
+ */
err = send_settings_rsp(sk, MGMT_OP_SET_ADVERTISING_INTERVALS, hdev);
new_settings(hdev, sk);
-unlock:
hci_dev_unlock(hdev);
return err;
