[Darwin] Handle short discriminator while pairing (project-chip#3550)

* Handle pairing with short setup codes * Update type casts * Use a different check to see if the discriminator has been set * Address review comments
selissia · Nov 5, 2020 · 1d812b7 · 1d812b7
1 parent 005b7ae
commit 1d812b7
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 7 deletions.
diff --git a/src/ble/BleLayer.cpp b/src/ble/BleLayer.cpp
@@ -374,7 +374,6 @@ BLE_ERROR BleLayer::NewBleConnection(void * appState, const uint16_t connDiscrim
     BLE_ERROR err = BLE_NO_ERROR;
 
     VerifyOrExit(mState == kState_Initialized, err = BLE_ERROR_INCORRECT_STATE);
-    VerifyOrExit(connDiscriminator != 0, err = BLE_ERROR_BAD_ARGS);
     VerifyOrExit(mConnectionDelegate != nullptr, err = BLE_ERROR_INCORRECT_STATE);
 
     mConnectionDelegate->OnConnectionComplete = onConnectionComplete;

diff --git a/src/darwin/CHIPTool/CHIPTool/View Controllers/QRCode/QRCodeViewController.m b/src/darwin/CHIPTool/CHIPTool/View Controllers/QRCode/QRCodeViewController.m
@@ -510,7 +510,6 @@ - (void)parseOptionalData:(CHIPSetupPayload *)payload
 - (void)handleRendezVous:(CHIPSetupPayload *)payload
 {
     switch (payload.rendezvousInformation) {
-    case kRendezvousInformationNone:
     case kRendezvousInformationThread:
     case kRendezvousInformationEthernet:
     case kRendezvousInformationAllMask:
@@ -520,6 +519,7 @@ - (void)handleRendezVous:(CHIPSetupPayload *)payload
         NSLog(@"Rendezvous Wi-Fi");
         [self handleRendezVousWiFi:[self getNetworkName:payload.discriminator]];
         break;
+    case kRendezvousInformationNone:
     case kRendezvousInformationBLE:
         NSLog(@"Rendezvous BLE");
         [self handleRendezVousBLE:payload.discriminator.unsignedShortValue setupPINCode:payload.setUpPINCode.unsignedIntValue];

diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn
@@ -177,6 +177,7 @@ if (chip_device_platform != "none" && chip_device_platform != "external") {
       "${chip_root}/src/lib/core",
       "${chip_root}/src/lib/core:chip_config_header",
       "${chip_root}/src/lib/support",
+      "${chip_root}/src/setup_payload",
       "${nlio_root}:nlio",
     ]
 

diff --git a/src/platform/Darwin/BleConnectionDelegateImpl.mm b/src/platform/Darwin/BleConnectionDelegateImpl.mm
@@ -26,6 +26,7 @@
 #include <ble/BleLayer.h>
 #include <ble/BleUUID.h>
 #include <platform/Darwin/BleConnectionDelegate.h>
+#include <setup_payload/SetupPayload.h>
 #include <support/logging/CHIPLogging.h>
 
 #import <CoreBluetooth/CoreBluetooth.h>
@@ -137,7 +138,7 @@ - (void)centralManager:(CBCentralManager *)central
                         discriminator = static_cast<uint16_t>(((bytes[1] & 0x0F) << 8) | bytes[2]);
                     }
 
-                    if (opCode == 0 && discriminator == _deviceDiscriminator) {
+                    if (opCode == 0 && [self checkDiscriminator:discriminator]) {
                         ChipLogProgress(Ble, "Connecting to device: %@", peripheral);
                         [self connect:peripheral];
                         [self stopScanning];
@@ -150,6 +151,17 @@ - (void)centralManager:(CBCentralManager *)central
     }
 }
 
+- (BOOL)checkDiscriminator:(uint16_t)discriminator
+{
+    // If the setup discriminator is only 4 bits, only match the lower 4 from the BLE advertisement
+    if (_deviceDiscriminator <= chip::kManualSetupDiscriminatorFieldBitMask) {
+        return _deviceDiscriminator == (discriminator & chip::kManualSetupDiscriminatorFieldBitMask);
+    } else {
+        // else compare the entire thing
+        return _deviceDiscriminator == discriminator;
+    }
+}
+
 - (void)centralManager:(CBCentralManager *)central didConnectPeripheral:(CBPeripheral *)peripheral
 {
     [peripheral setDelegate:self];

diff --git a/src/setup_payload/SetupPayload.h b/src/setup_payload/SetupPayload.h
@@ -54,6 +54,9 @@ const int kManualSetupProductIdCharLength = 5;
 const uint8_t kSerialNumberTag               = 128;
 const uint32_t kTag_QRCodeExensionDescriptor = 0x00;
 
+// The largest value of the 12-bit Payload discriminator
+const uint16_t kMaxDiscriminatorValue = 0xFFF;
+
 // clang-format off
 const int kTotalPayloadDataSizeInBits =
     kVersionFieldLengthInBits +

diff --git a/src/transport/RendezvousParameters.h b/src/transport/RendezvousParameters.h
@@ -27,6 +27,9 @@
 
 namespace chip {
 
+// The largest supported value for Rendezvous discriminators
+const uint16_t kMaxRendezvousDiscriminatorValue = 0xFFF;
+
 class RendezvousParameters
 {
 public:
@@ -42,7 +45,7 @@ class RendezvousParameters
         return *this;
     }
 
-    bool HasDiscriminator() const { return mDiscriminator != 0; }
+    bool HasDiscriminator() const { return mDiscriminator <= kMaxRendezvousDiscriminatorValue; }
     uint16_t GetDiscriminator() const { return mDiscriminator; }
     RendezvousParameters & SetDiscriminator(uint16_t discriminator)
     {
@@ -79,9 +82,9 @@ class RendezvousParameters
 #endif // CONFIG_NETWORK_LAYER_BLE
 
 private:
-    Optional<NodeId> mLocalNodeId; ///< the local node id
-    uint32_t mSetupPINCode  = 0;   ///< the target peripheral setup PIN Code
-    uint16_t mDiscriminator = 0;   ///< the target peripheral discriminator
+    Optional<NodeId> mLocalNodeId;        ///< the local node id
+    uint32_t mSetupPINCode  = 0;          ///< the target peripheral setup PIN Code
+    uint16_t mDiscriminator = UINT16_MAX; ///< the target peripheral discriminator
 
 #if CONFIG_NETWORK_LAYER_BLE
     Ble::BleLayer * mBleLayer               = nullptr;