Skip to content

Commit

Permalink
Fix max timeout for open commissioning window
Browse files Browse the repository at this point in the history
Commissioning window can be opened using timeout exceeding
the maximum value of 900 s defined by the spec. This can happen
if selected transport is IP, but the device uses BLE extended
announcement feature.

Added checking if device is commissioned to be able to determine
what max timeout should be used for the particular scenario.

Renamed CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING to the
CHIP_DEVICE_CONFIG_EXT_ADVERTISING, as config name sounds
misleading and it seems it relates only to BLE.

Fixes: #35505
  • Loading branch information
kkasperczyk-no committed Sep 11, 2024
1 parent 83d345e commit ab896f8
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 43 deletions.
14 changes: 14 additions & 0 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,20 @@ CHIP_ERROR CommissioningWindowManager::AdvertiseAndListenForPASE()
return CHIP_NO_ERROR;
}

System::Clock::Seconds32 CommissioningWindowManager::MaxCommissioningTimeout() const
{
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
/* Allow for extended announcement only if the device is uncomissioned. */
if (mServer->GetFabricTable().FabricCount() == 0)
{
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
return System::Clock::Seconds32(60 * 60 * 48);
}
#endif
// Specification section 5.4.2.3. Announcement Duration says 15 minutes.
return System::Clock::Seconds32(15 * 60);
}

CHIP_ERROR CommissioningWindowManager::OpenBasicCommissioningWindow(Seconds32 commissioningTimeout,
CommissioningWindowAdvertisement advertisementMode)
{
Expand Down
11 changes: 1 addition & 10 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
return CHIP_NO_ERROR;
}

static constexpr System::Clock::Seconds32 MaxCommissioningTimeout()
{
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
// Specification section 2.3.1 - Extended Announcement Duration up to 48h
return System::Clock::Seconds32(60 * 60 * 48);
#else
// Specification section 5.4.2.3. Announcement Duration says 15 minutes.
return System::Clock::Seconds32(15 * 60);
#endif
}
System::Clock::Seconds32 MaxCommissioningTimeout() const;

