Skip to content

Commit

Permalink
[nwprov] Make it possible to notify network commissioning cluster for…
Browse files Browse the repository at this point in the history
… autonomous connection (#15666)

* [nwprov] Move GetLast* attributes to Driver side

* Add placeholder for other platforms

* Fix Linux ThreadStackManagerImpl

* Add issue number

* Fix

* Set LastNetworkingStatus for manual Scan operations

* Let the driver push result instead of cluster getting the value
  • Loading branch information
erjiaqing authored and pull[bot] committed Aug 18, 2023
1 parent e1cde7b commit c04b81a
Show file tree
Hide file tree
Showing 28 changed files with 296 additions and 54 deletions.
40 changes: 38 additions & 2 deletions src/app/clusters/network-commissioning/network-commissioning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ CHIP_ERROR Instance::Init()
VerifyOrReturnError(registerAttributeAccessOverride(this), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(
DeviceLayer::PlatformMgrImpl().AddEventHandler(_OnCommissioningComplete, reinterpret_cast<intptr_t>(this)));
ReturnErrorOnFailure(mpBaseDriver->Init());
ReturnErrorOnFailure(mpBaseDriver->Init(this));
mLastNetworkingStatusValue.SetNull();
mLastConnectErrorValue.SetNull();
mLastNetworkIDLen = 0;
Expand Down Expand Up @@ -239,6 +239,34 @@ CHIP_ERROR Instance::Write(const ConcreteDataAttributePath & aPath, AttributeVal
}
}

void Instance::OnNetworkingStatusChange(DeviceLayer::NetworkCommissioning::Status aCommissioningError,
Optional<ByteSpan> aNetworkId, Optional<int32_t> aConnectStatus)
{
if (aNetworkId.HasValue() && aNetworkId.Value().size() > kMaxNetworkIDLen)
{
ChipLogError(DeviceLayer, "Invalid network id received when calling OnNetworkingStatusChange");
return;
}
mLastNetworkingStatusValue.SetNonNull(ToClusterObjectEnum(aCommissioningError));
if (aNetworkId.HasValue())
{
memcpy(mLastNetworkID, aNetworkId.Value().data(), aNetworkId.Value().size());
mLastNetworkIDLen = static_cast<uint8_t>(aNetworkId.Value().size());
}
else
{
mLastNetworkIDLen = 0;
}
if (aConnectStatus.HasValue())
{
mLastConnectErrorValue.SetNonNull(aConnectStatus.Value());
}
else
{
mLastConnectErrorValue.SetNull();
}
}

void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetworks::DecodableType & req)
{

Expand Down Expand Up @@ -321,7 +349,15 @@ void Instance::OnResult(Status commissioningError, CharSpan errorText, int32_t i
mLastNetworkIDLen = mConnectingNetworkIDLen;
memcpy(mLastNetworkID, mConnectingNetworkID, mLastNetworkIDLen);
mLastNetworkingStatusValue.SetNonNull(ToClusterObjectEnum(commissioningError));
mLastConnectErrorValue.SetNonNull(interfaceStatus);

if (commissioningError == Status::kSuccess)
{
mLastConnectErrorValue.SetNull();
}
else
{
mLastConnectErrorValue.SetNonNull(interfaceStatus);
}

if (commissioningError == Status::kSuccess)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace NetworkCommissioning {
// TODO: Use macro to disable some wifi or thread
class Instance : public CommandHandlerInterface,
public AttributeAccessInterface,
public DeviceLayer::NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback,
public DeviceLayer::NetworkCommissioning::Internal::WirelessDriver::ConnectCallback,
public DeviceLayer::NetworkCommissioning::WiFiDriver::ScanCallback,
public DeviceLayer::NetworkCommissioning::ThreadDriver::ScanCallback
Expand All @@ -54,6 +55,10 @@ class Instance : public CommandHandlerInterface,
CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;

// BaseDriver::NetworkStatusChangeCallback
void OnNetworkingStatusChange(DeviceLayer::NetworkCommissioning::Status aCommissioningError, Optional<ByteSpan> aNetworkId,
Optional<int32_t> aConnectStatus) override;

// WirelessDriver::ConnectCallback
void OnResult(DeviceLayer::NetworkCommissioning::Status commissioningError, CharSpan errorText,
int32_t interfaceStatus) override;
Expand Down Expand Up @@ -84,8 +89,9 @@ class Instance : public CommandHandlerInterface,
// Last* attributes
// Setting these values don't have to care about parallel requests, since we will reject other requests when there is another
// request ongoing.
// These values can be updated via OnNetworkingStatusChange callback, ScanCallback::OnFinished and ConnectCallback::OnResult.
DataModel::Nullable<NetworkCommissioningStatus> mLastNetworkingStatusValue;
DataModel::Nullable<Attributes::LastConnectErrorValue::TypeInfo::Type> mLastConnectErrorValue;
Attributes::LastConnectErrorValue::TypeInfo::Type mLastConnectErrorValue;
uint8_t mConnectingNetworkID[DeviceLayer::NetworkCommissioning::kMaxNetworkIDLen];
uint8_t mConnectingNetworkIDLen = 0;
uint8_t mLastNetworkID[DeviceLayer::NetworkCommissioning::kMaxNetworkIDLen];
Expand Down
22 changes: 4 additions & 18 deletions src/controller/python/test/test_scripts/network_commissioning.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,6 @@ async def test_wifi(self, endpointId):
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")

# Verify Last* attributes
logger.info(f"Read Last* attributes")
res = await self.readLastNetworkingStateAttributes(endpointId=endpointId)
if (res.lastNetworkID != NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue != NullValue):
raise AssertionError(
f"LastNetworkID and LastConnectErrorValue should be Null and LastNetworkingStatus should not be Null")

# Remove existing network
logger.info(f"Check network list")
res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(endpointId, Clusters.NetworkCommissioning.Attributes.Networks)], returnClusterObject=True)
Expand Down Expand Up @@ -187,9 +180,9 @@ async def test_wifi(self, endpointId):
# Verify Last* attributes
logger.info(f"Read Last* attributes")
res = await self.readLastNetworkingStateAttributes(endpointId=endpointId)
if (res.lastNetworkID == NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue == NullValue):
if (res.lastNetworkID == NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue != NullValue):
raise AssertionError(
f"LastNetworkID, LastConnectErrorValue and LastNetworkingStatus should not be Null")
f"LastNetworkID, LastNetworkingStatus should not be Null, LastConnectErrorValue should be Null for a successful network provision.")

