Bluetooth LE - add readCharacteristic/readDescriptor (OS X/iOS)

Add read descriptor/characteristic support for iOS and OS X.

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

5 files changed, 311 insertions, 66 deletions

diff --git a/src/bluetooth/osx/osxbtcentralmanager.mm b/src/bluetooth/osx/osxbtcentralmanager.mm
index bacea96b..08905da8 100644
--- a/src/bluetooth/osx/osxbtcentralmanager.mm
+++ b/src/bluetooth/osx/osxbtcentralmanager.mm
@@ -92,6 +92,8 @@ using namespace QT_NAMESPACE;
 - (void)discoverIncludedServices;
 - (void)readCharacteristics:(CBService *)service;
 - (void)serviceDetailsDiscoveryFinished:(CBService *)service;
+- (void)performNextRequest;
+- (void)performNextReadRequest;
 - (void)performNextWriteRequest;
 
 // Aux. functions.
@@ -124,7 +126,8 @@ using namespace QT_NAMESPACE;
         delegate = aDelegate;
         currentService = 0;
         lastValidHandle = 0;
-        writePending = false;
+        requestPending = false;
+        currentReadHandle = 0;
     }
 
     return self;
@@ -540,22 +543,81 @@ using namespace QT_NAMESPACE;
     delegate->serviceDetailsDiscoveryFinished(qtService);
 }
 
-- (void)performNextWriteRequest
+- (void)performNextRequest
+{
+    using namespace OSXBluetooth;
+
+    if (requestPending || !requests.size())
+        return;
+
+    switch (requests.head().type) {
+    case LERequest::CharRead:
+    case LERequest::DescRead:
+        return [self performNextReadRequest];
+    case LERequest::CharWrite:
+    case LERequest::DescWrite:
+    case LERequest::ClientConfiguration:
+        return [self performNextWriteRequest];
+    default:
+        // Should never happen.
+        Q_ASSERT(0);
+    }
+}
+
+- (void)performNextReadRequest
 {
     using namespace OSXBluetooth;
 
     Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)");
+    Q_ASSERT_X(!requestPending, Q_FUNC_INFO, "processing another request");
+    Q_ASSERT_X(requests.size(), Q_FUNC_INFO, "no requests to handle");
+    Q_ASSERT_X(requests.head().type == LERequest::CharRead
+               || requests.head().type == LERequest::DescRead,
+               Q_FUNC_INFO, "not a read request");
+
+    const LERequest request(requests.dequeue());
+    if (request.type == LERequest::CharRead) {
+        if (!charMap.contains(request.handle)) {
+            qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "characteristic with handle"
+                                 << request.handle << "not found";
+            return [self performNextRequest];
+        }
 
-    if (writePending || !writeQueue.size())
-        return;
+        requestPending = true;
+        currentReadHandle = request.handle;
+        [peripheral readValueForCharacteristic:charMap[request.handle]];
+    } else {
+        if (!descMap.contains(request.handle)) {
+            qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "descriptor with handle"
+                                 << request.handle << "not found";
+            return [self performNextRequest];
+        }
+
+        requestPending = true;
+        currentReadHandle = request.handle;
+        [peripheral readValueForDescriptor:descMap[request.handle]];
+    }
+}
 
-    LEWriteRequest request(writeQueue.dequeue());
+- (void)performNextWriteRequest
+{
+    using namespace OSXBluetooth;
+
+    Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)");
+    Q_ASSERT_X(!requestPending, Q_FUNC_INFO, "processing another request");
+    Q_ASSERT_X(requests.size(), Q_FUNC_INFO, "no requests to handle");
+    Q_ASSERT_X(requests.head().type == LERequest::CharWrite
+               || requests.head().type == LERequest::DescWrite
+               || requests.head().type == LERequest::ClientConfiguration,
+               Q_FUNC_INFO, "not a write request");
 
