diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index 056519629bf29b..692373e63394ea 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -68,6 +68,7 @@ class GeneralCommissioningAttrAccess : public AttributeAccessInterface GeneralCommissioningAttrAccess() : AttributeAccessInterface(Optional::Missing(), GeneralCommissioning::Id) {} CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override; private: CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder); @@ -99,6 +100,9 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath case SupportsConcurrentConnection::Id: { return ReadSupportsConcurrentConnection(aEncoder); } + case Breadcrumb::Id: { + return aEncoder.Encode(DeviceLayer::DeviceControlServer::DeviceControlSvr().GetBreadcrumb()); + } default: { break; } @@ -106,6 +110,27 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath return CHIP_NO_ERROR; } +CHIP_ERROR GeneralCommissioningAttrAccess::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) +{ + // TODO: There was discussion about moving the breadcrumb to the attribute store, which would make this function obsolete + + if (aPath.mClusterId != GeneralCommissioning::Id) + { + // We shouldn't have been called at all. + return CHIP_ERROR_INVALID_ARGUMENT; + } + + switch (aPath.mAttributeId) + { + case Attributes::Breadcrumb::Id: + Attributes::Breadcrumb::TypeInfo::DecodableType value; + ReturnErrorOnFailure(aDecoder.Decode(value)); + return DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(value); + default: + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; + } +} + CHIP_ERROR GeneralCommissioningAttrAccess::ReadIfSupported(CHIP_ERROR (ConfigurationManager::*getter)(uint8_t &), AttributeValueEncoder & aEncoder) { @@ -194,6 +219,7 @@ bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler * response.errorCode = CommissioningError::kOk; commandObj->AddResponse(commandPath, response); } + DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(commandData.breadcrumb); } else { @@ -243,6 +269,7 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( } } + DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0); commandObj->AddResponse(commandPath, response); return true; @@ -268,5 +295,6 @@ bool emberAfGeneralCommissioningClusterSetRegulatoryConfigCallback(app::CommandH void MatterGeneralCommissioningPluginServerInitCallback() { + DeviceLayer::DeviceControlServer::DeviceControlSvr().SetBreadcrumb(0); registerAttributeAccessOverride(&gAttrAccess); } diff --git a/src/app/server/CommissioningWindowManager.cpp b/src/app/server/CommissioningWindowManager.cpp index 339935c94f228e..1344998f4607fa 100644 --- a/src/app/server/CommissioningWindowManager.cpp +++ b/src/app/server/CommissioningWindowManager.cpp @@ -56,6 +56,9 @@ void CommissioningWindowManager::OnPlatformEvent(const DeviceLayer::ChipDeviceEv mCommissioningTimeoutTimerArmed = false; Cleanup(); mServer->GetSecureSessionManager().ExpireAllPASEPairings(); +#if CONFIG_NETWORK_LAYER_BLE + mServer->GetBleLayerObject()->CloseAllBleConnections(); +#endif } else if (event->Type == DeviceLayer::DeviceEventType::kFailSafeTimerExpired) { diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index d9f6d69ac4818d..0e24d1c971e8ca 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -260,11 +260,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) err = mCASESessionManager.Init(&DeviceLayer::SystemLayer(), caseSessionManagerConfig); SuccessOrExit(err); - err = mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mTransports, -#if CONFIG_NETWORK_LAYER_BLE - chip::DeviceLayer::ConnectivityMgr().GetBleLayer(), -#endif - &mSessions, &mFabrics, mSessionResumptionStorage, mGroupsProvider); + err = mCASEServer.ListenForSessionEstablishment(&mExchangeMgr, &mTransports, &mSessions, &mFabrics, mSessionResumptionStorage, + mGroupsProvider); SuccessOrExit(err); // This code is necessary to restart listening to existing groups after a reboot diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 418cf48f6e6555..016612dc15fdf3 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -478,6 +478,16 @@ CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(const Trans void DeviceCommissioner::ReleaseCommissioneeDevice(CommissioneeDeviceProxy * device) { + // TODO: Call CloseSession here see #16440 and #16805 (blocking) + +#if CONFIG_NETWORK_LAYER_BLE + if (mSystemState->BleLayer() != nullptr && device->GetDeviceTransportType() == Transport::Type::kBle) + { + // We only support one BLE connection, so if this is BLE, close it + ChipLogProgress(Discovery, "Closing all BLE connections"); + mSystemState->BleLayer()->CloseAllBleConnections(); + } +#endif mCommissioneeDevicePool.ReleaseObject(device); // Make sure that there will be no dangling pointer if (mDeviceInPASEEstablishment == device) @@ -1400,10 +1410,20 @@ void OnBasicFailure(void * context, CHIP_ERROR error) void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId, const CompletionStatus & completionStatus) { commissioningCompletionStatus = completionStatus; - if (completionStatus.err == CHIP_NO_ERROR || - (completionStatus.failedStage.HasValue() && completionStatus.failedStage.Value() >= kWiFiNetworkSetup)) + if (completionStatus.err == CHIP_NO_ERROR) + { + + CommissioneeDeviceProxy * commissionee = FindCommissioneeDevice(nodeId); + if (commissionee != nullptr) + { + ReleaseCommissioneeDevice(commissionee); + } + // Send the callbacks, we're done. + CommissioningStageComplete(CHIP_NO_ERROR); + SendCommissioningCompleteCallbacks(nodeId, commissioningCompletionStatus); + } + else if (completionStatus.failedStage.HasValue() && completionStatus.failedStage.Value() >= kWiFiNetworkSetup) { - // If we Completed successfully, send the callbacks, we're done. // If we were already doing network setup, we need to retain the pase session and start again from network setup stage. // We do not need to reset the failsafe here because we want to keep everything on the device up to this point, so just // send the completion callbacks. @@ -1502,6 +1522,7 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin completionStatus.err = status; completionStatus.failedStage = MakeOptional(report.stageCompleted); mCommissioningStage = CommissioningStage::kCleanup; + mDeviceBeingCommissioned = proxy; CleanupCommissioning(proxy, nodeId, completionStatus); } } @@ -1523,16 +1544,21 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDevicePr commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report); } } - else if (commissioner->mPairingDelegate != nullptr) - { - commissioner->mPairingDelegate->OnPairingComplete(CHIP_NO_ERROR); - } - - // Release any CommissioneeDeviceProxies we have here as we now have an OperationalDeviceProxy. - CommissioneeDeviceProxy * commissionee = commissioner->FindCommissioneeDevice(device->GetDeviceId()); - if (commissionee != nullptr) + else { - commissioner->ReleaseCommissioneeDevice(commissionee); + if (commissioner->mPairingDelegate != nullptr) + { + commissioner->mPairingDelegate->OnPairingComplete(CHIP_NO_ERROR); + } + // Only release the PASE session if we're not commissioning. If we're commissioning, we're going to hold onto that PASE + // session until we send the commissioning complete command just in case it fails and we need to go back to the PASE + // connection to re-setup the network. This is unlikely, given that we just connected over the operational network, but is + // required by the spec. + CommissioneeDeviceProxy * commissionee = commissioner->FindCommissioneeDevice(device->GetDeviceId()); + if (commissionee != nullptr) + { + commissioner->ReleaseCommissioneeDevice(commissionee); + } } } @@ -1554,7 +1580,6 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer error = CHIP_ERROR_INTERNAL; } - commissioner->mSystemState->CASESessionMgr()->ReleaseSession(peerId); if (commissioner->mCommissioningStage == CommissioningStage::kFindOperational && commissioner->mCommissioningDelegate != nullptr) { @@ -1564,13 +1589,7 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer { commissioner->mPairingDelegate->OnPairingComplete(error); } - - CommissioneeDeviceProxy * commissionee = commissioner->FindCommissioneeDevice(peerId.GetNodeId()); - // TODO: Determine if we really want the PASE session removed here. See #16089. - if (commissionee != nullptr) - { - commissioner->ReleaseCommissioneeDevice(commissionee); - } + commissioner->mSystemState->CASESessionMgr()->ReleaseSession(peerId); } // ClusterStateCache::Callback impl diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index 664e96e087b2e3..7e3fc042e52d1b 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -186,11 +186,8 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) // especially since it will interrupt other potential usages of BLE by the controller acting in a commissioning capacity. // ReturnErrorOnFailure(stateParams.caseServer->ListenForSessionEstablishment( - stateParams.exchangeMgr, stateParams.transportMgr, -#if CONFIG_NETWORK_LAYER_BLE - nullptr, -#endif - stateParams.sessionMgr, stateParams.fabricTable, stateParams.sessionResumptionStorage, stateParams.groupDataProvider)); + stateParams.exchangeMgr, stateParams.transportMgr, stateParams.sessionMgr, stateParams.fabricTable, + stateParams.sessionResumptionStorage, stateParams.groupDataProvider)); // // We need to advertise the port that we're listening to for unsolicited messages over UDP. However, we have both a IPv4 diff --git a/src/controller/CommissioneeDeviceProxy.cpp b/src/controller/CommissioneeDeviceProxy.cpp index c7b695e2e5a387..6a3000c99f3ef1 100644 --- a/src/controller/CommissioneeDeviceProxy.cpp +++ b/src/controller/CommissioneeDeviceProxy.cpp @@ -122,10 +122,7 @@ void CommissioneeDeviceProxy::Reset() mState = ConnectionState::NotConnected; mSessionManager = nullptr; mUDPEndPointManager = nullptr; -#if CONFIG_NETWORK_LAYER_BLE - mBleLayer = nullptr; -#endif - mExchangeMgr = nullptr; + mExchangeMgr = nullptr; } bool CommissioneeDeviceProxy::GetAddress(Inet::IPAddress & addr, uint16_t & port) const diff --git a/src/controller/CommissioneeDeviceProxy.h b/src/controller/CommissioneeDeviceProxy.h index e33f958b0de864..2b580be563dad9 100644 --- a/src/controller/CommissioneeDeviceProxy.h +++ b/src/controller/CommissioneeDeviceProxy.h @@ -45,11 +45,6 @@ #include #include -#if CONFIG_NETWORK_LAYER_BLE -#include -#include -#endif - namespace chip { constexpr size_t kAttestationNonceLength = 32; @@ -67,10 +62,7 @@ struct ControllerDeviceInitParams SessionManager * sessionManager = nullptr; Messaging::ExchangeManager * exchangeMgr = nullptr; Inet::EndPointManager * udpEndPointManager = nullptr; -#if CONFIG_NETWORK_LAYER_BLE - Ble::BleLayer * bleLayer = nullptr; -#endif - FabricTable * fabricsTable = nullptr; + FabricTable * fabricsTable = nullptr; }; class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegate @@ -117,9 +109,6 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat mExchangeMgr = params.exchangeMgr; mUDPEndPointManager = params.udpEndPointManager; mFabricIndex = fabric; -#if CONFIG_NETWORK_LAYER_BLE - mBleLayer = params.bleLayer; -#endif } /** @@ -243,10 +232,6 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat bool mActive = false; ConnectionState mState = ConnectionState::NotConnected; -#if CONFIG_NETWORK_LAYER_BLE - Ble::BleLayer * mBleLayer = nullptr; -#endif - PASESession mPairing; SessionManager * mSessionManager = nullptr; diff --git a/src/controller/python/test/test_scripts/commissioning_failure_test.py b/src/controller/python/test/test_scripts/commissioning_failure_test.py index 1a47a87086927c..1437c646c14f15 100755 --- a/src/controller/python/test/test_scripts/commissioning_failure_test.py +++ b/src/controller/python/test/test_scripts/commissioning_failure_test.py @@ -97,7 +97,7 @@ def main(): # TODO: Start at stage 2 once handling for arming failsafe on pase is done. if options.report: - for testFailureStage in range(3, 17): + for testFailureStage in range(3, 18): FailIfNot(test.TestPaseOnly(ip=options.deviceAddress1, setuppin=20202021, nodeid=1), @@ -106,7 +106,7 @@ def main(): "Commissioning failure tests failed for simulated report failure on stage {}".format(testFailureStage)) else: - for testFailureStage in range(3, 17): + for testFailureStage in range(3, 18): FailIfNot(test.TestPaseOnly(ip=options.deviceAddress1, setuppin=20202021, nodeid=1), diff --git a/src/include/platform/DeviceControlServer.h b/src/include/platform/DeviceControlServer.h index e2b03ccf4478c4..d22a67f5b781a2 100644 --- a/src/include/platform/DeviceControlServer.h +++ b/src/include/platform/DeviceControlServer.h @@ -95,6 +95,8 @@ class DeviceControlServer final void SetSwitchDelegate(SwitchDeviceControlDelegate * delegate) { mSwitchDelegate = delegate; } SwitchDeviceControlDelegate * GetSwitchDelegate() const { return mSwitchDelegate; } FailSafeContext & GetFailSafeContext() { return mFailSafeContext; } + CHIP_ERROR SetBreadcrumb(uint64_t breadcrumb); + uint64_t GetBreadcrumb(); static DeviceControlServer & DeviceControlSvr(); diff --git a/src/platform/DeviceControlServer.cpp b/src/platform/DeviceControlServer.cpp index 0c1a526e10c256..30f1821bc38283 100644 --- a/src/platform/DeviceControlServer.cpp +++ b/src/platform/DeviceControlServer.cpp @@ -47,6 +47,17 @@ CHIP_ERROR DeviceControlServer::CommissioningComplete(NodeId peerNodeId, FabricI return PlatformMgr().PostEvent(&event); } +CHIP_ERROR DeviceControlServer::SetBreadcrumb(uint64_t breadcrumb) +{ + return ConfigurationMgr().StoreBreadcrumb(breadcrumb); +} +uint64_t DeviceControlServer::GetBreadcrumb() +{ + uint64_t breadcrumb = 0; + ConfigurationMgr().GetBreadcrumb(breadcrumb); + return breadcrumb; +} + CHIP_ERROR DeviceControlServer::SetRegulatoryConfig(uint8_t location, const CharSpan & countryCode, uint64_t breadcrumb) { CHIP_ERROR err = CHIP_NO_ERROR; diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp index 8beed6da2accaa..e572a1d2b61346 100644 --- a/src/protocols/secure_channel/CASEServer.cpp +++ b/src/protocols/secure_channel/CASEServer.cpp @@ -30,9 +30,6 @@ using namespace ::chip::Credentials; namespace chip { CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, -#if CONFIG_NETWORK_LAYER_BLE - Ble::BleLayer * bleLayer, -#endif SessionManager * sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage, Credentials::GroupDataProvider * responderGroupDataProvider) @@ -43,9 +40,6 @@ CHIP_ERROR CASEServer::ListenForSessionEstablishment(Messaging::ExchangeManager VerifyOrReturnError(sessionManager != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(responderGroupDataProvider != nullptr, CHIP_ERROR_INVALID_ARGUMENT); -#if CONFIG_NETWORK_LAYER_BLE - mBleLayer = bleLayer; -#endif mSessionManager = sessionManager; mSessionResumptionStorage = sessionResumptionStorage; mFabrics = fabrics; @@ -60,15 +54,6 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec) { ReturnErrorCodeIf(ec == nullptr, CHIP_ERROR_INVALID_ARGUMENT); -#if CONFIG_NETWORK_LAYER_BLE - // Close all BLE connections now since a CASE handshake has been initiated. - if (mBleLayer != nullptr) - { - ChipLogProgress(Discovery, "CASE handshake initiated, closing all BLE Connections"); - mBleLayer->CloseAllBleConnections(); - } -#endif - // Setup CASE state machine using the credentials for the current fabric. GetSession().SetGroupDataProvider(mGroupDataProvider); ReturnErrorOnFailure( diff --git a/src/protocols/secure_channel/CASEServer.h b/src/protocols/secure_channel/CASEServer.h index f0baa12dbc4324..934d914a11377c 100644 --- a/src/protocols/secure_channel/CASEServer.h +++ b/src/protocols/secure_channel/CASEServer.h @@ -17,9 +17,6 @@ #pragma once -#if CONFIG_NETWORK_LAYER_BLE -#include -#endif #include #include #include @@ -42,9 +39,6 @@ class CASEServer : public SessionEstablishmentDelegate, } CHIP_ERROR ListenForSessionEstablishment(Messaging::ExchangeManager * exchangeManager, TransportMgrBase * transportMgr, -#if CONFIG_NETWORK_LAYER_BLE - Ble::BleLayer * bleLayer, -#endif SessionManager * sessionManager, FabricTable * fabrics, SessionResumptionStorage * sessionResumptionStorage, Credentials::GroupDataProvider * responderGroupDataProvider); @@ -70,9 +64,6 @@ class CASEServer : public SessionEstablishmentDelegate, CASESession mPairingSession; SessionManager * mSessionManager = nullptr; -#if CONFIG_NETWORK_LAYER_BLE - Ble::BleLayer * mBleLayer = nullptr; -#endif FabricTable * mFabrics = nullptr; Credentials::GroupDataProvider * mGroupDataProvider = nullptr; diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index 601a9a20062449..e5f1a642784015 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -311,9 +311,6 @@ void CASE_SecurePairingHandshakeServerTest(nlTestSuite * inSuite, void * inConte // components may work simultaneously on a single device. NL_TEST_ASSERT(inSuite, gPairingServer.ListenForSessionEstablishment(&ctx.GetExchangeManager(), &ctx.GetTransportMgr(), -#if CONFIG_NETWORK_LAYER_BLE - nullptr, -#endif &ctx.GetSecureSessionManager(), &gDeviceFabrics, nullptr, &gDeviceGroupDataProvider) == CHIP_NO_ERROR);