QLowEnergyService - ClientCharacteristicConfiguration

Core Bluetooth has a special method to enable/disable char update notifications and
even more - Core Bluetooth does not allow to use writeValue:forDescriptor:
with ClientCharacteristicConfiguration - that's why need this 'workaround'.

Change-Id: I4a01690a76aabf62397321ca6ba22c4abb1c420c
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

5 files changed, 238 insertions, 36 deletions

diff --git a/src/bluetooth/osx/osxbtcentralmanager.mm b/src/bluetooth/osx/osxbtcentralmanager.mm
index 2b4a2dd4..73d9d5bc 100644
--- a/src/bluetooth/osx/osxbtcentralmanager.mm
+++ b/src/bluetooth/osx/osxbtcentralmanager.mm
@@ -110,7 +110,8 @@ using namespace QT_NAMESPACE;
                       withProperties:(CBCharacteristicProperties)properties;
 - (CBDescriptor *)nextDescriptorForCharacteristic:(CBCharacteristic *)characteristic
                   startingFrom:(CBDescriptor *)descriptor;
-
+- (CBDescriptor *)descriptor:(const QBluetoothUuid &)dUuid
+                  forCharacteristic:(CBCharacteristic *)ch;
 // TODO: check _what_ exactly I have to reset ...
 - (void)reset;
 
@@ -527,7 +528,7 @@ using namespace QT_NAMESPACE;
                     // Create a Qt's internal descriptor:
                     QLowEnergyServicePrivate::DescData newDesc = {};
                     newDesc.uuid = qt_uuid(d.UUID);
-                    newDesc.value = qt_bytearray(d.value);
+                    newDesc.value = qt_bytearray(static_cast<NSObject *>(d.value));
                     descList[lastValidHandle] = newDesc;
                 }
 
@@ -580,27 +581,72 @@ using namespace QT_NAMESPACE;
         CBCharacteristic *const ch = charMap[request.handle];
         Q_ASSERT_X(ch, "-performNextWriteRequest", "invalid characteristic (nil)");
 
-        ObjCStrongReference<NSData> data(data_from_bytearray(request.value));
-        if (!data) {
-            // Even if qtData.size() == 0, we still need NSData object.
-            qCWarning(QT_BT_OSX) << "-performNextWriteRequest, "
-                                    "failed to allocate NSData object";
-            [self performNextWriteRequest];
-        }
-
-        // TODO: check what happens if I'm using NSData with length 0.
-        if (request.withResponse) {
+        if (request.isClientConfiguration) {
+            if (valuesToWrite.remove(request.handle)) {
+                // It can happen if something went wrong - we
+                // tried to set notification value and never received
+                // a callback.
+                qCDebug(QT_BT_OSX) << "-performNextWriteRequest:, "
+                                      "valuesToWrite already contains "
+                                      "a value for a given client configuration "
+                                      "descriptor, replacing it";
+            }
+            // We save the original value to report it later ...
+            valuesToWrite[request.handle] = request.value;
+            const bool enable = request.value[0] & 3;
             writePending = true;
-            [peripheral writeValue:data.data() forCharacteristic:ch
-                        type:CBCharacteristicWriteWithResponse];
+            [peripheral setNotifyValue:enable forCharacteristic:ch];
         } else {
-            [peripheral writeValue:data.data() forCharacteristic:ch
-                        type:CBCharacteristicWriteWithoutResponse];
-            [self performNextWriteRequest];
+            ObjCStrongReference<NSData> data(data_from_bytearray(request.value));
+            if (!data) {
+                // Even if qtData.size() == 0, we still need NSData object.
+                qCWarning(QT_BT_OSX) << "-performNextWriteRequest, "
+                                        "failed to allocate NSData object";
+                [self performNextWriteRequest];
+            }
+
+            // TODO: check what happens if I'm using NSData with length 0.
+            if (request.withResponse) {
+                NSLog(@"trying to write %@", data.data());
+                NSLog(@"initial value: %@", ch.value);
+                writePending = true;
+                [peripheral writeValue:data.data() forCharacteristic:ch
+                            type:CBCharacteristicWriteWithResponse];
+            } else {
+                [peripheral writeValue:data.data() forCharacteristic:ch
+                            type:CBCharacteristicWriteWithoutResponse];
+                [self performNextWriteRequest];
+            }
         }
     }
 }
 
