Rewrite discover-once to really stop right after the first PASE sessi…

…on failure (#25176)

examples/chip-tool/commands/pairing/PairingCommand.cpp

@@ -109,8 +109,17 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
 
 CHIP_ERROR PairingCommand::PaseWithCode(NodeId remoteId)
 {
-    DiscoveryType discoveryType =
-        mUseOnlyOnNetworkDiscovery.ValueOr(false) ? DiscoveryType::kDiscoveryNetworkOnly : DiscoveryType::kAll;
+    auto discoveryType = DiscoveryType::kAll;
+    if (mUseOnlyOnNetworkDiscovery.ValueOr(false))
+    {
+        discoveryType = DiscoveryType::kDiscoveryNetworkOnly;
+    }
+
+    if (mDiscoverOnce.ValueOr(false))
+    {
+        discoveryType = DiscoveryType::kDiscoveryNetworkOnlyWithoutPASEAutoRetry;
+    }
+
     return CurrentCommissioner().EstablishPASEConnection(remoteId, mOnboardingPayload, discoveryType);
 }
 
@@ -126,7 +135,17 @@ CHIP_ERROR PairingCommand::PairWithCode(NodeId remoteId)
         auto wiFiCredentials   = commissioningParams.GetWiFiCredentials();
         mUseOnlyOnNetworkDiscovery.SetValue(!threadCredentials.HasValue() && !wiFiCredentials.HasValue());
     }
-    DiscoveryType discoveryType = mUseOnlyOnNetworkDiscovery.Value() ? DiscoveryType::kDiscoveryNetworkOnly : DiscoveryType::kAll;
+
+    auto discoveryType = DiscoveryType::kAll;
+    if (mUseOnlyOnNetworkDiscovery.ValueOr(false))
+    {
+        discoveryType = DiscoveryType::kDiscoveryNetworkOnly;
+    }
+
+    if (mDiscoverOnce.ValueOr(false))
+    {
+        discoveryType = DiscoveryType::kDiscoveryNetworkOnlyWithoutPASEAutoRetry;
+    }
 
     return CurrentCommissioner().PairDevice(remoteId, mOnboardingPayload, commissioningParams, discoveryType);
 }
@@ -195,17 +214,6 @@ void PairingCommand::OnStatusUpdate(DevicePairingDelegate::Status status)
         ChipLogError(chipTool, "Secure Pairing Failed");
         SetCommandExitStatus(CHIP_ERROR_INCORRECT_STATE);
         break;
-    case DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices:
-        if (IsDiscoverOnce())
-        {
-            ChipLogError(chipTool, "Secure Pairing Failed");
-            SetCommandExitStatus(CHIP_ERROR_INCORRECT_STATE);
-        }
-        else
-        {
-            ChipLogProgress(chipTool, "Secure Pairing Discovering More Devices");
-        }
-        break;
     }
 }
 

examples/chip-tool/commands/pairing/PairingCommand.h

@@ -158,7 +158,6 @@ class PairingCommand : public CHIPCommand,
 
     /////////// DeviceDiscoveryDelegate Interface /////////
     void OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
-    bool IsDiscoverOnce() { return mDiscoverOnce.ValueOr(false); }
 
     /////////// DeviceAttestationDelegate /////////
     chip::Optional<uint16_t> FailSafeExpiryTimeoutSecs() const override;

examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm

@@ -36,7 +36,7 @@ - (void)onStatusUpdate:(MTRCommissioningStatus)status
         _commandBridge->SetCommandExitStatus(CHIP_ERROR_INCORRECT_STATE);
         break;
     case MTRCommissioningStatusDiscoveringMoreDevices:
-        ChipLogProgress(chipTool, "Secure Pairing Discovering More Devices");
+        ChipLogError(chipTool, "MTRCommissioningStatusDiscoveringMoreDevices: This should not happen.");
         break;
     case MTRCommissioningStatusUnknown:
         ChipLogError(chipTool, "Uknown Pairing Status");

examples/platform/linux/CommissionerMain.cpp

@@ -277,9 +277,6 @@ void PairingCommand::OnStatusUpdate(DevicePairingDelegate::Status status)
     case DevicePairingDelegate::Status::SecurePairingFailed:
         ChipLogError(AppServer, "Secure Pairing Failed");
         break;
-    case DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices:
-        ChipLogProgress(AppServer, "Secure Pairing Discovering More Device");
-        break;
     }
 }
 

src/app/tests/suites/commands/commissioner/CommissionerCommands.cpp

@@ -33,11 +33,13 @@ CommissionerCommands::PairWithCode(const char * identity,
     memcpy(code, value.payload.data(), value.payload.size());
     ChipLogError(chipTool, "Pairing Code is %s", code);
 
-    mDiscoverOnce = value.discoverOnce;
-
     // To reduce the scanning latency in some setups, and since the primary use for PairWithCode is to commission a device to
     // another commissioner, assume that the commissionable device is available on the network.
-    chip::Controller::DiscoveryType discoveryType = chip::Controller::DiscoveryType::kDiscoveryNetworkOnly;
+    auto discoveryType = chip::Controller::DiscoveryType::kDiscoveryNetworkOnly;
+    if (value.discoverOnce.ValueOr(false))
+    {
+        discoveryType = chip::Controller::DiscoveryType::kDiscoveryNetworkOnlyWithoutPASEAutoRetry;
+    }
     return GetCommissioner(identity).PairDevice(value.nodeId, code, discoveryType);
 }
 
