Fixed edge case and added unit tests

Author: jtung-apple

src/controller/SetUpCodePairer.cpp

@@ -901,12 +901,13 @@ void SetUpCodePairer::OnDeviceDiscoveredTimeoutCallback(System::Layer * layer, v
     ChipLogError(Controller, "Discovery timed out");
     auto * pairer = static_cast<SetUpCodePairer *>(context);
 
-    // If a PASE attempt is in progress, do not stop any discovery — give every
-    // transport that has started (DNS-SD, BLE, Wi-Fi PAF, NFC) a chance to
-    // complete, fail, or time out naturally.  DNS-SD will be stopped in
-    // OnPairingComplete once the PASE attempt it triggered completes.
+    // If a PASE attempt is in progress, do not stop physical-proximity
+    // transports (BLE, Wi-Fi PAF, NFC) — they have their own completion/timeout
+    // mechanisms.  DNS-SD, however, runs indefinitely, so stop it now to
+    // prevent DiscoveryInProgress() from being true forever.
     if (pairer->mWaitingForPASE)
     {
+        LogErrorOnFailure(pairer->StopDiscoveryOverDNSSD());
         return;
     }
 

src/controller/SetUpCodePairer.h

@@ -50,6 +50,13 @@
 #include <vector>
 
 namespace chip {
+
+namespace Testing {
+
+class SetUpCodePairerTestAccess;
+
+} // namespace Testing
+
 namespace Controller {
 
 class DeviceCommissioner;
@@ -98,6 +105,8 @@ class DLL_EXPORT SetUpCodePairer : public DevicePairingDelegate
                                    public Nfc::NFCReaderTransportDelegate
 #endif
 {
+    friend class chip::Testing::SetUpCodePairerTestAccess;
+
 public:
     SetUpCodePairer(DeviceCommissioner * commissioner) : mCommissioner(commissioner) {}
     virtual ~SetUpCodePairer() {}

src/controller/tests/BUILD.gn

@@ -52,6 +52,7 @@ chip_test_suite("tests") {
       test_sources += [
         "TestCommissioningWindowOpener.cpp",
         "TestExampleOperationalCredentialsIssuer.cpp",
+        "TestSetUpCodePairer.cpp",
       ]
     }
   }
@@ -64,7 +65,10 @@ chip_test_suite("tests") {
 
   cflags = [ "-Wconversion" ]
 
-  sources = [ "AutoCommissionerTestAccess.h" ]
+  sources = [
+    "AutoCommissionerTestAccess.h",
+    "SetUpCodePairerTestAccess.h",
+  ]
 
   public_deps = [
     "${chip_root}/src/app/common:cluster-objects",

src/controller/tests/SetUpCodePairerTestAccess.h

@@ -0,0 +1,53 @@
+/*
+ *
+ *    Copyright (c) 2025 Project CHIP Authors
+ *    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.
+ */
+#pragma once
+
+#include <controller/SetUpCodePairer.h>
+
+namespace chip {
+namespace Testing {
+
+// Provides access to private members of SetUpCodePairer for testing.
+class SetUpCodePairerTestAccess
+{
+public:
+    SetUpCodePairerTestAccess() = delete;
+    explicit SetUpCodePairerTestAccess(Controller::SetUpCodePairer * pairer) : mPairer(pairer) {}
+
+    // Re-export private transport type indices for use in tests.
+    static constexpr int kBLETransport     = Controller::SetUpCodePairer::kBLETransport;
+    static constexpr int kIPTransport      = Controller::SetUpCodePairer::kIPTransport;
+    static constexpr int kWiFiPAFTransport = Controller::SetUpCodePairer::kWiFiPAFTransport;
+
+    bool GetWaitingForDiscovery(int transport) const { return mPairer->mWaitingForDiscovery[transport]; }
+    void SetWaitingForDiscovery(int transport, bool val) { mPairer->mWaitingForDiscovery[transport] = val; }
+
+    bool GetWaitingForPASE() const { return mPairer->mWaitingForPASE; }
+    void SetWaitingForPASE(bool val) { mPairer->mWaitingForPASE = val; }
+
+    NodeId GetRemoteId() const { return mPairer->mRemoteId; }
+    void SetRemoteId(NodeId id) { mPairer->mRemoteId = id; }
+
+    void FireTimeoutCallback() { Controller::SetUpCodePairer::OnDeviceDiscoveredTimeoutCallback(nullptr, mPairer); }
+
+private:
+    Controller::SetUpCodePairer * mPairer = nullptr;
+};
+
+} // namespace Testing
+} // namespace chip

src/controller/tests/TestSetUpCodePairer.cpp

@@ -0,0 +1,81 @@
+/*
+ *
+ *    Copyright (c) 2025 Project CHIP Authors
+ *    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.
+ */
+
+#include <gtest/gtest.h>
+
+#include <controller/CHIPDeviceController.h>
+#include <controller/SetUpCodePairer.h>
+#include <controller/tests/SetUpCodePairerTestAccess.h>
+#include <lib/core/CHIPError.h>
+
+using namespace chip;
+using namespace chip::Controller;
+using PairerAccess = chip::Testing::SetUpCodePairerTestAccess;
+
+namespace {
+
+class TestSetUpCodePairer : public ::testing::Test
+{
+public:
+    static void SetUpTestSuite() { ASSERT_EQ(Platform::MemoryInit(), CHIP_NO_ERROR); }
+    static void TearDownTestSuite() { Platform::MemoryShutdown(); }
+
+protected:
+    DeviceCommissioner commissioner;
+    SetUpCodePairer pairer{ &commissioner };
+    PairerAccess access{ &pairer };
+};
+
+// When the discovery timeout fires while a PASE attempt is in progress,
+// DNS-SD should be stopped (it runs indefinitely) but other transports
+// (BLE, Wi-Fi PAF, NFC) should be left alone since they self-terminate.
+TEST_F(TestSetUpCodePairer, TimeoutDuringPASE_StopsDNSSD_PreservesOtherTransports)
+{
+    access.SetRemoteId(1);
+    access.SetWaitingForPASE(true);
+    access.SetWaitingForDiscovery(PairerAccess::kIPTransport, true);
+    access.SetWaitingForDiscovery(PairerAccess::kBLETransport, true);
+
+    access.FireTimeoutCallback();
+
+    // DNS-SD must be stopped to prevent DiscoveryInProgress() from being stuck true.
+    EXPECT_FALSE(access.GetWaitingForDiscovery(PairerAccess::kIPTransport));
+    // BLE must be preserved — it may still discover a commissionee.
+    EXPECT_TRUE(access.GetWaitingForDiscovery(PairerAccess::kBLETransport));
+    // PASE state must not be disturbed.
+    EXPECT_TRUE(access.GetWaitingForPASE());
+}
+
+// When the discovery timeout fires with no PASE in progress,
+// all transports should be stopped and failure reported.
+TEST_F(TestSetUpCodePairer, TimeoutNoPASE_StopsAllTransports)
+{
+    access.SetRemoteId(1);
+    access.SetWaitingForPASE(false);
+    access.SetWaitingForDiscovery(PairerAccess::kIPTransport, true);
+    access.SetWaitingForDiscovery(PairerAccess::kBLETransport, true);
+
+    access.FireTimeoutCallback();
+
+    EXPECT_FALSE(access.GetWaitingForDiscovery(PairerAccess::kIPTransport));
+    EXPECT_FALSE(access.GetWaitingForDiscovery(PairerAccess::kBLETransport));
+    // StopPairingIfTransportsExhausted should have reported failure and cleared mRemoteId.
+    EXPECT_EQ(access.GetRemoteId(), kUndefinedNodeId);
+}
+
+} // namespace