-    if (request.isDescriptor) {
+    const LERequest request(requests.dequeue());
+
+    if (request.type == LERequest::DescWrite) {
         if (!descMap.contains(request.handle)) {
             qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "handle: "
                                  << request.handle << " not found";
-            return [self performNextWriteRequest];
+            return [self performNextRequest];
         }
 
         CBDescriptor *const descriptor = descMap[request.handle];
@@ -564,58 +626,58 @@ using namespace QT_NAMESPACE;
             // Even if qtData.size() == 0, we still need NSData object.
             qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "failed "
                                     "to allocate an NSData object";
-            return [self performNextWriteRequest];
+            return [self performNextRequest];
         }
 
         if (![self cacheWriteValue:request.value for:descriptor])
-            return [self performNextWriteRequest];
+            return [self performNextRequest];
 
-        writePending = true;
+        requestPending = true;
         return [peripheral writeValue:data.data() forDescriptor:descriptor];
     } else {
         if (!charMap.contains(request.handle)) {
             qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "characteristic with "
                                     "handle: " << request.handle << " not found";
-            return [self performNextWriteRequest];
+            return [self performNextRequest];
         }
 
         CBCharacteristic *const characteristic = charMap[request.handle];
 
-        if (request.isClientConfiguration) {
+        if (request.type == LERequest::ClientConfiguration) {
             const QBluetoothUuid qtUuid(QBluetoothUuid::ClientCharacteristicConfiguration);
             CBDescriptor *const descriptor = [self descriptor:qtUuid forCharacteristic:characteristic];
             Q_ASSERT_X(descriptor, Q_FUNC_INFO, "no client characteristic "
                        "configuration descriptor found");
 
             if (![self cacheWriteValue:request.value for:descriptor])
-                return [self performNextWriteRequest];
+                return [self performNextRequest];
 
             bool enable = false;
             if (request.value.size())
                 enable = request.value[0] & 3;
 
-            writePending = true;
+            requestPending = true;
             [peripheral setNotifyValue:enable forCharacteristic:characteristic];
         } else {
             ObjCStrongReference<NSData> data(data_from_bytearray(request.value));
             if (!data) {
                 // Even if qtData.size() == 0, we still need NSData object.
                 qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "failed to allocate NSData object";
-                return [self performNextWriteRequest];
+                return [self performNextRequest];
             }
 
             // TODO: check what happens if I'm using NSData with length 0.
             if (request.withResponse) {
                 if (![self cacheWriteValue:request.value for:characteristic])
-                    return [self performNextWriteRequest];
+                    return [self performNextRequest];
 
-                writePending = true;
+                requestPending = true;
                 [peripheral writeValue:data.data() forCharacteristic:characteristic
                             type:CBCharacteristicWriteWithResponse];
             } else {
                 [peripheral writeValue:data.data() forCharacteristic:characteristic
                             type:CBCharacteristicWriteWithoutResponse];
-                [self performNextWriteRequest];
+                [self performNextRequest];
             }
         }
     }
@@ -624,6 +686,8 @@ using namespace QT_NAMESPACE;
 - (bool)setNotifyValue:(const QByteArray &)value
         forCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
 {
+    using namespace OSXBluetooth;
+
     Q_ASSERT_X(charHandle, Q_FUNC_INFO, "invalid characteristic handle (0)");
     Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
 
@@ -643,17 +707,37 @@ using namespace QT_NAMESPACE;
         return false;
     }
 
-    OSXBluetooth::LEWriteRequest request;
-    request.isDescriptor = false;
-    request.isClientConfiguration = true;
+    LERequest request;
+    request.type = LERequest::ClientConfiguration;
     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.
+    requests.enqueue(request);
+    [self performNextRequest];
+
+    return true;
+}
+
+- (bool)readCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
+{
+    using namespace OSXBluetooth;
+
+    Q_ASSERT_X(charHandle, Q_FUNC_INFO, "invalid characteristic handle (0)");
+
+    QT_BT_MAC_AUTORELEASEPOOL;
+
+    if (!charMap.contains(charHandle)) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "characteristic: " << charHandle << " not found";
+        return false;
+    }
+
+    LERequest request;
+    request.type = LERequest::CharRead;
+    request.handle = charHandle;
+
+    requests.enqueue(request);
+    [self performNextRequest];
+
     return true;
 }
 
@@ -672,14 +756,36 @@ using namespace QT_NAMESPACE;
         return false;
     }
 
-    LEWriteRequest request;
-    request.isDescriptor = false;
+    LERequest request;
+    request.type = LERequest::CharWrite;
     request.withResponse = withResponse;
     request.handle = charHandle;
     request.value = value;
 
