Skip to content

Commit 1ef047b

Browse files
pimpalemaheshesp
pimpalemahesh
authored and
esp
committed
Modified chip log statements, make string values mutable
1 parent 52da1f9 commit 1ef047b

File tree

6 files changed

+41
-59
lines changed

6 files changed

+41
-59
lines changed

‎src/tracing/esp32_diagnostic_trace/Counter.cpp

+1-7
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,8 @@ CHIP_ERROR ESPDiagnosticCounter::ReportMetrics(const char * label, CircularDiagn
4646
{
4747
VerifyOrReturnError(storageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE,
4848
ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));
49-
50-
// Allocate buffers for the diagnostic entry
51-
char labelBuffer[kMaxStringValueSize];
52-
strncpy(labelBuffer, label, kMaxStringValueSize);
53-
labelBuffer[kMaxStringValueSize - 1] = '\0';
54-
5549
// Create diagnostic entry
56-
DiagnosticEntry entry = { .label = labelBuffer,
50+
DiagnosticEntry entry = { .label = const_cast<char *>(label),
5751
.uintValue = GetInstanceCount(label),
5852
.type = ValueType::kUnsignedInteger,
5953
.timestamp = esp_log_timestamp() };

‎src/tracing/esp32_diagnostic_trace/DiagnosticEntry.cpp

+9-7
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace chip {
55
namespace Tracing {
66
namespace Diagnostics {
77

8-
CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
8+
CHIP_ERROR Encode(TLVWriter & writer, const DiagnosticEntry & entry)
99
{
1010
TLVType DiagnosticOuterContainer = kTLVType_NotSpecified;
1111
ReturnErrorOnFailure(writer.StartContainer(AnonymousTag(), kTLVType_Structure, DiagnosticOuterContainer));
@@ -16,7 +16,8 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
1616
// Write label
1717
if (entry.label != nullptr)
1818
{
19-
if (strlen(entry.label) > kMaxStringValueSize)
19+
uint32_t labelSize = strlen(entry.label) > kMaxStringValueSize ? kMaxStringValueSize : strlen(entry.label);
20+
if (labelSize >= kMaxStringValueSize)
2021
{
2122
char labelBuffer[kMaxStringValueSize + 1];
2223
memcpy(labelBuffer, entry.label, kMaxStringValueSize);
@@ -25,7 +26,7 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
2526
}
2627
else
2728
{
28-
ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), entry.label));
29+
ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::LABEL), entry.label, labelSize));
2930
}
3031
}
3132

@@ -35,7 +36,8 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
3536
case ValueType::kCharString:
3637
if (entry.stringValue != nullptr)
3738
{
38-
if (strlen(entry.stringValue) > kMaxStringValueSize)
39+
uint32_t valueSize = strlen(entry.stringValue) > kMaxStringValueSize ? kMaxStringValueSize : strlen(entry.stringValue);
40+
if (valueSize >= kMaxStringValueSize)
3941
{
4042
char valueBuffer[kMaxStringValueSize + 1];
4143
memcpy(valueBuffer, entry.stringValue, kMaxStringValueSize);
@@ -44,7 +46,7 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
4446
}
4547
else
4648
{
47-
ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), entry.stringValue));
49+
ReturnErrorOnFailure(writer.PutString(chip::TLV::ContextTag(DiagTag::VALUE), entry.stringValue, valueSize));
4850
}
4951
}
5052
break;
@@ -60,11 +62,11 @@ CHIP_ERROR Encode(CircularTLVWriter & writer, const DiagnosticEntry & entry)
6062

6163
ReturnErrorOnFailure(writer.EndContainer(DiagnosticOuterContainer));
6264
ReturnErrorOnFailure(writer.Finalize());
63-
ChipLogProgress(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", entry.label);
65+
ChipLogDetail(DeviceLayer, "Diagnostic Value written to storage successfully. label: %s\n", entry.label);
6466
return CHIP_NO_ERROR;
6567
}
6668

