[Group] Add Incoming Group message Dispatch #10985

jepenven-silabs · 2021-10-26T17:19:40Z

Problem

Processing of Incoming Group message is missing

Change overview

Add a Group message dispatch so that section 4.7.3 of the spec can be implemented for Group messages. The dispatch function is full of TODO since there's no actual code to handle Group messages properly as for now.

The Goal of this PR is mainly to put placeholders in the code showcasing what still needs to be implemented for processing incoming group messages. Each todo will be address in a separate PR to prevent huge pull request.

Testing

Regular Unit test and test suite since the code added is not yet functional (too many todos).

todo · 2021-10-26T17:19:43Z

Handle Group message counter here spec 4.7.3

connectedhomeip/src/transport/SessionManager.cpp

Lines 514 to 524 in 473a188

    
           // TODO: Handle Group message counter here spec 4.7.3 
        
           // spec 4.5.1.2 for msg counter 
        
           // Trial decryption with GroupDataProvider. TODO: Implement the GroupDataProvider Class 
        
           // VerifyOrExit(CHIP_NO_ERROR == groups->DecryptMessage(packetHeader, payloadHeader, msg), 
        
           //     ChipLogError(Inet, "Secure transport received group message, but failed to decode it, discarding")); 
        
           if (isDuplicate == SessionManagerDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) 
        
           { 
        
               ChipLogDetail(Inet,

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

todo · 2021-10-26T17:19:46Z

Implement the GroupDataProvider Class

connectedhomeip/src/transport/SessionManager.cpp

Lines 517 to 527 in 473a188

    
           // Trial decryption with GroupDataProvider. TODO: Implement the GroupDataProvider Class 
        
           // VerifyOrExit(CHIP_NO_ERROR == groups->DecryptMessage(packetHeader, payloadHeader, msg), 
        
           //     ChipLogError(Inet, "Secure transport received group message, but failed to decode it, discarding")); 
        
           if (isDuplicate == SessionManagerDelegate::DuplicateMessage::Yes && !payloadHeader.NeedsAck()) 
        
           { 
        
               ChipLogDetail(Inet, 
        
                             "Received a duplicate message with MessageCounter:" ChipLogFormatMessageCounter 
        
                             " on exchange " ChipLogFormatExchangeId, 
        
                             packetHeader.GetMessageCounter(), ChipLogValueExchangeIdFromSentHeader(payloadHeader));

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

todo · 2021-10-26T17:19:49Z

control message counter is not implemented yet

connectedhomeip/src/transport/SessionManager.cpp

Lines 538 to 548 in 473a188

    
               // TODO: control message counter is not implemented yet 
        
           } 
        
           else 
        
           { 
        
               // TODO: Commit Group Message Counter 
        
           } 
        
           if (mCB != nullptr) 
        
           { 
        
               // TODO: Update Session Handle for Group messages. 
        
               // SessionHandle session(state->GetPeerNodeId(), state->GetLocalSessionId(), state->GetPeerSessionId(),

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

todo · 2021-10-26T17:19:52Z

Commit Group Message Counter

connectedhomeip/src/transport/SessionManager.cpp

Lines 542 to 552 in 473a188

    
               // TODO: Commit Group Message Counter 
        
           } 
        
           if (mCB != nullptr) 
        
           { 
        
               // TODO: Update Session Handle for Group messages. 
        
               // SessionHandle session(state->GetPeerNodeId(), state->GetLocalSessionId(), state->GetPeerSessionId(), 
        
               //                       state->GetFabricIndex()); 
        
               // mCB->OnMessageReceived(packetHeader, payloadHeader, nullptr, peerAddress, isDuplicate, std::move(msg)); 
        
           }

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

todo · 2021-10-26T17:19:55Z

Update Session Handle for Group messages.

connectedhomeip/src/transport/SessionManager.cpp

Lines 547 to 557 in 473a188

    
                   // TODO: Update Session Handle for Group messages. 
        
                   // SessionHandle session(state->GetPeerNodeId(), state->GetLocalSessionId(), state->GetPeerSessionId(), 
        
                   //                       state->GetFabricIndex()); 
        
                   // mCB->OnMessageReceived(packetHeader, payloadHeader, nullptr, peerAddress, isDuplicate, std::move(msg)); 
        
               } 
        
           exit: 
        
               if (err != CHIP_NO_ERROR && mCB != nullptr) 
        
               { 
        
                   mCB->OnReceiveError(err, peerAddress); 
        
               }

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

github-actions · 2021-10-26T18:02:01Z

Size increase report for "gn_qpg-example-build" from 95d8dd5

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	48	48

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,819
.debug_loc,0,724
.debug_line,0,327
.debug_str,0,178
.strtab,0,142
.symtab,0,64
.debug_ranges,0,56
.text,48,48
.debug_abbrev,0,36
.debug_frame,0,32
.debug_aranges,0,8
.shstrtab,0,-2
[Unmapped],0,-48

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'

github-actions · 2021-10-26T18:11:51Z

PR #10985: Size comparison from 95d8dd5 to 07627cf

20 builds (for efr32, k32w, linux, p6, qpg, telink)

platform	target	config	section	`95d8dd5`	`07627cf`	change	% change
efr32	lighting-app	BRD4161A	.bss	113692	113692	0	0.0
			.data	1752	1752	0	0.0
			.text	736504	736600	96	0.0
	lock-app	BRD4161A	.bss	111548	111548	0	0.0
			.data	1712	1712	0	0.0
			.text	715768	715864	96	0.0
	window-app	BRD4161A	.bss	111860	111860	0	0.0
			.data	1716	1716	0	0.0
			.text	716668	716764	96	0.0
	lighting-app	BRD4161A+rpc	.bss	130196	130196	0	0.0
			.data	1852	1852	0	0.0
			.text	723928	724008	80	0.0
k32w	lock-app	k32w061+debug	.bss	69196	69196	0	0.0
			.data	1864	1864	0	0.0
			.text	515544	515592	48	0.0
	shell	k32w061+debug	.bss	63256	63256	0	0.0
			.data	672	672	0	0.0
			.text	359572	359620	48	0.0
	lighting-app	k32w061+se05x+release	.bss	78720	78720	0	0.0
			.data	1900	1900	0	0.0
			.text	614180	614228	48	0.0
linux	all-clusters-app	debug	.bss	50160	50160	0	0.0
			.data	978	978	0	0.0
			.data.rel.ro	60800	60800	0	0.0
			.dynamic	592	592	0	0.0
			.got	4088	4088	0	0.0
			.init	27	27	0	0.0
			.init_array	512	512	0	0.0
			.rodata	134613	134613	0	0.0
			.text	1358754	1359394	640	0.0
	chip-tool	debug	.bss	17680	17680	0	0.0
			.data	1584	1584	0	0.0
			.data.rel.ro	94640	94640	0	0.0
			.dynamic	592	592	0	0.0
			.got	4368	4368	0	0.0
			.init	27	27	0	0.0
			.init_array	416	416	0	0.0
			.rodata	207988	208020	32	0.0
			.text	3651029	3651669	640	0.0
	ota-provider-app	debug	.bss	37440	37440	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	24488	24488	0	0.0
			.dynamic	592	592	0	0.0
			.got	4016	4016	0	0.0
			.init	27	27	0	0.0
			.init_array	440	440	0	0.0
			.rodata	110440	110440	0	0.0
			.text	`1025714`	1026354	640	0.1
	ota-requestor-app	debug	.bss	208320	208320	0	0.0
			.data	752	752	0	0.0
			.data.rel.ro	25928	25928	0	0.0
			.dynamic	592	592	0	0.0
			.got	4144	4144	0	0.0
			.init	27	27	0	0.0
			.init_array	520	520	0	0.0
			.rodata	128936	128936	0	0.0
			.text	1147666	1148306	640	0.1
	shell	debug	.bss	16136	16136	0	0.0
			.data	242	242	0	0.0
			.data.rel.ro	36496	36496	0	0.0
			.dynamic	592	592	0	0.0
			.got	3528	3528	0	0.0
			.init	27	27	0	0.0
			.init_array	336	336	0	0.0
			.rodata	76495	76495	0	0.0
			.text	599458	600098	640	0.1
	tv-app	debug	.bss	215568	215568	0	0.0
			.data	2032	2032	0	0.0
			.data.rel.ro	58192	58192	0	0.0
			.dynamic	592	592	0	0.0
			.got	4408	4408	0	0.0
			.init	27	27	0	0.0
			.init_array	608	608	0	0.0
			.rodata	151992	152024	32	0.0
			.text	1456226	`1456866`	640	0.0
	bridge-app	debug+rpc	.bss	51856	51856	0	0.0
			.data	976	976	0	0.0
			.data.rel.ro	27272	27272	0	0.0
			.dynamic	592	592	0	0.0
			.got	3952	3952	0	0.0
			.init	27	27	0	0.0
			.init_array	400	400	0	0.0
			.rodata	109836	109836	0	0.0
			.text	`1066277`	`1066917`	640	0.1
	lighting-app	debug+rpc	.bss	41176	41176	0	0.0
			.data	1106	1106	0	0.0
			.data.rel.ro	53968	53968	0	0.0
			.dynamic	608	608	0	0.0
			.got	4112	4112	0	0.0
			.init	27	27	0	0.0
			.init_array	528	528	0	0.0
			.rodata	126993	127025	32	0.0
			.text	`1264178`	1264818	640	0.1
p6	lock-app	default	.bss	67184	67184	0	0.0
			.data	2416	2416	0	0.0
			.heap	963744	963744	0	0.0
			.text	1126784	`1126864`	80	0.0
qpg	lighting-app	qpg6100+debug	.bss	52416	52416	0	0.0
			.data	1000	1000	0	0.0
			.text	485440	485488	48	0.0
	lock-app	qpg6100+debug	.bss	51368	51368	0	0.0
			.data	956	956	0	0.0
			.text	461660	461708	48	0.0
	persistent-storage-app	qpg6100+debug	.bss	27752	27752	0	0.0
			.data	372	372	0	0.0
			.text	149900	149900	0	0.0
telink	lighting-app	tlsr9518adk80d	bss	69944	69944	0	0.0
			noinit	33216	33216	0	0.0
			text	457976	458036	60	0.0

kghost · 2021-10-26T18:19:41Z

The skeleton looks fine to me, I will check it again when all CI have been passed.

github-actions · 2021-10-26T18:29:32Z

PR #10985: Size comparison from 95d8dd5 to 07627cf

Increases (3 builds for mbed)

platform	target	config	section	`95d8dd5`	`07627cf`	change
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	.text	`1251904`	1252032	128
	lighting-app	CY8CPROTO_062_4343W+release	.text	`1219672`	`1219736`	64
	lock-app	CY8CPROTO_062_4343W+release	.text	1197704	1197768	64

Full report (4 builds for mbed)

platform	target	config	section	`95d8dd5`	`07627cf`	change
mbed	all-clusters-app	CY8CPROTO_062_4343W+release	.bss	179604	179604	0
			.data	5192	5192	0
			.heap	851648	851648	0
			.text	`1251904`	1252032	128
	lighting-app	CY8CPROTO_062_4343W+release	.bss	171060	171060	0
			.data	5464	5464	0
			.heap	859920	859920	0
			.text	`1219672`	`1219736`	64
	lock-app	CY8CPROTO_062_4343W+release	.bss	169988	169988	0
			.data	5432	5432	0
			.heap	861024	861024	0
			.text	1197704	1197768	64
	pigweed-app	CY8CPROTO_062_4343W+release	.bss	11760	11760	0
			.data	4360	4360	0
			.heap	1020328	1020328	0
			.text	103064	103064	0

github-actions · 2021-10-26T18:46:48Z

PR #10985: Size comparison from 95d8dd5 to 07627cf

Increases (11 builds for esp32, nrfconnect)

platform	target	config	section	`95d8dd5`	`07627cf`	change
esp32	all-clusters-app	c3devkit	.flash.text	875002	875062	60
		m5stack	.flash.text	906131	906275	144
nrfconnect	lighting-app	nrf52840dk_nrf52840	text	577512	577560	48
	lock-app	nrf52840dk_nrf52840	text	559072	559116	44
	pump-app	nrf52840dk_nrf52840	text	562176	562224	48
	pump-controller-app	nrf52840dk_nrf52840	text	558816	558864	48
	shell	nrf52840dk_nrf52840	text	520324	520372	48
	lighting-app	nrf52840dk_nrf52840+rpc	text	550704	550752	48
		nrf5340dk_nrf5340_cpuapp	text	506984	507028	44
	lock-app	nrf5340dk_nrf5340_cpuapp	text	488536	488580	44
	shell	nrf5340dk_nrf5340_cpuapp	text	440936	440980	44

Full report (12 builds for esp32, nrfconnect)

platform	target	config	section	`95d8dd5`	`07627cf`	change
esp32	all-clusters-app	c3devkit	.dram0.bss	58232	58232	0
			.dram0.data	16464	16464	0
			.flash.rodata	198176	198176	0
			.flash.text	875002	875062	60
			.iram0.text	57564	57564	0
		m5stack	.dram0.bss	60744	60744	0
			.dram0.data	32084	32084	0
			.flash.rodata	206928	206928	0
			.flash.text	906131	906275	144
			.iram0.text	125115	125115	0
nrfconnect	lighting-app	nrf52840dk_nrf52840	bss	112136	112136	0
			rodata	97200	97200	0
			text	577512	577560	48
	lock-app	nrf52840dk_nrf52840	bss	111208	111208	0
			rodata	93600	93600	0
			text	559072	559116	44
	pigweed-app	nrf52840dk_nrf52840	bss	51824	51824	0
			rodata	45776	45776	0
			text	339456	339456	0
	pump-app	nrf52840dk_nrf52840	bss	111308	111308	0
			rodata	94800	94800	0
			text	562176	562224	48
	pump-controller-app	nrf52840dk_nrf52840	bss	111208	111208	0
			rodata	93576	93576	0
			text	558816	558864	48
	shell	nrf52840dk_nrf52840	bss	109072	109072	0
			rodata	72536	72536	0
			text	520324	520372	48
	lighting-app	nrf52840dk_nrf52840+rpc	bss	108380	108380	0
			rodata	87976	87976	0
			text	550704	550752	48
		nrf5340dk_nrf5340_cpuapp	bss	113512	113512	0
			rodata	92440	92440	0
			text	506984	507028	44
	lock-app	nrf5340dk_nrf5340_cpuapp	bss	112584	112584	0
			rodata	88860	88860	0
			text	488536	488580	44
	shell	nrf5340dk_nrf5340_cpuapp	bss	110056	110056	0
			rodata	67180	67180	0
			text	440936	440980	44

github-actions · 2021-10-26T18:58:33Z

Size increase report for "nrfconnect-example-build" from 95d8dd5

File	Section	File	VM
chip-lock.elf	text	44	44
chip-lock.elf	device_handles	4	4
chip-shell.elf	text	48	48

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_info,0,817
.debug_loc,0,492
.debug_line,0,287
.debug_str,0,176
.strtab,0,142
.symtab,0,64
.debug_ranges,0,56
text,44,44
.debug_abbrev,0,36
.debug_frame,0,32
.debug_aranges,0,8
device_handles,4,4
.shstrtab,0,-2

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

sections,vmsize,filesize
.debug_info,0,817
.debug_loc,0,620
.debug_line,0,287
.debug_str,0,176
.strtab,0,142
.symtab,0,64
.debug_ranges,0,56
text,48,48
.debug_abbrev,0,36
.debug_frame,0,32
.debug_aranges,0,8
.shstrtab,0,2

github-actions · 2021-10-26T18:58:41Z

Size increase report for "esp32-example-build" from 95d8dd5

File	Section	File	VM
chip-all-clusters-app.elf	.flash.text	60	60

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,523
.debug_line,0,376
.debug_str,0,292
.debug_loc,0,264
.strtab,0,142
.debug_abbrev,0,85
.flash.text,60,60
.debug_frame,0,28
.symtab,0,16
.debug_aranges,0,8
.shstrtab,0,2
[Unmapped],0,-60

src/transport/SessionManager.cpp

* Add path for group message processing * fix styling

boring-cyborg bot added the transport label Oct 26, 2021

jepenven-silabs added 2 commits October 26, 2021 13:25

Add path for group message processing

930e80b

fix styling

07627cf

jepenven-silabs force-pushed the add_group_message_processing branch from 473a188 to 07627cf Compare October 26, 2021 17:27

pullapprove bot requested review from vivien-apple, wbschiller, woody-apple and yufengwangca October 26, 2021 17:27

pullapprove bot added the review - pending label Oct 26, 2021

tcarmelveilleux approved these changes Oct 26, 2021

View reviewed changes

woody-apple approved these changes Oct 27, 2021

View reviewed changes

msandstedt reviewed Oct 27, 2021

View reviewed changes

src/transport/SessionManager.cpp Show resolved Hide resolved

msandstedt approved these changes Oct 27, 2021

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Oct 27, 2021

kghost approved these changes Oct 27, 2021

View reviewed changes

woody-apple merged commit b90f360 into project-chip:master Oct 27, 2021

jepenven-silabs mentioned this pull request Oct 28, 2021

Implement the GroupDataProvider #11075

Closed

JasonLiuZhuoCheng pushed a commit to JasonLiuZhuoCheng/connectedhomeip that referenced this pull request Oct 28, 2021

[Group] Add Incoming Group message Dispatch (project-chip#10985)

7464b67

* Add path for group message processing * fix styling

carol-apple pushed a commit to carol-apple/connectedhomeip that referenced this pull request Oct 28, 2021

[Group] Add Incoming Group message Dispatch (project-chip#10985)

dd8cda9

* Add path for group message processing * fix styling

jepenven-silabs mentioned this pull request Oct 29, 2021

Apply comments for PR 10985 #11189

Merged

PSONALl pushed a commit to PSONALl/connectedhomeip that referenced this pull request Dec 3, 2021

[Group] Add Incoming Group message Dispatch (project-chip#10985)

aa257e3

* Add path for group message processing * fix styling

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Group] Add Incoming Group message Dispatch #10985

[Group] Add Incoming Group message Dispatch #10985

jepenven-silabs commented Oct 26, 2021

todo bot commented Oct 26, 2021

todo bot commented Oct 26, 2021

todo bot commented Oct 26, 2021

todo bot commented Oct 26, 2021

todo bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

kghost commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

[Group] Add Incoming Group message Dispatch #10985

[Group] Add Incoming Group message Dispatch #10985

Conversation

jepenven-silabs commented Oct 26, 2021

Problem

Change overview

Testing

todo bot commented Oct 26, 2021

Handle Group message counter here spec 4.7.3

This comment was generated by todo based on a TODO comment in 473a188 in #10985. cc @jepenven-silabs.

todo bot commented Oct 26, 2021

Implement the GroupDataProvider Class

This comment was generated by todo based on a TODO comment in 473a188 in #10985. cc @jepenven-silabs.

todo bot commented Oct 26, 2021

control message counter is not implemented yet

This comment was generated by todo based on a TODO comment in 473a188 in #10985. cc @jepenven-silabs.

todo bot commented Oct 26, 2021

Commit Group Message Counter

This comment was generated by todo based on a TODO comment in 473a188 in #10985. cc @jepenven-silabs.

todo bot commented Oct 26, 2021

Update Session Handle for Group messages.

This comment was generated by todo based on a TODO comment in 473a188 in #10985. cc @jepenven-silabs.

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

kghost commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

github-actions bot commented Oct 26, 2021

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.

This comment was generated by todo based on a `TODO` comment in `473a188` in #10985. cc @jepenven-silabs.