async def test_thread(self, endpointId):
logger.info(f"Get basic information of the endpoint")
Expand Down Expand Up @@ -221,13 +214,6 @@ async def test_thread(self, endpointId):
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")

# Verify Last* attributes
logger.info(f"Read Last* attributes")
res = await self.readLastNetworkingStateAttributes(endpointId=endpointId)
if (res.lastNetworkID != NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue != NullValue):
raise AssertionError(
f"LastNetworkID and LastConnectErrorValue should be Null and LastNetworkingStatus should not be Null")

# Remove existing network
logger.info(f"Check network list")
res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(endpointId, Clusters.NetworkCommissioning.Attributes.Networks)], returnClusterObject=True)
Expand Down Expand Up @@ -275,9 +261,9 @@ async def test_thread(self, endpointId):
# Verify Last* attributes
logger.info(f"Read Last* attributes")
res = await self.readLastNetworkingStateAttributes(endpointId=endpointId)
if (res.lastNetworkID == NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue == NullValue):
if (res.lastNetworkID == NullValue) or (res.lastNetworkingStatus == NullValue) or (res.lastConnectErrorValue != NullValue):
raise AssertionError(
f"LastNetworkID, LastConnectErrorValue and LastNetworkingStatus should not be Null")
f"LastNetworkID, LastNetworkingStatus should not be Null, LastConnectErrorValue should be Null for a successful network provision.")

