Remove sleep from ModelCommand

examples/chip-tool/commands/clusters/ModelCommand.cpp

@@ -37,25 +37,6 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC
  "Default DispatchSingleClusterCommand is called, this should be replaced by actual dispatched for cluster commands");
 }
 
-CHIP_ERROR WaitForSessionSetup(chip::Controller::Device * device)
-{
- constexpr time_t kWaitPerIterationSec = 1;
- constexpr uint16_t kIterationCount = 5;
-
- struct timespec sleep_time;
- sleep_time.tv_sec = kWaitPerIterationSec;
- sleep_time.tv_nsec = 0;
-
- for (uint32_t i = 0; i < kIterationCount && device->IsSessionSetupInProgress(); i++)
- {
- nanosleep(&sleep_time, nullptr);
- }
-
- ReturnErrorCodeIf(!device->IsSecureConnected(), CHIP_ERROR_TIMEOUT);
-
- return CHIP_NO_ERROR;
-}
-
 CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId)
 {
  CHIP_ERROR err = CHIP_NO_ERROR;
@@ -70,18 +51,10 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId)
  {
  chip::DeviceLayer::StackLock lock;
 
- err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
- VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId));
-
- // TODO - Implement notification from device object when the secure session is available.
- // Current code is polling the device object to check for secure session availability. This should
- // be updated to a notification/callback mechanism.
- err = WaitForSessionSetup(mDevice);
+ err = GetExecContext()->commissioner->GetConnectedDevice(remoteId, &mOnDeviceConnectedCallback,
+ &mOnDeviceConnectionFailureCallback);
  VerifyOrExit(err == CHIP_NO_ERROR,
- ChipLogError(chipTool, "Timed out while waiting for session setup for device: %" PRIu64, localId));
-
- err = SendCommand(mDevice, mEndPointId);
- VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err)));
+ ChipLogError(chipTool, "Failed in initiating connection to the device: %" PRIu64 ", error %d", remoteId, err));
  }
 
  WaitForResponse(kWaitDurationInSeconds);
@@ -91,3 +64,19 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId)
 exit:
  return err;
 }
+
+void ModelCommand::OnDeviceConnectedFn(void * context, chip::Controller::Device * device)
+{
+ ModelCommand * command = reinterpret_cast<ModelCommand *>(context);
+ VerifyOrReturn(command != nullptr,
+ ChipLogError(chipTool, "Device connected, but cannot send the command, as the context is null"));
+ command->SendCommand(device, command->mEndPointId);
+}
+
+void ModelCommand::OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error)
+{
+ ModelCommand * command = reinterpret_cast<ModelCommand *>(context);
+ ChipLogError(chipTool, "Failed in connecting to the device %" PRIu64 ". Error %d", deviceId, error);
+ VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "ModelCommand context is null"));
+ command->SetCommandExitStatus(false);
+}

examples/chip-tool/commands/clusters/ModelCommand.h

@@ -31,7 +31,10 @@
 class ModelCommand : public Command
 {
 public:
- ModelCommand(const char * commandName) : Command(commandName) {}
+ ModelCommand(const char * commandName) :
+ Command(commandName), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this),
+ mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this)
+ {}
 
  void AddArguments() { AddArgument("endpoint-id", CHIP_ZCL_ENDPOINT_MIN, CHIP_ZCL_ENDPOINT_MAX, &mEndPointId); }
 
@@ -41,6 +44,11 @@ class ModelCommand : public Command
  virtual CHIP_ERROR SendCommand(ChipDevice * device, uint8_t endPointId) = 0;
 
 private:
- ChipDevice * mDevice;
  uint8_t mEndPointId;
+
+ static void OnDeviceConnectedFn(void * context, chip::Controller::Device * device);
+ static void OnDeviceConnectionFailureFn(void * context, NodeId deviceId, CHIP_ERROR error);
+
+ chip::Callback::Callback<chip::Controller::OnDeviceConnected> mOnDeviceConnectedCallback;
+ chip::Callback::Callback<chip::Controller::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
 };

examples/chip-tool/commands/discover/Commands.h

