Move RendezvousServer and CommissionManager to a single class #9709

pan-apple · 2021-09-14T22:06:49Z

Problem

Fixes RendezvousServer and CommissionManager have circular dependency #9701

Change overview

Consolidate code for the two classes, RendezvousServer and CommissionManager, in a single class CommissioningWindowManager.

Testing

Run cluster tests app with chip-tool to test different commissioning modes.
Test using m5stack and chip-tool to ensure that commissioning over BLE is working.

woody-apple · 2021-09-15T01:14:04Z

@saurabhst @msandstedt @Damian-Nordic @andy31415 ?

src/app/server/CommissioningWindowManager.cpp

todo · 2021-09-15T15:25:23Z

Verify that the device supports BLE if the advertisementMode is kBLE

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 141 to 151 in 608f728

    
               // TODO: Verify that the device supports BLE if the advertisementMode is kBLE 
        
               // If not, then return an error. 
        
           #if CONFIG_NETWORK_LAYER_BLE 
        
               SetBLE(advertisementMode == chip::CommissioningWindowAdvertisement::kBle); 
        
           #else 
        
               SetBLE(false); 
        
           #endif // CONFIG_NETWORK_LAYER_BLE 
        
               if (resetFabrics == ResetFabrics::kYes) 
        
               { 
        
                   mServer->GetFabricTable().DeleteAllFabrics();

This comment was generated by todo based on a `TODO` comment in `608f728` in #9709. cc @pan-apple.

todo · 2021-09-15T15:25:26Z

Don't use BLE for commissioning additional fabrics on a device

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 186 to 196 in 608f728

    
               // TODO: Don't use BLE for commissioning additional fabrics on a device 
        
               SetBLE(true); 
        
           #else 
        
               SetBLE(false); 
        
           #endif 
        
               uint16_t keyID = 0; 
        
               CHIP_ERROR err = CHIP_NO_ERROR; 
        
               SuccessOrExit(err = SetTemporaryDiscriminator(discriminator)); 
        
               SuccessOrExit(err = PrepareCommissioningWindow(commissioningTimeoutSeconds, keyID));

This comment was generated by todo based on a `TODO` comment in `608f728` in #9709. cc @pan-apple.

todo · 2021-09-15T15:46:25Z

Don't use BLE for commissioning additional fabrics on a device

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 186 to 196 in ef59edb

    
               // TODO: Don't use BLE for commissioning additional fabrics on a device 
        
               SetBLE(true); 
        
           #else 
        
               SetBLE(false); 
        
           #endif 
        
               uint16_t keyID = 0; 
        
               CHIP_ERROR err = CHIP_NO_ERROR; 
        
               SuccessOrExit(err = SetTemporaryDiscriminator(discriminator)); 
        
               SuccessOrExit(err = PrepareCommissioningWindow(commissioningTimeoutSeconds, keyID));

This comment was generated by todo based on a `TODO` comment in `ef59edb` in #9709. cc @pan-apple.

pan-apple · 2021-09-15T15:46:30Z

rebased

src/app/server/CommissioningWindowManager.cpp

msandstedt

I had a couple comments inline, but in general this seems like some nice cleanup.

todo · 2021-09-15T21:11:54Z

Don't use BLE for commissioning additional fabrics on a device

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 176 to 186 in 4c1cae2

    
               // TODO: Don't use BLE for commissioning additional fabrics on a device 
        
               SetBLE(true); 
        
           #else 
        
               SetBLE(false); 
        
           #endif 
        
               uint16_t keyID = 0; 
        
               CHIP_ERROR err = CHIP_NO_ERROR; 
        
               SuccessOrExit(err = SetTemporaryDiscriminator(discriminator)); 
        
               SuccessOrExit(err = PrepareCommissioningWindow(commissioningTimeoutSeconds, keyID));

This comment was generated by todo based on a `TODO` comment in `4c1cae2` in #9709. cc @pan-apple.

todo · 2021-09-15T21:51:17Z

Don't use BLE for commissioning additional fabrics on a device

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 176 to 186 in 3bf486e

    
               // TODO: Don't use BLE for commissioning additional fabrics on a device 
        
               SetBLE(true); 
        
           #else 
        
               SetBLE(false); 
        
           #endif 
        
               uint16_t keyID = 0; 
        
               CHIP_ERROR err = CHIP_NO_ERROR; 
        
               SuccessOrExit(err = SetTemporaryDiscriminator(discriminator)); 
        
               SuccessOrExit(err = PrepareCommissioningWindow(commissioningTimeoutSeconds, keyID));

This comment was generated by todo based on a `TODO` comment in `3bf486e` in #9709. cc @pan-apple.

github-actions · 2021-09-15T22:10:24Z

Size increase report for "gn_qpg-example-build" from 4763c91

File	Section	File	VM
chip-qpg6100-lighting-example.out	.heap	0	24
chip-qpg6100-lighting-example.out	.bss	0	-24
chip-qpg6100-lighting-example.out	.text	-532	-532

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
[Unmapped],0,532
.heap,24,0
.bss,-24,0
.debug_aranges,0,-56
.debug_frame,0,-124
.symtab,0,-272
.strtab,0,-300
.text,-532,-532
.debug_ranges,0,-768
.debug_loc,0,-2039
.debug_abbrev,0,-4083
.debug_str,0,-4331
.debug_line,0,-6621
.debug_info,0,-54390

github-actions · 2021-09-15T22:42:06Z

Size increase report for "nrfconnect-example-build" from 4763c91

File	Section	File	VM
chip-lock.elf	rodata	0	-4
chip-lock.elf	bss	0	-32
chip-lock.elf	text	-528	-528

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
rodata,-4,0
bss,-32,0
.debug_aranges,0,-56
.debug_frame,0,-124
.symtab,0,-272
.strtab,0,-300
text,-528,-528
.debug_ranges,0,-824
.debug_loc,0,-2043
.debug_abbrev,0,-4154
.debug_str,0,-4349
.debug_line,0,-6338
.debug_info,0,-55392

todo · 2021-09-15T22:45:38Z

Don't use BLE for commissioning additional fabrics on a device

connectedhomeip/src/app/server/CommissioningWindowManager.cpp

Lines 176 to 186 in d1cd02a

    
               // TODO: Don't use BLE for commissioning additional fabrics on a device 
        
               SetBLE(true); 
        
           #else 
        
               SetBLE(false); 
        
           #endif 
        
               uint16_t keyID = 0; 
        
               CHIP_ERROR err = CHIP_NO_ERROR; 
        
               SuccessOrExit(err = SetTemporaryDiscriminator(discriminator)); 
        
               SuccessOrExit(err = PrepareCommissioningWindow(commissioningTimeoutSeconds, keyID));

This comment was generated by todo based on a `TODO` comment in `d1cd02a` in #9709. cc @pan-apple.

github-actions · 2021-09-15T22:58:44Z

Size increase report for "esp32-example-build" from 4763c91

File	Section	File	VM
chip-all-clusters-app.elf	.flash.rodata	8	8
chip-all-clusters-app.elf	.dram0.heap_start	0	-8
chip-all-clusters-app.elf	.dram0.bss	0	-24
chip-all-clusters-app.elf	.flash.text	-524	-524

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
.flash.rodata,8,8
.riscv.attributes,0,-1
.dram0.heap_start,-8,0
.dram0.bss,-24,0
.debug_aranges,0,-56
.debug_frame,0,-124
.symtab,0,-128
.strtab,0,-248
.flash.text,-524,-524
.debug_ranges,0,-648
.debug_loc,0,-898
[Unmapped],0,-3580
.debug_str,0,-4365
.debug_abbrev,0,-5967
.debug_line,0,-8738
.debug_info,0,-164359

boring-cyborg bot added app controller examples labels Sep 14, 2021

pullapprove bot requested review from andy31415, bzbarsky-apple, chrisdecenzo, Damian-Nordic, hawk248, jepenven-silabs and msandstedt September 14, 2021 22:11

pullapprove bot added the review - pending label Sep 14, 2021

woody-apple added restyled SDK Approved and removed restyled labels Sep 14, 2021

woody-apple approved these changes Sep 14, 2021

View reviewed changes

andy31415 reviewed Sep 15, 2021

View reviewed changes

pan-apple requested a review from andy31415 September 15, 2021 15:26

pan-apple added 3 commits September 15, 2021 08:45

Move RendezvousServer and CommissionManager to a single class

7e0074e

fix compilation of tv app

b21ecda

address review comments

ef59edb

pan-apple force-pushed the issue9701 branch from 608f728 to ef59edb Compare September 15, 2021 15:46

msandstedt reviewed Sep 15, 2021

View reviewed changes

src/app/server/CommissioningWindowManager.cpp Outdated Show resolved Hide resolved

msandstedt reviewed Sep 15, 2021

View reviewed changes

src/app/server/CommissioningWindowManager.cpp Show resolved Hide resolved

msandstedt reviewed Sep 15, 2021

View reviewed changes

remove fabric deletion code from CommissioningWindowManager

4c1cae2

pan-apple requested a review from msandstedt September 15, 2021 21:12

msandstedt approved these changes Sep 15, 2021

View reviewed changes

fix build issue

3bf486e

fix test compilation

d1cd02a

andy31415 approved these changes Sep 15, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Sep 15, 2021

bzbarsky-apple merged commit 2815a10 into project-chip:master Sep 16, 2021

todo bot mentioned this pull request Sep 16, 2021

Don't use BLE for commissioning additional fabrics on a device #9741

Closed

pan-apple deleted the issue9701 branch September 16, 2021 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move RendezvousServer and CommissionManager to a single class #9709

Move RendezvousServer and CommissionManager to a single class #9709

pan-apple commented Sep 14, 2021

woody-apple commented Sep 15, 2021

todo bot commented Sep 15, 2021

todo bot commented Sep 15, 2021

todo bot commented Sep 15, 2021

pan-apple commented Sep 15, 2021

msandstedt left a comment

todo bot commented Sep 15, 2021

todo bot commented Sep 15, 2021

github-actions bot commented Sep 15, 2021

github-actions bot commented Sep 15, 2021

todo bot commented Sep 15, 2021

github-actions bot commented Sep 15, 2021

Move RendezvousServer and CommissionManager to a single class #9709

Move RendezvousServer and CommissionManager to a single class #9709

Conversation

pan-apple commented Sep 14, 2021

Problem

Change overview

Testing

woody-apple commented Sep 15, 2021

todo bot commented Sep 15, 2021

Verify that the device supports BLE if the advertisementMode is kBLE

This comment was generated by todo based on a TODO comment in 608f728 in #9709. cc @pan-apple.

todo bot commented Sep 15, 2021

Don't use BLE for commissioning additional fabrics on a device

This comment was generated by todo based on a TODO comment in 608f728 in #9709. cc @pan-apple.

todo bot commented Sep 15, 2021

Don't use BLE for commissioning additional fabrics on a device

This comment was generated by todo based on a TODO comment in ef59edb in #9709. cc @pan-apple.

pan-apple commented Sep 15, 2021

msandstedt left a comment

Choose a reason for hiding this comment

todo bot commented Sep 15, 2021

Don't use BLE for commissioning additional fabrics on a device

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

todo bot commented Sep 15, 2021

Don't use BLE for commissioning additional fabrics on a device

This comment was generated by todo based on a TODO comment in 3bf486e in #9709. cc @pan-apple.

github-actions bot commented Sep 15, 2021

github-actions bot commented Sep 15, 2021

todo bot commented Sep 15, 2021

Don't use BLE for commissioning additional fabrics on a device

This comment was generated by todo based on a TODO comment in d1cd02a in #9709. cc @pan-apple.

github-actions bot commented Sep 15, 2021

This comment was generated by todo based on a `TODO` comment in `608f728` in #9709. cc @pan-apple.

This comment was generated by todo based on a `TODO` comment in `608f728` in #9709. cc @pan-apple.

This comment was generated by todo based on a `TODO` comment in `ef59edb` in #9709. cc @pan-apple.

This comment was generated by todo based on a `TODO` comment in `4c1cae2` in #9709. cc @pan-apple.

This comment was generated by todo based on a `TODO` comment in `3bf486e` in #9709. cc @pan-apple.

This comment was generated by todo based on a `TODO` comment in `d1cd02a` in #9709. cc @pan-apple.