logger.info(f"Check network list")
res = await self._devCtrl.ReadAttribute(nodeid=self._nodeid, attributes=[(endpointId, Clusters.NetworkCommissioning.Attributes.Networks)], returnClusterObject=True)
Expand Down
19 changes: 18 additions & 1 deletion src/include/platform/NetworkCommissioning.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include <lib/core/CHIPCore.h>
#include <lib/core/CHIPSafeCasts.h>
#include <lib/core/Optional.h>
#include <lib/support/ThreadOperationalDataset.h>
#include <platform/internal/DeviceNetworkInfo.h>

Expand Down Expand Up @@ -154,10 +155,26 @@ namespace Internal {
class BaseDriver
{
public:
class NetworkStatusChangeCallback
{
public:
/**
* @brief Callback for the network driver pushing the event of network status change to the network commissioning cluster.
* The platforms is explected to push the status from operations such as autonomous connection after loss of connectivity or
* during initial establishment.
*
* This function must be called in a thread-safe manner with CHIP stack.
*/
virtual void OnNetworkingStatusChange(Status commissioningError, Optional<ByteSpan> networkId,
Optional<int32_t> connectStatus) = 0;

virtual ~NetworkStatusChangeCallback() = default;
};

/**
* @brief Initializes the driver, this function will be called when initializing the network commissioning cluster.
*/
virtual CHIP_ERROR Init() { return CHIP_NO_ERROR; }
virtual CHIP_ERROR Init(NetworkStatusChangeCallback * networkStatusChangeCallback) { return CHIP_NO_ERROR; }

/**
* @brief Shuts down the driver, this function will be called when shutting down the network commissioning cluster.
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Ameba/NetworkCommissioningDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class AmebaWiFiDriver final : public WiFiDriver

// BaseDriver
NetworkIterator * GetNetworks() override { return new WiFiNetworkIterator(this); }
CHIP_ERROR Init() override;
CHIP_ERROR Init(NetworkStatusChangeCallback * networkStatusChangeCallback) override;
CHIP_ERROR Shutdown() override;

// WirelessDriver
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Ameba/NetworkCommissioningWiFiDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ constexpr char kWiFiSSIDKeyName[] = "wifi-ssid";
constexpr char kWiFiCredentialsKeyName[] = "wifi-pass";
} // namespace

CHIP_ERROR AmebaWiFiDriver::Init()
CHIP_ERROR AmebaWiFiDriver::Init(NetworkStatusChangeCallback *)
{
CHIP_ERROR err;
size_t ssidLen = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/EFR32/NetworkCommissioningWiFiDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ NetworkCommissioning::WiFiScanResponse * sScanResult;
SlScanResponseIterator<NetworkCommissioning::WiFiScanResponse> mScanResponseIter(sScanResult);
} // namespace

CHIP_ERROR SlWiFiDriver::Init()
CHIP_ERROR SlWiFiDriver::Init(NetworkStatusChangeCallback * networkStatusChangeCallback)
{
CHIP_ERROR err;
size_t ssidLen = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/EFR32/NetworkCommissioningWiFiDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class SlWiFiDriver final : public WiFiDriver

// BaseDriver
NetworkIterator * GetNetworks() override { return new WiFiNetworkIterator(this); }
CHIP_ERROR Init() override;
CHIP_ERROR Init(NetworkStatusChangeCallback * networkStatusChangeCallback) override;

// WirelessDriver
uint8_t GetMaxNetworks() override { return kMaxWiFiNetworks; }
Expand Down
2 changes: 1 addition & 1 deletion src/platform/ESP32/NetworkCommissioningDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class ESPWiFiDriver final : public WiFiDriver

// BaseDriver
NetworkIterator * GetNetworks() override { return new WiFiNetworkIterator(this); }
CHIP_ERROR Init() override;
CHIP_ERROR Init(NetworkStatusChangeCallback * networkStatusChangeCallback) override;
CHIP_ERROR Shutdown() override;

// WirelessDriver
Expand Down
2 changes: 1 addition & 1 deletion src/platform/ESP32/NetworkCommissioningWiFiDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ constexpr char kWiFiCredentialsKeyName[] = "wifi-pass";
static uint8_t WiFiSSIDStr[DeviceLayer::Internal::kMaxWiFiSSIDLength];
} // namespace

CHIP_ERROR ESPWiFiDriver::Init()
CHIP_ERROR ESPWiFiDriver::Init(NetworkStatusChangeCallback * networkStatusChangeCallback)
{
CHIP_ERROR err;
size_t ssidLen = 0;
Expand Down
49 changes: 48 additions & 1 deletion src/platform/Linux/ConnectivityManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,32 @@ void ConnectivityManagerImpl::_SetWiFiAPIdleTimeout(System::Clock::Timeout val)
DeviceLayer::SystemLayer().ScheduleWork(DriveAPState, NULL);
}

void ConnectivityManagerImpl::UpdateNetworkStatus()
{
Network configuredNetwork;

VerifyOrReturn(IsWiFiStationEnabled() && mpStatusChangeCallback != nullptr);

CHIP_ERROR err = GetConfiguredNetwork(configuredNetwork);
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to get configured network when updating network status: %s", err.AsString());
return;
}

// 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);
return;
}

mpStatusChangeCallback->OnNetworkingStatusChange(
Status::kUnknownError, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)),
MakeOptional(GetDisconnectReason()));
}

void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Interface * proxy, GVariant * changed_properties,
const gchar * const * invalidated_properties, gpointer user_data)
{
Expand Down Expand Up @@ -400,6 +426,8 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
delegate->OnAssociationFailureDetected(associationFailureCause, status);
}

DeviceLayer::SystemLayer().ScheduleLambda([]() { ConnectivityMgrImpl().UpdateNetworkStatus(); });

mAssociattionStarted = false;
}
else if (g_strcmp0(value_str, "\'associated\'") == 0)
Expand All @@ -408,6 +436,8 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
{
delegate->OnConnectionStatusChanged(static_cast<uint8_t>(WiFiConnectionStatus::kNotConnected));
}

DeviceLayer::SystemLayer().ScheduleLambda([]() { ConnectivityMgrImpl().UpdateNetworkStatus(); });
}
}

Expand Down Expand Up @@ -1277,13 +1307,30 @@ CHIP_ERROR ConnectivityManagerImpl::GetWiFiVersion(uint8_t & wiFiVersion)
return CHIP_NO_ERROR;
}

CHIP_ERROR ConnectivityManagerImpl::GetConnectedNetwork(NetworkCommissioning::Network & network)
int32_t ConnectivityManagerImpl::GetDisconnectReason()
{
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
std::unique_ptr<GError, GErrorDeleter> err;

gint errorValue = wpa_fi_w1_wpa_supplicant1_interface_get_disconnect_reason(mWpaSupplicant.iface);
// wpa_supplicant DBus API: DisconnectReason: The most recent IEEE 802.11 reason code for disconnect. Negative value
// indicates locally generated disconnection.
return errorValue;
}

CHIP_ERROR ConnectivityManagerImpl::GetConfiguredNetwork(NetworkCommissioning::Network & network)
{
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
std::unique_ptr<GError, GErrorDeleter> err;

const gchar * networkPath = wpa_fi_w1_wpa_supplicant1_interface_get_current_network(mWpaSupplicant.iface);

// wpa_supplicant DBus API: if network path of current network is "/", means no networks is currently selected.
if (strcmp(networkPath, "/") == 0)
{
return CHIP_ERROR_KEY_NOT_FOUND;
}

std::unique_ptr<WpaFiW1Wpa_supplicant1Network, GObjectDeleter> networkInfo(
wpa_fi_w1_wpa_supplicant1_network_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE,
kWpaSupplicantServiceName, networkPath, nullptr,
Expand Down
11 changes: 10 additions & 1 deletion src/platform/Linux/ConnectivityManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
public:
#if CHIP_DEVICE_CONFIG_ENABLE_WPA
CHIP_ERROR ProvisionWiFiNetwork(const char * ssid, const char * key);
void
SetNetworkStatusChangeCallback(NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback * statusChangeCallback)
{
mpStatusChangeCallback = statusChangeCallback;
}
CHIP_ERROR ConnectWiFiNetworkAsync(ByteSpan ssid, ByteSpan credentials,
NetworkCommissioning::Internal::WirelessDriver::ConnectCallback * connectCallback);
void PostNetworkConnect();
Expand All @@ -122,10 +127,11 @@ class ConnectivityManagerImpl final : public ConnectivityManager,

void StartWiFiManagement();
bool IsWiFiManagementStarted();
int32_t GetDisconnectReason();
CHIP_ERROR GetWiFiBssId(ByteSpan & value);
CHIP_ERROR GetWiFiSecurityType(uint8_t & securityType);
CHIP_ERROR GetWiFiVersion(uint8_t & wiFiVersion);
CHIP_ERROR GetConnectedNetwork(NetworkCommissioning::Network & network);
CHIP_ERROR GetConfiguredNetwork(NetworkCommissioning::Network & network);
CHIP_ERROR StartWiFiScan(ByteSpan ssid, NetworkCommissioning::WiFiDriver::ScanCallback * callback);
#endif

Expand Down Expand Up @@ -174,6 +180,7 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
void _MaintainOnDemandWiFiAP();
System::Clock::Timeout _GetWiFiAPIdleTimeout();
void _SetWiFiAPIdleTimeout(System::Clock::Timeout val);
void UpdateNetworkStatus();
static CHIP_ERROR StopAutoScan();

static void _OnWpaProxyReady(GObject * source_object, GAsyncResult * res, gpointer user_data);
Expand All @@ -193,6 +200,8 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
static BitFlags<ConnectivityFlags> mConnectivityFlag;
static struct GDBusWpaSupplicant mWpaSupplicant;
static std::mutex mWpaSupplicantMutex;

NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback * mpStatusChangeCallback = nullptr;
#endif

// ==================== ConnectivityManager Private Methods ====================
Expand Down
11 changes: 6 additions & 5 deletions src/platform/Linux/NetworkCommissioningDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class LinuxWiFiDriver final : public WiFiDriver

// BaseDriver
NetworkIterator * GetNetworks() override { return new WiFiNetworkIterator(this); }
CHIP_ERROR Init() override;
CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } // Nothing to do on linux for shutdown.
CHIP_ERROR Init(BaseDriver::NetworkStatusChangeCallback * networkStatusChangeCallback) override;
CHIP_ERROR Shutdown() override;

// WirelessDriver
uint8_t GetMaxNetworks() override { return 1; }
Expand All @@ -104,6 +104,7 @@ class LinuxWiFiDriver final : public WiFiDriver
WiFiNetworkIterator mWiFiIterator = WiFiNetworkIterator(this);
WiFiNetwork mSavedNetwork;
WiFiNetwork mStagingNetwork;
Optional<Status> mScanStatus;
};
#endif // CHIP_DEVICE_CONFIG_ENABLE_WPA

Expand All @@ -127,8 +128,8 @@ class LinuxThreadDriver final : public ThreadDriver

// BaseDriver
NetworkIterator * GetNetworks() override { return new ThreadNetworkIterator(this); }
CHIP_ERROR Init() override;
CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } // Nothing to do on linux for shutdown.
CHIP_ERROR Init(BaseDriver::NetworkStatusChangeCallback * networkStatusChangeCallback) override;
CHIP_ERROR Shutdown() override;

// WirelessDriver
uint8_t GetMaxNetworks() override { return 1; }
Expand All @@ -144,7 +145,7 @@ class LinuxThreadDriver final : public ThreadDriver

// ThreadDriver
Status AddOrUpdateNetwork(ByteSpan operationalDataset) override;
void ScanNetworks(ScanCallback * callback) override;
void ScanNetworks(ThreadDriver::ScanCallback * callback) override;

private:
ThreadNetworkIterator mThreadIterator = ThreadNetworkIterator(this);
Expand Down
Loading

0 comments on commit c04b81a

Please sign in to comment.