Fix tracking of Wi-Fi PAF discovery state in SetUpCodePairer. (#38623)
There were two problems:
1. The boolean indicating Wi-Fi PAF discovery is in progress could get set even
though discovery had not started (e.g. because there was only a short
discriminator).
2. Discovery timeout did not cancel Wi-Fi PAF discovery.
The rest of the changes are fixing the naming of things and refactoring the
"stop discovery" code to hopefully make problems like this less likely.
Fixes https://github.com/project-chip/connectedhomeip/issues/38425
diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp
index 5faf347..8449936 100644
--- a/src/controller/SetUpCodePairer.cpp
+++ b/src/controller/SetUpCodePairer.cpp
@@ -115,7 +115,7 @@
{
if (searchOverAll || payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kBLE))
{
- if (CHIP_NO_ERROR == (err = StartDiscoverOverBle(payload)))
+ if (CHIP_NO_ERROR == (err = StartDiscoveryOverBLE(payload)))
{
isRunning = true;
}
@@ -124,7 +124,7 @@
if (searchOverAll || payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kSoftAP))
{
- if (CHIP_NO_ERROR == (err = StartDiscoverOverSoftAP(payload)))
+ if (CHIP_NO_ERROR == (err = StartDiscoveryOverSoftAP(payload)))
{
isRunning = true;
}
@@ -133,7 +133,7 @@
if (searchOverAll || payload.rendezvousInformation.Value().Has(RendezvousInformationFlag::kWiFiPAF))
{
ChipLogProgress(Controller, "WiFi-PAF: has RendezvousInformationFlag::kWiFiPAF");
- if (CHIP_NO_ERROR == (err = StartDiscoverOverWiFiPAF(payload)))
+ if (CHIP_NO_ERROR == (err = StartDiscoveryOverWiFiPAF(payload)))
{
isRunning = true;
}
@@ -143,7 +143,7 @@
// We always want to search on network because any node that has already been commissioned will use on-network regardless of the
// QR code flag.
- if (CHIP_NO_ERROR == (err = StartDiscoverOverIP(payload)))
+ if (CHIP_NO_ERROR == (err = StartDiscoveryOverDNSSD(payload)))
{
isRunning = true;
}
@@ -152,7 +152,7 @@
return isRunning ? CHIP_NO_ERROR : CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
}
-CHIP_ERROR SetUpCodePairer::StartDiscoverOverBle(SetupPayload & payload)
+CHIP_ERROR SetUpCodePairer::StartDiscoveryOverBLE(SetupPayload & payload)
{
#if CONFIG_NETWORK_LAYER_BLE
#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
@@ -177,7 +177,7 @@
#endif // CONFIG_NETWORK_LAYER_BLE
}
-CHIP_ERROR SetUpCodePairer::StopConnectOverBle()
+CHIP_ERROR SetUpCodePairer::StopDiscoveryOverBLE()
{
// Make sure to not call CancelBleIncompleteConnection unless we are in fact
// waiting on BLE discovery. It will cancel connections that are in fact
@@ -201,7 +201,7 @@
#endif // CONFIG_NETWORK_LAYER_BLE
}
-CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(SetupPayload & payload)
+CHIP_ERROR SetUpCodePairer::StartDiscoveryOverDNSSD(SetupPayload & payload)
{
ChipLogProgress(Controller, "Starting commissioning discovery over DNS-SD");
@@ -229,7 +229,7 @@
return err;
}
-CHIP_ERROR SetUpCodePairer::StopConnectOverIP()
+CHIP_ERROR SetUpCodePairer::StopDiscoveryOverDNSSD()
{
ChipLogDetail(Controller, "Stopping commissioning discovery over DNS-SD");
@@ -242,23 +242,22 @@
return CHIP_NO_ERROR;
}
-CHIP_ERROR SetUpCodePairer::StartDiscoverOverSoftAP(SetupPayload & payload)
+CHIP_ERROR SetUpCodePairer::StartDiscoveryOverSoftAP(SetupPayload & payload)
{
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
}
-CHIP_ERROR SetUpCodePairer::StopConnectOverSoftAP()
+CHIP_ERROR SetUpCodePairer::StopDiscoveryOverSoftAP()
{
mWaitingForDiscovery[kSoftAPTransport] = false;
return CHIP_NO_ERROR;
}
-CHIP_ERROR SetUpCodePairer::StartDiscoverOverWiFiPAF(SetupPayload & payload)
+CHIP_ERROR SetUpCodePairer::StartDiscoveryOverWiFiPAF(SetupPayload & payload)
{
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
ChipLogProgress(Controller, "Starting commissioning discovery over WiFiPAF");
VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE);
- mWaitingForDiscovery[kWiFiPAFTransport] = true;
const SetupDiscriminator connDiscriminator(payload.discriminator);
VerifyOrReturnValue(!connDiscriminator.IsShortDiscriminator(), CHIP_ERROR_INVALID_ARGUMENT,
@@ -269,6 +268,8 @@
.discriminator = discriminator };
ReturnErrorOnFailure(
DeviceLayer::ConnectivityMgr().GetWiFiPAF()->AddPafSession(WiFiPAF::PafInfoAccess::kAccNodeInfo, sessionInfo));
+
+ mWaitingForDiscovery[kWiFiPAFTransport] = true;
CHIP_ERROR err = DeviceLayer::ConnectivityMgr().WiFiPAFSubscribe(discriminator, (void *) this, OnWiFiPAFSubscribeComplete,
OnWiFiPAFSubscribeError);
if (err != CHIP_NO_ERROR)
@@ -282,7 +283,7 @@
#endif // CONFIG_NETWORK_LAYER_BLE
}
-CHIP_ERROR SetUpCodePairer::StopConnectOverWiFiPAF()
+CHIP_ERROR SetUpCodePairer::StopDiscoveryOverWiFiPAF()
{
mWaitingForDiscovery[kWiFiPAFTransport] = false;
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
@@ -488,7 +489,7 @@
{
// 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();
+ StopDiscoveryOverDNSSD();
mDiscoveredParameters.emplace_back(resolutionData, 0);
}
else
@@ -547,18 +548,23 @@
return false;
}
-void SetUpCodePairer::ResetDiscoveryState()
+void SetUpCodePairer::StopAllDiscoveryAttempts()
{
- StopConnectOverBle();
- StopConnectOverIP();
- StopConnectOverSoftAP();
- StopConnectOverWiFiPAF();
+ LogErrorOnFailure(StopDiscoveryOverBLE());
+ LogErrorOnFailure(StopDiscoveryOverDNSSD());
+ LogErrorOnFailure(StopDiscoveryOverSoftAP());
+ LogErrorOnFailure(StopDiscoveryOverWiFiPAF());
// Just in case any of those failed to reset the waiting state properly.
for (auto & waiting : mWaitingForDiscovery)
{
waiting = false;
}
+}
+
+void SetUpCodePairer::ResetDiscoveryState()
+{
+ StopAllDiscoveryAttempts();
mDiscoveredParameters.clear();
mCurrentPASEParameters.ClearValue();
@@ -690,9 +696,7 @@
{
ChipLogError(Controller, "Discovery timed out");
auto * pairer = static_cast<SetUpCodePairer *>(context);
- LogErrorOnFailure(pairer->StopConnectOverBle());
- LogErrorOnFailure(pairer->StopConnectOverIP());
- LogErrorOnFailure(pairer->StopConnectOverSoftAP());
+ pairer->StopAllDiscoveryAttempts();
if (!pairer->mWaitingForPASE && pairer->mDiscoveredParameters.empty())
{
// We're not waiting on any more PASE attempts, and we're not going to
diff --git a/src/controller/SetUpCodePairer.h b/src/controller/SetUpCodePairer.h
index fe679e5..baa80bb 100644
--- a/src/controller/SetUpCodePairer.h
+++ b/src/controller/SetUpCodePairer.h
@@ -105,18 +105,22 @@
void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override;
CHIP_ERROR Connect(SetupPayload & paload);
- CHIP_ERROR StartDiscoverOverBle(SetupPayload & payload);
- CHIP_ERROR StopConnectOverBle();
- CHIP_ERROR StartDiscoverOverIP(SetupPayload & payload);
- CHIP_ERROR StopConnectOverIP();
- CHIP_ERROR StartDiscoverOverSoftAP(SetupPayload & payload);
- CHIP_ERROR StopConnectOverSoftAP();
- CHIP_ERROR StartDiscoverOverWiFiPAF(SetupPayload & payload);
- CHIP_ERROR StopConnectOverWiFiPAF();
+ CHIP_ERROR StartDiscoveryOverBLE(SetupPayload & payload);
+ CHIP_ERROR StopDiscoveryOverBLE();
+ CHIP_ERROR StartDiscoveryOverDNSSD(SetupPayload & payload);
+ CHIP_ERROR StopDiscoveryOverDNSSD();
+ CHIP_ERROR StartDiscoveryOverSoftAP(SetupPayload & payload);
+ CHIP_ERROR StopDiscoveryOverSoftAP();
+ CHIP_ERROR StartDiscoveryOverWiFiPAF(SetupPayload & payload);
+ CHIP_ERROR StopDiscoveryOverWiFiPAF();
// Returns whether we have kicked off a new connection attempt.
bool ConnectToDiscoveredDevice();
+ // Stop attempts to discover more things to connect to, but keep trying to
+ // connect to the ones we have already discovered.
+ void StopAllDiscoveryAttempts();
+
// Reset our mWaitingForDiscovery/mDiscoveredParameters state to indicate no
// pending work.
void ResetDiscoveryState();
