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

[OTA-R] Correct the implementation of mProviderRetryCount #18908

Merged
merged 4 commits into from
Jun 2, 2022

Conversation

isiu-apple
Copy link
Contributor

@isiu-apple isiu-apple commented May 27, 2022

Problem

mProviderRetryCount was not implemented correctly. For example:

So the current code doesn't properly reset mProviderRetryCount after a successful download but image is not applied. Given that we now have two retry counters and they are incremented in different locations, we could be incrementing mProviderRetryCount even on an invalid session error:

  1. Requestor successfully sends QueryImage (CASE established and mProviderRetryCount incremented to 1)
  2. Provider is restarted
  3. Requestor sends QueryImage -> invalid session detected -> CASE torn down
  4. mInvalidSessionRetryCount incremented to 1 in HandleIdleStateEnter
  5. mProviderRetryCount incremented to 2 in SendQueryImage

Given the current example app, after (5), we could get a successful query/download but never applied the image. The next time we attempt to query (and if it were busy), mProviderRetryCount already started at 2 instead of 0 and it would at most retry one more time.

Fixes: #18824

Change overview

Reset mProviderRetryCount to zero when QueryImage download has succeeded.

Testing

Ran QueryImage tests and verified that mProviderRetryCount is reset to zero upon successful QueryImage download.

@isiu-apple isiu-apple changed the title Issue 18824 [OTA-R] Correct the implementation of mProviderRetryCount May 27, 2022
@isiu-apple isiu-apple self-assigned this May 27, 2022
@isiu-apple isiu-apple added the ota label May 27, 2022
@github-actions
Copy link

github-actions bot commented May 27, 2022

PR #18908: Size comparison from dbc91b7 to e7b3062

