From e32d6acc3649d23b75b10599a53bb5e9b2477711 Mon Sep 17 00:00:00 2001 From: Catalin Voinea Date: Fri, 19 Jan 2024 13:34:37 -0500 Subject: [PATCH] adjust based on review comments - 1 --- src/app/server/CommissioningWindowManager.h | 2 +- src/include/platform/CHIPDeviceConfig.h | 42 +++++++++++++++++++- src/platform/silabs/BLEManagerImpl.h | 6 +-- src/platform/silabs/efr32/BLEManagerImpl.cpp | 34 +++++++--------- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/app/server/CommissioningWindowManager.h b/src/app/server/CommissioningWindowManager.h index 3d7c26e3ebb528..26a2f9ceb19b71 100644 --- a/src/app/server/CommissioningWindowManager.h +++ b/src/app/server/CommissioningWindowManager.h @@ -60,7 +60,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler, static constexpr System::Clock::Seconds16 MaxCommissioningTimeout() { #if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - // per new spec change/extension - Extended Announcement Duration up to 48h + // Specification section 2.3.1 - Extended Announcement Duration up to 48h return System::Clock::Seconds16(60 * 60 * 48); #else // Specification section 5.4.2.3. Announcement Duration says 15 minutes. diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index 085e5f93ea4568..c5211b184b6119 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/src/include/platform/CHIPDeviceConfig.h @@ -617,18 +617,56 @@ #define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME 30000 #endif +/** + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING + * + * Optional configuration to enable Extended Announcement Duration up to 48h + * Should be used together with extending CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS past 15 minutes + * Disabled by default + */ + #ifndef CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING #define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING 0 #endif #if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING +/** + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME + * + * The amount of time in miliseconds after which BLE advertisement should be switched from the slow + * advertising to the extended advertising, counting from the moment of advertisement commencement. + * + * Defaults to 15 minutes measured in ms. + */ #ifndef CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME #define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME (15 * 60 * 1000) #endif -#ifndef CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL -#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL 1920 +/** + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN + * + * The minimum interval (in units of 0.625ms) at which the device will send BLE advertisements while + * in extended advertising mode. The minimum interval should be smaller and not equal to the + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX. + * + * Defaults to 1920 (1200 ms). + */ +#ifndef CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN +#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN 1920 +#endif + +/** + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX + * + * The interval (in units of 0.625ms) at which the device will send BLE advertisements while + * in extended advertising mode. The maximum interval should be greater and not equal to the + * CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN. + * + * Defaults to 1936 (1210 ms). + */ +#ifndef CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX +#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX 1936 #endif #endif diff --git a/src/platform/silabs/BLEManagerImpl.h b/src/platform/silabs/BLEManagerImpl.h index d93839b63a811e..12b3ba121886af 100644 --- a/src/platform/silabs/BLEManagerImpl.h +++ b/src/platform/silabs/BLEManagerImpl.h @@ -151,6 +151,7 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla kRestartAdvertising = 0x0008, kEFRBLEStackInitialized = 0x0010, kDeviceNameSet = 0x0020, + kExtAdvertisingEnabled = 0x0040, }; enum @@ -208,11 +209,6 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla static void DriveBLEState(intptr_t arg); static void BleAdvTimeoutHandler(TimerHandle_t xTimer); uint8_t GetTimerHandle(uint8_t connectionHandle, bool allocate); - -#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - uint16_t Internal_Slow_Advertising_MIN = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN; - uint16_t Internal_Slow_Advertising_MAX = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX; -#endif }; /** diff --git a/src/platform/silabs/efr32/BLEManagerImpl.cpp b/src/platform/silabs/efr32/BLEManagerImpl.cpp index 12180cbe4701df..a2ec3bb382cbd3 100644 --- a/src/platform/silabs/efr32/BLEManagerImpl.cpp +++ b/src/platform/silabs/efr32/BLEManagerImpl.cpp @@ -460,12 +460,11 @@ CHIP_ERROR BLEManagerImpl::ConfigureAdvertisingData(void) advData[index++] = ShortUUID_CHIPoBLEService[1]; #if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - // check for extended advertisment interval - if (BLEMgrImpl().Internal_Slow_Advertising_MIN == CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL) + // Check for extended advertisement interval and redact VID/PID if past the initial period. + if (mFlags.Has(Flags::kExtAdvertisingEnabled)) { { mDeviceIdInfo.SetVendorId(0x0000); mDeviceIdInfo.SetProductId(0x0000); - mDeviceIdInfo.SetAdditionalDataFlag(true); mDeviceIdInfo.SetExtendedAnnouncementFlag(true); } #endif @@ -566,13 +565,14 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void) } else { -#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - interval_min = BLEMgrImpl().Internal_Slow_Advertising_MIN; - interval_max = BLEMgrImpl().Internal_Slow_Advertising_MAX; -#else - interval_min = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN; - interval_max = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX; -#endif + if (!(mFlags.Has(Flags::kExtAdvertisingEnabled))) { + interval_min = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN; + interval_max = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX; + } + else { + interval_min = CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN; + interval_max = CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX; + } } ret = sl_bt_advertiser_set_timing(advertising_set_handle, interval_min, interval_max, 0, 0); @@ -967,26 +967,20 @@ uint8_t BLEManagerImpl::GetTimerHandle(uint8_t connectionHandle, bool allocate) void BLEManagerImpl::BleAdvTimeoutHandler(TimerHandle_t xTimer) { - if (BLEMgrImpl().mFlags.Has(Flags::kFastAdvertisingEnabled)) - { + if (BLEMgrImpl().mFlags.Has(Flags::kFastAdvertisingEnabled)) { ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start slow advertissment"); -#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - BLEMgrImpl().Internal_Slow_Advertising_MIN = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN; - BLEMgrImpl().Internal_Slow_Advertising_MAX = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX; BLEMgrImpl().mFlags.Set(Flags::kAdvertising); -#endif BLEMgr().SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising); #if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING + BLEMgrImpl().mFlags.Clear(Flags::kExtAdvertisingEnabled); BLEMgrImpl().StartBleAdvTimeoutTimer(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME); #endif } #if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING - else - { + else { ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start extended advertisment"); - BLEMgrImpl().Internal_Slow_Advertising_MIN = CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL; - BLEMgrImpl().Internal_Slow_Advertising_MAX = CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL; BLEMgrImpl().mFlags.Set(Flags::kAdvertising); + BLEMgrImpl().mFlags.Set(Flags::kExtAdvertisingEnabled); BLEMgr().SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising); } #endif