Replace deprecated BT LE methods on Android API 33: characteristics

The overarching theme for these API deprecations is to
improve memory safety. This patch replaces the characteristic
related APIs.

The list of deprecated APIs, addressed by this and the followup commit:
* BluetoothGatt::writeCharacteristic()
* BluetoothGatt::writeDescriptor()
* BluetoothGattCharacteristic::getValue(), no replacement
* BluetoothGattCharacteristic::setValue(), no replacement
* BluetoothGattDescriptor::getValue(), no replacement
* BluetoothGattDescriptor::setValue(), no replacement
* BluetoothGattCallback::onCharacteristicChanged()
* BluetoothGattCallback::onCharacteristicRead()
* BluetoothGattCallback::onDescriptorRead()
* BluetoothGattServer::notifyCharacteristicChanged()

Pick-to: 6.5
Task-number: QTBUG-107180
Change-Id: I054db07301038ceb56b8af8471d71309d61ae777
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

4 files changed, 98 insertions, 27 deletions

diff --git a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothGattCharacteristic.java b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothGattCharacteristic.java
index c8779b00..375aebb5 100644
--- a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothGattCharacteristic.java
+++ b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothGattCharacteristic.java
@@ -4,7 +4,7 @@
 package org.qtproject.qt.android.bluetooth;
 
 import android.bluetooth.BluetoothGattCharacteristic;
-import android.util.Log;
+import android.os.Build;
 
 import java.util.UUID;
 
@@ -17,4 +17,28 @@ public class QtBluetoothGattCharacteristic extends BluetoothGattCharacteristic {
     }
     public int minValueLength;
     public int maxValueLength;
+    // Starting from API 33 Android Bluetooth deprecates characteristic local value caching by
+    // deprecating the getValue() and setValue() accessors. For peripheral role we store the value
+    // locally in the characteristic as a convenience - looking up the value on the C++ side would
+    // be somewhat complicated. This should be safe as all accesses to this class are synchronized.
+    // For clarity: For API levels below 33 we still need to use the setValue() of the base class
+    // because Android internally uses getValue() with APIs below 33.
+    public boolean setLocalValue(byte[] value) {
+        if (Build.VERSION.SDK_INT >= 33) {
+            m_localValue = value;
+            return true;
+        } else {
+            return setValue(value);
+        }
+    }
+
+    public byte[] getLocalValue()
+    {
+        if (Build.VERSION.SDK_INT >= 33)
+            return m_localValue;
+        else
+            return getValue();
+    }
+
+    private byte[] m_localValue = null;
 }
diff --git a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLE.java b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLE.java
index 8e8a0958..57accf3b 100644
--- a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLE.java
+++ b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLE.java
@@ -17,6 +17,7 @@ import android.bluetooth.le.ScanCallback;
 import android.bluetooth.le.ScanFilter;
 import android.bluetooth.le.ScanResult;
 import android.bluetooth.le.ScanSettings;
