-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[K32W] Fix apply action corner case in OTATlvProcessor interface #33214
Merged
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,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
harimau-qirex,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kkasperczyk-no,
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda,
mhazley and
mkardous-silabs
April 29, 2024 09:57
pullapprove
bot
requested review from
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
April 29, 2024 09:57
PR #33214: Size comparison from 040e5bf to 54239ff Increases (3 builds for nxp)
Decreases (2 builds for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
andy31415
reviewed
Apr 29, 2024
andy31415
reviewed
Apr 29, 2024
OTATlvProcessor::ApplyAction now has a default implementation, but derived classes are still able to overwrite this. The flag is used by the default ApplyAction implementation. If something goes wrong during ExitAction of the TLV processor, then mShouldNotApply should be set to true and the image processor should abort. In this case, the BDX transfer was already finished and calling CancelImageUpdate will not abort the transfer, hence the device will reboot even though it should not have. If ApplyAction fails during HandleApply, then the process will be aborted. Signed-off-by: marius-alex-tache <[email protected]>
During ExitAction, set mShouldNotApply to true if an error occurs. This ensures that the OTA will be aborted and the device does not reboot. Also remove the ApplyAction override, since the default implementation is enough. Signed-off-by: marius-alex-tache <[email protected]>
Signed-off-by: marius-alex-tache <[email protected]>
All OTA errors should be prefixed with CHIP_ERROR. Signed-off-by: marius-alex-tache <[email protected]>
Signed-off-by: marius-alex-tache <[email protected]>
marius-alex-tache
force-pushed
the
k32w-fix-ota-corner-case
branch
from
May 8, 2024 07:45
e3211ff
to
0f94347
Compare
PR #33214: Size comparison from e887e58 to 0f94347 Increases (3 builds for nxp)
Decreases (2 builds for efr32)
Full report (77 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
marius-alex-tache
changed the title
[K32W] Add mShouldNotApply flag in OTATlvProcessor interface
[K32W] Fix apply action corner case in OTATlvProcessor interface
May 8, 2024
andy31415
approved these changes
May 13, 2024
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.
OTATlvProcessor::ApplyAction
now has a default implementation, but derived classes are still able to overwrite it.The flag is used by the default
ApplyAction
implementation. If something goes wrong duringExitAction
of the TLV processor,then
mShouldNotApply
should be set totrue
and the image processor should abort. In this case, the BDX transfer was already finished and callingCancelImageUpdate
will not abort the transfer, hence the device will reboot even though it should not have. IfApplyAction
fails duringHandleApply
, then the process will be aborted.