-    writeQueue.enqueue(request);
-    [self performNextWriteRequest];
+    requests.enqueue(request);
+    [self performNextRequest];
+
+    return true;
+}
+
+- (bool)readDescriptor:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))descHandle
+{
+    using namespace OSXBluetooth;
+
+    Q_ASSERT_X(descHandle, Q_FUNC_INFO, "invalid descriptor handle (0)");
+
+    if (!descMap.contains(descHandle)) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "handle:"
+                             << descHandle << "not found";
+        return false;
+    }
+
+    LERequest request;
+    request.type = LERequest::DescRead;
+    request.handle = descHandle;
+
+    requests.enqueue(request);
+    [self performNextRequest];
 
     return true;
 }
@@ -696,13 +802,13 @@ using namespace QT_NAMESPACE;
         return false;
     }
 
-    LEWriteRequest request;
-    request.isDescriptor = true;
+    LERequest request;
+    request.type = LERequest::DescWrite;
     request.handle = descHandle;
     request.value = value;
 
-    writeQueue.enqueue(request);
-    [self performNextWriteRequest];
+    requests.enqueue(request);
+    [self performNextRequest];
 
     return true;
 }
@@ -906,15 +1012,15 @@ using namespace QT_NAMESPACE;
 
 - (void)reset
 {
-    writePending = false;
+    requestPending = false;
     valuesToWrite.clear();
-    writeQueue.clear();
+    requests.clear();
     servicesToDiscoverDetails.clear();
     lastValidHandle = 0;
     serviceMap.clear();
     charMap.clear();
     descMap.clear();
-
+    currentReadHandle = 0;
     // TODO: also serviceToVisit/VisitNext and visitedServices ?
 }
 
@@ -1193,7 +1299,8 @@ using namespace QT_NAMESPACE;
     }
 }
 
-- (void)peripheral:(CBPeripheral *)aPeripheral didUpdateValueForCharacteristic:(CBCharacteristic *)characteristic
+- (void)peripheral:(CBPeripheral *)aPeripheral
+        didUpdateValueForCharacteristic:(CBCharacteristic *)characteristic
         error:(NSError *)error
 {
     Q_UNUSED(aPeripheral)
@@ -1202,18 +1309,25 @@ using namespace QT_NAMESPACE;
 
     Q_ASSERT_X(managerState != CentralManagerUpdating, Q_FUNC_INFO, "invalid state");
     Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)");
+    Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
 
     QT_BT_MAC_AUTORELEASEPOOL;
     // First, let's check if we're discovering a service details now.
     CBService *const service = characteristic.service;
     const QBluetoothUuid qtUuid(qt_uuid(service.UUID));
     const bool isDetailsDiscovery = servicesToDiscoverDetails.contains(qtUuid);
+    const QLowEnergyHandle chHandle = charMap.key(characteristic);
 
     if (error) {
         // Use NSLog, not qCDebug/qCWarning to log the actual error.
         NSLog(@"%s failed with error %@", Q_FUNC_INFO, error);
         if (!isDetailsDiscovery) {
-            // TODO: this can be something else in a future (if needed at all).
+            if (chHandle && chHandle == currentReadHandle) {
+                currentReadHandle = 0;
+                requestPending = false;
+                delegate->error(qtUuid, QLowEnergyService::CharacteristicReadError);
+                [self performNextRequest];
+            }
             return;
         }
     }
@@ -1228,7 +1342,6 @@ using namespace QT_NAMESPACE;
             [self discoverDescriptors:characteristic.service];
     } else {
         // 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
@@ -1240,9 +1353,17 @@ using namespace QT_NAMESPACE;
             return;
         }
 
-        Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
-        delegate->characteristicUpdateNotification(chHandle,
-                                                   qt_bytearray(characteristic.value));
+        if (currentReadHandle == chHandle) {
+            // Even if it was not a reply to our read request (no way to test it)
+            // report it.
+            requestPending = false;
+            currentReadHandle = 0;
+            //
+            delegate->characteristicReadNotification(chHandle, qt_bytearray(characteristic.value));
+            [self performNextRequest];
+        } else {
+            delegate->characteristicUpdateNotification(chHandle, qt_bytearray(characteristic.value));
+        }
     }
 }
 