Increases (4 builds for cc13x2_26x2, efr32, linux, nrfconnect)
platform target config section dbc91b7 e7b3062 change % change
cc13x2_26x2 pump-controller-app LP_CC2652R7 (read only) 653783 653799 16 0.0
.text 570008 570024 16 0.0
efr32 lighting-app BRD4161A+rpc (read only) 948308 948324 16 0.0
.text 948300 948316 16 0.0
linux ota-requestor-app debug (read only) 2096449 2096465 16 0.0
.text 1760450 1760466 16 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1183423 1183439 16 0.0
text 811884 811888 4 0.0
Decreases (1 build for cc13x2_26x2)
platform target config section dbc91b7 e7b3062 change % change
cc13x2_26x2 pump-controller-app LP_CC2652R7 (read/write) 190336 190320 -16 -0.0
Full report (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section dbc91b7 e7b3062 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 645135 645135 0 0.0
(read/write) 159168 159168 0 0.0
.bss 74852 74852 0 0.0
.data 3400 3400 0 0.0
.rodata 83783 83783 0 0.0
.text 561116 561116 0 0.0
lock-ftd LP_CC2652R7 (read only) 679427 679427 0 0.0
(read/write) 163668 163668 0 0.0
.bss 72876 72876 0 0.0
.data 3264 3264 0 0.0
.rodata 96075 96075 0 0.0
.text 582868 582868 0 0.0
lock-mtd LP_CC2652R7 (read only) 628843 628843 0 0.0
(read/write) 145992 145992 0 0.0
.bss 68612 68612 0 0.0
.data 3264 3264 0 0.0
.rodata 95955 95955 0 0.0
.text 532396 532396 0 0.0
pump-app LP_CC2652R7 (read only) 675855 675855 0 0.0
(read/write) 168408 168408 0 0.0
.bss 73276 73276 0 0.0
.data 3300 3300 0 0.0
.rodata 88551 88551 0 0.0
.text 586820 586820 0 0.0
pump-controller-app LP_CC2652R7 (read only) 653783 653799 16 0.0
(read/write) 190336 190320 -16 -0.0
.bss 73132 73132 0 0.0
.data 3260 3260 0 0.0
.rodata 83295 83295 0 0.0
.text 570008 570024 16 0.0
shell LP_CC2652R7 (read only) 638134 638134 0 0.0
(read/write) 154724 154724 0 0.0
.bss 77204 77204 0 0.0
.data 3404 3404 0 0.0
.rodata 80758 80758 0 0.0
.text 557144 557144 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 621654 621654 0 0.0
.app_xip_area 524920 524920 0 0.0
.bss 79376 79376 0 0.0
.data 704 704 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 630370 630370 0 0.0
.app_xip_area 535108 535108 0 0.0
.bss 77936 77936 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) 570758 570758 0 0.0
.app_xip_area 465816 465816 0 0.0
.bss 87296 87296 0 0.0
.data 612 612 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 914152 914152 0 0.0
(read/write) 133464 133464 0 0.0
.bss 131368 131368 0 0.0
.data 2092 2092 0 0.0
.text 914144 914144 0 0.0
BRD4161A+rpc (read only) 948308 948324 16 0.0
(read/write) 150152 150152 0 0.0
.bss 147856 147856 0 0.0
.data 2296 2296 0 0.0
.text 948300 948316 16 0.0
BRD4161A+rs911x (read only) 788708 788708 0 0.0
(read/write) 129736 129736 0 0.0
.bss 127636 127636 0 0.0
.data 2100 2100 0 0.0
.text 788700 788700 0 0.0
lock-app BRD4161A+wf200 (read only) 954012 954012 0 0.0
(read/write) 128508 128508 0 0.0
.bss 126444 126444 0 0.0
.data 2064 2064 0 0.0
.text 954004 954004 0 0.0
window-app BRD4161A (read only) 899096 899096 0 0.0
(read/write) 133520 133520 0 0.0
.bss 131432 131432 0 0.0
.data 2088 2088 0 0.0
.text 899088 899088 0 0.0
esp32 all-clusters-app c3devkit (read only) 1002360 1002360 0 0.0
(read/write) 1479362 1479362 0 0.0
.dram0.bss 69416 69416 0 0.0
.dram0.data 14640 14640 0 0.0
.flash.rodata 209872 209872 0 0.0
.flash.text 1002360 1002360 0 0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1057611 1057611 0 0.0
(read/write) 481616 481616 0 0.0
.dram0.bss 74944 74944 0 0.0
.dram0.data 34208 34208 0 0.0
.flash.rodata 240468 240468 0 0.0
.flash.text 1052227 1052227 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 680892 680892 0 0.0
.bss 80440 80440 0 0.0
.data 2008 2008 0 0.0
.text 596740 596740 0 0.0
lock k32w061+release (read/write) 732216 732216 0 0.0
.bss 80872 80872 0 0.0
.data 1976 1976 0 0.0
.text 647664 647664 0 0.0
linux all-clusters-app debug (read only) 2752057 2752057 0 0.0
(read/write) 178336 178336 0 0.0
.bss 86528 86528 0 0.0
.data 2032 2032 0 0.0
.data.rel.ro 83608 83608 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 242205 242205 0 0.0
.text 2336258 2336258 0 0.0
bridge-app debug+rpc (read only) 2024593 2024593 0 0.0
(read/write) 148024 148024 0 0.0
.bss 73184 73184 0 0.0
.data 3936 3936 0 0.0
.data.rel.ro 65320 65320 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 168256 168256 0 0.0
.text 1700178 1700178 0 0.0
chip-tool debug (read only) 9680581 9680581 0 0.0
(read/write) 602512 602512 0 0.0
.bss 23968 23968 0 0.0
.data 1120 1120 0 0.0
.data.rel.ro 571136 571136 0 0.0
.dynamic 624 624 0 0.0
.got 5008 5008 0 0.0
.init 27 27 0 0.0
.init_array 648 648 0 0.0
.rodata 497213 497213 0 0.0
.text 7790725 7790725 0 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9424172 9424172 0 0.0
(read/write) 668849 668849 0 0.0
.bss 42257 42257 0 0.0
.data 1176 1176 0 0.0
.data.rel.ro 606552 606552 0 0.0
.dynamic 560 560 0 0.0
.got 15024 15024 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 460916 460916 0 0.0
.text 7435108 7435108 0 0.0
lighting-app debug+rpc (read only) 2314961 2314961 0 0.0
(read/write) 153632 153632 0 0.0
.bss 75008 75008 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 70824 70824 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 186952 186952 0 0.0
.text 1962642 1962642 0 0.0
lock-app debug (read only) 2254073 2254073 0 0.0
(read/write) 148728 148728 0 0.0
.bss 73696 73696 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 67752 67752 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 200328 200328 0 0.0
.text 1893810 1893810 0 0.0
ota-provider-app debug (read only) 2067233 2067233 0 0.0
(read/write) 141456 141456 0 0.0
.bss 73056 73056 0 0.0
.data 1768 1768 0 0.0
.data.rel.ro 60824 60824 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 179960 179960 0 0.0
.text 1728706 1728706 0 0.0
ota-requestor-app debug (read only) 2096449 2096465 16 0.0
(read/write) 144296 144296 0 0.0
.bss 73760 73760 0 0.0
.data 1960 1960 0 0.0
.data.rel.ro 62920 62920 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 175968 175968 0 0.0
.text 1760450 1760466 16 0.0
shell debug (read only) 2556401 2556401 0 0.0
(read/write) 201840 201840 0 0.0
.bss 117448 117448 0 0.0
.data 1376 1376 0 0.0
.data.rel.ro 77256 77256 0 0.0
.dynamic 608 608 0 0.0
.got 4192 4192 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 222386 222386 0 0.0
.text 2174930 2174930 0 0.0
thermostat-no-ble arm64 (read only) 2360108 2360108 0 0.0
(read/write) 177601 177601 0 0.0
.bss 88209 88209 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 80032 80032 0 0.0
.dynamic 560 560 0 0.0
.got 4808 4808 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147532 147532 0 0.0
.text 1983216 1983216 0 0.0
tv-app debug (read only) 2875745 2875745 0 0.0
(read/write) 280464 280464 0 0.0
.bss 191368 191368 0 0.0
.data 4672 4672 0 0.0
.data.rel.ro 78152 78152 0 0.0
.dynamic 592 592 0 0.0
.got 4728 4728 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 221888 221888 0 0.0
.text 2471442 2471442 0 0.0
tv-casting-app debug (read only) 5426689 5426689 0 0.0
(read/write) 226160 226160 0 0.0
.bss 78952 78952 0 0.0
.data 2400 2400 0 0.0
.data.rel.ro 138576 138576 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 339136 339136 0 0.0
.text 4728658 4728658 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2423968 2423968 0 0.0
.bss 202892 202892 0 0.0
.data 5872 5872 0 0.0
.text 1386612 1386612 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1183423 1183439 16 0.0
bss 139552 139552 0 0.0
rodata 153112 153112 0 0.0
text 811884 811888 4 0.0
p6 all-clusters-app default (read/write) 2536808 2536808 0 0.0
.bss 137376 137376 0 0.0
.data 2800 2800 0 0.0
.text 1495072 1495072 0 0.0
light-app default (read/write) 2420064 2420064 0 0.0
.bss 129712 129712 0 0.0
.data 2600 2600 0 0.0
.text 1378328 1378328 0 0.0
lock-app default (read/write) 2438112 2438112 0 0.0
.bss 129520 129520 0 0.0
.data 2568 2568 0 0.0
.text 1396376 1396376 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 779580 779580 0 0.0
bss 70840 70840 0 0.0
noinit 40416 40416 0 0.0
text 551256 551256 0 0.0
lighting-app tlsr9518adk80d (read/write) 799612 799612 0 0.0
bss 71100 71100 0 0.0
noinit 40416 40416 0 0.0
text 567990 567990 0 0.0

- Remove reset from DefaultOTARequestorDriver::UpdateDownloaded()
@carol-apple carol-apple merged commit dce3d8a into project-chip:master Jun 2, 2022
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.

[OTA-R] mProviderRetryCount is not implemented correctly
3 participants