@@ -66,10 +66,8 @@ class Update : public DiscoverCommand
  /////////// DiscoverCommand Interface /////////
  CHIP_ERROR RunCommand(NodeId remoteId, uint64_t fabricId) override
  {
- ChipDevice * device;
- ReturnErrorOnFailure(GetExecContext()->commissioner->GetDevice(remoteId, &device));
  ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", remoteId, fabricId);
- return GetExecContext()->commissioner->UpdateDevice(device, fabricId);
+ return GetExecContext()->commissioner->UpdateDevice(remoteId, fabricId);
  }
 
  /////////// DeviceAddressUpdateDelegate Interface /////////

examples/chip-tool/commands/pairing/PairingCommand.cpp

@@ -162,6 +162,20 @@ void PairingCommand::OnPairingDeleted(CHIP_ERROR err)
  SetCommandExitStatus(err == CHIP_NO_ERROR);
 }
 
+void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
+{
+ if (err == CHIP_NO_ERROR)
+ {
+ ChipLogProgress(chipTool, "Device commissioning completed with success");
+ }
+ else
+ {
+ ChipLogProgress(chipTool, "Device commissioning Failure: %s", ErrorStr(err));
+ }
+
+ SetCommandExitStatus(err == CHIP_NO_ERROR);
+}
+
 CHIP_ERROR PairingCommand::SetupNetwork()
 {
 
@@ -316,13 +330,17 @@ void PairingCommand::OnEnableNetworkResponse(void * context, uint8_t errorCode,
 
 CHIP_ERROR PairingCommand::UpdateNetworkAddress()
 {
- ReturnErrorOnFailure(GetExecContext()->commissioner->GetDevice(mRemoteId, &mDevice));
  ChipLogProgress(chipTool, "Mdns: Updating NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", mRemoteId, mFabricId);
- return GetExecContext()->commissioner->UpdateDevice(mDevice, mFabricId);
+ return GetExecContext()->commissioner->UpdateDevice(mRemoteId, mFabricId);
 }
 
 void PairingCommand::OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR err)
 {
  ChipLogProgress(chipTool, "OnAddressUpdateComplete: %s", ErrorStr(err));
- SetCommandExitStatus(CHIP_NO_ERROR == err);
+ if (err != CHIP_NO_ERROR)
+ {
+ // Set exit status only if the address update failed.
+ // Otherwise wait for OnCommissioningComplete() callback.
+ SetCommandExitStatus(false);
+ }
 }

examples/chip-tool/commands/pairing/PairingCommand.h

@@ -103,6 +103,7 @@ class PairingCommand : public Command,
  void OnStatusUpdate(chip::Controller::DevicePairingDelegate::Status status) override;
  void OnPairingComplete(CHIP_ERROR error) override;
  void OnPairingDeleted(CHIP_ERROR error) override;
+ void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR error) override;
 
  /////////// DeviceAddressUpdateDelegate Interface /////////
  void OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR error) override;

src/controller/CHIPDevice.cpp

@@ -448,6 +448,11 @@ CHIP_ERROR Device::LoadSecureSessionParameters(ResetTransport resetNeeded)
  ExitNow(err = CHIP_ERROR_INCORRECT_STATE);
  }
 
+ if (mState == ConnectionState::Connecting)
+ {
+ ExitNow(err = CHIP_NO_ERROR);
+ }
+
  if (resetNeeded == ResetTransport::kYes)
  {
  err = mTransportMgr->ResetTransport(
@@ -513,8 +518,6 @@ void Device::OperationalCertProvisioned()
  mSessionManager->ExpirePairing(mSecureSession);
  mState = ConnectionState::NotConnected;
  }
-
- WarmupCASESession();
 }
 
 CHIP_ERROR Device::WarmupCASESession()
@@ -545,6 +548,17 @@ void Device::OnSessionEstablishmentError(CHIP_ERROR error)
 {
  mState = ConnectionState::NotConnected;
  mIDAllocator->Free(mCASESession.GetLocalKeyId());
+
+ Cancelable ready;
+ mConnectionFailure.DequeueAll(ready);
+ while (ready.mNext != &ready)
+ {
+ Callback::Callback<OnDeviceConnectionFailure> * cb =
+ Callback::Callback<OnDeviceConnectionFailure>::FromCancelable(ready.mNext);
+
+ cb->Cancel();
+ cb->mCall(cb->mContext, GetDeviceId(), error);
+ }
 }
 
 void Device::OnSessionEstablished()