@@ -1261,7 +1382,7 @@ using namespace QT_NAMESPACE;
     if (error) {
         // Log the error using NSLog:
         NSLog(@"%s failed with error %@", Q_FUNC_INFO, error);
-        // Probably, we can continue though ...
+        // We can continue though ...
     }
 
     // Do we have more characteristics on this service to discover descriptors?
@@ -1288,12 +1409,19 @@ using namespace QT_NAMESPACE;
     CBService *const service = descriptor.characteristic.service;
     const QBluetoothUuid qtUuid(qt_uuid(service.UUID));
     const bool isDetailsDiscovery = servicesToDiscoverDetails.contains(qtUuid);
+    const QLowEnergyHandle dHandle = descMap.key(descriptor);
 
     if (error) {
         // NSLog to log the actual error ...
         NSLog(@"%s failed with error %@", Q_FUNC_INFO, error);
+
         if (!isDetailsDiscovery) {
-            // TODO: probably will be required in a future.
+            if (dHandle && dHandle == currentReadHandle) {
+                currentReadHandle = 0;
+                requestPending = false;
+                delegate->error(qtUuid, QLowEnergyService::DescriptorReadError);
+                [self performNextRequest];
+            }
             return;
         }
     }
@@ -1322,7 +1450,18 @@ using namespace QT_NAMESPACE;
             [self serviceDetailsDiscoveryFinished:service];
         }
     } else {
-        // TODO: this can be something else in a future (if needed at all).
+        if (!dHandle) {
+            qCCritical(QT_BT_OSX) << Q_FUNC_INFO << "unexpected value update notification, "
+                                     "no descriptor handle found";
+            return;
+        }
+
+        if (dHandle == currentReadHandle) {
+            currentReadHandle = 0;
+            requestPending = false;
+            delegate->descriptorReadNotification(dHandle, qt_bytearray(static_cast<NSObject *>(descriptor.value)));
+            [self performNextRequest];
+        }
     }
 }
 
@@ -1344,7 +1483,7 @@ using namespace QT_NAMESPACE;
 
     QT_BT_MAC_AUTORELEASEPOOL;
 
-    writePending = false;
+    requestPending = false;
 
     Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
 
@@ -1368,7 +1507,7 @@ using namespace QT_NAMESPACE;
         delegate->characteristicWriteNotification(cHandle, valueToReport);
     }
 
-    [self performNextWriteRequest];
+    [self performNextRequest];
 }
 
 - (void)peripheral:(CBPeripheral *)aPeripheral
@@ -1383,7 +1522,7 @@ using namespace QT_NAMESPACE;
 
     QT_BT_MAC_AUTORELEASEPOOL;
 
-    writePending = false;
+    requestPending = false;
 
     using namespace OSXBluetooth;
 
@@ -1404,7 +1543,7 @@ using namespace QT_NAMESPACE;
         delegate->descriptorWriteNotification(dHandle, valueToReport);
     }
 
-    [self performNextWriteRequest];
+    [self performNextRequest];
 }
 
 - (void)peripheral:(CBPeripheral *)aPeripheral
@@ -1417,7 +1556,7 @@ using namespace QT_NAMESPACE;
 
     QT_BT_MAC_AUTORELEASEPOOL;
 
-    writePending = false;
+    requestPending = false;
 
     Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (nil)");
 
@@ -1446,7 +1585,7 @@ using namespace QT_NAMESPACE;
         }
     }
 
-    [self performNextWriteRequest];
+    [self performNextRequest];
 }
 
 @end
diff --git a/src/bluetooth/osx/osxbtcentralmanager_p.h b/src/bluetooth/osx/osxbtcentralmanager_p.h
index b1908a48..e8d7d234 100644
--- a/src/bluetooth/osx/osxbtcentralmanager_p.h
+++ b/src/bluetooth/osx/osxbtcentralmanager_p.h
@@ -72,10 +72,14 @@ public:
     virtual void connectSuccess() = 0;
     virtual void serviceDiscoveryFinished(LEServices services) = 0;
     virtual void serviceDetailsDiscoveryFinished(LEService service) = 0;
+    virtual void characteristicReadNotification(QLowEnergyHandle charHandle,
+                                                const QByteArray &value) = 0;
     virtual void characteristicWriteNotification(QLowEnergyHandle charHandle,
                                                  const QByteArray &value) = 0;
     virtual void characteristicUpdateNotification(QLowEnergyHandle charHandle,
                                                   const QByteArray &value) = 0;
