-
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
Fix status report handling in read handler #21374
Fix status report handling in read handler #21374
Conversation
Co-authored-by: Boris Zbarsky <[email protected]>
bd3e88f
to
c098a67
Compare
PR #21374: Size comparison from 9665a20 to c098a67 Increases (30 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
Decreases (7 builds for cc13x2_26x2, linux)
Full report (30 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
|
PR #21374: Size comparison from 9665a20 to 7e4859c Increases (30 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
Decreases (9 builds for cc13x2_26x2, esp32, linux)
Full report (30 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
|
7e4859c
to
c815a20
Compare
PR #21374: Size comparison from b52e56d to c815a20 Increases (27 builds for bl602, cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
Decreases (19 builds for cc13x2_26x2, linux)
Full report (34 builds for bl602, cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
|
c815a20
to
2493526
Compare
PR #21374: Size comparison from b52e56d to 2493526 Increases (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, p6, telink)
Decreases (19 builds for cc13x2_26x2, linux)
Full report (39 builds for bl602, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, p6, telink)
|
2493526
to
8fdfc06
Compare
PR #21374: Size comparison from b52e56d to 8fdfc06 Increases (34 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
Decreases (20 builds for cc13x2_26x2, esp32, linux)
Full report (41 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
|
8fdfc06
to
bd924a8
Compare
PR #21374: Size comparison from 71c74b6 to bd924a8 Increases (35 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
Decreases (20 builds for cc13x2_26x2, esp32, linux)
Full report (41 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
|
PR #21374: Size comparison from 71c74b6 to 1b57c8d Increases above 0.2%:
Increases (34 builds for bl602, cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
Decreases (8 builds for cc13x2_26x2, linux)
Full report (34 builds for bl602, cc13x2_26x2, cyw30739, k32w, linux, mbed, p6, telink)
|
1. Once message is passed to OnReadInitialRequest in read handler, if anything bad in incoming message, handler would send status report with invalid action, if there is not enough resource for paths in IM engines, we would send status report with exhansted path. 2. Fix fake build in linux, it seems this build don't have IM schema check. Usually without CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK, if schema error happens, we would get a little bit more detailed error, here, we add additional error check in TestReadHandlerInvalidAttributePath.
1b57c8d
to
ab11812
Compare
PR #21374: Size comparison from c3cc4a2 to ab11812 Increases (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (9 builds for cc13x2_26x2, esp32, linux)
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes requested while I review the major update.
3b71c55
to
4962d87
Compare
PR #21374: Size comparison from 4b366e2 to 4962d87 Increases (35 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (19 builds for cc13x2_26x2, linux)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
* Fix status report handling in read handler * address comments * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Update src/app/tests/TestReadInteraction.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Fix 1. Once message is passed to OnReadInitialRequest in read handler, if anything bad in incoming message, handler would send status report with invalid action, if there is not enough resource for paths in IM engines, we would send status report with exhansted path. 2. Fix fake build in linux, it seems this build don't have IM schema check. Usually without CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK, if schema error happens, we would get a little bit more detailed error, here, we add additional error check in TestReadHandlerInvalidAttributePath. * address comments Co-authored-by: Boris Zbarsky <[email protected]>
Problem
Fix status report in ReadHandler
This PR is cut from #19356
Change overview
When receiving status report in read handler, if message is malformed, the caller would send status report.
Refactor OnMessageReceived, it would send status report if sendStatusReport is set, and close the handler if any error is generated.
Testing
report, client sends out invalid status report message, handler sends status report with invalid action and close
report, client sends out unknown message, handler sends status report with invalid action and close.