Skip to content

Commit

Permalink
Improve IM Message encoding (#19016)
Browse files Browse the repository at this point in the history
  • Loading branch information
yunhanw-google authored and pull[bot] committed Aug 25, 2023
1 parent 5d174f8 commit 5002275
Show file tree
Hide file tree
Showing 66 changed files with 629 additions and 868 deletions.
6 changes: 3 additions & 3 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
return AddStatus(concretePath, Protocols::InteractionModel::Status::NeedsTimedInteraction);
}

err = aCommandElement.GetData(&commandDataReader);
err = aCommandElement.GetFields(&commandDataReader);
if (CHIP_END_OF_TLV == err)
{
ChipLogDetail(DataManagement,
Expand Down Expand Up @@ -359,7 +359,7 @@ CHIP_ERROR CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo
ChipLogDetail(DataManagement, "Received group command for Group=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
groupId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId));

err = aCommandElement.GetData(&commandDataReader);
err = aCommandElement.GetFields(&commandDataReader);
if (CHIP_END_OF_TLV == err)
{
ChipLogDetail(DataManagement,
Expand Down Expand Up @@ -488,7 +488,7 @@ CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aCommandPa
ReturnErrorOnFailure(path.Encode(aCommandPath));
if (aStartDataStruct)
{
ReturnErrorOnFailure(commandData.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)),
ReturnErrorOnFailure(commandData.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)),
TLV::kTLVType_Structure, mDataElementContainerType));
}
MoveToState(State::AddingCommand);
Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ class CommandHandler
ReturnErrorOnFailure(PrepareCommand(path, false));
TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), aData));
ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), aData));

return FinishCommand(/* aEndDataStruct = */ false);
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ CHIP_ERROR CommandSender::ProcessInvokeResponseIB(InvokeResponseIB::Parser & aIn
ReturnErrorOnFailure(commandPath.GetEndpointId(&endpointId));
ReturnErrorOnFailure(commandPath.GetClusterId(&clusterId));
ReturnErrorOnFailure(commandPath.GetCommandId(&commandId));
commandData.GetData(&commandDataReader);
commandData.GetFields(&commandDataReader);
err = CHIP_NO_ERROR;
hasDataResponse = true;
}
Expand Down Expand Up @@ -336,7 +336,7 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP

if (aStartDataStruct)
{
ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)),
ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)),
TLV::kTLVType_Structure, mDataElementContainerType));
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/CommandSender.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class CommandSender final : public Messaging::ExchangeDelegate
ReturnErrorOnFailure(PrepareCommand(aCommandPath, /* aStartDataStruct = */ false));
TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), aData));
ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(to_underlying(CommandDataIB::Tag::kFields)), aData));
return FinishCommand(aTimedInvokeTimeoutMs);
}

Expand Down
3 changes: 1 addition & 2 deletions src/app/MessageDef/ArrayParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ CHIP_ERROR ArrayParser::Init(const TLV::TLVReader & aReader)
{
mReader.Init(aReader);
VerifyOrReturnError(TLV::kTLVType_Array == mReader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
ReturnErrorOnFailure(mReader.EnterContainer(mOuterContainerType));
return CHIP_NO_ERROR;
return mReader.EnterContainer(mOuterContainerType);
}
} // namespace app
} // namespace chip
33 changes: 12 additions & 21 deletions src/app/MessageDef/AttributeDataIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const

if (aDepth == 0)
{
PRETTY_PRINT("\tData = ");
PRETTY_PRINT("Data = ");
}
else
{
Expand Down Expand Up @@ -187,7 +187,7 @@ AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const
CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributeDataIB =");
Expand All @@ -207,8 +207,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kDataVersion):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kDataVersion))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kDataVersion));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -221,8 +221,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kPath):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kPath));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kPath))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kPath));
{
AttributePathIB::Parser path;
ReturnErrorOnFailure(path.Init(reader));
Expand All @@ -234,8 +234,8 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kData):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kData));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kData))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kData));

PRETTY_PRINT_INCDEPTH();
ReturnErrorOnFailure(ParseData(reader, 0));
Expand All @@ -253,29 +253,20 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
if (CHIP_END_OF_TLV == err)
{
// check for required fields:
const int RequiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));
const int requiredFields = (1 << to_underlying(Tag::kPath)) | (1 << to_underlying(Tag::kData));

if ((TagPresenceMask & RequiredFields) == RequiredFields)
{
err = CHIP_NO_ERROR;
}
else
{
err = CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT;
}
err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_DATA_IB;
}
ReturnErrorOnFailure(err);
ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
return CHIP_NO_ERROR;
return reader.ExitContainer(mOuterContainerType);
}
#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK

CHIP_ERROR AttributeDataIB::Parser::GetPath(AttributePathIB::Parser * const apPath) const
{
TLV::TLVReader reader;
ReturnErrorOnFailure(mReader.FindElementWithTag(TLV::ContextTag(to_underlying(Tag::kPath)), reader));
ReturnErrorOnFailure(apPath->Init(reader));
return CHIP_NO_ERROR;
return apPath->Init(reader);
}

