Issue #11760 - [ota-requestor-app] Abort query image download on timeout #15768
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
andy31415,
anush-apple,
balducci-apple,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
erjiaqing,
franck-apple,
gjc13,
hawk248,
holbrookt,
harsha-rajendran,
jelderton,
jepenven-silabs,
jmartinez-silabs,
lazarkov,
LuDuda,
lzgrablic02,
mrjerryjohns and
msandstedt
|
PR #15768: Size comparison from 3b83461 to 9b21b94 Increases above 0.2%:
Increases (11 builds for cyw30739, efr32, k32w, linux, nrfconnect, qpg)
Full report (31 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
isiu-apple
force-pushed
the
issue_11760_b
branch
from
9b21b94 to
0c6e727
Compare
bzbarsky-apple
approved these changes
Mar 4, 2022
|
/rebase |
1 similar comment
|
/rebase |
selissia
approved these changes
Mar 4, 2022
…ut midway in transfer
- Moved mTimeout to BDXDownloader class. - Moved prevPercentageComplete to BDXDownloader class. - Renamed StartTimeoutTimerHandler to TransferTimeoutCheckHandler. - Added sanity checks.
- Moved mTimeoutSec to a constant define kTimeoutSec. - Moved resetting of timeout timer and prevPercentageComplete to a Reset() function.
- Renamed prevPercentageComplete to mPrevPercentageComplete
- Renmed CheckTransferTimeout() to HasTransferTimedOut(). - Calculate timeout using block counter instead of percentage complete. - Use 5*60 instead of 300 for timeout value for readability.
isiu-apple
force-pushed
the
issue_11760_b
branch
from
0c6e727 to
ff3116f
Compare
|
PR #15768: Size comparison from 29eec86 to ff3116f Increases above 0.2%:
Increases (11 builds for cyw30739, efr32, k32w, linux, nrfconnect, qpg)
Full report (31 builds for cyw30739, 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
Currently, if OTA-P crashes or somehow stops responding during a QueryImage download, OTA-R app waits forever.
Fixes: #11760
Fixes: #14854
Change overview
Added a 5 minute timer in BDX downloader which checks whether there's been progress made in the QueryImage download session. If there's been progress, set the timer to check again. If there's been no progress, that means OTA-P hasn't been responsive, so OTA-R will abort the session.
This change addresses this part of the spec:
Section 11.20.3.5 Pg 665: Idle time-out SHALL be no less than 5 minutes for either Receiver-Driver or Asynchronous mode, before aborting a transfer.
Testing
Example:
Control-C to kill OTA-P app. Then verify OTA-R logs after ~5 minutes:
Chip-tool logs: