Fix subscription liveness check #11795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
erjiaqing,
franck-apple,
harimau-qirex,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kpschoedel,
LuDuda,
lzgrablic02,
mrjerryjohns,
msandstedt,
mspang,
pan-apple,
robszewczyk,
sagar-apple,
saurabhst,
selissia,
tcarmelveilleux and
tecimovic
|
PR #11795: Size comparison from ef6455d to 5445625 Increases (11 builds for esp32, linux, telink)
Full report (11 builds for esp32, linux, telink)
|
yunhanw-google
force-pushed
the
feature/fix_report_delivery_timer
branch
from
5445625 to
2437ea0
Compare
|
PR #11795: Size comparison from 7653f00 to 2437ea0 Increases (20 builds for esp32, linux, nrfconnect, telink)
Full report (21 builds for esp32, linux, nrfconnect, telink)
|
yunhanw-google
force-pushed
the
feature/fix_report_delivery_timer
branch
from
2437ea0 to
5529e57
Compare
|
PR #11795: Size comparison from ebadd33 to 5529e57 Increases (23 builds for efr32, k32w, linux, mbed, p6, qpg, telink)
Full report (26 builds for efr32, k32w, linux, mbed, p6, qpg, telink)
|
yunhanw-google
force-pushed
the
feature/fix_report_delivery_timer
branch
from
5529e57 to
72e538b
Compare
issue 12009, need actual tcp margin value considering restransmissionconnectedhomeip/src/app/ReadClient.cpp Lines 556 to 566 in 72e538b This comment was generated by todo based on a
|
|
PR #11795: Size comparison from ebadd33 to 72e538b Increases (34 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
msandstedt
approved these changes
Nov 19, 2021
yunhanw-google
force-pushed
the
feature/fix_report_delivery_timer
branch
from
72e538b to
26c7ea5
Compare
|
PR #11795: Size comparison from 244d8d4 to 26c7ea5 Increases (34 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
bzbarsky-apple
approved these changes
Nov 20, 2021
jmeg-sfy
approved these changes
Nov 22, 2021
jmartinez-silabs
approved these changes
Nov 22, 2021
mrjerryjohns
approved these changes
Nov 23, 2021
-- Send sync report with max interval, after min interval passed, send report immediately if there is any attribute change. -- Add additionam margin for MRP upon max interval when doing liveness check
yunhanw-google
force-pushed
the
feature/fix_report_delivery_timer
branch
from
26c7ea5 to
05cb801
Compare
|
PR #11795: Size comparison from 2e16d32 to 05cb801 Increases (35 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
What is being fixed? Examples:
-- Send sync report with max interval, after min interval passed, send report immediately if there is any attribute change.
-- Add additional margin for MRP upon max interval when doing liveness check
-- Fix bug:Report Engine would set handler dirty only when its subscribed path interested with dirty path, and fix UpdateReadHandlerDirty's dirty cleanup.
-- Call ScheduleRun() here since we may have accumulated dirty items in the global dirty set that need to get flushed now that the hold off(min interval has passed) has elapsed where handler has been set dirty.
--When the max-interval has elapsed, to definitely send out a report even if there is no actual data to be sent.
Change overview
See above
Testing
Existing test cover for happy path, manual test for negative cases, for manual test, I use im mock initiator and im mock responder which can communicate over crmp, and after subscription is estabalished, I tear down responder, then check the print out in initiator and see if the actual timeout call can be triggered after max(5s) + margin(19seconds)