[K32W] Improve debugging + CPU speed #7717

doru91 · 2021-06-17T14:31:00Z

.#### Problem

Improve logging capabilities
Increase CPU speed

Change overview

Add a timestamp for platform logging. Also increase the CPU speed
from 32Mhz to 48Mhz

Testing

manually tested

msandstedt · 2021-06-17T17:11:19Z

src/platform/K32W/Logging.cpp

    if (chipCategory != kLogCategory_None)
    {
        switch (chipCategory)
        {
        case kLogCategory_Error:
-            memcpy(buf, "[Error]", 7);
+            memcpy(buf + strlen(buf), "[Error]", 7);


I don't understand how this was supposed to work before or how it works now. buf isn't length delimited, but no null terminator is explicitly written.

Why not just use strcpy? That would give you the null character and also alleviate the need to count chars.

Seems like a strcat() scenario.

@msandstedt @jelderton , agree that the old code was messy. I refactored it using the snprintf function.

mspang · 2021-06-22T20:54:21Z

src/platform/K32W/Logging.cpp

@@ -21,26 +21,29 @@
 static bool isLogInitialized;
 extern uint8_t gOtLogUartInstance;
 extern "C" void K32WWriteBlocking(const uint8_t * aBuf, uint32_t len);
+extern "C" uint32_t otPlatAlarmMilliGetNow(void);

 namespace chip {
 namespace Logging {
 namespace Platform {

 void GetMessageString(char * buf, uint8_t chipCategory, uint8_t otLevelLog)


Can you add a buffer length parameter please?

@mspang, done. Thanks!

Add a timestamp for platform logging. Also increase the CPU speed from 32Mhz to 48Mhz. Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

github-actions · 2021-06-25T10:30:55Z

Size increase report for "gn_qpg6100-example-build" from 336533f

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	24	24

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.text,24,24
.debug_info,0,-1
.debug_str,0,-2
.debug_frame,0,-4
.debug_line,0,-9
.debug_abbrev,0,-17
[Unmapped],0,-24
.debug_loc,0,-115

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

github-actions · 2021-06-25T10:31:04Z

Size increase report for "esp32-example-build" from 336533f

File	Section	File	VM
chip-all-clusters-app.elf	.flash.rodata	16	16
chip-all-clusters-app.elf	.flash.text	4	4

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.flash.rodata,16,16
.flash.text,4,4
.debug_loc,0,-4
[Unmapped],0,-16

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

github-actions · 2021-06-25T10:49:48Z

Size increase report for "nrfconnect-example-build" from 336533f

File	Section	File	VM
chip-shell.elf	rodata	16	16
chip-lock.elf	rodata	24	20

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
rodata,16,16
.debug_info,0,-1
.debug_frame,0,-4
.debug_line,0,-6
.debug_abbrev,0,-17
.debug_loc,0,-116

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
rodata,20,24
.debug_info,0,-1
.debug_frame,0,-4
.debug_line,0,-10
.debug_abbrev,0,-17
.debug_loc,0,-120

bzbarsky-apple · 2021-06-25T20:06:45Z

src/platform/K32W/Logging.cpp

+    const char * categoryString;
+
+    writtenLen = snprintf(buf, bufLen, "[%lu]", otPlatAlarmMilliGetNow());
+    assert((writtenLen > 0) && (writtenLen < bufLen));


Why is this assert valid? What guarantees that writtenLen >= bufLen can't happen here?

Why is this assert valid? What guarantees that writtenLen >= bufLen can't happen here?

We can calculate the minimum size and document this as a precondition?

bzbarsky-apple · 2021-06-25T20:08:24Z

src/platform/K32W/Logging.cpp

        }
+
+        writtenLen = snprintf(buf + writtenLen, bufLen, "[%s]", categoryString);
+        assert((writtenLen > 0) && (writtenLen < bufLen));


Again, what guaratees the assert holds? I don't see anything.

bzbarsky-apple · 2021-06-25T20:08:43Z

src/platform/K32W/Logging.cpp

    }
+
+    writtenLen = snprintf(buf + writtenLen, bufLen, "[%s]", module);
+    assert((writtenLen > 0) && (writtenLen < bufLen));


stale · 2021-07-07T17:28:59Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2021-07-14T17:30:20Z

This stale pull request has been automatically closed. Thank you for your contributions.

* decrease stack size (allocated dinamically) for most of the tasks (by using the Stack Water Mark feature in FreeRTOS); * improve debugging by adding more logging related to timestamps and increase the CPU speed to 48Mhz (see project-chip#7717) Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> [THREADIP-3532] Improve debugging + CPU speed Add a timestamp for platform logging. Also increase the CPU speed from 32Mhz to 48Mhz. Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

* decrease stack size (allocated dinamically) for most of the tasks (by using the Stack Water Mark feature in FreeRTOS); * improve debugging by adding more logging related to timestamps and increase the CPU speed to 48Mhz (see project-chip#7717) Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com> [THREADIP-3532] Improve debugging + CPU speed

boring-cyborg bot added k32w platform labels Jun 17, 2021

pullapprove bot requested review from andy31415, bzbarsky-apple, chrisdecenzo, Damian-Nordic, hawk248, jepenven-silabs, msandstedt and vivien-apple June 17, 2021 14:31

pullapprove bot added the review - pending label Jun 17, 2021

doru91 mentioned this pull request Jun 17, 2021

chip-device-ctrl timeout #7682

Closed

andy31415 approved these changes Jun 17, 2021

View reviewed changes

andy31415 added the Tested manually label Jun 17, 2021

woody-apple approved these changes Jun 17, 2021

View reviewed changes

msandstedt reviewed Jun 17, 2021

View reviewed changes

mspang requested changes Jun 22, 2021

View reviewed changes

doru91 force-pushed the fix/k32w_log_speed branch from f64ddd1 to 0b54c6c Compare June 25, 2021 09:50

[K32W] Improve debugging + CPU speed

6ada606

Add a timestamp for platform logging. Also increase the CPU speed from 32Mhz to 48Mhz. Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>

doru91 force-pushed the fix/k32w_log_speed branch from 0b54c6c to 6ada606 Compare June 25, 2021 09:54

Restyled by clang-format

082128d

restyled-io bot mentioned this pull request Jun 25, 2021

Restyle [K32W] Improve debugging + CPU speed #7908

Closed

msandstedt approved these changes Jun 25, 2021

View reviewed changes

bzbarsky-apple requested changes Jun 25, 2021

View reviewed changes

stale bot added the stale Stale issue or PR label Jul 7, 2021

stale bot closed this Jul 14, 2021

doru91 mentioned this pull request Jul 15, 2021

Matter TE4 fixes #8419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[K32W] Improve debugging + CPU speed #7717

[K32W] Improve debugging + CPU speed #7717

doru91 commented Jun 17, 2021

msandstedt Jun 17, 2021

jelderton Jun 21, 2021

doru91 Jun 25, 2021

mspang Jun 22, 2021

doru91 Jun 25, 2021 •

edited

Loading

github-actions bot commented Jun 25, 2021

github-actions bot commented Jun 25, 2021

github-actions bot commented Jun 25, 2021

bzbarsky-apple Jun 25, 2021

mspang Jun 30, 2021

bzbarsky-apple Jun 25, 2021

bzbarsky-apple Jun 25, 2021

stale bot commented Jul 7, 2021

stale bot commented Jul 14, 2021

[K32W] Improve debugging + CPU speed #7717

[K32W] Improve debugging + CPU speed #7717

Conversation

doru91 commented Jun 17, 2021

Change overview

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doru91 Jun 25, 2021 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 25, 2021

github-actions bot commented Jun 25, 2021

github-actions bot commented Jun 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jul 7, 2021

stale bot commented Jul 14, 2021

doru91 Jun 25, 2021 •

edited

Loading