Skip to content

Commit

Permalink
[IM] Decouple CommandSender with CHIPDevice (#6802)
Browse files Browse the repository at this point in the history 
* [IM] Decouple CommandSender with CHIPDevice

* set CHIP_MAX_NUM_COMMAND_SENDER to 4

* Fix cirque test

* Run codegen
  • Loading branch information
@erjiaqing @pull
erjiaqing authored and pull[bot] committed Jun 29, 2021
1 parent a8ffd53 commit 1205755
Show file tree
Hide file tree
Showing 24 changed files with 4,559 additions and 3,318 deletions.
1,984 changes: 1,172 additions & 812 deletions examples/chip-tool/gen/CHIPClusters.cpp
View file

Large diffs are not rendered by default.

45 changes: 27 additions & 18 deletions examples/pump-controller-app/pump-controller-common/gen/CHIPClusters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,60 +35,69 @@ CHIP_ERROR OnOffCluster::Off(Callback::Cancelable * onSuccessCallback, Callback:
{
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

if (mpCommandSender == nullptr)
{
ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&mpCommandSender));
}

app::CommandPathParams cmdParams = { mEndpoint, /* group id */ 0, mClusterId, kOffCommandId,
(chip::app::CommandPathFlags::kEndpointIdValid) };
app::Command * ZCLcommand = mDevice->GetCommandSender();

ReturnErrorOnFailure(ZCLcommand->PrepareCommand(&cmdParams));
ReturnErrorOnFailure(mpCommandSender->PrepareCommand(&cmdParams));

// Command takes no arguments.

ReturnErrorOnFailure(ZCLcommand->FinishCommand());
ReturnErrorOnFailure(mpCommandSender->FinishCommand());

// #6308: This is a temporary solution before we fully support IM on application side and should be replaced by IMDelegate.
mDevice->AddIMResponseHandler(onSuccessCallback, onFailureCallback);
mDevice->AddIMResponseHandler(mpCommandSender, onSuccessCallback, onFailureCallback);

return mDevice->SendCommands();
return mDevice->SendCommands(mpCommandSender);
}

CHIP_ERROR OnOffCluster::On(Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback)
{
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

if (mpCommandSender == nullptr)
{
ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&mpCommandSender));
}

app::CommandPathParams cmdParams = { mEndpoint, /* group id */ 0, mClusterId, kOnCommandId,
(chip::app::CommandPathFlags::kEndpointIdValid) };
app::Command * ZCLcommand = mDevice->GetCommandSender();

ReturnErrorOnFailure(ZCLcommand->PrepareCommand(&cmdParams));
ReturnErrorOnFailure(mpCommandSender->PrepareCommand(&cmdParams));

// Command takes no arguments.

ReturnErrorOnFailure(ZCLcommand->FinishCommand());
ReturnErrorOnFailure(mpCommandSender->FinishCommand());

// #6308: This is a temporary solution before we fully support IM on application side and should be replaced by IMDelegate.
mDevice->AddIMResponseHandler(onSuccessCallback, onFailureCallback);
mDevice->AddIMResponseHandler(mpCommandSender, onSuccessCallback, onFailureCallback);

return mDevice->SendCommands();
return mDevice->SendCommands(mpCommandSender);
}

CHIP_ERROR OnOffCluster::Toggle(Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback)
{
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

if (mpCommandSender == nullptr)
{
ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&mpCommandSender));
}

app::CommandPathParams cmdParams = { mEndpoint, /* group id */ 0, mClusterId, kToggleCommandId,
(chip::app::CommandPathFlags::kEndpointIdValid) };
app::Command * ZCLcommand = mDevice->GetCommandSender();

ReturnErrorOnFailure(ZCLcommand->PrepareCommand(&cmdParams));
ReturnErrorOnFailure(mpCommandSender->PrepareCommand(&cmdParams));

// Command takes no arguments.

ReturnErrorOnFailure(ZCLcommand->FinishCommand());
ReturnErrorOnFailure(mpCommandSender->FinishCommand());

