Skip to content

Commit

Permalink
Add initial cluster status support
Browse files Browse the repository at this point in the history
  • Loading branch information
yunhanw-google committed Oct 26, 2021
1 parent 341c00d commit a2892a0
Show file tree
Hide file tree
Showing 24 changed files with 282 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ EmberAfStatus ExampleOTARequestor::HandleAnnounceOTAProvider(
}

mProviderNodeId = providerLocation;
mProviderFabricIndex = commandObj->GetExchangeContext()->GetSecureSession().GetFabricIndex();
mProviderFabricIndex = commandObj->GetExchangeContext()->GetSessionHandle().GetFabricIndex();

FabricInfo * providerFabric = GetProviderFabricInfo();
if (providerFabric == nullptr)
Expand Down
4 changes: 2 additions & 2 deletions src/app/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ class Command
return CHIP_ERROR_NOT_IMPLEMENTED;
}

virtual CHIP_ERROR AddClusterSpecificSuccess(ConcreteCommandPath & aCommandPath, uint8_t aClusterStatus)
virtual CHIP_ERROR AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}

virtual CHIP_ERROR AddClusterSpecificFailure(ConcreteCommandPath & aCommandPath, uint8_t aClusterStatus)
virtual CHIP_ERROR AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
}
Expand Down
25 changes: 23 additions & 2 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
return CHIP_NO_ERROR;
}

CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus)
CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aCommandPath,
const Protocols::InteractionModel::Status aStatus,
const Optional<ClusterStatus> & aClusterStatus)
{
CHIP_ERROR err = CHIP_NO_ERROR;
StatusIB::Builder statusIBBuilder;
Expand All @@ -174,7 +176,8 @@ CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, c
// above is always an IM code. Instead of fixing all the callers (which is a fairly sizeable change), we'll embark on fixing
// this more completely when we fix #9530.
//
statusIB.mStatus = aStatus;
statusIB.mStatus = aStatus;
statusIB.mClusterStatus = aClusterStatus;
statusIBBuilder.EncodeStatusIB(statusIB);
err = statusIBBuilder.GetError();
SuccessOrExit(err);
Expand All @@ -185,6 +188,24 @@ CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, c
return err;
}

CHIP_ERROR CommandHandler::AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus)
{
Optional<ClusterStatus> clusterStatus = Optional<ClusterStatus>::Missing();
return AddStatusInternal(aCommandPath, aStatus, clusterStatus);
}

CHIP_ERROR CommandHandler::AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
{
Optional<ClusterStatus> clusterStatus(aClusterStatus);
return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Success, clusterStatus);
}

CHIP_ERROR CommandHandler::AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus)
{
Optional<ClusterStatus> clusterStatus(aClusterStatus);
return AddStatusInternal(aCommandPath, Protocols::InteractionModel::Status::Failure, clusterStatus);
}

