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

OperationalDeviceProxy should not inherit from SessionEstablishmentDelegate. #17472

Closed
wants to merge 1 commit into from
Closed

OperationalDeviceProxy should not inherit from SessionEstablishmentDelegate. #17472

wants to merge 1 commit into from

Conversation

bzbarsky-apple
Copy link
Contributor

It does not implement any of the delegate methods and doesn't call them.

Problem

See above.

Change overview

Stop inheriting from a delegate interface we don't implement.

Testing

No behavior changes, just slightly smaller object.

@bzbarsky-apple
OperationalDeviceProxy should not inherit from SessionEstablishmentDe…
0dc3ed6 
…legate.

It does not implement any of the delegate methods and doesn't call them.
@boring-cyborg boring-cyborg bot added the app label Apr 18, 2022
@github-actions
Copy link

github-actions bot commented Apr 18, 2022 •
edited
Loading

PR #17472: Size comparison from 375fff3 to 0dc3ed6

Increases (2 builds for cc13x2_26x2)
platform target config section 375fff3 0dc3ed6 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 168408 168480 72 0.0
lock-ftd LP_CC2652R7 (read/write) 166432 166504 72 0.0
Decreases (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 375fff3 0dc3ed6 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 684775 684703 -72 -0.0
.rodata 103415 103383 -32 -0.0
.text 580880 580840 -40 -0.0
lock-ftd LP_CC2652R7 (read only) 677695 677623 -72 -0.0
.rodata 98903 98871 -32 -0.0
.text 578308 578268 -40 -0.0
lock-mtd LP_CC2652R7 (read only) 626439 626375 -64 -0.0
.rodata 98783 98751 -32 -0.0
.text 527164 527132 -32 -0.0
pump-app LP_CC2652R7 (read only) 649831 649759 -72 -0.0
.rodata 75719 75687 -32 -0.0
.text 573624 573584 -40 -0.0
pump-controller-app LP_CC2652R7 (read only) 643167 643103 -64 -0.0
.rodata 79055 79031 -24 -0.0
.text 563624 563584 -40 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 619302 619238 -64 -0.0
.app_xip_area 526060 525996 -64 -0.0
lock cyw930739m2evb_01 (read/write) 613562 613498 -64 -0.0
.app_xip_area 521824 521760 -64 -0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 566398 566334 -64 -0.0
.app_xip_area 465048 464984 -64 -0.0
efr32 lighting-app BRD4161A (read only) 909172 909092 -80 -0.0
.text 909164 909084 -80 -0.0
BRD4161A+rpc (read only) 943548 943476 -72 -0.0
.text 943540 943468 -72 -0.0
window-app BRD4161A (read only) 845540 845476 -64 -0.0
.text 845532 845468 -64 -0.0
esp32 all-clusters-app c3devkit (read only) 980666 980630 -36 -0.0
(read/write) 1397866 1397842 -24 -0.0
.flash.rodata 201936 201912 -24 -0.0
.flash.text 980666 980630 -36 -0.0
m5stack (read only) 1036119 1036067 -52 -0.0
(read/write) 465608 465576 -32 -0.0
.flash.rodata 231508 231476 -32 -0.0
.flash.text 1030735 1030683 -52 -0.0
k32w light k32w061+release (read/write) 684884 684804 -80 -0.0
.text 599180 599100 -80 -0.0
lock k32w061+release (read/write) 725732 725676 -56 -0.0
.text 639460 639404 -56 -0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 10430092 10429868 -224 -0.0
(read/write) 494193 494145 -48 -0.0
.data.rel.ro 390440 390384 -56 -0.0
.text 8804420 8804324 -96 -0.0
thermostat-no-ble arm64 (read only) 2359996 2359772 -224 -0.0
(read/write) 151137 151073 -64 -0.0
.data.rel.ro 78984 78928 -56 -0.1
.text 1985648 1985552 -96 -0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2410364 2410268 -96 -0.0
.text 1372964 1372868 -96 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1164707 1164647 -60 -0.0
rodata 147572 147544 -28 -0.0
text 801976 801948 -28 -0.0
p6 all-clusters-app default (read/write) 2517224 2517144 -80 -0.0
.text 1475488 1475408 -80 -0.0
light-app default (read/write) 2417144 2417064 -80 -0.0
.text 1375408 1375328 -80 -0.0
lock-app default (read/write) 2420608 2420544 -64 -0.0
.text 1378872 1378808 -64 -0.0
telink lighting-app tlsr9518adk80d (read/write) 802172 802112 -60 -0.0
text 570732 570696 -36 -0.0
Full report (23 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 375fff3 0dc3ed6 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 684775 684703 -72 -0.0
(read/write) 168408 168480 72 0.0
.bss 76168 76168 0 0.0
.data 3380 3380 0 0.0
.rodata 103415 103383 -32 -0.0
.text 580880 580840 -40 -0.0
lock-ftd LP_CC2652R7 (read only) 677695 677623 -72 -0.0
(read/write) 166432 166504 72 0.0
.bss 74168 74168 0 0.0
.data 3212 3212 0 0.0
.rodata 98903 98871 -32 -0.0
.text 578308 578268 -40 -0.0
lock-mtd LP_CC2652R7 (read only) 626439 626375 -64 -0.0
(read/write) 146956 146956 0 0.0
.bss 69888 69888 0 0.0
.data 3212 3212 0 0.0
.rodata 98783 98751 -32 -0.0
.text 527164 527132 -32 -0.0
pump-app LP_CC2652R7 (read only) 649831 649759 -72 -0.0
(read/write) 152492 152492 0 0.0
.bss 74624 74624 0 0.0
.data 3244 3244 0 0.0
.rodata 75719 75687 -32 -0.0
.text 573624 573584 -40 -0.0
pump-controller-app LP_CC2652R7 (read only) 643167 643103 -64 -0.0
(read/write) 152160 152160 0 0.0
.bss 74328 74328 0 0.0
.data 3208 3208 0 0.0
.rodata 79055 79031 -24 -0.0
.text 563624 563584 -40 -0.0
cyw30739 light cyw930739m2evb_01 (read/write) 619302 619238 -64 -0.0
.app_xip_area 526060 525996 -64 -0.0
.bss 75908 75908 0 0.0
.data 684 684 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 613562 613498 -64 -0.0
.app_xip_area 521824 521760 -64 -0.0
.bss 74436 74436 0 0.0
.data 648 648 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 566398 566334 -64 -0.0
.app_xip_area 465048 464984 -64 -0.0
.bss 83752 83752 0 0.0
.data 564 564 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 909172 909092 -80 -0.0
(read/write) 133128 133128 0 0.0
.bss 131088 131088 0 0.0
.data 2040 2040 0 0.0
.text 909164 909084 -80 -0.0
BRD4161A+rpc (read only) 943548 943476 -72 -0.0
(read/write) 149812 149812 0 0.0
.bss 147568 147568 0 0.0
.data 2244 2244 0 0.0
.text 943540 943468 -72 -0.0
window-app BRD4161A (read only) 845540 845476 -64 -0.0
(read/write) 131116 131116 0 0.0
.bss 129168 129168 0 0.0
.data 1948 1948 0 0.0
.text 845532 845468 -64 -0.0
esp32 all-clusters-app c3devkit (read only) 980666 980630 -36 -0.0
(read/write) 1397866 1397842 -24 -0.0
.dram0.bss 62600 62600 0 0.0
.dram0.data 14412 14412 0 0.0
.flash.rodata 201936 201912 -24 -0.0
.flash.text 980666 980630 -36 -0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1036119 1036067 -52 -0.0
(read/write) 465608 465576 -32 -0.0
.dram0.bss 68112 68112 0 0.0
.dram0.data 34152 34152 0 0.0
.flash.rodata 231508 231476 -32 -0.0
.flash.text 1030735 1030683 -52 -0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 684884 684804 -80 -0.0
.bss 77912 77912 0 0.0
.data 1992 1992 0 0.0
.text 599180 599100 -80 -0.0
lock k32w061+release (read/write) 725732 725676 -56 -0.0
.bss 78520 78520 0 0.0
.data 1952 1952 0 0.0
.text 639460 639404 -56 -0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 10430092 10429868 -224 -0.0
(read/write) 494193 494145 -48 -0.0
.bss 40865 40865 0 0.0
.data 1184 1184 0 0.0
.data.rel.ro 390440 390384 -56 -0.0
.dynamic 560 560 0 0.0
.got 57904 57904 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 516652 516652 0 0.0
.text 8804420 8804324 -96 -0.0
thermostat-no-ble arm64 (read only) 2359996 2359772 -224 -0.0
(read/write) 151137 151073 -64 -0.0
.bss 62945 62945 0 0.0
.data 1440 1440 0 0.0
.data.rel.ro 78984 78928 -56 -0.1
.dynamic 560 560 0 0.0
.got 4752 4752 0 0.0
.init 24 24 0 0.0
.init_array 368 368 0 0.0
.rodata 145356 145356 0 0.0
.text 1985648 1985552 -96 -0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2410364 2410268 -96 -0.0
.bss 185228 185228 0 0.0
.data 5840 5840 0 0.0
.text 1372964 1372868 -96 -0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1164707 1164647 -60 -0.0
bss 136520 136520 0 0.0
rodata 147572 147544 -28 -0.0
text 801976 801948 -28 -0.0
p6 all-clusters-app default (read/write) 2517224 2517144 -80 -0.0
.bss 118624 118624 0 0.0
.data 2768 2768 0 0.0
.text 1475488 1475408 -80 -0.0
light-app default (read/write) 2417144 2417064 -80 -0.0
.bss 112104 112104 0 0.0
.data 2576 2576 0 0.0
.text 1375408 1375328 -80 -0.0
lock-app default (read/write) 2420608 2420544 -64 -0.0
.bss 111880 111880 0 0.0
.data 2536 2536 0 0.0
.text 1378872 1378808 -64 -0.0
telink lighting-app tlsr9518adk80d (read/write) 802172 802112 -60 -0.0
bss 69952 69952 0 0.0
noinit 40416 40416 0 0.0
text 570732 570696 -36 -0.0

kghost
kghost suggested changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@kghost kghost left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Actually it should, see my solution in PR #17422

We should replacce HandleCASEConnected/HandleCASEConnectionFailure with SessionEstablishmentDelegate callbacks

@bzbarsky-apple
Copy link
Contributor Author

Actually it should, see my solution in PR #17422

That's fine. If we're actually using it as a delegate, great. We're just not right now....

@bzbarsky-apple bzbarsky-apple deleted the operational-proxy-not-session-establishment-delegate branch April 19, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 Awaiting requested review from andy31415

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

@austinh0 austinh0 Awaiting requested review from austinh0

@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

@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

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

@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

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@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

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

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

@kghost kghost Awaiting requested review from kghost

@wbschiller wbschiller Awaiting requested review from wbschiller

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

@xylophone21 xylophone21 Awaiting requested review from xylophone21

Assignees
No one assigned
Labels
app review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bzbarsky-apple @kghost