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.

Sign up for GitHub

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

Jump to bottom

Pass the ReadClient to ReadClient::Callback::OnDone. #18740

Merged
merged 1 commit into from
May 23, 2022
Merged

Pass the ReadClient to ReadClient::Callback::OnDone. #18740

merged 1 commit into from
May 23, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

This allows a single callback to be used across multiple read clients
as needed, and is symmetric with how WriteClient and CommandSender work.

Problem

chip-tool needs to be able to tell which of multiple subscriptions it's dealing with has completed.

Change overview

See above.

Testing

Code compiles. No other changes for now.

@bzbarsky-apple
Pass the ReadClient to ReadClient::Callback::OnDone.
58920d8 
This allows a single callback to be used across multiple read clients
as needed, and is symmetric with how WriteClient and CommandSender work.
@github-actions
Copy link

github-actions bot commented May 23, 2022 •
edited
Loading

PR #18740: Size comparison from 721e9dd to 58920d8

Increases (13 builds for cyw30739, esp32, linux, telink)
platform target config section 721e9dd 58920d8 change % change
cyw30739 ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570882 570890 8 0.0
.app_xip_area 465980 465988 8 0.0
esp32 all-clusters-app c3devkit (read only) 1002948 1002950 2 0.0
.flash.text 1002948 1002950 2 0.0
linux all-clusters-app debug (read only) 2759513 2759529 16 0.0
.text 2343474 2343490 16 0.0
chip-tool debug (read only) 9488845 9488893 48 0.0
.text 7675605 7675653 48 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9231604 9231652 48 0.0
.text 7319332 7319380 48 0.0
lighting-app debug+rpc (read only) 2326953 2326969 16 0.0
.text 1972530 1972546 16 0.0
ota-provider-app debug (read only) 2065657 2065673 16 0.0
.text 1727218 1727234 16 0.0
ota-requestor-app debug (read only) 2094609 2094625 16 0.0
.text 1758738 1758754 16 0.0
shell debug (read only) 2575625 2575641 16 0.0
.text 2191986 2192002 16 0.0
tv-app debug (read only) 2856401 2856433 32 0.0
.text 2453090 2453122 32 0.0
tv-casting-app debug (read only) 5450705 5450737 32 0.0
.text 4749250 4749282 32 0.0
telink light-switch-app tlsr9518adk80d text 553428 553430 2 0.0
lighting-app tlsr9518adk80d text 570140 570142 2 0.0
Full report (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 721e9dd 58920d8 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668519 668519 0 0.0
(read/write) 183120 183120 0 0.0
.bss 74620 74620 0 0.0
.data 3404 3404 0 0.0
.rodata 99919 99919 0 0.0
.text 568376 568376 0 0.0
lock-ftd LP_CC2652R7 (read only) 675927 675927 0 0.0
(read/write) 166856 166856 0 0.0
.bss 72820 72820 0 0.0
.data 3236 3236 0 0.0
.rodata 94407 94407 0 0.0
.text 581036 581036 0 0.0
lock-mtd LP_CC2652R7 (read only) 625335 625335 0 0.0
(read/write) 145652 145652 0 0.0
.bss 68556 68556 0 0.0
.data 3236 3236 0 0.0
.rodata 94287 94287 0 0.0
.text 530556 530556 0 0.0
pump-app LP_CC2652R7 (read only) 676011 676011 0 0.0
(read/write) 168260 168260 0 0.0
.bss 73284 73284 0 0.0
.data 3272 3272 0 0.0
.rodata 88899 88899 0 0.0
.text 586628 586628 0 0.0
pump-controller-app LP_CC2652R7 (read only) 653971 653971 0 0.0
(read/write) 189900 189900 0 0.0
.bss 73140 73140 0 0.0
.data 3232 3232 0 0.0
.rodata 83675 83675 0 0.0
.text 569816 569816 0 0.0
shell LP_CC2652R7 (read only) 661542 661542 0 0.0
(read/write) 185656 185656 0 0.0
.bss 76980 76980 0 0.0
.data 3408 3408 0 0.0
.rodata 96886 96886 0 0.0
.text 564428 564428 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 624150 624150 0 0.0
.app_xip_area 527508 527508 0 0.0
.bss 79284 79284 0 0.0
.data 708 708 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 626962 626962 0 0.0
.app_xip_area 531776 531776 0 0.0
.bss 77860 77860 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570882 570890 8 0.0
.app_xip_area 465980 465988 8 0.0
.bss 87280 87280 0 0.0
.data 584 584 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 915636 915636 0 0.0
(read/write) 133372 133372 0 0.0
.bss 131312 131312 0 0.0
.data 2060 2060 0 0.0
.text 915628 915628 0 0.0
BRD4161A+rpc (read only) 949824 949824 0 0.0
(read/write) 150056 150056 0 0.0
.bss 147792 147792 0 0.0
.data 2264 2264 0 0.0
.text 949816 949816 0 0.0
BRD4161A+rs911x (read only) 790196 790196 0 0.0
(read/write) 129640 129640 0 0.0
.bss 127572 127572 0 0.0
.data 2068 2068 0 0.0
.text 790188 790188 0 0.0
lock-app BRD4161A+wf200 (read only) 946776 946776 0 0.0
(read/write) 124124 124124 0 0.0
.bss 122100 122100 0 0.0
.data 2024 2024 0 0.0
.text 946768 946768 0 0.0
window-app BRD4161A (read only) 896908 896908 0 0.0
(read/write) 133432 133432 0 0.0
.bss 131384 131384 0 0.0
.data 2048 2048 0 0.0
.text 896900 896900 0 0.0
esp32 all-clusters-app c3devkit (read only) 1002948 1002950 2 0.0
(read/write) 1478906 1478906 0 0.0
.dram0.bss 69176 69176 0 0.0
.dram0.data 14624 14624 0 0.0
.flash.rodata 209672 209672 0 0.0
.flash.text 1002948 1002950 2 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1057747 1057747 0 0.0
(read/write) 480920 480920 0 0.0
.dram0.bss 74704 74704 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 240020 240020 0 0.0
.flash.text 1052363 1052363 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 682488 682488 0 0.0
.bss 80352 80352 0 0.0
.data 2016 2016 0 0.0
.text 598416 598416 0 0.0
lock k32w061+release (read/write) 728652 728652 0 0.0
.bss 80784 80784 0 0.0
.data 1976 1976 0 0.0
.text 644188 644188 0 0.0
linux all-clusters-app debug (read only) 2759513 2759529 16 0.0
(read/write) 177216 177216 0 0.0
.bss 86272 86272 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 82712 82712 0 0.0
.dynamic 608 608 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 1016 1016 0 0.0
.rodata 241437 241437 0 0.0
.text 2343474 2343490 16 0.0
bridge-app debug+rpc (read only) 2035241 2035241 0 0.0
(read/write) 148024 148024 0 0.0
.bss 73120 73120 0 0.0
.data 3936 3936 0 0.0
.data.rel.ro 65384 65384 0 0.0
.dynamic 592 592 0 0.0
.got 4272 4272 0 0.0
.init 27 27 0 0.0
.init_array 688 688 0 0.0
.rodata 169065 169065 0 0.0
.text 1709058 1709058 0 0.0
chip-tool debug (read only) 9488845 9488893 48 0.0
(read/write) 579704 579704 0 0.0
.bss 23936 23936 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 548328 548328 0 0.0
.dynamic 624 624 0 0.0
.got 5000 5000 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 485085 485085 0 0.0
.text 7675605 7675653 48 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9231604 9231652 48 0.0
(read/write) 645937 645937 0 0.0
.bss 42225 42225 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 583656 583656 0 0.0
.dynamic 560 560 0 0.0
.got 15016 15016 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 448884 448884 0 0.0
.text 7319332 7319380 48 0.0
lighting-app debug+rpc (read only) 2326953 2326969 16 0.0
(read/write) 153440 153440 0 0.0
.bss 74912 74912 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 70728 70728 0 0.0
.dynamic 608 608 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 792 792 0 0.0
.rodata 188073 188073 0 0.0
.text 1972530 1972546 16 0.0
lock-app debug (read only) 2240449 2240449 0 0.0
(read/write) 148152 148152 0 0.0
.bss 73600 73600 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 67272 67272 0 0.0
.dynamic 592 592 0 0.0
.got 4336 4336 0 0.0
.init 27 27 0 0.0
.init_array 752 752 0 0.0
.rodata 198713 198713 0 0.0
.text 1881778 1881778 0 0.0
ota-provider-app debug (read only) 2065657 2065673 16 0.0
(read/write) 141232 141232 0 0.0
.bss 73056 73056 0 0.0
.data 1768 1768 0 0.0
.data.rel.ro 60600 60600 0 0.0
.dynamic 608 608 0 0.0
.got 4504 4504 0 0.0
.init 27 27 0 0.0
.init_array 648 648 0 0.0
.rodata 179872 179872 0 0.0
.text 1727218 1727234 16 0.0
ota-requestor-app debug (read only) 2094609 2094625 16 0.0
(read/write) 144040 144040 0 0.0
.bss 73728 73728 0 0.0
.data 1992 1992 0 0.0
.data.rel.ro 62664 62664 0 0.0
.dynamic 592 592 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 672 672 0 0.0
.rodata 175840 175840 0 0.0
.text 1758738 1758754 16 0.0
shell debug (read only) 2575625 2575641 16 0.0
(read/write) 201240 201240 0 0.0
.bss 117192 117192 0 0.0
.data 1376 1376 0 0.0
.data.rel.ro 76912 76912 0 0.0
.dynamic 608 608 0 0.0
.got 4192 4192 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 222418 222418 0 0.0
.text 2191986 2192002 16 0.0
thermostat-no-ble arm64 (read only) 2357180 2357180 0 0.0
(read/write) 176849 176849 0 0.0
.bss 88113 88113 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 79416 79416 0 0.0
.dynamic 560 560 0 0.0
.got 4768 4768 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147596 147596 0 0.0
.text 1980560 1980560 0 0.0
tv-app debug (read only) 2856401 2856433 32 0.0
(read/write) 279072 279072 0 0.0
.bss 191032 191032 0 0.0
.data 4672 4672 0 0.0
.data.rel.ro 77120 77120 0 0.0
.dynamic 592 592 0 0.0
.got 4720 4720 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 221137 221137 0 0.0
.text 2453090 2453122 32 0.0
tv-casting-app debug (read only) 5450705 5450737 32 0.0
(read/write) 226048 226048 0 0.0
.bss 78808 78808 0 0.0
.data 2400 2400 0 0.0
.data.rel.ro 138632 138632 0 0.0
.dynamic 608 608 0 0.0
.got 4728 4728 0 0.0
.init 27 27 0 0.0
.init_array 864 864 0 0.0
.rodata 340737 340737 0 0.0
.text 4749250 4749282 32 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2418936 2418936 0 0.0
.bss 202796 202796 0 0.0
.data 5872 5872 0 0.0
.text 1381580 1381580 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1183339 1183339 0 0.0
bss 139348 139348 0 0.0
rodata 152820 152820 0 0.0
text 812372 812372 0 0.0
p6 all-clusters-app default (read/write) 2540360 2540360 0 0.0
.bss 137144 137144 0 0.0
.data 2808 2808 0 0.0
.text 1498624 1498624 0 0.0
light-app default (read/write) 2424216 2424216 0 0.0
.bss 129616 129616 0 0.0
.data 2608 2608 0 0.0
.text 1382480 1382480 0 0.0
lock-app default (read/write) 2434904 2434904 0 0.0
.bss 129432 129432 0 0.0
.data 2568 2568 0 0.0
.text 1393168 1393168 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782384 782384 0 0.0
bss 70744 70744 0 0.0
noinit 40416 40416 0 0.0
text 553428 553430 2 0.0
lighting-app tlsr9518adk80d (read/write) 802404 802404 0 0.0
bss 71000 71000 0 0.0
noinit 40416 40416 0 0.0
text 570140 570142 2 0.0

@isiu-apple isiu-apple merged commit 6c86c94 into project-chip:master May 23, 2022
@bzbarsky-apple bzbarsky-apple deleted the readclient-done-ref branch May 24, 2022 00:08
@andy31415
Copy link
Contributor

This seems to be the undo of #14013 (however partial since some changes were still needed ... eg #32257 looks to deal with lifecycle for OnError/OnDone).

Overall adding this comment to keep some track of history via github linking, however longer term we should try to remove the coupling that we have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anush-apple anush-apple Awaiting requested review from anush-apple

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@hawk248 hawk248 Awaiting requested review from hawk248

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@msandstedt msandstedt Awaiting requested review from msandstedt

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@tehampson tehampson Awaiting requested review from tehampson

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

Assignees
No one assigned
Labels
app controller darwin examples review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bzbarsky-apple @andy31415 @mrjerryjohns @isiu-apple