[ESP32] Platform diagnostics framework #36532
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
5ef9201 to
97d0be3
Compare
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
2 times, most recently
from
a93a065 to
7178050
Compare
|
PR #36532: Size comparison from 61e288a to 7178050 Full report (3 builds for cc32xx, stm32)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
3 times, most recently
from
667abcd to
ed14abb
Compare
|
PR #36532: Size comparison from 61e288a to ed14abb Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
Would have been helpful if something in the PR title or description made it clear this is esp32-only... |
pimpalemahesh
changed the title
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
ed14abb to
6571c5e
Compare
|
PR #36532: Size comparison from d8ededa to 6571c5e Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
shubhamdp
reviewed
Nov 19, 2024
examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp
Outdated
Show resolved
Hide resolved
...ples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h
Outdated
Show resolved
Hide resolved
...ples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h
Outdated
Show resolved
Hide resolved
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
9e148bb to
1ce3585
Compare
|
PR #36532: Size comparison from 0ac52eb to 1ce3585 Full report (11 builds for cc13x4_26x4, cc32xx, qpg, stm32, tizen)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
1ce3585 to
eb088ab
Compare
|
PR #36532: Size comparison from ffbc362 to eb088ab Full report (3 builds for cc32xx, stm32)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
2 times, most recently
from
ca96506 to
ba3ac54
Compare
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
ba3ac54 to
a98b1f8
Compare
|
PR #36532: Size comparison from ffbc362 to a98b1f8 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
03d2d73 to
e58bee7
Compare
|
PR #36532: Size comparison from 84fb78f to 3415978 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
3415978 to
0befb4f
Compare
|
PR #36532: Size comparison from 5d42d92 to 0befb4f Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
2 times, most recently
from
208f588 to
e8db459
Compare
|
PR #36532: Size comparison from 2c11741 to e8db459 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Working backend with metric, trace and counter diagnostics - Diagnostic interface implementation with ring buffer storage - Added option ENABLE_ESP_DIAGNOSTICS_TRACE in chip KConfig - Added required options for enabling matter diagnostic trace in project Kconfig - Enabled diagnostic trace for temperature-measurement-app example
- Resolve buffer related issues - Add store diagnostic call for trace_end and trace_instant - Update scoped macro - Remove extra namespace values, spaces and print statements - Remove evicthead call for circular buffer after read successful
- Resolve buffer issues - Use single buffer for store and retrieve of diagnostics - Resolve data loss issue
…essary comments, format files, namespace changes
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
2 times, most recently
from
9d4af7b to
8b2dc95
Compare
- Add related logic in temperature-measurement-app
pimpalemahesh
force-pushed
the
platform_diagnostics
branch
from
8b2dc95 to
6835f62
Compare
|
PR #36532: Size comparison from 017e8b5 to 6835f62 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
shubhamdp
reviewed
Dec 24, 2024
| @@ -108,8 +118,24 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE | |||
| switch (intent) | |||
| { | |||
| case IntentEnum::kEndUserSupport: { | |||
| #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE | |||
| CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); | |||
| MutableByteSpan endUserSupportSpan(endUserBuffer, 2048); | |||
There was a problem hiding this comment.
can we either define a const, or #define for this magic number 2048, and add some comment about its usage.
examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp
Show resolved
Hide resolved
Comment on lines
+29
to
+33
| #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE | ||
| #include <tracing/esp32_diagnostic_trace/DiagnosticStorageManager.h> | ||
| static uint8_t endUserBuffer[CONFIG_END_USER_BUFFER_SIZE]; | ||
| using namespace chip::Tracing::Diagnostics; | ||
| #endif // CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE |
There was a problem hiding this comment.
This is not used in this file, so can this be moved to .cpp?
| @@ -75,14 +79,19 @@ static AppDeviceCallbacks EchoCallbacks; | |||
| static void InitServer(intptr_t context) | |||
| { | |||
| Esp32AppServer::Init(); // Init ZCL Data Model and CHIP App Server AND Initialize device attestation config | |||
| #if CONFIG_ENABLE_ESP_DIAGNOSTICS_TRACE | |||
| CircularDiagnosticBuffer & diagnosticStorage = CircularDiagnosticBuffer::GetInstance(); | |||
There was a problem hiding this comment.
I think we can do this without singleton design. We already are passing the buffer and buffer size to the impl so IMO, singleton is not required?
|
|
||
| void Init(uint8_t * buffer, size_t bufferLength) { chip::TLV::TLVCircularBuffer::Init(buffer, bufferLength); } | ||
|
|
||
| CHIP_ERROR Store(DiagnosticEntry & entry) override |
There was a problem hiding this comment.
Suggested change
| CHIP_ERROR Store(DiagnosticEntry & entry) override | |
| CHIP_ERROR Store(const DiagnosticEntry & entry) override |
| using HashValue = uint32_t; | ||
|
|
||
| // Implements a murmurhash with 0 seed. | ||
| uint32_t MurmurHash(const void * key) |
There was a problem hiding this comment.
can we make this static or move inside the anonymous namespace?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
The existing implementation lacks the ability to collect diagnostics using the diagnostic-logs cluster.
Change Overview:
Platform Diagnostic Framework:
Designed to collect, store, and retrieve diagnostic data.
Implemented new backend:
Diagnostic data collection integrated using Matter Traces.
Diagnostic data is stored in a TLVCircularBuffer for efficient memory utilization.
Integration with the Temperature Measurement App for platform esp32.
Testing:
Verified commissioning and retrieval of diagnostics using the diagnostic-logs cluster over BDX on the ESP32-C3 chip with the temperature-measurement-app example.