+    virtual void descriptorReadNotification(QLowEnergyHandle descHandle,
+                                            const QByteArray &value) = 0;
     virtual void descriptorWriteNotification(QLowEnergyHandle descHandle,
                                              const QByteArray &value) = 0;
     virtual void disconnected() = 0;
@@ -111,23 +115,30 @@ typedef QHash<QLowEnergyHandle, CBService *> ServiceHash;
 typedef QHash<QLowEnergyHandle, CBCharacteristic *> CharHash;
 typedef QHash<QLowEnergyHandle, CBDescriptor *> DescHash;
 
-// Descriptor write request - we have to serialize 'concurrent' write requests.
-struct LEWriteRequest
-{
-    LEWriteRequest() : isDescriptor(false),
-                       isClientConfiguration(false),
-                       withResponse(false),
-                       handle(0)
+// Descriptor/charactesirsti read/write requests
+// - we have to serialize 'concurrent' write requests.
+struct LERequest {
+    enum RequestType {
+        CharRead,
+        CharWrite,
+        DescRead,
+        DescWrite,
+        ClientConfiguration,
+        Invalid
+    };
+
+    LERequest() : type(Invalid),
+                  withResponse(false),
+                  handle(0)
     {}
 
-    bool isDescriptor;
-    bool isClientConfiguration;
+    RequestType type;
     bool withResponse;
     QLowEnergyHandle handle;
     QByteArray value;
 };
 
-typedef QQueue<LEWriteRequest> WriteQueue;
+typedef QQueue<LERequest> RequestQueue;
 
 // Core Bluetooth's write confirmation does not provide
 // the updated value (characteristic or descriptor).
@@ -170,8 +181,9 @@ QT_END_NAMESPACE
 
     QT_PREPEND_NAMESPACE(QLowEnergyHandle) lastValidHandle;
 
-    bool writePending;
-    QT_PREPEND_NAMESPACE(OSXBluetooth)::WriteQueue writeQueue;
+    bool requestPending;
+    QT_PREPEND_NAMESPACE(OSXBluetooth)::RequestQueue requests;
+    QT_PREPEND_NAMESPACE(QLowEnergyHandle) currentReadHandle;
 
     QT_PREPEND_NAMESPACE(OSXBluetooth)::ValueHash valuesToWrite;
 }
@@ -190,10 +202,14 @@ QT_END_NAMESPACE
 - (bool)setNotifyValue:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
         forCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle;
 
+- (bool)readCharacteristic:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle;
+
 - (bool)write:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
         charHandle:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))charHandle
         withResponse:(bool)writeWithResponse;
 
+- (bool)readDescriptor:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))descHandle;
+
 - (bool)write:(const QT_PREPEND_NAMESPACE(QByteArray) &)value
         descHandle:(QT_PREPEND_NAMESPACE(QLowEnergyHandle))descHandle;
 @end
diff --git a/src/bluetooth/qlowenergycontroller_osx.mm b/src/bluetooth/qlowenergycontroller_osx.mm
index 80a94d64..e3cf57b1 100644
--- a/src/bluetooth/qlowenergycontroller_osx.mm
+++ b/src/bluetooth/qlowenergycontroller_osx.mm
@@ -308,6 +308,27 @@ void QLowEnergyControllerPrivateOSX::serviceDetailsDiscoveryFinished(LEService s
     qtService->setState(QLowEnergyService::ServiceDiscovered);
 }
 
