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

Resurrect "Cleanup PASE code, error handling and error logging (#6852)"" #6907

Merged
merged 5 commits into from
May 25, 2021

Conversation

pan-apple
Copy link
Contributor

@pan-apple pan-apple commented May 17, 2021

Problem

The PR #6852 was causing build issues for QPG6100. This PR tries to fix the error and resurrect the change.

Summary of Changes

Fix the issues caused by the reverted PR.

Testing

TestPASESession unit test suite
Manual testing using Python controller app
Manual testing using iOS CHIPTool app
CI runs Darwin tests that exercise the PASE Session setup process.

@boring-cyborg boring-cyborg bot added the lib label May 17, 2021
@pan-apple pan-apple marked this pull request as ready for review May 24, 2021 22:27
@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from 1a21a3f

File Section File VM
chip-qpg6100-lighting-example.out .text 184 184
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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'

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

sections,vmsize,filesize
.debug_info,0,1344
.debug_loc,0,580
.debug_str,0,223
.strtab,0,192
.text,184,184
.symtab,0,112
.debug_ranges,0,88
.debug_abbrev,0,39
.debug_frame,0,32
.debug_aranges,0,16
.debug_line,0,14
[Unmapped],0,-184


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 1a21a3f

File Section File VM
chip-shell.elf rodata 0 4
chip-lock.elf rodata 104 104
chip-lock.elf text 72 72
chip-lock.elf device_handles -8 -8
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,366
.debug_loc,0,32
.debug_str,0,32
rodata,4,0
.debug_line,0,2

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

sections,vmsize,filesize
.debug_info,0,1421
.debug_loc,0,536
.debug_str,0,224
.strtab,0,192
.symtab,0,112
rodata,104,104
text,72,72
.debug_abbrev,0,33
.debug_frame,0,32
.debug_ranges,0,24
.debug_aranges,0,16
device_handles,-8,-8
.debug_line,0,-22


@github-actions
Copy link

Size increase report for "esp32-example-build" from 1a21a3f

File Section File VM
chip-all-clusters-app.elf .flash.rodata 112 112
chip-all-clusters-app.elf .flash.text 88 88
chip-pigweed-app.elf .flash.rodata 8 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,1642
.debug_loc,0,394
.strtab,0,242
.debug_str,0,224
.flash.rodata,112,112
.flash.text,88,88
.debug_frame,0,48
.debug_abbrev,0,36
.symtab,0,32
.debug_aranges,0,16
.shstrtab,0,-2
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,-2
.debug_ranges,0,-16
.debug_line,0,-54
[Unmapped],0,-112

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

sections,vmsize,filesize
.debug_str,0,32
.debug_info,0,18
.flash.rodata,8,8
.debug_loc,0,6
[Unmapped],0,-8


@pan-apple pan-apple requested a review from andy31415 May 25, 2021 15:41
strlen(kSpake2pKeyExchangeSalt), myKeyId, delegate);
SuccessOrExit(err);
ReturnErrorOnFailure(WaitForPairing(0, kSpake2p_Iteration_Count,
reinterpret_cast<const unsigned char *>(kSpake2pKeyExchangeSalt),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should "unsigned char" here be uint8_t for consistency with the rest of the codebase?

Realise this code is not really changed at this level, so optional change for sure if it even makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WaitForPairing is exposed to the rest of the code as an API, and it currently takes unsigned char as the argument. We should be able to change it to uint8_t, but maybe better to do this in a separate PR.

@pan-apple pan-apple requested a review from bzbarsky-apple May 25, 2021 21:23
@bzbarsky-apple bzbarsky-apple merged commit 764df83 into project-chip:master May 25, 2021
@pan-apple pan-apple deleted the pase-cleanup branch May 25, 2021 22:20
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
…ct-chip#6852)"" (project-chip#6907)

* Resurrect "Cleanup PASE code, error handling and error logging (project-chip#6852)""

This reverts commit 6bfdd05.

* cleanup code

* Address review comments

* fix error check

* address review comments
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