Bluetooth: split setting params and enabling advertising (1/3)

Proper order of commands when starting advertising:
* set parameters
* set data
* enable advertising

This cannot be achieved when setting advertising parameters and enabling
advertising is put together in one function. Enabling before setting
data might cause first advertisements to be empty. If a device with
hardware filter pick up such advertisement, it might fail to properly
recognize our device.

Bug: 30622771
Bug: 19372779

Change-Id: I66c71f1b2c07c832eda0983ead816741798e923b

diff --git a/jni/com_android_bluetooth_gatt.cpp b/jni/com_android_bluetooth_gatt.cpp
index 7106a67..7c50bb5 100644 (file)
--- a/jni/com_android_bluetooth_gatt.cpp
+++ b/jni/com_android_bluetooth_gatt.cpp
@@ -158,10 +158,9 @@ static jmethodID method_onScanFilterConfig;
 static jmethodID method_onScanFilterParamsConfigured;
 static jmethodID method_onScanFilterEnableDisabled;
 static jmethodID method_onAdvertiserRegistered;
-static jmethodID method_onMultiAdvEnable;
-static jmethodID method_onMultiAdvUpdate;
+static jmethodID method_onMultiAdvSetParams;
 static jmethodID method_onMultiAdvSetAdvData;
-static jmethodID method_onMultiAdvDisable;
+static jmethodID method_onMultiAdvEnable;
 static jmethodID method_onClientCongestion;
 static jmethodID method_onBatchScanStorageConfigured;
 static jmethodID method_onBatchScanStartStopped;
@@ -605,17 +604,10 @@ void ble_advertiser_register_cb(int status, int advertiser_id, bt_uuid_t *uuid)
     checkAndClearExceptionFromCallback(sCallbackEnv, __FUNCTION__);
 }
 
-void ble_advertiser_enable_cb(int advertiser_id, int status)
+void ble_advertiser_set_params_cb(int advertiser_id, int status)
 {
     CHECK_CALLBACK_ENV
-    sCallbackEnv->CallVoidMethod(mCallbacksObj, method_onMultiAdvEnable, status,advertiser_id);
-    checkAndClearExceptionFromCallback(sCallbackEnv, __FUNCTION__);
-}
-
-void ble_advertiser_update_cb(int advertiser_id, int status)
-{
-    CHECK_CALLBACK_ENV
-    sCallbackEnv->CallVoidMethod(mCallbacksObj, method_onMultiAdvUpdate, status, advertiser_id);
+    sCallbackEnv->CallVoidMethod(mCallbacksObj, method_onMultiAdvSetParams, status, advertiser_id);
     checkAndClearExceptionFromCallback(sCallbackEnv, __FUNCTION__);
 }
 
@@ -626,19 +618,18 @@ void ble_advertiser_setadv_data_cb(int advertiser_id, int status)
     checkAndClearExceptionFromCallback(sCallbackEnv, __FUNCTION__);
 }
 
