Skip to content

Commit

Permalink
Refactor SessionHandle (#13330)
Browse files Browse the repository at this point in the history
* Fix EPS32 build warning

* app/ReadPrepareParams: store SessionHolder instead of SessionHandle

* Refactor SessionHandle

* Fix GetFabricIndex/GetPeerNodeId calls in general commissioning server

* Fix usage of . instead of -> (tested with esp32 compile)

* Remove the define of global for nxp compilation: conflicts with STL usage and does not seem to be utilized at least for matter  builds

* Restyle fixes

* Fix getting peer address compilation for exchange context

Co-authored-by: Andrei Litvin <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Sep 20, 2023
1 parent 11a74e6 commit 1787181
Show file tree
Hide file tree
Showing 69 changed files with 1,056 additions and 881 deletions.
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

0 comments on commit 1787181

Please sign in to comment.