Add and Leverage Linux Connectivity Callback Introspection, Mutation, and Action Delegation Methods (project-chip#43062)

* Rename 'mpScanCallback' to 'mpOneShotScanCallback'.

This more accurately reflects its set-scan-and-clear usage.

* Documentation: Document the 'mpOneShotScanCallback' data member.

* Rename 'mpConnectCallback' to 'mpOneShotConnectCallback'.

This more accurately reflects its set-connect-and-clear usage.

* Documentation: Document the 'mpOneShotConnectCallback' data member.

* Added network commissioning-related action delegation methods.

These three methods consolidate the handling and dispatch of scan,
connect, and network status change action delegation callback
invocation and, for scan and connect, post-invocation nullification.

* Add parallel setters for all scan, connect, and status change callbacks.

This not only makes it easier to keep callback members private as
implementations fan out from the current, single wpa_supplicant
implementation but also reduces duplication among those
implementations for common callback-related operations.

* Initialize callbacks in the explicit initializer only.

Previously, the status change callback was initialized with a default
initializer and the connect and scan callbacks were initialized in the
explicit initializer.

With this change, all three are now initialized in the explicit
initializer.

* Added two connect- and scan-related callback introspection methods.

* Leverage callback introspection, mutation, and action delegation methods.

Author: gerickson

src/platform/Linux/ConnectivityManagerImpl.cpp

@@ -63,6 +63,25 @@ ConnectivityManagerImpl & ConnectivityManagerImpl::GetDefaultInstance()
     return sInstance;
 }
 
+// Linux Implementation-specific Methods
+
+// Mutation
+
+void ConnectivityManagerImpl::SetOneShotConnectCallback(OneShotConnectCallback * inOneShotConnectCallback) noexcept
+{
+    mpOneShotConnectCallback = inOneShotConnectCallback;
+}
+
+void ConnectivityManagerImpl::SetOneShotScanCallback(OneShotScanCallback * inOneShotScanCallback) noexcept
+{
+    mpOneShotScanCallback = inOneShotScanCallback;
+}
+
+void ConnectivityManagerImpl::SetNetworkStatusChangeCallback(NetworkStatusChangeCallback * inStatusChangeCallback) noexcept
+{
+    mpStatusChangeCallback = inStatusChangeCallback;
+}
+
 void ConnectivityManagerImpl::UpdateEthernetNetworkingStatus()
 {
     if (mpStatusChangeCallback != nullptr)
@@ -82,8 +101,9 @@ CHIP_ERROR ConnectivityManagerImpl::_Init()
     mWiFiStationMode              = kWiFiStationMode_Disabled;
     mWiFiStationReconnectInterval = System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_WIFI_STATION_RECONNECT_INTERVAL);
 #endif
-    mpConnectCallback = nullptr;
-    mpScanCallback    = nullptr;
+    SetNetworkStatusChangeCallback(nullptr);
+    SetOneShotConnectCallback(nullptr);
+    SetOneShotScanCallback(nullptr);
 
     if (ConnectivityUtils::GetEthInterfaceName(mEthIfName, Inet::InterfaceId::kMaxIfNameLength) == CHIP_NO_ERROR)
     {
@@ -128,6 +148,18 @@ CHIP_ERROR ConnectivityManagerImpl::_Init()
 #endif
 }
 
+#if CHIP_DEVICE_CONFIG_ENABLE_WIFI
+bool ConnectivityManagerImpl::IsWiFiStationConnecting() const noexcept
+{
+    return mpOneShotConnectCallback != nullptr;
+}
+
+bool ConnectivityManagerImpl::IsWiFiStationScanning() const noexcept
+{
+    return mpOneShotScanCallback != nullptr;
+}
+#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI
+
 void ConnectivityManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
 {
     // Forward the event to the generic base classes as needed.
@@ -179,5 +211,35 @@ ConnectivityManagerImpl & ConnectivityMgrImpl(void)
     return ConnectivityManagerImpl::GetDefaultInstance();
 }
 