+void QLowEnergyControllerPrivateOSX::characteristicReadNotification(QLowEnergyHandle charHandle,
+                                                                    const QByteArray &value)
+{
+    Q_ASSERT_X(charHandle, Q_FUNC_INFO, "invalid characteristic handle(0)");
+
+    ServicePrivate service(serviceForHandle(charHandle));
+    if (service.isNull())
+        return;
+
+    QLowEnergyCharacteristic characteristic(characteristicForHandle(charHandle));
+    if (!characteristic.isValid()) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "unknown characteristic";
+        return;
+    }
+
+    if (characteristic.properties() & QLowEnergyCharacteristic::Read)
+        updateValueOfCharacteristic(charHandle, value, false);
+
+    emit service->characteristicRead(characteristic, value);
+}
+
 void QLowEnergyControllerPrivateOSX::characteristicWriteNotification(QLowEnergyHandle charHandle,
                                                                      const QByteArray &value)
 {
@@ -361,6 +382,21 @@ void QLowEnergyControllerPrivateOSX::characteristicUpdateNotification(QLowEnergy
     emit service->characteristicChanged(characteristic, value);
 }
 
+void QLowEnergyControllerPrivateOSX::descriptorReadNotification(QLowEnergyHandle dHandle, const QByteArray &value)
+{
+    Q_ASSERT_X(dHandle, Q_FUNC_INFO, "invalid descriptor handle (0)");
+
+    const QLowEnergyDescriptor qtDescriptor(descriptorForHandle(dHandle));
+    if (!qtDescriptor.isValid()) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "unknown descriptor " << dHandle;
+        return;
+    }
+
+    ServicePrivate service(serviceForHandle(qtDescriptor.characteristicHandle()));
+    updateValueOfDescriptor(qtDescriptor.characteristicHandle(), dHandle, value, false);
+    emit service->descriptorRead(qtDescriptor, value);
+}
+
 void QLowEnergyControllerPrivateOSX::descriptorWriteNotification(QLowEnergyHandle dHandle, const QByteArray &value)
 {
     Q_ASSERT_X(dHandle, Q_FUNC_INFO, "invalid descriptor handle (0)");
@@ -549,6 +585,28 @@ void QLowEnergyControllerPrivateOSX::setNotifyValue(QSharedPointer<QLowEnergySer
         service->setError(QLowEnergyService::DescriptorWriteError);
 }
 
+void QLowEnergyControllerPrivateOSX::readCharacteristic(QSharedPointer<QLowEnergyServicePrivate> service,
+                                                        QLowEnergyHandle charHandle)
+{
+    Q_ASSERT_X(!service.isNull(), Q_FUNC_INFO, "invalid service (null)");
+    Q_ASSERT_X(isValid(), Q_FUNC_INFO, "invalid controller");
+
+    if (!discoveredServices.contains(service->uuid)) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "no service with uuid:"
+                             << service->uuid << "found";
+        return;
+    }
+
+    if (!service->characteristicList.contains(charHandle)) {
+        qCDebug(QT_BT_OSX) << Q_FUNC_INFO << "no characteristic with handle:"
+                           << charHandle << "found";
+        return;
+    }
+
+    if (![centralManager readCharacteristic:charHandle])
+        service->setError(QLowEnergyService::CharacteristicReadError);
+}
+
 void QLowEnergyControllerPrivateOSX::writeCharacteristic(QSharedPointer<QLowEnergyServicePrivate> service,
                                                          QLowEnergyHandle charHandle, const QByteArray &newValue,
                                                          bool writeWithResponse)
@@ -595,6 +653,22 @@ quint16 QLowEnergyControllerPrivateOSX::updateValueOfCharacteristic(QLowEnergyHa
     return 0;
 }
 
+void QLowEnergyControllerPrivateOSX::readDescriptor(QSharedPointer<QLowEnergyServicePrivate> service,
+                                                    QLowEnergyHandle descriptorHandle)
+{
+    Q_ASSERT_X(!service.isNull(), Q_FUNC_INFO, "invalid service (null)");
+    Q_ASSERT_X(isValid(), Q_FUNC_INFO, "invalid controller");
+
+    if (!discoveredServices.contains(service->uuid)) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "no service with uuid:"
+                             << service->uuid << "found";
+        return;
+    }
+
+    if (![centralManager readDescriptor:descriptorHandle])
+        service->setError(QLowEnergyService::DescriptorReadError);
+}
+
 void QLowEnergyControllerPrivateOSX::writeDescriptor(QSharedPointer<QLowEnergyServicePrivate> service,
                                                      QLowEnergyHandle descriptorHandle,
                                                      const QByteArray &newValue)
diff --git a/src/bluetooth/qlowenergycontroller_osx_p.h b/src/bluetooth/qlowenergycontroller_osx_p.h
index d9457217..96af6db0 100644
--- a/src/bluetooth/qlowenergycontroller_osx_p.h
+++ b/src/bluetooth/qlowenergycontroller_osx_p.h
@@ -72,10 +72,14 @@ private:
 
     void serviceDiscoveryFinished(LEServices services) Q_DECL_OVERRIDE;
     void serviceDetailsDiscoveryFinished(LEService service) Q_DECL_OVERRIDE;
