Simplify UUID handling, CBUUID does most of it already

src/controller/python/chip/ble/darwin/Scanning.mm

@@ -1,7 +1,7 @@
 #include <ble/Ble.h>
 #include <lib/support/CHIPMem.h>
 #include <lib/support/logging/CHIPLogging.h>
-#include <platform/Darwin/MTRUUIDHelper.h>
+#include <platform/Darwin/BleUtils.h>
 
 #import <CoreBluetooth/CoreBluetooth.h>
 
@@ -45,7 +45,7 @@ - (id)initWithContext:(PyObject *)context
 {
     self = [super init];
     if (self) {
-        self.shortServiceUUID = [MTRUUIDHelper GetShortestServiceUUID:&chip::Ble::CHIP_BLE_SVC_ID];
+        self.shortServiceUUID = chip::DeviceLayer::Internal::CBUUIDFromBleUUID(chip::Ble::CHIP_BLE_SVC_ID);
 
         _workQueue = dispatch_queue_create("com.chip.python.ble.work_queue", DISPATCH_QUEUE_SERIAL);
         _timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, _workQueue);
@@ -78,7 +78,7 @@ - (void)centralManager:(CBCentralManager *)central
 
     NSDictionary * servicesData = [advertisementData objectForKey:CBAdvertisementDataServiceDataKey];
     for (CBUUID * serviceUUID in servicesData) {
-        if (![serviceUUID.data isEqualToData:_shortServiceUUID.data]) {
+        if (![serviceUUID isEqualTo:_shortServiceUUID]) {
             continue;
         }
         NSData * serviceData = [servicesData objectForKey:serviceUUID];

src/darwin/Framework/Matter.xcodeproj/project.pbxproj

@@ -652,6 +652,8 @@
 		3DA1A3532ABAB3B4004F0BB9 /* MTRAsyncWorkQueue.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRAsyncWorkQueue.mm; sourceTree = "<group>"; };
 		3DA1A3572ABABF69004F0BB9 /* MTRAsyncWorkQueueTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTRAsyncWorkQueueTests.m; sourceTree = "<group>"; };
 		3DB9DAE42D67EE5A00704FAB /* MTRBleTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MTRBleTests.m; sourceTree = "<group>"; };
+		3DB9DAE92D754C7200704FAB /* BleUtils.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BleUtils.h; sourceTree = "<group>"; };
+		3DB9DAEA2D754C7200704FAB /* BleUtils.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = BleUtils.mm; sourceTree = "<group>"; };
 		3DECCB6D29347D2C00585AEC /* Security.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Security.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk/System/Library/Frameworks/Security.framework; sourceTree = DEVELOPER_DIR; };
 		3DECCB6F2934AC1C00585AEC /* MTRLogging.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRLogging.h; sourceTree = "<group>"; };
 		3DECCB712934AFE200585AEC /* MTRLogging.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRLogging.mm; sourceTree = "<group>"; };
@@ -694,8 +696,6 @@
 		3DF5218B2D5E90BE008F8E52 /* Logging.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = Logging.mm; sourceTree = "<group>"; };
 		3DF5218C2D5E90BE008F8E52 /* MdnsError.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MdnsError.h; sourceTree = "<group>"; };
 		3DF5218D2D5E90BE008F8E52 /* MdnsError.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MdnsError.cpp; sourceTree = "<group>"; };
-		3DF5218E2D5E90BE008F8E52 /* MTRUUIDHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MTRUUIDHelper.h; sourceTree = "<group>"; };
-		3DF5218F2D5E90BE008F8E52 /* MTRUUIDHelperImpl.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRUUIDHelperImpl.mm; sourceTree = "<group>"; };
 		3DF521902D5E90BE008F8E52 /* NetworkCommissioningDriver.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = NetworkCommissioningDriver.h; sourceTree = "<group>"; };
 		3DF521912D5E90BE008F8E52 /* PlatformManagerImpl.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = PlatformManagerImpl.h; sourceTree = "<group>"; };
 		3DF521922D5E90BE008F8E52 /* PlatformManagerImpl.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = PlatformManagerImpl.cpp; sourceTree = "<group>"; };
@@ -1409,6 +1409,8 @@
 				3DF5216F2D5E90BE008F8E52 /* BlePlatformDelegateImpl.h */,
 				3DF521702D5E90BE008F8E52 /* BlePlatformDelegateImpl.mm */,
 				3DF521712D5E90BE008F8E52 /* BleScannerDelegate.h */,
+				3DB9DAE92D754C7200704FAB /* BleUtils.h */,
+				3DB9DAEA2D754C7200704FAB /* BleUtils.mm */,
 				3DF521732D5E90BE008F8E52 /* CHIPDevicePlatformConfig.h */,
 				3DF521742D5E90BE008F8E52 /* CHIPDevicePlatformEvent.h */,
 				3DF521752D5E90BE008F8E52 /* CHIPPlatformConfig.h */,
@@ -1436,8 +1438,6 @@
 				3DF5218B2D5E90BE008F8E52 /* Logging.mm */,
 				3DF5218C2D5E90BE008F8E52 /* MdnsError.h */,
 				3DF5218D2D5E90BE008F8E52 /* MdnsError.cpp */,
-				3DF5218E2D5E90BE008F8E52 /* MTRUUIDHelper.h */,
-				3DF5218F2D5E90BE008F8E52 /* MTRUUIDHelperImpl.mm */,
 				3DF521902D5E90BE008F8E52 /* NetworkCommissioningDriver.h */,
 				3DF521912D5E90BE008F8E52 /* PlatformManagerImpl.h */,
 				3DF521922D5E90BE008F8E52 /* PlatformManagerImpl.cpp */,

src/platform/Darwin/BUILD.gn

@@ -131,8 +131,7 @@ static_library("Darwin") {
       "BlePlatformDelegateImpl.h",
       "BlePlatformDelegateImpl.mm",
       "BleUtils.h",
-      "MTRUUIDHelper.h",
-      "MTRUUIDHelperImpl.mm",
+      "BleUtils.mm",
     ]
   }
 }

src/platform/Darwin/BleConnectionDelegateImpl.mm

@@ -35,7 +35,8 @@
 #include <setup_payload/SetupPayload.h>
 #include <tracing/metric_event.h>
 
-#import "MTRUUIDHelper.h"
+#import <CoreBluetooth/CoreBluetooth.h>
+
 #import "PlatformMetricKeys.h"
 
 using namespace chip::Ble;
@@ -60,7 +61,6 @@ @interface BleConnection : NSObject <CBCentralManagerDelegate, CBPeripheralDeleg
 @property (strong, nonatomic) dispatch_queue_t workQueue; // the CHIP work queue
 @property (strong, nonatomic) CBCentralManager * centralManager;
 @property (strong, nonatomic) CBPeripheral * peripheral;
-@property (strong, nonatomic) CBUUID * shortServiceUUID;
 @property (nonatomic, readonly, nullable) dispatch_source_t timer;
 @property (nonatomic, readonly) BleConnectionMode currentMode;
 @property (strong, nonatomic) NSMutableDictionary<CBPeripheral *, NSDictionary *> * cachedPeripherals;
@@ -212,13 +212,15 @@ @interface BleConnection ()
 @property (nonatomic, readonly) int32_t totalDevicesRemoved;
 @end
 
-@implementation BleConnection
+@implementation BleConnection {
+    CBUUID * _chipServiceUUID;
+}
 
 - (instancetype)init
 {
     self = [super init];
     if (self) {
-        self.shortServiceUUID = [MTRUUIDHelper GetShortestServiceUUID:&chip::Ble::CHIP_BLE_SVC_ID];
+        _chipServiceUUID = CBUUIDFromBleUUID(chip::Ble::CHIP_BLE_SVC_ID);
         _workQueue = chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue();
         _centralManager = [CBCentralManager alloc];
         _found = false;
@@ -338,15 +340,7 @@ - (void)centralManager:(CBCentralManager *)central
         advertisementData:(NSDictionary *)advertisementData
                      RSSI:(NSNumber *)RSSI
 {
-    NSDictionary * servicesData = [advertisementData objectForKey:CBAdvertisementDataServiceDataKey];
-    NSData * serviceData;
-    for (CBUUID * serviceUUID in servicesData) {
-        if ([serviceUUID.data isEqualToData:_shortServiceUUID.data]) {
-            serviceData = [servicesData objectForKey:serviceUUID];
-            break;
-        }
-    }
-
+    NSData * serviceData = advertisementData[CBAdvertisementDataServiceDataKey][_chipServiceUUID];
     if (!serviceData) {
         return;
     }
@@ -431,7 +425,7 @@ - (void)peripheral:(CBPeripheral *)peripheral didDiscoverServices:(NSError *)err
     MATTER_LOG_METRIC_END(kMetricBLEDiscoveredServices, CHIP_ERROR(chip::ChipError::Range::kOS, static_cast<uint32_t>(error.code)));
 
     for (CBService * service in peripheral.services) {
-        if ([service.UUID.data isEqualToData:_shortServiceUUID.data] && !self.found) {
+        if ([service.UUID isEqual:_chipServiceUUID] && !self.found) {
             MATTER_LOG_METRIC_BEGIN(kMetricBLEDiscoveredCharacteristics);
             [peripheral discoverCharacteristics:nil forService:service];
             self.found = true;
@@ -464,9 +458,8 @@ - (void)peripheral:(CBPeripheral *)peripheral
                              error:(NSError *)error
 {
     if (nil == error) {
-        chip::Ble::ChipBleUUID svcId;
-        chip::Ble::ChipBleUUID charId;
-        [BleConnection fillServiceWithCharacteristicUuids:characteristic svcId:&svcId charId:&charId];
+        ChipBleUUID svcId = BleUUIDFromCBUUD(characteristic.service.UUID);
+        ChipBleUUID charId = BleUUIDFromCBUUD(characteristic.UUID);
         _mBleLayer->HandleWriteConfirmation(BleConnObjectFromCBPeripheral(peripheral), &svcId, &charId);
     } else {
         ChipLogError(
@@ -483,10 +476,8 @@ - (void)peripheral:(CBPeripheral *)peripheral
     bool isNotifying = characteristic.isNotifying;
 
     if (nil == error) {
-        chip::Ble::ChipBleUUID svcId;
-        chip::Ble::ChipBleUUID charId;
-        [BleConnection fillServiceWithCharacteristicUuids:characteristic svcId:&svcId charId:&charId];
-
+        ChipBleUUID svcId = BleUUIDFromCBUUD(characteristic.service.UUID);
+        ChipBleUUID charId = BleUUIDFromCBUUD(characteristic.UUID);
         if (isNotifying) {
             _mBleLayer->HandleSubscribeComplete(BleConnObjectFromCBPeripheral(peripheral), &svcId, &charId);
         } else {
@@ -513,9 +504,8 @@ - (void)peripheral:(CBPeripheral *)peripheral
                               error:(NSError *)error
 {
     if (nil == error) {
-        chip::Ble::ChipBleUUID svcId;
-        chip::Ble::ChipBleUUID charId;
-        [BleConnection fillServiceWithCharacteristicUuids:characteristic svcId:&svcId charId:&charId];
+        ChipBleUUID svcId = BleUUIDFromCBUUD(characteristic.service.UUID);
+        ChipBleUUID charId = BleUUIDFromCBUUD(characteristic.UUID);
         auto * value = characteristic.value; // read immediately before dispatching
 
         // build a inet buffer from the rxEv and send to blelayer.
@@ -603,7 +593,7 @@ - (void)startScanning
     [self _resetCounters];
 
     auto scanOptions = @{ CBCentralManagerScanOptionAllowDuplicatesKey : @YES };
-    [_centralManager scanForPeripheralsWithServices:@[ _shortServiceUUID ] options:scanOptions];
+    [_centralManager scanForPeripheralsWithServices:@[ _chipServiceUUID ] options:scanOptions];
 }
 
 - (void)stopScanning
@@ -782,55 +772,6 @@ - (void)removePeripheralsFromCache
     }
 }
 
-/**
- * private static method to copy service and characteristic UUIDs from CBCharacteristic to a pair of ChipBleUUID objects.
- * this is used in calls into Chip layer to decouple it from CoreBluetooth
- *
- * @param[in] characteristic the source characteristic
- * @param[in] svcId the destination service UUID
- * @param[in] charId the destination characteristic UUID
- *
- */
-+ (void)fillServiceWithCharacteristicUuids:(CBCharacteristic *)characteristic
-                                     svcId:(chip::Ble::ChipBleUUID *)svcId
-                                    charId:(chip::Ble::ChipBleUUID *)charId
-{
-    static const size_t FullUUIDLength = 16;
-    if ((FullUUIDLength != sizeof(charId->bytes)) || (FullUUIDLength != sizeof(svcId->bytes))
-        || (FullUUIDLength != characteristic.UUID.data.length)) {
-        // we're dead. we expect the data length to be the same (16-byte) across the board
-        ChipLogError(Ble, "UUID of characteristic is incompatible");
-        return;
-    }
-
-    memcpy(charId->bytes, characteristic.UUID.data.bytes, sizeof(charId->bytes));
-    memset(svcId->bytes, 0, sizeof(svcId->bytes));
-
-    // Expand service UUID back to 16-byte long as that's what the BLE Layer expects
-    // this is a buffer pre-filled with BLE service UUID Base
-    // byte 0 to 3 are reserved for shorter versions of BLE service UUIDs
-    // For 4-byte service UUIDs, all bytes from 0 to 3 are used
-    // For 2-byte service UUIDs, byte 0 and 1 shall be 0
-    uint8_t serviceFullUUID[FullUUIDLength]
-        = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
-
-    switch (characteristic.service.UUID.data.length) {
-    case 2:
-        // copy the 2-byte service UUID onto the right offset
-        memcpy(serviceFullUUID + 2, characteristic.service.UUID.data.bytes, 2);
-        break;
-    case 4:
-        // flow through
-    case 16:
-        memcpy(serviceFullUUID, characteristic.service.UUID.data.bytes, characteristic.service.UUID.data.length);
-        break;
-    default:
-        // we're dead. we expect the data length to be the same (16-byte) across the board
-        ChipLogError(Ble, "Service UUIDs are incompatible");
-    }
-    memcpy(svcId->bytes, serviceFullUUID, sizeof(svcId->bytes));
-}
-
 - (void)setBleLayer:(chip::Ble::BleLayer *)bleLayer
 {
     _mBleLayer = bleLayer;

src/platform/Darwin/BlePlatformDelegateImpl.mm

@@ -30,68 +30,53 @@
 #include <platform/Darwin/BlePlatformDelegateImpl.h>
 #include <platform/Darwin/BleUtils.h>
 
-#import "MTRUUIDHelper.h"
+#import <CoreBluetooth/CoreBluetooth.h>
 
 using namespace chip::Ble;
 using chip::System::PacketBufferHandle;
 
 namespace chip {
 namespace DeviceLayer {
     namespace Internal {
-        CHIP_ERROR BlePlatformDelegateImpl::SubscribeCharacteristic(
-            BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId)
-        {
-            CHIP_ERROR err = BLE_ERROR_GATT_SUBSCRIBE_FAILED;
-
-            if (nullptr == svcId || nullptr == charId) {
-                return err;
-            }
-
-            CBUUID * serviceId = [MTRUUIDHelper GetShortestServiceUUID:svcId];
-            CBUUID * characteristicId = [CBUUID UUIDWithData:[NSData dataWithBytes:charId->bytes length:sizeof(charId->bytes)]];
-            CBPeripheral * peripheral = CBPeripheralFromBleConnObject(connObj);
 
-            for (CBService * service in peripheral.services) {
-                if ([service.UUID.data isEqualToData:serviceId.data]) {
-                    for (CBCharacteristic * characteristic in service.characteristics) {
-                        if ([characteristic.UUID.data isEqualToData:characteristicId.data]) {
-                            err = CHIP_NO_ERROR;
-                            [peripheral setNotifyValue:true forCharacteristic:characteristic];
-                            break;
+        namespace {
+            CBCharacteristic * FindCharacteristic(CBPeripheral * peripheral, const ChipBleUUID * svcId, const ChipBleUUID * charId)
+            {
+                VerifyOrReturnValue(svcId != nullptr && charId != nullptr, nil);
+                CBUUID * cbSvcId = CBUUIDFromBleUUID(*svcId);
+                for (CBService * service in peripheral.services) {
+                    if ([service.UUID isEqual:cbSvcId]) {
+                        CBUUID * cbCharId = CBUUIDFromBleUUID(*charId);
+                        for (CBCharacteristic * characteristic in service.characteristics) {
+                            if ([characteristic.UUID isEqual:cbCharId]) {
+                                return characteristic;
+                            }
                         }
+                        break;
                     }
                 }
+                return nil;
             }
+        }
 
-            return err;
+        CHIP_ERROR BlePlatformDelegateImpl::SubscribeCharacteristic(
+            BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId)
+        {
+            CBPeripheral * peripheral = CBPeripheralFromBleConnObject(connObj);
+            CBCharacteristic * characteristic = FindCharacteristic(peripheral, svcId, charId);
+            VerifyOrReturnError(characteristic != nil, BLE_ERROR_GATT_SUBSCRIBE_FAILED);
+            [peripheral setNotifyValue:YES forCharacteristic:characteristic];
+            return CHIP_NO_ERROR;
         }
 
         CHIP_ERROR BlePlatformDelegateImpl::UnsubscribeCharacteristic(
             BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId)
         {
-            CHIP_ERROR err = BLE_ERROR_GATT_UNSUBSCRIBE_FAILED;
-            if (nullptr == svcId || nullptr == charId) {
-                return err;
-            }
-
-            CBUUID * serviceId = [MTRUUIDHelper GetShortestServiceUUID:svcId];
-            CBUUID * characteristicId = characteristicId = [CBUUID UUIDWithData:[NSData dataWithBytes:charId->bytes
-                                                                                               length:sizeof(charId->bytes)]];
             CBPeripheral * peripheral = CBPeripheralFromBleConnObject(connObj);
-
-            for (CBService * service in peripheral.services) {
-                if ([service.UUID.data isEqualToData:serviceId.data]) {
-                    for (CBCharacteristic * characteristic in service.characteristics) {
-                        if ([characteristic.UUID.data isEqualToData:characteristicId.data]) {
-                            err = CHIP_NO_ERROR;
-                            [peripheral setNotifyValue:false forCharacteristic:characteristic];
-                            break;
-                        }
-                    }
-                }
-            }
-
-            return err;
+            CBCharacteristic * characteristic = FindCharacteristic(peripheral, svcId, charId);
+            VerifyOrReturnError(characteristic != nil, BLE_ERROR_GATT_UNSUBSCRIBE_FAILED);
+            [peripheral setNotifyValue:NO forCharacteristic:characteristic];
+            return CHIP_NO_ERROR;
         }
 
         CHIP_ERROR BlePlatformDelegateImpl::CloseConnection(BLE_CONNECTION_OBJECT connObj)
@@ -126,32 +111,12 @@
         CHIP_ERROR BlePlatformDelegateImpl::SendWriteRequest(
             BLE_CONNECTION_OBJECT connObj, const ChipBleUUID * svcId, const ChipBleUUID * charId, PacketBufferHandle pBuf)
         {
-            CHIP_ERROR err = BLE_ERROR_GATT_WRITE_FAILED;
-            if (nullptr == svcId || nullptr == charId || pBuf.IsNull()) {
-                return err;
-            }
-
-            CBUUID * serviceId = [MTRUUIDHelper GetShortestServiceUUID:svcId];
-            CBUUID * characteristicId = [CBUUID UUIDWithData:[NSData dataWithBytes:charId->bytes length:sizeof(charId->bytes)]];
-            NSData * data = [NSData dataWithBytes:pBuf->Start() length:pBuf->DataLength()];
             CBPeripheral * peripheral = CBPeripheralFromBleConnObject(connObj);
-
-            for (CBService * service in peripheral.services) {
-                if ([service.UUID.data isEqualToData:serviceId.data]) {
-                    for (CBCharacteristic * characteristic in service.characteristics) {
-                        if ([characteristic.UUID.data isEqualToData:characteristicId.data]) {
-                            err = CHIP_NO_ERROR;
-                            [peripheral writeValue:data forCharacteristic:characteristic type:CBCharacteristicWriteWithResponse];
-                            break;
-                        }
-                    }
-                }
-            }
-
-            // Going out of scope releases delegate's reference to pBuf. pBuf will be freed when both platform delegate and Chip
-            // stack free their references to it. We release pBuf's reference here since its payload bytes were copied into a new
-            // NSData object
-            return err;
+            CBCharacteristic * characteristic = FindCharacteristic(peripheral, svcId, charId);
+            VerifyOrReturnError(characteristic != nil && !pBuf.IsNull(), BLE_ERROR_GATT_WRITE_FAILED);
+            NSData * data = [NSData dataWithBytes:pBuf->Start() length:pBuf->DataLength()]; // copies data, pBuf can be freed
+            [peripheral writeValue:data forCharacteristic:characteristic type:CBCharacteristicWriteWithResponse];
+            return CHIP_NO_ERROR;
         }
 
     } // namespace Internal

src/platform/Darwin/BleUtils.h

@@ -14,9 +14,12 @@
  *    limitations under the License.
  */
 
+#pragma once
+
 #include <ble/Ble.h>
 
 @class CBPeripheral;
+@class CBUUID;
 
 namespace chip {
 namespace DeviceLayer {
@@ -32,6 +35,12 @@ namespace DeviceLayer {
             return (__bridge void *) peripheral;
         }
 
+        // Creates a CBUUID from a ChipBleUUID
+        CBUUID * CBUUIDFromBleUUID(Ble::ChipBleUUID const & uuid);
+
+        // Creates a ChipBleUUID from a CBUUID, expanding 16 or 32 bit UUIDs if necessary.
+        Ble::ChipBleUUID BleUUIDFromCBUUD(CBUUID * uuid);
+
     }
 }
 }