+// Network Commissioning Action Delegation Methods
+
+void ConnectivityManagerImpl::OnScanFinished(NetworkCommissioning::Status inStatus, CharSpan inDebugText,
+                                             NetworkCommissioning::WiFiScanResponseIterator * inNetworks) noexcept
+{
+    VerifyOrReturn(mpOneShotScanCallback != nullptr);
+
+    mpOneShotScanCallback->OnFinished(inStatus, inDebugText, inNetworks);
+
+    mpOneShotScanCallback = nullptr;
+}
+
+void ConnectivityManagerImpl::OnConnectResult(NetworkCommissioning::Status inCommissioningError, CharSpan inDebugText,
+                                              int32_t inConnectStatus) noexcept
+{
+    VerifyOrReturn(mpOneShotConnectCallback != nullptr);
+
+    mpOneShotConnectCallback->OnResult(inCommissioningError, inDebugText, inConnectStatus);
+
+    mpOneShotConnectCallback = nullptr;
+}
+
+void ConnectivityManagerImpl::OnStatusChange(NetworkCommissioning::Status inCommissioningError, Optional<ByteSpan> inNetworkId,
+                                             Optional<int32_t> inConnectStatus) noexcept
+{
+    VerifyOrReturn(mpStatusChangeCallback != nullptr);
+
+    mpStatusChangeCallback->OnNetworkingStatusChange(inCommissioningError, inNetworkId, inConnectStatus);
+}
+
 } // namespace DeviceLayer
 } // namespace chip

src/platform/Linux/ConnectivityManagerImpl.h

@@ -183,11 +183,13 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
     const char * GetEthernetIfName() { return (mEthIfName[0] == '\0') ? nullptr : mEthIfName; }
     void UpdateEthernetNetworkingStatus();
 
-    void
-    SetNetworkStatusChangeCallback(NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback * statusChangeCallback)
-    {
-        mpStatusChangeCallback = statusChangeCallback;
-    }
+    using NetworkStatusChangeCallback = NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback;
+    using OneShotScanCallback         = NetworkCommissioning::WiFiDriver::ScanCallback;
+    using OneShotConnectCallback      = NetworkCommissioning::Internal::WirelessDriver::ConnectCallback;
+
+    void SetOneShotConnectCallback(OneShotConnectCallback * inOneShotConnectCallback) noexcept;
+    void SetOneShotScanCallback(OneShotScanCallback * inOneShotScanCallback) noexcept;
+    void SetNetworkStatusChangeCallback(NetworkStatusChangeCallback * inStatusChangeCallback) noexcept;
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFI
     const char * GetWiFiIfName() { return (sWiFiIfName[0] == '\0') ? nullptr : sWiFiIfName; }
@@ -199,6 +201,11 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
     CHIP_ERROR _Init();
     void _OnPlatformEvent(const ChipDeviceEvent * event);
 
+#if CHIP_DEVICE_CONFIG_ENABLE_WIFI
+    bool IsWiFiStationConnecting() const noexcept;
+    bool IsWiFiStationScanning() const noexcept;
+#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI
+
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA
     WiFiStationMode _GetWiFiStationMode();
     CHIP_ERROR _SetWiFiStationMode(ConnectivityManager::WiFiStationMode val);
@@ -256,7 +263,13 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
     std::mutex mWpaSupplicantMutex;
 
 #endif
-    NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback * mpStatusChangeCallback = nullptr;
+    // Network Commissioning Action Delegation Methods
+
+    void OnScanFinished(NetworkCommissioning::Status inStatus, CharSpan inDebugText,
+                        NetworkCommissioning::WiFiScanResponseIterator * inNetworks) noexcept;
+    void OnConnectResult(NetworkCommissioning::Status inCommissioningError, CharSpan inDebugText, int32_t inConnectStatus) noexcept;
+    void OnStatusChange(NetworkCommissioning::Status inCommissioningError, Optional<ByteSpan> inNetworkId,
+                        Optional<int32_t> inConnectStatus) noexcept;
 
     // ==================== ConnectivityManager Private Methods ====================
 
@@ -287,8 +300,27 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA
     Internal::WiFiSSIDFixedBuffer mInterestedSSID;
 #endif
