Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NetworkCommissioning post-review from #32156 #32189

Closed
wants to merge 1 commit into from
Closed

Fix NetworkCommissioning post-review from #32156 #32189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/app/clusters/network-commissioning/network-commissioning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,9 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw
}
if (ssid.size() > DeviceLayer::Internal::kMaxWiFiSSIDLength)
{
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
// Clients should never use too large a SSID.
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError);
SetLastNetworkingStatusValue(MakeNullable(Status::kUnknownError));
return;
}
mCurrentOperationBreadcrumb = req.breadcrumb;
Expand All @@ -276,6 +278,13 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw
}
else if (mFeatureFlags.Has(NetworkCommissioningFeature::kThreadNetworkInterface))
{
// SSID present on Thread violates the `[WI]` conformance.
if (req.ssid.HasValue())
{
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
return;
}

mCurrentOperationBreadcrumb = req.breadcrumb;
mAsyncCommandHandle = CommandHandler::Handle(&ctx.mCommandHandler);
ctx.mCommandHandler.FlushAcksRightAwayOnSlowCommand();
Expand Down Expand Up @@ -322,6 +331,26 @@ void Instance::HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands

VerifyOrReturn(CheckFailSafeArmed(ctx));

if (req.ssid.empty() || req.ssid.size() > DeviceLayer::Internal::kMaxWiFiSSIDLength)
{
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError, "ssid");
return;
}

// Presence of a Network Identity indicates we're configuring for Per-Device Credentials
if (req.networkIdentity.HasValue())
{
#if CHIP_DEVICE_CONFIG_ENABLE_WIFI_PDC
if (mFeatureFlags.Has(Feature::kWiFiNetworkInterface))
{
HandleAddOrUpdateWiFiNetworkWithPDC(ctx, req);
return;
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_PDC
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
return;
}

// Spec 11.8.8.4
// Valid Credentials length are:
// - 0 bytes: Unsecured (open) connection
Expand Down Expand Up @@ -671,6 +700,17 @@ void Instance::OnFailSafeTimerExpired()
ChipLogDetail(Zcl, "Failsafe timeout, tell platform driver to revert network credentials.");
mpWirelessDriver->RevertConfiguration();
mAsyncCommandHandle.Release();

// Mark the network list changed since `mpWirelessDriver->RevertConfiguration()` may have updated it.
ReportNetworksListChanged();

// If no networks are left, clear-out errors;
if (mpBaseDriver && (CountAndRelease(mpBaseDriver->GetNetworks()) == 0))
{
SetLastNetworkId(ByteSpan{});
SetLastConnectErrorValue(NullNullable);
SetLastNetworkingStatusValue(NullNullable);
}
}

CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
Expand Down
26 changes: 18 additions & 8 deletions src/controller/python/test/test_scripts/network_commissioning.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,16 @@ async def test_wifi(self, endpointId):
f"LastNetworkID, LastNetworkingStatus and LastConnectErrorValue should be Null")

# Scan networks
logger.info(f"Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
logger.info("Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(breadcrumb=self.with_breadcrumb())
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
logger.info(f"Request: {req}")
res = await self._devCtrl.SendCommand(
nodeid=self._nodeid,
endpoint=endpointId,
payload=req,
interactionTimeoutMs=interactionTimeoutMs
)
logger.info(f"Received response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down Expand Up @@ -274,10 +280,14 @@ async def test_thread(self, endpointId):
f"LastNetworkID, LastNetworkingStatus and LastConnectErrorValue should be Null")

# Scan networks
logger.info(f"Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
res = await self._devCtrl.SendCommand(nodeid=self._nodeid, endpoint=endpointId, payload=req)
logger.info("Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(breadcrumb=self.with_breadcrumb())
logger.info(f"Request: {req}")
interactionTimeoutMs = self._devCtrl.ComputeRoundTripTimeout(self._nodeid, upperLayerProcessingTimeoutMs=30000)
res = await self._devCtrl.SendCommand(nodeid=self._nodeid,
endpoint=endpointId,
payload=req,
interactionTimeoutMs=interactionTimeoutMs)
logger.info(f"Received response: {res}")
if res.networkingStatus != Clusters.NetworkCommissioning.Enums.NetworkCommissioningStatus.kSuccess:
raise AssertionError(f"Unexpected result: {res.networkingStatus}")
Expand Down
Loading