Merge pull request #288 from openweave/issue/invalid-ble-scan-response

Fix for Issue #283: Invalid Weave BLE scan response from nRF5 Device Layer

Author: Jay Logue

src/adaptations/device-layer/ESP32/BLEManagerImpl.cpp

@@ -24,6 +24,7 @@
 
 #include <Weave/DeviceLayer/internal/WeaveDeviceLayerInternal.h>
 #include <Weave/DeviceLayer/internal/BLEManager.h>
+#include <BleLayer/WeaveBleServiceData.h>
 #include <new>
 
 #if WEAVE_DEVICE_CONFIG_ENABLE_WOBLE
@@ -45,17 +46,11 @@ namespace Internal {
 
 namespace {
 
-struct WeaveServiceData
+
+struct ESP32WeaveServiceData
 {
     uint8_t ServiceUUID[2];
-    uint8_t DataBlockLen;
-    uint8_t DataBlockType;
-    uint8_t DataBlockMajorVersion;
-    uint8_t DataBlockMinorVersion;
-    uint8_t DeviceVendorId[2];
-    uint8_t DeviceProductId[2];
-    uint8_t DeviceId[8];
-    uint8_t PairingStatus;
+    WeaveBLEDeviceIdentificationInfo DeviceIdInfo;
 };
 
 const uint16_t WoBLEAppId = 0x235A;
@@ -562,7 +557,7 @@ WEAVE_ERROR BLEManagerImpl::ConfigureAdvertisingData(void)
 {
     WEAVE_ERROR err;
     esp_ble_adv_data_t advertData;
-    WeaveServiceData weaveServiceData;
+    ESP32WeaveServiceData weaveServiceData;
 
     // If a custom device name has not been specified, generate a Nest-standard name based on the
     // bottom digits of the Weave device id.
@@ -604,16 +599,11 @@ WEAVE_ERROR BLEManagerImpl::ConfigureAdvertisingData(void)
         ExitNow();
     }
 
-    // Construct the Weave Service Data to be sent in the scan response packet.
+    // Construct the Weave BLE Service Data to be sent in the scan response packet.  On the ESP32,
+    // the buffer given to esp_ble_gap_config_adv_data() must begin with the Weave BLE service UUID.
     memcpy(weaveServiceData.ServiceUUID, ShortUUID_WoBLEService, sizeof(weaveServiceData.ServiceUUID));
-    weaveServiceData.DataBlockLen = 16;
-    weaveServiceData.DataBlockType = 0x16;
-    weaveServiceData.DataBlockMajorVersion = 0;
-    weaveServiceData.DataBlockMinorVersion = 1;
-    Encoding::LittleEndian::Put16(weaveServiceData.DeviceVendorId, (uint16_t)WEAVE_DEVICE_CONFIG_DEVICE_VENDOR_ID);
-    Encoding::LittleEndian::Put16(weaveServiceData.DeviceProductId, (uint16_t)WEAVE_DEVICE_CONFIG_DEVICE_PRODUCT_ID);
-    Encoding::LittleEndian::Put64(weaveServiceData.DeviceId, FabricState.LocalNodeId);
-    weaveServiceData.PairingStatus = ConfigurationMgr().IsPairedToAccount() ? 1 : 0;
+    err = ConfigurationMgr().GetBLEDeviceIdentificationInfo(weaveServiceData.DeviceIdInfo);
+    SuccessOrExit(err);
 
     // Configure the contents of the scan response packet.
     memset(&advertData, 0, sizeof(advertData));

src/adaptations/device-layer/include/Weave/DeviceLayer/ConfigurationManager.h

@@ -28,6 +28,12 @@
 #include <Weave/Profiles/device-description/DeviceDescription.h>
 #include <Weave/Profiles/network-provisioning/NetworkProvisioning.h>
 
+namespace nl {
+namespace Ble {
+struct WeaveBLEDeviceIdentificationInfo;
+}
+}
+
 namespace nl {
 namespace Weave {
 namespace DeviceLayer {
@@ -98,6 +104,8 @@ class ConfigurationManager
 
     WEAVE_ERROR GetWiFiAPSSID(char * buf, size_t bufSize);
 
+    WEAVE_ERROR GetBLEDeviceIdentificationInfo(Ble::WeaveBLEDeviceIdentificationInfo & deviceIdInfo);
+
     bool IsServiceProvisioned();
     bool IsPairedToAccount();
     bool IsMemberOfFabric();
@@ -362,6 +370,11 @@ inline WEAVE_ERROR ConfigurationManager::GetWiFiAPSSID(char * buf, size_t bufSiz
     return static_cast<ImplClass*>(this)->_GetWiFiAPSSID(buf, bufSize);
 }
 
+inline WEAVE_ERROR ConfigurationManager::GetBLEDeviceIdentificationInfo(Ble::WeaveBLEDeviceIdentificationInfo & deviceIdInfo)
+{
+    return static_cast<ImplClass*>(this)->_GetBLEDeviceIdentificationInfo(deviceIdInfo);
+}
+
 inline bool ConfigurationManager::IsServiceProvisioned()
 {
     return static_cast<ImplClass*>(this)->_IsServiceProvisioned();

src/adaptations/device-layer/include/Weave/DeviceLayer/internal/GenericConfigurationManagerImpl.h

@@ -85,6 +85,7 @@ class GenericConfigurationManagerImpl
     WEAVE_ERROR _GetDeviceDescriptorTLV(uint8_t * buf, size_t bufSize, size_t & encodedLen);
     WEAVE_ERROR _GetQRCodeString(char * buf, size_t bufSize);
     WEAVE_ERROR _GetWiFiAPSSID(char * buf, size_t bufSize);
+    WEAVE_ERROR _GetBLEDeviceIdentificationInfo(Ble::WeaveBLEDeviceIdentificationInfo & deviceIdInfo);
     bool _IsServiceProvisioned();
     bool _IsMemberOfFabric();
     bool _IsPairedToAccount();

src/adaptations/device-layer/include/Weave/DeviceLayer/internal/GenericConfigurationManagerImpl.ipp

@@ -27,6 +27,7 @@
 
 #include <Weave/DeviceLayer/internal/WeaveDeviceLayerInternal.h>
 #include <Weave/DeviceLayer/internal/GenericConfigurationManagerImpl.h>
+#include <BleLayer/WeaveBleServiceData.h>
 
 
 namespace nl {
@@ -615,6 +616,32 @@ exit:
     return err;
 }
 
+template<class ImplClass>
+WEAVE_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetBLEDeviceIdentificationInfo(Ble::WeaveBLEDeviceIdentificationInfo & deviceIdInfo)
+{
+    WEAVE_ERROR err;
+    uint16_t id;
+
+    deviceIdInfo.Init();
+
+    err = Impl()->_GetVendorId(id);
+    SuccessOrExit(err);
+    deviceIdInfo.SetVendorId(id);
+
+    err = Impl()->_GetProductId(id);
+    SuccessOrExit(err);
+    deviceIdInfo.SetProductId(id);
+
+    deviceIdInfo.SetDeviceId(FabricState.LocalNodeId);
+
+    deviceIdInfo.PairingStatus = Impl()->_IsPairedToAccount()
+        ? Ble::WeaveBLEDeviceIdentificationInfo::kPairingStatus_Paired
+        : Ble::WeaveBLEDeviceIdentificationInfo::kPairingStatus_Unpaired;
+
+exit:
+    return err;
+}
+
 template<class ImplClass>
 bool GenericConfigurationManagerImpl<ImplClass>::_IsServiceProvisioned()
 {

src/adaptations/device-layer/nRF5/BLEManagerImpl.cpp

@@ -24,6 +24,7 @@
 
 #include <Weave/DeviceLayer/internal/WeaveDeviceLayerInternal.h>
 #include <Weave/DeviceLayer/internal/BLEManager.h>
+#include <BleLayer/WeaveBleServiceData.h>
 #include <new>
 
 #if WEAVE_DEVICE_CONFIG_ENABLE_WOBLE
@@ -49,16 +50,6 @@ namespace Internal {
 
 namespace {
 
-struct WeaveServiceData
-{
-    uint8_t MajorVersion;
-    uint8_t MinorVersion;
-    uint8_t DeviceVendorId[2];
-    uint8_t DeviceProductId[2];
-    uint8_t DeviceId[8];
-    uint8_t PairingStatus;
-};
-
 const uint16_t UUID16_WoBLEService = 0xFEAF;
 const ble_uuid_t UUID_WoBLEService = { UUID16_WoBLEService, BLE_UUID_TYPE_BLE };
 
@@ -519,7 +510,7 @@ WEAVE_ERROR BLEManagerImpl::EncodeAdvertisingData(ble_gap_adv_data_t & gapAdvDat
     WEAVE_ERROR err = WEAVE_NO_ERROR;
     ble_advdata_t advData;
     ble_advdata_service_data_t serviceData;
-    WeaveServiceData weaveServiceData;
+    WeaveBLEDeviceIdentificationInfo deviceIdInfo;
 
     // Form the contents of the advertising packet.
     memset(&advData, 0, sizeof(advData));
@@ -533,20 +524,16 @@ WEAVE_ERROR BLEManagerImpl::EncodeAdvertisingData(ble_gap_adv_data_t & gapAdvDat
     err = ble_advdata_encode(&advData, mAdvDataBuf, &gapAdvData.adv_data.len);
     SuccessOrExit(err);
 
-    // Construct the Weave Service Data structure that will be sent in the scan response packet.
-    memset(&weaveServiceData, 0, sizeof(weaveServiceData));
-    weaveServiceData.MajorVersion = 0;
-    weaveServiceData.MinorVersion = 1;
-    Encoding::LittleEndian::Put16(weaveServiceData.DeviceVendorId, (uint16_t)WEAVE_DEVICE_CONFIG_DEVICE_VENDOR_ID);
-    Encoding::LittleEndian::Put16(weaveServiceData.DeviceProductId, (uint16_t)WEAVE_DEVICE_CONFIG_DEVICE_PRODUCT_ID);
-    Encoding::LittleEndian::Put64(weaveServiceData.DeviceId, FabricState.LocalNodeId);
-    weaveServiceData.PairingStatus = ConfigurationMgr().IsPairedToAccount() ? 1 : 0;
+    // Initialize the Weave BLE Device Identification Information block that will be sent as payload
+    // within the BLE service advertisement data.
+    err = ConfigurationMgr().GetBLEDeviceIdentificationInfo(deviceIdInfo);
+    SuccessOrExit(err);
 
     // Form the contents of the scan response packet.
     memset(&serviceData, 0, sizeof(serviceData));
     serviceData.service_uuid = UUID16_WoBLEService;
-    serviceData.data.size = sizeof(weaveServiceData);
-    serviceData.data.p_data = (uint8_t *)&weaveServiceData;
+    serviceData.data.size = sizeof(deviceIdInfo);
+    serviceData.data.p_data = (uint8_t *)&deviceIdInfo;
     memset(&advData, 0, sizeof(advData));
     advData.name_type = BLE_ADVDATA_NO_NAME;
     advData.include_appearance = false;

src/ble/WeaveBleServiceData.h

@@ -0,0 +1,111 @@
+/*
+ *
+ *    Copyright (c) 2019 Google LLC.
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+/**
+ *    @file
+ *          Definitions for Weave BLE service advertisement data.
+ */
+
+#ifndef WEAVE_BLE_SERVICE_DATA_H
+#define WEAVE_BLE_SERVICE_DATA_H
+
+namespace nl {
+namespace Ble {
+
+/**
+ * Weave data block types that may appear with Weave BLE service advertisement data.
+ */
+enum WeaveBLEServiceDataType
+{
+    kWeaveBLEServiceDataType_DeviceIdentificationInfo       = 0x01,
+    kWeaveBLEServiceDataType_TokenIdentificationInfo        = 0x02,
+};
+
+/**
+ * Weave BLE Device Identification Information Block
+ *
+ * Defines the over-the-air encoded format of the device identification information block that appears
+ * within Weave BLE service advertisement data.
+ */
+struct WeaveBLEDeviceIdentificationInfo
+{
+    enum
+    {
+        kMajorVersion           = 0,
+        kMinorVersion           = 1,
+    };
+
+    enum
+    {
+        kPairingStatus_Unpaired = 0,
+        kPairingStatus_Paired   = 1,
+    };
+
+    uint8_t BlockLen;
+    uint8_t BlockType;
+    uint8_t MajorVersion;
+    uint8_t MinorVersion;
+    uint8_t DeviceVendorId[2];
+    uint8_t DeviceProductId[2];
+    uint8_t DeviceId[8];
+    uint8_t PairingStatus;
+
+    void Init()
+    {
+        memset(this, 0, sizeof(*this));
+        BlockLen = sizeof(*this) - sizeof(BlockLen); // size of all fields EXCEPT BlockLen
+        BlockType = kWeaveBLEServiceDataType_DeviceIdentificationInfo;
+        MajorVersion = kMajorVersion;
+        MinorVersion = kMinorVersion;
+    }
+
+    uint16_t GetVendorId(void)
+    {
+        return nl::Weave::Encoding::LittleEndian::Get16(DeviceVendorId);
+    }
+
+    void SetVendorId(uint16_t vendorId)
+    {
+        nl::Weave::Encoding::LittleEndian::Put16(DeviceVendorId, vendorId);
+    }
+
+    uint16_t GetProductId(void)
+    {
+        return nl::Weave::Encoding::LittleEndian::Get16(DeviceProductId);
+    }
+
+    void SetProductId(uint16_t productId)
+    {
+        nl::Weave::Encoding::LittleEndian::Put16(DeviceProductId, productId);
+    }
+
+    uint64_t GetDeviceId(void)
+    {
+        return nl::Weave::Encoding::LittleEndian::Get64(DeviceId);
+    }
+
+    void SetDeviceId(uint64_t deviceId)
+    {
+        nl::Weave::Encoding::LittleEndian::Put64(DeviceId, deviceId);
+    }
+} __attribute__((packed));
+
+} /* namespace Ble */
+} /* namespace nl */
+
+#endif // WEAVE_BLE_SERVICE_DATA_H

src/include/Makefile.am

@@ -81,6 +81,7 @@ $(nl_public_BleLayer_ble_source_dirstem)/BleLayer.h \
 $(nl_public_BleLayer_ble_source_dirstem)/BlePlatformDelegate.h \
 $(nl_public_BleLayer_ble_source_dirstem)/BleUUID.h \
 $(nl_public_BleLayer_ble_source_dirstem)/WoBle.h \
+$(nl_public_BleLayer_ble_source_dirstem)/WeaveBleServiceData.h \
 $(NULL)
 
 dist_ble_ble_HEADERS = $(addprefix ../,$(nl_public_BleLayer_ble_header_sources))

src/include/Makefile.in

@@ -717,6 +717,7 @@ $(nl_public_BleLayer_ble_source_dirstem)/BleLayer.h \
 $(nl_public_BleLayer_ble_source_dirstem)/BlePlatformDelegate.h \
 $(nl_public_BleLayer_ble_source_dirstem)/BleUUID.h \
 $(nl_public_BleLayer_ble_source_dirstem)/WoBle.h \
+$(nl_public_BleLayer_ble_source_dirstem)/WeaveBleServiceData.h \
 $(NULL)
 
 dist_ble_ble_HEADERS = $(addprefix ../,$(nl_public_BleLayer_ble_header_sources))