+    void characteristicReadNotification(QLowEnergyHandle charHandle,
+                                        const QByteArray &value) Q_DECL_OVERRIDE;
     void characteristicWriteNotification(QLowEnergyHandle charHandle,
                                          const QByteArray &newValue) Q_DECL_OVERRIDE;
     void characteristicUpdateNotification(QLowEnergyHandle charHandle,
                                           const QByteArray &value) Q_DECL_OVERRIDE;
+    void descriptorReadNotification(QLowEnergyHandle descHandle,
+                                    const QByteArray &value) Q_DECL_OVERRIDE;
     void descriptorWriteNotification(QLowEnergyHandle descHandle,
                                      const QByteArray &newValue) Q_DECL_OVERRIDE;
     void disconnected() Q_DECL_OVERRIDE;
@@ -89,9 +93,14 @@ private:
     void discoverServices();
     void discoverServiceDetails(const QBluetoothUuid &serviceUuid);
 
+    // TODO: all these read/write /setNotify can be simplified -
+    // by just passing either characteristic or descriptor (that
+    // has all needed information - service, handle, etc.).
     void setNotifyValue(QSharedPointer<QLowEnergyServicePrivate> service,
                         QLowEnergyHandle charHandle, const QByteArray &newValue);
 
+    void readCharacteristic(QSharedPointer<QLowEnergyServicePrivate> service,
+                            QLowEnergyHandle charHandle);
     void writeCharacteristic(QSharedPointer<QLowEnergyServicePrivate> service,
                              QLowEnergyHandle charHandle, const QByteArray &newValue,
                              bool writeWithResponse);
@@ -100,6 +109,8 @@ private:
                                         const QByteArray &value,
                                         bool appendValue);
 
+    void readDescriptor(QSharedPointer<QLowEnergyServicePrivate> service,
+                        QLowEnergyHandle charHandle);
     void writeDescriptor(QSharedPointer<QLowEnergyServicePrivate> service,
                          QLowEnergyHandle descriptorHandle,
                          const QByteArray &newValue);
diff --git a/src/bluetooth/qlowenergyservice_osx.mm b/src/bluetooth/qlowenergyservice_osx.mm
index b5d89dd7..b424dc96 100644
--- a/src/bluetooth/qlowenergyservice_osx.mm
+++ b/src/bluetooth/qlowenergyservice_osx.mm
@@ -76,6 +76,11 @@ QLowEnergyService::QLowEnergyService(QSharedPointer<QLowEnergyServicePrivate> d,
             this, SIGNAL(characteristicWritten(QLowEnergyCharacteristic, QByteArray)));
     connect(d.data(), SIGNAL(descriptorWritten(QLowEnergyDescriptor, QByteArray)),
             this, SIGNAL(descriptorWritten(QLowEnergyDescriptor, QByteArray)));
+    connect(d.data(), SIGNAL(characteristicRead(QLowEnergyCharacteristic,QByteArray)),
+            this, SIGNAL(characteristicRead(QLowEnergyCharacteristic,QByteArray)));
+    connect(d.data(), SIGNAL(descriptorRead(QLowEnergyDescriptor,QByteArray)),
+            this, SIGNAL(descriptorRead(QLowEnergyDescriptor,QByteArray)));
+
 }
 
 QLowEnergyService::~QLowEnergyService()
@@ -185,7 +190,7 @@ void QLowEnergyService::readCharacteristic(const QLowEnergyCharacteristic &chara
         return;
     }
 
-    //TODO implement QLowEnergyService::readCharacteristic() on iOS/OSX
+    controller->readCharacteristic(characteristic.d_ptr, characteristic.attributeHandle());
 }
 
 
@@ -234,7 +239,7 @@ void QLowEnergyService::readDescriptor(const QLowEnergyDescriptor &descriptor)
         return;
     }
 
-    //TODO implement QLowEnergyService::readDescriptor() on iOS/OSX
+    controller->readDescriptor(descriptor.d_ptr, descriptor.handle());
 }
 
 void QLowEnergyService::writeDescriptor(const QLowEnergyDescriptor &descriptor,