Stop duplicating advertisement timeout handling. (#20007)

The commissioning window manager handles advertisement start/stop.  We
don't need per-advertisement-mechanism duplication of timeout timers.

Specific changes:

1) Remove the "stop advertisement" timing from the (few) platform BLE managers
   that did it.
2) Remove CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT altogether.
3) Remove commissioning advertisement timing from Dnssd.cpp.
4) Remove the Linux shell command for setting the "discovery timeout", because
   it's really not clear what it's supposed to do and how it should work.

Fixes #16693

examples/all-clusters-app/nxp/mw320/main.cpp

@@ -974,9 +974,6 @@ static void run_chip_srv(System::Layer * aSystemLayer, void * aAppState)
         // uint16_t unsecurePort = CHIP_UDC_PORT;
 
         // PRINTF("==> call chip::Server() \r\n");
-        // PRINTF("Orig DNSS Discovery Timeout: %d sec \r\n", chip::app::DnssdServer::Instance().GetDiscoveryTimeoutSecs());
-        // chip::app::DnssdServer::Instance().SetDiscoveryTimeoutSecs(60);
-        // chip::app::DnssdServer::Instance().SetDiscoveryTimeoutSecs(30);
         // chip::Server::GetInstance().Init(nullptr, securePort, unsecurePort);
 
         static chip::CommonCaseDeviceServerInitParams initParams;

examples/lighting-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h

@@ -118,16 +118,6 @@
  */
 #define CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_TIMEOUT (30 * 1000)
 
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of slow advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)
-
 /**
  *  @def CHIP_CONFIG_MAX_FABRICS
  *

examples/lock-app/nxp/k32w/k32w0/include/CHIPProjectConfig.h

@@ -118,16 +118,6 @@
  */
 #define CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_TIMEOUT (30 * 1000)
 
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of slow advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)
-
 /**
  *  @def CHIP_CONFIG_MAX_FABRICS
  *

examples/pigweed-app/efr32/include/CHIPProjectConfig.h

@@ -150,13 +150,3 @@
  * 30000 (30 secondes).
  */
 #define CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_TIMEOUT (30 * 1000)
-
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of slow advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)

examples/platform/linux/CommissioneeShellCommands.cpp

@@ -61,8 +61,14 @@ static CHIP_ERROR PrintAllCommands()
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
     streamer_printf(sout, "  sendudc <address> <port>   Send UDC message to address. Usage: commissionee sendudc ::1 5543\r\n");
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
+    // TODO: Figure out whether setdiscoverytimeout is a reasonable thing to do
+    // at all, and if so what semantics it should have.  Presumably it should
+    // affect BLE discovery too, not just DNS-SD.  How should it interact with
+    // explicit timeouts specified in OpenCommissioningWindow?
+#if 0
     streamer_printf(
         sout, "  setdiscoverytimeout <timeout>   Set discovery timeout in seconds. Usage: commissionee setdiscoverytimeout 30\r\n");
