Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ESP32] Use esp_log_writev() for logging rather than ESP_LOGx macros #26348

Merged
merged 5 commits into from
May 8, 2023

Conversation

shubhamdp
Copy link
Contributor

@shubhamdp shubhamdp commented May 3, 2023

Basically this un-reverts the #26282 with additional changes to fixed the rpc logging problem introduced in #26227.

Change Overview

Tests

  • RPC
    • Compiled using ./scripts/build/build_examples.py --target esp32-m5stack-all-clusters-rpc build.
    • Flashed onto ESP32 devkit using ./out/esp32-m5stack-all-clusters-rpc/chip-all-clusters-app.flash.py
    • Attached to pigweed rpc console python3 -m chip_rpc.console --device /dev/cu.usbserial-14340
    • Confirmed that chip and other logs can be seen on rpc console.
    • Tried commissioning using chip-tool
  • non-RPC
    • Compiled all-clusters-app/esp32, flashed on ESP32, commissioned the device and verified all CHIP[..] and other logs were seen.
  • For log level mapping
    • Validated the build/esp-idf/chip/args.gn, before change all log levels were set to true in here. After the change, correct values are being set.

Copy link
Contributor

@tehampson tehampson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for making sure this works with pigweed!

src/platform/ESP32/Logging.cpp Outdated Show resolved Hide resolved
examples/platform/esp32/PigweedLogger.cpp Show resolved Hide resolved
src/platform/ESP32/Logging.cpp Show resolved Hide resolved
src/platform/ESP32/Logging.cpp Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented May 4, 2023

PR #26348: Size comparison from de1c64a to 2ebccbe

Full report (1 build for cc32xx)
platform target config section de1c64a 2ebccbe change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 604866 604866 0 0.0
(read/write) 204156 204156 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197568 197568 0 0.0
.comment 206 206 0 0.0
.data 1468 1468 0 0.0
.debug_abbrev 957005 957005 0 0.0
.debug_aranges 101104 101104 0 0.0
.debug_frame 341416 341416 0 0.0
.debug_info 19522697 19522697 0 0.0
.debug_line 2666219 2666219 0 0.0
.debug_line_str 513 513 0 0.0
.debug_loc 33340 33340 0 0.0
.debug_loclists 1488771 1488771 0 0.0
.debug_ranges 4984 4984 0 0.0
.debug_rnglists 94291 94291 0 0.0
.debug_str 3100491 3100491 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104346 104346 0 0.0
.shstrtab 265 265 0 0.0
.stack 2048 2048 0 0.0
.strtab 482872 482872 0 0.0
.symtab 287120 287120 0 0.0
.text 498396 498396 0 0.0

@github-actions
Copy link

github-actions bot commented May 8, 2023

PR #26348: Size comparison from 6c0c5ea to 8cd120b

Increases (1 build for cc32xx)
platform target config section 6c0c5ea 8cd120b change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 19609755 19609756 1 0.0
Full report (1 build for cc32xx)
platform target config section 6c0c5ea 8cd120b change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 605090 605090 0 0.0
(read/write) 204164 204164 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197576 197576 0 0.0
.comment 206 206 0 0.0
.data 1468 1468 0 0.0
.debug_abbrev 957621 957621 0 0.0
.debug_aranges 101136 101136 0 0.0
.debug_frame 341512 341512 0 0.0
.debug_info 19609755 19609756 1 0.0
.debug_line 2666632 2666632 0 0.0
.debug_line_str 513 513 0 0.0
.debug_loc 33340 33340 0 0.0
.debug_loclists 1489225 1489225 0 0.0
.debug_ranges 4984 4984 0 0.0
.debug_rnglists 94315 94315 0 0.0
.debug_str 3108982 3108982 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104354 104354 0 0.0
.shstrtab 265 265 0 0.0
.stack 2048 2048 0 0.0
.strtab 483384 483384 0 0.0
.symtab 287328 287328 0 0.0
.text 498612 498612 0 0.0

examples/platform/esp32/PigweedLogger.cpp Outdated Show resolved Hide resolved
examples/platform/esp32/PigweedLogger.cpp Outdated Show resolved Hide resolved
@shubhamdp shubhamdp requested review from dhrishi and tehampson May 8, 2023 12:53
@github-actions
Copy link

github-actions bot commented May 8, 2023

PR #26348: Size comparison from 6c0c5ea to 30854b9

Full report (1 build for cc32xx)
platform target config section 6c0c5ea 30854b9 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 605090 605090 0 0.0
(read/write) 204164 204164 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197576 197576 0 0.0
.comment 206 206 0 0.0
.data 1468 1468 0 0.0
.debug_abbrev 957621 957621 0 0.0
.debug_aranges 101136 101136 0 0.0
.debug_frame 341512 341512 0 0.0
.debug_info 19609755 19609755 0 0.0
.debug_line 2666632 2666632 0 0.0
.debug_line_str 513 513 0 0.0
.debug_loc 33340 33340 0 0.0
.debug_loclists 1489225 1489225 0 0.0
.debug_ranges 4984 4984 0 0.0
.debug_rnglists 94315 94315 0 0.0
.debug_str 3108982 3108982 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104354 104354 0 0.0
.shstrtab 265 265 0 0.0
.stack 2048 2048 0 0.0
.strtab 483384 483384 0 0.0
.symtab 287328 287328 0 0.0
.text 498612 498612 0 0.0

@tehampson tehampson merged commit fda94b3 into project-chip:master May 8, 2023
@shubhamdp shubhamdp deleted the esp32_logging branch May 10, 2023 05:21
shubhamdp added a commit to shubhamdp/connectedhomeip that referenced this pull request May 10, 2023
…roject-chip#26348)

* [ESP32] Use esp_log_writev() for logging rather than ESP_LOGx macros

Basically this unreverts the project-chip#26282 with additional changes to fixed
the rpc logging problem introduced in project-chip#26227.

* Fix the bug when setting the log level

* remove the log level check

* Fix the color codes for pigweed logs

* Added comment explaining the addition of color codes in pigweed logger
andy31415 pushed a commit that referenced this pull request May 10, 2023
…26348) (#26477)

* [ESP32] Use esp_log_writev() for logging rather than ESP_LOGx macros

Basically this unreverts the #26282 with additional changes to fixed
the rpc logging problem introduced in #26227.

* Fix the bug when setting the log level

* remove the log level check

* Fix the color codes for pigweed logs

* Added comment explaining the addition of color codes in pigweed logger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants