No longer provide OperationDeviceProxy in OnDeviceConnected callback (#21256)
Previous implementations of OnDeviceConnected held onto OperationalDeviceProxy when they really should not have could lead to use after free should something else free that OperationalDeviceProxy.
diff --git a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
index 53d62c1..10d7c32 100644
--- a/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
+++ b/src/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/ChipClient.kt
@@ -61,6 +61,10 @@
* Wrapper around [ChipDeviceController.getConnectedDevicePointer] to return the value directly.
*/
suspend fun getConnectedDevicePointer(context: Context, nodeId: Long): Long {
+ // TODO (#21539) This is a memory leak because we currently never call releaseConnectedDevicePointer
+ // once we are done with the returned device pointer. Memory leak was introduced since the refactor
+ // that introduced it was very large in order to fix a use after free, which was considered to be
+ // worse than the memory leak that was introduced.
return suspendCoroutine { continuation ->
getDeviceController(context).getConnectedDevicePointer(
nodeId,
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index 9f82625..9dd72df 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -165,9 +165,9 @@
"MessageDef/WriteRequestMessage.cpp",
"MessageDef/WriteResponseMessage.cpp",
"OTAUserConsentCommon.h",
- "OperationalDeviceProxy.cpp",
- "OperationalDeviceProxy.h",
- "OperationalDeviceProxyPool.h",
+ "OperationalSessionSetup.cpp",
+ "OperationalSessionSetup.h",
+ "OperationalSessionSetupPool.h",
"ReadClient.cpp",
"ReadHandler.cpp",
"RequiredPrivilege.cpp",
diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp
index 0f1d08f..7ffa85d 100644
--- a/src/app/CASESessionManager.cpp
+++ b/src/app/CASESessionManager.cpp
@@ -34,12 +34,12 @@
ChipLogDetail(CASESessionManager, "FindOrEstablishSession: PeerId = [%d:" ChipLogFormatX64 "]", peerId.GetFabricIndex(),
ChipLogValueX64(peerId.GetNodeId()));
- OperationalDeviceProxy * session = FindExistingSession(peerId);
+ OperationalSessionSetup * session = FindExistingSessionSetup(peerId);
if (session == nullptr)
{
- ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing OperationalDeviceProxy instance found");
+ ChipLogDetail(CASESessionManager, "FindOrEstablishSession: No existing OperationalSessionSetup instance found");
- session = mConfig.devicePool->Allocate(mConfig.sessionInitParams, peerId);
+ session = mConfig.sessionSetupPool->Allocate(mConfig.sessionInitParams, peerId, this);
if (session == nullptr)
{
@@ -52,51 +52,48 @@
}
session->Connect(onConnection, onFailure);
- if (!session->IsConnected() && !session->IsConnecting() && !session->IsResolvingAddress())
- {
- // This session is not making progress toward anything. It will have
- // notified the consumer about the failure already via the provided
- // callbacks, if any.
- //
- // Release the peer rather than the pointer in case the failure handler
- // has already released the session.
- ReleaseSession(peerId);
- }
}
void CASESessionManager::ReleaseSession(const ScopedNodeId & peerId)
{
- ReleaseSession(FindExistingSession(peerId));
+ ReleaseSession(FindExistingSessionSetup(peerId));
}
void CASESessionManager::ReleaseSessionsForFabric(FabricIndex fabricIndex)
{
- mConfig.devicePool->ReleaseDevicesForFabric(fabricIndex);
+ mConfig.sessionSetupPool->ReleaseAllSessionSetupsForFabric(fabricIndex);
}
void CASESessionManager::ReleaseAllSessions()
{
- mConfig.devicePool->ReleaseAllDevices();
+ mConfig.sessionSetupPool->ReleaseAllSessionSetup();
}
CHIP_ERROR CASESessionManager::GetPeerAddress(const ScopedNodeId & peerId, Transport::PeerAddress & addr)
{
- OperationalDeviceProxy * session = FindExistingSession(peerId);
- VerifyOrReturnError(session != nullptr, CHIP_ERROR_NOT_CONNECTED);
- addr = session->GetPeerAddress();
+ ReturnErrorOnFailure(mConfig.sessionInitParams.Validate());
+ auto optionalSessionHandle = FindExistingSession(peerId);
+ ReturnErrorCodeIf(!optionalSessionHandle.HasValue(), CHIP_ERROR_NOT_CONNECTED);
+ addr = optionalSessionHandle.Value()->AsSecureSession()->GetPeerAddress();
return CHIP_NO_ERROR;
}
-OperationalDeviceProxy * CASESessionManager::FindExistingSession(const ScopedNodeId & peerId) const
+OperationalSessionSetup * CASESessionManager::FindExistingSessionSetup(const ScopedNodeId & peerId) const
{
- return mConfig.devicePool->FindDevice(peerId);
+ return mConfig.sessionSetupPool->FindSessionSetup(peerId);
}
-void CASESessionManager::ReleaseSession(OperationalDeviceProxy * session) const
+Optional<SessionHandle> CASESessionManager::FindExistingSession(const ScopedNodeId & peerId) const
+{
+ return mConfig.sessionInitParams.sessionManager->FindSecureSessionForNode(peerId,
+ MakeOptional(Transport::SecureSession::Type::kCASE));
+}
+
+void CASESessionManager::ReleaseSession(OperationalSessionSetup * session) const
{
if (session != nullptr)
{
- mConfig.devicePool->Release(session);
+ mConfig.sessionSetupPool->Release(session);
}
}
diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h
index c74cb47..ddc68b3 100644
--- a/src/app/CASESessionManager.h
+++ b/src/app/CASESessionManager.h
@@ -19,8 +19,8 @@
#pragma once
#include <app/CASEClientPool.h>
-#include <app/OperationalDeviceProxy.h>
-#include <app/OperationalDeviceProxyPool.h>
+#include <app/OperationalSessionSetup.h>
+#include <app/OperationalSessionSetupPool.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPCore.h>
#include <lib/support/Pool.h>
@@ -34,7 +34,7 @@
struct CASESessionManagerConfig
{
DeviceProxyInitParams sessionInitParams;
- OperationalDeviceProxyPoolDelegate * devicePool = nullptr;
+ OperationalSessionSetupPoolDelegate * sessionSetupPool = nullptr;
};
/**
@@ -45,7 +45,7 @@
* 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is
* successful)
*/
-class CASESessionManager
+class CASESessionManager : public OperationalSessionReleaseDelegate
{
public:
CASESessionManager() = default;
@@ -71,9 +71,9 @@
void FindOrEstablishSession(const ScopedNodeId & peerId, Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure);
- OperationalDeviceProxy * FindExistingSession(const ScopedNodeId & peerId) const;
+ OperationalSessionSetup * FindExistingSessionSetup(const ScopedNodeId & peerId) const;
- void ReleaseSession(const ScopedNodeId & peerId);
+ void ReleaseSession(const ScopedNodeId & peerId) override;
void ReleaseSessionsForFabric(FabricIndex fabricIndex);
@@ -90,7 +90,9 @@
CHIP_ERROR GetPeerAddress(const ScopedNodeId & peerId, Transport::PeerAddress & addr);
private:
- void ReleaseSession(OperationalDeviceProxy * device) const;
+ Optional<SessionHandle> FindExistingSession(const ScopedNodeId & peerId) const;
+
+ void ReleaseSession(OperationalSessionSetup * device) const;
CASESessionManagerConfig mConfig;
};
diff --git a/src/app/OperationalDeviceProxyPool.h b/src/app/OperationalDeviceProxyPool.h
deleted file mode 100644
index 5413624..0000000
--- a/src/app/OperationalDeviceProxyPool.h
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- *
- * Copyright (c) 2021 Project CHIP Authors
- *
- * 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 <app/OperationalDeviceProxy.h>
-#include <lib/support/Pool.h>
-#include <transport/SessionHandle.h>
-
-namespace chip {
-
-class OperationalDeviceProxyPoolDelegate
-{
-public:
- virtual OperationalDeviceProxy * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId) = 0;
-
- virtual void Release(OperationalDeviceProxy * device) = 0;
-
- virtual OperationalDeviceProxy * FindDevice(ScopedNodeId peerId) = 0;
-
- virtual void ReleaseDevicesForFabric(FabricIndex fabricIndex) = 0;
-
- virtual void ReleaseAllDevices() = 0;
-
- virtual ~OperationalDeviceProxyPoolDelegate() {}
-};
-
-template <size_t N>
-class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate
-{
-public:
- ~OperationalDeviceProxyPool() override { mDevicePool.ReleaseAll(); }
-
- OperationalDeviceProxy * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId) override
- {
- return mDevicePool.CreateObject(params, peerId);
- }
-
- void Release(OperationalDeviceProxy * device) override { mDevicePool.ReleaseObject(device); }
-
- OperationalDeviceProxy * FindDevice(ScopedNodeId peerId) override
- {
- OperationalDeviceProxy * foundDevice = nullptr;
- mDevicePool.ForEachActiveObject([&](auto * activeDevice) {
- if (activeDevice->GetPeerId() == peerId)
- {
- foundDevice = activeDevice;
- return Loop::Break;
- }
- return Loop::Continue;
- });
-
- return foundDevice;
- }
-
- void ReleaseDevicesForFabric(FabricIndex fabricIndex) override
- {
- mDevicePool.ForEachActiveObject([&](auto * activeDevice) {
- if (activeDevice->GetFabricIndex() == fabricIndex)
- {
- Release(activeDevice);
- }
- return Loop::Continue;
- });
- }
-
- void ReleaseAllDevices() override
- {
- mDevicePool.ForEachActiveObject([&](auto * activeDevice) {
- Release(activeDevice);
- return Loop::Continue;
- });
- }
-
-private:
- ObjectPool<OperationalDeviceProxy, N> mDevicePool;
-};
-
-}; // namespace chip
diff --git a/src/app/OperationalDeviceProxy.cpp b/src/app/OperationalSessionSetup.cpp
similarity index 76%
rename from src/app/OperationalDeviceProxy.cpp
rename to src/app/OperationalSessionSetup.cpp
index 45386ca..ab58a58 100644
--- a/src/app/OperationalDeviceProxy.cpp
+++ b/src/app/OperationalSessionSetup.cpp
@@ -24,7 +24,7 @@
* messages to and from the corresponding CHIP devices.
*/
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/CASEClient.h>
#include <app/InteractionModelEngine.h>
@@ -45,11 +45,11 @@
namespace chip {
-void OperationalDeviceProxy::MoveToState(State aTargetState)
+void OperationalSessionSetup::MoveToState(State aTargetState)
{
if (mState != aTargetState)
{
- ChipLogDetail(Controller, "OperationalDeviceProxy[%u:" ChipLogFormatX64 "]: State change %d --> %d",
+ ChipLogDetail(Controller, "OperationalSessionSetup[%u:" ChipLogFormatX64 "]: State change %d --> %d",
mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()), to_underlying(mState),
to_underlying(aTargetState));
mState = aTargetState;
@@ -61,7 +61,7 @@
}
}
-bool OperationalDeviceProxy::AttachToExistingSecureSession()
+bool OperationalSessionSetup::AttachToExistingSecureSession()
{
VerifyOrReturnError(mState == State::NeedsAddress || mState == State::ResolvingAddress || mState == State::HasAddress, false);
@@ -80,8 +80,8 @@
return true;
}
-void OperationalDeviceProxy::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
- Callback::Callback<OnDeviceConnectionFailure> * onFailure)
+void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
+ Callback::Callback<OnDeviceConnectionFailure> * onFailure)
{
CHIP_ERROR err = CHIP_NO_ERROR;
bool isConnected = false;
@@ -154,10 +154,14 @@
if (err != CHIP_NO_ERROR || isConnected)
{
DequeueConnectionCallbacks(err);
+ // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
+ // While it is odd to have an explicit return here at the end of the function, we do so
+ // as a precaution in case someone later on adds something to the end of this function.
+ return;
}
}
-void OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config)
+void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config)
{
if (mState == State::Uninitialized)
{
@@ -168,7 +172,7 @@
char peerAddrBuff[Transport::PeerAddress::kMaxToStringSize];
addr.ToString(peerAddrBuff);
- ChipLogDetail(Discovery, "OperationalDeviceProxy[%u:" ChipLogFormatX64 "]: Updating device address to %s while in state %d",
+ ChipLogDetail(Discovery, "OperationalSessionSetup[%u:" ChipLogFormatX64 "]: Updating device address to %s while in state %d",
mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()), peerAddrBuff, static_cast<int>(mState));
#endif
@@ -191,6 +195,8 @@
if (err != CHIP_NO_ERROR)
{
DequeueConnectionCallbacks(err);
+ // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
+ return;
}
}
else
@@ -208,7 +214,7 @@
}
}
-CHIP_ERROR OperationalDeviceProxy::EstablishConnection()
+CHIP_ERROR OperationalSessionSetup::EstablishConnection()
{
mCASEClient = mInitParams.clientPool->Allocate(CASEClientInitParams{
mInitParams.sessionManager, mInitParams.sessionResumptionStorage, mInitParams.certificateValidityPolicy,
@@ -227,8 +233,8 @@
return CHIP_NO_ERROR;
}
-void OperationalDeviceProxy::EnqueueConnectionCallbacks(Callback::Callback<OnDeviceConnected> * onConnection,
- Callback::Callback<OnDeviceConnectionFailure> * onFailure)
+void OperationalSessionSetup::EnqueueConnectionCallbacks(Callback::Callback<OnDeviceConnected> * onConnection,
+ Callback::Callback<OnDeviceConnectionFailure> * onFailure)
{
if (onConnection != nullptr)
{
@@ -241,7 +247,7 @@
}
}
-void OperationalDeviceProxy::DequeueConnectionCallbacks(CHIP_ERROR error)
+void OperationalSessionSetup::DequeueConnectionCallbacks(CHIP_ERROR error)
{
Cancelable failureReady, successReady;
@@ -253,6 +259,14 @@
mConnectionFailure.DequeueAll(failureReady);
mConnectionSuccess.DequeueAll(successReady);
+ // TODO Issue #20452: For now we need to make a copy of member fields inside `this` before calling any of the
+ // callbacks since the callbacks themselves can potentially release `this` OperationalSessionSetup.
+ auto * exchangeMgr = mInitParams.exchangeMgr;
+ auto sessionHandle = mSecureSession.Get();
+ auto peerId = mPeerId;
+ VerifyOrDie(mReleaseDelegate != nullptr);
+ auto * releaseDelegate = mReleaseDelegate;
+
//
// If we encountered no error, go ahead and call all success callbacks. Otherwise,
// call the failure callbacks.
@@ -266,7 +280,7 @@
if (error != CHIP_NO_ERROR)
{
- cb->mCall(cb->mContext, mPeerId, error);
+ cb->mCall(cb->mContext, peerId, error);
}
}
@@ -277,12 +291,16 @@
cb->Cancel();
if (error == CHIP_NO_ERROR)
{
- cb->mCall(cb->mContext, this);
+ // We know that we for sure have the SessionHandle in the successful case.
+ VerifyOrDie(exchangeMgr);
+ cb->mCall(cb->mContext, *exchangeMgr, sessionHandle.Value());
}
}
+
+ releaseDelegate->ReleaseSession(peerId);
}
-void OperationalDeviceProxy::OnSessionEstablishmentError(CHIP_ERROR error)
+void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error)
{
VerifyOrReturn(mState != State::Uninitialized && mState != State::NeedsAddress,
ChipLogError(Controller, "HandleCASEConnectionFailure was called while the device was not initialized"));
@@ -295,11 +313,10 @@
MoveToState(State::HasAddress);
DequeueConnectionCallbacks(error);
-
- // Do not touch device instance anymore; it might have been destroyed by a failure callback.
+ // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
}
-void OperationalDeviceProxy::OnSessionEstablished(const SessionHandle & session)
+void OperationalSessionSetup::OnSessionEstablished(const SessionHandle & session)
{
VerifyOrReturn(mState != State::Uninitialized,
ChipLogError(Controller, "HandleCASEConnected was called while the device was not initialized"));
@@ -308,12 +325,12 @@
return; // Got an invalid session, do not change any state
MoveToState(State::SecureConnected);
- DequeueConnectionCallbacks(CHIP_NO_ERROR);
- // Do not touch this instance anymore; it might have been destroyed by a callback.
+ DequeueConnectionCallbacks(CHIP_NO_ERROR);
+ // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
}
-void OperationalDeviceProxy::Disconnect()
+void OperationalSessionSetup::Disconnect()
{
VerifyOrReturn(mState == State::SecureConnected);
@@ -333,7 +350,7 @@
MoveToState(State::HasAddress);
}
-void OperationalDeviceProxy::CleanupCASEClient()
+void OperationalSessionSetup::CleanupCASEClient()
{
if (mCASEClient)
{
@@ -342,32 +359,27 @@
}
}
-void OperationalDeviceProxy::OnSessionReleased()
+void OperationalSessionSetup::OnSessionReleased()
{
MoveToState(State::HasAddress);
}
-void OperationalDeviceProxy::OnFirstMessageDeliveryFailed()
+void OperationalSessionSetup::OnFirstMessageDeliveryFailed()
{
LookupPeerAddress();
}
-void OperationalDeviceProxy::OnSessionHang()
+void OperationalSessionSetup::OnSessionHang()
{
Disconnect();
}
-void OperationalDeviceProxy::ShutdownSubscriptions()
-{
- app::InteractionModelEngine::GetInstance()->ShutdownSubscriptions(mPeerId.GetFabricIndex(), GetDeviceId());
-}
-
-OperationalDeviceProxy::~OperationalDeviceProxy()
+OperationalSessionSetup::~OperationalSessionSetup()
{
if (mAddressLookupHandle.IsActive())
{
ChipLogDetail(Discovery,
- "OperationalDeviceProxy[%u:" ChipLogFormatX64
+ "OperationalSessionSetup[%u:" ChipLogFormatX64
"]: Cancelling incomplete address resolution as device is being deleted.",
mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()));
@@ -387,7 +399,7 @@
}
}
-CHIP_ERROR OperationalDeviceProxy::LookupPeerAddress()
+CHIP_ERROR OperationalSessionSetup::LookupPeerAddress()
{
// NOTE: This is public API that can be used to update our stored peer
// address even when we are in State::Connected, so we do not make any
@@ -395,7 +407,7 @@
if (mAddressLookupHandle.IsActive())
{
ChipLogProgress(Discovery,
- "OperationalDeviceProxy[%u:" ChipLogFormatX64
+ "OperationalSessionSetup[%u:" ChipLogFormatX64
"]: Operational node lookup already in progress. Will NOT start a new one.",
mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()));
return CHIP_NO_ERROR;
@@ -411,14 +423,14 @@
return Resolver::Instance().LookupNode(request, mAddressLookupHandle);
}
-void OperationalDeviceProxy::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
+void OperationalSessionSetup::OnNodeAddressResolved(const PeerId & peerId, const ResolveResult & result)
{
UpdateDeviceData(result.address, result.mrpRemoteConfig);
}
-void OperationalDeviceProxy::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason)
+void OperationalSessionSetup::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP_ERROR reason)
{
- ChipLogError(Discovery, "OperationalDeviceProxy[%u:" ChipLogFormatX64 "]: operational discovery failed: %" CHIP_ERROR_FORMAT,
+ ChipLogError(Discovery, "OperationalSessionSetup[%u:" ChipLogFormatX64 "]: operational discovery failed: %" CHIP_ERROR_FORMAT,
mPeerId.GetFabricIndex(), ChipLogValueX64(mPeerId.GetNodeId()), reason.Format());
if (IsResolvingAddress())
@@ -427,6 +439,7 @@
}
DequeueConnectionCallbacks(reason);
+ // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
}
} // namespace chip
diff --git a/src/app/OperationalDeviceProxy.h b/src/app/OperationalSessionSetup.h
similarity index 69%
rename from src/app/OperationalDeviceProxy.h
rename to src/app/OperationalSessionSetup.h
index f83bb7a..a821d04 100644
--- a/src/app/OperationalDeviceProxy.h
+++ b/src/app/OperationalSessionSetup.h
@@ -70,9 +70,79 @@
}
};
-class OperationalDeviceProxy;
+/**
+ * @brief Delegate provided when creating OperationalSessionSetup.
+ *
+ * Once OperationalSessionSetup establishes a connection (or errors out) and has notified all
+ * registered application callbacks via OnDeviceConnected/OnDeviceConnectionFailure, this delegate
+ * is used to deallocate the OperationalSessionSetup.
+ */
+class OperationalSessionReleaseDelegate
+{
+public:
+ virtual ~OperationalSessionReleaseDelegate() = default;
+ // TODO Issue #20452: Once cleanup from #20452 takes place we can provide OperationalSessionSetup *
+ // instead of ScopedNodeId here.
+ virtual void ReleaseSession(const ScopedNodeId & peerId) = 0;
+};
-typedef void (*OnDeviceConnected)(void * context, OperationalDeviceProxy * device);
+/**
+ * @brief Minimal implementation of DeviceProxy that encapsulates a SessionHolder to track a CASE session.
+ *
+ * Deprecated - Avoid using this object.
+ *
+ * OperationalDeviceProxy is a minimal implementation of DeviceProxy. It is meant to provide a transition
+ * for existing consumers of OperationalDeviceProxy that were delivered a reference to that object in
+ * their respective OnDeviceConnected callback, but were incorrectly holding onto that object past
+ * the function call. OperationalDeviceProxy can be held on for as long as is desired, while still
+ * minimizing the code changes needed to transition to a more final solution by virtue of
+ * implementing DeviceProxy.
+ */
+class OperationalDeviceProxy : public DeviceProxy
+{
+public:
+ OperationalDeviceProxy(Messaging::ExchangeManager * exchangeMgr, const SessionHandle & sessionHandle) :
+ mExchangeMgr(exchangeMgr), mSecureSession(sessionHandle), mPeerScopedNodeId(sessionHandle->GetPeer())
+ {}
+ OperationalDeviceProxy() {}
+
+ // Recommended to use InteractionModelEngine::ShutdownSubscriptions directly.
+ void ShutdownSubscriptions() override { VerifyOrDie(false); } // Currently not implemented.
+ void Disconnect() override
+ {
+ if (IsSecureConnected())
+ {
+ GetSecureSession().Value()->AsSecureSession()->MarkAsDefunct();
+ }
+ mSecureSession.Release();
+ mExchangeMgr = nullptr;
+ mPeerScopedNodeId = ScopedNodeId();
+ }
+ Messaging::ExchangeManager * GetExchangeManager() const override { return mExchangeMgr; }
+ chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.Get(); }
+ NodeId GetDeviceId() const override { return mPeerScopedNodeId.GetNodeId(); }
+ ScopedNodeId GetPeerScopedNodeId() const { return mPeerScopedNodeId; }
+
+ bool ConnectionReady() const { return (mExchangeMgr != nullptr && IsSecureConnected()); }
+
+private:
+ bool IsSecureConnected() const override { return static_cast<bool>(mSecureSession); }
+
+ Messaging::ExchangeManager * mExchangeMgr = nullptr;
+ SessionHolder mSecureSession;
+ ScopedNodeId mPeerScopedNodeId;
+};
+
+/**
+ * @brief Callback prototype when secure session is established.
+ *
+ * Callback implementations are not supposed to store the exchangeMgr or the sessionHandle. Older
+ * application code does incorrectly hold onto this information so do not follow those incorrect
+ * implementations as an example.
+ */
+// TODO: OnDeviceConnected should not return ExchangeManager. Application should have this already. This
+// was provided initially to keep code churn down during a large refactor of OnDeviceConnected.
+typedef void (*OnDeviceConnected)(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
typedef void (*OnDeviceConnectionFailure)(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
/**
@@ -84,27 +154,29 @@
* - Establish a secure channel to it via CASE
* - Expose to consumers the secure session for talking to the device.
*/
-class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
- public SessionDelegate,
- public SessionEstablishmentDelegate,
- public AddressResolve::NodeListener
+class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
+ public SessionEstablishmentDelegate,
+ public AddressResolve::NodeListener
{
public:
- ~OperationalDeviceProxy() override;
+ ~OperationalSessionSetup() override;
- OperationalDeviceProxy(DeviceProxyInitParams & params, ScopedNodeId peerId) : mSecureSession(*this)
+ OperationalSessionSetup(DeviceProxyInitParams & params, ScopedNodeId peerId,
+ OperationalSessionReleaseDelegate * releaseDelegate) :
+ mSecureSession(*this)
{
mInitParams = params;
- if (params.Validate() != CHIP_NO_ERROR)
+ if (params.Validate() != CHIP_NO_ERROR || releaseDelegate == nullptr)
{
mState = State::Uninitialized;
return;
}
- mSystemLayer = params.exchangeMgr->GetSessionManager()->SystemLayer();
- mPeerId = peerId;
- mFabricTable = params.fabricTable;
- mState = State::NeedsAddress;
+ mSystemLayer = params.exchangeMgr->GetSessionManager()->SystemLayer();
+ mPeerId = peerId;
+ mFabricTable = params.fabricTable;
+ mReleaseDelegate = releaseDelegate;
+ mState = State::NeedsAddress;
mAddressLookupHandle.SetListener(this);
}
@@ -156,18 +228,10 @@
/**
* Mark any open session with the device as expired.
*/
- void Disconnect() override;
-
- NodeId GetDeviceId() const override { return mPeerId.GetNodeId(); }
+ void Disconnect();
ScopedNodeId GetPeerId() const { return mPeerId; }
- void ShutdownSubscriptions() override;
-
- Messaging::ExchangeManager * GetExchangeManager() const override { return mInitParams.exchangeMgr; }
-
- chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.Get(); }
-
Transport::PeerAddress GetPeerAddress() const { return mDeviceAddress; }
static Transport::PeerAddress ToPeerAddress(const Dnssd::ResolvedNodeData & nodeData)
@@ -204,7 +268,7 @@
private:
enum class State
{
- Uninitialized, // Error state: OperationalDeviceProxy is useless
+ Uninitialized, // Error state: OperationalSessionSetup is useless
NeedsAddress, // No address known, lookup not started yet.
ResolvingAddress, // Address lookup in progress.
HasAddress, // Have an address, CASE handshake not started yet.
@@ -233,9 +297,13 @@
Callback::CallbackDeque mConnectionSuccess;
Callback::CallbackDeque mConnectionFailure;
+ OperationalSessionReleaseDelegate * mReleaseDelegate;
+
/// This is used when a node address is required.
chip::AddressResolve::NodeLookupHandle mAddressLookupHandle;
+ ReliableMessageProtocolConfig mRemoteMRPConfig = GetDefaultMRPConfig();
+
CHIP_ERROR EstablishConnection();
/*
@@ -247,8 +315,6 @@
*/
bool AttachToExistingSecureSession();
- bool IsSecureConnected() const override { return mState == State::SecureConnected; }
-
void CleanupCASEClient();
void EnqueueConnectionCallbacks(Callback::Callback<OnDeviceConnected> * onConnection,
diff --git a/src/app/OperationalSessionSetupPool.h b/src/app/OperationalSessionSetupPool.h
new file mode 100644
index 0000000..bd80276
--- /dev/null
+++ b/src/app/OperationalSessionSetupPool.h
@@ -0,0 +1,96 @@
+/*
+ *
+ * Copyright (c) 2021 Project CHIP Authors
+ *
+ * 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 <app/CASESessionManager.h>
+#include <app/OperationalSessionSetup.h>
+#include <lib/support/Pool.h>
+#include <transport/SessionHandle.h>
+
+namespace chip {
+
+class OperationalSessionSetupPoolDelegate
+{
+public:
+ virtual OperationalSessionSetup * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId,
+ OperationalSessionReleaseDelegate * releaseDelegate) = 0;
+
+ virtual void Release(OperationalSessionSetup * device) = 0;
+
+ virtual OperationalSessionSetup * FindSessionSetup(ScopedNodeId peerId) = 0;
+
+ virtual void ReleaseAllSessionSetupsForFabric(FabricIndex fabricIndex) = 0;
+
+ virtual void ReleaseAllSessionSetup() = 0;
+
+ virtual ~OperationalSessionSetupPoolDelegate() {}
+};
+
+template <size_t N>
+class OperationalSessionSetupPool : public OperationalSessionSetupPoolDelegate
+{
+public:
+ ~OperationalSessionSetupPool() override { mSessionSetupPool.ReleaseAll(); }
+
+ OperationalSessionSetup * Allocate(DeviceProxyInitParams & params, ScopedNodeId peerId,
+ OperationalSessionReleaseDelegate * releaseDelegate) override
+ {
+ return mSessionSetupPool.CreateObject(params, peerId, releaseDelegate);
+ }
+
+ void Release(OperationalSessionSetup * device) override { mSessionSetupPool.ReleaseObject(device); }
+
+ OperationalSessionSetup * FindSessionSetup(ScopedNodeId peerId) override
+ {
+ OperationalSessionSetup * foundDevice = nullptr;
+ mSessionSetupPool.ForEachActiveObject([&](auto * activeSetup) {
+ if (activeSetup->GetPeerId() == peerId)
+ {
+ foundDevice = activeSetup;
+ return Loop::Break;
+ }
+ return Loop::Continue;
+ });
+
+ return foundDevice;
+ }
+
+ void ReleaseAllSessionSetupsForFabric(FabricIndex fabricIndex) override
+ {
+ mSessionSetupPool.ForEachActiveObject([&](auto * activeSetup) {
+ if (activeSetup->GetFabricIndex() == fabricIndex)
+ {
+ Release(activeSetup);
+ }
+ return Loop::Continue;
+ });
+ }
+
+ void ReleaseAllSessionSetup() override
+ {
+ mSessionSetupPool.ForEachActiveObject([&](auto * activeSetup) {
+ Release(activeSetup);
+ return Loop::Continue;
+ });
+ }
+
+private:
+ ObjectPool<OperationalSessionSetup, N> mSessionSetupPool;
+};
+
+}; // namespace chip
diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
index be5b735..90d4bf7 100644
--- a/src/app/ReadClient.cpp
+++ b/src/app/ReadClient.cpp
@@ -991,13 +991,13 @@
return CHIP_NO_ERROR;
}
-void ReadClient::HandleDeviceConnected(void * context, OperationalDeviceProxy * device)
+void ReadClient::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
ReadClient * const _this = static_cast<ReadClient *>(context);
VerifyOrDie(_this != nullptr);
- ChipLogProgress(DataManagement, "HandleDeviceConnected %d\n", device->GetSecureSession().HasValue());
- _this->mReadPrepareParams.mSessionHolder.Grab(device->GetSecureSession().Value());
+ ChipLogProgress(DataManagement, "HandleDeviceConnected");
+ _this->mReadPrepareParams.mSessionHolder.Grab(sessionHandle);
auto err = _this->SendSubscribeRequest(_this->mReadPrepareParams);
if (err != CHIP_NO_ERROR)
@@ -1044,17 +1044,6 @@
_this->mReadPrepareParams.mSessionHolder->AsSecureSession()->MarkAsDefunct();
}
- //
- // TODO: Until #19259 is merged, we cannot actually just get by with the above logic since marking sessions
- // defunct has no effect on resident OperationalDeviceProxy instances that are already bound
- // to a now-defunct CASE session.
- //
- auto proxy = caseSessionManager->FindExistingSession(_this->mPeer);
- if (proxy != nullptr)
- {
- proxy->Disconnect();
- }
-
caseSessionManager->FindOrEstablishSession(_this->mPeer, &_this->mOnConnectedCallback,
&_this->mOnConnectionFailureCallback);
return;
diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h
index f5c5162..dba2e7b 100644
--- a/src/app/ReadClient.h
+++ b/src/app/ReadClient.h
@@ -33,7 +33,7 @@
#include <app/MessageDef/StatusResponseMessage.h>
#include <app/MessageDef/SubscribeRequestMessage.h>
#include <app/MessageDef/SubscribeResponseMessage.h>
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/ReadPrepareParams.h>
#include <app/data-model/Decode.h>
#include <lib/core/CHIPCallback.h>
@@ -488,7 +488,7 @@
void StopResubscription();
void ClearActiveSubscriptionState();
- static void HandleDeviceConnected(void * context, OperationalDeviceProxy * device);
+ static void HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
CHIP_ERROR GetMinEventNumber(const ReadPrepareParams & aReadPrepareParams, Optional<EventNumber> & aEventMin);
diff --git a/src/app/app-platform/ContentAppPlatform.cpp b/src/app/app-platform/ContentAppPlatform.cpp
index 5557c09..de8e53f 100644
--- a/src/app/app-platform/ContentAppPlatform.cpp
+++ b/src/app/app-platform/ContentAppPlatform.cpp
@@ -406,11 +406,11 @@
// constexpr ClusterId kClusterIdApplicationLauncher = 0x050c;
// constexpr ClusterId kClusterIdAccountLogin = 0x050e;
-CHIP_ERROR ContentAppPlatform::ManageClientAccess(OperationalDeviceProxy * targetDeviceProxy, uint16_t targetVendorId,
- NodeId localNodeId, Controller::WriteResponseSuccessCallback successCb,
+CHIP_ERROR ContentAppPlatform::ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle,
+ uint16_t targetVendorId, NodeId localNodeId,
+ Controller::WriteResponseSuccessCallback successCb,
Controller::WriteResponseFailureCallback failureCb)
{
- VerifyOrReturnError(targetDeviceProxy != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(successCb != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(failureCb != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
@@ -419,9 +419,9 @@
Access::AccessControl::Entry entry;
ReturnErrorOnFailure(GetAccessControl().PrepareEntry(entry));
ReturnErrorOnFailure(entry.SetAuthMode(Access::AuthMode::kCase));
- entry.SetFabricIndex(targetDeviceProxy->GetFabricIndex());
+ entry.SetFabricIndex(sessionHandle->GetFabricIndex());
ReturnErrorOnFailure(entry.SetPrivilege(vendorPrivilege));
- ReturnErrorOnFailure(entry.AddSubject(nullptr, targetDeviceProxy->GetDeviceId()));
+ ReturnErrorOnFailure(entry.AddSubject(nullptr, sessionHandle->GetPeer().GetNodeId()));
std::vector<Binding::Structs::TargetStruct::Type> bindings;
@@ -522,13 +522,12 @@
}
// TODO: add a subject description on the ACL
- ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, targetDeviceProxy->GetFabricIndex(), nullptr, entry));
+ ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, sessionHandle->GetFabricIndex(), nullptr, entry));
ChipLogProgress(Controller, "Attempting to update Binding list");
BindingListType bindingList(bindings.data(), bindings.size());
- chip::Controller::BindingCluster cluster(*targetDeviceProxy->GetExchangeManager(),
- targetDeviceProxy->GetSecureSession().Value(), kTargetBindingClusterEndpointId);
+ chip::Controller::BindingCluster cluster(exchangeMgr, sessionHandle, kTargetBindingClusterEndpointId);
ReturnErrorOnFailure(
cluster.WriteAttribute<Binding::Attributes::Binding::TypeInfo>(bindingList, nullptr, successCb, failureCb));
diff --git a/src/app/app-platform/ContentAppPlatform.h b/src/app/app-platform/ContentAppPlatform.h
index 4246914..cac2441 100644
--- a/src/app/app-platform/ContentAppPlatform.h
+++ b/src/app/app-platform/ContentAppPlatform.h
@@ -23,7 +23,7 @@
#pragma once
#include <app-common/zap-generated/enums.h>
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/app-platform/ContentApp.h>
#include <app/util/attribute-storage.h>
#include <controller/CHIPCluster.h>
@@ -130,16 +130,17 @@
* Add ACLs on this device for the given client,
* and create bindings on the given client so that it knows what it has access to.
*
- * @param[in] targetDeviceProxy OperationalDeviceProxy for the target device.
- * @param[in] targetVendorId Vendor ID for the target device.
- * @param[in] localNodeId The NodeId for the local device.
- * @param[in] successCb The function to be called on success of adding the binding.
- * @param[in] failureCb The function to be called on failure of adding the binding.
+ * @param[in] exchangeMgr Exchange manager to be used to get an exchange context.
+ * @param[in] sessionHandle Reference to an established session.
+ * @param[in] targetVendorId Vendor ID for the target device.
+ * @param[in] localNodeId The NodeId for the local device.
+ * @param[in] successCb The function to be called on success of adding the binding.
+ * @param[in] failureCb The function to be called on failure of adding the binding.
*
* @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error
*/
- CHIP_ERROR ManageClientAccess(OperationalDeviceProxy * targetDeviceProxy, uint16_t targetVendorId, NodeId localNodeId,
- Controller::WriteResponseSuccessCallback successCb,
+ CHIP_ERROR ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle, uint16_t targetVendorId,
+ NodeId localNodeId, Controller::WriteResponseSuccessCallback successCb,
Controller::WriteResponseFailureCallback failureCb);
protected:
diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
index 73bcf93..d373e8c 100644
--- a/src/app/clusters/bindings/BindingManager.cpp
+++ b/src/app/clusters/bindings/BindingManager.cpp
@@ -121,13 +121,13 @@
return mLastSessionEstablishmentError;
}
-void BindingManager::HandleDeviceConnected(void * context, OperationalDeviceProxy * device)
+void BindingManager::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
BindingManager * manager = static_cast<BindingManager *>(context);
- manager->HandleDeviceConnected(device);
+ manager->HandleDeviceConnected(exchangeMgr, sessionHandle);
}
-void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device)
+void BindingManager::HandleDeviceConnected(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
FabricIndex fabricToRemove = kUndefinedFabricIndex;
NodeId nodeToRemove = kUndefinedNodeId;
@@ -138,11 +138,12 @@
{
EmberBindingTableEntry entry = BindingTable::GetInstance().GetAt(pendingNotification.mBindingEntryId);
- if (device->GetPeerId() == ScopedNodeId(entry.nodeId, entry.fabricIndex))
+ if (sessionHandle->GetPeer() == ScopedNodeId(entry.nodeId, entry.fabricIndex))
{
fabricToRemove = entry.fabricIndex;
nodeToRemove = entry.nodeId;
- mBoundDeviceChangedHandler(entry, device, pendingNotification.mContext->GetContext());
+ OperationalDeviceProxy device(&exchangeMgr, sessionHandle);
+ mBoundDeviceChangedHandler(entry, &device, pendingNotification.mContext->GetContext());
}
}
@@ -189,19 +190,9 @@
{
if (iter->type == EMBER_UNICAST_BINDING)
{
- OperationalDeviceProxy * peerDevice =
- mInitParams.mCASESessionManager->FindExistingSession(ScopedNodeId(iter->nodeId, iter->fabricIndex));
- if (peerDevice != nullptr && peerDevice->IsConnected())
- {
- // We already have an active connection
- mBoundDeviceChangedHandler(*iter, peerDevice, bindingContext->GetContext());
- }
- else
- {
- mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), bindingContext);
- error = EstablishConnection(ScopedNodeId(iter->nodeId, iter->fabricIndex));
- SuccessOrExit(error == CHIP_NO_ERROR);
- }
+ mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), bindingContext);
+ error = EstablishConnection(ScopedNodeId(iter->nodeId, iter->fabricIndex));
+ SuccessOrExit(error == CHIP_NO_ERROR);
}
else if (iter->type == EMBER_MULTICAST_BINDING)
{
diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h
index 308b672..578ad4a 100644
--- a/src/app/clusters/bindings/BindingManager.h
+++ b/src/app/clusters/bindings/BindingManager.h
@@ -37,8 +37,10 @@
*
* E.g. The application will send on/off commands to peer for the OnOff cluster.
*
+ * The handler is not allowed to hold onto the pointer to the SessionHandler that is passed in.
*/
-using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & binding, DeviceProxy * peer_device, void * context);
+using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & binding, OperationalDeviceProxy * peer_device,
+ void * context);
/**
* Application callback function when a context used in NotifyBoundClusterChanged will not be needed and should be
@@ -123,8 +125,8 @@
private:
static BindingManager sBindingManager;
- static void HandleDeviceConnected(void * context, OperationalDeviceProxy * device);
- void HandleDeviceConnected(OperationalDeviceProxy * device);
+ static void HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
+ void HandleDeviceConnected(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
void HandleDeviceConnectionFailure(const ScopedNodeId & peerId, CHIP_ERROR error);
diff --git a/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp b/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
index 1aecdb3..98f11b3 100644
--- a/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
+++ b/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
@@ -387,8 +387,7 @@
}
auto providerNodeId = GetProviderScopedId();
- mCASESessionManager->FindExistingSession(providerNodeId)->Disconnect();
- mCASESessionManager->ReleaseSession(providerNodeId);
+ mServer->GetSecureSessionManager().MarkSessionsAsDefunct(providerNodeId, MakeOptional(Transport::SecureSession::Type::kCASE));
}
// Requestor is directed to cancel image update in progress. All the Requestor state is
@@ -416,16 +415,15 @@
}
// Called whenever FindOrEstablishSession is successful
-void DefaultOTARequestor::OnConnected(void * context, OperationalDeviceProxy * deviceProxy)
+void DefaultOTARequestor::OnConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
DefaultOTARequestor * requestorCore = static_cast<DefaultOTARequestor *>(context);
VerifyOrDie(requestorCore != nullptr);
- VerifyOrDie(deviceProxy != nullptr);
switch (requestorCore->mOnConnectedAction)
{
case kQueryImage: {
- CHIP_ERROR err = requestorCore->SendQueryImageRequest(*deviceProxy);
+ CHIP_ERROR err = requestorCore->SendQueryImageRequest(exchangeMgr, sessionHandle);
if (err != CHIP_NO_ERROR)
{
@@ -436,7 +434,7 @@
break;
}
case kDownload: {
- CHIP_ERROR err = requestorCore->StartDownload(*deviceProxy);
+ CHIP_ERROR err = requestorCore->StartDownload(exchangeMgr, sessionHandle);
if (err != CHIP_NO_ERROR)
{
@@ -447,7 +445,7 @@
break;
}
case kApplyUpdate: {
- CHIP_ERROR err = requestorCore->SendApplyUpdateRequest(*deviceProxy);
+ CHIP_ERROR err = requestorCore->SendApplyUpdateRequest(exchangeMgr, sessionHandle);
if (err != CHIP_NO_ERROR)
{
@@ -458,7 +456,7 @@
break;
}
case kNotifyUpdateApplied: {
- CHIP_ERROR err = requestorCore->SendNotifyUpdateAppliedRequest(*deviceProxy);
+ CHIP_ERROR err = requestorCore->SendNotifyUpdateAppliedRequest(exchangeMgr, sessionHandle);
if (err != CHIP_NO_ERROR)
{
@@ -724,7 +722,7 @@
return CHIP_NO_ERROR;
}
-CHIP_ERROR DefaultOTARequestor::SendQueryImageRequest(OperationalDeviceProxy & deviceProxy)
+CHIP_ERROR DefaultOTARequestor::SendQueryImageRequest(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
@@ -760,8 +758,7 @@
args.location.SetValue(CharSpan("XX", strlen("XX")));
}
- Controller::OtaSoftwareUpdateProviderCluster cluster(*deviceProxy.GetExchangeManager(), deviceProxy.GetSecureSession().Value(),
- mProviderLocation.Value().endpoint);
+ Controller::OtaSoftwareUpdateProviderCluster cluster(exchangeMgr, sessionHandle, mProviderLocation.Value().endpoint);
return cluster.InvokeCommand(args, this, OnQueryImageResponse, OnQueryImageFailure);
}
@@ -792,7 +789,7 @@
return CHIP_NO_ERROR;
}
-CHIP_ERROR DefaultOTARequestor::StartDownload(OperationalDeviceProxy & deviceProxy)
+CHIP_ERROR DefaultOTARequestor::StartDownload(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
VerifyOrReturnError(mBdxDownloader != nullptr, CHIP_ERROR_INCORRECT_STATE);
@@ -804,13 +801,7 @@
initOptions.FileDesLength = static_cast<uint16_t>(mFileDesignator.size());
initOptions.FileDesignator = reinterpret_cast<const uint8_t *>(mFileDesignator.data());
- chip::Messaging::ExchangeManager * exchangeMgr = deviceProxy.GetExchangeManager();
- VerifyOrReturnError(exchangeMgr != nullptr, CHIP_ERROR_INCORRECT_STATE);
-
- Optional<SessionHandle> session = deviceProxy.GetSecureSession();
- VerifyOrReturnError(session.HasValue(), CHIP_ERROR_INCORRECT_STATE);
-
- chip::Messaging::ExchangeContext * exchangeCtx = exchangeMgr->NewContext(session.Value(), &mBdxMessenger);
+ chip::Messaging::ExchangeContext * exchangeCtx = exchangeMgr.NewContext(sessionHandle, &mBdxMessenger);
VerifyOrReturnError(exchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY);
mBdxMessenger.Init(mBdxDownloader, exchangeCtx);
@@ -831,7 +822,7 @@
return err;
}
-CHIP_ERROR DefaultOTARequestor::SendApplyUpdateRequest(OperationalDeviceProxy & deviceProxy)
+CHIP_ERROR DefaultOTARequestor::SendApplyUpdateRequest(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(GenerateUpdateToken());
@@ -840,13 +831,13 @@
args.updateToken = mUpdateToken;
args.newVersion = mTargetVersion;
- Controller::OtaSoftwareUpdateProviderCluster cluster(*deviceProxy.GetExchangeManager(), deviceProxy.GetSecureSession().Value(),
- mProviderLocation.Value().endpoint);
+ Controller::OtaSoftwareUpdateProviderCluster cluster(exchangeMgr, sessionHandle, mProviderLocation.Value().endpoint);
return cluster.InvokeCommand(args, this, OnApplyUpdateResponse, OnApplyUpdateFailure);
}
-CHIP_ERROR DefaultOTARequestor::SendNotifyUpdateAppliedRequest(OperationalDeviceProxy & deviceProxy)
+CHIP_ERROR DefaultOTARequestor::SendNotifyUpdateAppliedRequest(Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
VerifyOrReturnError(mProviderLocation.HasValue(), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(GenerateUpdateToken());
@@ -855,8 +846,7 @@
args.updateToken = mUpdateToken;
args.softwareVersion = mCurrentVersion;
- Controller::OtaSoftwareUpdateProviderCluster cluster(*deviceProxy.GetExchangeManager(), deviceProxy.GetSecureSession().Value(),
- mProviderLocation.Value().endpoint);
+ Controller::OtaSoftwareUpdateProviderCluster cluster(exchangeMgr, sessionHandle, mProviderLocation.Value().endpoint);
// There is no response for a notify so consider this OTA complete. Clear the provider location and reset any states to indicate
// so.
diff --git a/src/app/clusters/ota-requestor/DefaultOTARequestor.h b/src/app/clusters/ota-requestor/DefaultOTARequestor.h
index f80f755..0dc04ef 100644
--- a/src/app/clusters/ota-requestor/DefaultOTARequestor.h
+++ b/src/app/clusters/ota-requestor/DefaultOTARequestor.h
@@ -207,7 +207,6 @@
ScopedNodeId GetProviderScopedId() const
{
- VerifyOrDie(mProviderLocation.HasValue());
return ScopedNodeId(mProviderLocation.Value().providerNodeID, mProviderLocation.Value().fabricIndex);
}
@@ -229,7 +228,7 @@
/**
* Send QueryImage request using values matching Basic cluster
*/
- CHIP_ERROR SendQueryImageRequest(OperationalDeviceProxy & deviceProxy);
+ CHIP_ERROR SendQueryImageRequest(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
/**
* Validate and extract mandatory information from QueryImageResponse
@@ -260,17 +259,17 @@
/**
* Start download of the software image returned in QueryImageResponse
*/
- CHIP_ERROR StartDownload(OperationalDeviceProxy & deviceProxy);
+ CHIP_ERROR StartDownload(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
/**
* Send ApplyUpdate request using values obtained from QueryImageResponse
*/
- CHIP_ERROR SendApplyUpdateRequest(OperationalDeviceProxy & deviceProxy);
+ CHIP_ERROR SendApplyUpdateRequest(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
/**
* Send NotifyUpdateApplied request
*/
- CHIP_ERROR SendNotifyUpdateAppliedRequest(OperationalDeviceProxy & deviceProxy);
+ CHIP_ERROR SendNotifyUpdateAppliedRequest(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
/**
* Store current update information to KVS
@@ -285,7 +284,7 @@
/**
* Session connection callbacks
*/
- static void OnConnected(void * context, OperationalDeviceProxy * deviceProxy);
+ static void OnConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
Callback::Callback<OnDeviceConnected> mOnConnectedCallback;
Callback::Callback<OnDeviceConnectionFailure> mOnConnectionFailureCallback;
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
index 15674b1..bc8f943 100644
--- a/src/app/server/Server.cpp
+++ b/src/app/server/Server.cpp
@@ -294,7 +294,7 @@
.groupDataProvider = mGroupsProvider,
.mrpLocalConfig = GetLocalMRPConfig(),
},
- .devicePool = &mDevicePool,
+ .sessionSetupPool = &mSessionSetupPool,
};
err = mCASESessionManager.Init(&DeviceLayer::SystemLayer(), caseSessionManagerConfig);
diff --git a/src/app/server/Server.h b/src/app/server/Server.h
index af4796d..7e1419e 100644
--- a/src/app/server/Server.h
+++ b/src/app/server/Server.h
@@ -25,7 +25,7 @@
#include <app/CASESessionManager.h>
#include <app/DefaultAttributePersistenceProvider.h>
#include <app/FailSafeContext.h>
-#include <app/OperationalDeviceProxyPool.h>
+#include <app/OperationalSessionSetupPool.h>
#include <app/TestEventTriggerDelegate.h>
#include <app/server/AclStorage.h>
#include <app/server/AppDelegate.h>
@@ -491,7 +491,7 @@
CASESessionManager mCASESessionManager;
CASEClientPool<CHIP_CONFIG_DEVICE_MAX_ACTIVE_CASE_CLIENTS> mCASEClientPool;
- OperationalDeviceProxyPool<CHIP_CONFIG_DEVICE_MAX_ACTIVE_DEVICES> mDevicePool;
+ OperationalSessionSetupPool<CHIP_CONFIG_DEVICE_MAX_ACTIVE_DEVICES> mSessionSetupPool;
Protocols::SecureChannel::UnsolicitedStatusHandler mUnsolicitedStatusHandler;
Messaging::ExchangeManager mExchangeMgr;
diff --git a/src/app/tests/suites/certification/Test_TC_CADMIN_1_15.yaml b/src/app/tests/suites/certification/Test_TC_CADMIN_1_15.yaml
index 8933123..3c6e8d6 100644
--- a/src/app/tests/suites/certification/Test_TC_CADMIN_1_15.yaml
+++ b/src/app/tests/suites/certification/Test_TC_CADMIN_1_15.yaml
@@ -170,13 +170,13 @@
./chip-tool basic write node-label te5new 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
./chip-tool basic read node-label 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
disabled: true
diff --git a/src/app/tests/suites/certification/Test_TC_CADMIN_1_16.yaml b/src/app/tests/suites/certification/Test_TC_CADMIN_1_16.yaml
index 53669ad..de1771e 100644
--- a/src/app/tests/suites/certification/Test_TC_CADMIN_1_16.yaml
+++ b/src/app/tests/suites/certification/Test_TC_CADMIN_1_16.yaml
@@ -208,13 +208,13 @@
./chip-tool basic write node-label te5new 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
./chip-tool basic read node-label 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
disabled: true
diff --git a/src/app/tests/suites/certification/Test_TC_CADMIN_1_17.yaml b/src/app/tests/suites/certification/Test_TC_CADMIN_1_17.yaml
index ec94c4c..15f161f 100644
--- a/src/app/tests/suites/certification/Test_TC_CADMIN_1_17.yaml
+++ b/src/app/tests/suites/certification/Test_TC_CADMIN_1_17.yaml
@@ -203,13 +203,13 @@
./chip-tool basic write node-label te5new 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
./chip-tool basic read node-label 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
disabled: true
diff --git a/src/app/tests/suites/certification/Test_TC_CADMIN_1_18.yaml b/src/app/tests/suites/certification/Test_TC_CADMIN_1_18.yaml
index 70df68e..f27a2d4 100644
--- a/src/app/tests/suites/certification/Test_TC_CADMIN_1_18.yaml
+++ b/src/app/tests/suites/certification/Test_TC_CADMIN_1_18.yaml
@@ -215,13 +215,13 @@
./chip-tool basic write node-label te5new 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
./chip-tool basic read node-label 2 0
Received error (protocol code 2) during pairing process. ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter
- [1651819620.929567][4359:4364] CHIP:CTL: OperationalDeviceProxy[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
+ [1651819620.929567][4359:4364] CHIP:CTL: OperationalSessionSetup[B8070CD13C99D367:0000000000000002]: State change 3 --> 2
[1651819620.929700][4359:4364] CHIP:-: ../../third_party/connectedhomeip/src/protocols/secure_channel/CASESession.cpp:1551: CHIP Error 0x00000054: Invalid CASE parameter at ../../commands/clusters/ModelCommand.cpp:53
disabled: true
diff --git a/src/app/tests/suites/certification/Test_TC_DD_3_1.yaml b/src/app/tests/suites/certification/Test_TC_DD_3_1.yaml
index 7d21c82..7e8d68c 100644
--- a/src/app/tests/suites/certification/Test_TC_DD_3_1.yaml
+++ b/src/app/tests/suites/certification/Test_TC_DD_3_1.yaml
@@ -378,8 +378,8 @@
CHIP:CTL: Commissioning stage next step: "SendNOC" -> "FindOperational"
[1653471976.344532][30157:30162] CHIP:CTL: Performing next commissioning step "FindOperational"
[1653471976.344586][30157:30162] CHIP:CSM: FindOrEstablishSession: PeerId = CCCB8A2597E4538B:0000000000000001
- [1653471976.344642][30157:30162] CHIP:CSM: FindOrEstablishSession: No existing OperationalDeviceProxy instance found
- [1653471976.344715][30157:30162] CHIP:CTL: OperationalDeviceProxy[CCCB8A2597E4538B:0000000000000001]: State change 1 --> 2
+ [1653471976.344642][30157:30162] CHIP:CSM: FindOrEstablishSession: No existing OperationalSessionSetup instance found
+ [1653471976.344715][30157:30162] CHIP:CTL: OperationalSessionSetup[CCCB8A2597E4538B:0000000000000001]: State change 1 --> 2
[1653471976.344783][30157:30162] CHIP:DIS: Resolving CCCB8A2597E4538B:0000000000000001 ...
[1653471976.346864][30157:30162] CHIP:DIS: Operational node lookup already in progress. Will NOT start a new one.
[1653471976.347000][30157:30162] CHIP:DMG: ICR moving to [AwaitingDe]
diff --git a/src/app/tests/suites/certification/Test_TC_DD_3_5.yaml b/src/app/tests/suites/certification/Test_TC_DD_3_5.yaml
index 92c0210..3b325e8 100644
--- a/src/app/tests/suites/certification/Test_TC_DD_3_5.yaml
+++ b/src/app/tests/suites/certification/Test_TC_DD_3_5.yaml
@@ -275,8 +275,8 @@
Commissioning stage next step: "WiFiNetworkEnable" -> "FindOperational"
[1653471976.344532][30157:30162] CHIP:CTL: Performing next commissioning step "FindOperational"
[1653471976.344586][30157:30162] CHIP:CSM: FindOrEstablishSession: PeerId = CCCB8A2597E4538B:0000000000000001
- [1653471976.344642][30157:30162] CHIP:CSM: FindOrEstablishSession: No existing OperationalDeviceProxy instance found
- [1653471976.344715][30157:30162] CHIP:CTL: OperationalDeviceProxy[CCCB8A2597E4538B:0000000000000001]: State change 1 --> 2
+ [1653471976.344642][30157:30162] CHIP:CSM: FindOrEstablishSession: No existing OperationalSessionSetup instance found
+ [1653471976.344715][30157:30162] CHIP:CTL: OperationalSessionSetup[CCCB8A2597E4538B:0000000000000001]: State change 1 --> 2
[1653471976.344783][30157:30162] CHIP:DIS: Resolving CCCB8A2597E4538B:0000000000000001 ...
[1653471976.346864][30157:30162] CHIP:DIS: Operational node lookup already in progress. Will NOT start a new one.
[1653471976.347000][30157:30162] CHIP:DMG: ICR moving to [AwaitingDe]
diff --git a/src/app/tests/suites/certification/Test_TC_DD_3_7.yaml b/src/app/tests/suites/certification/Test_TC_DD_3_7.yaml
index cbf22f8..68eaf6b 100644
--- a/src/app/tests/suites/certification/Test_TC_DD_3_7.yaml
+++ b/src/app/tests/suites/certification/Test_TC_DD_3_7.yaml
@@ -419,7 +419,7 @@
CHIP:CTL: Performing next commissioning step "FindOperational"
CHIP:CSM: FindOrEstablishSession: PeerId = BFCBED670D527591:000000000001B669
- CHIP:CSM: FindOrEstablishSession: No existing OperationalDeviceProxy instance found
+ CHIP:CSM: FindOrEstablishSession: No existing OperationalSessionSetup instance found
disabled: true
- label: "Reboot TH"
@@ -446,7 +446,7 @@
CHIP:CTL: Performing next commissioning step "FindOperational"
CHIP:CSM: FindOrEstablishSession: PeerId = BFCBED670D527591:000000000001B669
- CHIP:CSM: FindOrEstablishSession: No existing OperationalDeviceProxy instance found
+ CHIP:CSM: FindOrEstablishSession: No existing OperationalSessionSetup instance found
disabled: true
- label: "Commissioner starts discovery of TH using Operational Discovery"
diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp
index 6a3b064..62cf5ff 100644
--- a/src/controller/AutoCommissioner.cpp
+++ b/src/controller/AutoCommissioner.cpp
@@ -513,7 +513,7 @@
ReleasePAI();
ReleaseDAC();
mCommissioneeDeviceProxy = nullptr;
- mOperationalDeviceProxy = nullptr;
+ mOperationalDeviceProxy = OperationalDeviceProxy();
mDeviceCommissioningInfo = ReadCommissioningInfo();
return CHIP_NO_ERROR;
default:
@@ -553,9 +553,9 @@
DeviceProxy * AutoCommissioner::GetDeviceProxyForStep(CommissioningStage nextStage)
{
if (nextStage == CommissioningStage::kSendComplete ||
- (nextStage == CommissioningStage::kCleanup && mOperationalDeviceProxy != nullptr))
+ (nextStage == CommissioningStage::kCleanup && mOperationalDeviceProxy.ConnectionReady()))
{
- return mOperationalDeviceProxy;
+ return &mOperationalDeviceProxy;
}
return mCommissioneeDeviceProxy;
}
diff --git a/src/controller/AutoCommissioner.h b/src/controller/AutoCommissioner.h
index 422b6ef..25932e2 100644
--- a/src/controller/AutoCommissioner.h
+++ b/src/controller/AutoCommissioner.h
@@ -84,9 +84,9 @@
DeviceCommissioner * mCommissioner = nullptr;
CommissioneeDeviceProxy * mCommissioneeDeviceProxy = nullptr;
- OperationalDeviceProxy * mOperationalDeviceProxy = nullptr;
OperationalCredentialsDelegate * mOperationalCredentialsDelegate = nullptr;
CommissioningParameters mParams = CommissioningParameters();
+ OperationalDeviceProxy mOperationalDeviceProxy;
// Memory space for the commisisoning parameters that come in as ByteSpans - the caller is not guaranteed to retain this memory
uint8_t mSsid[CommissioningParameters::kMaxSsidLen];
uint8_t mCredentials[CommissioningParameters::kMaxCredentialsLen];
diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
index 11ea5dc..da29dae 100644
--- a/src/controller/CHIPDeviceController.cpp
+++ b/src/controller/CHIPDeviceController.cpp
@@ -43,7 +43,7 @@
#include <app/server/Dnssd.h>
#include <app/InteractionModelEngine.h>
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/util/error-mapping.h>
#include <credentials/CHIPCert.h>
#include <credentials/DeviceAttestationCredsProvider.h>
@@ -364,16 +364,15 @@
{
ChipLogProgress(Controller, "Force close session for node 0x%" PRIx64, nodeId);
- OperationalDeviceProxy * proxy = mSystemState->CASESessionMgr()->FindExistingSession(GetPeerScopedId(nodeId));
- if (proxy == nullptr)
+ if (SessionMgr()->MarkSessionsAsDefunct(GetPeerScopedId(nodeId), MakeOptional(Transport::SecureSession::Type::kCASE)))
{
- ChipLogProgress(Controller, "Attempted to close a session that does not exist.");
return CHIP_NO_ERROR;
}
- if (proxy->IsConnected())
+ OperationalSessionSetup * proxy = mSystemState->CASESessionMgr()->FindExistingSessionSetup(GetPeerScopedId(nodeId));
+ if (proxy == nullptr)
{
- proxy->Disconnect();
+ ChipLogProgress(Controller, "Attempted to close a session that does not exist.");
return CHIP_NO_ERROR;
}
@@ -1615,7 +1614,8 @@
}
}
-void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device)
+void DeviceCommissioner::OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
// CASE session established.
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
@@ -1629,7 +1629,7 @@
}
if (commissioner->mDeviceBeingCommissioned == nullptr ||
- commissioner->mDeviceBeingCommissioned->GetDeviceId() != device->GetDeviceId())
+ commissioner->mDeviceBeingCommissioned->GetDeviceId() != sessionHandle->GetPeer().GetNodeId())
{
// Not the device we are trying to commission.
return;
@@ -1638,7 +1638,7 @@
if (commissioner->mCommissioningDelegate != nullptr)
{
CommissioningDelegate::CommissioningReport report;
- report.Set<OperationalNodeFoundData>(OperationalNodeFoundData(device));
+ report.Set<OperationalNodeFoundData>(OperationalNodeFoundData(OperationalDeviceProxy(&exchangeMgr, sessionHandle)));
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
}
}
@@ -2316,23 +2316,23 @@
{
VerifyOrReturnError(mState == State::Initialized && mFabricIndex != kUndefinedFabricIndex, CHIP_ERROR_INCORRECT_STATE);
- OperationalDeviceProxy * proxy = GetDeviceSession(GetPeerScopedId(peerNodeId));
- VerifyOrReturnError(proxy != nullptr, CHIP_ERROR_NOT_FOUND);
+ OperationalSessionSetup * sessionSetup = GetDeviceSession(GetPeerScopedId(peerNodeId));
+ VerifyOrReturnError(sessionSetup != nullptr, CHIP_ERROR_NOT_FOUND);
- return proxy->LookupPeerAddress();
+ return sessionSetup->LookupPeerAddress();
}
-OperationalDeviceProxy * DeviceController::GetDeviceSession(const ScopedNodeId & peerId)
+OperationalSessionSetup * DeviceController::GetDeviceSession(const ScopedNodeId & peerId)
{
- return mSystemState->CASESessionMgr()->FindExistingSession(peerId);
+ return mSystemState->CASESessionMgr()->FindExistingSessionSetup(peerId);
}
-OperationalDeviceProxy * DeviceCommissioner::GetDeviceSession(const ScopedNodeId & peerId)
+OperationalSessionSetup * DeviceCommissioner::GetDeviceSession(const ScopedNodeId & peerId)
{
mSystemState->CASESessionMgr()->FindOrEstablishSession(peerId, &mOnDeviceConnectedCallback,
&mOnDeviceConnectionFailureCallback);
- // If there is an OperationalDeviceProxy for this peerId now the call to the
+ // If there is an OperationalSessionSetup for this peerId now the call to the
// superclass will return it.
return DeviceController::GetDeviceSession(peerId);
}
diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h
index 8f04fe7..2f97348 100644
--- a/src/controller/CHIPDeviceController.h
+++ b/src/controller/CHIPDeviceController.h
@@ -31,8 +31,8 @@
#include <app/CASEClientPool.h>
#include <app/CASESessionManager.h>
#include <app/ClusterStateCache.h>
-#include <app/OperationalDeviceProxy.h>
-#include <app/OperationalDeviceProxyPool.h>
+#include <app/OperationalSessionSetup.h>
+#include <app/OperationalSessionSetupPool.h>
#include <controller/AbstractDnssdDiscoveryController.h>
#include <controller/AutoCommissioner.h>
#include <controller/CHIPCluster.h>
@@ -371,12 +371,12 @@
/// Fetches the session to use for the current device. Allows overriding
/// in case subclasses want to create the session if it does not yet exist
- virtual OperationalDeviceProxy * GetDeviceSession(const ScopedNodeId & peerId);
+ virtual OperationalSessionSetup * GetDeviceSession(const ScopedNodeId & peerId);
DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mCommissionableNodes); }
private:
- void ReleaseOperationalDevice(OperationalDeviceProxy * device);
+ void ReleaseOperationalDevice(OperationalSessionSetup * device);
};
#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
@@ -661,7 +661,7 @@
void OnDone(app::ReadClient *) override;
// Commissioner will establish new device connections after PASE.
- OperationalDeviceProxy * GetDeviceSession(const ScopedNodeId & peerId) override;
+ OperationalSessionSetup * GetDeviceSession(const ScopedNodeId & peerId) override;
// Issue an NOC chain using the associated OperationalCredentialsDelegate. The NOC chain will
// be provided in X509 DER format.
@@ -760,7 +760,7 @@
/* Callback called when adding root cert to device results in failure */
static void OnRootCertFailureResponse(void * context, CHIP_ERROR error);
- static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device);
+ static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
static void OnDeviceAttestationInformationVerification(void * context, Credentials::AttestationVerificationResult result);
diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
index 0b9b4ec..1c06e82 100644
--- a/src/controller/CHIPDeviceControllerFactory.cpp
+++ b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -24,7 +24,7 @@
#include <controller/CHIPDeviceControllerFactory.h>
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/util/DataModelHandler.h>
#include <lib/support/ErrorStr.h>
#include <messaging/ReliableMessageProtocolConfig.h>
@@ -242,8 +242,8 @@
chip::app::DnssdServer::Instance().StartServer();
}
- stateParams.operationalDevicePool = Platform::New<DeviceControllerSystemStateParams::OperationalDevicePool>();
- stateParams.caseClientPool = Platform::New<DeviceControllerSystemStateParams::CASEClientPool>();
+ stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();
+ stateParams.caseClientPool = Platform::New<DeviceControllerSystemStateParams::CASEClientPool>();
DeviceProxyInitParams deviceInitParams = {
.sessionManager = stateParams.sessionMgr,
@@ -257,7 +257,7 @@
CASESessionManagerConfig sessionManagerConfig = {
.sessionInitParams = deviceInitParams,
- .devicePool = stateParams.operationalDevicePool,
+ .sessionSetupPool = stateParams.sessionSetupPool,
};
// TODO: Need to be able to create a CASESessionManagerConfig here!
@@ -390,13 +390,13 @@
mCASESessionManager = nullptr;
}
- // mCASEClientPool and mDevicePool must be deallocated
+ // mCASEClientPool and mSessionSetupPool must be deallocated
// after mCASESessionManager, which uses them.
- if (mOperationalDevicePool != nullptr)
+ if (mSessionSetupPool != nullptr)
{
- Platform::Delete(mOperationalDevicePool);
- mOperationalDevicePool = nullptr;
+ Platform::Delete(mSessionSetupPool);
+ mSessionSetupPool = nullptr;
}
if (mCASEClientPool != nullptr)
diff --git a/src/controller/CHIPDeviceControllerSystemState.h b/src/controller/CHIPDeviceControllerSystemState.h
index 9bebb56..8cf8306 100644
--- a/src/controller/CHIPDeviceControllerSystemState.h
+++ b/src/controller/CHIPDeviceControllerSystemState.h
@@ -69,8 +69,8 @@
struct DeviceControllerSystemStateParams
{
- using OperationalDevicePool = OperationalDeviceProxyPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES>;
- using CASEClientPool = chip::CASEClientPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_CASE_CLIENTS>;
+ using SessionSetupPool = OperationalSessionSetupPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES>;
+ using CASEClientPool = chip::CASEClientPool<CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_CASE_CLIENTS>;
// Params that can outlive the DeviceControllerSystemState
System::Layer * systemLayer = nullptr;
@@ -93,7 +93,7 @@
secure_channel::MessageCounterManager * messageCounterManager = nullptr;
CASEServer * caseServer = nullptr;
CASESessionManager * caseSessionManager = nullptr;
- OperationalDevicePool * operationalDevicePool = nullptr;
+ SessionSetupPool * sessionSetupPool = nullptr;
CASEClientPool * caseClientPool = nullptr;
FabricTable::Delegate * fabricTableDelegate = nullptr;
};
@@ -107,8 +107,8 @@
// owned by DeviceControllerFactory.
class DeviceControllerSystemState
{
- using OperationalDevicePool = DeviceControllerSystemStateParams::OperationalDevicePool;
- using CASEClientPool = DeviceControllerSystemStateParams::CASEClientPool;
+ using SessionSetupPool = DeviceControllerSystemStateParams::SessionSetupPool;
+ using CASEClientPool = DeviceControllerSystemStateParams::CASEClientPool;
public:
~DeviceControllerSystemState()
@@ -124,7 +124,7 @@
mUDPEndPointManager(params.udpEndPointManager), mTransportMgr(params.transportMgr), mSessionMgr(params.sessionMgr),
mUnsolicitedStatusHandler(params.unsolicitedStatusHandler), mExchangeMgr(params.exchangeMgr),
mMessageCounterManager(params.messageCounterManager), mFabrics(params.fabricTable), mCASEServer(params.caseServer),
- mCASESessionManager(params.caseSessionManager), mOperationalDevicePool(params.operationalDevicePool),
+ mCASESessionManager(params.caseSessionManager), mSessionSetupPool(params.sessionSetupPool),
mCASEClientPool(params.caseClientPool), mGroupDataProvider(params.groupDataProvider),
mFabricTableDelegate(params.fabricTableDelegate), mSessionResumptionStorage(std::move(params.sessionResumptionStorage))
{
@@ -164,8 +164,8 @@
{
return mSystemLayer != nullptr && mUDPEndPointManager != nullptr && mTransportMgr != nullptr && mSessionMgr != nullptr &&
mUnsolicitedStatusHandler != nullptr && mExchangeMgr != nullptr && mMessageCounterManager != nullptr &&
- mFabrics != nullptr && mCASESessionManager != nullptr && mOperationalDevicePool != nullptr &&
- mCASEClientPool != nullptr && mGroupDataProvider != nullptr;
+ mFabrics != nullptr && mCASESessionManager != nullptr && mSessionSetupPool != nullptr && mCASEClientPool != nullptr &&
+ mGroupDataProvider != nullptr;
};
System::Layer * SystemLayer() const { return mSystemLayer; };
@@ -200,7 +200,7 @@
FabricTable * mFabrics = nullptr;
CASEServer * mCASEServer = nullptr;
CASESessionManager * mCASESessionManager = nullptr;
- OperationalDevicePool * mOperationalDevicePool = nullptr;
+ SessionSetupPool * mSessionSetupPool = nullptr;
CASEClientPool * mCASEClientPool = nullptr;
Credentials::GroupDataProvider * mGroupDataProvider = nullptr;
FabricTable::Delegate * mFabricTableDelegate = nullptr;
diff --git a/src/controller/CommissionerDiscoveryController.cpp b/src/controller/CommissionerDiscoveryController.cpp
index 50ce140..0177e8a 100644
--- a/src/controller/CommissionerDiscoveryController.cpp
+++ b/src/controller/CommissionerDiscoveryController.cpp
@@ -178,7 +178,8 @@
}
void CommissionerDiscoveryController::CommissioningSucceeded(uint16_t vendorId, uint16_t productId, NodeId nodeId,
- OperationalDeviceProxy * device)
+ Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
mVendorId = vendorId;
mProductId = productId;
@@ -186,7 +187,7 @@
if (mPostCommissioningListener != nullptr)
{
ChipLogDetail(Controller, "CommissionerDiscoveryController calling listener");
- mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, device);
+ mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, exchangeMgr, sessionHandle);
}
else
{
diff --git a/src/controller/CommissionerDiscoveryController.h b/src/controller/CommissionerDiscoveryController.h
index 143c164..8d0c36a 100644
--- a/src/controller/CommissionerDiscoveryController.h
+++ b/src/controller/CommissionerDiscoveryController.h
@@ -28,7 +28,7 @@
#pragma once
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPError.h>
#include <lib/core/NodeId.h>
@@ -39,7 +39,7 @@
#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
using chip::NodeId;
-using chip::OperationalDeviceProxy;
+using chip::OperationalSessionSetup;
using chip::Protocols::UserDirectedCommissioning::UDCClientState;
using chip::Protocols::UserDirectedCommissioning::UserConfirmationProvider;
using chip::Protocols::UserDirectedCommissioning::UserDirectedCommissioningServer;
@@ -133,10 +133,12 @@
* @param[in] vendorId The vendorid from the DAC of the new node.
* @param[in] productId The productid from the DAC of the new node.
* @param[in] nodeId The node id for the newly commissioned node.
- * @param[in] device The device proxy for use in cluster communication.
+ * @param[in] exchangeMgr The exchange manager to be used to get an exchange context.
+ * @param[in] sessionHandle A reference to an established session.
*
*/
- virtual void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, OperationalDeviceProxy * device) = 0;
+ virtual void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId,
+ chip::Messaging::ExchangeManager & exchangeMgr, chip::SessionHandle & sessionHandle) = 0;
virtual ~PostCommissioningListener() = default;
};
@@ -210,10 +212,12 @@
* @param[in] vendorId The vendorid from the DAC of the new node.
* @param[in] productId The productid from the DAC of the new node.
* @param[in] nodeId The node id for the newly commissioned node.
- * @param[in] device The device proxy for use in cluster communication.
+ * @param[in] exchangeMgr The exchange manager to be used to get an exchange context.
+ * @param[in] sessionHandle A reference to an established session.
*
*/
- void CommissioningSucceeded(uint16_t vendorId, uint16_t productId, NodeId nodeId, OperationalDeviceProxy * device);
+ void CommissioningSucceeded(uint16_t vendorId, uint16_t productId, NodeId nodeId,
+ chip::Messaging::ExchangeManager & exchangeMgr, chip::SessionHandle & sessionHandle);
/**
* This method should be called by the commissioner to indicate that commissioning failed.
diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h
index 22d651e..595978b 100644
--- a/src/controller/CommissioningDelegate.h
+++ b/src/controller/CommissioningDelegate.h
@@ -17,7 +17,7 @@
*/
#pragma once
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <controller/CommissioneeDeviceProxy.h>
#include <credentials/attestation_verifier/DeviceAttestationDelegate.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
@@ -444,8 +444,8 @@
struct OperationalNodeFoundData
{
- OperationalNodeFoundData(OperationalDeviceProxy * proxy) : operationalProxy(proxy) {}
- OperationalDeviceProxy * operationalProxy;
+ OperationalNodeFoundData(OperationalDeviceProxy proxy) : operationalProxy(proxy) {}
+ OperationalDeviceProxy operationalProxy;
};
struct NetworkClusterInfo
diff --git a/src/controller/CommissioningWindowOpener.cpp b/src/controller/CommissioningWindowOpener.cpp
index 95eb4d7..9afe3d1 100644
--- a/src/controller/CommissioningWindowOpener.cpp
+++ b/src/controller/CommissioningWindowOpener.cpp
@@ -124,14 +124,14 @@
return mController->GetConnectedDevice(mNodeId, &mDeviceConnected, &mDeviceConnectionFailure);
}
-CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(OperationalDeviceProxy * device)
+CHIP_ERROR CommissioningWindowOpener::OpenCommissioningWindowInternal(Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
ChipLogProgress(Controller, "OpenCommissioningWindow for device ID %" PRIu64, mNodeId);
constexpr EndpointId kAdministratorCommissioningClusterEndpoint = 0;
- AdministratorCommissioningCluster cluster(*device->GetExchangeManager(), device->GetSecureSession().Value(),
- kAdministratorCommissioningClusterEndpoint);
+ AdministratorCommissioningCluster cluster(exchangeMgr, sessionHandle, kAdministratorCommissioningClusterEndpoint);
if (mCommissioningWindowOption != CommissioningWindowOption::kOriginalSetupCode)
{
@@ -254,7 +254,8 @@
}
}
-void CommissioningWindowOpener::OnDeviceConnectedCallback(void * context, OperationalDeviceProxy * device)
+void CommissioningWindowOpener::OnDeviceConnectedCallback(void * context, Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
auto * self = static_cast<CommissioningWindowOpener *>(context);
@@ -267,7 +268,7 @@
{
case Step::kReadVID: {
constexpr EndpointId kBasicClusterEndpoint = 0;
- BasicCluster cluster(*device->GetExchangeManager(), device->GetSecureSession().Value(), kBasicClusterEndpoint);
+ BasicCluster cluster(exchangeMgr, sessionHandle, kBasicClusterEndpoint);
err = cluster.ReadAttribute<app::Clusters::Basic::Attributes::VendorID::TypeInfo>(context, OnVIDReadResponse,
OnVIDPIDReadFailureResponse);
#if CHIP_ERROR_LOGGING
@@ -277,7 +278,7 @@
}
case Step::kReadPID: {
constexpr EndpointId kBasicClusterEndpoint = 0;
- BasicCluster cluster(*device->GetExchangeManager(), device->GetSecureSession().Value(), kBasicClusterEndpoint);
+ BasicCluster cluster(exchangeMgr, sessionHandle, kBasicClusterEndpoint);
err = cluster.ReadAttribute<app::Clusters::Basic::Attributes::ProductID::TypeInfo>(context, OnPIDReadResponse,
OnVIDPIDReadFailureResponse);
#if CHIP_ERROR_LOGGING
@@ -286,7 +287,7 @@
break;
}
case Step::kOpenCommissioningWindow: {
- err = self->OpenCommissioningWindowInternal(device);
+ err = self->OpenCommissioningWindowInternal(exchangeMgr, sessionHandle);
#if CHIP_ERROR_LOGGING
messageIfError = "Could not connect to open commissioning window";
#endif // CHIP_ERROR_LOGGING
diff --git a/src/controller/CommissioningWindowOpener.h b/src/controller/CommissioningWindowOpener.h
index 63a0731..4e31466 100644
--- a/src/controller/CommissioningWindowOpener.h
+++ b/src/controller/CommissioningWindowOpener.h
@@ -17,7 +17,7 @@
#pragma once
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
#include <app/data-model/NullObject.h>
#include <controller/CHIPDeviceController.h>
#include <crypto/CHIPCryptoPAL.h>
@@ -120,13 +120,13 @@
kOpenCommissioningWindow,
};
- CHIP_ERROR OpenCommissioningWindowInternal(OperationalDeviceProxy * device);
+ CHIP_ERROR OpenCommissioningWindowInternal(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnPIDReadResponse(void * context, uint16_t value);
static void OnVIDReadResponse(void * context, VendorId value);
static void OnVIDPIDReadFailureResponse(void * context, CHIP_ERROR error);
static void OnOpenCommissioningWindowSuccess(void * context, const app::DataModel::NullObjectType &);
static void OnOpenCommissioningWindowFailure(void * context, CHIP_ERROR error);
- static void OnDeviceConnectedCallback(void * context, OperationalDeviceProxy * device);
+ static void OnDeviceConnectedCallback(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnDeviceConnectionFailureCallback(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
DeviceController * const mController = nullptr;
diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp
index 1966bd5..d873257 100644
--- a/src/controller/java/AndroidCallbacks.cpp
+++ b/src/controller/java/AndroidCallbacks.cpp
@@ -57,7 +57,8 @@
env->DeleteGlobalRef(mJavaCallbackRef);
}
-void GetConnectedDeviceCallback::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device)
+void GetConnectedDeviceCallback::OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr,
+ SessionHandle & sessionHandle)
{
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
auto * self = static_cast<GetConnectedDeviceCallback *>(context);
@@ -78,6 +79,8 @@
VerifyOrReturn(successMethod != nullptr, ChipLogError(Controller, "Could not find onDeviceConnected method"));
static_assert(sizeof(jlong) >= sizeof(void *), "Need to store a pointer in a Java handle");
+
+ OperationalDeviceProxy * device = new OperationalDeviceProxy(&exchangeMgr, sessionHandle);
DeviceLayer::StackUnlock unlock;
env->CallVoidMethod(javaCallback, successMethod, reinterpret_cast<jlong>(device));
}
diff --git a/src/controller/java/AndroidCallbacks.h b/src/controller/java/AndroidCallbacks.h
index 9ba9736..64232cd 100644
--- a/src/controller/java/AndroidCallbacks.h
+++ b/src/controller/java/AndroidCallbacks.h
@@ -33,7 +33,7 @@
GetConnectedDeviceCallback(jobject wrapperCallback, jobject javaCallback);
~GetConnectedDeviceCallback();
- static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device);
+ static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
static void OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
Callback::Callback<OnDeviceConnected> mOnSuccess;
diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp
index 070c993..420a17e 100644
--- a/src/controller/java/CHIPDeviceController-JNI.cpp
+++ b/src/controller/java/CHIPDeviceController-JNI.cpp
@@ -671,6 +671,16 @@
VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(Controller, "Error invoking GetConnectedDevice"));
}
+JNI_METHOD(void, releaseConnectedDevicePointer)(JNIEnv * env, jobject self, jlong devicePtr)
+{
+ chip::DeviceLayer::StackLock lock;
+ OperationalDeviceProxy * device = reinterpret_cast<OperationalDeviceProxy *>(devicePtr);
+ if (device != NULL)
+ {
+ delete device;
+ }
+}
+
JNI_METHOD(void, disconnectDevice)(JNIEnv * env, jobject self, jlong handle, jlong deviceId)
{
chip::DeviceLayer::StackLock lock;
diff --git a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java
index e617cb3..85809ad 100644
--- a/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java
+++ b/src/controller/java/src/chip/devicecontroller/ChipDeviceController.java
@@ -648,6 +648,8 @@
private native void getConnectedDevicePointer(
long deviceControllerPtr, long deviceId, long callbackHandle);
+ private native void releaseConnectedDevicePointer(long devicePtr);
+
private native boolean disconnectDevice(long deviceControllerPtr, long deviceId);
private native void deleteDeviceController(long deviceControllerPtr);
diff --git a/src/controller/python/ChipDeviceController-ScriptBinding.cpp b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
index 96a4a51..194004e 100644
--- a/src/controller/python/ChipDeviceController-ScriptBinding.cpp
+++ b/src/controller/python/ChipDeviceController-ScriptBinding.cpp
@@ -193,6 +193,7 @@
ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
DeviceAvailableFunc callback);
+ChipError::StorageType pychip_FreeOperationalDeviceProxy(chip::OperationalDeviceProxy * deviceProxy);
ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
CommissioneeDeviceProxy ** proxy);
ChipError::StorageType pychip_ExpireSessions(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId);
@@ -668,16 +669,18 @@
}
namespace {
+
struct GetDeviceCallbacks
{
GetDeviceCallbacks(DeviceAvailableFunc callback) :
mOnSuccess(OnDeviceConnectedFn, this), mOnFailure(OnConnectionFailureFn, this), mCallback(callback)
{}
- static void OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device)
+ static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
{
- auto * self = static_cast<GetDeviceCallbacks *>(context);
- self->mCallback(device, CHIP_NO_ERROR.AsInteger());
+ auto * self = static_cast<GetDeviceCallbacks *>(context);
+ auto * operationalDeviceProxy = new OperationalDeviceProxy(&exchangeMgr, sessionHandle);
+ self->mCallback(operationalDeviceProxy, CHIP_NO_ERROR.AsInteger());
delete self;
}
@@ -702,6 +705,15 @@
return devCtrl->GetConnectedDevice(nodeId, &callbacks->mOnSuccess, &callbacks->mOnFailure).AsInteger();
}
+ChipError::StorageType pychip_FreeOperationalDeviceProxy(chip::OperationalDeviceProxy * deviceProxy)
+{
+ if (deviceProxy != nullptr)
+ {
+ delete deviceProxy;
+ }
+ return CHIP_NO_ERROR.AsInteger();
+}
+
ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
CommissioneeDeviceProxy ** proxy)
{
diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py
index 089fdae..6c36350 100644
--- a/src/controller/python/chip/ChipDeviceCtrl.py
+++ b/src/controller/python/chip/ChipDeviceCtrl.py
@@ -117,6 +117,30 @@
COMMISSIONING = 5
+class DeviceProxyWrapper():
+ ''' Encapsulates a pointer to OperationalDeviceProxy on the c++ side that needs to be
+ freed when DeviceProxyWrapper goes out of scope. There is a potential issue where
+ if this is copied around that a double free will occure, but how this is used today
+ that is not an issue that needs to be accounted for and it will become very apparent
+ if that happens.
+ '''
+
+ def __init__(self, deviceProxy: ctypes.c_void_p, dmLib=None):
+ self._deviceProxy = deviceProxy
+ self._dmLib = dmLib
+
+ def __del__(self):
+ if (self._dmLib is not None and builtins.chipStack is not None):
+ # This destructor is called from any threading context, including on the Matter threading context.
+ # So, we cannot call chipStack.Call or chipStack.CallAsync which waits for the posted work to
+ # actually be executed. Instead, we just post/schedule the work and move on.
+ builtins.chipStack.PostTaskOnChipThread(lambda: self._dmLib.pychip_FreeOperationalDeviceProxy(self._deviceProxy))
+
+ @property
+ def deviceProxy(self) -> ctypes.c_void_p:
+ return self._deviceProxy
+
+
class DiscoveryFilterType(enum.IntEnum):
# These must match chip::Dnssd::DiscoveryFilterType values (barring the naming convention)
NONE = 0
@@ -626,6 +650,7 @@
return self._Cluster
def GetConnectedDeviceSync(self, nodeid, allowPASE=True, timeoutMs: int = None):
+ ''' Returns DeviceProxyWrapper upon success.'''
self.CheckIsActive()
returnDevice = c_void_p(None)
@@ -647,7 +672,7 @@
self.devCtrl, nodeid, byref(returnDevice)), timeoutMs)
if res == 0:
print('Using PASE connection')
- return returnDevice
+ return DeviceProxyWrapper(returnDevice)
res = self._ChipStack.Call(lambda: self._dmLib.pychip_GetConnectedDeviceByNodeId(
self.devCtrl, nodeid, DeviceAvailableCallback), timeoutMs)
@@ -668,7 +693,8 @@
if returnDevice.value is None:
raise self._ChipStack.ErrorToException(returnErr)
- return returnDevice
+
+ return DeviceProxyWrapper(returnDevice, self._dmLib)
def ComputeRoundTripTimeout(self, nodeid, upperLayerProcessingTimeoutMs: int = 0):
''' Returns a computed timeout value based on the round-trip time it takes for the peer at the other end of the session to
@@ -680,7 +706,7 @@
'''
device = self.GetConnectedDeviceSync(nodeid)
res = self._ChipStack.Call(lambda: self._dmLib.pychip_DeviceProxy_ComputeRoundTripTimeout(
- device, upperLayerProcessingTimeoutMs))
+ device.deviceProxy, upperLayerProcessingTimeoutMs))
return res
async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects.ClusterCommand, responseType=None, timedRequestTimeoutMs: int = None, interactionTimeoutMs: int = None):
@@ -700,7 +726,7 @@
device = self.GetConnectedDeviceSync(nodeid, timeoutMs=interactionTimeoutMs)
res = ClusterCommand.SendCommand(
- future, eventLoop, responseType, device, ClusterCommand.CommandPath(
+ future, eventLoop, responseType, device.deviceProxy, ClusterCommand.CommandPath(
EndpointId=endpoint,
ClusterId=payload.cluster_id,
CommandId=payload.command_id,
@@ -739,7 +765,7 @@
v[0], v[1], v[2], 1, v[1].value))
res = ClusterAttribute.WriteAttributes(
- future, eventLoop, device, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs)
+ future, eventLoop, device.deviceProxy, attrs, timedRequestTimeoutMs=timedRequestTimeoutMs, interactionTimeoutMs=interactionTimeoutMs)
if res != 0:
raise self._ChipStack.ErrorToException(res)
return await future
@@ -920,7 +946,7 @@
eventPaths = [self._parseEventPathTuple(
v) for v in events] if events else None
- res = ClusterAttribute.Read(future=future, eventLoop=eventLoop, device=device, devCtrl=self, attributes=attributePaths, dataVersionFilters=clusterDataVersionFilters, events=eventPaths, returnClusterObject=returnClusterObject,
+ res = ClusterAttribute.Read(future=future, eventLoop=eventLoop, device=device.deviceProxy, devCtrl=self, attributes=attributePaths, dataVersionFilters=clusterDataVersionFilters, events=eventPaths, returnClusterObject=returnClusterObject,
subscriptionParameters=ClusterAttribute.SubscriptionParameters(reportInterval[0], reportInterval[1]) if reportInterval else None, fabricFiltered=fabricFiltered, keepSubscriptions=keepSubscriptions)
if res != 0:
raise self._ChipStack.ErrorToException(res)
@@ -1205,6 +1231,10 @@
c_void_p, c_uint64, _DeviceAvailableFunct]
self._dmLib.pychip_GetConnectedDeviceByNodeId.restype = c_uint32
+ self._dmLib.pychip_FreeOperationalDeviceProxy.argtypes = [
+ c_void_p]
+ self._dmLib.pychip_FreeOperationalDeviceProxy.restype = c_uint32
+
self._dmLib.pychip_GetDeviceBeingCommissioned.argtypes = [
c_void_p, c_uint64, c_void_p]
self._dmLib.pychip_GetDeviceBeingCommissioned.restype = c_uint32
diff --git a/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.h b/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.h
index 07e83c7..9bc96cc 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.h
@@ -64,7 +64,7 @@
chip::Callback::Callback<chip::OnDeviceConnected> mOnConnected;
chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnConnectFailed;
- static void OnConnected(void * context, chip::OperationalDeviceProxy * device);
+ static void OnConnected(void * context, chip::Messaging::ExchangeManager & exchangeMgr, chip::SessionHandle & sessionHandle);
static void OnConnectionFailure(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error);
};
diff --git a/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.mm b/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.mm
index 6e9550c..f61aeee 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.mm
+++ b/src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.mm
@@ -19,10 +19,11 @@
#import "MTRBaseDevice_Internal.h"
#import "MTRError_Internal.h"
-void MTRDeviceConnectionBridge::OnConnected(void * context, chip::OperationalDeviceProxy * device)
+void MTRDeviceConnectionBridge::OnConnected(
+ void * context, chip::Messaging::ExchangeManager & exchangeMgr, chip::SessionHandle & sessionHandle)
{
auto * object = static_cast<MTRDeviceConnectionBridge *>(context);
- object->mCompletionHandler(device->GetExchangeManager(), device->GetSecureSession(), nil);
+ object->mCompletionHandler(&exchangeMgr, chip::MakeOptional<chip::SessionHandle>(*sessionHandle->AsSecureSession()), nil);
object->Release();
}
diff --git a/src/lib/support/logging/CHIPLogging.cpp b/src/lib/support/logging/CHIPLogging.cpp
index b92e04d..a374893 100644
--- a/src/lib/support/logging/CHIPLogging.cpp
+++ b/src/lib/support/logging/CHIPLogging.cpp
@@ -115,7 +115,7 @@
"DIS" // Discovery
"IM\0" // InteractionModel
"TST" // Test
- "ODP" // OperationalDeviceProxy
+ "OSS" // OperationalSessionSetup
"ATM" // Automation
"CSM" // CASESessionManager
;
diff --git a/src/lib/support/logging/Constants.h b/src/lib/support/logging/Constants.h
index 37cc149..c91cf1a 100644
--- a/src/lib/support/logging/Constants.h
+++ b/src/lib/support/logging/Constants.h
@@ -56,7 +56,7 @@
kLogModule_Discovery,
kLogModule_InteractionModel,
kLogModule_Test,
- kLogModule_OperationalDeviceProxy,
+ kLogModule_OperationalSessionSetup,
kLogModule_Automation,
kLogModule_CASESessionManager,
diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp
index a690ec8..5fa99e0 100644
--- a/src/transport/SessionManager.cpp
+++ b/src/transport/SessionManager.cpp
@@ -445,6 +445,22 @@
});
}
+bool SessionManager::MarkSessionsAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type)
+{
+ bool found = false;
+ mSecureSessions.ForEachSession([&node, &type, &found](auto session) {
+ if (session->IsActiveSession() && session->GetPeer() == node &&
+ (!type.HasValue() || type.Value() == session->GetSecureSessionType()))
+ {
+ session->AsSecureSession()->MarkAsDefunct();
+ found = true;
+ }
+ return Loop::Continue;
+ });
+
+ return found;
+}
+
Optional<SessionHandle> SessionManager::AllocateSession(SecureSession::Type secureSessionType,
const ScopedNodeId & sessionEvictionHint)
{
diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h
index 2876328..9454555 100644
--- a/src/transport/SessionManager.h
+++ b/src/transport/SessionManager.h
@@ -351,6 +351,19 @@
/**
* @brief
+ * Marks all active sessions that match provided arguments as defunct.
+ *
+ * @param node Scoped node ID of the active sessions we should mark as defunct.
+ * @param type Type of session we are looking to mark as defunct. If matching
+ * against all types of sessions is desired, NullOptional should
+ * be passed into type.
+ * @return True, if at least one session was marked as defunct, otherwise
+ * return is False.
+ */
+ bool MarkSessionsAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type);
+
+ /**
+ * @brief
* Return the System Layer pointer used by current SessionManager.
*/
System::Layer * SystemLayer() { return mSystemLayer; }
