Skip to content

Commit

Permalink
If we have onSetupFailure, remove onFailure
Browse files Browse the repository at this point in the history
  • Loading branch information
@yufengwangca
yufengwangca committed Oct 11, 2023
1 parent 55e4404 commit 04556d3
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 135 deletions.
189 changes: 98 additions & 91 deletions src/app/OperationalSessionSetup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,91 +98,9 @@ void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * on
}

void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure)
{
CHIP_ERROR err = CHIP_NO_ERROR;
bool isConnected = false;

//
// Always enqueue our user provided callbacks into our callback list.
// If anything goes wrong below, we'll trigger failures (including any queued from
// a previous iteration which in theory shouldn't happen, but this is written to be more defensive)
//
EnqueueConnectionCallbacks(onConnection, onFailure, onSetupFailure);

switch (mState)
{
case State::Uninitialized:
err = CHIP_ERROR_INCORRECT_STATE;
break;

case State::NeedsAddress:
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
// LookupPeerAddress could perhaps call back with a result
// synchronously, so do our state update first.
MoveToState(State::ResolvingAddress);
err = LookupPeerAddress();
if (err != CHIP_NO_ERROR)
{
// Roll back the state change, since we are presumably not in
// the middle of a lookup.
MoveToState(State::NeedsAddress);
}
}

break;

case State::ResolvingAddress:
case State::WaitingForRetry:
isConnected = AttachToExistingSecureSession();
break;

case State::HasAddress:
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
// We should not actually every be in be in State::HasAddress. This
// is because in the same call that we moved to State::HasAddress
// we either move to State::Connecting or call
// DequeueConnectionCallbacks with an error thus releasing
// ourselves before any call would reach this section of code.
err = CHIP_ERROR_INCORRECT_STATE;
}

break;

case State::Connecting:
break;

case State::SecureConnected:
isConnected = true;
break;

default:
err = CHIP_ERROR_INCORRECT_STATE;
}

if (isConnected)
{
MoveToState(State::SecureConnected);
}

//
// Dequeue all our callbacks on either encountering an error
// or if we successfully connected. Both should not be set
// simultaneously.
//
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;
}
Connect(onConnection, nullptr, onSetupFailure);
}

void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & addr, const ReliableMessageProtocolConfig & config)
Expand Down Expand Up @@ -269,6 +187,94 @@ CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessagePro
return CHIP_NO_ERROR;
}

void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure)
{
CHIP_ERROR err = CHIP_NO_ERROR;
bool isConnected = false;

//
// Always enqueue our user provided callbacks into our callback list.
// If anything goes wrong below, we'll trigger failures (including any queued from
// a previous iteration which in theory shouldn't happen, but this is written to be more defensive)
//
EnqueueConnectionCallbacks(onConnection, onFailure, onSetupFailure);

switch (mState)
{
case State::Uninitialized:
err = CHIP_ERROR_INCORRECT_STATE;
break;

case State::NeedsAddress:
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
// LookupPeerAddress could perhaps call back with a result
// synchronously, so do our state update first.
MoveToState(State::ResolvingAddress);
err = LookupPeerAddress();
if (err != CHIP_NO_ERROR)
{
// Roll back the state change, since we are presumably not in
// the middle of a lookup.
MoveToState(State::NeedsAddress);
}
}

break;

case State::ResolvingAddress:
case State::WaitingForRetry:
isConnected = AttachToExistingSecureSession();
break;

case State::HasAddress:
isConnected = AttachToExistingSecureSession();
if (!isConnected)
{
// We should not actually every be in be in State::HasAddress. This
// is because in the same call that we moved to State::HasAddress
// we either move to State::Connecting or call
// DequeueConnectionCallbacks with an error thus releasing
// ourselves before any call would reach this section of code.
err = CHIP_ERROR_INCORRECT_STATE;
}

break;

case State::Connecting:
break;

case State::SecureConnected:
isConnected = true;
break;

default:
err = CHIP_ERROR_INCORRECT_STATE;
}

if (isConnected)
{
MoveToState(State::SecureConnected);
}

//
// Dequeue all our callbacks on either encountering an error
// or if we successfully connected. Both should not be set
// simultaneously.
//
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 OperationalSessionSetup::EnqueueConnectionCallbacks(Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure)
Expand All @@ -289,7 +295,8 @@ void OperationalSessionSetup::EnqueueConnectionCallbacks(Callback::Callback<OnDe
}
}

