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

[OTA] Make OTAProviderDelegate responsible for responding to commands #16654

Merged
merged 1 commit into from
Apr 1, 2022
Merged

[OTA] Make OTAProviderDelegate responsible for responding to commands #16654

merged 1 commit into from
Apr 1, 2022

Conversation

carol-apple
Copy link
Contributor

Problem

The responsibility of whether the provider cluster or the delegate is sending the response and/or status was not well defined.

Fixes: #15889

Change overview

  • Make the OTAProviderDelegate responsible for sending both the response (for success cases) and status (for error cases)
  • Clearly spell this out in the delegate header file

Testing

  • Verified that all response/status through a normal OTA transfer flow is still working as expected

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR #16654: Size comparison from 4ea2bae to b2545c5

Decreases (1 build for linux)
platform target config section 4ea2bae b2545c5 change % change
linux thermostat-no-ble arm64 (read only) 2269668 2269460 -208 -0.0
.text 1906544 1906336 -208 -0.0
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 4ea2bae b2545c5 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 604482 604482 0 0.0
.app_xip_area 511652 511652 0 0.0
.bss 75576 75576 0 0.0
.data 604 604 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 562270 562270 0 0.0
.app_xip_area 470968 470968 0 0.0
.bss 74080 74080 0 0.0
.data 568 568 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 574634 574634 0 0.0
.app_xip_area 473684 473684 0 0.0
.bss 83408 83408 0 0.0
.data 508 508 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 925504 925504 0 0.0
(read/write) 128688 128688 0 0.0
.bss 126688 126688 0 0.0
.data 1996 1996 0 0.0
.text 925496 925496 0 0.0
BRD4161A+rpc (read only) 954320 954320 0 0.0
(read/write) 144640 144640 0 0.0
.bss 142464 142464 0 0.0
.data 2176 2176 0 0.0
.text 954312 954312 0 0.0
window-app BRD4161A (read only) 854900 854900 0 0.0
(read/write) 126648 126648 0 0.0
.bss 124776 124776 0 0.0
.data 1872 1872 0 0.0
.text 854892 854892 0 0.0
esp32 all-clusters-app c3devkit (read only) 965630 965630 0 0.0
(read/write) 1394162 1394162 0 0.0
.dram0.bss 61992 61992 0 0.0
.dram0.data 14212 14212 0 0.0
.flash.rodata 199032 199032 0 0.0
.flash.text 965630 965630 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1022107 1022107 0 0.0
(read/write) 461908 461908 0 0.0
.dram0.bss 67520 67520 0 0.0
.dram0.data 34024 34024 0 0.0
.flash.rodata 228528 228528 0 0.0
.flash.text 1016723 1016723 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 703000 703000 0 0.0
.bss 77568 77568 0 0.0
.data 1876 1876 0 0.0
.text 617756 617756 0 0.0
lock k32w061+release (read/write) 702068 702068 0 0.0
.bss 77544 77544 0 0.0
.data 1916 1916 0 0.0
.text 616808 616808 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9836652 9836652 0 0.0
(read/write) 473889 473889 0 0.0
.bss 40769 40769 0 0.0
.data 1144 1144 0 0.0
.data.rel.ro 372712 372712 0 0.0
.dynamic 560 560 0 0.0
.got 55464 55464 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 494020 494020 0 0.0
.text 8289012 8289012 0 0.0
thermostat-no-ble arm64 (read only) 2269668 2269460 -208 -0.0
(read/write) 148385 148385 0 0.0
.bss 62833 62833 0 0.0
.data 1040 1040 0 0.0
.data.rel.ro 77000 77000 0 0.0
.dynamic 560 560 0 0.0
.got 4504 4504 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 139716 139716 0 0.0
.text 1906544 1906336 -208 -0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2354708 2354708 0 0.0
.bss 184572 184572 0 0.0
.data 5760 5760 0 0.0
.text 1317308 1317308 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1140943 1140943 0 0.0
bss 142500 142500 0 0.0
rodata 142040 142040 0 0.0
text 781524 781524 0 0.0
p6 all-clusters-app default (read/write) 2496728 2496728 0 0.0
.bss 117992 117992 0 0.0
.data 2640 2640 0 0.0
.text 1454992 1454992 0 0.0
light-app default (read/write) 2399104 2399104 0 0.0
.bss 111464 111464 0 0.0
.data 2496 2496 0 0.0
.text 1357368 1357368 0 0.0
lock-app default (read/write) 2362632 2362632 0 0.0
.bss 111208 111208 0 0.0
.data 2456 2456 0 0.0
.text 1320896 1320896 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 897470 897470 0 0.0
bss 87356 87356 0 0.0
noinit 37160 37160 0 0.0
text 634874 634874 0 0.0

@carol-apple carol-apple force-pushed the ota_provider_delegate_responses branch 3 times, most recently from 13d92bd to d88c3a6 Compare April 1, 2022 03:59
@carol-apple carol-apple force-pushed the ota_provider_delegate_responses branch from d88c3a6 to 0e0d8f2 Compare April 1, 2022 04:01
@github-actions
Copy link

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

PR #16654: Size comparison from 3da3aa8 to 0e0d8f2