+- (bool)setNotifyValue:(const QByteArray &)value
+        forCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
+{
+    Q_ASSERT_X(charHandle, "-setNotifyValue:forCharacteristic:",
+               "invalid characteristic handle (0)");
+
+    if (!charMap.contains(charHandle)) {
+        qCWarning(QT_BT_OSX) << "-setNotifyValue:forCharacteristic:, "
+                                "unknown characteristic handle " << charHandle;
+        return false;
+    }
+
+    OSXBluetooth::LEWriteRequest request;
+    request.isDescriptor = false;
+    request.isClientConfiguration = true;
+    request.handle = charHandle;
+    request.value = value;
+
+    writeQueue.enqueue(request);
+    [self performNextWriteRequest];
+    // TODO: check if I need a special map - to reset notify to NO
+    // before disconnect/dealloc later! Also, this can be needed if notify was set
+    // to YES from another application - to be tested.
+    return true;
+}
+
 - (bool)write:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
         charHandle:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
         withResponse:(bool)withResponse
@@ -837,15 +883,40 @@ using namespace QT_NAMESPACE;
     return [ds objectAtIndex:1];
 }
 
+- (CBDescriptor *)descriptor:(const QBluetoothUuid &)qtUuid
+                  forCharacteristic:(CBCharacteristic *)ch
+{
+    if (qtUuid.isNull() || !ch)
+        return nil;
+
+    QT_BT_MAC_AUTORELEASEPOOL;
+
+    CBDescriptor *descriptor = nil;
+    NSArray *const ds = ch.descriptors;
+    if (ds && ds.count) {
+        for (CBDescriptor *d in ds) {
+            if (OSXBluetooth::equal_uuids(d.UUID, qtUuid)) {
+                descriptor = d;
+                break;
+            }
+        }
+    }
+
+    return descriptor;
+}
+
 - (void)reset
 {
     writePending = false;
+    valuesToWrite.clear();
     writeQueue.clear();
     servicesToDiscoverDetails.clear();
     lastValidHandle = 0;
     serviceMap.clear();
     charMap.clear();
     descMap.clear();
+
+    // TODO: also serviceToVisit/VisitNext and visitedServices ?
 }
 
 // CBCentralManagerDelegate (the real one).
@@ -990,6 +1061,9 @@ using namespace QT_NAMESPACE;
     Q_ASSERT_X(delegate, "-centralManager:didDisconnectPeripheral:error:",
                "invalid delegate (null)");
 
+    // Clear internal caches/data.
+    [self reset];
+
     if (error && managerState == OSXBluetooth::CentralManagerDisconnecting) {
         managerState = OSXBluetooth::CentralManagerIdle;
         qCWarning(QT_BT_OSX) << "-centralManager:didDisconnectPeripheral:, "
@@ -1168,7 +1242,23 @@ using namespace QT_NAMESPACE;
         else
             [self discoverDescriptors:characteristic.service];
     } else {
-        // TODO: this can be something else in a future (if needed at all).
+        // This is (probably) the result of update notification.
+        const QLowEnergyHandle chHandle = charMap.key(characteristic);
+        // It's very possible we can have an invalid handle here (0) -
+        // if something esle is wrong (we subscribed for a notification),
+        // disconnected (but other application is connected) and still receiveing
+        // updated values ...
+        // TODO: this must be properly tested.
+        if (!chHandle) {
+            qCCritical(QT_BT_OSX) << "-peripheral:didUpdateValueForCharacteristic:error:, "
+                                     "unexpected update notification, no characteristic handle found";
+            return;
+        }
+
+        Q_ASSERT_X(delegate, "-peripheral:didUpdateValueForCharacteristic:error:",
+                   "invalid delegate (null)");
+        delegate->characteristicUpdateNotification(chHandle,
+                                                   qt_bytearray(characteristic.value));
     }
 }
 
@@ -1271,6 +1361,8 @@ using namespace QT_NAMESPACE;
 
     using namespace OSXBluetooth;
 
+    QT_BT_MAC_AUTORELEASEPOOL;
+
     writePending = false;
 
     Q_ASSERT_X(delegate, "-peripheral:didWriteValueForCharacteristic:error",
@@ -1289,6 +1381,7 @@ using namespace QT_NAMESPACE;
         const QLowEnergyHandle cHandle = charMap.key(characteristic);
         Q_ASSERT_X(cHandle, "-peripheral:didWriteValueForCharacteristic:error",
                    "invalid handle, not found in the characteristics map");
+        NSLog(@"characteristic written, the value is %@", characteristic.value);
         delegate->characteristicWriteNotification(cHandle, qt_bytearray(characteristic.value));
     }
 