void OperationalSessionSetup::DequeueConnectionCallbacks(CHIP_ERROR error, SessionState state, ReleaseBehavior releaseBehavior)
void OperationalSessionSetup::DequeueConnectionCallbacks(CHIP_ERROR error, SessionEstablishmentStage stage,
ReleaseBehavior releaseBehavior)
{
Cancelable failureReady, setupFailureReady, successReady;

Expand Down Expand Up @@ -325,14 +332,14 @@ void OperationalSessionSetup::DequeueConnectionCallbacks(CHIP_ERROR error, Sessi

// DO NOT touch any members of this object after this point. It's dead.

NotifyConnectionCallbacks(failureReady, setupFailureReady, successReady, error, state, peerId, performingAddressUpdate,
NotifyConnectionCallbacks(failureReady, setupFailureReady, successReady, error, stage, peerId, performingAddressUpdate,
exchangeMgr, optionalSessionHandle);
}

void OperationalSessionSetup::NotifyConnectionCallbacks(Cancelable & failureReady, Cancelable & setupFailureReady,
Cancelable & successReady, CHIP_ERROR error, SessionState state,
const ScopedNodeId & peerId, bool performingAddressUpdate,
Messaging::ExchangeManager * exchangeMgr,
Cancelable & successReady, CHIP_ERROR error,
SessionEstablishmentStage stage, const ScopedNodeId & peerId,
bool performingAddressUpdate, Messaging::ExchangeManager * exchangeMgr,
const Optional<SessionHandle> & optionalSessionHandle)
{
//
Expand Down Expand Up @@ -365,7 +372,7 @@ void OperationalSessionSetup::NotifyConnectionCallbacks(Cancelable & failureRead
if (error != CHIP_NO_ERROR)
{
// Initialize the ConnnectionFailureInfo object
ConnnectionFailureInfo failureInfo(peerId, error, state);
ConnnectionFailureInfo failureInfo(peerId, error, stage);
cb->mCall(cb->mContext, failureInfo);
}
}
Expand All @@ -386,7 +393,7 @@ void OperationalSessionSetup::NotifyConnectionCallbacks(Cancelable & failureRead
}
}

void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error, SessionState state)
void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error, SessionEstablishmentStage stage)
{
VerifyOrReturn(mState == State::Connecting,
ChipLogError(Discovery, "OnSessionEstablishmentError was called while we were not connecting"));
Expand Down Expand Up @@ -431,7 +438,7 @@ void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error, Sess
#endif // CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
}

DequeueConnectionCallbacks(error, state);
DequeueConnectionCallbacks(error, stage);
// Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
}

Expand Down
33 changes: 17 additions & 16 deletions src/app/OperationalSessionSetup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,10 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
{
const ScopedNodeId peerId;
CHIP_ERROR error;
SessionState sessionState;
SessionEstablishmentStage sessionStage;

ConnnectionFailureInfo(const ScopedNodeId & peer, CHIP_ERROR err, SessionState state) :
peerId(peer), error(err), sessionState(state)
ConnnectionFailureInfo(const ScopedNodeId & peer, CHIP_ERROR err, SessionEstablishmentStage stage) :
peerId(peer), error(err), sessionStage(stage)
{}
};

Expand Down Expand Up @@ -193,8 +193,8 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
* The device is expected to have been commissioned, A CASE session
* setup will be triggered.
*
* On establishing the session, the callback function `onConnection` will be called. If the
* session setup fails, `onFailure` will be called.
* On establishing the session, if the session setup succeeds, the callback function `onConnection` will be called.
* If the session setup fails, `onFailure` will be called.
*
* If the session already exists, `onConnection` will be called immediately,
* before the Connect call returns.
Expand All @@ -211,24 +211,22 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
* The device is expected to have been commissioned, A CASE session
* setup will be triggered.
*
* On establishing the session, the callback function `onConnection` will be called. If the
* session setup fails, `onFailure` and `onSetupFailure` will be called.
* On establishing the session, if the session setup succeeds, the callback function `onConnection` will be called.
* If the session setup fails, `onSetupFailure` will be called.
*
* If the session already exists, `onConnection` will be called immediately,
* before the Connect call returns.
*
* `onFailure` and `onSetupFailure` may be called before the Connect call returns, for error
* cases that are detected synchronously (e.g. inability to start an address
* lookup).
* `onSetupFailure` may be called before the Connect call returns, for error cases that are detected synchronously
* (e.g. inability to start an address lookup).
*/
void Connect(Callback::Callback<OnDeviceConnected> * onConnection, Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure);
void Connect(Callback::Callback<OnDeviceConnected> * onConnection, Callback::Callback<OnSetupFailure> * onSetupFailure);