+#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
     streamer_printf(sout,
                     "  setextendeddiscoverytimeout <timeout>   Set extendeddiscovery timeout in seconds. Usage: commissionee "
@@ -94,15 +100,21 @@ static CHIP_ERROR CommissioneeHandler(int argc, char ** argv)
         return SendUDC(true, chip::Transport::PeerAddress::UDP(commissioner, port));
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
+    // TODO: Figure out whether setdiscoverytimeout is a reasonable thing to do
+    // at all, and if so what semantics it should have.  Presumably it should
+    // affect BLE discovery too, not just DNS-SD.  How should it interact with
+    // explicit timeouts specified in OpenCommissioningWindow?
+#if 0
     if (strcmp(argv[0], "setdiscoverytimeout") == 0)
     {
         char * eptr;
         int16_t timeout = (int16_t) strtol(argv[1], &eptr, 10);
         chip::app::DnssdServer::Instance().SetDiscoveryTimeoutSecs(timeout);
         return CHIP_NO_ERROR;
     }
+#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
-    else if (strcmp(argv[0], "setextendeddiscoverytimeout") == 0)
+    if (strcmp(argv[0], "setextendeddiscoverytimeout") == 0)
     {
         char * eptr;
         int16_t timeout = (int16_t) strtol(argv[1], &eptr, 10);

examples/shell/nxp/k32w/k32w0/include/CHIPProjectConfig.h

@@ -125,16 +125,6 @@
  */
 #define CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_TIMEOUT (30 * 1000)
 
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of slow advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)
-
 /**
  *  @def CHIP_CONFIG_MAX_FABRICS
  *

examples/window-app/efr32/include/CHIPProjectConfig.h

@@ -154,16 +154,6 @@
  */
 #define CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_TIMEOUT (30 * 1000)
 
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of slow advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)
-
 /**
  *  @def CHIP_CONFIG_MRP_LOCAL_ACTIVE_RETRY_INTERVAL
  *

src/app/server/Dnssd.cpp

@@ -107,12 +107,6 @@ void DnssdServer::OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, vo
 }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
 
-/// Callback from Discovery Expiration timer
-void HandleDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState)
-{
-    DnssdServer::Instance().OnDiscoveryExpiration(aSystemLayer, aAppState);
-}
-
 bool DnssdServer::OnExpiration(System::Clock::Timestamp expirationMs)
 {
     if (expirationMs == kTimeoutCleared)
@@ -166,47 +160,6 @@ bool DnssdServer::OnExpiration(System::Clock::Timestamp expirationMs)
     return true;
 }
 
-void DnssdServer::OnDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState)
-{
-    if (!DnssdServer::OnExpiration(mDiscoveryExpiration))
-    {
-        ChipLogDetail(Discovery, "OnDiscoveryExpiration callback for cleared session");
-        return;
-    }
-
-    ChipLogDetail(Discovery, "OnDiscoveryExpiration callback for valid session");
-
-#if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
-    int32_t extTimeout = GetExtendedDiscoveryTimeoutSecs();
-    if (extTimeout != CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED)
-    {
-        CHIP_ERROR err = AdvertiseCommissionableNode(chip::Dnssd::CommissioningMode::kDisabled);
-        if (err != CHIP_NO_ERROR)
-        {
-            ChipLogError(Discovery, "Failed to advertise extended commissionable node: %" CHIP_ERROR_FORMAT, err.Format());
-        }
-        // set timeout
-        ScheduleExtendedDiscoveryExpiration();
-    }
-#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
-
-    mDiscoveryExpiration = kTimeoutCleared;
-}
-
-CHIP_ERROR DnssdServer::ScheduleDiscoveryExpiration()
-{
-    if (mDiscoveryTimeoutSecs == CHIP_DEVICE_CONFIG_DISCOVERY_NO_TIMEOUT)
-    {
-        return CHIP_NO_ERROR;
-    }
-    ChipLogDetail(Discovery, "Scheduling discovery timeout in %ds", mDiscoveryTimeoutSecs);
-
-    mDiscoveryExpiration = mTimeSource.GetMonotonicTimestamp() + System::Clock::Seconds16(mDiscoveryTimeoutSecs);
-
-    return DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds16(mDiscoveryTimeoutSecs), HandleDiscoveryExpiration,
-                                                 nullptr);
-}
-
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
 CHIP_ERROR DnssdServer::ScheduleExtendedDiscoveryExpiration()
 {
@@ -455,13 +408,6 @@ void DnssdServer::StartServer(Dnssd::CommissioningMode mode)
         {
             ChipLogError(Discovery, "Failed to advertise commissionable node: %" CHIP_ERROR_FORMAT, err.Format());
         }
-
-        // If any fabrics exist, the commissioning window must have been opened by the administrator
-        // commissioning cluster commands which take care of the timeout.
-        if (!HaveOperationalCredentials())
-        {
-            ScheduleDiscoveryExpiration();
-        }
     }
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
     else if (GetExtendedDiscoveryTimeoutSecs() != CHIP_DEVICE_CONFIG_DISCOVERY_DISABLED)

src/app/server/Dnssd.h

@@ -59,12 +59,6 @@ class DLL_EXPORT DnssdServer
     /// Gets the interface id used for advertising
     Inet::InterfaceId GetInterfaceId() { return mInterfaceId; }
 
-    /// Sets the factory-new state commissionable node discovery timeout
-    void SetDiscoveryTimeoutSecs(int16_t secs) { mDiscoveryTimeoutSecs = secs; }
-
-    /// Gets the factory-new state commissionable node discovery timeout
-    int16_t GetDiscoveryTimeoutSecs() const { return mDiscoveryTimeoutSecs; }
-
     //
     // Override the referenced fabric table from the default that is present
     // in Server::GetInstance().GetFabricTable() to something else.
@@ -147,7 +141,6 @@ class DLL_EXPORT DnssdServer
 
     void ClearTimeouts()
     {
-        mDiscoveryExpiration = kTimeoutCleared;
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
         mExtendedDiscoveryExpiration = kTimeoutCleared;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
@@ -163,11 +156,6 @@ class DLL_EXPORT DnssdServer
     // Ephemeral discriminator to use instead of the default if set
     Optional<uint16_t> mEphemeralDiscriminator;
 
-    /// schedule next discovery expiration
-    CHIP_ERROR ScheduleDiscoveryExpiration();
-    int16_t mDiscoveryTimeoutSecs                 = CHIP_DEVICE_CONFIG_DISCOVERY_TIMEOUT_SECS;
-    System::Clock::Timestamp mDiscoveryExpiration = kTimeoutCleared;
-
     Optional<int32_t> mExtendedDiscoveryTimeoutSecs = NullOptional;
 
     /// return true if expirationMs is valid (not cleared and not in the future)

src/include/platform/CHIPDeviceConfig.h

@@ -584,18 +584,6 @@
 #define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME 30000
 #endif
 
-/**
- * CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
- *
- * The amount of time in miliseconds after which BLE advertisement should be disabled, counting
- * from the moment of advertisement commencement.
- *
- * Defaults to 9000000 (15 minutes).
- */
-#ifndef CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT
-#define CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT (15 * 60 * 1000)
-#endif
-
 // -------------------- Time Sync Configuration --------------------
 
 /**

src/platform/Ameba/BLEManagerImpl.cpp

@@ -127,8 +127,6 @@ const ChipBleUUID ChipUUID_CHIPoBLEChar_RX = { { 0x18, 0xEE, 0x2E, 0xF5, 0x26, 0
 const ChipBleUUID ChipUUID_CHIPoBLEChar_TX = { { 0x18, 0xEE, 0x2E, 0xF5, 0x26, 0x3D, 0x45, 0x59, 0x95, 0x9F, 0x4F, 0x9C, 0x42, 0x9F,
                                                  0x9D, 0x12 } };
 
-static constexpr System::Clock::Timeout kAdvertiseTimeout =
-    System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT);
 static constexpr System::Clock::Timeout kFastAdvertiseTimeout =
     System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME);
 System::Clock::Timestamp mAdvertiseStartTime;
@@ -380,7 +378,6 @@ CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val)
     if (val)
     {
         mAdvertiseStartTime = System::SystemClock().GetMonotonicTimestamp();
-        ReturnErrorOnFailure(DeviceLayer::SystemLayer().StartTimer(kAdvertiseTimeout, HandleAdvertisementTimer, this));
         ReturnErrorOnFailure(DeviceLayer::SystemLayer().StartTimer(kFastAdvertiseTimeout, HandleFastAdvertisementTimer, this));
     }
 
@@ -396,22 +393,6 @@ CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val)
     return err;
 }
 
-void BLEManagerImpl::HandleAdvertisementTimer(System::Layer * systemLayer, void * context)
-{
-    static_cast<BLEManagerImpl *>(context)->HandleAdvertisementTimer();
-}
-
-void BLEManagerImpl::HandleAdvertisementTimer()
-{
-    System::Clock::Timestamp currentTimestamp = System::SystemClock().GetMonotonicTimestamp();
-
-    if (currentTimestamp - mAdvertiseStartTime >= kAdvertiseTimeout)
-    {
-        mFlags.Set(Flags::kAdvertisingEnabled, 0);
-        PlatformMgr().ScheduleWork(DriveBLEState, 0);
-    }
-}
-
 void BLEManagerImpl::HandleFastAdvertisementTimer(System::Layer * systemLayer, void * context)
 {
     static_cast<BLEManagerImpl *>(context)->HandleFastAdvertisementTimer();

src/platform/Ameba/BLEManagerImpl.h

@@ -144,8 +144,6 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
 
     static void HandleFastAdvertisementTimer(System::Layer * systemLayer, void * context);
     void HandleFastAdvertisementTimer();
-    static void HandleAdvertisementTimer(System::Layer * systemLayer, void * context);
-    void HandleAdvertisementTimer();
 
     void HandleRXCharWrite(uint8_t *, uint16_t, uint8_t);
     void HandleTXCharRead(void * param);

src/platform/EFR32/BLEManagerImpl.cpp

@@ -691,7 +691,6 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void)
     sl_status_t ret;
     uint32_t interval_min;
     uint32_t interval_max;
-    uint32_t BleAdvTimeoutMs;
     uint16_t numConnectionss = NumConnections();
     uint8_t connectableAdv =
         (numConnectionss < kMaxConnections) ? sl_bt_advertiser_connectable_scannable : sl_bt_advertiser_scannable_non_connectable;
@@ -718,15 +717,13 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void)
 
     if (mFlags.Has(Flags::kFastAdvertisingEnabled))
     {
-        interval_min    = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MIN;
-        interval_max    = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MAX;
-        BleAdvTimeoutMs = CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME;
+        interval_min = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MIN;
+        interval_max = CHIP_DEVICE_CONFIG_BLE_FAST_ADVERTISING_INTERVAL_MAX;
     }
     else
     {
-        interval_min    = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN;
-        interval_max    = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX;
-        BleAdvTimeoutMs = CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT;
+        interval_min = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MIN;
+        interval_max = CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX;
     }
 
     ret = sl_bt_advertiser_set_timing(advertising_set_handle, interval_min, interval_max, 0, 0);
@@ -738,7 +735,10 @@ CHIP_ERROR BLEManagerImpl::StartAdvertising(void)
 
     if (SL_STATUS_OK == ret)
     {
-        StartBleAdvTimeoutTimer(BleAdvTimeoutMs);
+        if (mFlags.Has(Flags::kFastAdvertisingEnabled))
+        {
+            StartBleAdvTimeoutTimer(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME);
+        }
         mFlags.Set(Flags::kAdvertising);
     }
 
@@ -1076,12 +1076,6 @@ void BLEManagerImpl::BleAdvTimeoutHandler(TimerHandle_t xTimer)
         ChipLogDetail(DeviceLayer, "bleAdv Timeout : Start slow advertissment");
         BLEMgr().SetAdvertisingMode(BLEAdvertisingMode::kSlowAdvertising);
     }
-    else if (BLEMgrImpl().mFlags.Has(Flags::kAdvertising))
-    {
-        // Advertisement time expired. Stop advertising
-        ChipLogDetail(DeviceLayer, "bleAdv Timeout : Stop advertissement");
-        BLEMgr().SetAdvertisingEnabled(false);
-    }
 }
 
 void BLEManagerImpl::CancelBleAdvTimeoutTimer(void)

src/platform/ESP32/BLEManagerImpl.h

@@ -202,16 +202,12 @@ class BLEManagerImpl final : public BLEManager,
     CHIP_ERROR ConfigureAdvertisingData(void);
     CHIP_ERROR StartAdvertising(void);
 
-    static constexpr System::Clock::Timeout kAdvertiseTimeout =
-        System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_TIMEOUT);
     static constexpr System::Clock::Timeout kFastAdvertiseTimeout =
         System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_BLE_ADVERTISING_INTERVAL_CHANGE_TIME);
     System::Clock::Timestamp mAdvertiseStartTime;
 
     static void HandleFastAdvertisementTimer(System::Layer * systemLayer, void * context);
     void HandleFastAdvertisementTimer();
-    static void HandleAdvertisementTimer(System::Layer * systemLayer, void * context);
-    void HandleAdvertisementTimer();
 
 #if CONFIG_BT_BLUEDROID_ENABLED
     void HandleGATTControlEvent(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t * param);