-
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
Cluster object based reads on the client. #10524
Cluster object based reads on the client. #10524
Conversation
Figure out why I cannot enable this line below.connectedhomeip/src/controller/tests/data_model/TestRead.cpp Lines 226 to 236 in ffe65a7
This comment was generated by todo based on a
|
Figure out why I cannot enable this line below.connectedhomeip/src/controller/tests/data_model/TestRead.cpp Lines 237 to 247 in ffe65a7
This comment was generated by todo based on a
|
4890fbc
to
09339d0
Compare
84a43b0
to
d80720a
Compare
Like the TypedCommandCallback and InvokeCommand API added before, this adds a TypedReadCallback and ReadAttribute API that utilizes cluster objects to deliver attribute data to a reader. This utilizes the attribute type information struct that is code-generated currently to avoid needing callers to have to specify static schema information like the Cluster or Attribute Ids. This also avoids the use of gCallbackMgr and the entire dynamic dispatch logic contained within it given it's highly unsafe usage of reinterpret_casts, and achieves dispatch very compactly and safely within the implementation of the ReadAttribute API alone in conjunction with the safety afforded by using the templated DataModel::Decode methods. Tests: Added a TestRead unit test that exercises a pseudo end-to-end flow of a read that is sent from a client, processed by the server, and a response returned back to the client. It includes both positive and negative test-cases.
d80720a
to
0dca07b
Compare
PR #10524: Size comparison from 2377fea to d80720a 20 builds
2 builds
|
PR #10524: Size comparison from 2377fea to 0dca07b 20 builds
2 builds
12 builds
|
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.
I have a few comments inline. But for the most part, this looks great to me.
Co-authored-by: Michael Sandstedt <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
59a494e
to
2c9e1ff
Compare
2c9e1ff
to
516a6ed
Compare
PR #10524: Size comparison from 4f389d5 to 4c22e99 2 builds
20 builds
12 builds
|
4c22e99
to
a8de91c
Compare
PR #10524: Size comparison from ca22841 to a8de91c 1 build
17 builds
4 builds
2 builds
10 builds
|
PR #10524: Size comparison from ca22841 to 6edc6d1 8 builds (for k32w, p6, qpg, telink)
14 builds (for efr32, linux, mbed)
10 builds (for nrfconnect)
2 builds (for esp32)
|
Like the
TypedCommandCallback
andInvokeCommand
APIs added before,this adds a
TypedReadCallback
andReadAttribute
API that utilizescluster objects to deliver attribute data to a reader.
This utilizes the attribute type information struct that is
code-generated currently to avoid needing callers to have to specify
static schema information like the Cluster or Attribute Ids.
This also avoids the use of
gCallbackMgr
and the dynamic dispatchlogic contained within it given it's unsafe usage of reinterpret_casts, and achieves
dispatch compactly and safely within the implementation of the
ReadAttribute
API alone in conjunction with the type safety afforded by usingthe templated
DataModel::Decode
methods.Tests: Added a TestRead unit test that exercises a pseudo end-to-end
flow of a read that is sent from a client, processed by the server, and
a response returned back to the client. It includes both positive and
negative test-cases.