Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Restyle Refactor SessionHandle #13376

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions examples/chip-tool/commands/common/CommandInvoker.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,33 +100,22 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
return CHIP_NO_ERROR;
}

CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, GroupId groupId, const RequestType & aRequestData)
CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, GroupId groupId, const RequestType & aRequestData)
{
app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, RequestType::GetClusterId(), RequestType::GetCommandId(),
(app::CommandPathFlags::kGroupIdValid) };

auto commandSender = Platform::MakeUnique<app::CommandSender>(this, aDevice->GetExchangeManager());
auto commandSender = Platform::MakeUnique<app::CommandSender>(this, exchangeManager);
VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_NO_MEMORY);

ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));

if (aDevice->GetSecureSession().HasValue())
Optional<SessionHandle> session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId);
if (!session.HasValue())
{
SessionHandle session = aDevice->GetSecureSession().Value();
session.SetGroupId(groupId);

if (!session.IsGroupSession())
{
return CHIP_ERROR_INCORRECT_STATE;
}

ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
}
else
{
// something fishy is going on
return CHIP_ERROR_INCORRECT_STATE;
return CHIP_ERROR_NO_MEMORY;
}
ReturnErrorOnFailure(commandSender->SendCommandRequest(session));

commandSender.release();
return CHIP_NO_ERROR;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
{
// TODO: This uses the current node as the provider to supply the OTA image. This can be configurable such that the provider
// supplying the response is not the provider supplying the OTA image.
FabricIndex fabricIndex = commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex();
FabricIndex fabricIndex = commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
FabricInfo * fabricInfo = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex);
NodeId nodeId = fabricInfo->GetPeerId().GetNodeId();

Expand Down
8 changes: 0 additions & 8 deletions src/app/CASESessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,6 @@ CHIP_ERROR CASESessionManager::GetPeerAddress(PeerId peerId, Transport::PeerAddr
return CHIP_NO_ERROR;
}

void CASESessionManager::OnSessionReleased(const SessionHandle & sessionHandle)
{
OperationalDeviceProxy * session = FindSession(sessionHandle);
VerifyOrReturn(session != nullptr, ChipLogDetail(Controller, "OnSessionReleased was called for unknown device, ignoring it."));

session->OnSessionReleased(sessionHandle);
}

OperationalDeviceProxy * CASESessionManager::FindSession(const SessionHandle & session)
{
return mConfig.devicePool->FindDevice(session);
Expand Down
5 changes: 1 addition & 4 deletions src/app/CASESessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct CASESessionManagerConfig
* 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is
* successful)
*/
class CASESessionManager : public SessionReleaseDelegate, public Dnssd::ResolverDelegate
class CASESessionManager : public Dnssd::ResolverDelegate
{
public:
CASESessionManager() = delete;
Expand Down Expand Up @@ -105,9 +105,6 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
*/
CHIP_ERROR GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr);

//////////// SessionReleaseDelegate Implementation ///////////////
void OnSessionReleased(const SessionHandle & session) override;

//////////// ResolverDelegate Implementation ///////////////
void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override;
void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override;
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()

FabricIndex CommandHandler::GetAccessingFabricIndex() const
{
return mpExchangeCtx->GetSessionHandle().GetFabricIndex();
return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
}

CommandHandler * CommandHandler::Handle::Get()
Expand Down
4 changes: 2 additions & 2 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte
for (auto & readHandler : mReadHandlers)
{
if (!readHandler.IsFree() && readHandler.IsSubscriptionType() &&
readHandler.GetInitiatorNodeId() == apExchangeContext->GetSessionHandle().GetPeerNodeId() &&
readHandler.GetAccessingFabricIndex() == apExchangeContext->GetSessionHandle().GetFabricIndex())
readHandler.GetInitiatorNodeId() == apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId() &&
readHandler.GetAccessingFabricIndex() == apExchangeContext->GetSessionHandle()->AsSecureSession()->GetFabricIndex())
{
bool keepSubscriptions = true;
System::PacketBufferTLVReader reader;
Expand Down
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath);
*
* @param[in] aSubjectDescriptor The subject descriptor for the read.
* @param[in] aPath The concrete path of the data being read.
* @param[in] aAttributeReport The TLV Builder for Cluter attribute builder.
* @param[in] aAttributeReports The TLV Builder for Cluter attribute builder.
*
* @retval CHIP_NO_ERROR on success
*/
Expand Down
13 changes: 3 additions & 10 deletions src/app/OperationalDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
return CHIP_NO_ERROR;
}