bool IsForAddressUpdate() const { return mPerformingAddressUpdate; }

//////////// SessionEstablishmentDelegate Implementation ///////////////
void OnSessionEstablished(const SessionHandle & session) override;
void OnSessionEstablishmentError(CHIP_ERROR error, SessionState state) override;
void OnSessionEstablishmentError(CHIP_ERROR error, SessionEstablishmentStage stage) override;

ScopedNodeId GetPeerId() const { return mPeerId; }

Expand Down Expand Up @@ -331,6 +329,9 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,

void CleanupCASEClient();

void Connect(Callback::Callback<OnDeviceConnected> * onConnection, Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure);

void EnqueueConnectionCallbacks(Callback::Callback<OnDeviceConnected> * onConnection,
Callback::Callback<OnDeviceConnectionFailure> * onFailure,
Callback::Callback<OnSetupFailure> * onSetupFailure);
Expand All @@ -356,12 +357,12 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
*
* Setting releaseBehavior to DoNotRelease is meant for use from the destructor
*/
void DequeueConnectionCallbacks(CHIP_ERROR error, SessionState state,
void DequeueConnectionCallbacks(CHIP_ERROR error, SessionEstablishmentStage stage,
ReleaseBehavior releaseBehavior = ReleaseBehavior::Release);

void DequeueConnectionCallbacks(CHIP_ERROR error, ReleaseBehavior releaseBehavior = ReleaseBehavior::Release)
{
this->DequeueConnectionCallbacks(error, SessionState::kUndefined, releaseBehavior);
this->DequeueConnectionCallbacks(error, SessionEstablishmentStage::kNotInKeyExchange, releaseBehavior);
}

/**
Expand All @@ -370,7 +371,7 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate,
* being released.
*/
static void NotifyConnectionCallbacks(Callback::Cancelable & failureReady, Callback::Cancelable & setupFailureReady,
Callback::Cancelable & successReady, CHIP_ERROR error, SessionState state,
Callback::Cancelable & successReady, CHIP_ERROR error, SessionEstablishmentStage stage,
const ScopedNodeId & peerId, bool performingAddressUpdate,
Messaging::ExchangeManager * exchangeMgr,
const Optional<SessionHandle> & optionalSessionHandle);
Expand Down
20 changes: 11 additions & 9 deletions src/protocols/secure_channel/CASESession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec)

void CASESession::AbortPendingEstablish(CHIP_ERROR err)
{
SessionState state = MapCASEStateToSessionState(mState);
SessionEstablishmentStage state = MapCASEStateToSessionEstablishmentStage(mState);
Clear();
// Do this last in case the delegate frees us.
NotifySessionEstablishmentError(err, state);
Expand Down Expand Up @@ -2268,26 +2268,28 @@ bool CASESession::InvokeBackgroundWorkWatchdog()
return watchdogFired;
}

// Helper function to map CASESession::State to SessionState
SessionState CASESession::MapCASEStateToSessionState(State caseState)
// Helper function to map CASESession::State to SessionEstablishmentStage
SessionEstablishmentStage CASESession::MapCASEStateToSessionEstablishmentStage(State caseState)
{
switch (caseState)
{
case State::kInitialized:
return SessionState::kUndefined;
return SessionEstablishmentStage::kNotInKeyExchange;
case State::kSentSigma1:
case State::kSentSigma1Resume:
return SessionState::kSentSigma1;
return SessionEstablishmentStage::kSentSigma1;
case State::kSentSigma2:
case State::kSentSigma2Resume:
return SessionEstablishmentStage::kSentSigma2;
case State::kSendSigma3Pending:
case State::kHandleSigma3Pending:
return SessionState::kSentSigma2;
return SessionEstablishmentStage::kReceivedSigma2;
case State::kSentSigma3:
return SessionState::kSentSigma3;
return SessionEstablishmentStage::kSentSigma3;
case State::kHandleSigma3Pending:
return SessionEstablishmentStage::kReceivedSigma3;
// Add more mappings here for other states
default:
return SessionState::kUndefined; // Default mapping
return SessionEstablishmentStage::kUnknown; // Default mapping
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/protocols/secure_channel/CASESession.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
Optional<State> mStopHandshakeAtState = Optional<State>::Missing();
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST

SessionState MapCASEStateToSessionState(State caseState);
SessionEstablishmentStage MapCASEStateToSessionEstablishmentStage(State caseState);
};

} // namespace chip
Loading

0 comments on commit 04556d3

Please sign in to comment.