CHIP_ERROR AttributeDataIB::Parser::GetDataVersion(chip::DataVersion * const apVersion) const
Expand Down
24 changes: 9 additions & 15 deletions src/app/MessageDef/AttributeDataIBs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace app {
CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
size_t NumDataElement = 0;
size_t numDataElement = 0;
chip::TLV::TLVReader reader;

PRETTY_PRINT("AttributeDataIBs =");
Expand All @@ -51,21 +51,19 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const

while (CHIP_NO_ERROR == (err = reader.Next()))
{
VerifyOrExit(chip::TLV::AnonymousTag() == reader.GetTag(), err = CHIP_ERROR_INVALID_TLV_TAG);
VerifyOrExit(chip::TLV::kTLVType_Structure == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);
VerifyOrReturnError(TLV::AnonymousTag() == reader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG);
VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);

{
AttributeDataIB::Parser data;
err = data.Init(reader);
SuccessOrExit(err);
ReturnErrorOnFailure(data.Init(reader));

PRETTY_PRINT_INCDEPTH();
err = data.CheckSchemaValidity();
SuccessOrExit(err);
ReturnErrorOnFailure(data.CheckSchemaValidity());
PRETTY_PRINT_DECDEPTH();
}

++NumDataElement;
++numDataElement;
}

PRETTY_PRINT("],");
Expand All @@ -75,17 +73,13 @@ CHIP_ERROR AttributeDataIBs::Parser::CheckSchemaValidity() const
if (CHIP_END_OF_TLV == err)
{
// if we have at least one data element
if (NumDataElement > 0)
if (numDataElement > 0)
{
err = CHIP_NO_ERROR;
}
}
SuccessOrExit(err);
err = reader.ExitContainer(mOuterContainerType);

exit:

return err;
ReturnErrorOnFailure(err);
return reader.ExitContainer(mOuterContainerType);
}
#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK

Expand Down
37 changes: 18 additions & 19 deletions src/app/MessageDef/AttributePathIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace app {
CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
{
CHIP_ERROR err = CHIP_NO_ERROR;
int TagPresenceMask = 0;
int tagPresenceMask = 0;
TLV::TLVReader reader;

PRETTY_PRINT("AttributePathIB =");
Expand All @@ -53,8 +53,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
{
case to_underlying(Tag::kEnableTagCompression):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEnableTagCompression))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kEnableTagCompression));
#if CHIP_DETAIL_LOGGING
{
bool enableTagCompression;
Expand All @@ -66,8 +66,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
case to_underlying(Tag::kNode):
// check if this tag has appeared before

VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kNode));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kNode))), err = CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kNode));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -80,8 +80,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kEndpoint):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kEndpoint))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kEndpoint));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
{
Expand All @@ -93,8 +93,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kCluster):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kCluster));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kCluster))), err = CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kCluster));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE);

#if CHIP_DETAIL_LOGGING
Expand All @@ -107,8 +107,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kAttribute):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kAttribute))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kAttribute));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
{
Expand All @@ -120,8 +120,8 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
break;
case to_underlying(Tag::kListIndex):
// check if this tag has appeared before
VerifyOrReturnError(!(TagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
TagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kListIndex))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kListIndex));
VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType() || TLV::kTLVType_Null == reader.GetType(),
CHIP_ERROR_WRONG_TLV_TYPE);
#if CHIP_DETAIL_LOGGING
Expand Down Expand Up @@ -149,10 +149,10 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
// if we have exhausted this container
if (CHIP_END_OF_TLV == err)
{
if ((TagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
(TagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
if ((tagPresenceMask & (1 << to_underlying(Tag::kAttribute))) == 0 &&
(tagPresenceMask & (1 << to_underlying(Tag::kListIndex))) != 0)
{
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
}
else
{
Expand All @@ -161,8 +161,7 @@ CHIP_ERROR AttributePathIB::Parser::CheckSchemaValidity() const
}

ReturnErrorOnFailure(err);
ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
return CHIP_NO_ERROR;
return reader.ExitContainer(mOuterContainerType);
}
#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK

Expand Down Expand Up @@ -215,7 +214,7 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAt
else
{
// TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB;
}
}
else if (CHIP_END_OF_TLV == err)
Expand Down
3 changes: 1 addition & 2 deletions src/app/MessageDef/AttributePathIBs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ CHIP_ERROR AttributePathIBs::Parser::CheckSchemaValidity() const
}
}
ReturnErrorOnFailure(err);
ReturnErrorOnFailure(reader.ExitContainer(mOuterContainerType));
return CHIP_NO_ERROR;
return reader.ExitContainer(mOuterContainerType);
}
#endif // CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK

Expand Down
Loading

0 comments on commit 5002275

Please sign in to comment.