Skip to content

Commit

Permalink
Fix messagedef IM data value logging to be consistent. (#19569)
Browse files Browse the repository at this point in the history
There were a bunch of issues:
* Weird indentation for attribute data and event data
* No implementation of logging floating point values for attribute data
* Manual tabs inserted various places for command data instead of
  using the pretty print depth.

This fix consolidates the event/command/attribute logging (which was
all completely generic and almost identical except for the issues
above) into a single function that is called from all three places.

Verified that the following produce the same results as before or
better ones, in terms of the logging:

    ./chip-tool applicationbasic read application 17 3
    ./chip-tool mediaplayback fast-forward 17 3
    ./chip-tool mediaplayback read playback-speed 17 3
    ./chip-tool operationalcredentials read fabrics 17 0
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jun 23, 2022
1 parent 53386f0 commit 2297240
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 483 deletions.
157 changes: 1 addition & 156 deletions src/app/MessageDef/AttributeDataIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,161 +28,6 @@

namespace chip {
namespace app {
CHIP_ERROR
AttributeDataIB::Parser::ParseData(TLV::TLVReader & aReader, int aDepth) const
{
CHIP_ERROR err = CHIP_NO_ERROR;

if (aDepth == 0)
{
PRETTY_PRINT("Data = ");
}
else
{
if (TLV::IsContextTag(aReader.GetTag()))
{
PRETTY_PRINT("\t0x%" PRIx32 " = ", TLV::TagNumFromTag(aReader.GetTag()));
}
else if (TLV::IsProfileTag(aReader.GetTag()))
{
PRETTY_PRINT("\t0x%" PRIx32 "::0x%" PRIx32 " = ", TLV::ProfileIdFromTag(aReader.GetTag()),
TLV::TagNumFromTag(aReader.GetTag()));
}
else
{
// Anonymous tag, don't print anything
}
}

switch (aReader.GetType())
{
case TLV::kTLVType_Structure:
PRETTY_PRINT("\t{");
break;

case TLV::kTLVType_Array:
PRETTY_PRINT_SAMELINE("[");
PRETTY_PRINT("\t\t");
break;

case TLV::kTLVType_SignedInteger: {
int64_t value_s64;

ReturnErrorOnFailure(aReader.Get(value_s64));

PRETTY_PRINT_SAMELINE("%" PRId64 ", ", value_s64);
break;
}

case TLV::kTLVType_UnsignedInteger: {
uint64_t value_u64;

ReturnErrorOnFailure(aReader.Get(value_u64));

PRETTY_PRINT_SAMELINE("%" PRIu64 ", ", value_u64);
break;
}

case TLV::kTLVType_Boolean: {
bool value_b;

ReturnErrorOnFailure(aReader.Get(value_b));

PRETTY_PRINT_SAMELINE("%s, ", value_b ? "true" : "false");
break;
}

case TLV::kTLVType_UTF8String: {
char value_s[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];

err = aReader.GetString(value_s, sizeof(value_s));
VerifyOrReturnError(err == CHIP_NO_ERROR || err == CHIP_ERROR_BUFFER_TOO_SMALL, err);

if (err == CHIP_ERROR_BUFFER_TOO_SMALL)
{
PRETTY_PRINT_SAMELINE("... (byte string too long) ...");
err = CHIP_NO_ERROR;
}
else
{
PRETTY_PRINT_SAMELINE("\"%s\", ", value_s);
}
break;
}

case TLV::kTLVType_ByteString: {
uint8_t value_b[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
uint32_t len, readerLen;

readerLen = aReader.GetLength();

err = aReader.GetBytes(value_b, sizeof(value_b));
VerifyOrReturnError(err == CHIP_NO_ERROR || err == CHIP_ERROR_BUFFER_TOO_SMALL, err);

PRETTY_PRINT_SAMELINE("[");
PRETTY_PRINT("\t\t");

if (readerLen < sizeof(value_b))
{
len = readerLen;
}
else
{
len = sizeof(value_b);
}

if (err == CHIP_ERROR_BUFFER_TOO_SMALL)
{
PRETTY_PRINT_SAMELINE("... (byte string too long) ...");
}
else
{
for (size_t i = 0; i < len; i++)
{
PRETTY_PRINT_SAMELINE("0x%x, ", value_b[i]);
}
}

err = CHIP_NO_ERROR;
PRETTY_PRINT("\t]");
break;
}

case TLV::kTLVType_Null:
PRETTY_PRINT_SAMELINE("NULL");
break;

default:
PRETTY_PRINT_SAMELINE("--");
break;
}

if (aReader.GetType() == TLV::kTLVType_Structure || aReader.GetType() == TLV::kTLVType_Array)
{
const char terminating_char = (aReader.GetType() == TLV::kTLVType_Structure) ? '}' : ']';
TLV::TLVType type;

IgnoreUnusedVariable(terminating_char);

ReturnErrorOnFailure(aReader.EnterContainer(type));

while ((err = aReader.Next()) == CHIP_NO_ERROR)
{
PRETTY_PRINT_INCDEPTH();

ReturnErrorOnFailure(ParseData(aReader, aDepth + 1));

PRETTY_PRINT_DECDEPTH();
}

PRETTY_PRINT("\t%c,", terminating_char);

ReturnErrorOnFailure(aReader.ExitContainer(type));
}

return CHIP_NO_ERROR;
}

#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
{
Expand Down Expand Up @@ -238,7 +83,7 @@ CHIP_ERROR AttributeDataIB::Parser::CheckSchemaValidity() const
tagPresenceMask |= (1 << to_underlying(Tag::kData));

PRETTY_PRINT_INCDEPTH();
ReturnErrorOnFailure(ParseData(reader, 0));
ReturnErrorOnFailure(CheckIMPayload(reader, 0, "Data"));
PRETTY_PRINT_DECDEPTH();
break;
default:
Expand Down
4 changes: 0 additions & 4 deletions src/app/MessageDef/AttributeDataIB.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ class Parser : public StructParser
* #CHIP_END_OF_TLV if there is no such element
*/
CHIP_ERROR GetData(TLV::TLVReader * const apReader) const;

protected:
// A recursively callable function to parse a data element and pretty-print it.
CHIP_ERROR ParseData(TLV::TLVReader & aReader, int aDepth) const;
};

class Builder : public StructBuilder
Expand Down
174 changes: 3 additions & 171 deletions src/app/MessageDef/CommandDataIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,176 +28,6 @@

namespace chip {
namespace app {
CHIP_ERROR
CommandDataIB::Parser::ParseFields(TLV::TLVReader & aReader, int aDepth) const
{
CHIP_ERROR err = CHIP_NO_ERROR;

if (aDepth == 0)
{
PRETTY_PRINT("\tCommandFields = ");
}
else
{
if (TLV::IsContextTag(aReader.GetTag()))
{
PRETTY_PRINT("\t0x%" PRIx32 " = ", TLV::TagNumFromTag(aReader.GetTag()));
}
else if (TLV::IsProfileTag(aReader.GetTag()))
{
PRETTY_PRINT("\t0x%" PRIx32 "::0x%" PRIx32 " = ", TLV::ProfileIdFromTag(aReader.GetTag()),
TLV::TagNumFromTag(aReader.GetTag()));
}
else
{
// Anonymous tag, don't print anything
}
}

switch (aReader.GetType())
{
case TLV::kTLVType_Structure:
PRETTY_PRINT("\t{");
break;

case TLV::kTLVType_Array:
PRETTY_PRINT_SAMELINE("[");
PRETTY_PRINT("\t\t");
break;

case TLV::kTLVType_SignedInteger: {
int64_t value_s64;

err = aReader.Get(value_s64);
SuccessOrExit(err);

PRETTY_PRINT_SAMELINE("%" PRId64 ", ", value_s64);
break;
}

case TLV::kTLVType_UnsignedInteger: {
uint64_t value_u64;

err = aReader.Get(value_u64);
SuccessOrExit(err);

PRETTY_PRINT_SAMELINE("%" PRIu64 ", ", value_u64);
break;
}

case TLV::kTLVType_FloatingPointNumber: {
double value_fp;

err = aReader.Get(value_fp);
SuccessOrExit(err);

PRETTY_PRINT_SAMELINE("%lf, ", value_fp);
break;
}
case TLV::kTLVType_Boolean: {
bool value_b;

err = aReader.Get(value_b);
SuccessOrExit(err);

PRETTY_PRINT_SAMELINE("%s, ", value_b ? "true" : "false");
break;
}

case TLV::kTLVType_UTF8String: {
char value_s[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];

err = aReader.GetString(value_s, sizeof(value_s));
VerifyOrExit(err == CHIP_NO_ERROR || err == CHIP_ERROR_BUFFER_TOO_SMALL, );

if (err == CHIP_ERROR_BUFFER_TOO_SMALL)
{
PRETTY_PRINT_SAMELINE("... (byte string too long) ...");
err = CHIP_NO_ERROR;
}
else
{
PRETTY_PRINT_SAMELINE("\"%s\", ", value_s);
}
break;
}

case TLV::kTLVType_ByteString: {
uint8_t value_b[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
uint32_t len, readerLen;

readerLen = aReader.GetLength();

err = aReader.GetBytes(value_b, sizeof(value_b));
VerifyOrExit(err == CHIP_NO_ERROR || err == CHIP_ERROR_BUFFER_TOO_SMALL, );

PRETTY_PRINT_SAMELINE("[");
PRETTY_PRINT("\t\t");

if (readerLen < sizeof(value_b))
{
len = readerLen;
}
else
{
len = sizeof(value_b);
}

if (err == CHIP_ERROR_BUFFER_TOO_SMALL)
{
PRETTY_PRINT_SAMELINE("... (byte string too long) ...");
}
else
{
for (size_t i = 0; i < len; i++)
{
PRETTY_PRINT_SAMELINE("0x%x, ", value_b[i]);
}
}

err = CHIP_NO_ERROR;
PRETTY_PRINT("]");
break;
}

case TLV::kTLVType_Null:
PRETTY_PRINT_SAMELINE("NULL");
break;

default:
PRETTY_PRINT_SAMELINE("--");
break;
}

if (aReader.GetType() == TLV::kTLVType_Structure || aReader.GetType() == TLV::kTLVType_Array)
{
const char terminating_char = (aReader.GetType() == TLV::kTLVType_Structure) ? '}' : ']';
TLV::TLVType type;

IgnoreUnusedVariable(terminating_char);

err = aReader.EnterContainer(type);
SuccessOrExit(err);

while ((err = aReader.Next()) == CHIP_NO_ERROR)
{
PRETTY_PRINT_INCDEPTH();

err = ParseFields(aReader, aDepth + 1);
SuccessOrExit(err);

PRETTY_PRINT_DECDEPTH();
}

PRETTY_PRINT("\t%c,", terminating_char);

err = aReader.ExitContainer(type);
SuccessOrExit(err);
}

exit:
return err;
}

#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const
Expand Down Expand Up @@ -239,7 +69,9 @@ CHIP_ERROR CommandDataIB::Parser::CheckSchemaValidity() const
// check if this tag has appeared before
VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kFields))), CHIP_ERROR_INVALID_TLV_TAG);
tagPresenceMask |= (1 << to_underlying(Tag::kFields));
ReturnErrorOnFailure(ParseFields(reader, 0));
PRETTY_PRINT_INCDEPTH();
ReturnErrorOnFailure(CheckIMPayload(reader, 0, "CommandFields"));
PRETTY_PRINT_DECDEPTH();
break;
default:
PRETTY_PRINT("Unknown tag num %" PRIu32, tagNum);
Expand Down
4 changes: 0 additions & 4 deletions src/app/MessageDef/CommandDataIB.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ class Parser : public StructParser
* #CHIP_END_OF_TLV if there is no such element
*/
CHIP_ERROR GetFields(TLV::TLVReader * const apReader) const;

protected:
// A recursively callable function to parse a data element and pretty-print it.
CHIP_ERROR ParseFields(TLV::TLVReader & aReader, int aDepth) const;
};

class Builder : public StructBuilder
Expand Down
Loading

0 comments on commit 2297240

Please sign in to comment.