Transport::SecureSession * secureSession = mInitParams.sessionManager->GetSecureSession(mSecureSession.Get());
if (secureSession != nullptr)
{
secureSession->SetPeerAddress(addr);
}
mSecureSession.Get()->AsSecureSession()->SetPeerAddress(addr);
}

return err;
Expand Down Expand Up @@ -262,7 +258,7 @@ CHIP_ERROR OperationalDeviceProxy::Disconnect()
return CHIP_NO_ERROR;
}

void OperationalDeviceProxy::SetConnectedSession(SessionHandle handle)
void OperationalDeviceProxy::SetConnectedSession(const SessionHandle & handle)
{
mSecureSession.Grab(handle);
mState = State::SecureConnected;
Expand Down Expand Up @@ -296,12 +292,9 @@ void OperationalDeviceProxy::DeferCloseCASESession()
mSystemLayer->ScheduleWork(CloseCASESessionTask, this);
}

void OperationalDeviceProxy::OnSessionReleased(const SessionHandle & session)
void OperationalDeviceProxy::OnSessionReleased()
{
VerifyOrReturn(mSecureSession.Contains(session),
ChipLogDetail(Controller, "Connection expired, but it doesn't match the current session"));
mState = State::Initialized;
mSecureSession.Release();
}

CHIP_ERROR OperationalDeviceProxy::ShutdownSubscriptions()
Expand Down
8 changes: 4 additions & 4 deletions src/app/OperationalDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
{
public:
virtual ~OperationalDeviceProxy();
OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId)
OperationalDeviceProxy(DeviceProxyInitParams & params, PeerId peerId) : mSecureSession(*this)
{
VerifyOrReturn(params.Validate() == CHIP_NO_ERROR);

Expand Down Expand Up @@ -124,7 +124,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
* Called when a connection is closing.
* The object releases all resources associated with the connection.
*/
void OnSessionReleased(const SessionHandle & session) override;
void OnSessionReleased() override;

void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeResolutionData)
{
Expand All @@ -150,7 +150,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele
*
* Note: Avoid using this function generally as it is Deprecated
*/
void SetConnectedSession(SessionHandle handle);
void SetConnectedSession(const SessionHandle & handle);

NodeId GetDeviceId() const override { return mPeerId.GetNodeId(); }

Expand Down Expand Up @@ -219,7 +219,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy, SessionReleaseDele

State mState = State::Uninitialized;

SessionHolder mSecureSession;
SessionHolderWithDelegate mSecureSession;

uint8_t mSequenceNumber = 0;

Expand Down
22 changes: 9 additions & 13 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,16 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
ReturnErrorOnFailure(writer.Finalize(&msgBuf));
}

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHandle, this);
mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);

mpExchangeCtx->SetResponseTimeout(aReadPrepareParams.mTimeout);

ReturnErrorOnFailure(mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));

mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex();
mPeerNodeId = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();

MoveToState(ClientState::AwaitingInitialReport);

Expand Down Expand Up @@ -576,8 +576,10 @@ CHIP_ERROR ReadClient::RefreshLivenessCheckTimer()
CHIP_ERROR err = CHIP_NO_ERROR;
CancelLivenessCheckTimer();
VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mpExchangeCtx->HasSessionHandle(), err = CHIP_ERROR_INCORRECT_STATE);

