From b2666132a287bfcc0403cf7b9da14d667059b6fc Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 10 Nov 2021 13:06:38 -0800 Subject: [PATCH 1/7] Implement CASESessionManager class - This class enables device to device communication - It extracts out relevant code from DeviceController class to a minimal implementation. The controller class is refactored to use this class internally. --- src/app/BUILD.gn | 2 + src/app/CASESessionManager.cpp | 185 ++++++++++++++++++++++++ src/app/CASESessionManager.h | 87 +++++++++++ src/app/OperationalDeviceProxy.h | 17 ++- src/controller/CHIPDeviceController.cpp | 150 ++++--------------- src/controller/CHIPDeviceController.h | 42 ++---- 6 files changed, 329 insertions(+), 154 deletions(-) create mode 100644 src/app/CASESessionManager.cpp create mode 100644 src/app/CASESessionManager.h diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 793fc2a6bfc3b0..673dd007804cb7 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -39,6 +39,8 @@ static_library("app") { "AttributePathExpandIterator.h", "AttributePathParams.cpp", "AttributePathParams.h", + "CASESessionManager.cpp", + "CASESessionManager.h", "Command.cpp", "Command.h", "CommandHandler.cpp", diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp new file mode 100644 index 00000000000000..18ec0c89602fa2 --- /dev/null +++ b/src/app/CASESessionManager.cpp @@ -0,0 +1,185 @@ +/* + * + * Copyright (c) 2020-2021 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace chip { + +CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId deviceId, Transport::PeerAddress address, + Callback::Callback * onConnection, + Callback::Callback * onFailure) +{ + VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); + PeerId peerId = mInitParams.sessionInitParams.fabricInfo->GetPeerIdForNode(deviceId); + OperationalDeviceProxy * session = FindSession(deviceId); + if (session == nullptr) + { + session = mActiveSessions.CreateObject(mInitParams.sessionInitParams, peerId); + if (session == nullptr) + { + onFailure->mCall(onFailure->mContext, deviceId, CHIP_ERROR_NO_MEMORY); + return CHIP_ERROR_NO_MEMORY; + } + } + + bool validAddress = (address != Transport::PeerAddress::UDP(Inet::IPAddress::Any)); + + if (!validAddress && mInitParams.dnsCache != nullptr) + { + Dnssd::ResolvedNodeData nodeData; + if (mInitParams.dnsCache->Lookup(peerId, nodeData) == CHIP_NO_ERROR) + { + address = ToPeerAddress(nodeData); + validAddress = true; + } + } + + if (validAddress) + { + uint32_t idleInterval; + uint32_t activeInterval; + session->GetMRPIntervals(idleInterval, activeInterval); + CHIP_ERROR err = session->UpdateDeviceData(address, idleInterval, activeInterval); + if (err != CHIP_NO_ERROR) + { + ReleaseSession(session); + return err; + } + } + + CHIP_ERROR err = session->Connect(onConnection, onFailure); + if (err != CHIP_NO_ERROR) + { + ReleaseSession(session); + } + + return err; +} + +void CASESessionManager::ReleaseSession(NodeId deviceId) +{ + ReleaseSession(FindSession(deviceId)); +} + +CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId deviceId) +{ + VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); + return Dnssd::Resolver::Instance().ResolveNodeId(mInitParams.sessionInitParams.fabricInfo->GetPeerIdForNode(deviceId), + Inet::IPAddressType::kAny); +} + +Transport::PeerAddress CASESessionManager::ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) const +{ + Inet::InterfaceId interfaceId; + + // Only use the mDNS resolution's InterfaceID for addresses that are IPv6 LLA. + // For all other addresses, we should rely on the device's routing table to route messages sent. + // Forcing messages down an InterfaceId might fail. For example, in bridged networks like Thread, + // mDNS advertisements are not usually received on the same interface the peer is reachable on. + if (nodeData.mAddress[0].IsIPv6LinkLocal()) + { + interfaceId = nodeData.mInterfaceId; + } + + return Transport::PeerAddress::UDP(nodeData.mAddress[0], nodeData.mPort, interfaceId); +} + +void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) +{ + VerifyOrReturn(mInitialized); + + if (mInitParams.dnsCache != nullptr) + { + LogErrorOnFailure(mInitParams.dnsCache->Insert(nodeData)); + } + + OperationalDeviceProxy * session = FindSession(nodeData.mPeerId.GetNodeId()); + VerifyOrReturn(session != nullptr); + + LogErrorOnFailure(session->UpdateDeviceData( + ToPeerAddress(nodeData), nodeData.GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL), + nodeData.GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL))); +} + +void CASESessionManager::OnNodeIdResolutionFailed(const PeerId & peer, CHIP_ERROR error) +{ + ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); +} + +CHIP_ERROR CASESessionManager::GetDeviceAddressAndPort(NodeId deviceId, Inet::IPAddress & addr, uint16_t & port) +{ + VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); + + OperationalDeviceProxy * session = FindSession(deviceId); + VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED); + VerifyOrReturnError(session->GetAddress(addr, port), CHIP_ERROR_NOT_CONNECTED); + return CHIP_NO_ERROR; +} + +void CASESessionManager::OnNewConnection(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) {} + +void CASESessionManager::OnConnectionExpired(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) +{ + VerifyOrReturn(mInitialized); + + OperationalDeviceProxy * session = FindSession(sessionHandle); + VerifyOrReturn(session != nullptr, + ChipLogDetail(Controller, "OnConnectionExpired was called for unknown device, ignoring it.")); + + session->OnConnectionExpired(sessionHandle); +} + +OperationalDeviceProxy * CASESessionManager::FindSession(SessionHandle session) +{ + OperationalDeviceProxy * foundSession = nullptr; + mActiveSessions.ForEachActiveObject([&](auto * activeSession) { + if (activeSession->MatchesSession(session)) + { + foundSession = activeSession; + return false; + } + return true; + }); + + return foundSession; +} + +OperationalDeviceProxy * CASESessionManager::FindSession(NodeId id) +{ + OperationalDeviceProxy * foundSession = nullptr; + mActiveSessions.ForEachActiveObject([&](auto * activeSession) { + if (activeSession->GetDeviceId() == id) + { + foundSession = activeSession; + return false; + } + return true; + }); + + return foundSession; +} + +void CASESessionManager::ReleaseSession(OperationalDeviceProxy * session) +{ + if (session != nullptr) + { + mActiveSessions.ReleaseObject(session); + } +} + +} // namespace chip diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h new file mode 100644 index 00000000000000..77d71e8778e898 --- /dev/null +++ b/src/app/CASESessionManager.h @@ -0,0 +1,87 @@ +/* + * + * Copyright (c) 2020-2021 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include +#include +#include + +#include + +namespace chip { + +struct CASESessionManagerInitParams +{ + DeviceProxyInitParams sessionInitParams; + Dnssd::DnssdCache * dnsCache = nullptr; +}; + +class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd::ResolverDelegate +{ +public: + CASESessionManager(CASESessionManagerInitParams & params) + { + VerifyOrReturn(params.sessionInitParams.Validate() == CHIP_NO_ERROR); + + mInitParams = params; + mInitialized = true; + } + + virtual ~CASESessionManager() {} + + void Shutdown() { mInitialized = false; } + + CHIP_ERROR FindOrEstablishSession(NodeId deviceId, Transport::PeerAddress addr, + Callback::Callback * onConnection, + Callback::Callback * onFailure); + + OperationalDeviceProxy * FindExistingSession(NodeId deviceId) { return FindSession(deviceId); } + + void ReleaseSession(NodeId deviceId); + + CHIP_ERROR ResolveDeviceAddress(NodeId deviceId); + CHIP_ERROR GetDeviceAddressAndPort(NodeId deviceId, Inet::IPAddress & addr, uint16_t & port); + + //////////// ExchangeMgrDelegate Implementation /////////////// + void OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) override; + void OnConnectionExpired(SessionHandle session, Messaging::ExchangeManager * mgr) override; + + //////////// ResolverDelegate Implementation /////////////// + void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override; + void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override; + void OnNodeDiscoveryComplete(const Dnssd::DiscoveredNodeData & nodeData) override {} + +private: + OperationalDeviceProxy * FindSession(SessionHandle session); + OperationalDeviceProxy * FindSession(NodeId id); + void ReleaseSession(OperationalDeviceProxy * device); + + Transport::PeerAddress ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) const; + + BitMapObjectPool mActiveSessions; + + CASESessionManagerInitParams mInitParams; + + bool mInitialized = false; +}; + +} // namespace chip diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index fd495860d23ce1..5d4cd48ffd5107 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -50,6 +50,16 @@ struct DeviceProxyInitParams FabricInfo * fabricInfo = nullptr; Controller::DeviceControllerInteractionModelDelegate * imDelegate = nullptr; + + CHIP_ERROR Validate() + { + ReturnErrorCodeIf(sessionManager == nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorCodeIf(exchangeMgr == nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorCodeIf(idAllocator == nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorCodeIf(fabricInfo == nullptr, CHIP_ERROR_INCORRECT_STATE); + + return CHIP_NO_ERROR; + } }; class OperationalDeviceProxy; @@ -61,12 +71,9 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta { public: virtual ~OperationalDeviceProxy(); - OperationalDeviceProxy(DeviceProxyInitParams params, PeerId peerId) + OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) { - VerifyOrReturn(params.sessionManager != nullptr); - VerifyOrReturn(params.exchangeMgr != nullptr); - VerifyOrReturn(params.idAllocator != nullptr); - VerifyOrReturn(params.fabricInfo != nullptr); + VerifyOrReturn(params.Validate() == CHIP_NO_ERROR); mInitParams = params; mPeerId = peerId; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 138af750d4aa49..370b6bb4b87fdf 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -149,6 +149,18 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) ReturnErrorOnFailure(ProcessControllerNOCChain(params)); + CASESessionManagerInitParams sessionParams = { + .sessionInitParams.sessionManager = params.systemState->SessionMgr(), + .sessionInitParams.exchangeMgr = params.systemState->ExchangeMgr(), + .sessionInitParams.idAllocator = &mIDAllocator, + .sessionInitParams.fabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex), + .sessionInitParams.imDelegate = params.systemState->IMDelegate(), + .dnsCache = &mDNSCache, + }; + + mCASESessionManager = chip::Platform::New(sessionParams); + VerifyOrReturnError(mCASESessionManager != nullptr, CHIP_ERROR_NO_MEMORY); + mSystemState = params.systemState->Retain(); mState = State::Initialized; return CHIP_NO_ERROR; @@ -227,6 +239,9 @@ CHIP_ERROR DeviceController::Shutdown() mDeviceDiscoveryDelegate = nullptr; #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD + chip::Platform::Delete(mCASESessionManager); + mCASESessionManager = nullptr; + return CHIP_NO_ERROR; } @@ -240,117 +255,17 @@ bool DeviceController::DoesDevicePairingExist(const PeerId & deviceId) return false; } -CHIP_ERROR DeviceController::GetOperationalDeviceWithAddress(NodeId deviceId, const Transport::PeerAddress & addr, - Callback::Callback * onConnection, - Callback::Callback * onFailure) +void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId) { - OperationalDeviceProxy * device = FindOperationalDevice(deviceId); - if (device == nullptr) - { - FabricInfo * fabric = mSystemState->Fabrics()->FindFabricWithIndex(mFabricIndex); - VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - DeviceProxyInitParams initParams = { - .sessionManager = mSystemState->SessionMgr(), - .exchangeMgr = mSystemState->ExchangeMgr(), - .idAllocator = &mIDAllocator, - .fabricInfo = fabric, - .imDelegate = mSystemState->IMDelegate(), - }; - - PeerId peerID = fabric->GetPeerId(); - peerID.SetNodeId(deviceId); - - device = mOperationalDevices.CreateObject(initParams, peerID); - if (device == nullptr) - { - onFailure->mCall(onFailure->mContext, deviceId, CHIP_ERROR_NO_MEMORY); - return CHIP_ERROR_NO_MEMORY; - } - } - - CHIP_ERROR err = CHIP_NO_ERROR; - if (addr != Transport::PeerAddress::UDP(Inet::IPAddress::Any)) - { - uint32_t idleInterval; - uint32_t activeInterval; - device->GetMRPIntervals(idleInterval, activeInterval); - err = device->UpdateDeviceData(addr, idleInterval, activeInterval); - if (err != CHIP_NO_ERROR) - { - ReleaseOperationalDevice(device); - return err; - } - } - - err = device->Connect(onConnection, onFailure); - if (err != CHIP_NO_ERROR) - { - ReleaseOperationalDevice(device); - } - - return err; + VerifyOrReturn(mState == State::Initialized, + ChipLogError(Controller, "ReleaseOperationalDevice was called in incorrect state")); + mCASESessionManager->ReleaseSession(remoteDeviceId); } -CHIP_ERROR DeviceController::UpdateDevice(NodeId deviceId) -{ -#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD - return Dnssd::Resolver::Instance().ResolveNodeId(PeerId().SetCompressedFabricId(GetCompressedFabricId()).SetNodeId(deviceId), - chip::Inet::IPAddressType::kAny); -#else - return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; -#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD -} - -void DeviceController::OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) {} - void DeviceController::OnConnectionExpired(SessionHandle session, Messaging::ExchangeManager * mgr) { VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnConnectionExpired was called in incorrect state")); - - OperationalDeviceProxy * device = FindOperationalDevice(session); - VerifyOrReturn(device != nullptr, ChipLogDetail(Controller, "OnConnectionExpired was called for unknown device, ignoring it.")); - - device->OnConnectionExpired(session); -} - -OperationalDeviceProxy * DeviceController::FindOperationalDevice(SessionHandle session) -{ - OperationalDeviceProxy * foundDevice = nullptr; - mOperationalDevices.ForEachActiveObject([&](auto * deviceProxy) { - if (deviceProxy->MatchesSession(session)) - { - foundDevice = deviceProxy; - return false; - } - return true; - }); - - return foundDevice; -} - -OperationalDeviceProxy * DeviceController::FindOperationalDevice(NodeId id) -{ - OperationalDeviceProxy * foundDevice = nullptr; - mOperationalDevices.ForEachActiveObject([&](auto * deviceProxy) { - if (deviceProxy->GetDeviceId() == id) - { - foundDevice = deviceProxy; - return false; - } - return true; - }); - - return foundDevice; -} - -void DeviceController::ReleaseOperationalDevice(NodeId id) -{ - ReleaseOperationalDevice(FindOperationalDevice(id)); -} - -void DeviceController::ReleaseOperationalDevice(OperationalDeviceProxy * device) -{ - mOperationalDevices.ReleaseObject(device); + mCASESessionManager->OnConnectionExpired(session, mgr); } CHIP_ERROR DeviceController::InitializePairedDeviceList() @@ -422,10 +337,8 @@ void DeviceController::PersistNextKeyId() CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port) { - VerifyOrReturnError(GetCompressedFabricId() == peerId.GetCompressedFabricId(), CHIP_ERROR_INVALID_ARGUMENT); - OperationalDeviceProxy * device = FindOperationalDevice(peerId.GetNodeId()); - VerifyOrReturnError(device->GetAddress(addr, port), CHIP_ERROR_NOT_CONNECTED); - return CHIP_NO_ERROR; + VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + return mCASESessionManager->GetDeviceAddressAndPort(peerId.GetNodeId(), addr, port); } void DeviceController::OnOpenPairingWindowSuccessResponse(void * context) @@ -473,7 +386,7 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowWithCallback(NodeId deviceId ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, deviceId); VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); - OperationalDeviceProxy * device = FindOperationalDevice(deviceId); + OperationalDeviceProxy * device = mCASESessionManager->FindExistingSession(deviceId); VerifyOrReturnError(device != nullptr, CHIP_ERROR_INVALID_ARGUMENT); std::string QRCode; @@ -567,22 +480,19 @@ Transport::PeerAddress DeviceController::ToPeerAddress(const chip::Dnssd::Resolv void DeviceController::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) { - OperationalDeviceProxy * device = FindOperationalDevice(nodeData.mPeerId.GetNodeId()); - VerifyOrReturn(device != nullptr); - - CHIP_ERROR err = device->UpdateDeviceData( - ToPeerAddress(nodeData), nodeData.GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL), - nodeData.GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL)); - + VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnNodeIdResolved was called in incorrect state")); + mCASESessionManager->OnNodeIdResolved(nodeData); if (mDeviceAddressUpdateDelegate != nullptr) { - mDeviceAddressUpdateDelegate->OnAddressUpdateComplete(nodeData.mPeerId.GetNodeId(), err); + mDeviceAddressUpdateDelegate->OnAddressUpdateComplete(nodeData.mPeerId.GetNodeId(), CHIP_NO_ERROR); } }; void DeviceController::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error) { ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); + VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnNodeIdResolved was called in incorrect state")); + mCASESessionManager->OnNodeIdResolutionFailed(peer, error); if (mDeviceAddressUpdateDelegate != nullptr) { @@ -1614,8 +1524,8 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & mDeviceBeingCommissioned = nullptr; } - GetOperationalDeviceWithAddress(nodeData.mPeerId.GetNodeId(), ToPeerAddress(nodeData), &mOnDeviceConnectedCallback, - &mOnDeviceConnectionFailureCallback); + mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId.GetNodeId(), ToPeerAddress(nodeData), &mOnDeviceConnectedCallback, + &mOnDeviceConnectionFailureCallback); DeviceController::OnNodeIdResolved(nodeData); } diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 86315339e57964..31a81cb7f0dc0f 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -28,6 +28,7 @@ #pragma once +#include #include #include #include @@ -216,17 +217,6 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, */ virtual CHIP_ERROR Shutdown(); - CHIP_ERROR GetOperationalDevice(NodeId deviceId, Callback::Callback * onConnection, - Callback::Callback * onFailure) - { - return GetOperationalDeviceWithAddress(deviceId, Transport::PeerAddress::UDP(Inet::IPAddress::Any), onConnection, - onFailure); - } - - CHIP_ERROR GetOperationalDeviceWithAddress(NodeId deviceId, const Transport::PeerAddress & addr, - Callback::Callback * onConnection, - Callback::Callback * onFailure); - CHIP_ERROR GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port); /** @@ -243,8 +233,9 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, virtual CHIP_ERROR GetConnectedDevice(NodeId deviceId, Callback::Callback * onConnection, Callback::Callback * onFailure) { - return GetOperationalDeviceWithAddress(deviceId, Transport::PeerAddress::UDP(Inet::IPAddress::Any), onConnection, - onFailure); + VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + return mCASESessionManager->FindOrEstablishSession(deviceId, Transport::PeerAddress::UDP(Inet::IPAddress::Any), + onConnection, onFailure); } /** @@ -255,7 +246,11 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, * * @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error code. */ - CHIP_ERROR UpdateDevice(NodeId deviceId); + CHIP_ERROR UpdateDevice(NodeId deviceId) + { + VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); + return mCASESessionManager->ResolveDeviceAddress(deviceId); + } /** * @brief @@ -336,15 +331,6 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, */ uint64_t GetFabricId() const { return mFabricId; } - DeviceControllerInteractionModelDelegate * GetInteractionModelDelegate() - { - if (mSystemState != nullptr) - { - return mSystemState->IMDelegate(); - } - return nullptr; - } - void ReleaseOperationalDevice(NodeId remoteDeviceId); protected: @@ -356,7 +342,9 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, State mState; - BitMapObjectPool mOperationalDevices; + CASESessionManager * mCASESessionManager = nullptr; + + Dnssd::DnssdCache mDNSCache; SerializableU64Set mPairedDevices; bool mPairedDevicesInitialized; @@ -390,7 +378,7 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, uint16_t mVendorId; //////////// ExchangeMgrDelegate Implementation /////////////// - void OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) override; + void OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) override {} void OnConnectionExpired(SessionHandle session, Messaging::ExchangeManager * mgr) override; #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD @@ -401,12 +389,8 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD private: - OperationalDeviceProxy * FindOperationalDevice(SessionHandle session); - OperationalDeviceProxy * FindOperationalDevice(NodeId id); void ReleaseOperationalDevice(OperationalDeviceProxy * device); - void ReleaseAllDevices(); - Callback::Callback mOpenPairingSuccessCallback; Callback::Callback mOpenPairingFailureCallback; From 9ba906fa1b753625ff6eb2a593099b9412aae8b1 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 11 Nov 2021 16:07:44 -0800 Subject: [PATCH 2/7] fix build errors --- src/controller/CHIPDeviceController.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 370b6bb4b87fdf..ac43b88aaad239 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -149,16 +149,20 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) ReturnErrorOnFailure(ProcessControllerNOCChain(params)); - CASESessionManagerInitParams sessionParams = { - .sessionInitParams.sessionManager = params.systemState->SessionMgr(), - .sessionInitParams.exchangeMgr = params.systemState->ExchangeMgr(), - .sessionInitParams.idAllocator = &mIDAllocator, - .sessionInitParams.fabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex), - .sessionInitParams.imDelegate = params.systemState->IMDelegate(), - .dnsCache = &mDNSCache, + DeviceProxyInitParams deviceInitParams = { + .sessionManager = params.systemState->SessionMgr(), + .exchangeMgr = params.systemState->ExchangeMgr(), + .idAllocator = &mIDAllocator, + .fabricInfo = params.systemState->Fabrics()->FindFabricWithIndex(mFabricIndex), + .imDelegate = params.systemState->IMDelegate(), }; - mCASESessionManager = chip::Platform::New(sessionParams); + CASESessionManagerInitParams sessionManagerParams = { + .sessionInitParams = deviceInitParams, + .dnsCache = &mDNSCache, + }; + + mCASESessionManager = chip::Platform::New(sessionManagerParams); VerifyOrReturnError(mCASESessionManager != nullptr, CHIP_ERROR_NO_MEMORY); mSystemState = params.systemState->Retain(); From 83037da4fc98e14712773ce3e37da934813a4531 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 15 Nov 2021 14:33:50 -0800 Subject: [PATCH 3/7] address review comments --- src/app/CASESessionManager.cpp | 92 +++++++++---------------- src/app/CASESessionManager.h | 43 ++++++++---- src/app/OperationalDeviceProxy.h | 58 +++++++++------- src/controller/CHIPDeviceController.cpp | 28 ++++++-- src/controller/CHIPDeviceController.h | 6 +- 5 files changed, 123 insertions(+), 104 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 18ec0c89602fa2..aa7be9ef58476d 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -20,45 +20,29 @@ namespace chip { -CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId deviceId, Transport::PeerAddress address, - Callback::Callback * onConnection, +CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::Callback * onConnection, Callback::Callback * onFailure) { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - PeerId peerId = mInitParams.sessionInitParams.fabricInfo->GetPeerIdForNode(deviceId); - OperationalDeviceProxy * session = FindSession(deviceId); + OperationalDeviceProxy * session = FindExistingSession(nodeId); if (session == nullptr) { - session = mActiveSessions.CreateObject(mInitParams.sessionInitParams, peerId); - if (session == nullptr) - { - onFailure->mCall(onFailure->mContext, deviceId, CHIP_ERROR_NO_MEMORY); - return CHIP_ERROR_NO_MEMORY; - } - } + Dnssd::ResolvedNodeData * nodeResolutionData = nullptr; + Dnssd::ResolvedNodeData cachedResolutionData; - bool validAddress = (address != Transport::PeerAddress::UDP(Inet::IPAddress::Any)); + PeerId peerId = GetFabricInfo()->GetPeerIdForNode(nodeId); - if (!validAddress && mInitParams.dnsCache != nullptr) - { - Dnssd::ResolvedNodeData nodeData; - if (mInitParams.dnsCache->Lookup(peerId, nodeData) == CHIP_NO_ERROR) + if (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, cachedResolutionData) == CHIP_NO_ERROR) { - address = ToPeerAddress(nodeData); - validAddress = true; + nodeResolutionData = &cachedResolutionData; } - } - if (validAddress) - { - uint32_t idleInterval; - uint32_t activeInterval; - session->GetMRPIntervals(idleInterval, activeInterval); - CHIP_ERROR err = session->UpdateDeviceData(address, idleInterval, activeInterval); - if (err != CHIP_NO_ERROR) + // TODO - Implement LRU to evict least recently used session to handle mActiveSessions pool exhaustion + session = mActiveSessions.CreateObject(mConfig.sessionInitParams, peerId, nodeResolutionData); + if (session == nullptr) { - ReleaseSession(session); - return err; + onFailure->mCall(onFailure->mContext, nodeId, CHIP_ERROR_NO_MEMORY); + return CHIP_ERROR_NO_MEMORY; } } @@ -71,48 +55,33 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId deviceId, Transport return err; } -void CASESessionManager::ReleaseSession(NodeId deviceId) +void CASESessionManager::ReleaseSession(NodeId nodeId) { - ReleaseSession(FindSession(deviceId)); + ReleaseSession(FindExistingSession(nodeId)); } -CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId deviceId) +CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId nodeId) { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - return Dnssd::Resolver::Instance().ResolveNodeId(mInitParams.sessionInitParams.fabricInfo->GetPeerIdForNode(deviceId), - Inet::IPAddressType::kAny); -} - -Transport::PeerAddress CASESessionManager::ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) const -{ - Inet::InterfaceId interfaceId; - - // Only use the mDNS resolution's InterfaceID for addresses that are IPv6 LLA. - // For all other addresses, we should rely on the device's routing table to route messages sent. - // Forcing messages down an InterfaceId might fail. For example, in bridged networks like Thread, - // mDNS advertisements are not usually received on the same interface the peer is reachable on. - if (nodeData.mAddress[0].IsIPv6LinkLocal()) - { - interfaceId = nodeData.mInterfaceId; - } - - return Transport::PeerAddress::UDP(nodeData.mAddress[0], nodeData.mPort, interfaceId); + return Dnssd::Resolver::Instance().ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny); } void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) { - VerifyOrReturn(mInitialized); + ChipLogProgress(Controller, "Address resolved for node: 0x" ChipLogFormatX64, ChipLogValueX64(nodeData.mPeerId.GetNodeId())); + VerifyOrReturn(mInitialized, ChipLogError(Controller, "OnNodeIdResolved was called in uninitialized state")); - if (mInitParams.dnsCache != nullptr) + if (mConfig.dnsCache != nullptr) { - LogErrorOnFailure(mInitParams.dnsCache->Insert(nodeData)); + LogErrorOnFailure(mConfig.dnsCache->Insert(nodeData)); } - OperationalDeviceProxy * session = FindSession(nodeData.mPeerId.GetNodeId()); - VerifyOrReturn(session != nullptr); + OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId.GetNodeId()); + VerifyOrReturn(session != nullptr, + ChipLogDetail(Controller, "OnNodeIdResolved was called for a device with no active sessions, ignoring it.")); LogErrorOnFailure(session->UpdateDeviceData( - ToPeerAddress(nodeData), nodeData.GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL), + session->ToPeerAddress(nodeData), nodeData.GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL), nodeData.GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL))); } @@ -121,21 +90,24 @@ void CASESessionManager::OnNodeIdResolutionFailed(const PeerId & peer, CHIP_ERRO ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); } -CHIP_ERROR CASESessionManager::GetDeviceAddressAndPort(NodeId deviceId, Inet::IPAddress & addr, uint16_t & port) +CHIP_ERROR CASESessionManager::GetDeviceAddressAndPort(NodeId nodeId, Inet::IPAddress & addr, uint16_t & port) { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - OperationalDeviceProxy * session = FindSession(deviceId); + OperationalDeviceProxy * session = FindExistingSession(nodeId); VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED); VerifyOrReturnError(session->GetAddress(addr, port), CHIP_ERROR_NOT_CONNECTED); return CHIP_NO_ERROR; } -void CASESessionManager::OnNewConnection(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) {} +void CASESessionManager::OnNewConnection(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) +{ + // TODO Update the MRP params based on the MRP params extracted from CASE, when this is available. +} void CASESessionManager::OnConnectionExpired(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) { - VerifyOrReturn(mInitialized); + VerifyOrReturn(mInitialized, ChipLogError(Controller, "OnConnectionExpired was called in uninitialized state")); OperationalDeviceProxy * session = FindSession(sessionHandle); VerifyOrReturn(session != nullptr, @@ -159,7 +131,7 @@ OperationalDeviceProxy * CASESessionManager::FindSession(SessionHandle session) return foundSession; } -OperationalDeviceProxy * CASESessionManager::FindSession(NodeId id) +OperationalDeviceProxy * CASESessionManager::FindExistingSession(NodeId id) { OperationalDeviceProxy * foundSession = nullptr; mActiveSessions.ForEachActiveObject([&](auto * activeSession) { diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index 77d71e8778e898..9a4f850ce7760e 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -29,20 +29,28 @@ namespace chip { -struct CASESessionManagerInitParams +struct CASESessionManagerConfig { DeviceProxyInitParams sessionInitParams; Dnssd::DnssdCache * dnsCache = nullptr; }; +/** + * This class provides the following + * 1. Manage a pool of operational device proxy objects for peer nodes that have active message exchange with the local node. + * 2. The pool contains atmost one device proxy object for a given peer node. + * 3. API to lookup an existing proxy object, or allocate a new one by triggering session establishment with the peer node. + * 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is + * successful) + */ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd::ResolverDelegate { public: - CASESessionManager(CASESessionManagerInitParams & params) + CASESessionManager(CASESessionManagerConfig & params) { VerifyOrReturn(params.sessionInitParams.Validate() == CHIP_NO_ERROR); - mInitParams = params; + mConfig = params; mInitialized = true; } @@ -50,16 +58,28 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: void Shutdown() { mInitialized = false; } - CHIP_ERROR FindOrEstablishSession(NodeId deviceId, Transport::PeerAddress addr, - Callback::Callback * onConnection, + /** + * Find an existing session for the given node ID, or trigger a new session request. + */ + CHIP_ERROR FindOrEstablishSession(NodeId nodeId, Callback::Callback * onConnection, Callback::Callback * onFailure); - OperationalDeviceProxy * FindExistingSession(NodeId deviceId) { return FindSession(deviceId); } + OperationalDeviceProxy * FindExistingSession(NodeId nodeId); + + void ReleaseSession(NodeId nodeId); + + FabricInfo * GetFabricInfo() + { + if (!mInitialized) + { + return nullptr; + } - void ReleaseSession(NodeId deviceId); + return mConfig.sessionInitParams.fabricInfo; + } - CHIP_ERROR ResolveDeviceAddress(NodeId deviceId); - CHIP_ERROR GetDeviceAddressAndPort(NodeId deviceId, Inet::IPAddress & addr, uint16_t & port); + CHIP_ERROR ResolveDeviceAddress(NodeId nodeId); + CHIP_ERROR GetDeviceAddressAndPort(NodeId nodeId, Inet::IPAddress & addr, uint16_t & port); //////////// ExchangeMgrDelegate Implementation /////////////// void OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) override; @@ -72,14 +92,11 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: private: OperationalDeviceProxy * FindSession(SessionHandle session); - OperationalDeviceProxy * FindSession(NodeId id); void ReleaseSession(OperationalDeviceProxy * device); - Transport::PeerAddress ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) const; - BitMapObjectPool mActiveSessions; - CASESessionManagerInitParams mInitParams; + CASESessionManagerConfig mConfig; bool mInitialized = false; }; diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index 5d4cd48ffd5107..77c21180012d08 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -40,6 +40,8 @@ #include #include +#include + namespace chip { struct DeviceProxyInitParams @@ -71,13 +73,27 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta { public: virtual ~OperationalDeviceProxy(); - OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) + OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId, const Dnssd::ResolvedNodeData * nodeResolutionData) { VerifyOrReturn(params.Validate() == CHIP_NO_ERROR); mInitParams = params; mPeerId = peerId; - mState = State::NeedsAddress; + + if (nodeResolutionData != nullptr) + { + mDeviceAddress = ToPeerAddress(*nodeResolutionData); + + mMrpIdleInterval = nodeResolutionData->GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL); + mMrpActiveInterval = + nodeResolutionData->GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); + + mState = State::Initialized; + } + else + { + mState = State::NeedsAddress; + } } void Clear(); @@ -112,27 +128,6 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta CHIP_ERROR Disconnect() override; NodeId GetDeviceId() const override { return mPeerId.GetNodeId(); } - /* - // ----- Messaging ----- - CHIP_ERROR SendReadAttributeRequest(app::AttributePathParams aPath, Callback::Cancelable * onSuccessCallback, - Callback::Cancelable * onFailureCallback, app::TLVDataFilter aTlvDataFilter) override; - - CHIP_ERROR SendSubscribeAttributeRequest(app::AttributePathParams aPath, uint16_t mMinIntervalFloorSeconds, - uint16_t mMaxIntervalCeilingSeconds, Callback::Cancelable * onSuccessCallback, - Callback::Cancelable * onFailureCallback) override; - - CHIP_ERROR SendWriteAttributeRequest(app::WriteClientHandle aHandle, Callback::Cancelable * onSuccessCallback, - Callback::Cancelable * onFailureCallback) override; - - CHIP_ERROR SendCommands(app::CommandSender * commandObj) override; - - void AddReportHandler(EndpointId endpoint, ClusterId cluster, AttributeId attribute, Callback::Cancelable * - onReportCallback, app::TLVDataFilter tlvDataFilter) override; - - void AddIMResponseHandler(void * commandObj, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * - onFailureCallback, app::TLVDataFilter tlvDataFilter = nullptr) override; void CancelIMResponseHandler(void * commandObj) - override; - */ /** * Update data of the device. @@ -164,6 +159,23 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta bool GetAddress(Inet::IPAddress & addr, uint16_t & port) const override; + static Transport::PeerAddress ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) + { + Inet::InterfaceId interfaceId = Inet::InterfaceId::Null(); + + // TODO - Revisit usage of InterfaceID only for addresses that are IPv6 LLA + // Only use the DNS-SD resolution's InterfaceID for addresses that are IPv6 LLA. + // For all other addresses, we should rely on the device's routing table to route messages sent. + // Forcing messages down an InterfaceId might fail. For example, in bridged networks like Thread, + // mDNS advertisements are not usually received on the same interface the peer is reachable on. + if (nodeData.mAddress[0].IsIPv6LinkLocal()) + { + interfaceId = nodeData.mInterfaceId; + } + + return Transport::PeerAddress::UDP(nodeData.mAddress[0], nodeData.mPort, interfaceId); + } + private: enum class State { diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index ac43b88aaad239..f8fc27c413cbdd 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -157,12 +157,12 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params) .imDelegate = params.systemState->IMDelegate(), }; - CASESessionManagerInitParams sessionManagerParams = { + CASESessionManagerConfig sessionManagerConfig = { .sessionInitParams = deviceInitParams, .dnsCache = &mDNSCache, }; - mCASESessionManager = chip::Platform::New(sessionManagerParams); + mCASESessionManager = chip::Platform::New(sessionManagerConfig); VerifyOrReturnError(mCASESessionManager != nullptr, CHIP_ERROR_NO_MEMORY); mSystemState = params.systemState->Retain(); @@ -495,7 +495,8 @@ void DeviceController::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & no void DeviceController::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error) { ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); - VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnNodeIdResolved was called in incorrect state")); + VerifyOrReturn(mState == State::Initialized, + ChipLogError(Controller, "OnNodeIdResolutionFailed was called in incorrect state")); mCASESessionManager->OnNodeIdResolutionFailed(peer, error); if (mDeviceAddressUpdateDelegate != nullptr) @@ -801,10 +802,15 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam device->SetActive(true); err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID, exchangeCtxt, this); + SuccessOrExit(err); + // Immediately persist the updted mNextKeyID value // TODO maybe remove FreeRendezvousSession() since mNextKeyID is always persisted immediately PersistNextKeyId(); + mDeviceCommissioningInProgress = true; + mNodeIdBeingCommissioned = remoteDeviceId; + exit: if (err != CHIP_NO_ERROR) { @@ -834,6 +840,7 @@ CHIP_ERROR DeviceCommissioner::StopPairing(NodeId remoteDeviceId) FreeRendezvousSession(); ReleaseCommissioneeDevice(device); + mDeviceCommissioningInProgress = false; return CHIP_NO_ERROR; } @@ -883,6 +890,7 @@ void DeviceCommissioner::OnSessionEstablishmentError(CHIP_ERROR err) ReleaseCommissioneeDevice(mDeviceBeingCommissioned); mDeviceBeingCommissioned = nullptr; } + mDeviceCommissioningInProgress = false; } void DeviceCommissioner::OnSessionEstablished() @@ -1528,8 +1536,13 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & mDeviceBeingCommissioned = nullptr; } - mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId.GetNodeId(), ToPeerAddress(nodeData), &mOnDeviceConnectedCallback, - &mOnDeviceConnectionFailureCallback); + mDNSCache.Insert(nodeData); + + if (mDeviceCommissioningInProgress && mNodeIdBeingCommissioned == nodeData.mPeerId.GetNodeId()) + { + mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId.GetNodeId(), &mOnDeviceConnectedCallback, + &mOnDeviceConnectionFailureCallback); + } DeviceController::OnNodeIdResolved(nodeData); } @@ -1572,6 +1585,7 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, DeviceProxy * devic VerifyOrReturn(commissioner->mPairingDelegate != nullptr, ChipLogProgress(Controller, "Device connected callback with null pairing delegate. Ignoring")); + commissioner->mDeviceCommissioningInProgress = false; commissioner->mPairingDelegate->OnCommissioningComplete(device->GetDeviceId(), CHIP_NO_ERROR); } @@ -1583,6 +1597,7 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, NodeId devi ChipLogProgress(Controller, "Device connection failure callback with null context. Ignoring")); VerifyOrReturn(commissioner->mPairingDelegate != nullptr, ChipLogProgress(Controller, "Device connection failure callback with null pairing delegate. Ignoring")); + commissioner->mDeviceCommissioningInProgress = false; commissioner->mPairingDelegate->OnCommissioningComplete(deviceId, error); } @@ -1780,7 +1795,8 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err) { mPairingDelegate->OnCommissioningComplete(mDeviceOperational->GetDeviceId(), CHIP_NO_ERROR); } - mDeviceOperational = nullptr; + mDeviceOperational = nullptr; + mDeviceCommissioningInProgress = false; break; case CommissioningStage::kSecurePairing: case CommissioningStage::kError: diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 31a81cb7f0dc0f..b4c90f7fb0cebf 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -234,8 +234,7 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeMgrDelegate, Callback::Callback * onFailure) { VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); - return mCASESessionManager->FindOrEstablishSession(deviceId, Transport::PeerAddress::UDP(Inet::IPAddress::Any), - onConnection, onFailure); + return mCASESessionManager->FindOrEstablishSession(deviceId, onConnection, onFailure); } /** @@ -608,6 +607,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, persist the device list */ bool mPairedDevicesUpdated; + bool mDeviceCommissioningInProgress = false; + NodeId mNodeIdBeingCommissioned; + CommissioningStage mCommissioningStage = CommissioningStage::kSecurePairing; BitMapObjectPool mCommissioneeDevicePool; From f1cc79033e1a34bb8c50d7bdb7ea938499dfc847 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 15 Nov 2021 16:05:36 -0800 Subject: [PATCH 4/7] fix apps --- examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp | 2 +- examples/ota-requestor-app/linux/main.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp index 8c523855e5573a..fd5a8d202c50d4 100644 --- a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp +++ b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp @@ -294,7 +294,7 @@ void ConnectToProvider(const char * ipAddress, uint32_t nodeId) PeerId peerID = fabric->GetPeerId(); peerID.SetNodeId(providerNodeId); - operationalDeviceProxy = new chip::OperationalDeviceProxy(initParams, peerID); + operationalDeviceProxy = new chip::OperationalDeviceProxy(initParams, peerID, nullptr); server->SetOperationalDeviceProxy(operationalDeviceProxy); // Explicitly calling UpdateDeviceData() should not be needed once OperationalDeviceProxy can resolve IP address from node diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 1a83052fa6ab46..318c9cd8786bbc 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -272,7 +272,7 @@ void SendQueryImageCommand(chip::NodeId peerNodeId = providerNodeId, chip::Fabri }; chip::OperationalDeviceProxy * operationalDeviceProxy = - chip::Platform::New(initParams, fabric->GetPeerIdForNode(peerNodeId)); + chip::Platform::New(initParams, fabric->GetPeerIdForNode(peerNodeId), nullptr); if (operationalDeviceProxy == nullptr) { ChipLogError(SoftwareUpdate, "Failed in creating an instance of OperationalDeviceProxy"); From d05654eb0512acd95591d0e770886b4892acec64 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 15 Nov 2021 17:07:52 -0800 Subject: [PATCH 5/7] cleanup --- src/app/CASESessionManager.cpp | 25 +++++++++++-------- src/app/OperationalDeviceProxy.h | 33 ++++++++++++++----------- src/controller/CHIPDeviceController.cpp | 18 +++----------- src/controller/CHIPDeviceController.h | 3 --- 4 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index aa7be9ef58476d..7f964f2b7d96c3 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -24,19 +24,20 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::C Callback::Callback * onFailure) { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - OperationalDeviceProxy * session = FindExistingSession(nodeId); - if (session == nullptr) - { - Dnssd::ResolvedNodeData * nodeResolutionData = nullptr; - Dnssd::ResolvedNodeData cachedResolutionData; - PeerId peerId = GetFabricInfo()->GetPeerIdForNode(nodeId); + Dnssd::ResolvedNodeData * nodeResolutionData = nullptr; + Dnssd::ResolvedNodeData cachedResolutionData; - if (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, cachedResolutionData) == CHIP_NO_ERROR) - { - nodeResolutionData = &cachedResolutionData; - } + PeerId peerId = GetFabricInfo()->GetPeerIdForNode(nodeId); + if (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, cachedResolutionData) == CHIP_NO_ERROR) + { + nodeResolutionData = &cachedResolutionData; + } + + OperationalDeviceProxy * session = FindExistingSession(nodeId); + if (session == nullptr) + { // TODO - Implement LRU to evict least recently used session to handle mActiveSessions pool exhaustion session = mActiveSessions.CreateObject(mConfig.sessionInitParams, peerId, nodeResolutionData); if (session == nullptr) @@ -45,6 +46,10 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::C return CHIP_ERROR_NO_MEMORY; } } + else if (nodeResolutionData != nullptr) + { + session->OnNodeIdResolved(nodeResolutionData); + } CHIP_ERROR err = session->Connect(onConnection, onFailure); if (err != CHIP_NO_ERROR) diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index 77c21180012d08..c7c0e456312806 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -80,20 +80,8 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta mInitParams = params; mPeerId = peerId; - if (nodeResolutionData != nullptr) - { - mDeviceAddress = ToPeerAddress(*nodeResolutionData); - - mMrpIdleInterval = nodeResolutionData->GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL); - mMrpActiveInterval = - nodeResolutionData->GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); - - mState = State::Initialized; - } - else - { - mState = State::NeedsAddress; - } + mState = State::NeedsAddress; + OnNodeIdResolved(nodeResolutionData); } void Clear(); @@ -122,6 +110,23 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta */ void OnConnectionExpired(SessionHandle session) override; + void OnNodeIdResolved(const Dnssd::ResolvedNodeData * nodeResolutionData) + { + if (nodeResolutionData != nullptr) + { + mDeviceAddress = ToPeerAddress(*nodeResolutionData); + + mMrpIdleInterval = nodeResolutionData->GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL); + mMrpActiveInterval = + nodeResolutionData->GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); + + if (mState == State::NeedsAddress) + { + mState = State::Initialized; + } + } + } + /** * Mark any open session with the device as expired. */ diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index f8fc27c413cbdd..c7b2f76c39a577 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -808,9 +808,6 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam // TODO maybe remove FreeRendezvousSession() since mNextKeyID is always persisted immediately PersistNextKeyId(); - mDeviceCommissioningInProgress = true; - mNodeIdBeingCommissioned = remoteDeviceId; - exit: if (err != CHIP_NO_ERROR) { @@ -840,7 +837,6 @@ CHIP_ERROR DeviceCommissioner::StopPairing(NodeId remoteDeviceId) FreeRendezvousSession(); ReleaseCommissioneeDevice(device); - mDeviceCommissioningInProgress = false; return CHIP_NO_ERROR; } @@ -890,7 +886,6 @@ void DeviceCommissioner::OnSessionEstablishmentError(CHIP_ERROR err) ReleaseCommissioneeDevice(mDeviceBeingCommissioned); mDeviceBeingCommissioned = nullptr; } - mDeviceCommissioningInProgress = false; } void DeviceCommissioner::OnSessionEstablished() @@ -1538,12 +1533,8 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & mDNSCache.Insert(nodeData); - if (mDeviceCommissioningInProgress && mNodeIdBeingCommissioned == nodeData.mPeerId.GetNodeId()) - { - mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId.GetNodeId(), &mOnDeviceConnectedCallback, - &mOnDeviceConnectionFailureCallback); - } - + mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId.GetNodeId(), &mOnDeviceConnectedCallback, + &mOnDeviceConnectionFailureCallback); DeviceController::OnNodeIdResolved(nodeData); } @@ -1585,7 +1576,6 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, DeviceProxy * devic VerifyOrReturn(commissioner->mPairingDelegate != nullptr, ChipLogProgress(Controller, "Device connected callback with null pairing delegate. Ignoring")); - commissioner->mDeviceCommissioningInProgress = false; commissioner->mPairingDelegate->OnCommissioningComplete(device->GetDeviceId(), CHIP_NO_ERROR); } @@ -1597,7 +1587,6 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, NodeId devi ChipLogProgress(Controller, "Device connection failure callback with null context. Ignoring")); VerifyOrReturn(commissioner->mPairingDelegate != nullptr, ChipLogProgress(Controller, "Device connection failure callback with null pairing delegate. Ignoring")); - commissioner->mDeviceCommissioningInProgress = false; commissioner->mPairingDelegate->OnCommissioningComplete(deviceId, error); } @@ -1795,8 +1784,7 @@ void DeviceCommissioner::AdvanceCommissioningStage(CHIP_ERROR err) { mPairingDelegate->OnCommissioningComplete(mDeviceOperational->GetDeviceId(), CHIP_NO_ERROR); } - mDeviceOperational = nullptr; - mDeviceCommissioningInProgress = false; + mDeviceOperational = nullptr; break; case CommissioningStage::kSecurePairing: case CommissioningStage::kError: diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index b4c90f7fb0cebf..8cf10ea0f8dbb8 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -607,9 +607,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, persist the device list */ bool mPairedDevicesUpdated; - bool mDeviceCommissioningInProgress = false; - NodeId mNodeIdBeingCommissioned; - CommissioningStage mCommissioningStage = CommissioningStage::kSecurePairing; BitMapObjectPool mCommissioneeDevicePool; From 4e0725def2a5c0a2b45f298966ef8c830b98d5ce Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 16 Nov 2021 08:51:39 -0800 Subject: [PATCH 6/7] address review comments --- .../esp32/main/OTARequesterImpl.cpp | 2 +- examples/ota-requestor-app/linux/main.cpp | 2 +- src/app/CASESessionManager.cpp | 22 +++++++++------- src/app/CASESessionManager.h | 16 ++++++++++++ src/app/OperationalDeviceProxy.cpp | 7 +++--- src/app/OperationalDeviceProxy.h | 25 ++++++++++--------- 6 files changed, 48 insertions(+), 26 deletions(-) diff --git a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp index fd5a8d202c50d4..8c523855e5573a 100644 --- a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp +++ b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp @@ -294,7 +294,7 @@ void ConnectToProvider(const char * ipAddress, uint32_t nodeId) PeerId peerID = fabric->GetPeerId(); peerID.SetNodeId(providerNodeId); - operationalDeviceProxy = new chip::OperationalDeviceProxy(initParams, peerID, nullptr); + operationalDeviceProxy = new chip::OperationalDeviceProxy(initParams, peerID); server->SetOperationalDeviceProxy(operationalDeviceProxy); // Explicitly calling UpdateDeviceData() should not be needed once OperationalDeviceProxy can resolve IP address from node diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 318c9cd8786bbc..1a83052fa6ab46 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -272,7 +272,7 @@ void SendQueryImageCommand(chip::NodeId peerNodeId = providerNodeId, chip::Fabri }; chip::OperationalDeviceProxy * operationalDeviceProxy = - chip::Platform::New(initParams, fabric->GetPeerIdForNode(peerNodeId), nullptr); + chip::Platform::New(initParams, fabric->GetPeerIdForNode(peerNodeId)); if (operationalDeviceProxy == nullptr) { ChipLogError(SoftwareUpdate, "Failed in creating an instance of OperationalDeviceProxy"); diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 7f964f2b7d96c3..05f54af5fdd414 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -25,30 +25,34 @@ CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::C { VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - Dnssd::ResolvedNodeData * nodeResolutionData = nullptr; - Dnssd::ResolvedNodeData cachedResolutionData; + Dnssd::ResolvedNodeData resolutionData; PeerId peerId = GetFabricInfo()->GetPeerIdForNode(nodeId); - if (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, cachedResolutionData) == CHIP_NO_ERROR) - { - nodeResolutionData = &cachedResolutionData; - } + bool nodeIDWasResolved = (mConfig.dnsCache != nullptr && mConfig.dnsCache->Lookup(peerId, resolutionData) == CHIP_NO_ERROR); OperationalDeviceProxy * session = FindExistingSession(nodeId); if (session == nullptr) { // TODO - Implement LRU to evict least recently used session to handle mActiveSessions pool exhaustion - session = mActiveSessions.CreateObject(mConfig.sessionInitParams, peerId, nodeResolutionData); + if (nodeIDWasResolved) + { + session = mActiveSessions.CreateObject(mConfig.sessionInitParams, peerId, resolutionData); + } + else + { + session = mActiveSessions.CreateObject(mConfig.sessionInitParams, peerId); + } + if (session == nullptr) { onFailure->mCall(onFailure->mContext, nodeId, CHIP_ERROR_NO_MEMORY); return CHIP_ERROR_NO_MEMORY; } } - else if (nodeResolutionData != nullptr) + else if (nodeIDWasResolved) { - session->OnNodeIdResolved(nodeResolutionData); + session->OnNodeIdResolved(resolutionData); } CHIP_ERROR err = session->Connect(onConnection, onFailure); diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index 9a4f850ce7760e..f89169557865d5 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -60,6 +60,9 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: /** * Find an existing session for the given node ID, or trigger a new session request. + * The caller can optionally provide `onConnection` and `onFailure` callback objects. If provided, + * these will be used to inform the caller about successful or failed connection establishment. + * If the connection is already established, the `onConnection` callback will be immediately called. */ CHIP_ERROR FindOrEstablishSession(NodeId nodeId, Callback::Callback * onConnection, Callback::Callback * onFailure); @@ -78,7 +81,20 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: return mConfig.sessionInitParams.fabricInfo; } + /** + * This API triggers the DNS-SD resolution for the given node ID. The node ID will be looked up + * on the fabric that was configured for the CASESessionManager object. + * + * The results of the DNS-SD resolution request is provided to the class via `ResolverDelegate` + * implementation of CASESessionManager. + */ CHIP_ERROR ResolveDeviceAddress(NodeId nodeId); + + /** + * This API returns the address and port for the given node ID. It is expected that there is + * an ongoing session with the peer node. If the session doesn't exist, the API will return + * `CHIP_ERROR_NOT_CONNECTED` error. + */ CHIP_ERROR GetDeviceAddressAndPort(NodeId nodeId, Inet::IPAddress & addr, uint16_t & port); //////////// ExchangeMgrDelegate Implementation /////////////// diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalDeviceProxy.cpp index 4aaddb4ce40df7..140beb734867c8 100644 --- a/src/app/OperationalDeviceProxy.cpp +++ b/src/app/OperationalDeviceProxy.cpp @@ -24,10 +24,11 @@ * messages to and from the corresponding CHIP devices. */ -#include +#include "OperationalDeviceProxy.h" + +#include "CommandSender.h" +#include "ReadPrepareParams.h" -#include -#include #include #include #include diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index c7c0e456312806..d2259d73a03ebd 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -73,7 +73,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta { public: virtual ~OperationalDeviceProxy(); - OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId, const Dnssd::ResolvedNodeData * nodeResolutionData) + OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) { VerifyOrReturn(params.Validate() == CHIP_NO_ERROR); @@ -81,6 +81,11 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta mPeerId = peerId; mState = State::NeedsAddress; + } + + OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId, const Dnssd::ResolvedNodeData & nodeResolutionData) : + OperationalDeviceProxy(params, peerId) + { OnNodeIdResolved(nodeResolutionData); } @@ -110,20 +115,16 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta */ void OnConnectionExpired(SessionHandle session) override; - void OnNodeIdResolved(const Dnssd::ResolvedNodeData * nodeResolutionData) + void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeResolutionData) { - if (nodeResolutionData != nullptr) - { - mDeviceAddress = ToPeerAddress(*nodeResolutionData); + mDeviceAddress = ToPeerAddress(nodeResolutionData); - mMrpIdleInterval = nodeResolutionData->GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL); - mMrpActiveInterval = - nodeResolutionData->GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); + mMrpIdleInterval = nodeResolutionData.GetMrpRetryIntervalIdle().ValueOr(CHIP_CONFIG_MRP_DEFAULT_IDLE_RETRY_INTERVAL); + mMrpActiveInterval = nodeResolutionData.GetMrpRetryIntervalActive().ValueOr(CHIP_CONFIG_MRP_DEFAULT_ACTIVE_RETRY_INTERVAL); - if (mState == State::NeedsAddress) - { - mState = State::Initialized; - } + if (mState == State::NeedsAddress) + { + mState = State::Initialized; } } From baa2a6f3f202c2704d5cbba98fd932bfd712fe0f Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 16 Nov 2021 10:58:03 -0800 Subject: [PATCH 7/7] address review comments --- src/app/CASESessionManager.cpp | 18 ++++++++--------- src/app/CASESessionManager.h | 26 +++++++++---------------- src/app/OperationalDeviceProxy.h | 2 ++ src/controller/CHIPDeviceController.cpp | 6 +++++- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 05f54af5fdd414..ff152e47e6a22a 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -23,8 +23,6 @@ namespace chip { CHIP_ERROR CASESessionManager::FindOrEstablishSession(NodeId nodeId, Callback::Callback * onConnection, Callback::Callback * onFailure) { - VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); - Dnssd::ResolvedNodeData resolutionData; PeerId peerId = GetFabricInfo()->GetPeerIdForNode(nodeId); @@ -71,14 +69,12 @@ void CASESessionManager::ReleaseSession(NodeId nodeId) CHIP_ERROR CASESessionManager::ResolveDeviceAddress(NodeId nodeId) { - VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); return Dnssd::Resolver::Instance().ResolveNodeId(GetFabricInfo()->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny); } void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) { ChipLogProgress(Controller, "Address resolved for node: 0x" ChipLogFormatX64, ChipLogValueX64(nodeData.mPeerId.GetNodeId())); - VerifyOrReturn(mInitialized, ChipLogError(Controller, "OnNodeIdResolved was called in uninitialized state")); if (mConfig.dnsCache != nullptr) { @@ -99,13 +95,19 @@ void CASESessionManager::OnNodeIdResolutionFailed(const PeerId & peer, CHIP_ERRO ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error)); } -CHIP_ERROR CASESessionManager::GetDeviceAddressAndPort(NodeId nodeId, Inet::IPAddress & addr, uint16_t & port) +CHIP_ERROR CASESessionManager::GetPeerAddress(NodeId nodeId, Transport::PeerAddress & addr) { - VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE); + if (mConfig.dnsCache != nullptr) + { + Dnssd::ResolvedNodeData resolutionData; + ReturnErrorOnFailure(mConfig.dnsCache->Lookup(GetFabricInfo()->GetPeerIdForNode(nodeId), resolutionData)); + addr = OperationalDeviceProxy::ToPeerAddress(resolutionData); + return CHIP_NO_ERROR; + } OperationalDeviceProxy * session = FindExistingSession(nodeId); VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED); - VerifyOrReturnError(session->GetAddress(addr, port), CHIP_ERROR_NOT_CONNECTED); + addr = session->GetPeerAddress(); return CHIP_NO_ERROR; } @@ -116,8 +118,6 @@ void CASESessionManager::OnNewConnection(SessionHandle sessionHandle, Messaging: void CASESessionManager::OnConnectionExpired(SessionHandle sessionHandle, Messaging::ExchangeManager * mgr) { - VerifyOrReturn(mInitialized, ChipLogError(Controller, "OnConnectionExpired was called in uninitialized state")); - OperationalDeviceProxy * session = FindSession(sessionHandle); VerifyOrReturn(session != nullptr, ChipLogDetail(Controller, "OnConnectionExpired was called for unknown device, ignoring it.")); diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index f89169557865d5..56fbdfca449d2a 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -46,18 +46,17 @@ struct CASESessionManagerConfig class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd::ResolverDelegate { public: + CASESessionManager() = delete; + CASESessionManager(CASESessionManagerConfig & params) { VerifyOrReturn(params.sessionInitParams.Validate() == CHIP_NO_ERROR); - mConfig = params; - mInitialized = true; + mConfig = params; } virtual ~CASESessionManager() {} - void Shutdown() { mInitialized = false; } - /** * Find an existing session for the given node ID, or trigger a new session request. * The caller can optionally provide `onConnection` and `onFailure` callback objects. If provided, @@ -71,15 +70,7 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: void ReleaseSession(NodeId nodeId); - FabricInfo * GetFabricInfo() - { - if (!mInitialized) - { - return nullptr; - } - - return mConfig.sessionInitParams.fabricInfo; - } + FabricInfo * GetFabricInfo() { return mConfig.sessionInitParams.fabricInfo; } /** * This API triggers the DNS-SD resolution for the given node ID. The node ID will be looked up @@ -91,11 +82,14 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: CHIP_ERROR ResolveDeviceAddress(NodeId nodeId); /** - * This API returns the address and port for the given node ID. It is expected that there is + * This API returns the address for the given node ID. + * If the CASESessionManager is configured with a DNS-SD cache, the cache is looked up + * for the node ID. + * If the DNS-SD cache is not available, the CASESessionManager looks up the list for * an ongoing session with the peer node. If the session doesn't exist, the API will return * `CHIP_ERROR_NOT_CONNECTED` error. */ - CHIP_ERROR GetDeviceAddressAndPort(NodeId nodeId, Inet::IPAddress & addr, uint16_t & port); + CHIP_ERROR GetPeerAddress(NodeId nodeId, Transport::PeerAddress & addr); //////////// ExchangeMgrDelegate Implementation /////////////// void OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr) override; @@ -113,8 +107,6 @@ class CASESessionManager : public Messaging::ExchangeMgrDelegate, public Dnssd:: BitMapObjectPool mActiveSessions; CASESessionManagerConfig mConfig; - - bool mInitialized = false; }; } // namespace chip diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalDeviceProxy.h index d2259d73a03ebd..a9cfa2725dcbd5 100644 --- a/src/app/OperationalDeviceProxy.h +++ b/src/app/OperationalDeviceProxy.h @@ -165,6 +165,8 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, public SessionEsta bool GetAddress(Inet::IPAddress & addr, uint16_t & port) const override; + Transport::PeerAddress GetPeerAddress() const { return mDeviceAddress; } + static Transport::PeerAddress ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData) { Inet::InterfaceId interfaceId = Inet::InterfaceId::Null(); diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index c7b2f76c39a577..4791b0bf10578f 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -342,7 +342,11 @@ void DeviceController::PersistNextKeyId() CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port) { VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE); - return mCASESessionManager->GetDeviceAddressAndPort(peerId.GetNodeId(), addr, port); + Transport::PeerAddress peerAddr; + ReturnErrorOnFailure(mCASESessionManager->GetPeerAddress(peerId.GetNodeId(), peerAddr)); + addr = peerAddr.GetIPAddress(); + port = peerAddr.GetPort(); + return CHIP_NO_ERROR; } void DeviceController::OnOpenPairingWindowSuccessResponse(void * context)