@@ -1303,8 +1396,12 @@ using namespace QT_NAMESPACE;
 
     using namespace OSXBluetooth;
 
+    QT_BT_MAC_AUTORELEASEPOOL;
+
     writePending = false;
 
+    using namespace OSXBluetooth;
+
     if (error) {
         // NSLog to log the actual NSError:
         NSLog(@"-peripheral:didWriteValueForDescriptor:error:, failed with error %@",
@@ -1319,7 +1416,58 @@ using namespace QT_NAMESPACE;
         const QLowEnergyHandle dHandle = descMap.key(descriptor);
         Q_ASSERT_X(dHandle, "-peripheral:didWriteValueForDescriptor:error:",
                    "invalid descriptor, not found in the descriptors map");
-        delegate->descriptorWriteNotification(dHandle, qt_bytearray(descriptor.value));
+        delegate->descriptorWriteNotification(dHandle, qt_bytearray(static_cast<NSObject *>(descriptor.value)));
+    }
+
+    [self performNextWriteRequest];
+}
+
+- (void)peripheral:(CBPeripheral *)aPeripheral
+        didUpdateNotificationStateForCharacteristic:(CBCharacteristic *)characteristic
+        error:(NSError *)error
+{
+    Q_UNUSED(aPeripheral)
+
+    using namespace OSXBluetooth;
+
+    QT_BT_MAC_AUTORELEASEPOOL;
+
+    writePending = false;
+
+    Q_ASSERT_X(delegate, "-peripheral:didUpdateNotificationStateForCharacteristic:",
+               "invalid delegate (nil)");
+
+    const QBluetoothUuid qtUuid(QBluetoothUuid::ClientCharacteristicConfiguration);
+    CBDescriptor *const descriptor = [self descriptor:qtUuid forCharacteristic:characteristic];
+    const QLowEnergyHandle dHandle = descMap.key(descriptor);// 0 if descriptor is nil or unknown.
+    const QByteArray valueToReport(valuesToWrite.value(dHandle, QByteArray()));
+
+    if (!valuesToWrite.remove(dHandle)) {
+        // In future it can be a special case: we can, in principle,
+        // set notify value on a characteristic without 'writeDescriptor'.
+        // It can also be some error. Right now we report it as error, can change.
+        qCWarning(QT_BT_OSX) << "-peripheral:didUpdateNotificationStateForCharacteristic:, "
+                                "setNotifyValue called, but no client characteristic descriptor "
+                                "found or no writeDescriptor call";
+    }
+
+    if (error) {
+        // NSLog to log the actual NSError:
+        NSLog(@"-peripheral:didUpdateNotificationStateForCharacteristic:, failed with error %@",
+              error);
+        delegate->error(qt_uuid(characteristic.service.UUID), 0,
+                        // In Qt's API it's a descriptor write actually.
+                        QLowEnergyService::DescriptorWriteError);
+    } else {
+        if (!valueToReport.isNull()) {
+            delegate->descriptorWriteNotification(dHandle, valueToReport);
+        } else {
+            // TODO: can we in future have another way to set notify value without writeDescriptor?
+            qCWarning(QT_BT_OSX) << "-peripheral:didUpdateNotificationStateForCharacteristic:, "
+                                    "notification value set to " << int(characteristic.isNotifying)
+                                 << " but no client characteristic configuration descriptor found"
+                                    "or no previous writeDescriptor request found";
+        }
     }
 
     [self performNextWriteRequest];
diff --git a/src/bluetooth/osx/osxbtcentralmanager_p.h b/src/bluetooth/osx/osxbtcentralmanager_p.h
index 86c621c2..6d93816f 100644
--- a/src/bluetooth/osx/osxbtcentralmanager_p.h
+++ b/src/bluetooth/osx/osxbtcentralmanager_p.h
@@ -82,6 +82,8 @@ public:
     virtual void serviceDetailsDiscoveryFinished(LEService service) = 0;
     virtual void characteristicWriteNotification(QLowEnergyHandle charHandle,
                                                  const QByteArray &value) = 0;
+    virtual void characteristicUpdateNotification(QLowEnergyHandle charHandle,
+                                                  const QByteArray &value) = 0;
     virtual void descriptorWriteNotification(QLowEnergyHandle descHandle,
                                              const QByteArray &value) = 0;
     virtual void disconnected() = 0;
@@ -122,11 +124,13 @@ typedef QHash<QLowEnergyHandle, CBDescriptor *> DescHash;
 struct LEWriteRequest
 {
     LEWriteRequest() : isDescriptor(false),
+                       isClientConfiguration(false),
                        withResponse(false),
                        handle(0)
     {}
 
     bool isDescriptor;
+    bool isClientConfiguration;
     bool withResponse;
     QLowEnergyHandle handle;
     QByteArray value;
@@ -134,6 +138,16 @@ struct LEWriteRequest
 
 typedef QQueue<LEWriteRequest> WriteQueue;
 
+// It can happen that Qt's API wants to write something
+// and expects the confirmation about this value written,
+// but under the hood (Core Bluetooth) we have something like
+// a special method without any values at all.
+// To report our user a successful write, we have this map:
+// handle -> value for a write operation.
+// Since write operations are serialized, the key is guaranteed
+// to be unique.
+typedef QHash<QLowEnergyHandle, QByteArray> ValueHash;
+
 }
 
 QT_END_NAMESPACE
@@ -169,6 +183,8 @@ QT_END_NAMESPACE
 
     bool writePending;
     QT_PREPEND_NAMESPACE(OSXBluetooth)::WriteQueue writeQueue;
+
+    QT_PREPEND_NAMESPACE(OSXBluetooth)::ValueHash valuesToWrite;
 }
 
 - (id)initWithDelegate:(QT_PREPEND_NAMESPACE(OSXBluetooth)::CentralManagerDelegate *)aDelegate;