System::Clock::Seconds32 MinCommissioningTimeout() const
{
Expand Down
14 changes: 7 additions & 7 deletions src/include/platform/CHIPDeviceConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,18 +640,18 @@
#endif

/**
* CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
* CHIP_DEVICE_CONFIG_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
#ifndef CHIP_DEVICE_CONFIG_EXT_ADVERTISING
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING 0
#endif

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING

/**
* CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS
Expand Down Expand Up @@ -735,18 +735,18 @@ static_assert(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN <= CHIP_DEVICE
* Time in seconds that a factory new device will advertise commissionable node discovery.
*/
#ifndef CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
/**
* By default, the extended announcement, when enabled, starts its extended advertising 15 mins
* after the standard slow advertisement. Time at which the default discovery time would close the
* commissioning window and stop the BLE.
* Therefore, when CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING is enabled bump the default Discovery timeout
* Therefore, when CHIP_DEVICE_CONFIG_EXT_ADVERTISING is enabled bump the default Discovery timeout
* to the maximum allowed by the spec. 48h.
*/
#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS (60 * 60 * 48)
#else
#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS (15 * 60)
#endif // CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#endif // CHIP_DEVICE_CONFIG_EXT_ADVERTISING
#endif // CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS

/**
Expand Down
4 changes: 2 additions & 2 deletions src/platform/ESP32/CHIPDevicePlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@
#define CHIP_DEVICE_CONFIG_ENABLE_PAIRING_AUTOSTART CONFIG_CHIP_ENABLE_PAIRING_AUTOSTART

#ifdef CONFIG_ENABLE_BLE_EXT_ANNOUNCEMENT
#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING CONFIG_ENABLE_BLE_EXT_ANNOUNCEMENT
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING CONFIG_ENABLE_BLE_EXT_ANNOUNCEMENT
#else
#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING 0
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING 0
#endif

// Options for background chip task
Expand Down
8 changes: 4 additions & 4 deletions src/platform/ESP32/nimble/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,12 @@ void BLEManagerImpl::BleAdvTimeoutHandler(System::Layer *, void *)
BLEMgrImpl().mFlags.Set(Flags::kFastAdvertisingEnabled, 0);
BLEMgrImpl().mFlags.Set(Flags::kAdvertisingRefreshNeeded, 1);

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
BLEMgrImpl().mFlags.Clear(Flags::kExtAdvertisingEnabled);
BLEMgrImpl().StartBleAdvTimeoutTimer(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS);
#endif
}
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
else
{
ChipLogProgress(DeviceLayer, "bleAdv Timeout : Start extended advertisement");
Expand Down Expand Up @@ -1066,7 +1066,7 @@ CHIP_ERROR BLEManagerImpl::ConfigureAdvertisingData(void)
ExitNow();
}

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
// Check for extended advertisement interval and redact VID/PID if past the initial period.
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
{
Expand Down Expand Up @@ -1671,7 +1671,7 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void)
}
else
{
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (!mFlags.Has(Flags::kExtAdvertisingEnabled))
{
adv_params.itvl_min = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN;
Expand Down
8 changes: 4 additions & 4 deletions src/platform/Linux/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ static constexpr System::Clock::Timeout kNewConnectionScanTimeout = System::Cloc
static constexpr System::Clock::Timeout kConnectTimeout = System::Clock::Seconds16(20);
static constexpr System::Clock::Timeout kFastAdvertiseTimeout =
System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME);
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
// The CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS specifies the transition time
// starting from advertisement commencement. Since the extended advertisement timer is started after
// the fast-to-slow transition, we have to subtract the time spent in fast advertising.
Expand Down Expand Up @@ -584,7 +584,7 @@ void BLEManagerImpl::DriveBLEState()

// Setup service data for advertising.
auto serviceDataFlags = BluezAdvertisement::kServiceDataNone;
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
serviceDataFlags |= BluezAdvertisement::kServiceDataExtendedAnnouncement;
#endif
Expand Down Expand Up @@ -667,7 +667,7 @@ BluezAdvertisement::AdvertisingIntervals BLEManagerImpl::GetAdvertisingIntervals
{
if (mFlags.Has(Flags::kFastAdvertisingEnabled))
return { CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MIN, CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MAX };
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
return { CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN, CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX };
#endif
Expand All @@ -682,7 +682,7 @@ void BLEManagerImpl::HandleAdvertisingTimer(chip::System::Layer *, void * appSta
{
ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start slow advertisement");
self->_SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising);
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
self->mFlags.Clear(Flags::kExtAdvertisingEnabled);
DeviceLayer::SystemLayer().StartTimer(kSlowAdvertiseTimeout, HandleAdvertisingTimer, self);
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Linux/bluez/BluezAdvertisement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ CHIP_ERROR BluezAdvertisement::SetupServiceData(ServiceDataFlags aFlags)
deviceInfo.SetAdditionalDataFlag(true);
#endif

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (aFlags & kServiceDataExtendedAnnouncement)
{
deviceInfo.SetExtendedAnnouncementFlag(true);
Expand Down
8 changes: 4 additions & 4 deletions src/platform/NuttX/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static constexpr System::Clock::Timeout kNewConnectionScanTimeout = System::Cloc
static constexpr System::Clock::Timeout kConnectTimeout = System::Clock::Seconds16(20);
static constexpr System::Clock::Timeout kFastAdvertiseTimeout =
System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME);
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
// The CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS specifies the transition time
// starting from advertisement commencement. Since the extended advertisement timer is started after
// the fast-to-slow transition, we have to subtract the time spent in fast advertising.
Expand Down Expand Up @@ -593,7 +593,7 @@ void BLEManagerImpl::DriveBLEState()

// Setup service data for advertising.
auto serviceDataFlags = BluezAdvertisement::kServiceDataNone;
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
serviceDataFlags |= BluezAdvertisement::kServiceDataExtendedAnnouncement;
#endif
Expand Down Expand Up @@ -662,7 +662,7 @@ BluezAdvertisement::AdvertisingIntervals BLEManagerImpl::GetAdvertisingIntervals
{
if (mFlags.Has(Flags::kFastAdvertisingEnabled))
return { CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MIN, CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MAX };
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
return { CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN, CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MAX };
#endif
Expand All @@ -677,7 +677,7 @@ void BLEManagerImpl::HandleAdvertisingTimer(chip::System::Layer *, void * appSta
{
ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start slow advertisement");
self->_SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising);
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
self->mFlags.Clear(Flags::kExtAdvertisingEnabled);
DeviceLayer::SystemLayer().StartTimer(kSlowAdvertiseTimeout, HandleAdvertisingTimer, self);
}
Expand Down
8 changes: 4 additions & 4 deletions src/platform/Zephyr/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ inline CHIP_ERROR BLEManagerImpl::PrepareAdvertisingRequest()
Encoding::LittleEndian::Put16(serviceData.uuid, UUID16_CHIPoBLEService.val);
ReturnErrorOnFailure(ConfigurationMgr().GetBLEDeviceIdentificationInfo(serviceData.deviceIdInfo));

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (mFlags.Has(Flags::kExtendedAdvertisingEnabled))
{
serviceData.deviceIdInfo.SetVendorId(DEVICE_HANDLE_NULL);
Expand All @@ -316,7 +316,7 @@ inline CHIP_ERROR BLEManagerImpl::PrepareAdvertisingRequest()
mAdvertisingRequest.minInterval = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MIN;
mAdvertisingRequest.maxInterval = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MAX;
}
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
else if (mFlags.Has(Flags::kExtendedAdvertisingEnabled))
{
mAdvertisingRequest.minInterval = CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN;
Expand Down Expand Up @@ -421,7 +421,7 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising()
System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME),
HandleSlowBLEAdvertisementInterval, this);

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
// Start timer to schedule start of the extended advertising
DeviceLayer::SystemLayer().StartTimer(
System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS),
Expand All @@ -443,7 +443,7 @@ CHIP_ERROR BLEManagerImpl::StopAdvertising()
mFlags.Clear(Flags::kAdvertising);
mFlags.Set(Flags::kFastAdvertisingEnabled, true);

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
mFlags.Clear(Flags::kExtendedAdvertisingEnabled);
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/platform/Zephyr/CHIPDevicePlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
#endif // CONFIG_CHIP_EXTENDED_DISCOVERY

#ifdef CONFIG_CHIP_BLE_EXT_ADVERTISING
#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING 1
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING 1
#endif // CONFIG_CHIP_BLE_EXT_ADVERTISING

#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS (CONFIG_CHIP_BLE_ADVERTISING_DURATION * 60)
2 changes: 1 addition & 1 deletion src/platform/nrfconnect/CHIPDevicePlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@
#endif // CONFIG_CHIP_EXTENDED_DISCOVERY

#ifdef CONFIG_CHIP_BLE_EXT_ADVERTISING
#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING 1
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING 1
#endif // CONFIG_CHIP_BLE_EXT_ADVERTISING

#define CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS (CONFIG_CHIP_BLE_ADVERTISING_DURATION * 60)
Expand Down
2 changes: 1 addition & 1 deletion src/platform/silabs/CHIPDevicePlatformConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@

#define CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE 25

#define CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING SL_MATTER_BLE_EXTENDED_ADV
#define CHIP_DEVICE_CONFIG_EXT_ADVERTISING SL_MATTER_BLE_EXTENDED_ADV

/*
ICD Configuration Defines
Expand Down
8 changes: 4 additions & 4 deletions src/platform/silabs/efr32/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ CHIP_ERROR BLEManagerImpl::ConfigureAdvertisingData(void)
advData[index++] = ShortUUID_CHIPoBLEService[0]; // AD value
advData[index++] = ShortUUID_CHIPoBLEService[1];

#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
// Check for extended advertisement interval and redact VID/PID if past the initial period.
if (mFlags.Has(Flags::kExtAdvertisingEnabled))
{
Expand Down Expand Up @@ -553,7 +553,7 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void)
}
else
{
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
if (!mFlags.Has(Flags::kExtAdvertisingEnabled))
{
interval_min = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN;
Expand Down Expand Up @@ -993,12 +993,12 @@ void BLEManagerImpl::BleAdvTimeoutHandler(void * arg)
ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start slow advertisement");
BLEMgrImpl().mFlags.Set(Flags::kAdvertising);
BLEMgr().SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising);
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
BLEMgrImpl().mFlags.Clear(Flags::kExtAdvertisingEnabled);
BLEMgrImpl().StartBleAdvTimeoutTimer(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_CHANGE_TIME_MS);
#endif
}
#if CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING
#if CHIP_DEVICE_CONFIG_EXT_ADVERTISING
else
{
ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start extended advertisement");
Expand Down

0 comments on commit ab896f8

Please sign in to comment.