@@ -559,6 +573,48 @@ void Device::OnSessionEstablished()
  OnSessionEstablishmentError(err);
  return;
  }
+
+ Cancelable ready;
+ mConnectionSuccess.DequeueAll(ready);
+ while (ready.mNext != &ready)
+ {
+ Callback::Callback<OnDeviceConnected> * cb = Callback::Callback<OnDeviceConnected>::FromCancelable(ready.mNext);
+
+ cb->Cancel();
+ cb->mCall(cb->mContext, this);
+ }
+}
+
+CHIP_ERROR Device::EstablishConnectivity(Callback::Callback<OnDeviceConnected> * onConnection,
+ Callback::Callback<OnDeviceConnectionFailure> * onFailure)
+{
+ bool loadedSecureSession = false;
+ ReturnErrorOnFailure(LoadSecureSessionParametersIfNeeded(loadedSecureSession));
+
+ if (loadedSecureSession)
+ {
+ if (IsOperationalCertProvisioned())
+ {
+ if (onConnection != nullptr)
+ {
+ mConnectionSuccess.Enqueue(onConnection->Cancel());
+ }
+
+ if (onFailure != nullptr)
+ {
+ mConnectionFailure.Enqueue(onFailure->Cancel());
+ }
+ }
+ else
+ {
+ if (onConnection != nullptr)
+ {
+ onConnection->mCall(onConnection->mContext, this);
+ }
+ }
+ }
+
+ return CHIP_NO_ERROR;
 }
 
 void Device::AddResponseHandler(uint8_t seqNum, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback)

src/controller/CHIPDevice.h

@@ -87,6 +87,11 @@ struct ControllerDeviceInitParams
 #endif
 };
 
+class Device;
+
+typedef void (*OnDeviceConnected)(void * context, Device * device);
+typedef void (*OnDeviceConnectionFailure)(void * context, NodeId deviceId, CHIP_ERROR error);
+
 class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEstablishmentDelegate
 {
 public:
@@ -352,15 +357,22 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
 
  ByteSpan GetCSRNonce() const { return ByteSpan(mCSRNonce, sizeof(mCSRNonce)); }
 
- /**
- * @brief
- * This function triggers CASE session setup if the device has been provisioned with
- * operational credentials, and there is no currently active session.
+ /*
+ * This function can be called to establish a secure session with the device.
+ *
+ * If the device doesn't have operational credentials, and is under commissioning process,
+ * PASE keys will be used for secure session.
+ *
+ * If the device has been commissioned and has operational credentials, CASE session
+ * setup will be triggered.
  *
- * @return CHIP_NO_ERROR if the session setup was triggered or a session is already available.
+ * On establishing the session, 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.
  */
-
- CHIP_ERROR WarmupCASESession();
+ CHIP_ERROR EstablishConnectivity(Callback::Callback<OnDeviceConnected> * onConnection,
+  Callback::Callback<OnDeviceConnectionFailure> * onFailure);
 
 private:
  enum class ConnectionState
@@ -430,6 +442,13 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
  */
  CHIP_ERROR LoadSecureSessionParametersIfNeeded(bool & didLoad);
 
+ /**
+ * This function triggers CASE session setup if the device has been provisioned with
+ * operational credentials, and there is no currently active session.
+ */
+
+ CHIP_ERROR WarmupCASESession();
+
  uint16_t mListenPort;
 
  Transport::AdminId mAdminId = Transport::kUndefinedAdminId;
@@ -445,6 +464,9 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
  uint8_t mCSRNonce[kOpCSRNonceLength];
 
  SessionIDAllocator * mIDAllocator = nullptr;
+
+ Callback::CallbackDeque mConnectionSuccess;
+ Callback::CallbackDeque mConnectionFailure;
 };
 
 /**