Decreases (1 build for linux)
platform target config section 3da3aa8 0e0d8f2 change % change
linux thermostat-no-ble arm64 (read only) 2292692 2292484 -208 -0.0
.text 1926704 1926496 -208 -0.0
Full report (22 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 3da3aa8 0e0d8f2 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 667427 667427 0 0.0
(read/write) 184316 184316 0 0.0
.bss 81784 81784 0 0.0
.data 3132 3132 0 0.0
.rodata 79595 79595 0 0.0
.text 587352 587352 0 0.0
lock-mtd LP_CC2652R7 (read only) 616555 616555 0 0.0
(read/write) 154500 154500 0 0.0
.bss 77512 77512 0 0.0
.data 3132 3132 0 0.0
.rodata 79475 79475 0 0.0
.text 536592 536592 0 0.0
pump-app LP_CC2652R7 (read only) 686903 686903 0 0.0
(read/write) 166000 166000 0 0.0
.bss 82176 82176 0 0.0
.data 3164 3164 0 0.0
.rodata 81671 81671 0 0.0
.text 604748 604748 0 0.0
pump-controller-app LP_CC2652R7 (read only) 669159 669159 0 0.0
(read/write) 183488 183488 0 0.0
.bss 81920 81920 0 0.0
.data 3128 3128 0 0.0
.rodata 78007 78007 0 0.0
.text 590668 590668 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 609642 609642 0 0.0
.app_xip_area 516384 516384 0 0.0
.bss 76004 76004 0 0.0
.data 600 600 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 567154 567154 0 0.0
.app_xip_area 475432 475432 0 0.0
.bss 74508 74508 0 0.0
.data 564 564 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 578934 578934 0 0.0
.app_xip_area 477556 477556 0 0.0
.bss 83836 83836 0 0.0
.data 504 504 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 914856 914856 0 0.0
(read/write) 129776 129776 0 0.0
.bss 127816 127816 0 0.0
.data 1960 1960 0 0.0
.text 914848 914848 0 0.0
BRD4161A+rpc (read only) 942720 942720 0 0.0
(read/write) 145736 145736 0 0.0
.bss 143592 143592 0 0.0
.data 2140 2140 0 0.0
.text 942712 942712 0 0.0
window-app BRD4161A (read only) 850152 850152 0 0.0
(read/write) 127784 127784 0 0.0
.bss 125944 125944 0 0.0
.data 1840 1840 0 0.0
.text 850144 850144 0 0.0
esp32 all-clusters-app c3devkit (read only) 970498 970498 0 0.0
(read/write) 1395138 1395138 0 0.0
.dram0.bss 62456 62456 0 0.0
.dram0.data 14220 14220 0 0.0
.flash.rodata 199544 199544 0 0.0
.flash.text 970498 970498 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1026227 1026227 0 0.0
(read/write) 462900 462900 0 0.0
.dram0.bss 67984 67984 0 0.0
.dram0.data 34024 34024 0 0.0
.flash.rodata 229056 229056 0 0.0
.flash.text 1020843 1020843 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 707240 707240 0 0.0
.bss 77992 77992 0 0.0
.data 1872 1872 0 0.0
.text 621576 621576 0 0.0
lock k32w061+release (read/write) 706504 706504 0 0.0
.bss 77960 77960 0 0.0
.data 1912 1912 0 0.0
.text 620832 620832 0 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9973684 9973684 0 0.0
(read/write) 477361 477361 0 0.0
.bss 40401 40401 0 0.0
.data 1136 1136 0 0.0
.data.rel.ro 375464 375464 0 0.0
.dynamic 560 560 0 0.0
.got 56552 56552 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 505356 505356 0 0.0
.text 8405828 8405828 0 0.0
thermostat-no-ble arm64 (read only) 2292692 2292484 -208 -0.0
(read/write) 148657 148657 0 0.0
.bss 62849 62849 0 0.0
.data 1040 1040 0 0.0
.data.rel.ro 77216 77216 0 0.0
.dynamic 560 560 0 0.0
.got 4536 4536 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 142052 142052 0 0.0
.text 1926704 1926496 -208 -0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2358540 2358540 0 0.0
.bss 185052 185052 0 0.0
.data 5760 5760 0 0.0
.text 1321140 1321140 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1147843 1147843 0 0.0
bss 143092 143092 0 0.0
rodata 143204 143204 0 0.0
text 786740 786740 0 0.0
p6 all-clusters-app default (read/write) 2502872 2502872 0 0.0
.bss 118488 118488 0 0.0
.data 2640 2640 0 0.0
.text 1461136 1461136 0 0.0
light-app default (read/write) 2404360 2404360 0 0.0
.bss 111944 111944 0 0.0
.data 2496 2496 0 0.0
.text 1362624 1362624 0 0.0
lock-app default (read/write) 2367968 2367968 0 0.0
.bss 111688 111688 0 0.0
.data 2456 2456 0 0.0
.text 1326232 1326232 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 791300 791300 0 0.0
bss 70296 70296 0 0.0
noinit 40416 40416 0 0.0
text 561264 561264 0 0.0

@Damian-Nordic Damian-Nordic merged commit 5dcb123 into project-chip:master Apr 1, 2022
chencheung pushed a commit to chencheung/connectedhomeip that referenced this pull request Apr 6, 2022
@carol-apple @chencheung
[OTA] Make OTAProviderDelegate responsible for responding to commands (…
33d6e8c 
…project-chip#16654)
chencheung pushed a commit to chencheung/connectedhomeip that referenced this pull request Apr 6, 2022
@carol-apple @chencheung
[OTA] Make OTAProviderDelegate responsible for responding to commands (…
0962984 
…project-chip#16654)
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@carol-apple @andrei-menzopol
[OTA] Make OTAProviderDelegate responsible for responding to commands (…
0862b98 
…project-chip#16654)
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

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@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

@kghost kghost Awaiting requested review from kghost

@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

@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

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@wbschiller wbschiller Awaiting requested review from wbschiller

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

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

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

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

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

Successfully merging this pull request may close these issues.

OTAProviderDelegate needs to clearly define who is responsible for responding
4 participants
@carol-apple @woody-apple @bzbarsky-apple @Damian-Nordic