-void ble_advertiser_disable_cb(int advertiser_id, int status)
+void ble_advertiser_enable_cb(int advertiser_id, int status, bool enable)
 {
     CHECK_CALLBACK_ENV
-    sCallbackEnv->CallVoidMethod(mCallbacksObj, method_onMultiAdvDisable, status, advertiser_id);
+    sCallbackEnv->CallVoidMethod(mCallbacksObj, method_onMultiAdvEnable, status, advertiser_id, enable);
     checkAndClearExceptionFromCallback(sCallbackEnv, __FUNCTION__);
 }
 
 static const ble_advertiser_callbacks_t sGattAdvertiserCallbacks = {
     ble_advertiser_register_cb,
-    ble_advertiser_enable_cb,
-    ble_advertiser_update_cb,
+    ble_advertiser_set_params_cb,
     ble_advertiser_setadv_data_cb,
-    ble_advertiser_disable_cb
+    ble_advertiser_enable_cb
 };
 
 /**
@@ -855,10 +846,9 @@ static void classInitNative(JNIEnv* env, jclass clazz) {
     method_onScanFilterParamsConfigured = env->GetMethodID(clazz, "onScanFilterParamsConfigured", "(IIII)V");
     method_onScanFilterEnableDisabled = env->GetMethodID(clazz, "onScanFilterEnableDisabled", "(III)V");
     method_onAdvertiserRegistered = env->GetMethodID(clazz, "onAdvertiserRegistered", "(IIJJ)V");
-    method_onMultiAdvEnable = env->GetMethodID(clazz, "onAdvertiseInstanceEnabled", "(II)V");
-    method_onMultiAdvUpdate = env->GetMethodID(clazz, "onAdvertiseDataUpdated", "(II)V");
+    method_onMultiAdvSetParams = env->GetMethodID(clazz, "onAdvertiseParamsSet", "(II)V");
     method_onMultiAdvSetAdvData = env->GetMethodID(clazz, "onAdvertiseDataSet", "(II)V");
-    method_onMultiAdvDisable = env->GetMethodID(clazz, "onAdvertiseInstanceDisabled", "(II)V");
+    method_onMultiAdvEnable = env->GetMethodID(clazz, "onAdvertiseInstanceEnabled", "(IIZ)V");
     method_onClientCongestion = env->GetMethodID(clazz, "onClientCongestion", "(IZ)V");
     method_onBatchScanStorageConfigured = env->GetMethodID(clazz, "onBatchScanStorageConfigured", "(II)V");
     method_onBatchScanStartStopped = env->GetMethodID(clazz, "onBatchScanStartStopped", "(III)V");
@@ -1393,23 +1383,20 @@ static void unregisterAdvertiserNative(JNIEnv* env, jobject object, jint adverti
 }
 
 static void gattClientEnableAdvNative(JNIEnv* env, jobject object, jint client_if,
-       jint min_interval, jint max_interval, jint adv_type, jint chnl_map, jint tx_power,
-       jint timeout_s)
+       jboolean enable, jint timeout_s)
 {
     if (!sGattIf) return;
 
-    sGattIf->advertiser->multi_adv_enable(client_if, min_interval, max_interval, adv_type, chnl_map,
-        tx_power, timeout_s);
+    sGattIf->advertiser->multi_adv_enable(client_if, enable, timeout_s);
 }
 
-static void gattClientUpdateAdvNative(JNIEnv* env, jobject object, jint client_if,
-       jint min_interval, jint max_interval, jint adv_type, jint chnl_map, jint tx_power,
-       jint timeout_s)
+static void gattClientSetAdvParamsNative(JNIEnv* env, jobject object, jint client_if,
+       jint min_interval, jint max_interval, jint adv_type, jint chnl_map, jint tx_power)
 {
     if (!sGattIf) return;
 
-    sGattIf->advertiser->multi_adv_update(client_if, min_interval, max_interval, adv_type, chnl_map,
-        tx_power, timeout_s);
+    sGattIf->advertiser->multi_adv_set_params(client_if, min_interval, max_interval, adv_type,
+        chnl_map, tx_power);
 }
 
 static void gattClientSetAdvDataNative(JNIEnv* env, jobject object , jint client_if,
@@ -1437,12 +1424,6 @@ static void gattClientSetAdvDataNative(JNIEnv* env, jobject object , jint client
                                              std::move(serv_data_vec), std::move(serv_uuid_vec));
 }
 
-static void gattClientDisableAdvNative(JNIEnv* env, jobject object, jint client_if)
-{
-    if (!sGattIf) return;
-    sGattIf->advertiser->multi_adv_disable(client_if);
-}
-
 static void gattClientConfigBatchScanStorageNative(JNIEnv* env, jobject object, jint client_if,
             jint max_full_reports_percent, jint max_trunc_reports_percent,
             jint notify_threshold_level_percent)
@@ -1678,10 +1659,9 @@ static void gattTestNative(JNIEnv *env, jobject object, jint command,
 static JNINativeMethod sAdvertiseMethods[] = {
     {"registerAdvertiserNative", "(JJ)V", (void *) registerAdvertiserNative},
     {"unregisterAdvertiserNative", "(I)V", (void *) unregisterAdvertiserNative},
-    {"gattClientEnableAdvNative", "(IIIIIII)V", (void *) gattClientEnableAdvNative},
-    {"gattClientUpdateAdvNative", "(IIIIIII)V", (void *) gattClientUpdateAdvNative},
+    {"gattClientSetParamsNative", "(IIIIII)V", (void *) gattClientSetAdvParamsNative},
     {"gattClientSetAdvDataNative", "(IZZZI[B[B[B)V", (void *) gattClientSetAdvDataNative},
-    {"gattClientDisableAdvNative", "(I)V", (void *) gattClientDisableAdvNative},
+    {"gattClientEnableAdvNative", "(IZI)V", (void *) gattClientEnableAdvNative},
     {"gattSetAdvDataNative", "(IZZZIII[B[B[B)V", (void *) gattSetAdvDataNative},
     {"gattAdvertiseNative", "(IZ)V", (void *) gattAdvertiseNative},
 };

diff --git a/src/com/android/bluetooth/gatt/AdvertiseManager.java b/src/com/android/bluetooth/gatt/AdvertiseManager.java
index 6778d2b..b3f1476 100644 (file)
--- a/src/com/android/bluetooth/gatt/AdvertiseManager.java
+++ b/src/com/android/bluetooth/gatt/AdvertiseManager.java
@@ -219,7 +219,6 @@ class AdvertiseManager {
             }
 
             mAdvertiseClients.add(client);
-            postCallback(advertiserId, AdvertiseCallback.ADVERTISE_SUCCESS);
         }
 
         // Handles stop advertising.
@@ -299,7 +298,7 @@ class AdvertiseManager {
         boolean startMultiAdvertising(AdvertiseClient client) {
             logd("starting multi advertising");
             resetCountDownLatch();
-            enableAdvertising(client);
+            setAdvertisingParameters(client);
             if (!waitForCallback()) {
                 return false;
             }
@@ -315,13 +314,19 @@ class AdvertiseManager {
                     return false;
                 }
             }
+            resetCountDownLatch();
+            enableAdvertising(client, true);
+            if (!waitForCallback()) {
+                return false;
+            }
+
             return true;
         }
 
         boolean startSingleAdvertising(AdvertiseClient client) {
             logd("starting single advertising");
             resetCountDownLatch();
-            enableAdvertising(client);
+            enableAdvertising(client, true);
             if (!waitForCallback()) {
                 return false;
             }
@@ -331,14 +336,14 @@ class AdvertiseManager {
 
         void stopAdvertising(AdvertiseClient client) {
             if (mAdapterService.isMultiAdvertisementSupported()) {
-                gattClientDisableAdvNative(client.advertiserId);
+                gattClientEnableAdvNative(client.advertiserId, false, 0);
             } else {
                 gattAdvertiseNative(client.advertiserId, false);
                 try {
-                    mService.onAdvertiseInstanceDisabled(
-                            AdvertiseCallback.ADVERTISE_SUCCESS, client.advertiserId);
+                    mService.onAdvertiseInstanceEnabled(
+                            AdvertiseCallback.ADVERTISE_SUCCESS, client.advertiserId, false);
                 } catch (RemoteException e) {
-                    Log.d(TAG, "failed onAdvertiseInstanceDisabled", e);
+                        Log.d(TAG, "failed onAdvertiseInstanceDisabled", e);
                 }
             }
         }
@@ -356,24 +361,32 @@ class AdvertiseManager {
             }
         }
 
-        private void enableAdvertising(AdvertiseClient client) {
+        private void setAdvertisingParameters(AdvertiseClient client) {
             int advertiserId = client.advertiserId;
             int minAdvertiseUnit = (int) getAdvertisingIntervalUnit(client.settings);
             int maxAdvertiseUnit = minAdvertiseUnit + ADVERTISING_INTERVAL_DELTA_UNIT;
             int advertiseEventType = getAdvertisingEventType(client);
             int txPowerLevel = getTxPowerLevel(client.settings);
-            int advertiseTimeoutSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(
-                    client.settings.getTimeout());
             if (mAdapterService.isMultiAdvertisementSupported()) {
-                gattClientEnableAdvNative(
+                gattClientSetParamsNative(
                         advertiserId,
                         minAdvertiseUnit, maxAdvertiseUnit,
                         advertiseEventType,
                         ADVERTISING_CHANNEL_ALL,
-                        txPowerLevel,
-                        advertiseTimeoutSeconds);
+                        txPowerLevel);
+            } else {
+                //there is no method to set parameters if multi advertising not enabled
+            }
+        }
+
+        private void enableAdvertising(AdvertiseClient client, boolean enable) {
+            int advertiserId = client.advertiserId;
+            int advertiseTimeoutSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(
+                    client.settings.getTimeout());
+            if (mAdapterService.isMultiAdvertisementSupported()) {
+                gattClientEnableAdvNative(advertiserId, enable, advertiseTimeoutSeconds);
             } else {
-                gattAdvertiseNative(client.advertiserId, true);
+                gattAdvertiseNative(client.advertiserId, enable);
             }
         }
 
@@ -504,20 +517,15 @@ class AdvertiseManager {
 
         private native void unregisterAdvertiserNative(int advertiserId);
 
-        private native void gattClientDisableAdvNative(int advertiserId);
-
-        private native void gattClientEnableAdvNative(int advertiserId,
-                int min_interval, int max_interval, int adv_type, int chnl_map,
-                int tx_power, int timeout_s);
-
-        private native void gattClientUpdateAdvNative(int advertiserId,
-                int min_interval, int max_interval, int adv_type, int chnl_map,
-                int tx_power, int timeout_s);
+        private native void gattClientSetParamsNative(int advertiserId,
+                int min_interval, int max_interval, int adv_type, int chnl_map, int tx_power);
 
         private native void gattClientSetAdvDataNative(int advertiserId,
                 boolean set_scan_rsp, boolean incl_name, boolean incl_txpower, int appearance,
                 byte[] manufacturer_data, byte[] service_data, byte[] service_uuid);
 
+        private native void gattClientEnableAdvNative(int advertiserId, boolean enable, int timeout_s);
+
         private native void gattSetAdvDataNative(int advertiserId, boolean setScanRsp, boolean inclName,
                 boolean inclTxPower, int minSlaveConnectionInterval, int maxSlaveConnectionInterval,
                 int appearance, byte[] manufacturerData, byte[] serviceData, byte[] serviceUuid);

diff --git a/src/com/android/bluetooth/gatt/GattService.java b/src/com/android/bluetooth/gatt/GattService.java
index 3449dc4..7c46df2 100644 (file)
--- a/src/com/android/bluetooth/gatt/GattService.java
+++ b/src/com/android/bluetooth/gatt/GattService.java
@@ -1171,17 +1171,11 @@ public class GattService extends ProfileService {
         }
     }
 
-    // Callback when advertise instance is enabled.
-    void onAdvertiseInstanceEnabled(int status, int advertiserId) {
-        if (DBG) Log.d(TAG, "onAdvertiseInstanceEnabled() - "
-                + "advertiserId=" + advertiserId + ", status=" + status);
-        mAdvertiseManager.callbackDone(advertiserId, status);
-    }
-
-    // Not really used.
-    void onAdvertiseDataUpdated(int status, int advertiserId) {
-        if (DBG) Log.d(TAG, "onAdvertiseDataUpdated() - advertiserId=" + advertiserId
+    // Callback when advertise parameters are set.
+    void onAdvertiseParamsSet(int status, int advertiserId) {
+        if (DBG) Log.d(TAG, "onAdvertiseParamsSet() - advertiserId=" + advertiserId
             + ", status=" + status);
+        mAdvertiseManager.callbackDone(advertiserId, status);
     }
 
     // Callback when advertise data or scan response is set.
@@ -1191,20 +1185,23 @@ public class GattService extends ProfileService {
         mAdvertiseManager.callbackDone(advertiserId, status);
     }
 
-    // Callback when advertise instance is disabled
-    void onAdvertiseInstanceDisabled(int status, int advertiserId) throws RemoteException {
-        if (DBG) Log.d(TAG, "onAdvertiseInstanceDisabled() - advertiserId=" + advertiserId
-            + ", status=" + status);
+    // Callback when advertise instance is enabled.
+    void onAdvertiseInstanceEnabled(int status, int advertiserId,
+                                    boolean enable) throws RemoteException {
+        if (DBG) Log.d(TAG, "onAdvertiseInstanceEnabled() - "
+                + "advertiserId=" + advertiserId + ", status=" + status + ", enable=" + enable);
+
+        if (enable)
+            mAdvertiseManager.callbackDone(advertiserId, status);
+
         AdvertiserMap.App app = mAdvertiserMap.getById(advertiserId);
         if (app != null) {
-            Log.d(TAG, "Advertiser app is not null!");
-            boolean isStart = false;
             if (status == 0) {
                 app.callback.onMultiAdvertiseCallback(AdvertiseCallback.ADVERTISE_SUCCESS,
-                        isStart, null);
+                        enable, null);
             } else {
                 app.callback.onMultiAdvertiseCallback(
-                        AdvertiseCallback.ADVERTISE_FAILED_INTERNAL_ERROR, isStart, null);
+                        AdvertiseCallback.ADVERTISE_FAILED_INTERNAL_ERROR, enable, null);
             }
         }
     }