-
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
[IM] Make CommandHandler::AddResponseData atomic #15906
Conversation
8d1cc1a
to
3ac709c
Compare
PR #15906: Size comparison from b18b03a to 3ac709c Increases above 0.2%:
Increases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
367dcf9
to
5393fd6
Compare
5393fd6
to
b641b12
Compare
PR #15906: Size comparison from b18b03a to b641b12 Increases above 0.2%:
Increases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (4 builds for esp32, linux, nrfconnect)
Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
@bzbarsky-apple - are your comments addressed? The comment history seems to indicate that. |
Will review tomorrow. |
examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
Outdated
Show resolved
Hide resolved
examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
Outdated
Show resolved
Hide resolved
PR #15906: Size comparison from 58cd5fb to d9c959d Increases above 0.2%:
Increases (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (4 builds for esp32, linux, nrfconnect)
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #15906: Size comparison from 58cd5fb to 7be9a8d Increases above 0.2%:
Increases (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (2 builds for esp32)
Full report (18 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
* Add encode failure tests * [IM] Make CommandHandler::AddResponseData atomic * Reset -> Rollback * Rename: AddResponseData -> AddResponse * Address comments * Update cluster code * 50 ms is too short on CI * Fix * Fix
* Add encode failure tests * [IM] Make CommandHandler::AddResponseData atomic * Reset -> Rollback * Rename: AddResponseData -> AddResponse * Address comments * Update cluster code * 50 ms is too short on CI * Fix * Fix
Problem
Fixes #15685
CommandHandler::AddResponseData is not atomic, then we cannot encode status since the backed buffer might be corrupted and we are in a wrong state.
Change overview
ResetResponse
Testing
TestCommandHandlerCommandEncodeFailure
, andTestCommandHandlerCommandEncodeExternalFailure