@@ -182,6 +198,9 @@ QT_END_NAMESPACE
 - (void)discoverServices;
 - (bool)discoverServiceDetails:(const QT_PREPEND_NAMESPACE(QBluetoothUuid) &)serviceUuid;
 
+- (bool)setNotifyValue:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
+        forCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle;
+
 - (bool)write:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
         charHandle:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
         withResponse:(bool)writeWithResponse;
diff --git a/src/bluetooth/qlowenergycontroller_osx.mm b/src/bluetooth/qlowenergycontroller_osx.mm
index 365f918b..48e144cc 100644
--- a/src/bluetooth/qlowenergycontroller_osx.mm
+++ b/src/bluetooth/qlowenergycontroller_osx.mm
@@ -332,6 +332,32 @@ void QLowEnergyControllerPrivateOSX::characteristicWriteNotification(QLowEnergyH
     emit service->characteristicWritten(characteristic, value);
 }
 
+void QLowEnergyControllerPrivateOSX::characteristicUpdateNotification(QLowEnergyHandle charHandle,
+                                                                     const QByteArray &value)
+{
+    // TODO: write/update notifications are quite similar (except asserts/warnings messages
+    // and different signals emitted). Merge them into one function?
+    Q_ASSERT_X(charHandle, "characteristicUpdateNotification",
+               "invalid characteristic handle(0)");
+
+    ServicePrivate service(serviceForHandle(charHandle));
+    if (service.isNull()) {
+        qCWarning(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::characteristicUpdateNotification(), "
+                                "can not find service for characteristic handle " << charHandle;
+        return;
+    }
+
+    QLowEnergyCharacteristic characteristic(characteristicForHandle(charHandle));
+    if (!characteristic.isValid()) {
+        qCWarning(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::characteristicUpdateNotification(), "
+                                "unknown characteristic";
+        return;
+    }
+
+    updateValueOfCharacteristic(charHandle, value, false);
+    emit service->characteristicChanged(characteristic, value);
+}
+
 void QLowEnergyControllerPrivateOSX::descriptorWriteNotification(QLowEnergyHandle dHandle, const QByteArray &value)
 {
     Q_ASSERT_X(dHandle, "descriptorWriteNotification", "invalid descriptor handle (0)");
@@ -492,11 +518,26 @@ void QLowEnergyControllerPrivateOSX::discoverServiceDetails(const QBluetoothUuid
 }
 
 void QLowEnergyControllerPrivateOSX::setNotifyValue(QSharedPointer<QLowEnergyServicePrivate> service,
-                                                    QLowEnergyHandle charHandle, const QByteArray &newValue)
+                                                    QLowEnergyHandle charHandle,
+                                                    const QByteArray &newValue)
 {
-    Q_UNUSED(service)
-    Q_UNUSED(charHandle)
-    Q_UNUSED(newValue)
+    Q_ASSERT_X(!service.isNull(), "setNotifyValue", "invalid service (null)");
+    Q_ASSERT_X(isValid(), "setNotifyValue", "invalid controller");
+
+    if (!discoveredServices.contains(service->uuid)) {
+        qCWarning(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::setNotifyValue(), "
+                                "no service with uuid: " << service->uuid << " found";
+        return;
+    }
+
+    if (!service->characteristicList.contains(charHandle)) {
+        qCDebug(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::setNotifyValue(), "
+                              "no characteristic with handle: " << charHandle << " found";
+        return;
+    }
+
+    if (![centralManager setNotifyValue:newValue forCharacteristic:charHandle])
+        service->setError(QLowEnergyService::DescriptorWriteError);
 }
 
 void QLowEnergyControllerPrivateOSX::writeCharacteristic(QSharedPointer<QLowEnergyServicePrivate> service,
@@ -504,12 +545,7 @@ void QLowEnergyControllerPrivateOSX::writeCharacteristic(QSharedPointer<QLowEner
                                                          bool writeWithResponse)
 {
     Q_ASSERT_X(!service.isNull(), "writeCharacteristic", "invalid service (null)");
-
-    if (!isValid()) {
-        qCWarning(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::writeCharacteristic(), "
-                                "invalid controller";
-        return;
-    }
+    Q_ASSERT_X(isValid(), "writeCharacteristic", "invalid controller");
 
     // We can work only with services, found on a given peripheral
     // (== created by the given LE controller),
@@ -555,12 +591,7 @@ void QLowEnergyControllerPrivateOSX::writeDescriptor(QSharedPointer<QLowEnergySe
                                                      const QByteArray &newValue)
 {
     Q_ASSERT_X(!service.isNull(), "writeDescriptor", "invalid service (null)");
-
-    if (!isValid()) {
-        qCWarning(QT_BT_OSX) << "QLowEnergyControllerPrivateOSX::writeDescriptor(), "
-                                "invalid controller";
-        return;
-    }
+    Q_ASSERT_X(isValid(), "writeDescriptor", "invalid controller");
 
     // We can work only with services found on a given peripheral
     // (== created by the given LE controller),
diff --git a/src/bluetooth/qlowenergycontroller_osx_p.h b/src/bluetooth/qlowenergycontroller_osx_p.h
index c723b1a5..dbf22776 100644
--- a/src/bluetooth/qlowenergycontroller_osx_p.h
+++ b/src/bluetooth/qlowenergycontroller_osx_p.h
@@ -74,6 +74,8 @@ private:
     void serviceDetailsDiscoveryFinished(LEService service) Q_DECL_OVERRIDE;
     void characteristicWriteNotification(QLowEnergyHandle charHandle,
                                          const QByteArray &newValue) Q_DECL_OVERRIDE;
+    void characteristicUpdateNotification(QLowEnergyHandle charHandle,
+                                          const QByteArray &value) Q_DECL_OVERRIDE;
     void descriptorWriteNotification(QLowEnergyHandle descHandle,
                                      const QByteArray &newValue) Q_DECL_OVERRIDE;
     void disconnected() Q_DECL_OVERRIDE;
diff --git a/src/bluetooth/qlowenergyservice_osx.mm b/src/bluetooth/qlowenergyservice_osx.mm
index 05763811..cb1c1f30 100644
--- a/src/bluetooth/qlowenergyservice_osx.mm
+++ b/src/bluetooth/qlowenergyservice_osx.mm
@@ -236,7 +236,8 @@ void QLowEnergyService::writeDescriptor(const QLowEnergyDescriptor &descriptor,
     }
 
     if (descriptor.uuid() == QBluetoothUuid::ClientCharacteristicConfiguration) {
-        // Core Bluetooth:
+        // We have to identify a special case - ClientCharacteristicConfiguration
+        // since with Core Bluetooth:
         //
         // "You cannot use this method to write the value of a client configuration descriptor
         // (represented by the CBUUIDClientCharacteristicConfigurationString constant),
@@ -244,6 +245,7 @@ void QLowEnergyService::writeDescriptor(const QLowEnergyDescriptor &descriptor,
         // characteristic’s value with respect to a client. If you want to manage
         // notifications or indications for a characteristic’s value, you must
         // use the setNotifyValue:forCharacteristic: method instead."
+        controller->setNotifyValue(descriptor.d_ptr, descriptor.characteristicHandle(), newValue);
     } else {
         controller->writeDescriptor(descriptor.d_ptr, descriptor.handle(), newValue);
     }