67-
CHIP_ERROR Decode(CircularTLVReader & reader, DiagnosticEntry & entry)
69+
CHIP_ERROR Decode(TLVReader & reader, DiagnosticEntry & entry)
6870
{
6971
TLVType containerType;
7072
ReturnErrorOnFailure(reader.EnterContainer(containerType));

‎src/tracing/esp32_diagnostic_trace/DiagnosticEntry.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace chip {
2323
namespace Tracing {
2424
namespace Diagnostics {
2525

26-
static constexpr size_t kMaxStringValueSize = 128;
26+
static constexpr size_t kMaxStringValueSize = 64;
2727

2828
enum class ValueType
2929
{
@@ -34,9 +34,11 @@ enum class ValueType
3434

3535
struct DiagnosticEntry
3636
{
37+
// mutable label because modified in decode
3738
char * label;
3839
union
3940
{
41+
// mutable stringValue because modified in decode
4042
char * stringValue;
4143
uint32_t uintValue;
4244
int32_t intValue;
@@ -47,13 +49,13 @@ struct DiagnosticEntry
4749

4850
enum class DiagTag : uint8_t
4951
{
50-
TIMESTAMP = 1,
51-
LABEL = 2,
52-
VALUE = 3
52+
TIMESTAMP = 0,
53+
LABEL = 1,
54+
VALUE = 2,
5355
};
5456

55-
CHIP_ERROR Encode(chip::TLV::CircularTLVWriter & writer, const DiagnosticEntry & entry);
56-
CHIP_ERROR Decode(chip::TLV::CircularTLVReader & reader, DiagnosticEntry & entry);
57+
CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, const DiagnosticEntry & entry);
58+
CHIP_ERROR Decode(chip::TLV::TLVReader & reader, DiagnosticEntry & entry);
5759

5860
} // namespace Diagnostics
5961
} // namespace Tracing

‎src/tracing/esp32_diagnostic_trace/DiagnosticStorage.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ CHIP_ERROR CircularDiagnosticBuffer::Retrieve(MutableByteSpan & span, uint32_t &
7070
successful_written_bytes = writer.GetLengthWritten();
7171
}
7272
span.reduce_size(successful_written_bytes);
73-
ChipLogProgress(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes);
73+
ChipLogDetail(DeviceLayer, "---------------Total Retrieved bytes : %ld----------------\n", successful_written_bytes);
7474
return CHIP_NO_ERROR;
7575
}
7676

‎src/tracing/esp32_diagnostic_trace/DiagnosticTracing.cpp

+20-36
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ uint32_t MurmurHash(const void * key)
5252

5353
if (hash == 0)
5454
{
55-
ESP_LOGW("Tracing", "MurmurHash resulted in a hash value of 0");
55+
ChipLogError(DeviceLayer, "MurmurHash resulted in a hash value of 0");
5656
}
5757

5858
return hash;
@@ -63,18 +63,23 @@ constexpr size_t kPermitListMaxSize = CONFIG_MAX_PERMIT_LIST_SIZE;
6363
using HashValue = uint32_t;
6464
using namespace Utils;
6565

66-
// Only traces with scope in gPermitList are allowed.
67-
// Used for MATTER_TRACE_SCOPE()
66+
/*
67+
* gPermitList will allow the following traces to be stored in the storage instance while other traces are skipped.
68+
* Only traces with scope in gPermitList are allowed.
69+
* Used for MATTER_TRACE_SCOPE()
70+
*/
6871
HashValue gPermitList[kPermitListMaxSize] = { MurmurHash("PASESession"),
6972
MurmurHash("CASESession"),
7073
MurmurHash("NetworkCommissioning"),
7174
MurmurHash("GeneralCommissioning"),
7275
MurmurHash("OperationalCredentials"),
7376
MurmurHash("CASEServer"),
74-
MurmurHash("Fabric") }; // namespace
77+
MurmurHash("Fabric") };
7578

76-
// All traces with value from gSkipList are skipped.
77-
// Used for MATTER_TRACE_INSTANT()
79+
/*
80+
* gSkipList will skip the following traces from being stored in the storage instance while other traces are stored.
81+
* Used for MATTER_TRACE_INSTANT()
82+
*/
7883
HashValue gSkipList[kPermitListMaxSize] = {
7984
MurmurHash("Resolver"),
8085
};
@@ -111,14 +116,8 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)
111116
switch (event.ValueType())
112117
{
113118
case ValueType::kInt32: {
114-
ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32());
115-
116-
// Allocate buffers for the diagnostic entry
117-
char labelBuffer[kMaxStringValueSize];
118-
strncpy(labelBuffer, event.key(), kMaxStringValueSize);
119-
labelBuffer[kMaxStringValueSize - 1] = '\0';
120-
121-
DiagnosticEntry entry = { .label = labelBuffer,
119+
ChipLogDetail(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueInt32());
120+
DiagnosticEntry entry = { .label = const_cast<char *>(event.key()),
122121
.intValue = event.ValueInt32(),
123122
.type = Diagnostics::ValueType::kSignedInteger,
124123
.timestamp = esp_log_timestamp() };
@@ -127,14 +126,8 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)
127126
break;
128127

129128
case ValueType::kUInt32: {
130-
ChipLogProgress(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32());
131-
132-
// Allocate buffers for the diagnostic entry
133-
char labelBuffer[kMaxStringValueSize];
134-
strncpy(labelBuffer, event.key(), kMaxStringValueSize);
135-
labelBuffer[kMaxStringValueSize - 1] = '\0';
136-
137-
DiagnosticEntry entry = { .label = labelBuffer,
129+
ChipLogDetail(DeviceLayer, "The value of %s is %" PRId32, event.key(), event.ValueUInt32());
130+
DiagnosticEntry entry = { .label = const_cast<char *>(event.key()),
138131
.uintValue = event.ValueUInt32(),
139132
.type = Diagnostics::ValueType::kUnsignedInteger,
140133
.timestamp = esp_log_timestamp() };
@@ -143,15 +136,15 @@ void ESP32Diagnostics::LogMetricEvent(const MetricEvent & event)
143136
break;
144137

145138
case ValueType::kChipErrorCode:
146-
ChipLogProgress(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());
139+
ChipLogDetail(DeviceLayer, "The value of %s is error with code %lu ", event.key(), event.ValueErrorCode());
147140
break;
148141

149142
case ValueType::kUndefined:
150-
ChipLogProgress(DeviceLayer, "The value of %s is undefined", event.key());
143+
ChipLogDetail(DeviceLayer, "The value of %s is undefined", event.key());
151144
break;
152145

153146
default:
154-
ChipLogProgress(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key());
147+
ChipLogDetail(DeviceLayer, "The value of %s is of an UNKNOWN TYPE", event.key());
155148
break;
156149
}
157150
}
@@ -180,18 +173,9 @@ CHIP_ERROR ESP32Diagnostics::StoreDiagnostics(const char * label, const char * g
180173
VerifyOrReturnError(mStorageInstance != nullptr, CHIP_ERROR_INCORRECT_STATE,
181174
ChipLogError(DeviceLayer, "Diagnostic Storage Instance cannot be NULL"));
182175

183-
// Allocate buffers for the diagnostic entry
184-
char labelBuffer[kMaxStringValueSize];
185-
strncpy(labelBuffer, label, kMaxStringValueSize);
186-
labelBuffer[kMaxStringValueSize - 1] = '\0';
187-
188-
char groupBuffer[kMaxStringValueSize];
189-
strncpy(groupBuffer, group, kMaxStringValueSize);
190-
groupBuffer[kMaxStringValueSize - 1] = '\0';
191-
192176
// Create diagnostic entry
193-
DiagnosticEntry entry = { .label = labelBuffer,
194-
.stringValue = groupBuffer,
177+
DiagnosticEntry entry = { .label = const_cast<char *>(label),
178+
.stringValue = const_cast<char *>(group),
195179
.type = Diagnostics::ValueType::kCharString,
196180
.timestamp = esp_log_timestamp() };
197181

‎src/tracing/esp32_diagnostic_trace/DiagnosticTracing.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
namespace chip {
2626
namespace Tracing {
2727
namespace Diagnostics {
28-
/// A Backend that outputs data to chip logging.
28+
/// A Backend that stores data to storage instance
2929
///
30-
/// Structured data is formatted as json strings.
30+
/// Structured data is formatted as TLV.
3131
class ESP32Diagnostics : public ::chip::Tracing::Backend
3232
{
3333
public:

0 commit comments

Comments
 (0)