-    NetworkCommissioning::WiFiDriver::ScanCallback * mpScanCallback;
-    NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * mpConnectCallback;
+    /**
+     *  A callback through which, when non-null, the Wi-Fi driver
+     *  'OnFinished' method is invoked after a Wi-Fi scan is
+     *  complete. The semantics of this callback are one-shot in that
+     *  it is set-scan-invoke-and-clear.
+     *
+     *  A non-null value implies that a Wi-Fi scan is in progress.
+     */
+    OneShotScanCallback * mpOneShotScanCallback;
+
+    /**
+     *  A callback through which, when non-null, the wireless driver
+     *  'OnResult' method is invoked after a Wi-Fi association is
+     *  complete. The semantics of this callback are one-shot in that
+     *  it is set-associate-invoke-and-clear.
+     *
+     *  A non-null value implies that a Wi-Fi association is in
+     *  progress.
+     */
+    OneShotConnectCallback * mpOneShotConnectCallback;
+    NetworkStatusChangeCallback * mpStatusChangeCallback;
 };
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA

src/platform/Linux/ConnectivityManagerImpl_NetworkManagementWpaSupplicant.cpp

@@ -245,7 +245,7 @@ void ConnectivityManagerImpl::UpdateNetworkStatus()
 {
     Network configuredNetwork;
 
-    VerifyOrReturn(IsWiFiStationEnabled() && mpStatusChangeCallback != nullptr);
+    VerifyOrReturn(IsWiFiStationEnabled());
 
     CHIP_ERROR err = GetConfiguredNetwork(configuredNetwork);
     if (err != CHIP_NO_ERROR)
@@ -257,14 +257,13 @@ void ConnectivityManagerImpl::UpdateNetworkStatus()
     // If we have already connected to the WiFi AP, then return null to indicate a success state.
     if (IsWiFiStationConnected())
     {
-        mpStatusChangeCallback->OnNetworkingStatusChange(
-            Status::kSuccess, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)), NullOptional);
+        OnStatusChange(Status::kSuccess, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)),
+                       NullOptional);
         return;
     }
 
-    mpStatusChangeCallback->OnNetworkingStatusChange(
-        Status::kUnknownError, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)),
-        MakeOptional(GetDisconnectReason()));
+    OnStatusChange(Status::kUnknownError, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)),
+                   MakeOptional(GetDisconnectReason()));
 }
 
 void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaSupplicant1Interface * iface, GVariant * changedProperties)
@@ -350,13 +349,8 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaSupplicant1Interface *
                 break;
             }
 
