Skip to content

Commit

Permalink
Fix NetworkCommissioning post-review from #32156 (#32172)
Browse files Browse the repository at this point in the history
* Fix NetworkCommissioning post-review from #32156

- Found a regression on Thread scanning.
- Changed some ConstraintError to InvalidCommand where
  more applicable.
- Removed an update of cluster state on fail safe expiry.

Testing done:

- Retested on Wi-Fi
- Testing on Thread as well

* Re-notify errors on empty network at fail-safe expiry

* Fix MobileDeviceTest

* Fix Cirque tests
  • Loading branch information
tcarmelveilleux authored Feb 16, 2024
1 parent 13f3e05 commit 8937d27
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 13 deletions.
23 changes: 14 additions & 9 deletions src/app/clusters/network-commissioning/network-commissioning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw
}
if (ssid.size() > DeviceLayer::Internal::kMaxWiFiSSIDLength)
{
// This should not happen, it means it's a broken driver.
// Clients should never use too large a SSID.
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError);
SetLastNetworkingStatusValue(MakeNullable(Status::kUnknownError));
return;
Expand All @@ -491,10 +491,10 @@ void Instance::HandleScanNetworks(HandlerContext & ctx, const Commands::ScanNetw
}
else if (mFeatureFlags.Has(Feature::kThreadNetworkInterface))
{
// Not allowed to populate SSID for Thread.
if (!req.ssid.HasValue())
// SSID present on Thread violates the `[WI]` conformance.
if (req.ssid.HasValue())
{
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
return;
}

Expand Down Expand Up @@ -559,7 +559,7 @@ void Instance::HandleAddOrUpdateWiFiNetwork(HandlerContext & ctx, const Commands
return;
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_PDC
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::ConstraintError);
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::InvalidCommand);
return;
}

Expand Down Expand Up @@ -1202,11 +1202,16 @@ void Instance::OnFailSafeTimerExpired()
mpWirelessDriver->RevertConfiguration();
mAsyncCommandHandle.Release();

// Reset state on failsafe expiry.
// Mark the network list changed since `mpWirelessDriver->RevertConfiguration()` may have updated it.
ReportNetworksListChanged();
SetLastNetworkId(ByteSpan{});
SetLastConnectErrorValue(NullNullable);
SetLastNetworkingStatusValue(NullNullable);

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@ async def test_wifi(self, endpointId):

# Scan networks
logger.info("Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
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,
Expand Down Expand Up @@ -309,8 +309,8 @@ async def test_thread(self, endpointId):

# Scan networks
logger.info("Scan networks")
req = Clusters.NetworkCommissioning.Commands.ScanNetworks(
ssid=b'', breadcrumb=self.with_breadcrumb())
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,
Expand Down

0 comments on commit 8937d27

Please sign in to comment.