System::Clock::Timeout timeout = System::Clock::Seconds16(mMaxIntervalCeilingSeconds) + mpExchangeCtx->GetAckTimeout();
System::Clock::Timeout timeout =
System::Clock::Seconds16(mMaxIntervalCeilingSeconds) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout();
// EFR32/MBED/INFINION/K32W's chrono count return long unsinged, but other platform returns unsigned
ChipLogProgress(DataManagement, "Refresh LivenessCheckTime with %lu milliseconds", static_cast<long unsigned>(timeout.count()));
err = InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer(
Expand Down Expand Up @@ -699,21 +701,15 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara

ReturnErrorOnFailure(writer.Finalize(&msgBuf));

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHandle, this);
mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);

mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
if (mpExchangeCtx->IsBLETransport())
{
ChipLogError(DataManagement, "IM Subscribe cannot work with BLE");
return CHIP_ERROR_INCORRECT_STATE;
}

ReturnErrorOnFailure(mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse)));

mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHandle.GetFabricIndex();
mPeerNodeId = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetPeerNodeId();
mFabricIndex = aReadPrepareParams.mSessionHolder->AsSecureSession()->GetFabricIndex();

MoveToState(ClientState::AwaitingInitialReport);

Expand Down
9 changes: 5 additions & 4 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ CHIP_ERROR ReadHandler::Init(Messaging::ExchangeManager * apExchangeMgr, Interac
mActiveSubscription = false;
mIsChunkedReport = false;
mInteractionType = aInteractionType;
mInitiatorNodeId = apExchangeContext->GetSessionHandle().GetPeerNodeId();
mSubjectDescriptor = apExchangeContext->GetSessionHandle().GetSubjectDescriptor();
mInitiatorNodeId = apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId();
mSubjectDescriptor = apExchangeContext->GetSessionHandle()->GetSubjectDescriptor();
mHoldSync = false;
mLastWrittenEventsBytes = 0;
if (apExchangeContext != nullptr)
Expand Down Expand Up @@ -201,12 +201,13 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
VerifyOrReturnLogError(IsReportable(), CHIP_ERROR_INCORRECT_STATE);
if (IsPriming() || IsChunkedReport())
{
mSessionHandle.SetValue(mpExchangeCtx->GetSessionHandle());
mSessionHandle.Grab(mpExchangeCtx->GetSessionHandle());
}
else
{
VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
mpExchangeCtx = mpExchangeMgr->NewContext(mSessionHandle.Value(), this);
VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
mpExchangeCtx = mpExchangeMgr->NewContext(mSessionHandle.Get(), this);
mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
}
VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
uint64_t mSubscriptionId = 0;
uint16_t mMinIntervalFloorSeconds = 0;
uint16_t mMaxIntervalCeilingSeconds = 0;
Optional<SessionHandle> mSessionHandle;
SessionHolder mSessionHandle;
bool mHoldReport = false;
bool mDirty = false;
bool mActiveSubscription = false;
Expand Down
8 changes: 4 additions & 4 deletions src/app/ReadPrepareParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace chip {
namespace app {
struct ReadPrepareParams
{
SessionHandle mSessionHandle;
SessionHolder mSessionHolder;
EventPathParams * mpEventPathParamsList = nullptr;
size_t mEventPathParamsListSize = 0;
AttributePathParams * mpAttributePathParamsList = nullptr;
Expand All @@ -40,8 +40,8 @@ struct ReadPrepareParams
uint16_t mMaxIntervalCeilingSeconds = 0;
bool mKeepSubscriptions = true;

ReadPrepareParams(const SessionHandle & sessionHandle) : mSessionHandle(sessionHandle) {}
ReadPrepareParams(ReadPrepareParams && other) : mSessionHandle(other.mSessionHandle)
ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
ReadPrepareParams(ReadPrepareParams && other) : mSessionHolder(other.mSessionHolder)
{
mKeepSubscriptions = other.mKeepSubscriptions;
mpEventPathParamsList = other.mpEventPathParamsList;
Expand All @@ -64,7 +64,7 @@ struct ReadPrepareParams
return *this;

mKeepSubscriptions = other.mKeepSubscriptions;
mSessionHandle = other.mSessionHandle;
mSessionHolder = other.mSessionHolder;
mpEventPathParamsList = other.mpEventPathParamsList;
mEventPathParamsListSize = other.mEventPathParamsListSize;
mpAttributePathParamsList = other.mpAttributePathParamsList;
Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::
exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DataManagement, "Write client failed to SendWriteRequest");
ChipLogError(DataManagement, "Write client failed to SendWriteRequest: %s", ErrorStr(err));
ClearExistingExchangeContext();
}
else
Expand All @@ -254,7 +254,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::
// handle this object dying (e.g. due to IM enging shutdown) while the
// async bits are pending we'd need to malloc some state bit that we can
// twiddle if we die. For now just do the OnDone callback sync.
if (session.IsGroupSession())
if (session->IsGroupSession())
{
// Always shutdown on Group communication
ChipLogDetail(DataManagement, "Closing on group Communication ");
Expand Down
2 changes: 1 addition & 1 deletion src/app/WriteClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class WriteClient : public Messaging::ExchangeDelegate

NodeId GetSourceNodeId() const
{
return mpExchangeCtx != nullptr ? mpExchangeCtx->GetSessionHandle().GetPeerNodeId() : kUndefinedNodeId;
return mpExchangeCtx != nullptr ? mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetPeerNodeId() : kUndefinedNodeId;
}

private:
Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
CHIP_ERROR err = CHIP_NO_ERROR;

ReturnErrorCodeIf(mpExchangeCtx == nullptr, CHIP_ERROR_INTERNAL);
const Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle().GetSubjectDescriptor();
const Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle()->GetSubjectDescriptor();

while (CHIP_NO_ERROR == (err = aAttributeDataIBsReader.Next()))
{
Expand Down Expand Up @@ -279,7 +279,7 @@ CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathPar

FabricIndex WriteHandler::GetAccessingFabricIndex() const
{
return mpExchangeCtx->GetSessionHandle().GetFabricIndex();
return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
}

const char * WriteHandler::GetStateStr() const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback(
* This allows device to send messages back to commissioner.
* Once bindings are implemented, this may no longer be needed.
*/
server->SetFabricIndex(commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex());
server->SetPeerNodeId(commandObj->GetExchangeContext()->GetSessionHandle().GetPeerNodeId());
SessionHandle handle = commandObj->GetExchangeContext()->GetSessionHandle();
server->SetFabricIndex(handle->AsSecureSession()->GetFabricIndex());
server->SetPeerNodeId(handle->AsSecureSession()->GetPeerNodeId());

CheckSuccess(server->CommissioningComplete(), Failure);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,8 @@ CHIP_ERROR ComputeAttestationSignature(app::CommandHandler * commandObj,

// TODO: Create an alternative way to retrieve the Attestation Challenge without this huge amount of calls.
// Retrieve attestation challenge
ByteSpan attestationChallenge = commandObj->GetExchangeContext()
->GetExchangeMgr()
->GetSessionManager()
->GetSecureSession(commandObj->GetExchangeContext()->GetSessionHandle())
->GetCryptoContext()
.GetAttestationChallenge();
ByteSpan attestationChallenge =
commandObj->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetCryptoContext().GetAttestationChallenge();

Hash_SHA256_stream hashStream;
ReturnErrorOnFailure(hashStream.Begin());
Expand Down Expand Up @@ -291,7 +287,7 @@ class FabricCleanupExchangeDelegate : public chip::Messaging::ExchangeDelegate
void OnResponseTimeout(chip::Messaging::ExchangeContext * ec) override {}
void OnExchangeClosing(chip::Messaging::ExchangeContext * ec) override
{
FabricIndex currentFabricIndex = ec->GetSessionHandle().GetFabricIndex();
FabricIndex currentFabricIndex = ec->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
ec->GetExchangeMgr()->GetSessionManager()->ExpireAllPairingsForFabric(currentFabricIndex);
}
};
Expand Down
Loading