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

Integrate KVS with device session tables #4976

Merged
merged 9 commits into from
Feb 24, 2021

Conversation

pan-apple
Copy link
Contributor

@pan-apple pan-apple commented Feb 23, 2021

Problem

The previous PR (#4952) was reverted due to a crash loop on the device.

Summary of Changes

This PR fixes the crash issue caused by #4952. The crash was due to using MemoryAlloc instead of New<> for allocating PASESession objects. It caused memory to be allocated, but the object was not initialized.

The old PR added the following functionality:
Added code to store relevant information for Admin and Peer Connection/PASE Session in KVS storage. The device will read KVS and recreate the tables at boot up. Tested the change on m5stack.

@todo
Copy link

todo bot commented Feb 23, 2021

The admin ID space allocation should be re-evaluated. With the current approach, the space could be

// TODO: The admin ID space allocation should be re-evaluated. With the current approach, the space could be
// exhausted while IDs are still available (e.g. if the admin IDs are allocated and freed over a period of time).
// Also, the current approach can make ID lookup slower as more IDs are allocated and freed.
for (AdminId id = 0; id < nextAvailableId; id++)
{
AdminPairingInfo * admin = adminPairings.AssignAdminId(id);
// Recreate the binding if one exists in persistent storage. Else skip to the next ID
if (admin->FetchFromKVS(gServerStorage) != CHIP_NO_ERROR)
{
adminPairings.ReleaseAdminId(id);
}


This comment was generated by todo based on a TODO comment in 4ada40b in #4976. cc @pan-apple.

@pan-apple
Copy link
Contributor Author

rebased

@boring-cyborg boring-cyborg bot added the k32w label Feb 23, 2021
@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 7c09658

File Section File VM
chip-lighting.elf text 2016 2016
chip-lighting.elf rodata 552 556
chip-lighting.elf [LOAD #3 [RW]] 0 31
chip-lighting.elf bss 0 1
chip-lock.elf text 2016 2016
chip-lock.elf rodata 560 556
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,101149
.debug_line,0,12555
.debug_abbrev,0,9628
.debug_loc,0,8202
.debug_str,0,5538
.strtab,0,2789
text,2016,2016
.debug_ranges,0,1728
.symtab,0,1712
.debug_frame,0,928
rodata,556,552
.debug_aranges,0,336
[LOAD #3 [RW]],31,0
.shstrtab,0,3
bss,1,0

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

sections,vmsize,filesize
.debug_info,0,101149
.debug_line,0,12559
.debug_abbrev,0,9628
.debug_loc,0,8198
.debug_str,0,5538
.strtab,0,2789
text,2016,2016
.debug_ranges,0,1728
.symtab,0,1712
.debug_frame,0,928
rodata,556,560
.debug_aranges,0,336
.shstrtab,0,3

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

sections,vmsize,filesize


@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from 7c09658

File Section File VM
chip-qpg6100-lock-example.out .text 1352 1352
chip-qpg6100-lock-example.out .bss 0 16
chip-qpg6100-lock-example.out .data -4 -4
chip-qpg6100-lock-example.out .heap 0 -16
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,50366
.debug_line,0,8491
.debug_abbrev,0,6118
.debug_str,0,4369
.debug_loc,0,3104
.strtab,0,2136
.symtab,0,1440
.text,1352,1352
.debug_frame,0,908
.debug_aranges,0,320
.debug_ranges,0,176
[ELF Headers],0,32
.bss,16,0
.data,-4,-4
.heap,-16,0
[Unmapped],0,-1384


friend class KeyValueStoreManager;

public:
// NOTE: Currently this platform does not support partial and offset reads

Choose a reason for hiding this comment

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

Maybe this comment needs to be updated to state that every function returns NOT_IMPLEMENTED (same on other platforms), or maybe just a comment/TODO on the class to explain.

@andy31415 andy31415 merged commit 9a791a8 into project-chip:master Feb 24, 2021
@pan-apple pan-apple deleted the kvs-integration branch February 24, 2021 16:50
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