CHIP_ERROR CommandHandler::PrepareResponse(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommand)
{
CommandPathParams params = { aRequestCommandPath.mEndpointId,
Expand Down
7 changes: 7 additions & 0 deletions src/app/CommandHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class CommandHandler : public Command
System::PacketBufferHandle && payload);
CHIP_ERROR AddStatus(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus) override;

CHIP_ERROR AddClusterSpecificSuccess(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus) override;

CHIP_ERROR AddClusterSpecificFailure(const ConcreteCommandPath & aCommandPath, ClusterStatus aClusterStatus) override;

/**
* API for adding a data response. The template parameter T is generally
* expected to be a ClusterName::Commands::CommandName::Type struct, but any
Expand Down Expand Up @@ -107,6 +111,9 @@ class CommandHandler : public Command
CHIP_ERROR SendCommandResponse();
CHIP_ERROR ProcessCommandDataIB(CommandDataIB::Parser & aCommandElement) override;
CHIP_ERROR PrepareResponse(const ConcreteCommandPath & aRequestCommandPath, CommandId aResponseCommand);
CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
const Optional<ClusterStatus> & aClusterStatus);

Callback * mpCallback = nullptr;
};
} // namespace app
Expand Down
16 changes: 10 additions & 6 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha
{
if (err != CHIP_NO_ERROR)
{
mpCallback->OnError(this, Protocols::InteractionModel::Status::Failure, err);
StatusIB status;
status.mStatus = Protocols::InteractionModel::Status::Failure;
mpCallback->OnError(this, status, err);
}
}

Expand All @@ -95,7 +97,9 @@ void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeCon

if (mpCallback != nullptr)
{
mpCallback->OnError(this, Protocols::InteractionModel::Status::Failure, CHIP_ERROR_TIMEOUT);
StatusIB status;
status.mStatus = Protocols::InteractionModel::Status::Failure;
mpCallback->OnError(this, status, CHIP_ERROR_TIMEOUT);
}

Close();
Expand All @@ -119,6 +123,8 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
chip::ClusterId clusterId;
chip::CommandId commandId;
chip::EndpointId endpointId;
// Default to success when an invoke response is received.
StatusIB statusIB;

{
CommandPathIB::Parser commandPath;
Expand All @@ -140,8 +146,6 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
bool hasDataResponse = false;
chip::TLV::TLVReader commandDataReader;

// Default to success when an invoke response is received.
StatusIB statusIB;
StatusIB::Parser statusIBParser;
err = aCommandElement.GetStatusIB(&statusIBParser);
if (CHIP_NO_ERROR == err)
Expand Down Expand Up @@ -182,12 +186,12 @@ CHIP_ERROR CommandSender::ProcessCommandDataIB(CommandDataIB::Parser & aCommandE
{
if (statusIB.mStatus == Protocols::InteractionModel::Status::Success)
{
mpCallback->OnResponse(this, ConcreteCommandPath(endpointId, clusterId, commandId),
mpCallback->OnResponse(this, ConcreteCommandPath(endpointId, clusterId, commandId), statusIB,
hasDataResponse ? &commandDataReader : nullptr);
}
else
{
mpCallback->OnError(this, statusIB.mStatus, CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
mpCallback->OnError(this, statusIB, CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
}
}
}
Expand Down
24 changes: 13 additions & 11 deletions src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,16 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate
* The CommandSender object MUST continue to exist after this call is completed. The application shall wait until it
* receives an OnDone call to destroy the object.
*
* @param[in] apCommandSender The command sender object that initiated the command transaction.
* @param[in] aPath The command path field in invoke command response.
* @param[in] aData The command data, will be nullptr if the server returns a StatusIB.
* @param[in] apCommandSender: The command sender object that initiated the command transaction.
* @param[in] aPath: The command path field in invoke command response.
* @param[in] aStatusIB: It will always have a success status. If apData is null, it can be any success status, including
* possibly a cluster-specific one. If apData is not null it aStatusIB will always be a generic
* SUCCESS status with no-cluster specific information.
* @param[in] aData: The command data, will be nullptr if the server returns a StatusIB.
*/
virtual void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, TLV::TLVReader * aData) {}
virtual void OnResponse(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, const StatusIB & aStatusIB,
TLV::TLVReader * apData)
{}

/**
* OnError will be called when an error occurr *after* a successful call to SendCommandRequest(). The following
Expand All @@ -84,14 +89,11 @@ class CommandSender final : public Command, public Messaging::ExchangeDelegate
* The CommandSender object MUST continue to exist after this call is completed. The application shall wait until it
* receives an OnDone call to destroy and free the object.
*
* @param[in] apCommandSender The command sender object that initiated the command transaction.
* @param[in] aInteractionModelStatus Contains an IM status code. This SHALL never be IM::Success, and will contain a valid
* server-side emitted error if aProtocolError == CHIP_ERROR_IM_STATUS_CODE_RECEIVED.
* @param[in] aError A system error code that conveys the overall error code.
* @param[in] apCommandSender: The command sender object that initiated the command transaction.
* @param[in] aStatusIB: The status code including IM status code and optional cluster status code
* @param[in] aError: A system error code that conveys the overall error code.
*/
virtual void OnError(const CommandSender * apCommandSender, Protocols::InteractionModel::Status aInteractionModelStatus,
CHIP_ERROR aError)
{}
virtual void OnError(const CommandSender * apCommandSender, const StatusIB & aStatusIB, CHIP_ERROR aError) {}