// #6308: This is a temporary solution before we fully support IM on application side and should be replaced by IMDelegate.
mDevice->AddIMResponseHandler(onSuccessCallback, onFailureCallback);
mDevice->AddIMResponseHandler(mpCommandSender, onSuccessCallback, onFailureCallback);

return mDevice->SendCommands();
return mDevice->SendCommands(mpCommandSender);
}

// OnOff Cluster Attributes
Expand Down
6 changes: 4 additions & 2 deletions src/app/CommandSender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ void CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeCon
// This needs to be done before the Reset() call, because Reset() aborts mpExchangeCtx if its not null.
mpExchangeCtx->Close();
mpExchangeCtx = nullptr;
Reset();

if (mpDelegate != nullptr)
{
Expand All @@ -107,18 +106,21 @@ void CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeCon
mpDelegate->CommandResponseProcessed(this);
}
}

Shutdown();
}

void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogProgress(DataManagement, "Time out! failed to receive invoke command response from Exchange: %d",
apExchangeContext->GetExchangeId());
Reset();

if (mpDelegate != nullptr)
{
mpDelegate->CommandResponseError(this, CHIP_ERROR_TIMEOUT);
}

Shutdown();
}

CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement)
Expand Down
5 changes: 3 additions & 2 deletions src/app/InteractionModelEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@
#include <app/reporting/Engine.h>
#include <app/util/basic-types.h>

#define CHIP_MAX_NUM_COMMAND_HANDLER 1
#define CHIP_MAX_NUM_COMMAND_SENDER 1
// TODO: Make number of command handler and command sender configurable
#define CHIP_MAX_NUM_COMMAND_HANDLER 4
#define CHIP_MAX_NUM_COMMAND_SENDER 4
#define CHIP_MAX_NUM_READ_CLIENT 1
#define CHIP_MAX_NUM_READ_HANDLER 1
#define CHIP_MAX_REPORTS_IN_FLIGHT 1
Expand Down
24 changes: 11 additions & 13 deletions src/app/tests/integration/chip_im_initiator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ constexpr size_t kMaxReadMessageCount = 3;
constexpr int32_t gMessageIntervalSeconds = 1;
constexpr chip::Transport::AdminId gAdminId = 0;

// The CommandSender object.
chip::app::CommandSender * gpCommandSender = nullptr;

// The ReadClient object.
chip::app::ReadClient * gpReadClient = nullptr;

Expand All @@ -79,10 +76,12 @@ uint64_t gReadRespCount = 0;

std::condition_variable gCond;

CHIP_ERROR SendCommandRequest(void)
CHIP_ERROR SendCommandRequest(chip::app::CommandSender * commandSender)
{
CHIP_ERROR err = CHIP_NO_ERROR;

VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_INCORRECT_STATE);

gLastMessageTime = chip::System::Timer::GetCurrentEpoch();

printf("\nSend invoke command request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId);
Expand All @@ -99,20 +98,20 @@ CHIP_ERROR SendCommandRequest(void)
uint8_t effectVariant = 1;
chip::TLV::TLVWriter * writer;

err = gpCommandSender->PrepareCommand(&commandPathParams);
err = commandSender->PrepareCommand(&commandPathParams);
SuccessOrExit(err);

writer = gpCommandSender->GetCommandDataElementTLVWriter();
writer = commandSender->GetCommandDataElementTLVWriter();
err = writer->Put(chip::TLV::ContextTag(1), effectIdentifier);
SuccessOrExit(err);

err = writer->Put(chip::TLV::ContextTag(2), effectVariant);
SuccessOrExit(err);

err = gpCommandSender->FinishCommand();
err = commandSender->FinishCommand();
SuccessOrExit(err);

err = gpCommandSender->SendCommandRequest(chip::kTestDeviceNodeId, gAdminId);
err = commandSender->SendCommandRequest(chip::kTestDeviceNodeId, gAdminId);
SuccessOrExit(err);

if (err == CHIP_NO_ERROR)
Expand Down Expand Up @@ -335,16 +334,16 @@ int main(int argc, char * argv[])
err = EstablishSecureSession();
SuccessOrExit(err);

err = chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&gpCommandSender);
SuccessOrExit(err);