-            TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda([this, reason]() {
-                if (mpConnectCallback != nullptr)
-                {
-                    mpConnectCallback->OnResult(NetworkCommissioning::Status::kUnknownError, CharSpan(), reason);
-                    mpConnectCallback = nullptr;
-                }
-            });
+            TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda(
+                [this, reason]() { OnConnectResult(NetworkCommissioning::Status::kUnknownError, CharSpan(), reason); });
             if (delegate != nullptr)
             {
                 TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda([delegate, associationFailureCause, status]() {
@@ -384,11 +378,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaSupplicant1Interface *
         if (mAssociationStarted)
         {
             TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda([this]() {
-                if (mpConnectCallback != nullptr)
-                {
-                    mpConnectCallback->OnResult(NetworkCommissioning::Status::kSuccess, CharSpan(), 0);
-                    mpConnectCallback = nullptr;
-                }
+                OnConnectResult(NetworkCommissioning::Status::kSuccess, CharSpan(), 0);
                 ConnectivityMgrImpl().PostNetworkConnect();
             });
         }
@@ -875,7 +865,7 @@ ConnectivityManagerImpl::_ConnectWiFiNetworkAsync(GVariant * args,
     // Network was provisioned successfully - emit a connectivity change event so the application can update its state.
     NotifyWiFiConnectivityChange(kConnectivity_NoChange);
 
-    mpConnectCallback = apCallback;
+    SetOneShotConnectCallback(apCallback);
     return CHIP_NO_ERROR;
 }
 
@@ -893,7 +883,7 @@ ConnectivityManagerImpl::ConnectWiFiNetworkAsync(ByteSpan ssid, ByteSpan credent
     VerifyOrReturnError(mWpaSupplicant.iface, CHIP_ERROR_INCORRECT_STATE);
 
     // There is another ongoing connect request, reject the new one.
-    VerifyOrReturnError(mpConnectCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(!IsWiFiStationConnecting(), CHIP_ERROR_INCORRECT_STATE);
 
     GVariantBuilder builder;
     g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
@@ -948,7 +938,7 @@ CHIP_ERROR ConnectivityManagerImpl::ConnectWiFiNetworkWithPDCAsync(
     VerifyOrReturnError(mWpaSupplicant.iface, CHIP_ERROR_INCORRECT_STATE);
 
     // There is another ongoing connect request, reject the new one.
-    VerifyOrReturnError(mpConnectCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(!IsWiFiStationConnecting(), CHIP_ERROR_INCORRECT_STATE);
 
     // Convert identities and our key pair to DER and add them to wpa_supplicant as blobs
     {
@@ -1269,7 +1259,7 @@ CHIP_ERROR ConnectivityManagerImpl::StartWiFiScan(ByteSpan ssid, WiFiDriver::Sca
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
     VerifyOrReturnError(mWpaSupplicant.iface, CHIP_ERROR_INCORRECT_STATE);
     // There is another ongoing scan request, reject the new one.
-    VerifyOrReturnError(mpScanCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(!IsWiFiStationScanning(), CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(ssid.size() <= mInterestedSSID.capacity(), CHIP_ERROR_INVALID_ARGUMENT);
 
     GAutoPtr<GError> err;
@@ -1282,11 +1272,11 @@ CHIP_ERROR ConnectivityManagerImpl::StartWiFiScan(ByteSpan ssid, WiFiDriver::Sca
     g_variant_builder_add(&builder, "{sv}", "Type", g_variant_new_string("active"));
     args = g_variant_builder_end(&builder);
 
-    mpScanCallback = callback;
+    SetOneShotScanCallback(callback);
     if (!wpa_supplicant_1_interface_call_scan_sync(mWpaSupplicant.iface.get(), args, nullptr, &err.GetReceiver()))
     {
         ChipLogProgress(DeviceLayer, "wpa_supplicant: failed to start network scan: %s", err ? err->message : "unknown error");
-        mpScanCallback = nullptr;
+        SetOneShotScanCallback(nullptr);
         return CHIP_ERROR_INTERNAL;
     }
 
@@ -1570,13 +1560,8 @@ void ConnectivityManagerImpl::_OnWpaInterfaceScanDone(WpaSupplicant1Interface *
     if (bsss == nullptr)
     {
         ChipLogProgress(DeviceLayer, "wpa_supplicant: no network found");
-        TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda([this]() {
-            if (mpScanCallback != nullptr)
-            {
-                mpScanCallback->OnFinished(Status::kSuccess, CharSpan(), nullptr);
-                mpScanCallback = nullptr;
-            }
-        });
+        TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda(
+            [this]() { OnScanFinished(Status::kSuccess, CharSpan(), nullptr); });
         return;
     }
 
@@ -1597,12 +1582,9 @@ void ConnectivityManagerImpl::_OnWpaInterfaceScanDone(WpaSupplicant1Interface *
 
     TEMPORARY_RETURN_IGNORED DeviceLayer::SystemLayer().ScheduleLambda([this, networkScanned]() {
         // Note: We cannot post an event in ScheduleLambda since std::vector is not trivial copyable.
-        if (mpScanCallback != nullptr)
-        {
-            LinuxScanResponseIterator<WiFiScanResponse> iter(networkScanned);
-            mpScanCallback->OnFinished(Status::kSuccess, CharSpan(), &iter);
-            mpScanCallback = nullptr;
-        }
+        LinuxScanResponseIterator<WiFiScanResponse> iter(networkScanned);
+
+        OnScanFinished(Status::kSuccess, CharSpan(), &iter);
 
         delete networkScanned;
     });