/**
* OnDone will be called when CommandSender has finished all work and is safe to destory and free the
Expand Down
8 changes: 4 additions & 4 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,18 @@ class MockCommandSenderCallback : public CommandSender::Callback
{
public:
void OnResponse(chip::app::CommandSender * apCommandSender, const chip::app::ConcreteCommandPath & aPath,
chip::TLV::TLVReader * aData) override
const chip::app::StatusIB & aStatus, chip::TLV::TLVReader * aData) override
{
IgnoreUnusedVariable(apCommandSender);
IgnoreUnusedVariable(aData);
ChipLogDetail(Controller, "Received Cluster Command: Cluster=%" PRIx32 " Command=%" PRIx32 " Endpoint=%" PRIx16,
aPath.mClusterId, aPath.mCommandId, aPath.mEndpointId);
onResponseCalledTimes++;
}
void OnError(const chip::app::CommandSender * apCommandSender, chip::Protocols::InteractionModel::Status aStatus,
CHIP_ERROR aError) override
void OnError(const chip::app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus, CHIP_ERROR aError) override
{
ChipLogError(Controller, "OnError happens with %" PRIx16 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus), aError.Format());
ChipLogError(Controller, "OnError happens with %" PRIx16 " %" CHIP_ERROR_FORMAT, to_underlying(aStatus.mStatus),
aError.Format());
onErrorCalledTimes++;
}
void OnDone(chip::app::CommandSender * apCommandSender) override { onFinalCalledTimes++; }
Expand Down
5 changes: 2 additions & 3 deletions src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate,
}

void OnResponse(chip::app::CommandSender * apCommandSender, const chip::app::ConcreteCommandPath & aPath,
chip::TLV::TLVReader * aData) override
const chip::app::StatusIB & aStatus, chip::TLV::TLVReader * aData) override
{
printf("Command Response Success with EndpointId %d, ClusterId %d, CommandId %d", aPath.mEndpointId, aPath.mClusterId,
aPath.mCommandId);
Expand All @@ -178,8 +178,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate,
static_cast<double>(gCommandRespCount) * 100 / static_cast<double>(gCommandCount),
static_cast<double>(transitTime) / 1000);
}
void OnError(const chip::app::CommandSender * apCommandSender, chip::Protocols::InteractionModel::Status aProtocolCode,
CHIP_ERROR aError) override
void OnError(const chip::app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus, CHIP_ERROR aError) override
{
gCommandRespCount += (aError == CHIP_ERROR_IM_STATUS_CODE_RECEIVED);
gLastCommandResult = TestCommandResult::kFailure;
Expand Down
6 changes: 3 additions & 3 deletions src/app/zap-templates/templates/app/CHIPClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,12 @@ CHIP_ERROR ClusterBase::InvokeCommand(const RequestDataT & requestData, void * c
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(mDevice->LoadSecureSessionParametersIfNeeded());

auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const ResponseDataT & responseData) {
auto onSuccessCb = [context, successCb](const app::ConcreteCommandPath & commandPath, const app::StatusIB & aStatus, const ResponseDataT & responseData) {
successCb(context, responseData);
};

auto onFailureCb = [context, failureCb](Protocols::InteractionModel::Status aIMStatus, CHIP_ERROR aError) {
failureCb(context, app::ToEmberAfStatus(aIMStatus));
auto onFailureCb = [context, failureCb](const app::StatusIB & aStatus, CHIP_ERROR aError) {
failureCb(context, app::ToEmberAfStatus(aStatus.mStatus));
};

return InvokeCommandRequest<ResponseDataT>(mDevice->GetExchangeManager(), mDevice->GetSecureSession().Value(), mEndpoint,
Expand Down
7 changes: 4 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,8 @@ void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNo
#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY

void DeviceControllerInteractionModelDelegate::OnResponse(app::CommandSender * apCommandSender,
const app::ConcreteCommandPath & aPath, TLV::TLVReader * aData)
const app::ConcreteCommandPath & aPath,
const chip::app::StatusIB & aStatus, TLV::TLVReader * aData)
{
// Generally IM has more detailed errors than ember library, here we always use the, the actual handling of the
// commands should implement full IMDelegate.
Expand All @@ -1687,7 +1688,7 @@ void DeviceControllerInteractionModelDelegate::OnResponse(app::CommandSender * a
}

void DeviceControllerInteractionModelDelegate::OnError(const app::CommandSender * apCommandSender,
Protocols::InteractionModel::Status aClusterStatus, CHIP_ERROR aError)
const chip::app::StatusIB & aStatus, CHIP_ERROR aError)
{
// The IMDefaultResponseCallback started out life as an Ember function, so it only accepted
// Ember status codes. Consequently, let's convert the IM code over to a meaningful Ember status before dispatching.
Expand All @@ -1696,7 +1697,7 @@ void DeviceControllerInteractionModelDelegate::OnError(const app::CommandSender
// well, this will be an even bigger problem.
//
// For now, #10331 tracks this issue.
IMDefaultResponseCallback(apCommandSender, app::ToEmberAfStatus(aClusterStatus));
IMDefaultResponseCallback(apCommandSender, app::ToEmberAfStatus(aStatus.mStatus));
}

void DeviceControllerInteractionModelDelegate::OnDone(app::CommandSender * apCommandSender)
Expand Down
5 changes: 3 additions & 2 deletions src/controller/DeviceControllerInteractionModelDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ class DeviceControllerInteractionModelDelegate : public chip::app::InteractionMo
public chip::app::WriteClient::Callback
{
public:
void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aPath, TLV::TLVReader * aData) override;
void OnError(const app::CommandSender * apCommandSender, Protocols::InteractionModel::Status aInteractionModelStatus,
void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aPath,
const chip::app::StatusIB & aStatus, TLV::TLVReader * aData) override;
void OnError(const app::CommandSender * apCommandSender, const chip::app::StatusIB & aStatus,
CHIP_ERROR aProtocolError) override;
void OnDone(app::CommandSender * apCommandSender) override;

Expand Down
Loading

0 comments on commit a2892a0

Please sign in to comment.