@@ -129,13 +131,6 @@ void CommissionerCommands::OnStatusUpdate(DevicePairingDelegate::Status status)
         ChipLogError(chipTool, "Secure Pairing Failed");
         OnResponse(ConvertToStatusIB(CHIP_ERROR_INCORRECT_STATE), nullptr);
         break;
-    case DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices:
-        if (mDiscoverOnce.ValueOr(false))
-        {
-            ChipLogError(chipTool, "Secure Pairing Failed");
-            OnResponse(ConvertToStatusIB(CHIP_ERROR_INCORRECT_STATE), nullptr);
-        }
-        break;
     }
 }
 

src/app/tests/suites/commands/commissioner/CommissionerCommands.h

@@ -45,7 +45,4 @@ class CommissionerCommands : public chip::Controller::DevicePairingDelegate
     void OnPairingComplete(CHIP_ERROR error) override;
     void OnPairingDeleted(CHIP_ERROR error) override;
     void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR error) override;
-
-private:
-    chip::Optional<bool> mDiscoverOnce;
 };

src/controller/DevicePairingDelegate.h

@@ -40,7 +40,6 @@ class DLL_EXPORT DevicePairingDelegate
     {
         SecurePairingSuccess = 0,
         SecurePairingFailed,
-        SecurePairingDiscoveringMoreDevices,
     };
 
     /**

src/controller/SetUpCodePairer.cpp

@@ -348,9 +348,20 @@ void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::Discover
     ChipLogProgress(Controller, "Discovered device to be commissioned over DNS-SD");
 
     auto & resolutionData = nodeData.resolutionData;
-    for (size_t i = 0; i < resolutionData.numIPs; i++)
+
+    if (mDiscoveryType == DiscoveryType::kDiscoveryNetworkOnlyWithoutPASEAutoRetry)
+    {
+        // If the discovery type does not want the PASE auto retry mechanism, we will just store
+        // a single IP. So the discovery process is stopped as it won't be of any help anymore.
+        StopConnectOverIP();
+        mDiscoveredParameters.emplace_back(nodeData.resolutionData, 0);
+    }
+    else
     {
-        mDiscoveredParameters.emplace_back(nodeData.resolutionData, i);
+        for (size_t i = 0; i < resolutionData.numIPs; i++)
+        {
+            mDiscoveredParameters.emplace_back(nodeData.resolutionData, i);
+        }
     }
 
     ConnectToDiscoveredDevice();
@@ -447,14 +458,14 @@ void SetUpCodePairer::OnStatusUpdate(DevicePairingDelegate::Status status)
         if (!mDiscoveredParameters.empty())
         {
             ChipLogProgress(Controller, "Ignoring SecurePairingFailed status for now; we have more discovered devices to try");
-            status = DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices;
+            return;
         }
 
         if (DiscoveryInProgress())
         {
             ChipLogProgress(Controller,
                             "Ignoring SecurePairingFailed status for now; we are waiting to see if we discover more devices");
-            status = DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices;
+            return;
         }
     }
 

src/controller/SetUpCodePairer.h

@@ -68,6 +68,7 @@ enum class SetupCodePairerBehaviour : uint8_t
 enum class DiscoveryType : uint8_t
 {
     kDiscoveryNetworkOnly,
+    kDiscoveryNetworkOnlyWithoutPASEAutoRetry,
     kAll,
 };
 

src/controller/python/ChipDeviceController-ScriptDevicePairingDelegate.cpp

@@ -84,8 +84,6 @@ void ScriptDevicePairingDelegate::OnStatusUpdate(DevicePairingDelegate::Status s
             mOnPairingCompleteCallback(ToPyChipError(CHIP_ERROR_INCORRECT_STATE));
         }
         break;
-    case DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices:
-        break;
     }
 }
 

src/darwin/Framework/CHIP/MTRDeviceControllerDelegate.h

@@ -23,7 +23,8 @@ typedef NS_ENUM(NSInteger, MTRCommissioningStatus) {
     MTRCommissioningStatusUnknown = 0,
     MTRCommissioningStatusSuccess = 1,
     MTRCommissioningStatusFailed = 2,
-    MTRCommissioningStatusDiscoveringMoreDevices = 3
+    MTRCommissioningStatusDiscoveringMoreDevices MTR_NEWLY_DEPRECATED("MTRCommissioningStatusDiscoveringMoreDevices is not used.")
+    = 3,
 } API_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
 
 @class MTRDeviceController;
@@ -64,8 +65,8 @@ typedef NS_ENUM(NSUInteger, MTRPairingStatus) {
     MTRPairingStatusFailed API_DEPRECATED(
         "Please use MTRCommissioningStatusFailed", ios(16.1, 16.4), macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4))
     = 2,
-    MTRPairingStatusDiscoveringMoreDevices API_DEPRECATED("Please use MTRCommissioningStatusDiscoveringMoreDevices",
-        ios(16.1, 16.4), macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4))
+    MTRPairingStatusDiscoveringMoreDevices API_DEPRECATED("MTRPairingStatusDiscoveringMoreDevices is not used.", ios(16.1, 16.4),
+        macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4))
     = 3
 } API_DEPRECATED("Please use MTRCommissioningStatus", ios(16.1, 16.4), macos(13.0, 13.3), watchos(9.1, 9.4), tvos(16.1, 16.4));
 

src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm

@@ -51,9 +51,6 @@
     case chip::Controller::DevicePairingDelegate::Status::SecurePairingFailed:
         rv = MTRCommissioningStatusFailed;
         break;
-    case chip::Controller::DevicePairingDelegate::Status::SecurePairingDiscoveringMoreDevices:
-        rv = MTRCommissioningStatusDiscoveringMoreDevices;
-        break;
     }
     return rv;
 }