Skip to content

Commit

Permalink
Fix wrong IM statusCode container (#6302)
Browse files Browse the repository at this point in the history
Problems:
-- IM status code in command data element has been wrapped with one extra container

Summary of Changes:
-- Remove one extra container for IM status code in command data element
via adding one additional parameter, isStatusCode in PrepareCommand and FinishCommand
  • Loading branch information
yunhanw-google authored and pull[bot] committed Apr 27, 2021
1 parent 1b250e3 commit 2212920
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
18 changes: 12 additions & 6 deletions src/app/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void Command::Shutdown()
return;
}

CHIP_ERROR Command::PrepareCommand(const CommandPathParams * const apCommandPathParams)
CHIP_ERROR Command::PrepareCommand(const CommandPathParams * const apCommandPathParams, bool aIsStatus)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -159,8 +159,11 @@ CHIP_ERROR Command::PrepareCommand(const CommandPathParams * const apCommandPath
SuccessOrExit(err);
}

err = commandDataElement.GetWriter()->StartContainer(TLV::ContextTag(CommandDataElement::kCsTag_Data), TLV::kTLVType_Structure,
mDataElementContainerType);
if (!aIsStatus)
{
err = commandDataElement.GetWriter()->StartContainer(TLV::ContextTag(CommandDataElement::kCsTag_Data),
TLV::kTLVType_Structure, mDataElementContainerType);
}
exit:
ChipLogFunctError(err);
return err;
Expand All @@ -171,13 +174,16 @@ TLV::TLVWriter * Command::GetCommandDataElementTLVWriter()
return mInvokeCommandBuilder.GetCommandListBuilder().GetCommandDataElementBuilder().GetWriter();
}

CHIP_ERROR Command::FinishCommand()
CHIP_ERROR Command::FinishCommand(bool aIsStatus)
{
CHIP_ERROR err = CHIP_NO_ERROR;

CommandDataElement::Builder commandDataElement = mInvokeCommandBuilder.GetCommandListBuilder().GetCommandDataElementBuilder();
err = commandDataElement.GetWriter()->EndContainer(mDataElementContainerType);
SuccessOrExit(err);
if (!aIsStatus)
{
err = commandDataElement.GetWriter()->EndContainer(mDataElementContainerType);
SuccessOrExit(err);
}
commandDataElement.EndOfCommandDataElement();
err = commandDataElement.GetError();
SuccessOrExit(err);
Expand Down
4 changes: 2 additions & 2 deletions src/app/Command.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ class Command
*/
CHIP_ERROR FinalizeCommandsMessage();

CHIP_ERROR PrepareCommand(const CommandPathParams * const apCommandPathParams);
CHIP_ERROR PrepareCommand(const CommandPathParams * const apCommandPathParams, bool aIsStatus = false);
TLV::TLVWriter * GetCommandDataElementTLVWriter();
CHIP_ERROR FinishCommand();
CHIP_ERROR FinishCommand(bool aIsStatus = false);
virtual CHIP_ERROR AddStatusCode(const CommandPathParams * apCommandPathParams,
const Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const Protocols::Id aProtocolId, const uint16_t aProtocolCode)
Expand Down
4 changes: 2 additions & 2 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ CHIP_ERROR CommandHandler::AddStatusCode(const CommandPathParams * apCommandPath
CHIP_ERROR err = CHIP_NO_ERROR;
StatusElement::Builder statusElementBuilder;

err = PrepareCommand(apCommandPathParams);
err = PrepareCommand(apCommandPathParams, true /* isStatus */);
SuccessOrExit(err);

statusElementBuilder =
Expand All @@ -131,7 +131,7 @@ CHIP_ERROR CommandHandler::AddStatusCode(const CommandPathParams * apCommandPath
err = statusElementBuilder.GetError();
SuccessOrExit(err);

err = FinishCommand();
err = FinishCommand(true /* isStatus */);

exit:
ChipLogFunctError(err);
Expand Down
16 changes: 14 additions & 2 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,20 @@ void TestCommandInteraction::ValidateCommandHandlerWithSendCommand(nlTestSuite *
commandHandler.mpExchangeCtx->SetDelegate(&delegate);

AddCommandDataElement(apSuite, apContext, &commandHandler, aNeedStatusCode, aIsEmptyResponse);
err = commandHandler.SendCommandResponse();
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_NOT_CONNECTED);
err = commandHandler.FinalizeCommandsMessage();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
chip::System::PacketBufferTLVReader reader;
InvokeCommand::Parser invokeCommandParser;
reader.Init(std::move(commandHandler.mCommandMessageBuf));
err = reader.Next();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = invokeCommandParser.Init(reader);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
err = invokeCommandParser.CheckSchemaValidity();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
#endif
}

void TestCommandInteraction::TestCommandHandlerWithSendSimpleCommandData(nlTestSuite * apSuite, void * apContext)
Expand Down

0 comments on commit 2212920

Please sign in to comment.