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.

Sign up for GitHub

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

Improve IM Message encoding #19016

Merged
merged 3 commits into from
Jun 2, 2022
Merged

Improve IM Message encoding #19016

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a403d87
Improve IM MessageDef
yunhanw-google May 26, 2022
971bad7
address comments
yunhanw-google Jun 1, 2022
f89f79f
address comments
yunhanw-google Jun 2, 2022
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/CommandHandler.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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