Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup status element in IM #5721

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/app/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,16 @@ CHIP_ERROR Command::AddCommand(CommandParams & aCommandParams)
return err;
}

CHIP_ERROR Command::AddStatusCode(const uint16_t aGeneralCode, Protocols::Id aProtocolId, const uint16_t aProtocolCode,
const chip::ClusterId aClusterId)
CHIP_ERROR Command::AddStatusCode(const Protocols::SecureChannel::GeneralStatusCode aGeneralCode, Protocols::Id aProtocolId,
const uint16_t aProtocolCode)
{
CHIP_ERROR err = CHIP_NO_ERROR;
StatusElement::Builder statusElementBuilder;

err = statusElementBuilder.Init(mInvokeCommandBuilder.GetWriter());
SuccessOrExit(err);

statusElementBuilder.EncodeStatusElement(aGeneralCode, aProtocolId.ToFullyQualifiedSpecForm(), aProtocolCode, aProtocolCode)
statusElementBuilder.EncodeStatusElement(aGeneralCode, aProtocolId.ToFullyQualifiedSpecForm(), aProtocolCode)
.EndOfStatusElement();
err = statusElementBuilder.GetError();

Expand Down
4 changes: 2 additions & 2 deletions src/app/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ class Command
CHIP_ERROR AddCommand(chip::EndpointId aEndpintId, chip::GroupId aGroupId, chip::ClusterId aClusterId,
chip::CommandId aCommandId, BitFlags<CommandPathFlags> Flags);
CHIP_ERROR AddCommand(CommandParams & aCommandParams);
CHIP_ERROR AddStatusCode(const uint16_t aGeneralCode, Protocols::Id aProtocolId, const uint16_t aProtocolCode,
const chip::ClusterId aClusterId);
CHIP_ERROR AddStatusCode(const Protocols::SecureChannel::GeneralStatusCode aGeneralCode, Protocols::Id aProtocolId,
const uint16_t aProtocolCode);

/**
* Gets the inner exchange context object, without ownership.
Expand Down
3 changes: 1 addition & 2 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataElement(CommandDataElement::Parser
// Empty Command, Add status code in invoke command response, notify cluster handler to hand it further.
err = CHIP_NO_ERROR;
ChipLogDetail(DataManagement, "Add Status code for empty command, cluster Id is %d", clusterId);
AddStatusCode(static_cast<uint16_t>(GeneralStatusCode::kSuccess), Protocols::SecureChannel::Id,
Protocols::SecureChannel::kProtocolCodeSuccess, clusterId);
AddStatusCode(GeneralStatusCode::kSuccess, Protocols::SecureChannel::Id, Protocols::SecureChannel::kProtocolCodeSuccess);
}
else if (CHIP_NO_ERROR == err)
{
Expand Down
16 changes: 5 additions & 11 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser &
chip::ClusterId clusterId;
chip::CommandId commandId;
chip::EndpointId endpointId;
uint16_t generalCode = 0;
uint32_t protocolId = 0;
uint16_t protocolCode = 0;
Protocols::SecureChannel::GeneralStatusCode generalCode = Protocols::SecureChannel::GeneralStatusCode::kSuccess;
uint32_t protocolId = 0;
uint16_t protocolCode = 0;
StatusElement::Parser statusElementParser;

mCommandIndex++;
Expand All @@ -144,7 +144,7 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser &
err = statusElementParser.CheckSchemaValidity();
SuccessOrExit(err);

err = statusElementParser.DecodeStatusElement(&generalCode, &protocolId, &protocolCode, &clusterId);
err = statusElementParser.DecodeStatusElement(&generalCode, &protocolId, &protocolCode);
SuccessOrExit(err);
if (mpDelegate != nullptr)
{
Expand All @@ -155,13 +155,7 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser &
else if (CHIP_END_OF_TLV == err)
{
err = aCommandElement.GetData(&commandDataReader);
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
ChipLogDetail(DataManagement, "Add Status code for empty command, cluster Id is %d", clusterId);
AddStatusCode(static_cast<uint16_t>(GeneralStatusCode::kSuccess), Protocols::SecureChannel::Id,
Protocols::SecureChannel::kProtocolCodeSuccess, clusterId);
}
SuccessOrExit(err);
// TODO(#4503): Should call callbacks of cluster that sends the command.
DispatchSingleClusterCommand(clusterId, commandId, endpointId, commandDataReader, this);
}
Expand Down
3 changes: 2 additions & 1 deletion src/app/InteractionModelDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ class InteractionModelDelegate
* same command Id
* @retval # CHIP_ERROR_NOT_IMPLEMENTED if not implemented
*/
virtual CHIP_ERROR CommandResponseStatus(const CommandSender * apCommandSender, const uint16_t aGeneralCode,
virtual CHIP_ERROR CommandResponseStatus(const CommandSender * apCommandSender,
const Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const uint32_t aProtocolId, const uint16_t aProtocolCode, chip::EndpointId aEndpointId,
const chip::ClusterId aClusterId, chip::CommandId aCommandId, uint8_t aCommandIndex)
{
Expand Down
26 changes: 9 additions & 17 deletions src/app/MessageDef/StatusElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,21 @@ CHIP_ERROR StatusElement::Parser::Init(const chip::TLV::TLVReader & aReader)
return err;
}

CHIP_ERROR StatusElement::Parser::DecodeStatusElement(uint16_t * apGeneralCode, uint32_t * apProtocolId, uint16_t * apProtocolCode,
chip::ClusterId * apClusterId) const
CHIP_ERROR StatusElement::Parser::DecodeStatusElement(Protocols::SecureChannel::GeneralStatusCode * apGeneralCode,
uint32_t * apProtocolId, uint16_t * apProtocolCode) const
{
CHIP_ERROR err = CHIP_NO_ERROR;
chip::TLV::TLVReader lReader;

uint16_t generalCode;
lReader.Init(mReader);

err = lReader.Next();
SuccessOrExit(err);
VerifyOrExit(lReader.GetType() == chip::TLV::kTLVType_UnsignedInteger, err = CHIP_ERROR_WRONG_TLV_TYPE);
err = lReader.Get(*apGeneralCode);
err = lReader.Get(generalCode);
SuccessOrExit(err);
*apGeneralCode = static_cast<Protocols::SecureChannel::GeneralStatusCode>(generalCode);

err = lReader.Next();
SuccessOrExit(err);
Expand All @@ -77,12 +79,6 @@ CHIP_ERROR StatusElement::Parser::DecodeStatusElement(uint16_t * apGeneralCode,
err = lReader.Get(*apProtocolCode);
SuccessOrExit(err);

err = lReader.Next();
SuccessOrExit(err);
VerifyOrExit(lReader.GetType() == chip::TLV::kTLVType_UnsignedInteger, err = CHIP_ERROR_WRONG_TLV_TYPE);
err = lReader.Get(*apClusterId);
SuccessOrExit(err);

exit:
ChipLogFunctError(err);
return err;
Expand Down Expand Up @@ -208,24 +204,20 @@ CHIP_ERROR StatusElement::Builder::Init(chip::TLV::TLVWriter * const apWriter, c
return ListBuilder::Init(apWriter, aContextTagToUse);
}

StatusElement::Builder & StatusElement::Builder::EncodeStatusElement(const uint16_t aGeneralCode, const uint32_t aProtocolId,
const uint16_t aStatusElement,
const chip::ClusterId aClusterId)
StatusElement::Builder & StatusElement::Builder::EncodeStatusElement(const Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const uint32_t aProtocolId, const uint16_t aProtocolCode)
{
uint64_t tag = chip::TLV::AnonymousTag;

SuccessOrExit(mError);

mError = mpWriter->Put(tag, aGeneralCode);
mError = mpWriter->Put(tag, static_cast<uint16_t>(aGeneralCode));
SuccessOrExit(mError);

mError = mpWriter->Put(tag, aProtocolId);
SuccessOrExit(mError);

mError = mpWriter->Put(tag, aStatusElement);
SuccessOrExit(mError);

mError = mpWriter->Put(tag, aClusterId);
mError = mpWriter->Put(tag, aProtocolCode);
SuccessOrExit(mError);

exit:
Expand Down
10 changes: 5 additions & 5 deletions src/app/MessageDef/StatusElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

#include <core/CHIPCore.h>
#include <core/CHIPTLV.h>
#include <protocols/secure_channel/Constants.h>
#include <support/CodeUtils.h>
#include <support/logging/CHIPLogging.h>
#include <util/basic-types.h>
Expand Down Expand Up @@ -85,8 +86,8 @@ class Parser : public ListParser
* element is missing. CHIP_ERROR_WRONG_TLV_TYPE if the elements are of the wrong
* type.
*/
CHIP_ERROR DecodeStatusElement(uint16_t * apGeneralCode, uint32_t * apProtocolId, uint16_t * apProtocolCode,
chip::ClusterId * apClusterId) const;
CHIP_ERROR DecodeStatusElement(Protocols::SecureChannel::GeneralStatusCode * apGeneralCode, uint32_t * apProtocolId,
uint16_t * apProtocolCode) const;
};

class Builder : public ListBuilder
Expand Down Expand Up @@ -118,14 +119,13 @@ class Builder : public ListBuilder
* @param[in] aGeneralCode General status code
* @param[in] aProtocolId A protocol ID (32-bit integer composed of a 16-bit vendor id and 16-bit Scoped id)
* @param[in] aProtocolCode 16-bit protocol-specific error code
* @param[in] aClusterId Cluster Id for ZCL
*
* @return CHIP_ERROR codes returned by chip::TLV objects. CHIP_END_OF_TLV if either
* element is missing. CHIP_ERROR_WRONG_TLV_TYPE if the elements are of the wrong
* type.
*/
StatusElement::Builder & EncodeStatusElement(const uint16_t aGeneralCode, const uint32_t aProtocolId,
const uint16_t aProtocolCode, const chip::ClusterId aClusterId);
StatusElement::Builder & EncodeStatusElement(const Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const uint32_t aProtocolId, const uint16_t aProtocolCode);

/**
* @brief Mark the end of this StatusElement
Expand Down
18 changes: 11 additions & 7 deletions src/app/tests/TestMessageDef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ void BuildStatusElement(nlTestSuite * apSuite, StatusElement::Builder & aStatusE
{
CHIP_ERROR err = CHIP_NO_ERROR;

aStatusElementBuilder.EncodeStatusElement(1, 2, 3, 4).EndOfStatusElement();
aStatusElementBuilder.EncodeStatusElement(chip::Protocols::SecureChannel::GeneralStatusCode::kFailure, 2, 3)
.EndOfStatusElement();
err = aStatusElementBuilder.GetError();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
}
Expand All @@ -334,16 +335,19 @@ void ParseStatusElement(nlTestSuite * apSuite, StatusElement::Parser & aStatusEl
CHIP_ERROR err = CHIP_NO_ERROR;
StatusElement::Parser StatusElementParser;

uint16_t generalCode = 0;
uint32_t protocolId = 0;
uint16_t protocolCode = 0;
chip::ClusterId clusterId = 0;
chip::Protocols::SecureChannel::GeneralStatusCode generalCode = chip::Protocols::SecureChannel::GeneralStatusCode::kFailure;
uint32_t protocolId = 0;
uint16_t protocolCode = 0;

err = aStatusElementParser.CheckSchemaValidity();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

err = aStatusElementParser.DecodeStatusElement(&generalCode, &protocolId, &protocolCode, &clusterId);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && generalCode == 1 && protocolId == 2 && protocolCode == 3 && clusterId == 4);
err = aStatusElementParser.DecodeStatusElement(&generalCode, &protocolId, &protocolCode);
NL_TEST_ASSERT(apSuite,
err == CHIP_NO_ERROR &&
static_cast<uint16_t>(generalCode) ==
static_cast<uint16_t>(chip::Protocols::SecureChannel::GeneralStatusCode::kFailure) &&
protocolId == 2 && protocolCode == 3);
}

void BuildAttributeStatusElement(nlTestSuite * apSuite, AttributeStatusElement::Builder & aAttributeStatusElementBuilder)
Expand Down
5 changes: 3 additions & 2 deletions src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,15 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
printf("ReportError with err %d", aError);
return CHIP_NO_ERROR;
}
CHIP_ERROR CommandResponseStatus(const chip::app::CommandSender * apCommandSender, const uint16_t aGeneralCode,
CHIP_ERROR CommandResponseStatus(const chip::app::CommandSender * apCommandSender,
const chip::Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const uint32_t aProtocolId, const uint16_t aProtocolCode, const chip::EndpointId aEndpointId,
const chip::ClusterId aClusterId, const chip::CommandId aCommandId,
uint8_t aCommandIndex) override
{
printf("CommandResponseStatus with GeneralCode %d, ProtocolId %d, ProtocolCode %d, EndpointId %d, ClusterId %d, CommandId "
"%d, CommandIndex %d",
aGeneralCode, aProtocolId, aProtocolCode, aEndpointId, aClusterId, aCommandId, aCommandIndex);
static_cast<uint16_t>(aGeneralCode), aProtocolId, aProtocolCode, aEndpointId, aClusterId, aCommandId, aCommandIndex);
return CHIP_NO_ERROR;
}

Expand Down