err = chip::app::InteractionModelEngine::GetInstance()->NewReadClient(&gpReadClient);
SuccessOrExit(err);

// Connection has been established. Now send the CommandRequests.
for (unsigned int i = 0; i < kMaxCommandMessageCount; i++)
{
err = SendCommandRequest();
chip::app::CommandSender * commandSender;
err = chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&commandSender);
SuccessOrExit(err);
err = SendCommandRequest(commandSender);
if (err != CHIP_NO_ERROR)
{
printf("Send command request failed: %s\n", chip::ErrorStr(err));
Expand Down Expand Up @@ -373,7 +372,6 @@ int main(int argc, char * argv[])
}
}

gpCommandSender->Shutdown();
gpReadClient->Shutdown();
chip::app::InteractionModelEngine::GetInstance()->Shutdown();
ShutdownChip();
Expand Down
17 changes: 10 additions & 7 deletions src/app/zap-templates/templates/app/CHIPClusters-src.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,18 @@ CHIP_ERROR {{asCamelCased clusterName false}}Cluster::{{asCamelCased name false}
{
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

if (mpCommandSender == nullptr)
{
ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&mpCommandSender));
}

app::CommandPathParams cmdParams = { mEndpoint, /* group id */ 0, mClusterId, k{{asCamelCased name false}}CommandId,
(chip::app::CommandPathFlags::kEndpointIdValid) };
app::Command * ZCLcommand = mDevice->GetCommandSender();

ReturnErrorOnFailure(ZCLcommand->PrepareCommand(&cmdParams));
ReturnErrorOnFailure(mpCommandSender->PrepareCommand(&cmdParams));

