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

Duplicate MRP message handling and test #7893

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

If the MRP ack is lost, the retransmitted MRP message doesn't trigger an ack. The message counter check detects the retransmitted message as duplicate, and drops it before it could reach MRP layer.

Change overview

Detect and mark the message as duplicate. If the message requires MRP ack, let it go up the stack to the MRP layer.
The MRP and exchange layer sends the ack for the message, and drops it.

Testing

Added a new unit test CheckDuplicateMessage to test this condition.
Also tested E2E flow using Python controller and chip-tool apps.

@pan-apple
Copy link
Contributor Author

Fixes #7292

@mspang mspang requested a review from yufengwangca June 24, 2021 22:56
src/messaging/ExchangeMgr.cpp Outdated Show resolved Hide resolved
src/messaging/ExchangeMgr.h Show resolved Hide resolved
src/messaging/tests/TestReliableMessageProtocol.cpp Outdated Show resolved Hide resolved
src/transport/SecureSessionMgr.h Outdated Show resolved Hide resolved
src/transport/SecureSessionMgr.h Outdated Show resolved Hide resolved
src/messaging/ExchangeMgr.h Show resolved Hide resolved
src/messaging/ExchangeMgr.cpp Outdated Show resolved Hide resolved
@pan-apple pan-apple requested a review from bzbarsky-apple June 25, 2021 22:23
@pan-apple
Copy link
Contributor Author

/rebase

@github-actions
Copy link

Size increase report for "gn_qpg6100-example-build" from 17dbc19

File Section File VM
chip-qpg6100-lighting-example.out .text -40 -40
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,1932
.debug_loc,0,184
[Unmapped],0,40
.shstrtab,0,1
.debug_ranges,0,-8
.text,-40,-40
.debug_aranges,0,-56
.debug_abbrev,0,-64
.debug_line,0,-91
.symtab,0,-128
.debug_frame,0,-148
.debug_str,0,-149
.strtab,0,-253

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
Copy link

Size increase report for "esp32-example-build" from 17dbc19

File Section File VM
chip-all-clusters-app.elf .flash.text 60 60
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-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,2292
.debug_line,0,137
.debug_loc,0,125
.debug_ranges,0,112
[Unmapped],0,80
.flash.text,60,60
.xt.lit._ZN4chip16BitMapObjectPoolINS_9Messaging15ExchangeContextELj8EE12CreateObjectIJPNS1_15ExchangeManagerEtRNS_19SecureSessionHandleEbRPNS1_16ExchangeDelegateEEEEPS2_DpOT_,0,40
.xt.lit._ZNK4chip9Transport11PeerAddress8ToStringEPcj,0,40
.xt.prop._ZN4chip16BitMapObjectPoolINS_9Messaging15ExchangeContextELj8EE12CreateObjectIJPNS1_15ExchangeManagerEtRNS_19SecureSessionHandleEbRPNS1_16ExchangeDelegateEEEEPS2_DpOT_,0,40
[1 Others],0,1
.debug_aranges,0,-16
.xt.prop._ZN4chip9Messaging15ExchangeManagerD0Ev,0,-24
.xt.prop._ZN4chip9Messaging15ExchangeManagerD2Ev,0,-24
.xt.prop._ZN4chip9Transport19PeerConnectionStateC5Ev,0,-40
.debug_abbrev,0,-61
.symtab,0,-64
.flash.rodata,-80,-80
.xt.lit._ZN4chip9Transport19PeerConnectionStateC5Ev,0,-80
.debug_frame,0,-120
.debug_str,0,-145
.strtab,0,-253


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 17dbc19

File Section File VM
chip-shell.elf text 76 76
chip-shell.elf device_handles 4 4
chip-shell.elf rodata -72 -76
chip-lock.elf text 76 76
chip-lock.elf device_handles 4 4
chip-lock.elf rodata -72 -76
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
.debug_info,0,681
.debug_loc,0,168
text,76,76
.debug_ranges,0,40
device_handles,4,4
.shstrtab,0,1
.debug_aranges,0,-56
.debug_line,0,-65
rodata,-76,-72
.symtab,0,-96
.debug_abbrev,0,-100
.debug_frame,0,-156
.debug_str,0,-184
.strtab,0,-293

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

sections,vmsize,filesize
.debug_info,0,1969
.debug_loc,0,164
text,76,76
.debug_ranges,0,40
device_handles,4,4
.shstrtab,0,1
.debug_line,0,-48
.debug_aranges,0,-56
.debug_abbrev,0,-65
rodata,-76,-72
.symtab,0,-96
.debug_str,0,-144
.debug_frame,0,-148
.strtab,0,-253


@pan-apple
Copy link
Contributor Author

@saurabhst, @mspang any feedback?

Copy link
Contributor

@mspang mspang left a comment

Choose a reason for hiding this comment

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

rubber stamping @yufengwangca's review

@mspang mspang merged commit 48ed12b into project-chip:master Jun 28, 2021
@pan-apple pan-apple deleted the crmp-duplicates branch June 28, 2021 17:54
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Duplicate CRMP message handling and test

* address review comments

* update test with 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.

7 participants