+import android.bluetooth.BluetoothStatusCodes;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
@@ -345,8 +346,9 @@ public class QtBluetoothLE {
     }
 
     private synchronized void handleOnCharacteristicRead(BluetoothGatt gatt,
-                                 BluetoothGattCharacteristic characteristic,
-                                 int status)
+                                BluetoothGattCharacteristic characteristic,
+                                byte[] value,
+                                int status)
     {
         int foundHandle = handleForCharacteristic(characteristic);
         if (foundHandle == -1 || foundHandle >= entries.size() ) {
@@ -380,7 +382,7 @@ public class QtBluetoothLE {
             leCharacteristicRead(qtObject,
                     characteristic.getService().getUuid().toString(),
                     foundHandle + 1, characteristic.getUuid().toString(),
-                    characteristic.getProperties(), characteristic.getValue());
+                    characteristic.getProperties(), value);
         } else {
             if (isServiceDiscoveryRun) {
                 Log.w(TAG, "onCharacteristicRead during discovery error: " + status);
@@ -389,7 +391,7 @@ public class QtBluetoothLE {
                                 " for service " + characteristic.getService().getUuid());
                 leCharacteristicRead(qtObject, characteristic.getService().getUuid().toString(),
                     foundHandle + 1, characteristic.getUuid().toString(),
-                    characteristic.getProperties(), characteristic.getValue());
+                    characteristic.getProperties(), value);
             } else {
                 // This must be in sync with QLowEnergyService::CharacteristicReadError
                 final int characteristicReadError = 5;
@@ -412,7 +414,8 @@ public class QtBluetoothLE {
     }
 
     private synchronized void handleOnCharacteristicChanged(android.bluetooth.BluetoothGatt gatt,
-                                    android.bluetooth.BluetoothGattCharacteristic characteristic)
+                                    android.bluetooth.BluetoothGattCharacteristic characteristic,
+                                    byte[] value)
     {
         int handle = handleForCharacteristic(characteristic);
         if (handle == -1) {
@@ -420,7 +423,7 @@ public class QtBluetoothLE {
             return;
         }
 
-        leCharacteristicChanged(qtObject, handle+1, characteristic.getValue());
+        leCharacteristicChanged(qtObject, handle+1, value);
     }
 
     private synchronized void handleOnCharacteristicWrite(android.bluetooth.BluetoothGatt gatt,
@@ -651,12 +654,24 @@ public class QtBluetoothLE {
 
         }
 
+        // API < 33
         public void onCharacteristicRead(android.bluetooth.BluetoothGatt gatt,
                                          android.bluetooth.BluetoothGattCharacteristic characteristic,
                                          int status)
         {
             super.onCharacteristicRead(gatt, characteristic, status);
-            handleOnCharacteristicRead(gatt, characteristic, status);
+            handleOnCharacteristicRead(gatt, characteristic, characteristic.getValue(), status);
+        }
+
+        // API >= 33
+        public void onCharacteristicRead(android.bluetooth.BluetoothGatt gatt,
+                                    android.bluetooth.BluetoothGattCharacteristic characteristic,
+                                    byte[] value,
+                                    int status)
+        {
+            // Note: here we don't call the super implementation as it calls the old "< API 33"
+            // callback, and the callback would be handled twice
+            handleOnCharacteristicRead(gatt, characteristic, value, status);
         }
 
         public void onCharacteristicWrite(android.bluetooth.BluetoothGatt gatt,
@@ -667,11 +682,22 @@ public class QtBluetoothLE {
             handleOnCharacteristicWrite(gatt, characteristic, status);
         }
 
+        // API < 33
         public void onCharacteristicChanged(android.bluetooth.BluetoothGatt gatt,
                                             android.bluetooth.BluetoothGattCharacteristic characteristic)
         {
             super.onCharacteristicChanged(gatt, characteristic);
-            handleOnCharacteristicChanged(gatt, characteristic);
+            handleOnCharacteristicChanged(gatt, characteristic, characteristic.getValue());
+        }
+
+        // API >= 33
+        public void onCharacteristicChanged(android.bluetooth.BluetoothGatt gatt,
+                                    android.bluetooth.BluetoothGattCharacteristic characteristic,
+                                    byte[] value)
+        {
+            // Note: here we don't call the super implementation as it calls the old "< API 33"
+            // callback, and the callback would be handled twice
+            handleOnCharacteristicChanged(gatt, characteristic, value);
         }
 
         public void onDescriptorRead(android.bluetooth.BluetoothGatt gatt,
@@ -1527,7 +1553,7 @@ public class QtBluetoothLE {
                                 + " for service " + entry.characteristic.getService().getUuid());
                             leCharacteristicRead(qtObject, entry.characteristic.getService().getUuid().toString(),
                                     handle + 1, entry.characteristic.getUuid().toString(),
-                                    entry.characteristic.getProperties(), entry.characteristic.getValue());
+                                    entry.characteristic.getProperties(), null);
                             break;
                         case Descriptor:
                             Log.d(TAG,
@@ -1594,6 +1620,11 @@ public class QtBluetoothLE {
         boolean result;
         switch (nextJob.entry.type) {
             case Characteristic:
+                if (Build.VERSION.SDK_INT >= 33) {
+                    int writeResult = mBluetoothGatt.writeCharacteristic(
+                       nextJob.entry.characteristic, nextJob.newValue, nextJob.requestedWriteType);
+                    return (writeResult != BluetoothStatusCodes.SUCCESS);
+                }
                 if (mHandler != null || mCharacteristicConstructor == null) {
                     if (nextJob.entry.characteristic.getWriteType() != nextJob.requestedWriteType) {
                         nextJob.entry.characteristic.setWriteType(nextJob.requestedWriteType);
diff --git a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLEServer.java b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLEServer.java
index f595dec0..6d4a1fa7 100644
--- a/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLEServer.java
+++ b/src/android/bluetooth/src/org/qtproject/qt/android/bluetooth/QtBluetoothLEServer.java
@@ -20,6 +20,7 @@ import android.bluetooth.le.AdvertiseData.Builder;
 import android.bluetooth.le.AdvertiseSettings;
 import android.bluetooth.le.BluetoothLeAdvertiser;
 import android.os.ParcelUuid;
+import android.os.Build;
 import android.util.Log;
 import android.util.Pair;
 
@@ -389,15 +390,18 @@ public class QtBluetoothLEServer {
             Log.w(TAG, "Ignoring characteristic read, server is disconnected");
             return;
         }
-        byte[] dataArray;
+
+        byte[] characteristicData =
+                                ((QtBluetoothGattCharacteristic)characteristic).getLocalValue();
+
         try {
-            dataArray = Arrays.copyOfRange(characteristic.getValue(),
-                                           offset, characteristic.getValue().length);
+            byte[] dataArray = Arrays.copyOfRange(characteristicData,
+                                                  offset, characteristicData.length);
             mGattServer.sendResponse(device, requestId, BluetoothGatt.GATT_SUCCESS,
                                      offset, dataArray);
         } catch (Exception ex) {
             Log.w(TAG, "onCharacteristicReadRequest: " + requestId + " "
-                        + offset + " " + characteristic.getValue().length);
+                        + offset + " " + characteristicData.length);
             ex.printStackTrace();
             mGattServer.sendResponse(device, requestId, BluetoothGatt.GATT_FAILURE,
                                      offset, null);
@@ -432,7 +436,7 @@ public class QtBluetoothLEServer {
                       + value.length + ", min: " + minValueLen + ", max: " + maxValueLen);
                 resultStatus = BluetoothGatt.GATT_INVALID_ATTRIBUTE_LENGTH;
             } else if (offset == 0) {
-                characteristic.setValue(value);
+                ((QtBluetoothGattCharacteristic)characteristic).setLocalValue(value);
                 leServerCharacteristicChanged(qtObject, characteristic, value);
                 sendNotificationOrIndication = true;
             } else {
@@ -559,8 +563,8 @@ public class QtBluetoothLEServer {
                 byte[] newValue = null;
                 // The target can be a descriptor or a characteristic
                 byte[] currentValue = (entry.target instanceof BluetoothGattCharacteristic)
-                                ? ((BluetoothGattCharacteristic)entry.target).getValue()
-                                : ((BluetoothGattDescriptor)entry.target).getValue();
+                                    ? ((QtBluetoothGattCharacteristic)entry.target).getLocalValue()
+                                    : ((BluetoothGattDescriptor)entry.target).getValue();
 
                 // Iterate writes and apply them to the currentValue in received order
                 for (Pair<byte[], Integer> write : entry.writes) {
@@ -607,7 +611,7 @@ public class QtBluetoothLEServer {
 
                 // Update value and inform the Qt/C++ side on the update
                 if (entry.target instanceof BluetoothGattCharacteristic) {
-                    ((BluetoothGattCharacteristic)entry.target).setValue(newValue);
+                    ((QtBluetoothGattCharacteristic)entry.target).setLocalValue(newValue);
                     leServerCharacteristicChanged(
                         qtObject, (BluetoothGattCharacteristic)entry.target, newValue);
                 } else {
@@ -819,12 +823,25 @@ public class QtBluetoothLEServer {
         //      devices at the same time.
         while (iter.hasNext()) {
             final BluetoothDevice device = iter.next();
-            final byte[] clientCharacteristicConfig = clientCharacteristicManager.valueFor(characteristic, device);
+            final byte[] clientCharacteristicConfig =
+                                      clientCharacteristicManager.valueFor(characteristic, device);
             if (clientCharacteristicConfig != null) {
-                if (Arrays.equals(clientCharacteristicConfig, BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE)) {
-                    mGattServer.notifyCharacteristicChanged(device, characteristic, false);
-                } else if (Arrays.equals(clientCharacteristicConfig, BluetoothGattDescriptor.ENABLE_INDICATION_VALUE)) {
-                    mGattServer.notifyCharacteristicChanged(device, characteristic, true);
+                if (Arrays.equals(clientCharacteristicConfig,
+                                  BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE)) {
+                    if (Build.VERSION.SDK_INT >= 33) {
+                        mGattServer.notifyCharacteristicChanged(device, characteristic, false,
+                                  ((QtBluetoothGattCharacteristic)characteristic).getLocalValue());
+                    } else {
+                        mGattServer.notifyCharacteristicChanged(device, characteristic, false);
+                    }
+                } else if (Arrays.equals(clientCharacteristicConfig,
+                                         BluetoothGattDescriptor.ENABLE_INDICATION_VALUE)) {
+                    if (Build.VERSION.SDK_INT >= 33) {
+                        mGattServer.notifyCharacteristicChanged(device, characteristic, true,
+                                  ((QtBluetoothGattCharacteristic)characteristic).getLocalValue());
+                    } else {
+                        mGattServer.notifyCharacteristicChanged(device, characteristic, true);
+                    }
                 }
             }
         }
@@ -832,7 +849,7 @@ public class QtBluetoothLEServer {
 
     /*
         Updates the local database value for the given characteristic with \a charUuid and
-        \a newValue. If notifications for this task are enabled an approproiate notification will
+        \a newValue. If notifications for this task are enabled an appropriate notification will
         be send to the remote client.
 
         This function is called from the Qt thread.
@@ -869,7 +886,7 @@ public class QtBluetoothLEServer {
 
         synchronized (this) // a value update might be in progress
         {
-            foundChar.setValue(newValue);
+            ((QtBluetoothGattCharacteristic)foundChar).setLocalValue(newValue);
             // Value is updated even if server is not connected, but notifying is not possible
             if (mGattServer != null)
                 sendNotificationsOrIndications(foundChar);
diff --git a/src/bluetooth/qlowenergycontroller_android.cpp b/src/bluetooth/qlowenergycontroller_android.cpp
index 2d7839ad..0350b163 100644
--- a/src/bluetooth/qlowenergycontroller_android.cpp
+++ b/src/bluetooth/qlowenergycontroller_android.cpp
@@ -1220,10 +1220,9 @@ void QLowEnergyControllerPrivateAndroid::addToGenericAttributeList(const QLowEne
         QJniEnvironment env;
         jbyteArray jb = env->NewByteArray(charData.value().size());
         env->SetByteArrayRegion(jb, 0, charData.value().size(), (jbyte*)charData.value().data());
-        jboolean success = javaChar.callMethod<jboolean>("setValue", jb);
+        jboolean success = javaChar.callMethod<jboolean>("setLocalValue", jb);
         if (!success)
             qCWarning(QT_BT_ANDROID) << "Cannot setup initial characteristic value for " << charData.uuid();
-
         env->DeleteLocalRef(jb);
 
         const QList<QLowEnergyDescriptorData> descriptorList = charData.descriptors();