{{#chip_server_cluster_command_arguments}}
{{#first}}
TLV::TLVWriter * writer = ZCLcommand->GetCommandDataElementTLVWriter();
TLV::TLVWriter * writer = mpCommandSender->GetCommandDataElementTLVWriter();
uint8_t argSeqNumber = 0;
{{/first}}
// {{asCamelCased label}}: {{asCamelCased type}}
Expand All @@ -38,12 +41,12 @@ CHIP_ERROR {{asCamelCased clusterName false}}Cluster::{{asCamelCased name false}
// Command takes no arguments.
{{/chip_server_cluster_command_arguments}}

ReturnErrorOnFailure(ZCLcommand->FinishCommand());
ReturnErrorOnFailure(mpCommandSender->FinishCommand());

// #6308: This is a temporary solution before we fully support IM on application side and should be replaced by IMDelegate.
mDevice->AddIMResponseHandler(onSuccessCallback, onFailureCallback);
mDevice->AddIMResponseHandler(mpCommandSender, onSuccessCallback, onFailureCallback);

return mDevice->SendCommands();
return mDevice->SendCommands(mpCommandSender);
}

{{/chip_server_cluster_commands}}
Expand Down
13 changes: 9 additions & 4 deletions src/controller/CHIPCluster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,30 @@
* the CHIP device.
*/

#include <app/InteractionModelEngine.h>
#include <controller/CHIPCluster.h>
#include <protocols/temp_zcl/TempZCL.h>
#include <support/CodeUtils.h>

namespace chip {
namespace Controller {

CHIP_ERROR ClusterBase::Associate(Device * device, EndpointId endpoint)
CHIP_ERROR ClusterBase::Associate(Device * device, EndpointId endpoint, app::CommandSender * commandSender)
{
CHIP_ERROR err = CHIP_NO_ERROR;
// TODO: Check if the device supports mCluster at the requested endpoint

mDevice = device;
mEndpoint = endpoint;
mDevice = device;
mEndpoint = endpoint;
mpCommandSender = commandSender;

return err;
}

void ClusterBase::Dissociate()
{
mDevice = nullptr;
mDevice = nullptr;
mpCommandSender = nullptr;
}

CHIP_ERROR ClusterBase::SendCommand(uint8_t seqNum, chip::System::PacketBufferHandle payload,
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPCluster.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class DLL_EXPORT ClusterBase
public:
virtual ~ClusterBase() {}

CHIP_ERROR Associate(Device * device, EndpointId endpoint);
CHIP_ERROR Associate(Device * device, EndpointId endpoint, app::CommandSender * commandSender = nullptr);

void Dissociate();

Expand Down Expand Up @@ -73,6 +73,7 @@ class DLL_EXPORT ClusterBase

const ClusterId mClusterId;
Device * mDevice;
app::CommandSender * mpCommandSender = nullptr;
EndpointId mEndpoint;
};

Expand Down
26 changes: 8 additions & 18 deletions src/controller/CHIPDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ CHIP_ERROR Device::LoadSecureSessionParametersIfNeeded(bool & didLoad)
return CHIP_NO_ERROR;
}

CHIP_ERROR Device::SendCommands()
CHIP_ERROR Device::SendCommands(app::CommandSender * commandObj)
{
bool loadedSecureSession = false;
ReturnErrorOnFailure(LoadSecureSessionParametersIfNeeded(loadedSecureSession));
VerifyOrReturnError(mCommandSender != nullptr, CHIP_ERROR_INCORRECT_STATE);
return mCommandSender->SendCommandRequest(mDeviceId, mAdminId, &mSecureSession);
VerifyOrReturnError(commandObj != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
return commandObj->SendCommandRequest(mDeviceId, mAdminId, &mSecureSession);
}

CHIP_ERROR Device::Serialize(SerializedDevice & output)
Expand Down Expand Up @@ -442,24 +442,25 @@ void Device::CancelResponseHandler(uint8_t seqNum)
mCallbacksMgr.CancelResponseCallback(mDeviceId, seqNum);
}

void Device::AddIMResponseHandler(Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback)
void Device::AddIMResponseHandler(app::Command * commandObj, Callback::Cancelable * onSuccessCallback,
Callback::Cancelable * onFailureCallback)
{
// We are using the pointer to command sender object as the identifier of command transactions. This makes sense as long as
// there are only one active command transaction on one command sender object. This is a bit tricky, we try to assume that
// chip::NodeId is uint64_t so the pointer can be used as a NodeId for CallbackMgr.
static_assert(std::is_same<chip::NodeId, uint64_t>::value, "chip::NodeId is not uint64_t");
chip::NodeId transactionId = reinterpret_cast<chip::NodeId>(static_cast<app::Command *>(mCommandSender));
chip::NodeId transactionId = reinterpret_cast<chip::NodeId>(commandObj);
mCallbacksMgr.AddResponseCallback(transactionId, 0 /* seqNum, always 0 for IM before #6559 */, onSuccessCallback,
onFailureCallback);
}

void Device::CancelIMResponseHandler()
void Device::CancelIMResponseHandler(app::Command * commandObj)
{
// We are using the pointer to command sender object as the identifier of command transactions. This makes sense as long as
// there are only one active command transaction on one command sender object. This is a bit tricky, we try to assume that
// chip::NodeId is uint64_t so the pointer can be used as a NodeId for CallbackMgr.
static_assert(std::is_same<chip::NodeId, uint64_t>::value, "chip::NodeId is not uint64_t");
chip::NodeId transactionId = reinterpret_cast<chip::NodeId>(static_cast<app::Command *>(mCommandSender));
chip::NodeId transactionId = reinterpret_cast<chip::NodeId>(commandObj);
mCallbacksMgr.CancelResponseCallback(transactionId, 0 /* seqNum, always 0 for IM before #6559 */);
}

Expand All @@ -469,16 +470,5 @@ void Device::AddReportHandler(EndpointId endpoint, ClusterId cluster, AttributeI
mCallbacksMgr.AddReportCallback(mDeviceId, endpoint, cluster, attribute, onReportCallback);
}

void Device::InitCommandSender()
{
if (mCommandSender != nullptr)
{
mCommandSender->Shutdown();
mCommandSender = nullptr;
}
CHIP_ERROR err = chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&mCommandSender);
ChipLogFunctError(err);
}

} // namespace Controller
} // namespace chip
Loading

0 comments on commit 1205755

Please sign in to comment.