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

Rename admin pairing class and usage to fabric #8479

Merged
merged 2 commits into from
Jul 23, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

AdminPairingTable and AdminPairingInfo should be renamed to Fabric to match latest specifications.

Change overview

Update the class and its usage.
Note: more changes are needed in the class to bring it up to the spec. This PR is specifically only renaming the class and its usage, to prevent mixing the large rename change with more specific logic change in the class.

Testing

Existing unit tests, and pairing using chip-tool, Python controller, and iOS controller apps.

@mspang
Copy link
Contributor

mspang commented Jul 19, 2021

Problem

AdminPairingTable and AdminPairingInfo should be renamed to Fabric to match latest specifications.

Change overview

Update the class and its usage.
Note: more changes are needed in the class to bring it up to the spec. This PR is specifically only renaming the class and its usage, to prevent mixing the large rename change with more specific logic change in the class.

Testing

Existing unit tests, and pairing using chip-tool, Python controller, and iOS controller apps.

Any functional changes? If not can you assert this in the description please?

If so can you avoid functional changes in a rename PR?

"Just a rename, no functional change."

@pan-apple pan-apple force-pushed the rename-admin-table branch from ba6781d to 3a62b8e Compare July 22, 2021 00:31
@github-actions
Copy link

Size increase report for "esp32-example-build" from f15ed2e

File Section File VM
chip-all-clusters-app.elf .dram0.data 8 16
chip-all-clusters-app.elf .flash.text 12 12
chip-all-clusters-app.elf .dram0.bss 0 8
chip-all-clusters-app.elf .dram0.heap_start 0 -8
chip-shell.elf .flash.text 48 48
chip-ipv6only-app.elf .flash.text 172 172
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_loc,0,159
.dram0.data,16,8
.flash.text,12,12
.dram0.bss,8,0
.riscv.attributes,0,-1
.dram0.heap_start,-8,0
.debug_ranges,0,-16
.debug_abbrev,0,-20
[Unmapped],0,-20
[ELF Headers],0,-40
.debug_line,0,-75
.strtab,0,-244
.debug_str,0,-460
.debug_info,0,-651

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

sections,vmsize,filesize
.debug_line,0,355
.xt.prop._ZN4chip8Platform3NewINS_9Transport10FabricInfo18StorableFabricInfoEJEEEPT_DpOT0_,0,116
.debug_loc,0,115
.flash.text,48,48
.debug_ranges,0,8
.shstrtab,0,-24
[Unmapped],0,-48
.strtab,0,-60
.debug_info,0,-67
.xt.prop._ZN4chip8Platform3NewINS_9Transport16AdminPairingInfo24StorableAdminPairingInfoEJEEEPT_DpOT0_,0,-116
.debug_str,0,-407

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

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

sections,vmsize,filesize

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

sections,vmsize,filesize
.flash.text,172,172
[Unmapped],0,-172

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: integer overflow

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

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: integer overflow


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from f15ed2e

File Section File VM
chip-lock.elf rodata 16 16
chip-lock.elf device_handles 8 8
chip-lock.elf text -8 -8
chip-shell.elf rodata 0 4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_ranges,0,16
rodata,16,16
device_handles,8,8
.debug_line,0,7
.shstrtab,0,-2
text,-8,-8
.symtab,0,-16
.debug_abbrev,0,-20
.debug_frame,0,-24
.debug_loc,0,-42
.strtab,0,-266
.debug_str,0,-498
.debug_info,0,-607

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

sections,vmsize,filesize
.debug_line,0,134
.debug_loc,0,54
.debug_ranges,0,16
rodata,4,0
.debug_frame,0,-12
.symtab,0,-16
.strtab,0,-72
.debug_info,0,-142
.debug_str,0,-410


@pan-apple
Copy link
Contributor Author

@saurabhst do you have any review feedback?

@woody-apple woody-apple merged commit 2a63309 into project-chip:master Jul 23, 2021
@pan-apple pan-apple deleted the rename-admin-table branch July 23, 2021 16:16
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Rename AdminPairing class and usage to Fabric

- Just a rename, no functional changes

* 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.

6 participants