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

Refactor the server to organize as a class #9200

Merged
merged 9 commits into from
Sep 10, 2021

Conversation

gjc13
Copy link
Contributor

@gjc13 gjc13 commented Aug 24, 2021

Problem

What is being fixed? Examples:

  • For device to device interaction, we need to access the globals like the exchange manager and the session manager while we don't want to add too many getters to static variables.
  • The Server.h and Server.cpp itself is not well organized with some functions in namespace and some not. The static variables are added arbitrarily.

Change overview

The PR refactors the Server.cpp file and aggerates its functions to a chip::Server class.

Testing

How was this tested? (at least one bullet point required)

  • Build chip-tool and linux all cluster app. The pairing and control flow works well.

Fixes #9521

@todo
Copy link

todo bot commented Aug 24, 2021

(cecille): If this is re-called when the window is already open, what should happen?

// TODO(cecille): If this is re-called when the window is already open, what should happen?
mDeviceDiscriminatorCache.RestoreDiscriminator();
uint32_t pinCode;
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetSetupPinCode(pinCode));
RendezvousParameters params;
params.SetSetupPINCode(pinCode);
#if CONFIG_NETWORK_LAYER_BLE
SetBLE(advertisementMode == chip::PairingWindowAdvertisement::kBle);


This comment was generated by todo based on a TODO comment in fe88beb in #9200. cc @gjc13.

@andy31415
Copy link
Contributor

@woody-apple - src/app/tests/TestCommissionManager.cpp exists now
@gjc13 - could you update to pass CI?

@gjc13 gjc13 force-pushed the server-refactor branch 2 times, most recently from beeee90 to 800aaab Compare September 10, 2021 04:07
@todo
Copy link

todo bot commented Sep 10, 2021

The platform memory was intentionally left not deinitialized so that minimal mdns can destruct

// TODO: The platform memory was intentionally left not deinitialized so that minimal mdns can destruct
return (nlTestRunnerStats(&theSuite));
}
CHIP_REGISTER_TEST_SUITE(TestCommissionManager)


This comment was generated by todo based on a TODO comment in 277bd55 in #9200. cc @gjc13.

@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 44f8ea7

File Section File VM
chip-lock.elf bss 0 32
chip-lock.elf device_handles 8 8
chip-lock.elf devices -4 -4
chip-lock.elf datas -20 -20
chip-lock.elf text -24 -24
chip-lock.elf rodata -80 -80
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

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

sections,vmsize,filesize
.debug_info,0,59021
.debug_line,0,6955
.debug_abbrev,0,4064
.debug_str,0,2038
.debug_loc,0,970
bss,32,0
device_handles,8,8
devices,-4,-4
datas,-20,-20
.debug_aranges,0,-24
text,-24,-24
rodata,-80,-80
.debug_ranges,0,-96
.debug_frame,0,-104
.symtab,0,-560
.strtab,0,-804


@github-actions
Copy link

Size increase report for "esp32-example-build" from 44f8ea7

File Section File VM
chip-shell.elf .flash.text 48 48
chip-ipv6only-app.elf .flash.text 172 172
chip-all-clusters-app.elf .flash.text 252 252
chip-all-clusters-app.elf .dram0.bss 0 8
chip-all-clusters-app.elf .dram0.heap_start 0 -8
chip-all-clusters-app.elf .dram0.data -24 -16
chip-all-clusters-app.elf .flash.rodata -80 -80
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize

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

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

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

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

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

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,175087
.debug_line,0,9695
.debug_abbrev,0,6001
.debug_str,0,1954
.debug_loc,0,488
.flash.text,252,252
.debug_frame,0,96
.debug_aranges,0,24
.dram0.bss,8,0
.riscv.attributes,0,-1
.shstrtab,0,-3
.debug_ranges,0,-8
.dram0.heap_start,-8,0
.dram0.data,-16,-24
.flash.rodata,-80,-80
[Unmapped],0,-148
.symtab,0,-208
.strtab,0,-681

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-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

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


@gjc13
Copy link
Contributor Author

gjc13 commented Sep 10, 2021

@woody-apple Unit tests added.

@andy31415 Pullapprove bot complained reviewers-lg team was not found on this repository. Looks like some bug in policy settings? Other CIs are fine now.

@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from 2addf00

File Section File VM
chip-qpg6100-lighting-example.out .bss 0 24
chip-qpg6100-lighting-example.out .heap 0 -8
chip-qpg6100-lighting-example.out .data -20 -20
chip-qpg6100-lighting-example.out .text -108 -108
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,57696
.debug_line,0,7413
.debug_abbrev,0,4100
.debug_str,0,2039
.debug_loc,0,908
[Unmapped],0,76
[ELF Program Headers],0,32
.bss,24,0
.shstrtab,0,1
.heap,-8,0
.data,-20,-20
.debug_aranges,0,-24
.debug_ranges,0,-40
.debug_frame,0,-104
.text,-108,-108
.symtab,0,-560
.strtab,0,-817

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'


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.

Server: support creating new ExchangeContext for other protocols
7 participants