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

Add ability to redirect CHIP logs to a callback and add python log callback integration #5024

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

andy31415
Copy link
Contributor

Problem

Python scripting cannot easily capture logs (and generally log redirects do not seem possible)

Summary of Changes

Add the ability to redirect CHIP Logging to a callback, implement such a log redirect in python.

@andy31415 andy31415 changed the title Log redirect callback Add ability to redirect CHIP logs to a callback and add python log callback integration Feb 25, 2021
@github-actions
Copy link

Size increase report for "esp32-example-build" from 5ca7308

File Section File VM
chip-all-clusters-app.elf .flash.text 20 20
chip-all-clusters-app.elf .dram0.bss 0 8
chip-pigweed-app.elf .flash.text 20 20
chip-pigweed-app.elf .dram0.bss 0 8
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
.debug_info,0,151
.debug_str,0,134
.debug_line,0,104
.debug_abbrev,0,62
.strtab,0,54
.debug_frame,0,24
.flash.text,20,20
.symtab,0,16
.debug_aranges,0,8
.debug_ranges,0,8
.dram0.bss,8,0
.shstrtab,0,2
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,2
.debug_loc,0,-1

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

sections,vmsize,filesize
.debug_info,0,151
.debug_str,0,134
.debug_line,0,104
.debug_abbrev,0,62
.strtab,0,54
.debug_frame,0,24
.flash.text,20,20
.symtab,0,16
.debug_aranges,0,8
.debug_ranges,0,8
.dram0.bss,8,0
.shstrtab,0,2
.debug_loc,0,-1
.xt.prop._ZSt9__find_ifIPKSt4byteN9__gnu_cxx5__ops10_Iter_predIPFbS0_EEEET_S9_S9_T0_St26random_access_iterator_tag,0,-2


@msandstedt
Copy link
Contributor

Can this be rebased on master? The history is pretty hard to look at with all of the commits from #4960.

@andy31415
Copy link
Contributor Author

Can this be rebased on master? The history is pretty hard to look at with all of the commits from #4960.

The best I could do would be to squash all commits. Master does squash merges today so I cannot properly rebase it (or at least I tried and failed).

@msandstedt
Copy link
Contributor

Can this be rebased on master? The history is pretty hard to look at with all of the commits from #4960.

The best I could do would be to squash all commits. Master does squash merges today so I cannot properly rebase it (or at least I tried and failed).

I meant rebase and squash locally, and then force-push. This is fine though as long as whomever merges uses the squash option.

@github-actions
Copy link

github-actions bot commented Mar 2, 2021

Size increase report for "gn_qpg6100-example-build" from 8a6335b

File Section File VM
chip-qpg6100-lock-example.out .text 16 16
chip-qpg6100-lock-example.out .bss 0 8
chip-qpg6100-lock-example.out .heap 0 -8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,174
.debug_str,0,129
.debug_abbrev,0,60
.strtab,0,54
.debug_line,0,51
.symtab,0,48
.debug_loc,0,42
.debug_frame,0,16
.text,16,16
.bss,8,0
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2
.heap,-8,0
[Unmapped],0,-16


@github-actions
Copy link

github-actions bot commented Mar 2, 2021

Size increase report for "nrfconnect-example-build" from 8a6335b

File Section File VM
chip-shell.elf text 16 16
chip-lighting.elf text 16 16
chip-lock.elf [LOAD #3 [RW]] 0 31
chip-lock.elf text 16 16
chip-lock.elf bss 0 1
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
.debug_info,0,154
.debug_str,0,131
.debug_abbrev,0,65
.debug_line,0,61
.strtab,0,54
.symtab,0,48
.debug_frame,0,20
text,16,16
.debug_loc,0,9
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2

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

sections,vmsize,filesize
.debug_info,0,154
.debug_str,0,131
.debug_abbrev,0,65
.debug_line,0,61
.strtab,0,54
.symtab,0,48
.debug_loc,0,33
.debug_frame,0,20
text,16,16
.debug_aranges,0,8
.debug_ranges,0,8
.shstrtab,0,2

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

sections,vmsize,filesize
.debug_info,0,154
.debug_str,0,131
.debug_abbrev,0,65
.debug_line,0,61
.strtab,0,54
.symtab,0,48
[LOAD #3 [RW]],31,0
.debug_loc,0,25
.debug_frame,0,20
text,16,16
.debug_aranges,0,8
.debug_ranges,0,8
bss,1,0
.shstrtab,0,-2


mspang
mspang previously requested changes Mar 2, 2021
Copy link
Contributor

@mspang mspang left a comment

Choose a reason for hiding this comment

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

  1. There is missing synchronization between replacing and accessing callbacks.
  2. How does this work with the Python GIL ?

@CLAassistant
Copy link

CLAassistant commented Mar 4, 2021

CLA assistant check
All committers have signed the CLA.

@andy31415
Copy link
Contributor Author

andy31415 commented Mar 4, 2021

  1. There is missing synchronization between replacing and accessing callbacks.
  2. How does this work with the Python GIL ?

Python sets the callbacks at startup, before the CHIP threads are initialized or have a chance to log anything.

Did not have trouble here with GIL while running iPython - logs do show up while in the editor.

@andy31415 andy31415 force-pushed the log_redirect_callback branch from 556040f to d153b30 Compare March 4, 2021 13:48
@andy31415 andy31415 requested a review from mspang March 4, 2021 13:52
@andy31415
Copy link
Contributor Author

Added synchronization via std::atomic

@woody-apple woody-apple dismissed mspang’s stale review March 4, 2021 18:21

Comments were addressed

@woody-apple woody-apple merged commit 456e79c into project-chip:master Mar 4, 2021
@andy31415 andy31415 deleted the log_redirect_callback branch October 28, 2021 14:03
agners added a commit to agners/connectedhomeip that referenced this pull request Jun 3, 2024
Since project-chip#5024 there is a new logging callback working. The old code has
partially been removed in project-chip#4690, but never completely. Drop the old
logging code for good.
agners added a commit to agners/connectedhomeip that referenced this pull request Jun 3, 2024
Since project-chip#5024 there is a new logging callback working. The old code has
partially been removed in project-chip#4690, but never completely. Drop the old
logging code for good.
agners added a commit to agners/connectedhomeip that referenced this pull request Jun 3, 2024
Since project-chip#5024 there is a new logging callback working. The old code has
partially been removed in project-chip#4690, but never completely. Drop the old
logging code for good.
mergify bot pushed a commit that referenced this pull request Jun 4, 2024
Since #5024 there is a new logging callback working. The old code has
partially been removed in #4690, but never completely. Drop the old
logging code for good.
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.

6 participants